From c2cb511634012344e3d0fe49a037a33b12d8a98a Mon Sep 17 00:00:00 2001 From: Prasad J Pandit Date: Wed, 11 Nov 2020 18:36:36 +0530 Subject: [PATCH 1/5] hw/net/e1000e: advance desc_offset in case of null descriptor While receiving packets via e1000e_write_packet_to_guest() routine, 'desc_offset' is advanced only when RX descriptor is processed. And RX descriptor is not processed if it has NULL buffer address. This may lead to an infinite loop condition. Increament 'desc_offset' to process next descriptor in the ring to avoid infinite loop. Reported-by: Cheol-woo Myung <330cjfdn@gmail.com> Signed-off-by: Prasad J Pandit Signed-off-by: Jason Wang --- hw/net/e1000e_core.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/hw/net/e1000e_core.c b/hw/net/e1000e_core.c index d8b9e4b2f4..095c01ebc6 100644 --- a/hw/net/e1000e_core.c +++ b/hw/net/e1000e_core.c @@ -1596,13 +1596,13 @@ e1000e_write_packet_to_guest(E1000ECore *core, struct NetRxPkt *pkt, (const char *) &fcs_pad, e1000x_fcs_len(core->mac)); } } - desc_offset += desc_size; - if (desc_offset >= total_size) { - is_last = true; - } } else { /* as per intel docs; skip descriptors with null buf addr */ trace_e1000e_rx_null_descriptor(); } + desc_offset += desc_size; + if (desc_offset >= total_size) { + is_last = true; + } e1000e_write_rx_descr(core, desc, is_last ? core->rx_pkt : NULL, rss_info, do_ps ? ps_hdr_len : 0, &bastate.written); From ad6f932fe8d1fc7f37ec17c7520aec68d3ee3706 Mon Sep 17 00:00:00 2001 From: Paolo Bonzini Date: Wed, 11 Nov 2020 05:52:22 -0500 Subject: [PATCH 2/5] net: do not exit on "netdev_add help" monitor command "netdev_add help" is causing QEMU to exit because the code that invokes show_netdevs is shared between CLI and HMP processing. Move the check to the callers so that exit(0) remains only in the CLI flow. "netdev_add help" is not fixed by this patch; that is left for later work. Signed-off-by: Paolo Bonzini Signed-off-by: Jason Wang --- include/net/net.h | 1 + monitor/hmp-cmds.c | 6 ++++ net/net.c | 72 +++++++++++++++++++++++----------------------- 3 files changed, 43 insertions(+), 36 deletions(-) diff --git a/include/net/net.h b/include/net/net.h index 897b2d7595..778fc787ca 100644 --- a/include/net/net.h +++ b/include/net/net.h @@ -199,6 +199,7 @@ extern const char *host_net_devices[]; /* from net.c */ int net_client_parse(QemuOptsList *opts_list, const char *str); +void show_netdevs(void); int net_init_clients(Error **errp); void net_check_clients(void); void net_cleanup(void); diff --git a/monitor/hmp-cmds.c b/monitor/hmp-cmds.c index a6a6684df1..65d8ff4849 100644 --- a/monitor/hmp-cmds.c +++ b/monitor/hmp-cmds.c @@ -24,6 +24,7 @@ #include "qemu/option.h" #include "qemu/timer.h" #include "qemu/sockets.h" +#include "qemu/help_option.h" #include "monitor/monitor-internal.h" #include "qapi/error.h" #include "qapi/clone-visitor.h" @@ -1631,7 +1632,12 @@ void hmp_netdev_add(Monitor *mon, const QDict *qdict) { Error *err = NULL; QemuOpts *opts; + const char *type = qdict_get_try_str(qdict, "type"); + if (type && is_help_option(type)) { + show_netdevs(); + return; + } opts = qemu_opts_from_qdict(qemu_find_opts("netdev"), qdict, &err); if (err) { goto out; diff --git a/net/net.c b/net/net.c index 794c652282..6362d3077c 100644 --- a/net/net.c +++ b/net/net.c @@ -44,6 +44,7 @@ #include "qemu/config-file.h" #include "qemu/ctype.h" #include "qemu/iov.h" +#include "qemu/qemu-print.h" #include "qemu/main-loop.h" #include "qemu/option.h" #include "qapi/error.h" @@ -1025,7 +1026,7 @@ static int net_client_init1(const Netdev *netdev, bool is_netdev, Error **errp) return 0; } -static void show_netdevs(void) +void show_netdevs(void) { int idx; const char *available_netdevs[] = { @@ -1055,9 +1056,9 @@ static void show_netdevs(void) #endif }; - printf("Available netdev backend types:\n"); + qemu_printf("Available netdev backend types:\n"); for (idx = 0; idx < ARRAY_SIZE(available_netdevs); idx++) { - puts(available_netdevs[idx]); + qemu_printf("%s\n", available_netdevs[idx]); } } @@ -1068,42 +1069,35 @@ static int net_client_init(QemuOpts *opts, bool is_netdev, Error **errp) int ret = -1; Visitor *v = opts_visitor_new(opts); - const char *type = qemu_opt_get(opts, "type"); + /* Parse convenience option format ip6-net=fec0::0[/64] */ + const char *ip6_net = qemu_opt_get(opts, "ipv6-net"); - if (is_netdev && type && is_help_option(type)) { - show_netdevs(); - exit(0); - } else { - /* Parse convenience option format ip6-net=fec0::0[/64] */ - const char *ip6_net = qemu_opt_get(opts, "ipv6-net"); + if (ip6_net) { + char *prefix_addr; + unsigned long prefix_len = 64; /* Default 64bit prefix length. */ - if (ip6_net) { - char *prefix_addr; - unsigned long prefix_len = 64; /* Default 64bit prefix length. */ - - substrings = g_strsplit(ip6_net, "/", 2); - if (!substrings || !substrings[0]) { - error_setg(errp, QERR_INVALID_PARAMETER_VALUE, "ipv6-net", - "a valid IPv6 prefix"); - goto out; - } - - prefix_addr = substrings[0]; - - /* Handle user-specified prefix length. */ - if (substrings[1] && - qemu_strtoul(substrings[1], NULL, 10, &prefix_len)) - { - error_setg(errp, QERR_INVALID_PARAMETER_VALUE, - "ipv6-prefixlen", "a number"); - goto out; - } - - qemu_opt_set(opts, "ipv6-prefix", prefix_addr, &error_abort); - qemu_opt_set_number(opts, "ipv6-prefixlen", prefix_len, - &error_abort); - qemu_opt_unset(opts, "ipv6-net"); + substrings = g_strsplit(ip6_net, "/", 2); + if (!substrings || !substrings[0]) { + error_setg(errp, QERR_INVALID_PARAMETER_VALUE, "ipv6-net", + "a valid IPv6 prefix"); + goto out; } + + prefix_addr = substrings[0]; + + /* Handle user-specified prefix length. */ + if (substrings[1] && + qemu_strtoul(substrings[1], NULL, 10, &prefix_len)) + { + error_setg(errp, QERR_INVALID_PARAMETER_VALUE, + "ipv6-prefixlen", "a number"); + goto out; + } + + qemu_opt_set(opts, "ipv6-prefix", prefix_addr, &error_abort); + qemu_opt_set_number(opts, "ipv6-prefixlen", prefix_len, + &error_abort); + qemu_opt_unset(opts, "ipv6-net"); } /* Create an ID for -net if the user did not specify one */ @@ -1421,6 +1415,12 @@ static int net_init_client(void *dummy, QemuOpts *opts, Error **errp) static int net_init_netdev(void *dummy, QemuOpts *opts, Error **errp) { + const char *type = qemu_opt_get(opts, "type"); + + if (type && is_help_option(type)) { + show_netdevs(); + exit(0); + } return net_client_init(opts, true, errp); } From d2abc563e4ba4057b9cb5c04dd203b8c3a949115 Mon Sep 17 00:00:00 2001 From: Yuri Benditovich Date: Thu, 12 Nov 2020 11:46:53 +0200 Subject: [PATCH 3/5] net: purge queued rx packets on queue deletion https://bugzilla.redhat.com/show_bug.cgi?id=1829272 When deleting queue pair, purge pending RX packets if any. Example of problematic flow: 1. Bring up q35 VM with tap (vhost off) and virtio-net or e1000e 2. Run ping flood to the VM NIC ( 1 ms interval) 3. Hot unplug the NIC device (device_del) During unplug process one or more packets come, the NIC can't receive, tap disables read_poll 4. Hot plug the device (device_add) with the same netdev The tap stays with read_poll disabled and does not receive any packets anymore (tap_send never triggered) Signed-off-by: Yuri Benditovich Signed-off-by: Jason Wang --- net/net.c | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/net/net.c b/net/net.c index 6362d3077c..6a2c3d9567 100644 --- a/net/net.c +++ b/net/net.c @@ -412,10 +412,14 @@ void qemu_del_nic(NICState *nic) qemu_macaddr_set_free(&nic->conf->macaddr); - /* If this is a peer NIC and peer has already been deleted, free it now. */ - if (nic->peer_deleted) { - for (i = 0; i < queues; i++) { - qemu_free_net_client(qemu_get_subqueue(nic, i)->peer); + for (i = 0; i < queues; i++) { + NetClientState *nc = qemu_get_subqueue(nic, i); + /* If this is a peer NIC and peer has already been deleted, free it now. */ + if (nic->peer_deleted) { + qemu_free_net_client(nc->peer); + } else if (nc->peer) { + /* if there are RX packets pending, complete them */ + qemu_purge_queued_packets(nc->peer); } } From f012bec8909820390936f1cf68b3e73a2d4ac966 Mon Sep 17 00:00:00 2001 From: yuanjungong Date: Thu, 19 Nov 2020 17:25:32 +0800 Subject: [PATCH 4/5] tap: fix a memory leak Close fd before returning. Buglink: https://bugs.launchpad.net/qemu/+bug/1904486 Signed-off-by: yuanjungong Reviewed-by: Peter Maydell Signed-off-by: Jason Wang --- net/tap.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/net/tap.c b/net/tap.c index c46ff66184..fe95fa7073 100644 --- a/net/tap.c +++ b/net/tap.c @@ -817,6 +817,7 @@ int net_init_tap(const Netdev *netdev, const char *name, if (ret < 0) { error_setg_errno(errp, -ret, "%s: Can't use file descriptor %d", name, fd); + close(fd); return -1; } @@ -831,6 +832,7 @@ int net_init_tap(const Netdev *netdev, const char *name, vhostfdname, vnet_hdr, fd, &err); if (err) { error_propagate(errp, err); + close(fd); return -1; } } else if (tap->has_fds) { From 9925990d01a92564af55f6f69d0f5f59b47609b1 Mon Sep 17 00:00:00 2001 From: Keqian Zhu Date: Mon, 23 Nov 2020 14:29:54 +0800 Subject: [PATCH 5/5] net: Use correct default-path macro for downscript Fixes: 63c4db4c2e6d (net: relocate paths to helpers and scripts) Signed-off-by: Keqian Zhu Signed-off-by: Jason Wang --- net/tap.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/net/tap.c b/net/tap.c index fe95fa7073..b7512853f4 100644 --- a/net/tap.c +++ b/net/tap.c @@ -953,7 +953,8 @@ free_fail: script = default_script = get_relocated_path(DEFAULT_NETWORK_SCRIPT); } if (!downscript) { - downscript = default_downscript = get_relocated_path(DEFAULT_NETWORK_SCRIPT); + downscript = default_downscript = + get_relocated_path(DEFAULT_NETWORK_DOWN_SCRIPT); } if (tap->has_ifname) {