From: Tariq Toukan <tariqt@nvidia.com>
To: Andrew Lunn <andrew+netdev@lunn.ch>,
"David S. Miller" <davem@davemloft.net>,
Eric Dumazet <edumazet@google.com>,
Jakub Kicinski <kuba@kernel.org>, <netdev@vger.kernel.org>,
Paolo Abeni <pabeni@redhat.com>
Cc: Akiva Goldberger <agoldberger@nvidia.com>,
Edward Srouji <edwards@nvidia.com>, Gal Pressman <gal@nvidia.com>,
Kees Cook <kees@kernel.org>, Leon Romanovsky <leon@kernel.org>,
<linux-kernel@vger.kernel.org>, <linux-rdma@vger.kernel.org>,
Maher Sanalla <msanalla@nvidia.com>,
Mark Bloch <mbloch@nvidia.com>, Or Har-Toov <ohartoov@nvidia.com>,
Parav Pandit <parav@nvidia.com>,
Patrisious Haddad <phaddad@nvidia.com>,
Saeed Mahameed <saeedm@nvidia.com>, Shay Drori <shayd@nvidia.com>,
Simon Horman <horms@kernel.org>, Tariq Toukan <tariqt@nvidia.com>
Subject: [PATCH net 1/4] net/mlx5: SD, serialize SD LAG init/cleanup against LAG mode changes
Date: Wed, 2 Sep 2026 19:37:13 +0300 [thread overview]
Message-ID: <20260902163716.3656279-2-tariqt@nvidia.com> (raw)
In-Reply-To: <20260902163716.3656279-1-tariqt@nvidia.com>
From: Shay Drory <shayd@nvidia.com>
A secondary SD is spliced into the primary's shared LAG device (ldev)
by sd_lag_init() and removed by sd_lag_cleanup(). The LAG mode-change
paths drop ldev->lock mid-operation while iterating ldev->pfs, and rely
on ldev->mode_changes_in_progress to keep the member set stable across
that window.
sd_lag_cleanup() did not honor ldev->mode_changes_in_progress. Which
means a concurrent SD teardown could xa_erase() and kfree() a pf out of
ldev->pfs during a mode change's dropped-lock window, leading to a
NULL/use-after-free dereference of the secondary pf.
Make sd_lag_init() and sd_lag_cleanup() wait until
mode_changes_in_progress drops to zero before touching ldev, mirroring
mlx5_lag_remove_mdev().
In addition, fold the SD shared-FDB teardown in
mlx5_lag_disable_change() into the main locked section. Otherwise the
two acquire sd_devcom and mode_changes_in_progress in opposite orders -
sd_lag_init/cleanup takes sd_devcom then waits on
mode_changes_in_progress, while disable_change increments
mode_changes_in_progress then takes sd_devcom - an ABBA deadlock.
The same ABBA deadlock is present in mlx5_eswitch_disable(), which takes
sd_devcom between mlx5_lag_disable_change() and
mlx5_lag_enable_change(). Move that call after mlx5_lag_enable_change(),
matching the ordering already used by mlx5_devlink_eswitch_mode_set().
Fixes: 3c103110835d ("net/mlx5: SD, introduce Socket Direct LAG")
Signed-off-by: Shay Drory <shayd@nvidia.com>
Reviewed-by: Akiva Goldberger <agoldberger@nvidia.com>
Signed-off-by: Tariq Toukan <tariqt@nvidia.com>
---
.../net/ethernet/mellanox/mlx5/core/eswitch.c | 2 +-
.../net/ethernet/mellanox/mlx5/core/lag/lag.c | 30 +++++++++----------
.../net/ethernet/mellanox/mlx5/core/lib/sd.c | 13 ++++++++
3 files changed, 28 insertions(+), 17 deletions(-)
Internal sashiko comment:
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/lib/sd.c
b/drivers/net/ethernet/mellanox/mlx5/core/lib/sd.c
> index ee2fdefa1945..2b2b7f3916f7 100644
> --- a/drivers/net/ethernet/mellanox/mlx5/core/lib/sd.c
> +++ b/drivers/net/ethernet/mellanox/mlx5/core/lib/sd.c
> @@ -342,7 +342,14 @@ static void sd_lag_init(struct mlx5_core_dev *dev)
> return;
> }
>
> +recheck:
> mutex_lock(&ldev->lock);
> + if (ldev->mode_changes_in_progress) {
> + mutex_unlock(&ldev->lock);
> + msleep(100);
> + goto recheck;
> +
}
> +
Does this open-coded retry loop reimplement a wait mechanism without
immediate
wakeups or fairness?
It looks like we are polling the mode_changes_in_progress flag using a
hard
coded msleep(100). Could this unnecessarily delay the initialization
path if
the condition clears much sooner than 100ms? Would it be better to use a
proper
synchronization primitive like a waitqueue here instead of an ad-hoc
flag loop?
[SD] This is the same check as in mlx5_lag_remove_mdev().
I agree we need to change it, but this is net-next material
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/eswitch.c b/drivers/net/ethernet/mellanox/mlx5/core/eswitch.c
index b6e2c153b4f7..50e158b6a684 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/eswitch.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/eswitch.c
@@ -2062,8 +2062,8 @@ void mlx5_eswitch_disable(struct mlx5_eswitch *esw)
mlx5_esw_reps_unblock(esw);
esw->mode = MLX5_ESWITCH_LEGACY;
- mlx5_sd_eswitch_mode_set(esw->dev, MLX5_ESWITCH_LEGACY);
mlx5_lag_enable_change(esw->dev);
+ mlx5_sd_eswitch_mode_set(esw->dev, MLX5_ESWITCH_LEGACY);
}
static int mlx5_esw_sf_max_pf_functions(struct mlx5_core_dev *dev,
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/lag/lag.c b/drivers/net/ethernet/mellanox/mlx5/core/lag/lag.c
index 2285c889c215..aee5ce471eba 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/lag/lag.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/lag/lag.c
@@ -2589,6 +2589,8 @@ void mlx5_lag_disable_change(struct mlx5_core_dev *dev)
mpesw = ldev->mode == MLX5_LAG_MODE_MPESW;
if (mpesw)
mlx5_mpesw_sd_devcoms_lock(ldev);
+ else if (sd_devcom)
+ mlx5_devcom_comp_lock(sd_devcom);
mutex_lock(&ldev->lock);
ldev->mode_changes_in_progress++;
@@ -2599,26 +2601,22 @@ void mlx5_lag_disable_change(struct mlx5_core_dev *dev)
mlx5_disable_lag(ldev);
}
+ if (sd_devcom) {
+ mlx5_lag_for_each(i, 0, ldev, MLX5_LAG_FILTER_ALL) {
+ pf = mlx5_lag_pf(ldev, i);
+ if (pf->dev == dev && pf->sd_fdb_active) {
+ mlx5_lag_shared_fdb_destroy(ldev, pf->group_id);
+ break;
+ }
+ }
+ }
+
mutex_unlock(&ldev->lock);
if (mpesw)
mlx5_mpesw_sd_devcoms_unlock(ldev);
+ else if (sd_devcom)
+ mlx5_devcom_comp_unlock(sd_devcom);
mlx5_devcom_comp_unlock(primary->priv.hca_devcom_comp);
-
- if (!sd_devcom)
- return;
-
- /* Teardown SD shared FDB for this device's group if active */
- mlx5_devcom_comp_lock(sd_devcom);
- mutex_lock(&ldev->lock);
- mlx5_lag_for_each(i, 0, ldev, MLX5_LAG_FILTER_ALL) {
- pf = mlx5_lag_pf(ldev, i);
- if (pf->dev == dev && pf->sd_fdb_active) {
- mlx5_lag_shared_fdb_destroy(ldev, pf->group_id);
- break;
- }
- }
- mutex_unlock(&ldev->lock);
- mlx5_devcom_comp_unlock(sd_devcom);
}
void mlx5_lag_enable_change(struct mlx5_core_dev *dev)
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/lib/sd.c b/drivers/net/ethernet/mellanox/mlx5/core/lib/sd.c
index 4cdc50cd6f03..99cf455a61e1 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/lib/sd.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/lib/sd.c
@@ -345,7 +345,14 @@ static void sd_lag_init(struct mlx5_core_dev *dev)
return;
}
+recheck:
mutex_lock(&ldev->lock);
+ if (ldev->mode_changes_in_progress) {
+ mutex_unlock(&ldev->lock);
+ msleep(100);
+ goto recheck;
+ }
+
pf = mlx5_lag_pf_by_dev(ldev, primary);
if (!pf) {
sd_warn(primary, "%s: primary not registered in ldev, skipping\n",
@@ -388,7 +395,13 @@ static void sd_lag_cleanup(struct mlx5_core_dev *dev)
if (!ldev)
return;
+recheck:
mutex_lock(&ldev->lock);
+ if (ldev->mode_changes_in_progress) {
+ mutex_unlock(&ldev->lock);
+ msleep(100);
+ goto recheck;
+ }
mlx5_sd_for_each_secondary(i, primary, pos)
mlx5_ldev_remove_mdev(ldev, pos);
--
2.44.0
next prev parent reply other threads:[~2026-09-02 16:40 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-09-02 16:37 [PATCH net 0/4] net/mlx5: SD LAG and devcom stability fixes Tariq Toukan
2026-09-02 16:37 ` Tariq Toukan [this message]
2026-09-02 16:37 ` [PATCH net 2/4] net/mlx5: devcom, Base component size on linked devices Tariq Toukan
2026-09-02 16:37 ` [PATCH net 3/4] net/mlx5: SD, unload reps on shared FDB create error path Tariq Toukan
2026-09-02 16:37 ` [PATCH net 4/4] net/mlx5: LAG, reload IB reps of LAG master before the rest Tariq Toukan
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=20260902163716.3656279-2-tariqt@nvidia.com \
--to=tariqt@nvidia.com \
--cc=agoldberger@nvidia.com \
--cc=andrew+netdev@lunn.ch \
--cc=davem@davemloft.net \
--cc=edumazet@google.com \
--cc=edwards@nvidia.com \
--cc=gal@nvidia.com \
--cc=horms@kernel.org \
--cc=kees@kernel.org \
--cc=kuba@kernel.org \
--cc=leon@kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-rdma@vger.kernel.org \
--cc=mbloch@nvidia.com \
--cc=msanalla@nvidia.com \
--cc=netdev@vger.kernel.org \
--cc=ohartoov@nvidia.com \
--cc=pabeni@redhat.com \
--cc=parav@nvidia.com \
--cc=phaddad@nvidia.com \
--cc=saeedm@nvidia.com \
--cc=shayd@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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox