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

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

On 11/15/26 2:39 AM, Nimrod Oren wrote:
> On 11/11/2026 19:05, Dimitri Daskalakis wrote:
>> 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.
> 
> Can you please share those scenarios?
> 
> I think blocking 'disabled' was the right direction. Allowing it doesn't
> give users anything new, only a way to pin HW-GRO off, at the cost of
> another dependency between the two knobs in mlx5e_fix_features().
> 
> We can still report 'disabled' when HW-GRO is supported but off.

I should have saved the review, I reapplied v3 and added an else to 
disable tcp-data-split if the merge type is not SHAMPO, here's what sashiko
reports.

 [Medium]
 Does this reject a value that get_ringparam reports?

 When HW-GRO is off, mlx5e_ethtool_get_ringparam() reports DISABLED. That
 is accurate, because SHAMPO is not in use. Before this patch,
 "ethtool -G eth0 tcp-data-split off" was accepted in that state. Now it
 fails with -EINVAL whatever the HW-GRO state is.

 This affects any netlink client that reads the rings config and writes
 the same attributes back. The whole request fails, including any ring
 size change sent with it. The changelog does not mention that existing
 users of "off" will break.

 A narrower fix may be possible. mlx5e_fix_features() could clear
 NETIF_F_GRO_HW when hds_config is DISABLED, like it already handles
 ENABLED. That would also fix the mis-reporting and keep DISABLED usable.

Netlink users that call get then set with the received parameters will
get a failure. This generally applies to users who save the current ring state,
and try to revert back (via netlink or ethtool). A subsequent sashiko run
highlighted the manual use case via ethtool.

I recall sashiko had more severe feedback, but those cases may have been
from the incorrect feature updates I fixed here [1]. The feedback is technically
correct, though I'm not sure how much it matters in practice. v4 implements the
narrower fix sashiko suggests, so it works for users who write back stored ring state.

[1] https://patchwork.kernel.org/project/netdevbpf/patch/20261017005401.1582755-1-dimitri.daskalakis1@gmail.com/