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

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

[Dimitri Daskalakis]

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, hds._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.

Re-adding the feature advertisement fixes the self test.

Signed-off-by: Dimitri Daskalakis <daskald@meta.com>
---
Before:
 # Interface: eth0, driver: mlx5_core
 TAP version 13
 1..13
 ok 1 hds.get_hds # SKIP tcp-data-split not supported by device
 ok 2 hds.get_hds_thresh
 # Exception while handling defer / cleanup (callback 1 of 1)!
 ...
 # Defer Exception| KeyError: 'tcp-data-split'
 # Defer Exception|
 not ok 3 hds.set_hds_disable
 # Exception while handling defer / cleanup (callback 1 of 1)!
 ...
 # Defer Exception| KeyError: 'tcp-data-split'
 # Defer Exception|
 not ok 4 hds.set_hds_enable
 ok 5 hds.set_hds_thresh_random # SKIP hds-thresh-max is too small
 ok 6 hds.set_hds_thresh_zero
 ok 7 hds.set_hds_thresh_max
 ok 8 hds.set_hds_thresh_gt
 ok 9 hds.set_xdp # SKIP tcp-data-split not supported by device
 ok 10 hds.enabled_set_xdp # SKIP tcp-data-split not supported by device
 ok 11 hds.ioctl # SKIP tcp-data-split not supported by device
 ok 12 hds.ioctl_set_xdp # SKIP tcp-data-split not supported by device
 ok 13 hds.ioctl_enabled_set_xdp # SKIP tcp-data-split not supported by device

After:
 # Interface: eth0, driver: mlx5_core
 TAP version 13
 1..13
 ok 1 hds.get_hds
 ok 2 hds.get_hds_thresh
 ok 3 hds.set_hds_disable
 ok 4 hds.set_hds_enable # SKIP disabling of HDS not supported by the device
 ok 5 hds.set_hds_thresh_random # SKIP hds-thresh-max is too small
 ok 6 hds.set_hds_thresh_zero
 ok 7 hds.set_hds_thresh_max
 ok 8 hds.set_hds_thresh_gt
 ok 9 hds.set_xdp
 ...
 # Exception| net.lib.ynl.pyynl.lib.ynl.NlError: Netlink error: mlx5_core: TCP-data-split is not supported when GRO HW is disabled: Invalid argument
 # Exception|
 not ok 10 hds.enabled_set_xdp
 ok 11 hds.ioctl
 ok 12 hds.ioctl_set_xdp
 ...
 # Exception| net.lib.ynl.pyynl.lib.ynl.NlError: Netlink error: mlx5_core: TCP-data-split is not supported when GRO HW is disabled: Invalid argument
 # Exception|
 not ok 13 hds.ioctl_enabled_set_xdp
 # Totals: pass:9 fail:2 xfail:0 xpass:0 skip:2 error:0

---
 drivers/net/ethernet/mellanox/mlx5/core/en_ethtool.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_ethtool.c b/drivers/net/ethernet/mellanox/mlx5/core/en_ethtool.c
index 261c466a4d36..62060a6fa185 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/en_ethtool.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/en_ethtool.c
@@ -378,6 +378,11 @@ void mlx5e_ethtool_get_ringparam(struct mlx5e_priv *priv,
 
 	kernel_param->hds_thresh = 0;
 	kernel_param->hds_thresh_max = 0;
+
+	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;
 }
 
 static void mlx5e_get_ringparam(struct net_device *dev,
-- 
2.52.0

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

[netdev-bot+sashiko]

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

Critical: 0 · High: 0 · Medium: 3 · 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] mlx5e_ethtool_get_ringparam() now reports tcp-data-split from
  the live SHAMPO state (priv->channels.params.packet_merge.type).
- [Medium] With HW-GRO off (the default, en_main.c:5949 clears
  NETIF_F_GRO_HW), GET now reports 'disabled', so hds.py _get_hds_mode()…
- [Medium] net/mlx5e: ethtool tcp-data-split API contract violation on
  unsupported hardware
- [Low] The patch puts back exactly the tcp_data_split assignment in
  mlx5e_ethtool_get_ringparam() that commit 46bcce5dfd33 ('net/mlx5e:…

--

net/mlx5e: advertise tcp-data-split support

This patch makes mlx5e_ethtool_get_ringparam() report tcp-data-split
again. The value is ENABLED when the channel packet merge type is SHAMPO
and DISABLED otherwise. Commit 46bcce5dfd33 dropped this reporting.
After commit d347b28c492e started reporting hds_thresh, the hds.py
selftests hit a KeyError on tcp-data-split.

> 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?

> diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_ethtool.c b/drivers/net/ethernet/mellanox/mlx5/core/en_ethtool.c
> index 261c466a4d362..62060a6fa185c 100644
> --- a/drivers/net/ethernet/mellanox/mlx5/core/en_ethtool.c
> +++ b/drivers/net/ethernet/mellanox/mlx5/core/en_ethtool.c
> @@ -378,6 +378,11 @@ void mlx5e_ethtool_get_ringparam(struct mlx5e_priv *priv,
>  
>  	kernel_param->hds_thresh = 0;
>  	kernel_param->hds_thresh_max = 0;
> +
> +	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?

If the SHAMPO capability bits, such as MLX5_CAP_GEN(mdev, shampo),
are not set, HW-GRO can never be turned on. In that case
packet_merge.type can never be MLX5E_PACKET_MERGE_SHAMPO, so this
assignment always reports ETHTOOL_TCP_DATA_SPLIT_DISABLED.

The ethtool netlink code leaves ETHTOOL_A_RINGS_TCP_DATA_SPLIT out of
the RINGS_GET reply only when the value is
ETHTOOL_TCP_DATA_SPLIT_UNKNOWN. Userspace therefore treats the presence
of the attribute as "the device supports header-data split".

With this change every mlx5e device advertises the feature as
supported but off. That includes devices where
mlx5e_ethtool_set_tcp_data_split() can only reject a later
"tcp-data-split on", because GRO HW is not available.

hds.py shows the effect. _get_hds_mode() skips only when
'tcp-data-split' is missing from the reply. On such hardware, get_hds
and the xdp tests would now run instead of skipping, and the enable
paths would fail with EINVAL.

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.

[Severity: Medium]
Can this report "enabled" right after a "tcp-data-split off" request has
succeeded?

GET now reflects the live SHAMPO state. The set path does not always
turn SHAMPO off when DISABLED is requested. In
mlx5e_ethtool_set_tcp_data_split():

	/* Might need to disable HW-GRO if it was kept on due to hds. */
	if (tcp_data_split == ETHTOOL_TCP_DATA_SPLIT_DISABLED &&
	    dev->cfg->hds_config == ETHTOOL_TCP_DATA_SPLIT_ENABLED)
		netdev_update_features(priv->netdev);

	return true;

Suppose the user has run "ethtool -K ethX rx-gro-hw on", so
NETIF_F_GRO_HW is in wanted_features:

- If hds_config is UNKNOWN, netdev_update_features() is never called.
- If hds_config is ENABLED, mlx5e_fix_features() only stops forcing
  GRO_HW on. GRO_HW stays on because it is still wanted.

In both cases SHAMPO stays active.

mlx5e_ethtool_set_ringparam() then returns early because the ring sizes
did not change:

	if (log_rq_size == priv->channels.params.log_rq_mtu_frames &&
	    log_sq_size == priv->channels.params.log_sq_size)
		return 0;

and ethnl_set_rings() commits the request:

	dev->cfg_pending->hds_config = kernel_ringparam.tcp_data_split;

On the next RINGS_GET, rings_prepare_data() prefills tcp_data_split
from dev->cfg->hds_config, which is DISABLED. This new assignment then
overwrites it with ENABLED.

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?

[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?

Both tests always request:

    netnl.rings_set({'header': {'dev-index': cfg.ifindex},
                     'tcp-data-split': 'enabled'})

but mlx5e_ethtool_set_tcp_data_split() rejects that unless HW-GRO is
already on:

	if (tcp_data_split == ETHTOOL_TCP_DATA_SPLIT_ENABLED &&
	    !(dev->features & NETIF_F_GRO_HW)) {
		NL_SET_ERR_MSG_MOD(extack,
				   "TCP-data-split is not supported when GRO HW is disabled");
		return false;
	}

set_hds_enable() gets the same -EINVAL. The test maps it to a skip, and
the existing skip message looks copy-pasted from set_hds_disable():

        if e.error == errno.EINVAL:
            raise KsftSkipEx("disabling of HDS not supported by the device")

So the rejected enable shows up as a skip with the wrong reason, and the
enable path is never tested.

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.

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

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

[Dimitri Daskalakis]

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