netdev - Re: [PATCH net-next] selftests: drv-net: ping: make sure the ping test restores checksum offload

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <e339c382-c0f5-40dd-994e-34b388c68181@davidwei.uk>
Date: Thu, 8 May 2025 14:59:12 -0700
From: David Wei <dw@...idwei.uk>
To: Jakub Kicinski <kuba@...nel.org>, davem@...emloft.net
Cc: netdev@...r.kernel.org, edumazet@...gle.com, pabeni@...hat.com,
 andrew+netdev@...n.ch, horms@...nel.org, shuah@...nel.org,
 linux-kselftest@...r.kernel.org
Subject: Re: [PATCH net-next] selftests: drv-net: ping: make sure the ping
 test restores checksum offload

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@...nel.org>
> ---
> CC: shuah@...nel.org
> CC: linux-kselftest@...r.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?