| Message ID | 20250508214005.1518013-1-kuba@kernel.org |
|---|---|
| State | New |
| Headers | show |
| Series | [net-next] selftests: drv-net: ping: make sure the ping test restores checksum offload | expand |
On 5/8/25 14:40, Jakub Kicinski wrote: > The ping test flips checksum offload on and off. > Make sure the original value is restored if test fails. > > Signed-off-by: Jakub Kicinski <kuba@kernel.org> > --- > CC: shuah@kernel.org > CC: linux-kselftest@vger.kernel.org > --- > tools/testing/selftests/drivers/net/ping.py | 16 ++++++++++++++++ > 1 file changed, 16 insertions(+) > > diff --git a/tools/testing/selftests/drivers/net/ping.py b/tools/testing/selftests/drivers/net/ping.py > index af8df2313a3b..e0f114612c1a 100755 > --- a/tools/testing/selftests/drivers/net/ping.py > +++ b/tools/testing/selftests/drivers/net/ping.py > @@ -50,6 +50,16 @@ no_sleep=False > cmd(f"echo {test_string} | socat -t 2 -u STDIN TCP:{cfg.remote_baddr}:{port}", shell=True) > ksft_eq(nc.stdout.strip(), test_string) > > +def _schedule_checksum_reset(cfg, netnl) -> None: > + features = ethtool(f"-k {cfg.ifname}", json=True) > + setting = "" > + for side in ["tx", "rx"]: > + f = features[0][side + "-checksumming"] > + if not f["fixed"]: I checked and found that "fixed" is a ternary: "rx-checksumming": { "active": true, "fixed": false, "requested": true }, "tx-checksumming": { "active": true, "fixed": null, "requested": null }, Python loads this JSON as False and None types respectively, and `not f["fixed"]` is true for both False and None. Maybe this doesn't matter but flagging it. > + setting += " " + side > + setting += " " + ("on" if f["requested"] or f["active"] else "off") > + defer(ethtool, f" -K {cfg.ifname} " + setting) This does rx/tx-gro too even if not explicitly requested. I assume that is okay?
On Thu, 8 May 2025 14:59:12 -0700 David Wei wrote: > > +def _schedule_checksum_reset(cfg, netnl) -> None: > > + features = ethtool(f"-k {cfg.ifname}", json=True) > > + setting = "" > > + for side in ["tx", "rx"]: > > + f = features[0][side + "-checksumming"] > > + if not f["fixed"]: > > I checked and found that "fixed" is a ternary: > > "rx-checksumming": { > "active": true, > "fixed": false, > "requested": true > }, > "tx-checksumming": { > "active": true, > "fixed": null, > "requested": null > }, > > Python loads this JSON as False and None types respectively, and `not > f["fixed"]` is true for both False and None. Maybe this doesn't matter > but flagging it. I think so, yes. > > + setting += " " + side > > + setting += " " + ("on" if f["requested"] or f["active"] else "off") > > + defer(ethtool, f" -K {cfg.ifname} " + setting) > > This does rx/tx-gro too even if not explicitly requested. I assume that > is okay? You mean because those are automatically updated when we change checksumming? If so then yes.
diff --git a/tools/testing/selftests/drivers/net/ping.py b/tools/testing/selftests/drivers/net/ping.py index af8df2313a3b..e0f114612c1a 100755 --- a/tools/testing/selftests/drivers/net/ping.py +++ b/tools/testing/selftests/drivers/net/ping.py @@ -50,6 +50,16 @@ no_sleep=False cmd(f"echo {test_string} | socat -t 2 -u STDIN TCP:{cfg.remote_baddr}:{port}", shell=True) ksft_eq(nc.stdout.strip(), test_string) +def _schedule_checksum_reset(cfg, netnl) -> None: + features = ethtool(f"-k {cfg.ifname}", json=True) + setting = "" + for side in ["tx", "rx"]: + f = features[0][side + "-checksumming"] + if not f["fixed"]: + setting += " " + side + setting += " " + ("on" if f["requested"] or f["active"] else "off") + defer(ethtool, f" -K {cfg.ifname} " + setting) + def _set_offload_checksum(cfg, netnl, on) -> None: try: ethtool(f" -K {cfg.ifname} rx {on} tx {on} ") @@ -139,6 +149,7 @@ no_sleep=False def test_default_v4(cfg, netnl) -> None: cfg.require_ipver("4") + _schedule_checksum_reset(cfg, netnl) _set_offload_checksum(cfg, netnl, "off") _test_v4(cfg) _test_tcp(cfg) @@ -149,6 +160,7 @@ no_sleep=False def test_default_v6(cfg, netnl) -> None: cfg.require_ipver("6") + _schedule_checksum_reset(cfg, netnl) _set_offload_checksum(cfg, netnl, "off") _test_v6(cfg) _test_tcp(cfg) @@ -157,6 +169,7 @@ no_sleep=False _test_tcp(cfg) def test_xdp_generic_sb(cfg, netnl) -> None: + _schedule_checksum_reset(cfg, netnl) _set_xdp_generic_sb_on(cfg) _set_offload_checksum(cfg, netnl, "off") _test_v4(cfg) @@ -168,6 +181,7 @@ no_sleep=False _test_tcp(cfg) def test_xdp_generic_mb(cfg, netnl) -> None: + _schedule_checksum_reset(cfg, netnl) _set_xdp_generic_mb_on(cfg) _set_offload_checksum(cfg, netnl, "off") _test_v4(cfg) @@ -179,6 +193,7 @@ no_sleep=False _test_tcp(cfg) def test_xdp_native_sb(cfg, netnl) -> None: + _schedule_checksum_reset(cfg, netnl) _set_xdp_native_sb_on(cfg) _set_offload_checksum(cfg, netnl, "off") _test_v4(cfg) @@ -190,6 +205,7 @@ no_sleep=False _test_tcp(cfg) def test_xdp_native_mb(cfg, netnl) -> None: + _schedule_checksum_reset(cfg, netnl) _set_xdp_native_mb_on(cfg) _set_offload_checksum(cfg, netnl, "off") _test_v4(cfg)
The ping test flips checksum offload on and off. Make sure the original value is restored if test fails. Signed-off-by: Jakub Kicinski <kuba@kernel.org> --- CC: shuah@kernel.org CC: linux-kselftest@vger.kernel.org --- tools/testing/selftests/drivers/net/ping.py | 16 ++++++++++++++++ 1 file changed, 16 insertions(+)