Re: [PATCH net-next v3 3/3] net: stmmac: Don't set or get RSS parameters when not supported

[netdev-bot+sashiko]

Thank you for your contribution! Sashiko AI review found more severe
issue(s) in other patches of this series. This mail is only an FYI, in case
you want to address the nit picks below when you respin.

Low priority / nit picks for your consideration:

- [Low] The commit fixes a user-visible bug that goes back to 2019 but
  has no Fixes: trailer.

--

net: stmmac: Don't set or get RSS parameters when not supported

This commit makes stmmac_get_rxfh_key_size() and
stmmac_get_rxfh_indir_size() return 0 when stmmac_rss_is_supported() is
false. The ethtool core then reports -EOPNOTSUPP instead of returning a
bogus key and indirection table for ethtool -x, or failing with -EINVAL
for ethtool -X.

> The RSS kselftests fail on stmmac, and this is partly due to the driver
> reporting bogus data for the RSS ops :
>
>  - ethtool -x reports an indirection table and a key while the hardware
>    doesn't have any of that
>  - ethtool -X fails with -EINVAL.
>
> Return a 0 indir size and key size when rss isn't supported, effectively
> meaning we don't support RSS here.
>
> Signed-off-by: Maxime Chevallier <maxime.chevallier@xxxxxxxxxxx>

[Severity: Low]
The commit message describes a user-visible bug fix, but there is no
Fixes: tag. Should one be added?

Both callbacks were added by commit 76067459c686 ("net: stmmac: Implement
RSS and enable it in XGMAC core"). Since then they have returned
sizeof(priv->rss.key) and ARRAY_SIZE(priv->rss.table) whether or not RSS
is supported.

Because those sizes were non-zero, ethtool_get_rxfh() and
ethtool_set_rxfh() in net/ethtool/ioctl.c, and rss_prepare_data() in
net/ethtool/rss.c, never reached their -EOPNOTSUPP path:

    if (!rxfh_dev.indir_size && !rxfh_dev.key_size)
        return -EOPNOTSUPP;

Instead, ethtool -x reached stmmac_get_rxfh() and returned the defaults
set at probe time. ethtool -X went through
stmmac_set_rxfh()->stmmac_rss_configure(), and stmmac_do_callback()
returns -EINVAL there when the core has no rss_configure callback.

The previous patch, "net: stmmac: Introduce a helper to check RSS
support", notes that no platform sets rss_en yet. That means this affects
every stmmac user today.

Would something like this help stable maintainers?

    Fixes: 76067459c686 ("net: stmmac: Implement RSS and enable it in XGMAC core")

Any backport would also need the helper patch from the previous commit,
which also has no Fixes: tag.

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261002142806.1299041-1-maxime.chevallier%40bootlin.com