Re: [PATCH net-next 3/3] selftests: drv-net: hds: restore HDS settings in ioctl_set_xdp

[netdev-bot+sashiko@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential
issue(s) to consider.

Critical: 0 · High: 0 · Medium: 1 · Low: 1

If you believe none of the issues are legitimate, please reply to each
issue of Medium severity or higher and explain why the AI is wrong.
If any of them are legitimate and you plan to respin, please let patchwork
know by including "pw-bot: cr" as a separate line at the end of your reply
(one such reply per series is enough).

- [Medium] The fix is incomplete: two sibling tests in
  tools/testing/selftests/drivers/net/hds.py still change tcp-data-split…
- [Low] The patch has no Fixes: tag.

--

selftests: drv-net: hds: restore HDS settings in ioctl_set_xdp

This patch makes ioctl_set_xdp() in the drv-net hds.py selftest call
_defer_reset_hds() before it sets tcp-data-split to "unknown". The
original HDS configuration is then restored when the test finishes, the
same way set_xdp() already does it.

> diff --git a/tools/testing/selftests/drivers/net/hds.py b/tools/testing/selftests/drivers/net/hds.py
> index 7f052c3196039..a1ea195743b56 100755
> --- a/tools/testing/selftests/drivers/net/hds.py
> +++ b/tools/testing/selftests/drivers/net/hds.py
> @@ -254,6 +254,7 @@ def ioctl_set_xdp(cfg, netnl) -> None:
>      """
>      mode = _get_hds_mode(cfg, netnl)
>      if mode == 'enabled':
> +        _defer_reset_hds(cfg, netnl)
>          netnl.rings_set({'header': {'dev-index': cfg.ifindex},
>                           'tcp-data-split': 'unknown'})

[Severity: Medium]
Is this fix complete? Two sibling tests in hds.py still change
tcp-data-split, and they defer only a fixed reset to "unknown" instead of
going through _defer_reset_hds():

tools/testing/selftests/drivers/net/hds.py:enabled_set_xdp() {
    ...
    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'})
    ...
}

ioctl_enabled_set_xdp() uses the same pattern.

When "unknown" does not bring back the saved value, _hds_reset() falls
back to setting that value explicitly:

        if cur.get('tcp-data-split') == rings['tcp-data-split']:
            del arg['tcp-data-split']
        else:
            # Try the explicit setting
            arg['tcp-data-split'] = rings['tcp-data-split']

The fixed resets have no such fallback. If the operator set "enabled" or
"disabled" explicitly, and the driver reports a different value under
"unknown", that setting is lost.

Here is the test order in main():

                  set_xdp,
                  enabled_set_xdp,
                  ioctl,
                  ioctl_set_xdp,
                  ioctl_enabled_set_xdp],

In a default full run, enabled_set_xdp() has already left the device at
"unknown" before ioctl_set_xdp() starts. The new _defer_reset_hds() call
therefore only saves that already-reset state. ioctl_enabled_set_xdp()
runs last and leaves the device at "unknown" again.

Doesn't that mean this change only has a visible effect when
ioctl_set_xdp is run by itself with -t?

The previous patch in this series, "selftests: drv-net: hds: perturb
settings via ioctl in ioctl_enabled_set_xdp", edited
ioctl_enabled_set_xdp() but kept its fixed "unknown" reset.

Could enabled_set_xdp() and ioctl_enabled_set_xdp() also call
_defer_reset_hds(cfg, netnl) before they set 'enabled'?

[Severity: Low]
Should this patch carry a Fixes: tag? Commit ee3ae27721fb
("selftests: drv-net: hds: restore hds settings") added _defer_reset_hds()
to set_xdp() and the set_hds_* tests, but not to ioctl_set_xdp().
Something like:

Fixes: ee3ae27721fb ("selftests: drv-net: hds: restore hds settings")

The test itself was added by 29b036be1b0b ("selftests: drv-net: test XDP,
HDS auto and the ioctl path"), which would also be a reasonable target.

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261106031208.465456-1-dimitri.daskalakis1%40gmail.com