| Message ID | 20250415074341.12461-4-minhquangbui99@gmail.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | virtio-net: disable delayed refill when pausing rx | expand |
On Tue, 15 Apr 2025 14:43:41 +0700 Bui Quang Minh wrote: > +def setup_xsk(cfg, xdp_queue_id = 0) -> bkg: > + # Probe for support > + xdp = cmd(f'{cfg.net_lib_dir / "xdp_helper"} - -', fail=False) > + if xdp.ret == 255: > + raise KsftSkipEx('AF_XDP unsupported') > + elif xdp.ret > 0: > + raise KsftFailEx('unable to create AF_XDP socket') > + > + return bkg(f'{cfg.net_lib_dir / "xdp_helper"} {cfg.ifindex} {xdp_queue_id}', > + ksft_wait=3) > + > +def check_xdp_bind(cfg): > + ip(f"link set dev %s xdp obj %s sec xdp" % > + (cfg.ifname, cfg.net_lib_dir / "xdp_dummy.bpf.o")) > + ip(f"link set dev %s xdp off" % cfg.ifname) > + > +def check_rx_resize(cfg, queue_size = 128): > + rx_ring = _get_rx_ring_entries(cfg) > + ethtool(f"-G %s rx %d" % (cfg.ifname, queue_size)) > + ethtool(f"-G %s rx %d" % (cfg.ifname, rx_ring)) Unfortunately this doesn't work on a basic QEMU setup: # ethtool -G eth0 rx 128 [ 15.680655][ T287] virtio_net virtio2 eth0: resize rx fail: rx queue index: 0 err: -2 netlink error: No such file or directory Is there a way to enable more capable virtio_net with QEMU?
On 4/16/25 11:27, Jakub Kicinski wrote: > On Tue, 15 Apr 2025 14:43:41 +0700 Bui Quang Minh wrote: >> +def setup_xsk(cfg, xdp_queue_id = 0) -> bkg: >> + # Probe for support >> + xdp = cmd(f'{cfg.net_lib_dir / "xdp_helper"} - -', fail=False) >> + if xdp.ret == 255: >> + raise KsftSkipEx('AF_XDP unsupported') >> + elif xdp.ret > 0: >> + raise KsftFailEx('unable to create AF_XDP socket') >> + >> + return bkg(f'{cfg.net_lib_dir / "xdp_helper"} {cfg.ifindex} {xdp_queue_id}', >> + ksft_wait=3) >> + >> +def check_xdp_bind(cfg): >> + ip(f"link set dev %s xdp obj %s sec xdp" % >> + (cfg.ifname, cfg.net_lib_dir / "xdp_dummy.bpf.o")) >> + ip(f"link set dev %s xdp off" % cfg.ifname) >> + >> +def check_rx_resize(cfg, queue_size = 128): >> + rx_ring = _get_rx_ring_entries(cfg) >> + ethtool(f"-G %s rx %d" % (cfg.ifname, queue_size)) >> + ethtool(f"-G %s rx %d" % (cfg.ifname, rx_ring)) > Unfortunately this doesn't work on a basic QEMU setup: > > # ethtool -G eth0 rx 128 > [ 15.680655][ T287] virtio_net virtio2 eth0: resize rx fail: rx queue index: 0 err: -2 > netlink error: No such file or directory > > Is there a way to enable more capable virtio_net with QEMU? I guess that virtio-pci-legacy is used in your setup. Here is how I setup virtio-net with Qemu -netdev tap,id=hostnet1,vhost=on,script=$NETWORK_SCRIPT,downscript=no \ -device virtio-net-pci,netdev=hostnet1,iommu_platform=on,disable-legacy=on \ The iommu_platform=on is necessary to make vring use dma API which is a requirement to enable xsk_pool in virtio-net (XDP socket will be in zerocopy mode for this case). Otherwise, the XDP socket will fallback to copy mode, xsk_pool is not enabled in virtio-net that makes the probability to reproduce bug to be very small. Currently, when you don't have iommu_platform=on, you can pass the test even before the fix, so I think I will try to harden the selftest to make it return skip in this case. Thanks, Quang Minh.
On Wed, Apr 16, 2025 at 2:54 PM Bui Quang Minh <minhquangbui99@gmail.com> wrote: > > On 4/16/25 11:27, Jakub Kicinski wrote: > > On Tue, 15 Apr 2025 14:43:41 +0700 Bui Quang Minh wrote: > >> +def setup_xsk(cfg, xdp_queue_id = 0) -> bkg: > >> + # Probe for support > >> + xdp = cmd(f'{cfg.net_lib_dir / "xdp_helper"} - -', fail=False) > >> + if xdp.ret == 255: > >> + raise KsftSkipEx('AF_XDP unsupported') > >> + elif xdp.ret > 0: > >> + raise KsftFailEx('unable to create AF_XDP socket') > >> + > >> + return bkg(f'{cfg.net_lib_dir / "xdp_helper"} {cfg.ifindex} {xdp_queue_id}', > >> + ksft_wait=3) > >> + > >> +def check_xdp_bind(cfg): > >> + ip(f"link set dev %s xdp obj %s sec xdp" % > >> + (cfg.ifname, cfg.net_lib_dir / "xdp_dummy.bpf.o")) > >> + ip(f"link set dev %s xdp off" % cfg.ifname) > >> + > >> +def check_rx_resize(cfg, queue_size = 128): > >> + rx_ring = _get_rx_ring_entries(cfg) > >> + ethtool(f"-G %s rx %d" % (cfg.ifname, queue_size)) > >> + ethtool(f"-G %s rx %d" % (cfg.ifname, rx_ring)) > > Unfortunately this doesn't work on a basic QEMU setup: > > > > # ethtool -G eth0 rx 128 > > [ 15.680655][ T287] virtio_net virtio2 eth0: resize rx fail: rx queue index: 0 err: -2 > > netlink error: No such file or directory > > > > Is there a way to enable more capable virtio_net with QEMU? What's the qemu command line and version? Resize depends on queue_reset which should be supported from Qemu 7.2 > > I guess that virtio-pci-legacy is used in your setup. Note that modern devices are used by default. > > Here is how I setup virtio-net with Qemu > > -netdev tap,id=hostnet1,vhost=on,script=$NETWORK_SCRIPT,downscript=no \ > -device > virtio-net-pci,netdev=hostnet1,iommu_platform=on,disable-legacy=on \ > > The iommu_platform=on is necessary to make vring use dma API which is a > requirement to enable xsk_pool in virtio-net (XDP socket will be in > zerocopy mode for this case). Otherwise, the XDP socket will fallback to > copy mode, xsk_pool is not enabled in virtio-net that makes the > probability to reproduce bug to be very small. Currently, when you don't > have iommu_platform=on, you can pass the test even before the fix, so I > think I will try to harden the selftest to make it return skip in this case. I would like to keep the resize test as it doesn't require iommu_platform. Thanks > > Thanks, > Quang Minh. >
On 4/16/25 14:46, Jason Wang wrote: > On Wed, Apr 16, 2025 at 2:54 PM Bui Quang Minh <minhquangbui99@gmail.com> wrote: >> On 4/16/25 11:27, Jakub Kicinski wrote: >>> On Tue, 15 Apr 2025 14:43:41 +0700 Bui Quang Minh wrote: >>>> +def setup_xsk(cfg, xdp_queue_id = 0) -> bkg: >>>> + # Probe for support >>>> + xdp = cmd(f'{cfg.net_lib_dir / "xdp_helper"} - -', fail=False) >>>> + if xdp.ret == 255: >>>> + raise KsftSkipEx('AF_XDP unsupported') >>>> + elif xdp.ret > 0: >>>> + raise KsftFailEx('unable to create AF_XDP socket') >>>> + >>>> + return bkg(f'{cfg.net_lib_dir / "xdp_helper"} {cfg.ifindex} {xdp_queue_id}', >>>> + ksft_wait=3) >>>> + >>>> +def check_xdp_bind(cfg): >>>> + ip(f"link set dev %s xdp obj %s sec xdp" % >>>> + (cfg.ifname, cfg.net_lib_dir / "xdp_dummy.bpf.o")) >>>> + ip(f"link set dev %s xdp off" % cfg.ifname) >>>> + >>>> +def check_rx_resize(cfg, queue_size = 128): >>>> + rx_ring = _get_rx_ring_entries(cfg) >>>> + ethtool(f"-G %s rx %d" % (cfg.ifname, queue_size)) >>>> + ethtool(f"-G %s rx %d" % (cfg.ifname, rx_ring)) >>> Unfortunately this doesn't work on a basic QEMU setup: >>> >>> # ethtool -G eth0 rx 128 >>> [ 15.680655][ T287] virtio_net virtio2 eth0: resize rx fail: rx queue index: 0 err: -2 >>> netlink error: No such file or directory >>> >>> Is there a way to enable more capable virtio_net with QEMU? > What's the qemu command line and version? > > Resize depends on queue_reset which should be supported from Qemu 7.2 > >> I guess that virtio-pci-legacy is used in your setup. > Note that modern devices are used by default. > >> Here is how I setup virtio-net with Qemu >> >> -netdev tap,id=hostnet1,vhost=on,script=$NETWORK_SCRIPT,downscript=no \ >> -device >> virtio-net-pci,netdev=hostnet1,iommu_platform=on,disable-legacy=on \ >> >> The iommu_platform=on is necessary to make vring use dma API which is a >> requirement to enable xsk_pool in virtio-net (XDP socket will be in >> zerocopy mode for this case). Otherwise, the XDP socket will fallback to >> copy mode, xsk_pool is not enabled in virtio-net that makes the >> probability to reproduce bug to be very small. Currently, when you don't >> have iommu_platform=on, you can pass the test even before the fix, so I >> think I will try to harden the selftest to make it return skip in this case. > I would like to keep the resize test as it doesn't require iommu_platform. Okay, in next version I will force the XDP socket binding to zerocopy to setup xsk_pool. When the binding fails, 2 tests still run but I will print a warning message. Thanks, Quang Minh.
On Wed, 16 Apr 2025 15:46:42 +0800 Jason Wang wrote: > On Wed, Apr 16, 2025 at 2:54 PM Bui Quang Minh <minhquangbui99@gmail.com> wrote: > > On 4/16/25 11:27, Jakub Kicinski wrote: > > > Unfortunately this doesn't work on a basic QEMU setup: > > > > > > # ethtool -G eth0 rx 128 > > > [ 15.680655][ T287] virtio_net virtio2 eth0: resize rx fail: rx queue index: 0 err: -2 > > > netlink error: No such file or directory > > > > > > Is there a way to enable more capable virtio_net with QEMU? > > What's the qemu command line and version? > > Resize depends on queue_reset which should be supported from Qemu 7.2 I'm using virtme-ng with --net loop and: QEMU emulator version 9.1.3 (qemu-9.1.3-2.fc41) --net loop resolves to: -device virtio-net-device,netdev=n0 \ -netdev hubport,id=n0,hubid=0 \ -device virtio-net-device,netdev=n1 \ -netdev hubport,id=n1,hubid=0 > > I guess that virtio-pci-legacy is used in your setup. > > Note that modern devices are used by default. > > > > > Here is how I setup virtio-net with Qemu > > > > -netdev tap,id=hostnet1,vhost=on,script=$NETWORK_SCRIPT,downscript=no \ > > -device > > virtio-net-pci,netdev=hostnet1,iommu_platform=on,disable-legacy=on \ That works! I rejigged the CI, for posterity I used two times: -device virtio-net-pci,netdev=n0,iommu_platform=on,disable-legacy=on,mq=on,vectors=18 -netdev tap,id=n0,ifname=tap4,vhost=on,script=no,downscript=no,queues=8 and then manually bridged the taps together on the hypervisor side. > > The iommu_platform=on is necessary to make vring use dma API which is a > > requirement to enable xsk_pool in virtio-net (XDP socket will be in > > zerocopy mode for this case). Otherwise, the XDP socket will fallback to > > copy mode, xsk_pool is not enabled in virtio-net that makes the > > probability to reproduce bug to be very small. Currently, when you don't > > have iommu_platform=on, you can pass the test even before the fix, so I > > think I will try to harden the selftest to make it return skip in this case. > > I would like to keep the resize test as it doesn't require iommu_platform. Sounds good but lets just add them to the drivers/net/hw directory. I don't think there's anything virtio specific in the test itself? Right now drivers/net/virtio_net has a test which expects to see both netdevs in the VM, while drivers / Python based tests expect to have the env prepared where only one end is on the local machine, and the other is accessible over SSH or in another netns. So it's a bit painful to marry the two kinds of tests in the CI. At least our netdev CI does not know how to figure this out :( It preps the env and then runs the whole kselftest TARGET in the same setup.
diff --git a/tools/testing/selftests/Makefile b/tools/testing/selftests/Makefile index c77c8c8e3d9b..0a6b096f98b7 100644 --- a/tools/testing/selftests/Makefile +++ b/tools/testing/selftests/Makefile @@ -130,7 +130,7 @@ TARGETS_HOTPLUG = cpu-hotplug TARGETS_HOTPLUG += memory-hotplug # Networking tests want the net/lib target, include it automatically -ifneq ($(filter net drivers/net drivers/net/hw,$(TARGETS)),) +ifneq ($(filter net drivers/net drivers/net/hw drivers/net/virtio_net,$(TARGETS)),) ifeq ($(filter net/lib,$(TARGETS)),) INSTALL_DEP_TARGETS := net/lib endif diff --git a/tools/testing/selftests/drivers/net/virtio_net/Makefile b/tools/testing/selftests/drivers/net/virtio_net/Makefile index 7ec7cd3ab2cc..82adda96ef15 100644 --- a/tools/testing/selftests/drivers/net/virtio_net/Makefile +++ b/tools/testing/selftests/drivers/net/virtio_net/Makefile @@ -1,6 +1,7 @@ # SPDX-License-Identifier: GPL-2.0+ OR MIT TEST_PROGS = basic_features.sh \ + xsk_pool.py \ # TEST_FILES = \ @@ -8,6 +9,7 @@ TEST_FILES = \ # TEST_INCLUDES = \ + $(wildcard lib/py/*.py ../lib/py/*.py) \ ../../../net/forwarding/lib.sh \ ../../../net/lib.sh \ # diff --git a/tools/testing/selftests/drivers/net/virtio_net/config b/tools/testing/selftests/drivers/net/virtio_net/config index bcf7555eaffe..12e8caa22613 100644 --- a/tools/testing/selftests/drivers/net/virtio_net/config +++ b/tools/testing/selftests/drivers/net/virtio_net/config @@ -6,3 +6,4 @@ CONFIG_NET_L3_MASTER_DEV=y CONFIG_NET_VRF=m CONFIG_VIRTIO_DEBUG=y CONFIG_VIRTIO_NET=y +CONFIG_XDP_SOCKETS=y diff --git a/tools/testing/selftests/drivers/net/virtio_net/lib/py/__init__.py b/tools/testing/selftests/drivers/net/virtio_net/lib/py/__init__.py new file mode 100644 index 000000000000..b582885786f5 --- /dev/null +++ b/tools/testing/selftests/drivers/net/virtio_net/lib/py/__init__.py @@ -0,0 +1,16 @@ +# SPDX-License-Identifier: GPL-2.0 + +import sys +from pathlib import Path + +KSFT_DIR = (Path(__file__).parent / "../../../../..").resolve() + +try: + sys.path.append(KSFT_DIR.as_posix()) + from net.lib.py import * + from drivers.net.lib.py import * +except ModuleNotFoundError as e: + ksft_pr("Failed importing `net` library from kernel sources") + ksft_pr(str(e)) + ktap_result(True, comment="SKIP") + sys.exit(4) diff --git a/tools/testing/selftests/drivers/net/virtio_net/xsk_pool.py b/tools/testing/selftests/drivers/net/virtio_net/xsk_pool.py new file mode 100755 index 000000000000..3f80afd97d4e --- /dev/null +++ b/tools/testing/selftests/drivers/net/virtio_net/xsk_pool.py @@ -0,0 +1,52 @@ +#!/usr/bin/env python3 +# SPDX-License-Identifier: GPL-2.0 + +# This is intended to be run on a virtio-net guest interface. +# The test binds the XDP socket to the interface without setting +# the fill ring to trigger delayed refill_work. This helps to +# make it easier to reproduce the deadlock when XDP program, +# XDP socket bind/unbind, rx ring resize race with refill_work on +# the buggy kernel. + +from lib.py import ksft_exit, ksft_run +from lib.py import KsftSkipEx, KsftFailEx +from lib.py import NetDrvEnv +from lib.py import bkg, ip, cmd, ethtool +import re + +def _get_rx_ring_entries(cfg): + output = ethtool(f"-g {cfg.ifname}").stdout + values = re.findall(r'RX:\s+(\d+)', output) + return int(values[1]) + +def setup_xsk(cfg, xdp_queue_id = 0) -> bkg: + # Probe for support + xdp = cmd(f'{cfg.net_lib_dir / "xdp_helper"} - -', fail=False) + if xdp.ret == 255: + raise KsftSkipEx('AF_XDP unsupported') + elif xdp.ret > 0: + raise KsftFailEx('unable to create AF_XDP socket') + + return bkg(f'{cfg.net_lib_dir / "xdp_helper"} {cfg.ifindex} {xdp_queue_id}', + ksft_wait=3) + +def check_xdp_bind(cfg): + ip(f"link set dev %s xdp obj %s sec xdp" % + (cfg.ifname, cfg.net_lib_dir / "xdp_dummy.bpf.o")) + ip(f"link set dev %s xdp off" % cfg.ifname) + +def check_rx_resize(cfg, queue_size = 128): + rx_ring = _get_rx_ring_entries(cfg) + ethtool(f"-G %s rx %d" % (cfg.ifname, queue_size)) + ethtool(f"-G %s rx %d" % (cfg.ifname, rx_ring)) + +def main(): + with NetDrvEnv(__file__, nsim_test=False) as cfg: + xsk_bkg = setup_xsk(cfg) + + ksft_run([check_xdp_bind, check_rx_resize], + args=(cfg, )) + ksft_exit() + +if __name__ == "__main__": + main()
The selftest reproduces the deadlock scenario when binding/unbinding XDP program, XDP socket, rx ring resize on virtio_net interface. Signed-off-by: Bui Quang Minh <minhquangbui99@gmail.com> --- tools/testing/selftests/Makefile | 2 +- .../selftests/drivers/net/virtio_net/Makefile | 2 + .../selftests/drivers/net/virtio_net/config | 1 + .../drivers/net/virtio_net/lib/py/__init__.py | 16 ++++++ .../drivers/net/virtio_net/xsk_pool.py | 52 +++++++++++++++++++ 5 files changed, 72 insertions(+), 1 deletion(-) create mode 100644 tools/testing/selftests/drivers/net/virtio_net/lib/py/__init__.py create mode 100755 tools/testing/selftests/drivers/net/virtio_net/xsk_pool.py