From 65c2ab808571dcd9322020690a63df63281a67f0 Mon Sep 17 00:00:00 2001 From: Laurent Vivier Date: Wed, 24 Jan 2024 11:29:03 +0100 Subject: igb: fix link state on resume On resume igb_vm_state_change() always calls igb_autoneg_resume() that sets link_down to false, and thus activates the link even if we have disabled it. The problem can be reproduced starting qemu in paused state (-S) and then set the link to down. When we resume the machine the link appears to be up. Reproducer: # qemu-system-x86_64 ... -device igb,netdev=netdev0,id=net0 -S {"execute": "qmp_capabilities" } {"execute": "set_link", "arguments": {"name": "net0", "up": false}} {"execute": "cont" } To fix the problem, merge the content of igb_vm_state_change() into igb_core_post_load() as e1000 does. Buglink: https://issues.redhat.com/browse/RHEL-21867 Fixes: 3a977deebe6b ("Intrdocue igb device emulation") Cc: akihiko.odaki@daynix.com Suggested-by: Akihiko Odaki Signed-off-by: Laurent Vivier Signed-off-by: Jason Wang --- hw/net/igb_core.c | 51 +++++++-------------------------------------------- hw/net/igb_core.h | 2 -- 2 files changed, 7 insertions(+), 46 deletions(-) (limited to 'hw') diff --git a/hw/net/igb_core.c b/hw/net/igb_core.c index 2a7a11a..bcd5f6c 100644 --- a/hw/net/igb_core.c +++ b/hw/net/igb_core.c @@ -161,14 +161,6 @@ igb_intmgr_timer_resume(IGBIntrDelayTimer *timer) } static void -igb_intmgr_timer_pause(IGBIntrDelayTimer *timer) -{ - if (timer->running) { - timer_del(timer->timer); - } -} - -static void igb_intrmgr_on_msix_throttling_timer(void *opaque) { IGBIntrDelayTimer *timer = opaque; @@ -213,16 +205,6 @@ igb_intrmgr_resume(IGBCore *core) } static void -igb_intrmgr_pause(IGBCore *core) -{ - int i; - - for (i = 0; i < IGB_INTR_NUM; i++) { - igb_intmgr_timer_pause(&core->eitr[i]); - } -} - -static void igb_intrmgr_reset(IGBCore *core) { int i; @@ -4290,12 +4272,6 @@ igb_core_read(IGBCore *core, hwaddr addr, unsigned size) return 0; } -static inline void -igb_autoneg_pause(IGBCore *core) -{ - timer_del(core->autoneg_timer); -} - static void igb_autoneg_resume(IGBCore *core) { @@ -4307,22 +4283,6 @@ igb_autoneg_resume(IGBCore *core) } } -static void -igb_vm_state_change(void *opaque, bool running, RunState state) -{ - IGBCore *core = opaque; - - if (running) { - trace_e1000e_vm_state_running(); - igb_intrmgr_resume(core); - igb_autoneg_resume(core); - } else { - trace_e1000e_vm_state_stopped(); - igb_autoneg_pause(core); - igb_intrmgr_pause(core); - } -} - void igb_core_pci_realize(IGBCore *core, const uint16_t *eeprom_templ, @@ -4335,8 +4295,6 @@ igb_core_pci_realize(IGBCore *core, igb_autoneg_timer, core); igb_intrmgr_pci_realize(core); - core->vmstate = qemu_add_vm_change_state_handler(igb_vm_state_change, core); - for (i = 0; i < IGB_NUM_QUEUES; i++) { net_tx_pkt_init(&core->tx[i].tx_pkt, E1000E_MAX_TX_FRAGS); } @@ -4360,8 +4318,6 @@ igb_core_pci_uninit(IGBCore *core) igb_intrmgr_pci_unint(core); - qemu_del_vm_change_state_handler(core->vmstate); - for (i = 0; i < IGB_NUM_QUEUES; i++) { net_tx_pkt_uninit(core->tx[i].tx_pkt); } @@ -4586,5 +4542,12 @@ igb_core_post_load(IGBCore *core) */ nc->link_down = (core->mac[STATUS] & E1000_STATUS_LU) == 0; + /* + * we need to restart intrmgr timers, as an older version of + * QEMU can have stopped them before migration + */ + igb_intrmgr_resume(core); + igb_autoneg_resume(core); + return 0; } diff --git a/hw/net/igb_core.h b/hw/net/igb_core.h index bf8c46f..d70b54e 100644 --- a/hw/net/igb_core.h +++ b/hw/net/igb_core.h @@ -90,8 +90,6 @@ struct IGBCore { IGBIntrDelayTimer eitr[IGB_INTR_NUM]; - VMChangeStateEntry *vmstate; - uint32_t eitr_guest_value[IGB_INTR_NUM]; uint8_t permanent_mac[ETH_ALEN]; -- cgit v1.1 From 4cadf10234989861398e19f3bb441d3861f3bb7c Mon Sep 17 00:00:00 2001 From: Laurent Vivier Date: Wed, 24 Jan 2024 11:40:06 +0100 Subject: e1000e: fix link state on resume On resume e1000e_vm_state_change() always calls e1000e_autoneg_resume() that sets link_down to false, and thus activates the link even if we have disabled it. The problem can be reproduced starting qemu in paused state (-S) and then set the link to down. When we resume the machine the link appears to be up. Reproducer: # qemu-system-x86_64 ... -device e1000e,netdev=netdev0,id=net0 -S {"execute": "qmp_capabilities" } {"execute": "set_link", "arguments": {"name": "net0", "up": false}} {"execute": "cont" } To fix the problem, merge the content of e1000e_vm_state_change() into e1000e_core_post_load() as e1000 does. Buglink: https://issues.redhat.com/browse/RHEL-21867 Fixes: 6f3fbe4ed06a ("net: Introduce e1000e device emulation") Suggested-by: Akihiko Odaki Signed-off-by: Laurent Vivier Signed-off-by: Jason Wang --- hw/net/e1000e_core.c | 60 ++++++---------------------------------------------- hw/net/e1000e_core.h | 2 -- 2 files changed, 7 insertions(+), 55 deletions(-) (limited to 'hw') diff --git a/hw/net/e1000e_core.c b/hw/net/e1000e_core.c index e324c02..3ae2a18 100644 --- a/hw/net/e1000e_core.c +++ b/hw/net/e1000e_core.c @@ -123,14 +123,6 @@ e1000e_intmgr_timer_resume(E1000IntrDelayTimer *timer) } } -static void -e1000e_intmgr_timer_pause(E1000IntrDelayTimer *timer) -{ - if (timer->running) { - timer_del(timer->timer); - } -} - static inline void e1000e_intrmgr_stop_timer(E1000IntrDelayTimer *timer) { @@ -399,24 +391,6 @@ e1000e_intrmgr_resume(E1000ECore *core) } static void -e1000e_intrmgr_pause(E1000ECore *core) -{ - int i; - - e1000e_intmgr_timer_pause(&core->radv); - e1000e_intmgr_timer_pause(&core->rdtr); - e1000e_intmgr_timer_pause(&core->raid); - e1000e_intmgr_timer_pause(&core->tidv); - e1000e_intmgr_timer_pause(&core->tadv); - - e1000e_intmgr_timer_pause(&core->itr); - - for (i = 0; i < E1000E_MSIX_VEC_NUM; i++) { - e1000e_intmgr_timer_pause(&core->eitr[i]); - } -} - -static void e1000e_intrmgr_reset(E1000ECore *core) { int i; @@ -3334,12 +3308,6 @@ e1000e_core_read(E1000ECore *core, hwaddr addr, unsigned size) return 0; } -static inline void -e1000e_autoneg_pause(E1000ECore *core) -{ - timer_del(core->autoneg_timer); -} - static void e1000e_autoneg_resume(E1000ECore *core) { @@ -3351,22 +3319,6 @@ e1000e_autoneg_resume(E1000ECore *core) } } -static void -e1000e_vm_state_change(void *opaque, bool running, RunState state) -{ - E1000ECore *core = opaque; - - if (running) { - trace_e1000e_vm_state_running(); - e1000e_intrmgr_resume(core); - e1000e_autoneg_resume(core); - } else { - trace_e1000e_vm_state_stopped(); - e1000e_autoneg_pause(core); - e1000e_intrmgr_pause(core); - } -} - void e1000e_core_pci_realize(E1000ECore *core, const uint16_t *eeprom_templ, @@ -3379,9 +3331,6 @@ e1000e_core_pci_realize(E1000ECore *core, e1000e_autoneg_timer, core); e1000e_intrmgr_pci_realize(core); - core->vmstate = - qemu_add_vm_change_state_handler(e1000e_vm_state_change, core); - for (i = 0; i < E1000E_NUM_QUEUES; i++) { net_tx_pkt_init(&core->tx[i].tx_pkt, E1000E_MAX_TX_FRAGS); } @@ -3405,8 +3354,6 @@ e1000e_core_pci_uninit(E1000ECore *core) e1000e_intrmgr_pci_unint(core); - qemu_del_vm_change_state_handler(core->vmstate); - for (i = 0; i < E1000E_NUM_QUEUES; i++) { net_tx_pkt_uninit(core->tx[i].tx_pkt); } @@ -3576,5 +3523,12 @@ e1000e_core_post_load(E1000ECore *core) */ nc->link_down = (core->mac[STATUS] & E1000_STATUS_LU) == 0; + /* + * we need to restart intrmgr timers, as an older version of + * QEMU can have stopped them before migration + */ + e1000e_intrmgr_resume(core); + e1000e_autoneg_resume(core); + return 0; } diff --git a/hw/net/e1000e_core.h b/hw/net/e1000e_core.h index 66b025c..01510ca 100644 --- a/hw/net/e1000e_core.h +++ b/hw/net/e1000e_core.h @@ -98,8 +98,6 @@ struct E1000Core { E1000IntrDelayTimer eitr[E1000E_MSIX_VEC_NUM]; - VMChangeStateEntry *vmstate; - uint32_t itr_guest_value; uint32_t eitr_guest_value[E1000E_MSIX_VEC_NUM]; -- cgit v1.1 From 6a5287ce80470bb8df95901d73ee779a64e70c3a Mon Sep 17 00:00:00 2001 From: Nick Briggs Date: Thu, 1 Feb 2024 10:11:17 -0800 Subject: Avoid unaligned fetch in ladr_match() There is no guarantee that the PCNetState is allocated such that csr[8] is allocated on an 8-byte boundary. Since not all hosts are capable of unaligned fetches the 16-bit elements need to be fetched individually to avoid a potential fault. Closes issue #2143 Resolves: https://gitlab.com/qemu-project/qemu/-/issues/2143 Signed-off-by: Nick Briggs Reviewed-by: Peter Maydell Signed-off-by: Jason Wang --- hw/net/pcnet.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'hw') diff --git a/hw/net/pcnet.c b/hw/net/pcnet.c index 494eab8..ad675ab 100644 --- a/hw/net/pcnet.c +++ b/hw/net/pcnet.c @@ -632,7 +632,7 @@ static inline int ladr_match(PCNetState *s, const uint8_t *buf, int size) { struct qemu_ether_header *hdr = (void *)buf; if ((*(hdr->ether_dhost)&0x01) && - ((uint64_t *)&s->csr[8])[0] != 0LL) { + (s->csr[8] | s->csr[9] | s->csr[10] | s->csr[11]) != 0) { uint8_t ladr[8] = { s->csr[8] & 0xff, s->csr[8] >> 8, s->csr[9] & 0xff, s->csr[9] >> 8, -- cgit v1.1 From 6b230b7dfcd8123a902e41cd313714b5a57dcac4 Mon Sep 17 00:00:00 2001 From: Andrew Melnychenko Date: Mon, 5 Feb 2024 18:54:33 +0200 Subject: virtio-net: Added property to load eBPF RSS with fds. eBPF RSS program and maps may now be passed during initialization. Initially was implemented for libvirt to launch qemu without permissions, and initialized eBPF program through the helper. Signed-off-by: Andrew Melnychenko Signed-off-by: Jason Wang --- hw/net/virtio-net.c | 54 +++++++++++++++++++++++++++++++++++++++++++++++------ 1 file changed, 48 insertions(+), 6 deletions(-) (limited to 'hw') diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c index a3c711b..403a693 100644 --- a/hw/net/virtio-net.c +++ b/hw/net/virtio-net.c @@ -42,6 +42,7 @@ #include "sysemu/sysemu.h" #include "trace.h" #include "monitor/qdev.h" +#include "monitor/monitor.h" #include "hw/pci/pci_device.h" #include "net_rx_pkt.h" #include "hw/virtio/vhost.h" @@ -1328,14 +1329,53 @@ static void virtio_net_detach_epbf_rss(VirtIONet *n) virtio_net_attach_ebpf_to_backend(n->nic, -1); } -static bool virtio_net_load_ebpf(VirtIONet *n) +static bool virtio_net_load_ebpf_fds(VirtIONet *n, Error **errp) { - if (!virtio_net_attach_ebpf_to_backend(n->nic, -1)) { - /* backend doesn't support steering ebpf */ - return false; + int fds[EBPF_RSS_MAX_FDS] = { [0 ... EBPF_RSS_MAX_FDS - 1] = -1}; + int ret = true; + int i = 0; + + ERRP_GUARD(); + + if (n->nr_ebpf_rss_fds != EBPF_RSS_MAX_FDS) { + error_setg(errp, + "Expected %d file descriptors but got %d", + EBPF_RSS_MAX_FDS, n->nr_ebpf_rss_fds); + return false; + } + + for (i = 0; i < n->nr_ebpf_rss_fds; i++) { + fds[i] = monitor_fd_param(monitor_cur(), n->ebpf_rss_fds[i], errp); + if (*errp) { + ret = false; + goto exit; + } + } + + ret = ebpf_rss_load_fds(&n->ebpf_rss, fds[0], fds[1], fds[2], fds[3]); + +exit: + if (!ret || *errp) { + for (i = 0; i < n->nr_ebpf_rss_fds && fds[i] != -1; i++) { + close(fds[i]); + } } - return ebpf_rss_load(&n->ebpf_rss); + return ret; +} + +static bool virtio_net_load_ebpf(VirtIONet *n, Error **errp) +{ + bool ret = false; + + if (virtio_net_attach_ebpf_to_backend(n->nic, -1)) { + if (!(n->ebpf_rss_fds + && virtio_net_load_ebpf_fds(n, errp))) { + ret = ebpf_rss_load(&n->ebpf_rss); + } + } + + return ret; } static void virtio_net_unload_ebpf(VirtIONet *n) @@ -3768,7 +3808,7 @@ static void virtio_net_device_realize(DeviceState *dev, Error **errp) net_rx_pkt_init(&n->rx_pkt); if (virtio_has_feature(n->host_features, VIRTIO_NET_F_RSS)) { - virtio_net_load_ebpf(n); + virtio_net_load_ebpf(n, errp); } } @@ -3930,6 +3970,8 @@ static Property virtio_net_properties[] = { VIRTIO_NET_F_RSS, false), DEFINE_PROP_BIT64("hash", VirtIONet, host_features, VIRTIO_NET_F_HASH_REPORT, false), + DEFINE_PROP_ARRAY("ebpf-rss-fds", VirtIONet, nr_ebpf_rss_fds, + ebpf_rss_fds, qdev_prop_string, char*), DEFINE_PROP_BIT64("guest_rsc_ext", VirtIONet, host_features, VIRTIO_NET_F_RSC_EXT, false), DEFINE_PROP_UINT32("rsc_interval", VirtIONet, rsc_timeout, -- cgit v1.1