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

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

Thanks for the review.

On 10/12/26 4:35 AM, Nimrod Oren wrote:
> Thanks for working on this, a few comments below.
> 
> On 09/10/2026 5:51, Dimitri Daskalakis wrote:
>> diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_ethtool.c b/drivers/net/ethernet/mellanox/mlx5/core/en_ethtool.c
>> index 261c466a4d36..c30d751cac2f 100644
>> --- a/drivers/net/ethernet/mellanox/mlx5/core/en_ethtool.c
>> +++ b/drivers/net/ethernet/mellanox/mlx5/core/en_ethtool.c
>> @@ -378,6 +378,9 @@ void mlx5e_ethtool_get_ringparam(struct mlx5e_priv *priv,
>>  
>>  	kernel_param->hds_thresh = 0;
>>  	kernel_param->hds_thresh_max = 0;
>> +
>> +	if (priv->channels.params.packet_merge.type == MLX5E_PACKET_MERGE_SHAMPO)
>> +		kernel_param->tcp_data_split = ETHTOOL_TCP_DATA_SPLIT_ENABLED;
>>  }
> 
> With HW-GRO off, tcp-data-split is omitted and appears unsupported.
> I think it should report disabled when HW-GRO is supported but inactive.
> 

Sashiko also pushed for this. I verified it should be ok, so I'll include
that in the next version.

> ...
> 
>> +	if (tcp_data_split == ETHTOOL_TCP_DATA_SPLIT_DISABLED) {
>> +		NL_SET_ERR_MSG_MOD(extack,
>> +				   "TCP-data-split can not be disabled");
>> +		return false;
> 
> I think the message should emphasize that TCP-data-split can't be
> directly/explicitly disabled.
> 

Any suggestions on the wording?

> ...
>> +	/* Disable HW-GRO if it was only kept on for HDS. */
>> +	if (dev->cfg->hds_config != kernel_param->tcp_data_split)
>> +		netdev_update_features(priv->netdev);
> 
> nit: the only transition that needs this is enabled -> auto,
> so maybe something like this is preferable:
> 
>   if (dev->cfg->hds_config == ETHTOOL_TCP_DATA_SPLIT_ENABLED &&
>       kernel_param->tcp_data_split != ETHTOOL_TCP_DATA_SPLIT_ENABLED)

It's harmless to be less restrictive here, but if you feel strongly I can
go with what you proposed.

> 
> Thanks,
> Nimrod