* Re: [PATCH net v2 2/2] selftests: drv-net: test XDP, HDS auto and the ioctl path
2025-02-21 2:51 ` [PATCH net v2 2/2] selftests: drv-net: test XDP, HDS auto and the ioctl path Jakub Kicinski
@ 2025-02-21 16:07 ` Stanislav Fomichev
2025-02-23 12:32 ` Taehee Yoo
1 sibling, 0 replies; 3+ messages in thread
From: Stanislav Fomichev @ 2025-02-21 16:07 UTC (permalink / raw)
To: Jakub Kicinski
Cc: davem, netdev, edumazet, pabeni, andrew+netdev, horms, dxu, shuah,
hawk, petrm, willemb, jstancek, linux-kselftest
On 02/20, Jakub Kicinski wrote:
> Test XDP and HDS interaction. While at it add a test for using the IOCTL,
> as that turned out to be the real culprit.
>
> Testing bnxt:
>
> # NETIF=eth0 ./ksft-net-drv/drivers/net/hds.py
> KTAP version 1
> 1..12
> ok 1 hds.get_hds
> ok 2 hds.get_hds_thresh
> ok 3 hds.set_hds_disable # SKIP disabling of HDS not supported by the device
> ok 4 hds.set_hds_enable
> ok 5 hds.set_hds_thresh_zero
> ok 6 hds.set_hds_thresh_max
> ok 7 hds.set_hds_thresh_gt
> ok 8 hds.set_xdp
> ok 9 hds.enabled_set_xdp
> ok 10 hds.ioctl
> ok 11 hds.ioctl_set_xdp
> ok 12 hds.ioctl_enabled_set_xdp
> # Totals: pass:11 fail:0 xfail:0 xpass:0 skip:1 error:0
>
> and netdevsim:
>
> # ./ksft-net-drv/drivers/net/hds.py
> KTAP version 1
> 1..12
> ok 1 hds.get_hds
> ok 2 hds.get_hds_thresh
> ok 3 hds.set_hds_disable
> ok 4 hds.set_hds_enable
> ok 5 hds.set_hds_thresh_zero
> ok 6 hds.set_hds_thresh_max
> ok 7 hds.set_hds_thresh_gt
> ok 8 hds.set_xdp
> ok 9 hds.enabled_set_xdp
> ok 10 hds.ioctl
> ok 11 hds.ioctl_set_xdp
> ok 12 hds.ioctl_enabled_set_xdp
> # Totals: pass:12 fail:0 xfail:0 xpass:0 skip:0 error:0
>
> Netdevsim needs a sane default for tx/rx ring size.
>
> ethtool 6.11 is needed for the --disable-netlink option.
>
> Signed-off-by: Jakub Kicinski <kuba@kernel.org>
Acked-by: Stanislav Fomichev <sdf@fomichev.me>
> ---
> Since this is targeting net there is no cfg.rpath(),
> I'll follow up once in net-next.
>
> v2:
> - add the ioctl tests
> - factor out some common logic
>
> CC: shuah@kernel.org
> CC: hawk@kernel.org
> CC: petrm@nvidia.com
> CC: willemb@google.com
> CC: jstancek@redhat.com
> CC: linux-kselftest@vger.kernel.org
> ---
> tools/testing/selftests/net/lib/Makefile | 3 +
> drivers/net/netdevsim/ethtool.c | 2 +
> .../testing/selftests/net/lib/xdp_dummy.bpf.c | 13 ++
> tools/testing/selftests/drivers/net/hds.py | 145 +++++++++++++++++-
> 4 files changed, 160 insertions(+), 3 deletions(-)
> create mode 100644 tools/testing/selftests/net/lib/xdp_dummy.bpf.c
>
> diff --git a/tools/testing/selftests/net/lib/Makefile b/tools/testing/selftests/net/lib/Makefile
> index bc6b6762baf3..c22623b9a2a5 100644
> --- a/tools/testing/selftests/net/lib/Makefile
> +++ b/tools/testing/selftests/net/lib/Makefile
> @@ -9,7 +9,10 @@ TEST_FILES := ../../../../../Documentation/netlink/specs
> TEST_FILES += ../../../../net/ynl
>
> TEST_GEN_FILES += csum
> +TEST_GEN_FILES += $(patsubst %.c,%.o,$(wildcard *.bpf.c))
>
> TEST_INCLUDES := $(wildcard py/*.py sh/*.sh)
>
> include ../../lib.mk
> +
> +include ../bpf.mk
> diff --git a/drivers/net/netdevsim/ethtool.c b/drivers/net/netdevsim/ethtool.c
> index 5c80fbee7913..7ab358616e03 100644
> --- a/drivers/net/netdevsim/ethtool.c
> +++ b/drivers/net/netdevsim/ethtool.c
> @@ -184,9 +184,11 @@ static const struct ethtool_ops nsim_ethtool_ops = {
>
> static void nsim_ethtool_ring_init(struct netdevsim *ns)
> {
> + ns->ethtool.ring.rx_pending = 512;
> ns->ethtool.ring.rx_max_pending = 4096;
> ns->ethtool.ring.rx_jumbo_max_pending = 4096;
> ns->ethtool.ring.rx_mini_max_pending = 4096;
> + ns->ethtool.ring.tx_pending = 512;
> ns->ethtool.ring.tx_max_pending = 4096;
> }
>
> diff --git a/tools/testing/selftests/net/lib/xdp_dummy.bpf.c b/tools/testing/selftests/net/lib/xdp_dummy.bpf.c
> new file mode 100644
> index 000000000000..d988b2e0cee8
> --- /dev/null
> +++ b/tools/testing/selftests/net/lib/xdp_dummy.bpf.c
> @@ -0,0 +1,13 @@
> +// SPDX-License-Identifier: GPL-2.0
> +
> +#define KBUILD_MODNAME "xdp_dummy"
> +#include <linux/bpf.h>
> +#include <bpf/bpf_helpers.h>
> +
> +SEC("xdp")
> +int xdp_dummy_prog(struct xdp_md *ctx)
> +{
> + return XDP_PASS;
> +}
> +
> +char _license[] SEC("license") = "GPL";
> diff --git a/tools/testing/selftests/drivers/net/hds.py b/tools/testing/selftests/drivers/net/hds.py
> index 394971b25c0b..90807b21a6eb 100755
> --- a/tools/testing/selftests/drivers/net/hds.py
> +++ b/tools/testing/selftests/drivers/net/hds.py
> @@ -2,17 +2,54 @@
> # SPDX-License-Identifier: GPL-2.0
>
> import errno
> +import os
> from lib.py import ksft_run, ksft_exit, ksft_eq, ksft_raises, KsftSkipEx
> -from lib.py import EthtoolFamily, NlError
> +from lib.py import CmdExitFailure, EthtoolFamily, NlError
> from lib.py import NetDrvEnv
> +from lib.py import defer, ethtool, ip
>
> -def get_hds(cfg, netnl) -> None:
> +
> +def _get_hds_mode(cfg, netnl) -> str:
> try:
> rings = netnl.rings_get({'header': {'dev-index': cfg.ifindex}})
> except NlError as e:
> raise KsftSkipEx('ring-get not supported by device')
> if 'tcp-data-split' not in rings:
> raise KsftSkipEx('tcp-data-split not supported by device')
> + return rings['tcp-data-split']
> +
> +
> +def _xdp_onoff(cfg):
> + test_dir = os.path.dirname(os.path.realpath(__file__))
> + prog = test_dir + "/../../net/lib/xdp_dummy.bpf.o"
> + ip(f"link set dev %s xdp obj %s sec xdp" %
> + (cfg.ifname, prog))
> + ip(f"link set dev %s xdp off" % cfg.ifname)
nit: any reason you're not using {format} strings here?
ip(f"link set dev {cfg.ifname} xdp obj {prog} sec xdp")
ip(f"link set dev {cfg.ifname} xdp off")
^ permalink raw reply [flat|nested] 3+ messages in thread* Re: [PATCH net v2 2/2] selftests: drv-net: test XDP, HDS auto and the ioctl path
2025-02-21 2:51 ` [PATCH net v2 2/2] selftests: drv-net: test XDP, HDS auto and the ioctl path Jakub Kicinski
2025-02-21 16:07 ` Stanislav Fomichev
@ 2025-02-23 12:32 ` Taehee Yoo
1 sibling, 0 replies; 3+ messages in thread
From: Taehee Yoo @ 2025-02-23 12:32 UTC (permalink / raw)
To: Jakub Kicinski
Cc: davem, netdev, edumazet, pabeni, andrew+netdev, horms, dxu, shuah,
hawk, petrm, willemb, jstancek, linux-kselftest
On Fri, Feb 21, 2025 at 11:53 AM Jakub Kicinski <kuba@kernel.org> wrote:
>
> Test XDP and HDS interaction. While at it add a test for using the IOCTL,
> as that turned out to be the real culprit.
>
> Testing bnxt:
>
> # NETIF=eth0 ./ksft-net-drv/drivers/net/hds.py
> KTAP version 1
> 1..12
> ok 1 hds.get_hds
> ok 2 hds.get_hds_thresh
> ok 3 hds.set_hds_disable # SKIP disabling of HDS not supported by the device
> ok 4 hds.set_hds_enable
> ok 5 hds.set_hds_thresh_zero
> ok 6 hds.set_hds_thresh_max
> ok 7 hds.set_hds_thresh_gt
> ok 8 hds.set_xdp
> ok 9 hds.enabled_set_xdp
> ok 10 hds.ioctl
> ok 11 hds.ioctl_set_xdp
> ok 12 hds.ioctl_enabled_set_xdp
> # Totals: pass:11 fail:0 xfail:0 xpass:0 skip:1 error:0
>
> and netdevsim:
>
> # ./ksft-net-drv/drivers/net/hds.py
> KTAP version 1
> 1..12
> ok 1 hds.get_hds
> ok 2 hds.get_hds_thresh
> ok 3 hds.set_hds_disable
> ok 4 hds.set_hds_enable
> ok 5 hds.set_hds_thresh_zero
> ok 6 hds.set_hds_thresh_max
> ok 7 hds.set_hds_thresh_gt
> ok 8 hds.set_xdp
> ok 9 hds.enabled_set_xdp
> ok 10 hds.ioctl
> ok 11 hds.ioctl_set_xdp
> ok 12 hds.ioctl_enabled_set_xdp
> # Totals: pass:12 fail:0 xfail:0 xpass:0 skip:0 error:0
>
> Netdevsim needs a sane default for tx/rx ring size.
>
> ethtool 6.11 is needed for the --disable-netlink option.
>
> Signed-off-by: Jakub Kicinski <kuba@kernel.org>
> ---
Tested-by: Taehee Yoo <ap420073@gmail.com>
> Since this is targeting net there is no cfg.rpath(),
> I'll follow up once in net-next.
>
> v2:
> - add the ioctl tests
> - factor out some common logic
>
> CC: shuah@kernel.org
> CC: hawk@kernel.org
> CC: petrm@nvidia.com
> CC: willemb@google.com
> CC: jstancek@redhat.com
> CC: linux-kselftest@vger.kernel.org
> ---
> tools/testing/selftests/net/lib/Makefile | 3 +
> drivers/net/netdevsim/ethtool.c | 2 +
> .../testing/selftests/net/lib/xdp_dummy.bpf.c | 13 ++
> tools/testing/selftests/drivers/net/hds.py | 145 +++++++++++++++++-
> 4 files changed, 160 insertions(+), 3 deletions(-)
> create mode 100644 tools/testing/selftests/net/lib/xdp_dummy.bpf.c
>
> diff --git a/tools/testing/selftests/net/lib/Makefile b/tools/testing/selftests/net/lib/Makefile
> index bc6b6762baf3..c22623b9a2a5 100644
> --- a/tools/testing/selftests/net/lib/Makefile
> +++ b/tools/testing/selftests/net/lib/Makefile
> @@ -9,7 +9,10 @@ TEST_FILES := ../../../../../Documentation/netlink/specs
> TEST_FILES += ../../../../net/ynl
>
> TEST_GEN_FILES += csum
> +TEST_GEN_FILES += $(patsubst %.c,%.o,$(wildcard *.bpf.c))
>
> TEST_INCLUDES := $(wildcard py/*.py sh/*.sh)
>
> include ../../lib.mk
> +
> +include ../bpf.mk
> diff --git a/drivers/net/netdevsim/ethtool.c b/drivers/net/netdevsim/ethtool.c
> index 5c80fbee7913..7ab358616e03 100644
> --- a/drivers/net/netdevsim/ethtool.c
> +++ b/drivers/net/netdevsim/ethtool.c
> @@ -184,9 +184,11 @@ static const struct ethtool_ops nsim_ethtool_ops = {
>
> static void nsim_ethtool_ring_init(struct netdevsim *ns)
> {
> + ns->ethtool.ring.rx_pending = 512;
> ns->ethtool.ring.rx_max_pending = 4096;
> ns->ethtool.ring.rx_jumbo_max_pending = 4096;
> ns->ethtool.ring.rx_mini_max_pending = 4096;
> + ns->ethtool.ring.tx_pending = 512;
> ns->ethtool.ring.tx_max_pending = 4096;
> }
>
> diff --git a/tools/testing/selftests/net/lib/xdp_dummy.bpf.c b/tools/testing/selftests/net/lib/xdp_dummy.bpf.c
> new file mode 100644
> index 000000000000..d988b2e0cee8
> --- /dev/null
> +++ b/tools/testing/selftests/net/lib/xdp_dummy.bpf.c
> @@ -0,0 +1,13 @@
> +// SPDX-License-Identifier: GPL-2.0
> +
> +#define KBUILD_MODNAME "xdp_dummy"
> +#include <linux/bpf.h>
> +#include <bpf/bpf_helpers.h>
> +
> +SEC("xdp")
> +int xdp_dummy_prog(struct xdp_md *ctx)
> +{
> + return XDP_PASS;
> +}
> +
> +char _license[] SEC("license") = "GPL";
> diff --git a/tools/testing/selftests/drivers/net/hds.py b/tools/testing/selftests/drivers/net/hds.py
> index 394971b25c0b..90807b21a6eb 100755
> --- a/tools/testing/selftests/drivers/net/hds.py
> +++ b/tools/testing/selftests/drivers/net/hds.py
> @@ -2,17 +2,54 @@
> # SPDX-License-Identifier: GPL-2.0
>
> import errno
> +import os
> from lib.py import ksft_run, ksft_exit, ksft_eq, ksft_raises, KsftSkipEx
> -from lib.py import EthtoolFamily, NlError
> +from lib.py import CmdExitFailure, EthtoolFamily, NlError
> from lib.py import NetDrvEnv
> +from lib.py import defer, ethtool, ip
>
> -def get_hds(cfg, netnl) -> None:
> +
> +def _get_hds_mode(cfg, netnl) -> str:
> try:
> rings = netnl.rings_get({'header': {'dev-index': cfg.ifindex}})
> except NlError as e:
> raise KsftSkipEx('ring-get not supported by device')
> if 'tcp-data-split' not in rings:
> raise KsftSkipEx('tcp-data-split not supported by device')
> + return rings['tcp-data-split']
> +
> +
> +def _xdp_onoff(cfg):
> + test_dir = os.path.dirname(os.path.realpath(__file__))
> + prog = test_dir + "/../../net/lib/xdp_dummy.bpf.o"
> + ip(f"link set dev %s xdp obj %s sec xdp" %
> + (cfg.ifname, prog))
> + ip(f"link set dev %s xdp off" % cfg.ifname)
> +
> +
> +def _ioctl_ringparam_modify(cfg, netnl) -> None:
> + """
> + Helper for performing a hopefully unimportant IOCTL SET.
> + IOCTL does not support HDS, so it should not affect the HDS config.
> + """
> + try:
> + rings = netnl.rings_get({'header': {'dev-index': cfg.ifindex}})
> + except NlError as e:
> + raise KsftSkipEx('ring-get not supported by device')
> +
> + if 'tx' not in rings:
> + raise KsftSkipEx('setting Tx ring size not supported')
> +
> + try:
> + ethtool(f"--disable-netlink -G {cfg.ifname} tx {rings['tx'] // 2}")
> + except CmdExitFailure as e:
> + ethtool(f"--disable-netlink -G {cfg.ifname} tx {rings['tx'] * 2}")
> + defer(ethtool, f"-G {cfg.ifname} tx {rings['tx']}")
> +
> +
> +def get_hds(cfg, netnl) -> None:
> + _get_hds_mode(cfg, netnl)
> +
>
> def get_hds_thresh(cfg, netnl) -> None:
> try:
> @@ -104,6 +141,103 @@ from lib.py import NetDrvEnv
> netnl.rings_set({'header': {'dev-index': cfg.ifindex}, 'hds-thresh': hds_gt})
> ksft_eq(e.exception.nl_msg.error, -errno.EINVAL)
>
> +
> +def set_xdp(cfg, netnl) -> None:
> + """
> + Enable single-buffer XDP on the device.
> + When HDS is in "auto" / UNKNOWN mode, XDP installation should work.
> + """
> + mode = _get_hds_mode(cfg, netnl)
> + if mode == 'enabled':
> + netnl.rings_set({'header': {'dev-index': cfg.ifindex},
> + 'tcp-data-split': 'unknown'})
> +
> + _xdp_onoff(cfg)
> +
> +
> +def enabled_set_xdp(cfg, netnl) -> None:
> + """
> + Enable single-buffer XDP on the device.
> + When HDS is in "enabled" mode, XDP installation should not work.
> + """
> + _get_hds_mode(cfg, netnl)
> + netnl.rings_set({'header': {'dev-index': cfg.ifindex},
> + 'tcp-data-split': 'enabled'})
> +
> + defer(netnl.rings_set, {'header': {'dev-index': cfg.ifindex},
> + 'tcp-data-split': 'unknown'})
> +
> + with ksft_raises(CmdExitFailure) as e:
> + _xdp_onoff(cfg)
> +
> +
> +def set_xdp(cfg, netnl) -> None:
> + """
> + Enable single-buffer XDP on the device.
> + When HDS is in "auto" / UNKNOWN mode, XDP installation should work.
> + """
> + mode = _get_hds_mode(cfg, netnl)
> + if mode == 'enabled':
> + netnl.rings_set({'header': {'dev-index': cfg.ifindex},
> + 'tcp-data-split': 'unknown'})
> +
> + _xdp_onoff(cfg)
> +
> +
> +def enabled_set_xdp(cfg, netnl) -> None:
> + """
> + Enable single-buffer XDP on the device.
> + When HDS is in "enabled" mode, XDP installation should not work.
> + """
> + _get_hds_mode(cfg, netnl) # Trigger skip if not supported
> +
> + netnl.rings_set({'header': {'dev-index': cfg.ifindex},
> + 'tcp-data-split': 'enabled'})
> + defer(netnl.rings_set, {'header': {'dev-index': cfg.ifindex},
> + 'tcp-data-split': 'unknown'})
> +
> + with ksft_raises(CmdExitFailure) as e:
> + _xdp_onoff(cfg)
> +
> +
> +def ioctl(cfg, netnl) -> None:
> + mode1 = _get_hds_mode(cfg, netnl)
> + _ioctl_ringparam_modify(cfg, netnl)
> + mode2 = _get_hds_mode(cfg, netnl)
> +
> + ksft_eq(mode1, mode2)
> +
> +
> +def ioctl_set_xdp(cfg, netnl) -> None:
> + """
> + Like set_xdp(), but we perturb the settings via the legacy ioctl.
> + """
> + mode = _get_hds_mode(cfg, netnl)
> + if mode == 'enabled':
> + netnl.rings_set({'header': {'dev-index': cfg.ifindex},
> + 'tcp-data-split': 'unknown'})
> +
> + _ioctl_ringparam_modify(cfg, netnl)
> +
> + _xdp_onoff(cfg)
> +
> +
> +def ioctl_enabled_set_xdp(cfg, netnl) -> None:
> + """
> + Enable single-buffer XDP on the device.
> + When HDS is in "enabled" mode, XDP installation should not work.
> + """
> + _get_hds_mode(cfg, netnl) # Trigger skip if not supported
> +
> + netnl.rings_set({'header': {'dev-index': cfg.ifindex},
> + 'tcp-data-split': 'enabled'})
> + defer(netnl.rings_set, {'header': {'dev-index': cfg.ifindex},
> + 'tcp-data-split': 'unknown'})
> +
> + with ksft_raises(CmdExitFailure) as e:
> + _xdp_onoff(cfg)
> +
> +
> def main() -> None:
> with NetDrvEnv(__file__, queue_count=3) as cfg:
> ksft_run([get_hds,
> @@ -112,7 +246,12 @@ from lib.py import NetDrvEnv
> set_hds_enable,
> set_hds_thresh_zero,
> set_hds_thresh_max,
> - set_hds_thresh_gt],
> + set_hds_thresh_gt,
> + set_xdp,
> + enabled_set_xdp,
> + ioctl,
> + ioctl_set_xdp,
> + ioctl_enabled_set_xdp],
> args=(cfg, EthtoolFamily()))
> ksft_exit()
>
> --
> 2.48.1
>
>
^ permalink raw reply [flat|nested] 3+ messages in thread