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

["Björn Töpel" <bjorn@kernel.org>]

Dimitri Daskalakis <dimitri.daskalakis1@gmail.com> writes:

> From: Dimitri Daskalakis <daskald@meta.com>
>
> 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. commit d347b28c492e ("net/mlx5e: Add hds-thresh
> query support via ethtool") added support for ETHTOOL_RING_USE_HDS_THRS,
> and modified mlx5e_ethtool_get_ringparam() to report an hds_thresh of 0.
>
> Between these two commits, the hds selftest helper _defer_reset_hds()
> would skip the reset because neither hds-thresh or tcp-data-split was
> present in the ring config. After the second commit this introduced
> a KeyError in the hds.set_hds_enable/disable tests.
>
> mlx5 couples tcp-data-split with rx-gro-hw. Users can currently disable
> tcp-data-split then enable HW GRO. The kernel reports tcp-data-split
> off, but the HW is presumably splitting.
>
> Add back tcp-data-split reporting in mlx5e_ethtool_get_ringparam(),
> and de-feature tcp-data-split disable. Users can either enable it
> (if HW GRO is enabled), or leave it under driver control.
>
> Additionally, move the call to netdev_update_features() out of
> mlx5e_ethtool_set_tcp_data_split(). If mlx5e_ethtool_set_ringparam()
> fails, this can cause the driver/kernel feature to de-sync.
>
> This is not a fix since HDS/tcp-data-split is still functional with HW
> GRO enabled.

Hmm, but with this move it is a fix, no? (We can probably get in similar
inconsist state as the one you describe with "ethtool blabla rx 1"?)

Maybe:
Fixes: 46bcce5dfd33 ("net/mlx5e: Support ethtool tcp-data-split
settings")

Regardless;
Reviewed-by: Björn Töpel <bjorn@kernel.org>