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

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

[Dimitri Daskalakis]

From: Dimitri Daskalakis <daskald@meta.com>

Since commit 46bcce5dfd33 ("net/mlx5e: Support ethtool tcp-data-split
settings"), mlx5 no longer reports tcp-data-split from
mlx5e_ethtool_get_ringparam() and the reply carries only the stored
config. With the default config (auto) the attribute is omitted, so the
device looks like it does not support tcp-data-split, even while HW-GRO
is on and SHAMPO is splitting headers.

On mlx5, header-data split is performed by SHAMPO, which is used only
when HW-GRO is enabled. The driver already keeps HW-GRO on while
tcp-data-split is enabled, but nothing enforces the other direction.
A user can disable tcp-data-split and then enable rx-gro-hw, leaving
the device splitting while the config says it is off.

To make the feature handling consistent, report tcp-data-split as on
when SHAMPO is active and off otherwise. Only allow disabling
tcp-data-split while HW-GRO is off, and keep HW-GRO off in
mlx5e_fix_features() when tcp-data-split is already disabled.

Signed-off-by: Dimitri Daskalakis <daskald@meta.com>
---
Changes in v4:
- Nimrod and sashiko pointed out that not reporting DISABLED for
  tcp-data-split makes the feature look unsupported. This conflicted
  with my attempt to block users from setting DISABLED, locally sashiko
  found several scenarios where ring changes would get blocked.
  Instead I took a different approach, if a user sets hw-gro on ->
  tcp-data-split on -> hw-gro off mlx forces hw-gro on. I mirrored this
  in the opposite direction, i.e force hw-gro off if a user sets
  tcp-data-split off -> hw-gro on.
- Clarify extack messages so users know the sequence of commands to run
  to enable/disable tcp-data-split.
- Directly going between tcp-data-split on <-> off is intentionally
  unsupported. Going from off -> on would require forcing HW-GRO on,
  which would require careful validation with features (XDP, XSK) that
  cannot support HW-GRO.
- Link to v3: https://lore.kernel.org/netdev/20261009025137.2631215-1-dimitri.daskalakis1@gmail.com/
Changes in v3:
- Drop redundant hw_features check in mlx5e_ethtool_get_ringparam(). Packet
  merge type can only be MLX5E_PACKET_MERGE_SHAMPO when HW GRO is active.
- Address Bjorn's feedback, and drop premature netdev_update_features() call
  in mlx5e_ethtool_set_tcp_data_split()
- Link to v2: https://lore.kernel.org/all/20261006225925.568263-1-dimitri.daskalakis1@gmail.com/
Changes in v2:
- Leave kernel_param->tcp_data_split as ETHTOOL_TCP_DATA_SPLIT_UNKNOWN if the
  device does not support HW GRO
- Prevent users from disabling tcp-data-split
- Link to v1: https://lore.kernel.org/all/20260923230521.1267511-1-dimitri.daskalakis1@gmail.com/

hds.py with GRO off

 # 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
 ok 10 hds.enabled_set_xdp # SKIP enabling of HDS not supported by the device
 ok 11 hds.ioctl
 ok 12 hds.ioctl_set_xdp
 ok 13 hds.ioctl_enabled_set_xdp # SKIP enabling of HDS not supported by the device
 # Totals: pass:9 fail:0 xfail:0 xpass:0 skip:4 error:0

hds.py with GRO on

 # 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 # SKIP disabling of HDS not supported by the device
 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
 # Exception| Traceback (most recent call last):
 ...
 # Exception| CMD: ip link set dev eth0 xdp obj /tmp/ksft-net-drv/net/lib/xdp_dummy.bpf.o sec xdp
 # Exception|   EXIT: 2
 # Exception|   STDERR: RTNETLINK answers: Invalid argument
 # Exception|
 not ok 9 hds.set_xdp
 ok 10 hds.enabled_set_xdp
 ok 11 hds.ioctl
 # Exception| Traceback (most recent call last):
 ...
 # Exception| CMD: ip link set dev eth0 xdp obj /tmp/ksft-net-drv/net/lib/xdp_dummy.bpf.o sec xdp
 # Exception|   EXIT: 2
 # Exception|   STDERR: RTNETLINK answers: Invalid argument
 # Exception|
 not ok 12 hds.ioctl_set_xdp
 ok 13 hds.ioctl_enabled_set_xdp
 # Totals: pass:9 fail:2 xfail:0 xpass:0 skip:2 error:0
---
 .../ethernet/mellanox/mlx5/core/en_ethtool.c  | 21 +++++++++++++++----
 .../net/ethernet/mellanox/mlx5/core/en_main.c |  7 +++++++
 2 files changed, 24 insertions(+), 4 deletions(-)

diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_ethtool.c b/drivers/net/ethernet/mellanox/mlx5/core/en_ethtool.c
index 1f534051cdfe..8e7c748caf2d 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/en_ethtool.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/en_ethtool.c
@@ -388,6 +388,13 @@ static void mlx5e_get_ringparam(struct net_device *dev,
 	struct mlx5e_priv *priv = netdev_priv(dev);
 
 	mlx5e_ethtool_get_ringparam(priv, param, kernel_param);
+
+	if (dev->hw_features & NETIF_F_GRO_HW)
+		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 bool mlx5e_ethtool_set_tcp_data_split(struct mlx5e_priv *priv,
@@ -399,7 +406,14 @@ static bool mlx5e_ethtool_set_tcp_data_split(struct mlx5e_priv *priv,
 	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");
+				   "TCP-data-split can not be enabled without HW-GRO, set it to 'auto' and enable HW-GRO first");
+		return false;
+	}
+
+	if (tcp_data_split == ETHTOOL_TCP_DATA_SPLIT_DISABLED &&
+	    dev->features & NETIF_F_GRO_HW) {
+		NL_SET_ERR_MSG_MOD(extack,
+				   "TCP-data-split can not be disabled while HW-GRO is on, set it to 'auto' and disable HW-GRO first");
 		return false;
 	}
 
@@ -474,9 +488,8 @@ static int mlx5e_set_ringparam(struct net_device *dev,
 	if (err)
 		return err;
 
-	/* Might need to disable HW-GRO if it was kept on due to hds. */
-	if (kernel_param->tcp_data_split == ETHTOOL_TCP_DATA_SPLIT_DISABLED &&
-	    dev->cfg->hds_config == ETHTOOL_TCP_DATA_SPLIT_ENABLED)
+	/* Might need to adjust HW-GRO if it was kept on/off due to hds. */
+	if (dev->cfg->hds_config != kernel_param->tcp_data_split)
 		netdev_update_features(priv->netdev);
 
 	return 0;
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_main.c b/drivers/net/ethernet/mellanox/mlx5/core/en_main.c
index c3c2c5720c6f..e2f657b36824 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/en_main.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/en_main.c
@@ -4627,6 +4627,13 @@ static netdev_features_t mlx5e_fix_features(struct net_device *netdev,
 		features |= NETIF_F_GRO_HW;
 	}
 
+	/* HW GRO cannot be enabled if header-data split is disabled. */
+	if (cfg && cfg->hds_config == ETHTOOL_TCP_DATA_SPLIT_DISABLED &&
+	    features & NETIF_F_GRO_HW) {
+		netdev_warn(netdev, "Disabling HW-GRO, TCP header-data split is disabled\n");
+		features &= ~NETIF_F_GRO_HW;
+	}
+
 	if (mlx5e_is_uplink_rep(priv)) {
 		features = mlx5e_fix_uplink_rep_features(netdev, features);
 		netdev->netns_immutable = true;
-- 
2.52.0

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

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential
issues to consider:
- [Medium] net/mlx5e: missing HW-GRO cap check in
  mlx5e_ethtool_set_tcp_data_split()
- [Low] net/mlx5e: missing Fixes trailer for commit 46bcce5dfd33
--

commit 10e87fc2f39f7859cabd43f8c342a731e4043353
Author: Dimitri Daskalakis <daskald@meta.com>

> net/mlx5e: advertise tcp-data-split support
>
> Since commit 46bcce5dfd33 ("net/mlx5e: Support ethtool tcp-data-split
> settings"), mlx5 no longer reports tcp-data-split from
> mlx5e_ethtool_get_ringparam() and the reply carries only the stored
> config. With the default config (auto) the attribute is omitted, so the
> device looks like it does not support tcp-data-split, even while HW-GRO
> is on and SHAMPO is splitting headers.
>
> On mlx5, header-data split is performed by SHAMPO, which is used only
> when HW-GRO is enabled. The driver already keeps HW-GRO on while
> tcp-data-split is enabled, but nothing enforces the other direction.
> A user can disable tcp-data-split and then enable rx-gro-hw, leaving
> the device splitting while the config says it is off.
>
> To make the feature handling consistent, report tcp-data-split as on
> when SHAMPO is active and off otherwise. Only allow disabling
> tcp-data-split while HW-GRO is off, and keep HW-GRO off in
> mlx5e_fix_features() when tcp-data-split is already disabled.
>
> Signed-off-by: Dimitri Daskalakis <daskald@meta.com>

[Severity: Low]
As reported by Björn Töpel on v3 [1], should this include a Fixes tag for
stable backports:

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

> diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_ethtool.c b/drivers/net/ethernet/mellanox/mlx5/core/en_ethtool.c
> index 1f534051cdfe4..8e7c748caf2d2 100644
> --- a/drivers/net/ethernet/mellanox/mlx5/core/en_ethtool.c
> +++ b/drivers/net/ethernet/mellanox/mlx5/core/en_ethtool.c
> @@ -388,6 +388,13 @@ static void mlx5e_get_ringparam(struct net_device *dev,
>  	struct mlx5e_priv *priv = netdev_priv(dev);
>  
>  	mlx5e_ethtool_get_ringparam(priv, param, kernel_param);
> +
> +	if (dev->hw_features & NETIF_F_GRO_HW)
> +		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 bool mlx5e_ethtool_set_tcp_data_split(struct mlx5e_priv *priv,
> @@ -399,7 +406,14 @@ static bool mlx5e_ethtool_set_tcp_data_split(struct mlx5e_priv *priv,
>  	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");
> +				   "TCP-data-split can not be enabled without HW-GRO, set it to 'auto' and enable HW-GRO first");
> +		return false;
> +	}
> +
> +	if (tcp_data_split == ETHTOOL_TCP_DATA_SPLIT_DISABLED &&
> +	    dev->features & NETIF_F_GRO_HW) {
> +		NL_SET_ERR_MSG_MOD(extack,
> +				   "TCP-data-split can not be disabled while HW-GRO is on, set it to 'auto' and disable HW-GRO first");
>  		return false;
>  	}

[Severity: Medium]
Does mlx5e_ethtool_set_tcp_data_split() allow setting
ETHTOOL_TCP_DATA_SPLIT_DISABLED on devices that do not support
NETIF_F_GRO_HW?

On hardware without SHAMPO support, dev->features & NETIF_F_GRO_HW is
always 0, so requesting ETHTOOL_TCP_DATA_SPLIT_DISABLED succeeds and saves
that value in dev->cfg->hds_config.  Because rings_prepare_data() copies
dev->cfg->hds_config into kernel_ringparam.tcp_data_split before calling
mlx5e_get_ringparam(), and mlx5e_get_ringparam() leaves tcp_data_split
untouched when NETIF_F_GRO_HW is not in dev->hw_features, subsequent
ETHTOOL_MSG_RINGS_GET queries will report tcp-data-split as supported and
disabled on unsupported hardware.  Requesting ETHTOOL_TCP_DATA_SPLIT_ENABLED
on such devices also returns an extack telling the user to enable HW-GRO
first when the device cannot support it.

Rejecting non-ETHTOOL_TCP_DATA_SPLIT_UNKNOWN values in
mlx5e_ethtool_set_tcp_data_split() when !(dev->hw_features & NETIF_F_GRO_HW)
would prevent this.

[1] https://lore.kernel.org/r/87a4ons7dd.fsf@all.your.base.are.belong.to.us

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261111170558.1654691-1-dimitri.daskalakis1@gmail.com?part=1

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

[Dimitri Daskalakis]

On 11/12/26 9:06 AM, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 2 potential
> issues to consider:
> - [Medium] net/mlx5e: missing HW-GRO cap check in
>   mlx5e_ethtool_set_tcp_data_split()
> - [Low] net/mlx5e: missing Fixes trailer for commit 46bcce5dfd33
> --
> 
> commit 10e87fc2f39f7859cabd43f8c342a731e4043353
> Author: Dimitri Daskalakis <daskald@meta.com>
> 
>> net/mlx5e: advertise tcp-data-split support
>>
>> Since commit 46bcce5dfd33 ("net/mlx5e: Support ethtool tcp-data-split
>> settings"), mlx5 no longer reports tcp-data-split from
>> mlx5e_ethtool_get_ringparam() and the reply carries only the stored
>> config. With the default config (auto) the attribute is omitted, so the
>> device looks like it does not support tcp-data-split, even while HW-GRO
>> is on and SHAMPO is splitting headers.
>>
>> On mlx5, header-data split is performed by SHAMPO, which is used only
>> when HW-GRO is enabled. The driver already keeps HW-GRO on while
>> tcp-data-split is enabled, but nothing enforces the other direction.
>> A user can disable tcp-data-split and then enable rx-gro-hw, leaving
>> the device splitting while the config says it is off.
>>
>> To make the feature handling consistent, report tcp-data-split as on
>> when SHAMPO is active and off otherwise. Only allow disabling
>> tcp-data-split while HW-GRO is off, and keep HW-GRO off in
>> mlx5e_fix_features() when tcp-data-split is already disabled.
>>
>> Signed-off-by: Dimitri Daskalakis <daskald@meta.com>
> 
> [Severity: Low]
> As reported by Björn Töpel on v3 [1], should this include a Fixes tag for
> stable backports:
> 
>   Fixes: 46bcce5dfd33 ("net/mlx5e: Support ethtool tcp-data-split settings")
> 

The fix was for updating netdev features after a successful ring param
change. That was sent here [1].

As for the rest of this patch, it's more of a user behavior change than a
fix. The HW splitting capability still works when GRO is enabled, it's just
reporting to the stack/user that gets corrected.

[1]
https://lore.kernel.org/all/20261017005401.1582755-1-dimitri.daskalakis1@gmail.com/

>> diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_ethtool.c b/drivers/net/ethernet/mellanox/mlx5/core/en_ethtool.c
>> index 1f534051cdfe4..8e7c748caf2d2 100644
>> --- a/drivers/net/ethernet/mellanox/mlx5/core/en_ethtool.c
>> +++ b/drivers/net/ethernet/mellanox/mlx5/core/en_ethtool.c
>> @@ -388,6 +388,13 @@ static void mlx5e_get_ringparam(struct net_device *dev,
>>  	struct mlx5e_priv *priv = netdev_priv(dev);
>>  
>>  	mlx5e_ethtool_get_ringparam(priv, param, kernel_param);
>> +
>> +	if (dev->hw_features & NETIF_F_GRO_HW)
>> +		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 bool mlx5e_ethtool_set_tcp_data_split(struct mlx5e_priv *priv,
>> @@ -399,7 +406,14 @@ static bool mlx5e_ethtool_set_tcp_data_split(struct mlx5e_priv *priv,
>>  	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");
>> +				   "TCP-data-split can not be enabled without HW-GRO, set it to 'auto' and enable HW-GRO first");
>> +		return false;
>> +	}
>> +
>> +	if (tcp_data_split == ETHTOOL_TCP_DATA_SPLIT_DISABLED &&
>> +	    dev->features & NETIF_F_GRO_HW) {
>> +		NL_SET_ERR_MSG_MOD(extack,
>> +				   "TCP-data-split can not be disabled while HW-GRO is on, set it to 'auto' and disable HW-GRO first");
>>  		return false;
>>  	}
> 
> [Severity: Medium]
> Does mlx5e_ethtool_set_tcp_data_split() allow setting
> ETHTOOL_TCP_DATA_SPLIT_DISABLED on devices that do not support
> NETIF_F_GRO_HW?
> 
> On hardware without SHAMPO support, dev->features & NETIF_F_GRO_HW is
> always 0, so requesting ETHTOOL_TCP_DATA_SPLIT_DISABLED succeeds and saves
> that value in dev->cfg->hds_config.  Because rings_prepare_data() copies
> dev->cfg->hds_config into kernel_ringparam.tcp_data_split before calling
> mlx5e_get_ringparam(), and mlx5e_get_ringparam() leaves tcp_data_split
> untouched when NETIF_F_GRO_HW is not in dev->hw_features, subsequent
> ETHTOOL_MSG_RINGS_GET queries will report tcp-data-split as supported and
> disabled on unsupported hardware.  Requesting ETHTOOL_TCP_DATA_SPLIT_ENABLED
> on such devices also returns an extack telling the user to enable HW-GRO
> first when the device cannot support it.
> 
> Rejecting non-ETHTOOL_TCP_DATA_SPLIT_UNKNOWN values in
> mlx5e_ethtool_set_tcp_data_split() when !(dev->hw_features & NETIF_F_GRO_HW)
> would prevent this.
>

I don't think it's worth the extra code. Presumably these devices cannot
split anyway, so it's fine for a user to disable the feature.

> [1] https://lore.kernel.org/r/87a4ons7dd.fsf@all.your.base.are.belong.to.us
>