| Message ID | 20250221025141.1132944-2-kuba@kernel.org (mailing list archive) |
|---|---|
| State | New |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | [net,v2,1/2] net: ethtool: fix ioctl confusing drivers about desired HDS user config | expand |
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")
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()
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> --- 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