Re: [PATCH net-next] net/mlx5e: advertise tcp-data-split support

[Dimitri Daskalakis <dimitri.daskalakis1@gmail.com>]

On Sun, Sep 27, 2026 at 11:21:03PM +0000, netdev-bot+sashiko@kernel.org wrote:
> > The ability to set tcp-data-split was introduced in
> > commit 46bcce5dfd33 ("net/mlx5e: Support ethtool tcp-data-split settings")
> > This commit dropped reporting from mlx5e_ethtool_get_ringparam()
> > without explaining why.
>
> [Severity: Low]
> The assignment added here is the same one that 46bcce5dfd33 removed from
> mlx5e_ethtool_get_ringparam(). The message also names both commits that
> led to the selftest failure.
>
> Should this have a Fixes: tag, for example:
>
> Fixes: 46bcce5dfd33 ("net/mlx5e: Support ethtool tcp-data-split settings")
>
> so that stable and backport tooling picks it up?

The feature is still usable. Without the advertisement some selftest
expectations break. That's why I did not include a Fixes: tag.

> > +	kernel_param->tcp_data_split =
> > +		(priv->channels.params.packet_merge.type == MLX5E_PACKET_MERGE_SHAMPO) ?
> > +		ETHTOOL_TCP_DATA_SPLIT_ENABLED :
> > +		ETHTOOL_TCP_DATA_SPLIT_DISABLED;
>
> [Severity: Medium]
> What does this report on a device that cannot do SHAMPO at all?
>
> [...]
>
> Should the value stay ETHTOOL_TCP_DATA_SPLIT_UNKNOWN when the device
> lacks SHAMPO support? For example, the driver could check the SHAMPO
> capability, or whether NETIF_F_GRO_HW is in priv->netdev->hw_features,
> before reporting ENABLED or DISABLED.

My thought was to restore the code that previously existed. But it should be
fine to gate this on the device's HW-GRO support.

> [Severity: Medium]
> Can this report "enabled" right after a "tcp-data-split off" request has
> succeeded?
>
> [...]
>
> With rx-gro-hw on, wouldn't set_hds_disable() in hds.py still fail?
> It would now fail on ksft_eq('disabled', rings['tcp-data-split'])
> instead of the KeyError.
>
> Should the driver actually turn SHAMPO off for DISABLED, or reject the
> request?

mlx5e_ethtool_set_tcp_data_split() can be modified to reject tcp-data-split
disables if rx-gro-hw is enabled. I'm ok with this,
but if any maintainers disagree please let me know.

> [Severity: Medium]
> HW-GRO is off by default, so this now reports "disabled" and
> _get_hds_mode() in hds.py no longer skips. Is that why enabled_set_xdp
> and ioctl_enabled_set_xdp go from SKIP to FAIL in the "After" log in the
> commit message?
>
> [...]
>
> Should the driver turn on HW-GRO when tcp-data-split=enabled is
> requested? Or should the tests enable rx-gro-hw first, or skip on EINVAL?
> As things stand, the state the driver reports does not match what the
> selftests expect.

It's ok for the tests to fail. We shouldn't modify the tests to account for
MLXs coupling of HW-GRO and tcp-data-split.

pw-bot: cr