[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

[netdev-bot+sashiko]

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

Critical: 0 · High: 0 · Medium: 1 · 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_get_ringparam() now reports tcp-data-split as ENABLED
  whenever SHAMPO is active, even when dev->cfg->hds_config is still…
- [Low] The patch has no Fixes: tag, even though its commit message
  describes regressions caused by commit 46bcce5dfd33 ("net/mlx5e:…

--

net/mlx5e: advertise tcp-data-split support

On devices that support HW-GRO, mlx5e_get_ringparam() now reports
tcp-data-split as enabled when SHAMPO is active and as disabled otherwise.
The patch also rejects disabling tcp-data-split while HW-GRO is on.
mlx5e_fix_features() now keeps HW-GRO off when tcp-data-split is disabled.

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

[Severity: Low]
This isn't a bug, but should this patch carry a Fixes: tag?

The commit message names 46bcce5dfd33 as the cause. That commit removed
the assignment of kernel_param->tcp_data_split from the SHAMPO state in
mlx5e_ethtool_get_ringparam().

Since then, rings_prepare_data() seeds the value from
dev->cfg->hds_config. rings_fill_reply() only emits
ETHTOOL_A_RINGS_TCP_DATA_SPLIT when the value is non-zero, so in auto mode
the attribute is missing.

46bcce5dfd33 also enforced only one direction in mlx5e_fix_features().
This patch restores the reporting and adds the missing direction, so
would this be appropriate:

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

Without the tag, stable kernels that contain 46bcce5dfd33 would keep the
missing attribute in auto mode.

> 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;
>  }

[Severity: Medium]
With this change, tcp-data-split is reported as enabled whenever SHAMPO
is active, even while dev->cfg->hds_config is still auto
(ETHTOOL_TCP_DATA_SPLIT_UNKNOWN). Does this break the XDP cases in
tools/testing/selftests/drivers/net/hds.py?

With HW-GRO on, set_xdp() and ioctl_set_xdp() now see 'enabled' and set
tcp-data-split back to 'unknown'. hds_config is already unknown, so
ethnl_set_rings() sees no change and returns without calling the
driver. HW-GRO and SHAMPO stay on.

The core checks in dev_xdp_attach() and netif_xdp_propagate() only
reject single-buffer XDP when hds_config is
ETHTOOL_TCP_DATA_SPLIT_ENABLED. The request therefore reaches the driver,
and mlx5e_xdp_allowed() rejects it:

	if (params->packet_merge.type != MLX5E_PACKET_MERGE_NONE) {
		netdev_warn(netdev, "can't set XDP while HW-GRO/LRO is on, disable them first\n");
		return -EINVAL;

That matches "not ok 9 hds.set_xdp" and "not ok 12 hds.ioctl_set_xdp" in
the posted run. Before this patch, ioctl_set_xdp found the attribute
missing in auto mode and skipped.

In auto mode with HDS reported as on, the ring param also can't be used
to turn HDS off. mlx5e_ethtool_set_tcp_data_split() now rejects
'disabled' while HW-GRO is on.

With HW-GRO off, enabled_set_xdp() and ioctl_enabled_set_xdp() call
rings_set() with tcp-data-split set to 'enabled' and no exception
handling. The driver now returns -EINVAL for that request.

The "SKIP enabling of HDS not supported by the device" lines in the
posted results don't seem to exist anywhere under
tools/testing/selftests/. Did those results come from a locally modified
hds.py?

Because of the _hds_reset() logic in hds.py, some of these cases could
already fail before this patch, depending on test order.

Should hds.py be updated together with this change? If not, could the
commit message mention the expected hds.py failures on mlx5?

[ ... ]

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

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

[Dimitri Daskalakis]

On 11/13/26 12:06 AM, netdev-bot+sashiko@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 2 potential
> issue(s) to consider.
> 
> Critical: 0 · High: 0 · Medium: 1 · 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_get_ringparam() now reports tcp-data-split as ENABLED
>   whenever SHAMPO is active, even when dev->cfg->hds_config is still…
> - [Low] The patch has no Fixes: tag, even though its commit message
>   describes regressions caused by commit 46bcce5dfd33 ("net/mlx5e:…
> 
> --
> 
> net/mlx5e: advertise tcp-data-split support
> 
> On devices that support HW-GRO, mlx5e_get_ringparam() now reports
> tcp-data-split as enabled when SHAMPO is active and as disabled otherwise.
> The patch also rejects disabling tcp-data-split while HW-GRO is on.
> mlx5e_fix_features() now keeps HW-GRO off when tcp-data-split is disabled.
> 
>> 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.
> 
> [Severity: Low]
> This isn't a bug, but should this patch carry a Fixes: tag?
> 
> The commit message names 46bcce5dfd33 as the cause. That commit removed
> the assignment of kernel_param->tcp_data_split from the SHAMPO state in
> mlx5e_ethtool_get_ringparam().
> 
> Since then, rings_prepare_data() seeds the value from
> dev->cfg->hds_config. rings_fill_reply() only emits
> ETHTOOL_A_RINGS_TCP_DATA_SPLIT when the value is non-zero, so in auto mode
> the attribute is missing.
> 
> 46bcce5dfd33 also enforced only one direction in mlx5e_fix_features().
> This patch restores the reporting and adds the missing direction, so
> would this be appropriate:
> 
> Fixes: 46bcce5dfd33 ("net/mlx5e: Support ethtool tcp-data-split settings")
> 
> Without the tag, stable kernels that contain 46bcce5dfd33 would keep the
> missing attribute in auto mode.
> 

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;
>>  }
> 
> [Severity: Medium]
> With this change, tcp-data-split is reported as enabled whenever SHAMPO
> is active, even while dev->cfg->hds_config is still auto
> (ETHTOOL_TCP_DATA_SPLIT_UNKNOWN). Does this break the XDP cases in
> tools/testing/selftests/drivers/net/hds.py?
> 
> With HW-GRO on, set_xdp() and ioctl_set_xdp() now see 'enabled' and set
> tcp-data-split back to 'unknown'. hds_config is already unknown, so
> ethnl_set_rings() sees no change and returns without calling the
> driver. HW-GRO and SHAMPO stay on.
> 
> The core checks in dev_xdp_attach() and netif_xdp_propagate() only
> reject single-buffer XDP when hds_config is
> ETHTOOL_TCP_DATA_SPLIT_ENABLED. The request therefore reaches the driver,
> and mlx5e_xdp_allowed() rejects it:
> 
> 	if (params->packet_merge.type != MLX5E_PACKET_MERGE_NONE) {
> 		netdev_warn(netdev, "can't set XDP while HW-GRO/LRO is on, disable them first\n");
> 		return -EINVAL;
> 
> That matches "not ok 9 hds.set_xdp" and "not ok 12 hds.ioctl_set_xdp" in
> the posted run. Before this patch, ioctl_set_xdp found the attribute
> missing in auto mode and skipped.

It's fine for set_xdp() and ioctl_set_xdp() to fail. We should not modify the test
to account for mlx5's tcp-data-split and HW-GRO coupling.

> 
> In auto mode with HDS reported as on, the ring param also can't be used
> to turn HDS off. mlx5e_ethtool_set_tcp_data_split() now rejects
> 'disabled' while HW-GRO is on.
> 
> With HW-GRO off, enabled_set_xdp() and ioctl_enabled_set_xdp() call
> rings_set() with tcp-data-split set to 'enabled' and no exception
> handling. The driver now returns -EINVAL for that request.
> 

The above two cases are expected behavior. If HW-GRO is on, users
cannot disable tcp-data-split. And vice-versa, if HW-GRO is off users
cannot enable tcp-data-split.

> The "SKIP enabling of HDS not supported by the device" lines in the
> posted results don't seem to exist anywhere under
> tools/testing/selftests/. Did those results come from a locally modified
> hds.py?
> 

This was from a local change I forgot to revert. Here's the results from an
unmodified hds.py with HW-GRO disabled.

# 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| Traceback (most recent call last):
...
# Exception| net.lib.ynl.pyynl.lib.ynl.NlError: Netlink error: mlx5_core: TCP-data-split can not be enabled without HW-GRO, set it to 'auto' and enable HW-GRO first: Invalid argument
# Exception|
not ok 10 hds.enabled_set_xdp
ok 11 hds.ioctl
ok 12 hds.ioctl_set_xdp
# Exception| Traceback (most recent call last):
...
# Exception| net.lib.ynl.pyynl.lib.ynl.NlError: Netlink error: mlx5_core: TCP-data-split can not be enabled without HW-GRO, set it to 'auto' and enable HW-GRO first: Invalid argument
# Exception|
not ok 13 hds.ioctl_enabled_set_xdp

The failures are acceptable, we should not modify the selftest to account for mlx5's
tcp-data-split and HW-GRO coupling. 

> Because of the _hds_reset() logic in hds.py, some of these cases could
> already fail before this patch, depending on test order.
> 
> Should hds.py be updated together with this change? If not, could the
> commit message mention the expected hds.py failures on mlx5?
> 
> [ ... ]
> 

Updates to hds.py have already been merged [2]. The behavior here is expected.
If this series needs a re-spin for other reasons, I can modify the commit message.

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

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

[Nimrod Oren]

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.

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

[Dimitri Daskalakis]

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/