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

[PATCH net-next v2] 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, 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.

This is not a fix since the feature is still functional with
HW GRO enabled.

Signed-off-by: Dimitri Daskalakis <daskald@meta.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 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

hds.py after:
 # 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
 ok 3 hds.set_hds_disable # SKIP disabling of HDS not supported by the device
 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 # 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
 # Totals: pass:4 fail:0 xfail:0 xpass:0 skip:9 error:0
---
 .../net/ethernet/mellanox/mlx5/core/en_ethtool.c    | 13 +++++++++++--
 1 file changed, 11 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_ethtool.c b/drivers/net/ethernet/mellanox/mlx5/core/en_ethtool.c
index 261c466a4d36..f08a968acec4 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/en_ethtool.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/en_ethtool.c
@@ -378,6 +378,10 @@ void mlx5e_ethtool_get_ringparam(struct mlx5e_priv *priv,
 
 	kernel_param->hds_thresh = 0;
 	kernel_param->hds_thresh_max = 0;
+
+	if (priv->netdev->hw_features & NETIF_F_GRO_HW &&
+	    priv->channels.params.packet_merge.type == MLX5E_PACKET_MERGE_SHAMPO)
+		kernel_param->tcp_data_split = ETHTOOL_TCP_DATA_SPLIT_ENABLED;
 }
 
 static void mlx5e_get_ringparam(struct net_device *dev,
@@ -403,9 +407,14 @@ static bool mlx5e_ethtool_set_tcp_data_split(struct mlx5e_priv *priv,
 		return false;
 	}
 
+	if (tcp_data_split == ETHTOOL_TCP_DATA_SPLIT_DISABLED) {
+		NL_SET_ERR_MSG_MOD(extack,
+				   "TCP-data-split can not be disabled");
+		return false;
+	}
+
 	/* 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)
+	if (dev->cfg->hds_config != tcp_data_split)
 		netdev_update_features(priv->netdev);
 
 	return true;
-- 
2.52.0

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

[Björn Töpel]

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.
>
> This is not a fix since the feature is still functional with
> HW GRO enabled.
>
> Signed-off-by: Dimitri Daskalakis <daskald@meta.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 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
>
> hds.py after:
>  # 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
>  ok 3 hds.set_hds_disable # SKIP disabling of HDS not supported by the device
>  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 # 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
>  # Totals: pass:4 fail:0 xfail:0 xpass:0 skip:9 error:0
> ---
>  .../net/ethernet/mellanox/mlx5/core/en_ethtool.c    | 13 +++++++++++--
>  1 file changed, 11 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_ethtool.c b/drivers/net/ethernet/mellanox/mlx5/core/en_ethtool.c
> index 261c466a4d36..f08a968acec4 100644
> --- a/drivers/net/ethernet/mellanox/mlx5/core/en_ethtool.c
> +++ b/drivers/net/ethernet/mellanox/mlx5/core/en_ethtool.c
> @@ -378,6 +378,10 @@ void mlx5e_ethtool_get_ringparam(struct mlx5e_priv *priv,
>  
>  	kernel_param->hds_thresh = 0;
>  	kernel_param->hds_thresh_max = 0;
> +
> +	if (priv->netdev->hw_features & NETIF_F_GRO_HW &&
> +	    priv->channels.params.packet_merge.type == MLX5E_PACKET_MERGE_SHAMPO)
> +		kernel_param->tcp_data_split = ETHTOOL_TCP_DATA_SPLIT_ENABLED;
>  }
>  
>  static void mlx5e_get_ringparam(struct net_device *dev,
> @@ -403,9 +407,14 @@ static bool mlx5e_ethtool_set_tcp_data_split(struct mlx5e_priv *priv,
>  		return false;
>  	}
>  
> +	if (tcp_data_split == ETHTOOL_TCP_DATA_SPLIT_DISABLED) {
> +		NL_SET_ERR_MSG_MOD(extack,
> +				   "TCP-data-split can not be disabled");
> +		return false;
> +	}
> +
>  	/* 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)
> +	if (dev->cfg->hds_config != tcp_data_split)
>  		netdev_update_features(priv->netdev);

Not directly related to your change, but is it correct to call
netdev_update_features() here? What if later changes in set_ringparam()
fail? Shouldn't the update be done when we know that set_ringparam() is
successful? Seems like we can get into an inconsistent state?


Björn

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

[Dimitri Daskalakis]

On 10/8/26 9:33 AM, Björn Töpel wrote:
> 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.
>>
>> This is not a fix since the feature is still functional with
>> HW GRO enabled.
>>
>> Signed-off-by: Dimitri Daskalakis <daskald@meta.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 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
>>
>> hds.py after:
>>  # 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
>>  ok 3 hds.set_hds_disable # SKIP disabling of HDS not supported by the device
>>  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 # 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
>>  # Totals: pass:4 fail:0 xfail:0 xpass:0 skip:9 error:0
>> ---
>>  .../net/ethernet/mellanox/mlx5/core/en_ethtool.c    | 13 +++++++++++--
>>  1 file changed, 11 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_ethtool.c b/drivers/net/ethernet/mellanox/mlx5/core/en_ethtool.c
>> index 261c466a4d36..f08a968acec4 100644
>> --- a/drivers/net/ethernet/mellanox/mlx5/core/en_ethtool.c
>> +++ b/drivers/net/ethernet/mellanox/mlx5/core/en_ethtool.c
>> @@ -378,6 +378,10 @@ void mlx5e_ethtool_get_ringparam(struct mlx5e_priv *priv,
>>  
>>  	kernel_param->hds_thresh = 0;
>>  	kernel_param->hds_thresh_max = 0;
>> +
>> +	if (priv->netdev->hw_features & NETIF_F_GRO_HW &&
>> +	    priv->channels.params.packet_merge.type == MLX5E_PACKET_MERGE_SHAMPO)
>> +		kernel_param->tcp_data_split = ETHTOOL_TCP_DATA_SPLIT_ENABLED;
>>  }
>>  
>>  static void mlx5e_get_ringparam(struct net_device *dev,
>> @@ -403,9 +407,14 @@ static bool mlx5e_ethtool_set_tcp_data_split(struct mlx5e_priv *priv,
>>  		return false;
>>  	}
>>  
>> +	if (tcp_data_split == ETHTOOL_TCP_DATA_SPLIT_DISABLED) {
>> +		NL_SET_ERR_MSG_MOD(extack,
>> +				   "TCP-data-split can not be disabled");
>> +		return false;
>> +	}
>> +
>>  	/* 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)
>> +	if (dev->cfg->hds_config != tcp_data_split)
>>  		netdev_update_features(priv->netdev);
> 
> Not directly related to your change, but is it correct to call
> netdev_update_features() here? What if later changes in set_ringparam()
> fail? Shouldn't the update be done when we know that set_ringparam() is
> successful? Seems like we can get into an inconsistent state?
> 
> 
> Björn

Good catch! This can cause the kernel/driver to de-sync. I validated this
by setting rx-gro-hw on -> tcp-data-split on -> rx-gro-hw off, then running
`ethtool -G eth0 tcp-data-split auto rx 1`. The driver disables GRO, but
the subsequent mlx5e_ethtool_set_ringparam() fails, so the kernel still
report tcp-data-split as on.

I'll move the netdev_update_features() call to the end of
mlx5e_set_ringparam() to address this.

pw-bot: cr