All of lore.kernel.org
 help / color / mirror / Atom feed
From: Dimitri Daskalakis <dimitri.daskalakis1@gmail.com>
To: "David S . Miller" <davem@davemloft.net>
Cc: "Saeed Mahameed" <saeedm@nvidia.com>,
	"Tariq Toukan" <tariqt@nvidia.com>,
	"Mark Bloch" <mbloch@nvidia.com>,
	"Leon Romanovsky" <leon@kernel.org>,
	"Andrew Lunn" <andrew+netdev@lunn.ch>,
	"Eric Dumazet" <edumazet@google.com>,
	"Jakub Kicinski" <kuba@kernel.org>,
	"Paolo Abeni" <pabeni@redhat.com>,
	"Gal Pressman" <gal@nvidia.com>,
	"Cosmin Ratiu" <cratiu@nvidia.com>,
	"Dragos Tatulea" <dtatulea@nvidia.com>,
	"Nimrod Oren" <noren@nvidia.com>,
	"Carolina Jubran" <cjubran@nvidia.com>,
	"Björn Töpel" <bjorn@kernel.org>,
	"Dimitri Daskalakis" <dimitri.daskalakis1@gmail.com>,
	netdev@vger.kernel.org
Subject: [PATCH net] net/mlx5e: Update netdev features only after a successful ringparam set
Date: Fri, 16 Oct 2026 17:54:01 -0700	[thread overview]
Message-ID: <20261017005401.1582755-1-dimitri.daskalakis1@gmail.com> (raw)

From: Dimitri Daskalakis <daskald@meta.com>

mlx5 couples HW-GRO with tcp-data-split. If a user sets rx-gro-hw on ->
tcp-data-split on -> rx-gro-hw off, the driver will leave HW-GRO on.
mlx5e_ethtool_set_tcp_data_split() calls netdev_update_features() when
tcp-data-split goes from enabled to disabled in case HW-GRO needs to be
released. mlx5e_fix_features() reads the new HDS config from
dev->cfg_pending, so HW-GRO is released before all ring parameters are
validated.

If the subsequent call to mlx5e_ethtool_set_ringparam() fails, the
ethtool core discards the pending change and reports tcp-data-split
enabled. The device is no longer splitting, and subsequent ring changes
are blocked.

Move the call to netdev_update_features() after
mlx5e_ethtool_set_ringparam() exits successfully.

The issue was discovered by manual code inspection during review of
a change that stops mlx5 users from disabling tcp-data-split. It was
reproduced on a ConnectX-7 with ethtool alone.

Steps to reproduce:
ethtool -K eth0 rx-gro-hw on
ethtool -G eth0 tcp-data-split on
ethtool -K eth0 rx-gro-hw off # HW-GRO kept on for tcp-data-split

ethtool -G eth0 tcp-data-split off rx 1 # de-sync driver & kernel state
 netlink error: mlx5_core: rx (1) < min (16)
 netlink error: Invalid argument

ethtool -G eth0 rx 1024 # valid ring changes are now blocked
 netlink error: mlx5_core: TCP-data-split is not supported when GRO HW is disabled
 netlink error: Invalid argument

This is not a terminal state, users can recover by setting rx-gro-hw on.
With the fix, the failing ringparam set leaves rx-gro-hw on and later
ring changes succeed.

Fixes: 46bcce5dfd33 ("net/mlx5e: Support ethtool tcp-data-split settings")
Reported-by: Björn Töpel <bjorn@kernel.org>
Closes: https://lore.kernel.org/87se2grw3d.fsf@all.your.base.are.belong.to.us/
Signed-off-by: Dimitri Daskalakis <daskald@meta.com>
---
 .../ethernet/mellanox/mlx5/core/en_ethtool.c    | 17 +++++++++++------
 1 file changed, 11 insertions(+), 6 deletions(-)

diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_ethtool.c b/drivers/net/ethernet/mellanox/mlx5/core/en_ethtool.c
index 3ed59ced0407..f38e033ab8fa 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/en_ethtool.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/en_ethtool.c
@@ -403,11 +403,6 @@ static bool mlx5e_ethtool_set_tcp_data_split(struct mlx5e_priv *priv,
 		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)
-		netdev_update_features(priv->netdev);
-
 	return true;
 }
 
@@ -468,13 +463,23 @@ static int mlx5e_set_ringparam(struct net_device *dev,
 			       struct netlink_ext_ack *extack)
 {
 	struct mlx5e_priv *priv = netdev_priv(dev);
+	int err;
 
 	if (!mlx5e_ethtool_set_tcp_data_split(priv,
 					      kernel_param->tcp_data_split,
 					      extack))
 		return -EINVAL;
 
-	return mlx5e_ethtool_set_ringparam(priv, param, extack);
+	err = mlx5e_ethtool_set_ringparam(priv, param, extack);
+	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)
+		netdev_update_features(priv->netdev);
+
+	return 0;
 }
 
 void mlx5e_ethtool_get_channels(struct mlx5e_priv *priv,
-- 
2.52.0


             reply	other threads:[~2026-10-17  0:54 UTC|newest]

Thread overview: 2+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-10-17  0:54 Dimitri Daskalakis [this message]
2026-10-17 14:24 ` [PATCH net] net/mlx5e: Update netdev features only after a successful ringparam set Björn Töpel

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20261017005401.1582755-1-dimitri.daskalakis1@gmail.com \
    --to=dimitri.daskalakis1@gmail.com \
    --cc=andrew+netdev@lunn.ch \
    --cc=bjorn@kernel.org \
    --cc=cjubran@nvidia.com \
    --cc=cratiu@nvidia.com \
    --cc=davem@davemloft.net \
    --cc=dtatulea@nvidia.com \
    --cc=edumazet@google.com \
    --cc=gal@nvidia.com \
    --cc=kuba@kernel.org \
    --cc=leon@kernel.org \
    --cc=mbloch@nvidia.com \
    --cc=netdev@vger.kernel.org \
    --cc=noren@nvidia.com \
    --cc=pabeni@redhat.com \
    --cc=saeedm@nvidia.com \
    --cc=tariqt@nvidia.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.