[PATCH net V3 0/8] net/mlx5e: Fix offload lifetime and exclusion bugs

[PATCH net V3 0/8] net/mlx5e: Fix offload lifetime and exclusion bugs

[Tariq Toukan]

Hi,

This series by Cosmin fixes several mlx5e offload issues.

1.   Fix fs root table cleanup bug.
2.   Fix IPsec encap serialization bugs.
3.   Fix a deadlock between HCA devcom lock and mode_lock.
4.   Fix uplink unbind deadlocks by not holding mode_lock across
     removal. All eswitch operation initiators wait for ongoing ones to
     finish first. Unblocked by 3.
5.   Fix races between profile cleanup/init and IPsec policy/state add.
     Unblocked by 4.
6+7. Block eswitch mode changes before accessing the IPsec context
     during policy and SA creation. Unblocked by 5.
8.   Serialize TC/IPsec exclusion counters with a per-device spinlock.
     Unblocked by 6+7.

Regards,
Tariq

V3:
- Added several new fixes, all found by internal Sashiko.
- Addressed a few Sashiko comments from V2.
- Renamed V2 patches 1-2 to "Block E-Switch mode changes during IPsec
  policy creation" and "... during IPsec state creation".
- Dropped V2 patch 4, "net/mlx5e: tc: Tie esw & accel blocking refs
  to the flow's lifetime", to be submitted separately.
- Exclusion counters: use a spinlock instead of a mutex, as some
  callers run in atomic context.

V2:
https://lore.kernel.org/netdev/20260930121119.141953-1-tariqt@nvidia.com/
- Use mlx5_esw_lock() in mlx5_eswitch_block_mode() (Sashiko)
- Clarified the fix split across first two patches (Sashiko)
- More careful dereferencing priv->mdev in mlx5e_xfrm_add_state() (Sashiko)
- Avoided spurious error message during IPsec SW fallback (Sashiko)
- Harmonized check_users arg for policy & state add (Sashiko)
- Clarified the scope of the "tc: tie..." fix (Sashiko)
- Moved macsec patches into a separate series.

V1:
https://lore.kernel.org/netdev/20260917175433.4090878-1-tariqt@nvidia.com/

Cosmin Ratiu (8):
  net/mlx5e: Release IPsec root table reference on RX setup failure
  net/mlx5e: Serialize ipsec encap reservations with eswitch operations
  net/mlx5e: Take HCA devcom lock before esw->mode_lock
  net/mlx5e: Fix E-Switch races in uplink unbind
  net/mlx5e: Fix ipsec core device access during profile changes
  net/mlx5e: Block E-Switch mode changes during IPsec policy creation
  net/mlx5e: Block E-Switch mode changes during IPsec state creation
  net/mlx5e: Serialize TC and IPsec offload exclusion counters

 .../mellanox/mlx5/core/en_accel/ipsec.c       | 95 +++++++++++++------
 .../mellanox/mlx5/core/en_accel/ipsec.h       |  2 +-
 .../mellanox/mlx5/core/en_accel/ipsec_fs.c    | 88 ++++++++---------
 .../net/ethernet/mellanox/mlx5/core/en_main.c |  4 +-
 .../net/ethernet/mellanox/mlx5/core/en_tc.c   | 18 ++--
 .../mellanox/mlx5/core/esw/ipsec_fs.c         | 15 ++-
 .../mellanox/mlx5/core/esw/ipsec_fs.h         |  6 ++
 .../net/ethernet/mellanox/mlx5/core/eswitch.c | 24 +++++
 .../net/ethernet/mellanox/mlx5/core/eswitch.h | 15 ++-
 .../mellanox/mlx5/core/eswitch_offloads.c     | 88 +++++++++--------
 .../net/ethernet/mellanox/mlx5/core/main.c    |  1 +
 include/linux/mlx5/driver.h                   |  8 +-
 12 files changed, 231 insertions(+), 133 deletions(-)


base-commit: fafbc4051123d5459ea6950899cec08ed946e278
-- 
2.44.0

[PATCH net V3 1/8] net/mlx5e: Release IPsec root table reference on RX setup failure

[Tariq Toukan]

From: Cosmin Ratiu <cratiu@nvidia.com>

Switchdev RX setup takes a root-chain table reference before creating
its flow tables. If creation fails, rx_get() returns without
incrementing the RX reference count, so rx_destroy() never releases the
chain reference.

Extract the cleanup logic in a counter-partner to the function acquiring
the root table ref, and call it on error to properly release everything.
Also call it from the normal destroy path to reduce code duplication.

mlx5_esw_ipsec_rx_status_pass_dest_get() ignored chain table acquisition
errors, returning success with an error pointer as the destination. The
new unwind could then release a reference that was never taken.
Propagate the error so cleanup only runs after a successful get.

Discovered by Sashiko. Not triggered as it's an unlikely error path.

Fixes: 1762f132d542 ("net/mlx5e: Support IPsec packet offload for RX in switchdev mode")
Signed-off-by: Cosmin Ratiu <cratiu@nvidia.com>
Reviewed-by: Jianbo Liu <jianbol@nvidia.com>
Signed-off-by: Tariq Toukan <tariqt@nvidia.com>
---
 .../mellanox/mlx5/core/en_accel/ipsec_fs.c    | 20 +++++++++++--------
 .../mellanox/mlx5/core/esw/ipsec_fs.c         | 15 ++++++++++++--
 .../mellanox/mlx5/core/esw/ipsec_fs.h         |  6 ++++++
 3 files changed, 31 insertions(+), 10 deletions(-)

diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_accel/ipsec_fs.c b/drivers/net/ethernet/mellanox/mlx5/core/en_accel/ipsec_fs.c
index 8ffa8068e90a..fd9fbf359333 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/en_accel/ipsec_fs.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/en_accel/ipsec_fs.c
@@ -720,6 +720,16 @@ static void ipsec_rx_sa_selector_destroy(struct mlx5_core_dev *mdev,
 	mlx5_destroy_flow_table(rx->ft.sa_sel);
 }
 
+static void
+ipsec_rx_status_pass_dest_put(struct mlx5e_ipsec *ipsec,
+			      struct mlx5e_ipsec_rx *rx,
+			      u32 family)
+{
+	mlx5_ipsec_fs_roce_rx_destroy(ipsec->roce, family, ipsec->mdev);
+	if (rx == ipsec->rx_esw)
+		mlx5_esw_ipsec_rx_status_pass_dest_put(ipsec);
+}
+
 static void rx_destroy(struct mlx5_core_dev *mdev, struct mlx5e_ipsec *ipsec,
 		       struct mlx5e_ipsec_rx *rx, u32 family)
 {
@@ -739,13 +749,7 @@ static void rx_destroy(struct mlx5_core_dev *mdev, struct mlx5e_ipsec *ipsec,
 
 	ipsec_rx_policy_destroy(rx);
 
-	mlx5_ipsec_fs_roce_rx_destroy(ipsec->roce, family, mdev);
-
-#ifdef CONFIG_MLX5_ESWITCH
-	if (rx == ipsec->rx_esw)
-		mlx5_chains_put_table(esw_chains(ipsec->mdev->priv.eswitch),
-				      0, 1, 0);
-#endif
+	ipsec_rx_status_pass_dest_put(ipsec, rx, family);
 }
 
 static void ipsec_rx_create_attr_set(struct mlx5e_ipsec *ipsec,
@@ -1115,7 +1119,7 @@ static int rx_create(struct mlx5_core_dev *mdev, struct mlx5e_ipsec *ipsec,
 err_fs_ft_sa_sel:
 	mlx5_destroy_flow_table(rx->ft.status);
 err_fs_ft_status:
-	mlx5_ipsec_fs_roce_rx_destroy(ipsec->roce, family, mdev);
+	ipsec_rx_status_pass_dest_put(ipsec, rx, family);
 	return err;
 }
 
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/esw/ipsec_fs.c b/drivers/net/ethernet/mellanox/mlx5/core/esw/ipsec_fs.c
index ab50d2c734ed..f8c97954b6ea 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/esw/ipsec_fs.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/esw/ipsec_fs.c
@@ -34,12 +34,23 @@ void mlx5_esw_ipsec_rx_create_attr_set(struct mlx5e_ipsec *ipsec,
 int mlx5_esw_ipsec_rx_status_pass_dest_get(struct mlx5e_ipsec *ipsec,
 					   struct mlx5_flow_destination *dest)
 {
-	dest->type = MLX5_FLOW_DESTINATION_TYPE_FLOW_TABLE;
-	dest->ft = mlx5_chains_get_table(esw_chains(ipsec->mdev->priv.eswitch), 0, 1, 0);
+	struct mlx5_flow_table *ft;
+
+	ft = mlx5_chains_get_table(esw_chains(ipsec->mdev->priv.eswitch),
+				   0, 1, 0);
+	if (IS_ERR(ft))
+		return PTR_ERR(ft);
 
+	dest->type = MLX5_FLOW_DESTINATION_TYPE_FLOW_TABLE;
+	dest->ft = ft;
 	return 0;
 }
 
+void mlx5_esw_ipsec_rx_status_pass_dest_put(struct mlx5e_ipsec *ipsec)
+{
+	mlx5_chains_put_table(esw_chains(ipsec->mdev->priv.eswitch), 0, 1, 0);
+}
+
 int mlx5_esw_ipsec_rx_setup_modify_header(struct mlx5e_ipsec_sa_entry *sa_entry,
 					  struct mlx5_flow_act *flow_act)
 {
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/esw/ipsec_fs.h b/drivers/net/ethernet/mellanox/mlx5/core/esw/ipsec_fs.h
index 514c15258b1d..bb6f2423078b 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/esw/ipsec_fs.h
+++ b/drivers/net/ethernet/mellanox/mlx5/core/esw/ipsec_fs.h
@@ -12,6 +12,7 @@ void mlx5_esw_ipsec_rx_create_attr_set(struct mlx5e_ipsec *ipsec,
 				       struct mlx5e_ipsec_rx_create_attr *attr);
 int mlx5_esw_ipsec_rx_status_pass_dest_get(struct mlx5e_ipsec *ipsec,
 					   struct mlx5_flow_destination *dest);
+void mlx5_esw_ipsec_rx_status_pass_dest_put(struct mlx5e_ipsec *ipsec);
 int mlx5_esw_ipsec_rx_setup_modify_header(struct mlx5e_ipsec_sa_entry *sa_entry,
 					  struct mlx5_flow_act *flow_act);
 void mlx5_esw_ipsec_rx_id_mapping_remove(struct mlx5e_ipsec_sa_entry *sa_entry);
@@ -32,6 +33,11 @@ static inline int mlx5_esw_ipsec_rx_status_pass_dest_get(struct mlx5e_ipsec *ips
 	return -EINVAL;
 }
 
+static inline
+void mlx5_esw_ipsec_rx_status_pass_dest_put(struct mlx5e_ipsec *ipsec)
+{
+}
+
 static inline int mlx5_esw_ipsec_rx_setup_modify_header(struct mlx5e_ipsec_sa_entry *sa_entry,
 							struct mlx5_flow_act *flow_act)
 {
-- 
2.44.0

[PATCH net V3 2/8] net/mlx5e: Serialize ipsec encap reservations with eswitch operations

[Tariq Toukan]

From: Cosmin Ratiu <cratiu@nvidia.com>

mlx5_eswitch_block_encap() takes mode_lock but ignores
eswitch_operation_in_progress. mlx5_devlink_eswitch_encap_mode_set()
claims the operation and drops mode_lock while rebuilding the FDB and
updating offloads.encap, so a reservation can read a stale or
rolled-back encap value. In diagram form:

T1: first IPsec SA (NIC table)   T2: encap none -> basic
                                 mlx5_devlink_eswitch_encap_mode_set()
                                   down_write(&esw->mode_lock)
                                   sees num_block_encap == 0
                                   eswitch_operation_in_progress = true
                                   up_write(&esw->mode_lock)
                                   esw_destroy_offloads_fdb_tables()
mlx5e_xfrm_add_state()
  mlx5e_ipsec_fs_tunnel_allowed()
    mlx5_eswitch_block_encap()
      down_write(&esw->mode_lock)
      reads encap == NONE
      num_block_encap++
      up_write(&esw->mode_lock)
                                   esw->offloads.encap = BASIC
                                   esw_create_offloads_fdb_tables()

The race happens therefore because offloads.encap is not consistently
accessed under mode_lock. There are other variations of the above race
(e.g. in rollback, or in tx_create()).

Use mlx5_esw_lock() to reject a reservation while an operation is in
progress. This implies changing mlx5e_ipsec_fs_tunnel_allowed() to
propagate the error. Note that these are not new failure modes as it
seems on first glance, because if the eswitch_operation_in_progress flag
is set, rx_get()/tx_get()->mlx5_eswitch_block_mode() already fails with
-EBUSY. The new code simply protects against an operation initiated in
the short window after mlx5_eswitch_block_mode().

This race was found by Sashiko during inspection of another patch of
this series. No attempt has been made to trigger it, as it requires very
precise timing.

Fixes: baac8351f74c ("net/mlx5e: Reduce eswitch mode_lock protection context")
Signed-off-by: Cosmin Ratiu <cratiu@nvidia.com>
Reviewed-by: Jianbo Liu <jianbol@nvidia.com>
Signed-off-by: Tariq Toukan <tariqt@nvidia.com>
---
 .../mellanox/mlx5/core/en_accel/ipsec.c       |  5 ++++-
 .../mellanox/mlx5/core/en_accel/ipsec.h       |  2 +-
 .../mellanox/mlx5/core/en_accel/ipsec_fs.c    | 21 ++++++++++++-------
 .../net/ethernet/mellanox/mlx5/core/eswitch.h |  6 +++---
 .../mellanox/mlx5/core/eswitch_offloads.c     | 20 +++++++++++-------
 5 files changed, 35 insertions(+), 19 deletions(-)

diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_accel/ipsec.c b/drivers/net/ethernet/mellanox/mlx5/core/en_accel/ipsec.c
index 7e991a09b3ff..7bc5d07b4321 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/en_accel/ipsec.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/en_accel/ipsec.c
@@ -812,7 +812,10 @@ static int mlx5e_xfrm_add_state(struct net_device *dev,
 
 	if (x->props.mode == XFRM_MODE_TUNNEL &&
 	    x->xso.type == XFRM_DEV_OFFLOAD_PACKET) {
-		allow_tunnel_mode = mlx5e_ipsec_fs_tunnel_allowed(sa_entry);
+		err = mlx5e_ipsec_fs_tunnel_allowed(sa_entry);
+		if (err < 0)
+			goto unblock_mode;
+		allow_tunnel_mode = err;
 		if (!allow_tunnel_mode) {
 			NL_SET_ERR_MSG_MOD(extack,
 					   "Packet offload tunnel mode is disabled due to encap settings");
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_accel/ipsec.h b/drivers/net/ethernet/mellanox/mlx5/core/en_accel/ipsec.h
index abcbd38db9db..f4b2d7b950c0 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/en_accel/ipsec.h
+++ b/drivers/net/ethernet/mellanox/mlx5/core/en_accel/ipsec.h
@@ -320,7 +320,7 @@ void mlx5e_accel_ipsec_fs_del_rule(struct mlx5e_ipsec_sa_entry *sa_entry);
 int mlx5e_accel_ipsec_fs_add_pol(struct mlx5e_ipsec_pol_entry *pol_entry);
 void mlx5e_accel_ipsec_fs_del_pol(struct mlx5e_ipsec_pol_entry *pol_entry);
 void mlx5e_accel_ipsec_fs_modify(struct mlx5e_ipsec_sa_entry *sa_entry);
-bool mlx5e_ipsec_fs_tunnel_allowed(struct mlx5e_ipsec_sa_entry *sa_entry);
+int mlx5e_ipsec_fs_tunnel_allowed(struct mlx5e_ipsec_sa_entry *sa_entry);
 
 int mlx5_ipsec_create_sa_ctx(struct mlx5e_ipsec_sa_entry *sa_entry);
 void mlx5_ipsec_free_sa_ctx(struct mlx5e_ipsec_sa_entry *sa_entry);
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_accel/ipsec_fs.c b/drivers/net/ethernet/mellanox/mlx5/core/en_accel/ipsec_fs.c
index fd9fbf359333..1529d4730323 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/en_accel/ipsec_fs.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/en_accel/ipsec_fs.c
@@ -1072,9 +1072,12 @@ static int rx_create(struct mlx5_core_dev *mdev, struct mlx5e_ipsec *ipsec,
 		goto err_fs_ft_sa_sel;
 
 	/* Create FT */
-	if (mlx5_ipsec_device_caps(mdev) & MLX5_IPSEC_CAP_TUNNEL)
-		rx->allow_tunnel_mode =
-			mlx5_eswitch_block_encap(mdev, rx == ipsec->rx_esw);
+	if (mlx5_ipsec_device_caps(mdev) & MLX5_IPSEC_CAP_TUNNEL) {
+		err = mlx5_eswitch_block_encap(mdev, rx == ipsec->rx_esw);
+		if (err < 0)
+			goto err_fs_ft;
+		rx->allow_tunnel_mode = err;
+	}
 
 	if (rx->allow_tunnel_mode)
 		flags = MLX5_FLOW_TABLE_TUNNEL_EN_REFORMAT;
@@ -1315,9 +1318,12 @@ static int tx_create(struct mlx5e_ipsec *ipsec, struct mlx5e_ipsec_tx *tx,
 	if (err)
 		goto err_status_rule;
 
-	if (mlx5_ipsec_device_caps(mdev) & MLX5_IPSEC_CAP_TUNNEL)
-		tx->allow_tunnel_mode =
-			mlx5_eswitch_block_encap(mdev, tx == ipsec->tx_esw);
+	if (mlx5_ipsec_device_caps(mdev) & MLX5_IPSEC_CAP_TUNNEL) {
+		err = mlx5_eswitch_block_encap(mdev, tx == ipsec->tx_esw);
+		if (err < 0)
+			goto err_encap;
+		tx->allow_tunnel_mode = err;
+	}
 
 	if (tx->allow_tunnel_mode)
 		flags = MLX5_FLOW_TABLE_TUNNEL_EN_REFORMAT;
@@ -1387,6 +1393,7 @@ static int tx_create(struct mlx5e_ipsec *ipsec, struct mlx5e_ipsec_tx *tx,
 err_sa_ft:
 	if (tx->allow_tunnel_mode)
 		mlx5_eswitch_unblock_encap(mdev);
+err_encap:
 	mlx5_del_flow_rules(tx->status.rule);
 err_status_rule:
 	mlx5_destroy_flow_table(tx->ft.status);
@@ -2854,7 +2861,7 @@ void mlx5e_accel_ipsec_fs_modify(struct mlx5e_ipsec_sa_entry *sa_entry)
 	memcpy(sa_entry, &sa_entry_shadow, sizeof(*sa_entry));
 }
 
-bool mlx5e_ipsec_fs_tunnel_allowed(struct mlx5e_ipsec_sa_entry *sa_entry)
+int mlx5e_ipsec_fs_tunnel_allowed(struct mlx5e_ipsec_sa_entry *sa_entry)
 {
 	struct mlx5e_ipsec *ipsec = sa_entry->ipsec;
 	struct xfrm_state *x = sa_entry->x;
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/eswitch.h b/drivers/net/ethernet/mellanox/mlx5/core/eswitch.h
index e9cbcd73b23f..928b20e73d37 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/eswitch.h
+++ b/drivers/net/ethernet/mellanox/mlx5/core/eswitch.h
@@ -967,7 +967,7 @@ void mlx5_eswitch_unload_reps(struct mlx5_eswitch *esw);
 bool mlx5_eswitch_is_peer(struct mlx5_eswitch *esw,
 			  struct mlx5_eswitch *peer_esw);
 
-bool mlx5_eswitch_block_encap(struct mlx5_core_dev *dev, bool from_fdb);
+int mlx5_eswitch_block_encap(struct mlx5_core_dev *dev, bool from_fdb);
 void mlx5_eswitch_unblock_encap(struct mlx5_core_dev *dev);
 
 int mlx5_eswitch_block_mode(struct mlx5_core_dev *dev);
@@ -1071,10 +1071,10 @@ mlx5_eswitch_reload_ib_reps(struct mlx5_eswitch *esw)
 static inline void
 mlx5_eswitch_unload_reps(struct mlx5_eswitch *esw) {}
 
-static inline bool
+static inline int
 mlx5_eswitch_block_encap(struct mlx5_core_dev *dev, bool from_fdb)
 {
-	return true;
+	return 1;
 }
 
 static inline void mlx5_eswitch_unblock_encap(struct mlx5_core_dev *dev)
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/eswitch_offloads.c b/drivers/net/ethernet/mellanox/mlx5/core/eswitch_offloads.c
index c712848b202d..7a0131e26ff0 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/eswitch_offloads.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/eswitch_offloads.c
@@ -4712,25 +4712,31 @@ int mlx5_devlink_eswitch_inline_mode_get(struct devlink *devlink, u8 *mode)
 	return esw_inline_mode_to_devlink(esw->offloads.inline_mode, mode);
 }
 
-bool mlx5_eswitch_block_encap(struct mlx5_core_dev *dev, bool from_fdb)
+/* Return 1 if allowed, 0 if disallowed by the configuration, or -EBUSY
+ * if an eswitch operation is in progress.
+ */
+int mlx5_eswitch_block_encap(struct mlx5_core_dev *dev, bool from_fdb)
 {
 	struct mlx5_eswitch *esw = dev->priv.eswitch;
 	enum devlink_eswitch_encap_mode encap;
-	bool allow_tunnel = false;
+	int ret;
 
 	if (!mlx5_esw_allowed(esw))
-		return true;
+		return 1;
+
+	ret = mlx5_esw_lock(esw);
+	if (ret)
+		return ret;
 
-	down_write(&esw->mode_lock);
 	encap = esw->offloads.encap;
 	if (esw->mode == MLX5_ESWITCH_LEGACY ||
 	    (encap == DEVLINK_ESWITCH_ENCAP_MODE_NONE && !from_fdb)) {
-		allow_tunnel = true;
+		ret = 1;
 		esw->offloads.num_block_encap++;
 	}
-	up_write(&esw->mode_lock);
+	mlx5_esw_unlock(esw);
 
-	return allow_tunnel;
+	return ret;
 }
 
 void mlx5_eswitch_unblock_encap(struct mlx5_core_dev *dev)
-- 
2.44.0

[PATCH net V3 3/8] net/mlx5e: Take HCA devcom lock before esw->mode_lock

[Tariq Toukan]

From: Cosmin Ratiu <cratiu@nvidia.com>

Uplink ETH driver removal waits for any eswitch operation in progress
before removing REPs. Removal through mlx5_unregister_device() or
mlx5_detach_device() does this while holding the HCA devcom lock, which
is used by peer devices in the same group.

An E-Switch mode change can therefore race with a Socket Direct
secondary device removal:

T1: primary mode set             T2: SD secondary dev removal
mlx5_devlink_eswitch_mode_set()
  sets operation in progress
                                 mlx5_unregister_device()
                                   mlx5_devcom_comp_lock(hca)
                                   ...
                                   mlx5e_remove()
                                     mlx5_sd_get_adev() -> T1 dev
                                     mlx5_eswitch_safe_aux_devs_remove()
                                       waits for T1's operation
  esw_offloads_start()
    esw_mode_change()
      mlx5_devcom_comp_lock(hca)
        waits for T2's lock

The timeout in mlx5_eswitch_safe_aux_devs_remove() makes this not a
deadlock, but representor removal is skipped in that case which is
incorrect.

This bug was found by Sashiko inspecting the next patch in the series,
which removes the timeout. It wasn't reproduced but the scenario seems
plausible, albeit unlikely. This was undetectable by lockdep because the
down_read_trylock() in mlx5_esw_hold() doesn't register as a lock edge,
and also because it can't see the wait on the flag as a lock edge.

Fix the lock order so the HCA devcom lock is taken in
mlx5_devlink_eswitch_mode_set() before esw->mode_lock and held for the
duration of the mode change.
Expanding the HCA devcom lock critical section implies new lock edges
being added, but this is safe:
- mlx5_devlink_netdev_netns_immutable_set() adds HCA devcom -> RTNL, but
  there's already a HCA devcom -> RTNL dependency in
  mlx5_register_device() -..-> register_netdev()
- esw_offloads_disable/enable() -> esw devcom cleanup/init: adds HCA
  devcom -> esw devcom. Nothing takes the HCA devcom lock while holding
  the esw devcom lock.

Fixes: 2ae8c7edea87 ("net/mlx5: Fix Unbinding uplink-netdev in switchdev mode")
Signed-off-by: Cosmin Ratiu <cratiu@nvidia.com>
Reviewed-by: Jianbo Liu <jianbol@nvidia.com>
Signed-off-by: Tariq Toukan <tariqt@nvidia.com>
---
 .../net/ethernet/mellanox/mlx5/core/eswitch_offloads.c   | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/drivers/net/ethernet/mellanox/mlx5/core/eswitch_offloads.c b/drivers/net/ethernet/mellanox/mlx5/core/eswitch_offloads.c
index 7a0131e26ff0..4939e10336c2 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/eswitch_offloads.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/eswitch_offloads.c
@@ -2457,8 +2457,8 @@ void mlx5_esw_reps_unblock(struct mlx5_eswitch *esw)
 
 static void esw_mode_change(struct mlx5_eswitch *esw, u16 mode)
 {
+	mlx5_devcom_comp_assert_locked(esw->dev->priv.hca_devcom_comp);
 	mlx5_esw_reps_unblock(esw);
-	mlx5_devcom_comp_lock(esw->dev->priv.hca_devcom_comp);
 	if (esw->dev->priv.flags & MLX5_PRIV_FLAGS_DISABLE_IB_ADEV ||
 	    mlx5_core_mp_enabled(esw->dev)) {
 		esw->mode = mode;
@@ -2471,7 +2471,6 @@ static void esw_mode_change(struct mlx5_eswitch *esw, u16 mode)
 	esw->dev->priv.flags &= ~MLX5_PRIV_FLAGS_DISABLE_IB_ADEV;
 out:
 	mlx5_rescan_drivers_locked(esw->dev);
-	mlx5_devcom_comp_unlock(esw->dev->priv.hca_devcom_comp);
 	mlx5_esw_reps_block(esw);
 }
 
@@ -4508,10 +4507,11 @@ int mlx5_devlink_eswitch_mode_set(struct devlink *devlink, u16 mode,
 		return 0;
 
 	mlx5_lag_disable_change(esw->dev);
+	mlx5_devcom_comp_lock(esw->dev->priv.hca_devcom_comp);
 	err = mlx5_esw_try_lock(esw);
 	if (err < 0) {
 		NL_SET_ERR_MSG_MOD(extack, "Can't change mode, E-Switch is busy");
-		goto enable_lag;
+		goto unlock_hca;
 	}
 	cur_mlx5_mode = err;
 	err = 0;
@@ -4574,7 +4574,8 @@ int mlx5_devlink_eswitch_mode_set(struct devlink *devlink, u16 mode,
 	esw->eswitch_operation_in_progress = false;
 unlock:
 	mlx5_esw_unlock(esw);
-enable_lag:
+unlock_hca:
+	mlx5_devcom_comp_unlock(esw->dev->priv.hca_devcom_comp);
 	mlx5_lag_enable_change(esw->dev);
 	/* Shared FDB activation is creating LAG which is changing reps. */
 	if (!err)
-- 
2.44.0

[PATCH net V3 4/8] net/mlx5e: Fix E-Switch races in uplink unbind

[Tariq Toukan]

From: Cosmin Ratiu <cratiu@nvidia.com>

Unbinding the uplink Ethernet auxiliary driver in switchdev mode with
offloaded IPsec policies deadlocks on the esw->mode_lock semaphore:
mlx5_eswitch_safe_aux_devs_remove()
  mlx5_esw_hold()
    down_read_trylock(&esw->mode_lock)                 (*)
  mlx5_core_reps_aux_devs_remove()
    ...
    mlx5e_vport_uplink_rep_unload()
      unregister_netdev()
        xfrm_dev_unregister()
          xfrm_dev_policy_flush()
            xfrm_policy_kill()
              xfrm_dev_policy_delete()
                mlx5e_xfrm_del_policy()
                  mlx5e_accel_ipsec_fs_del_pol()
                    rx_ft_put_policy()
                      rx_put()
                        mlx5_eswitch_unblock_mode()
                          down_write(&esw->mode_lock)  (BOOM)

Another deadlock with IPsec states:
mlx5_eswitch_safe_aux_devs_remove()
  mlx5_esw_hold()
    down_read_trylock(&esw->mode_lock)                (*)
  mlx5_core_reps_aux_devs_remove()
    ...
    mlx5e_vport_uplink_rep_unload()
      unregister_netdev()
        xfrm_dev_unregister()
          xfrm_dev_state_flush()
            xfrm_dev_state_free()
              mlx5e_xfrm_free_state()
                mlx5e_accel_ipsec_fs_del_rule()
                  rx_ft_put()
                    rx_put()
                      rx_destroy()
                        mlx5_eswitch_unblock_encap()
                          down_write(&esw->mode_lock)  (BOOM)

To fix these, first refactor initiators of eswitch operations so they no
longer step on each other's toes.

The active/inactive toggle and setting the inline/encap mode now use the
new mlx5_esw_lock_wait(), which waits for an operation in progress to
finish. They all hold devl_lock, so the only operation they can find in
progress is an uplink unbind, which doesn't need devl_lock.
Setting the esw mode keeps using mlx5_esw_try_lock(), which also rejects
the change while eswitch users exist and bails out with -EBUSY.

mlx5_eswitch_safe_aux_devs_remove() now also waits for an operation in
progress with the same mlx5_esw_lock_wait() before initiating one,
dropping mode_lock and then removing representors.
The deadlocks are avoided.

Finishing an operation in progress now requires mlx5_esw_unlock() to
wake up any waiters instead of plain up_write(&mode_lock).

As a side effect, the timeout in mlx5_eswitch_safe_aux_devs_remove() is
gone, which fixes another scary corner case, of representor devs not
being removed on timeout but leaked instead. The timeout hid the SD
deadlock fixed in the previous patch. Now that that's gone, no reason
for the timeout to stay.

This was found by internal testing and reproduced on HW.
It was independently found by Sashiko during analysis of another patch
in this series.

Fixes: 2ae8c7edea87 ("net/mlx5: Fix Unbinding uplink-netdev in switchdev mode")
Signed-off-by: Cosmin Ratiu <cratiu@nvidia.com>
Reviewed-by: Jianbo Liu <jianbol@nvidia.com>
Signed-off-by: Tariq Toukan <tariqt@nvidia.com>
---
 .../net/ethernet/mellanox/mlx5/core/eswitch.c | 24 +++++++++
 .../net/ethernet/mellanox/mlx5/core/eswitch.h |  1 +
 .../mellanox/mlx5/core/eswitch_offloads.c     | 50 +++++++++----------
 3 files changed, 48 insertions(+), 27 deletions(-)

diff --git a/drivers/net/ethernet/mellanox/mlx5/core/eswitch.c b/drivers/net/ethernet/mellanox/mlx5/core/eswitch.c
index 2d289fb1353b..f50993cfcf7a 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/eswitch.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/eswitch.c
@@ -3051,12 +3051,36 @@ int mlx5_esw_lock(struct mlx5_eswitch *esw)
 	return 0;
 }
 
+static void esw_mode_lock(struct mlx5_eswitch *esw)
+{
+	down_write(&esw->mode_lock);
+}
+
+static void esw_mode_unlock(struct mlx5_eswitch *esw)
+{
+	up_write(&esw->mode_lock);
+}
+
+/* Lock the eswitch and wait for an operation in progress to finish. */
+void mlx5_esw_lock_wait(struct mlx5_eswitch *esw)
+{
+	down_write(&esw->mode_lock);
+	wait_var_event_any_lock(&esw->eswitch_operation_in_progress,
+				!esw->eswitch_operation_in_progress,
+				esw, esw_mode, TASK_UNINTERRUPTIBLE);
+}
+
 /**
  * mlx5_esw_unlock() - Release write lock on esw mode lock
  * @esw: eswitch device.
+ *
+ * Also wakes up mlx5_esw_lock_wait() waiters, so it must be used to release
+ * mode_lock after clearing eswitch_operation_in_progress.
  */
 void mlx5_esw_unlock(struct mlx5_eswitch *esw)
 {
+	wake_up_var_locked(&esw->eswitch_operation_in_progress,
+			   &esw->mode_lock);
 	up_write(&esw->mode_lock);
 }
 
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/eswitch.h b/drivers/net/ethernet/mellanox/mlx5/core/eswitch.h
index 928b20e73d37..1fb67fc6d49a 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/eswitch.h
+++ b/drivers/net/ethernet/mellanox/mlx5/core/eswitch.h
@@ -948,6 +948,7 @@ void mlx5_esw_get(struct mlx5_core_dev *dev);
 void mlx5_esw_put(struct mlx5_core_dev *dev);
 int mlx5_esw_try_lock(struct mlx5_eswitch *esw);
 int mlx5_esw_lock(struct mlx5_eswitch *esw);
+void mlx5_esw_lock_wait(struct mlx5_eswitch *esw);
 void mlx5_esw_unlock(struct mlx5_eswitch *esw);
 
 void esw_vport_change_handle_locked(struct mlx5_vport *vport);
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/eswitch_offloads.c b/drivers/net/ethernet/mellanox/mlx5/core/eswitch_offloads.c
index 4939e10336c2..4da89fcf215b 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/eswitch_offloads.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/eswitch_offloads.c
@@ -4441,7 +4441,7 @@ static bool mlx5_devlink_switchdev_active_mode_change(struct mlx5_eswitch *esw,
 	    !esw->offloads_inactive)
 		return false;
 
-	down_write(&esw->mode_lock);
+	mlx5_esw_lock_wait(esw);
 	esw->offloads_inactive = !esw->offloads_inactive;
 	esw->eswitch_operation_in_progress = true;
 	up_write(&esw->mode_lock);
@@ -4453,34 +4453,30 @@ static bool mlx5_devlink_switchdev_active_mode_change(struct mlx5_eswitch *esw,
 
 	down_write(&esw->mode_lock);
 	esw->eswitch_operation_in_progress = false;
-	up_write(&esw->mode_lock);
+	mlx5_esw_unlock(esw);
 	return true;
 }
 
-#define MLX5_ESW_HOLD_TIMEOUT_MS 7000
-#define MLX5_ESW_HOLD_RETRY_DELAY_MS 500
-
 void mlx5_eswitch_safe_aux_devs_remove(struct mlx5_core_dev *dev)
 {
-	unsigned long timeout;
-	bool hold_esw = true;
+	struct mlx5_eswitch *esw = dev->priv.eswitch;
 
-	/* Wait for any concurrent eswitch mode transition to complete. */
-	if (!mlx5_esw_hold(dev)) {
-		timeout = jiffies + msecs_to_jiffies(MLX5_ESW_HOLD_TIMEOUT_MS);
-		while (!mlx5_esw_hold(dev)) {
-			if (!time_before(jiffies, timeout)) {
-				hold_esw = false;
-				break;
-			}
-			msleep(MLX5_ESW_HOLD_RETRY_DELAY_MS);
-		}
-	}
-	if (hold_esw) {
-		if (mlx5_eswitch_mode(dev) == MLX5_ESWITCH_OFFLOADS)
-			mlx5_core_reps_aux_devs_remove(dev);
-		mlx5_esw_release(dev);
-	}
+	if (!mlx5_esw_allowed(esw))
+		return;
+
+	/* Wait for any concurrent eswitch mode transition to complete.
+	 * Deadlock risk if ever called from within an operation in progress.
+	 */
+	mlx5_esw_lock_wait(esw);
+	esw->eswitch_operation_in_progress = true;
+	up_write(&esw->mode_lock);
+
+	if (mlx5_eswitch_mode(dev) == MLX5_ESWITCH_OFFLOADS)
+		mlx5_core_reps_aux_devs_remove(dev);
+
+	down_write(&esw->mode_lock);
+	esw->eswitch_operation_in_progress = false;
+	mlx5_esw_unlock(esw);
 }
 
 int mlx5_devlink_eswitch_mode_set(struct devlink *devlink, u16 mode,
@@ -4656,7 +4652,7 @@ int mlx5_devlink_eswitch_inline_mode_set(struct devlink *devlink, u8 mode,
 	if (IS_ERR(esw))
 		return PTR_ERR(esw);
 
-	down_write(&esw->mode_lock);
+	mlx5_esw_lock_wait(esw);
 
 	switch (MLX5_CAP_ETH(dev, wqe_inline_mode)) {
 	case MLX5_CAP_INLINE_MODE_NOT_REQUIRED:
@@ -4694,7 +4690,7 @@ int mlx5_devlink_eswitch_inline_mode_set(struct devlink *devlink, u8 mode,
 
 	down_write(&esw->mode_lock);
 	esw->eswitch_operation_in_progress = false;
-	up_write(&esw->mode_lock);
+	mlx5_esw_unlock(esw);
 	return 0;
 
 out:
@@ -4764,7 +4760,7 @@ int mlx5_devlink_eswitch_encap_mode_set(struct devlink *devlink,
 	if (IS_ERR(esw))
 		return PTR_ERR(esw);
 
-	down_write(&esw->mode_lock);
+	mlx5_esw_lock_wait(esw);
 
 	if (encap != DEVLINK_ESWITCH_ENCAP_MODE_NONE &&
 	    (!MLX5_CAP_ESW_FLOWTABLE_FDB(dev, reformat) ||
@@ -4820,7 +4816,7 @@ int mlx5_devlink_eswitch_encap_mode_set(struct devlink *devlink,
 	esw->eswitch_operation_in_progress = false;
 
 unlock:
-	up_write(&esw->mode_lock);
+	mlx5_esw_unlock(esw);
 	return err;
 }
 
-- 
2.44.0

[PATCH net V3 5/8] net/mlx5e: Fix ipsec core device access during profile changes

[Tariq Toukan]

From: Cosmin Ratiu <cratiu@nvidia.com>

Devlink E-Switch mode changes replace the netdev profile without
unregistering the netdev. XFRM state and policy creation can therefore
run while mlx5e_priv_cleanup() clears priv with memset() or
mlx5e_priv_init() initializes it again. Accesses to priv->mdev can
observe NULL or a torn pointer, and repeated loads may return different
values.

Read priv->mdev once in each add callback and use the saved pointer
throughout the callback, including error unwinding. The core device
survives the in-place profile change, so a saved non-NULL pointer
remains valid even if priv is cleared in the meantime.

A NULL priv->mdev means a profile change is in progress, which only
happens during an eswitch mode change. Return -EBUSY in that case, as
mlx5_eswitch_block_mode() does for the same mode change. -EOPNOTSUPP
would make xfrm_dev_state_add() silently fall back to software for a
crypto offload SA for its whole lifetime.

Use WRITE_ONCE() when initializing priv->mdev and when clearing it
before memset(). This makes the transition to NULL atomic; the
subsequent bulk clear only writes zero bytes over an already NULL
pointer. Annotate the READ_ONCE() snapshots with data_race() to allow
that harmless overlap with the unmarked memset() without suppressing
checks for the rest of priv.

This fixes the core-device pointer accesses. Protecting the IPsec
context itself during state and policy creation is handled separately.

This was found by Sashiko, during review of the next patches in the
series. No attempt was made at reproducing this race, the chances seem
vanishingly small.

Fixes: 7a9fb35e8c3a ("net/mlx5e: Do not reload ethernet ports when changing eswitch mode")
Signed-off-by: Cosmin Ratiu <cratiu@nvidia.com>
Reviewed-by: Jianbo Liu <jianbol@nvidia.com>
Signed-off-by: Tariq Toukan <tariqt@nvidia.com>
---
 .../mellanox/mlx5/core/en_accel/ipsec.c       | 38 +++++++++++++------
 .../net/ethernet/mellanox/mlx5/core/en_main.c |  4 +-
 2 files changed, 30 insertions(+), 12 deletions(-)

diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_accel/ipsec.c b/drivers/net/ethernet/mellanox/mlx5/core/en_accel/ipsec.c
index 7bc5d07b4321..33a7434a7bb9 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/en_accel/ipsec.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/en_accel/ipsec.c
@@ -773,12 +773,20 @@ static int mlx5e_xfrm_add_state(struct net_device *dev,
 {
 	struct mlx5e_ipsec_sa_entry *sa_entry = NULL;
 	bool allow_tunnel_mode = false;
+	struct mlx5_core_dev *mdev;
 	struct mlx5e_ipsec *ipsec;
 	struct mlx5e_priv *priv;
 	gfp_t gfp;
 	int err;
 
 	priv = netdev_priv(dev);
+	/* Profile cleanup can memset an already NULL mdev. */
+	mdev = data_race(READ_ONCE(priv->mdev));
+	/* NULL only during a profile change, which is an eswitch operation. */
+	if (!mdev) {
+		NL_SET_ERR_MSG_MOD(extack, "Eswitch busy, can't add state");
+		return -EBUSY;
+	}
 	if (!priv->ipsec)
 		return -EOPNOTSUPP;
 
@@ -797,16 +805,16 @@ static int mlx5e_xfrm_add_state(struct net_device *dev,
 		return 0;
 	}
 
-	err = mlx5e_xfrm_validate_state(priv->mdev, x, extack);
+	err = mlx5e_xfrm_validate_state(mdev, x, extack);
 	if (err)
 		goto err_xfrm;
 
-	if (!mlx5_eswitch_block_ipsec(priv->mdev)) {
+	if (!mlx5_eswitch_block_ipsec(mdev)) {
 		err = -EBUSY;
 		goto err_xfrm;
 	}
 
-	err = mlx5_eswitch_block_mode(priv->mdev);
+	err = mlx5_eswitch_block_mode(mdev);
 	if (err)
 		goto unblock_ipsec;
 
@@ -877,9 +885,9 @@ static int mlx5e_xfrm_add_state(struct net_device *dev,
 
 	x->xso.offload_handle = (unsigned long)sa_entry;
 	if (allow_tunnel_mode)
-		mlx5_eswitch_unblock_encap(priv->mdev);
+		mlx5_eswitch_unblock_encap(mdev);
 
-	mlx5_eswitch_unblock_mode(priv->mdev);
+	mlx5_eswitch_unblock_mode(mdev);
 
 	return 0;
 
@@ -895,11 +903,11 @@ static int mlx5e_xfrm_add_state(struct net_device *dev,
 	kfree(sa_entry->work);
 unblock_encap:
 	if (allow_tunnel_mode)
-		mlx5_eswitch_unblock_encap(priv->mdev);
+		mlx5_eswitch_unblock_encap(mdev);
 unblock_mode:
-	mlx5_eswitch_unblock_mode(priv->mdev);
+	mlx5_eswitch_unblock_mode(mdev);
 unblock_ipsec:
-	mlx5_eswitch_unblock_ipsec(priv->mdev);
+	mlx5_eswitch_unblock_ipsec(mdev);
 err_xfrm:
 	kfree(sa_entry);
 	NL_SET_ERR_MSG_WEAK_MOD(extack, "Device failed to offload this state");
@@ -1268,16 +1276,24 @@ static int mlx5e_xfrm_add_policy(struct xfrm_policy *x,
 {
 	struct net_device *netdev = x->xdo.dev;
 	struct mlx5e_ipsec_pol_entry *pol_entry;
+	struct mlx5_core_dev *mdev;
 	struct mlx5e_priv *priv;
 	int err;
 
 	priv = netdev_priv(netdev);
+	/* Profile cleanup can memset an already NULL mdev. */
+	mdev = data_race(READ_ONCE(priv->mdev));
+	if (!mdev) {
+		NL_SET_ERR_MSG_MOD(extack, "Eswitch busy, can't add policy");
+		return -EBUSY;
+	}
+
 	if (!priv->ipsec) {
 		NL_SET_ERR_MSG_MOD(extack, "Device doesn't support IPsec packet offload");
 		return -EOPNOTSUPP;
 	}
 
-	err = mlx5e_xfrm_validate_policy(priv->mdev, x, extack);
+	err = mlx5e_xfrm_validate_policy(mdev, x, extack);
 	if (err)
 		return err;
 
@@ -1288,7 +1304,7 @@ static int mlx5e_xfrm_add_policy(struct xfrm_policy *x,
 	pol_entry->x = x;
 	pol_entry->ipsec = priv->ipsec;
 
-	if (!mlx5_eswitch_block_ipsec(priv->mdev)) {
+	if (!mlx5_eswitch_block_ipsec(mdev)) {
 		err = -EBUSY;
 		goto ipsec_busy;
 	}
@@ -1302,7 +1318,7 @@ static int mlx5e_xfrm_add_policy(struct xfrm_policy *x,
 	return 0;
 
 err_fs:
-	mlx5_eswitch_unblock_ipsec(priv->mdev);
+	mlx5_eswitch_unblock_ipsec(mdev);
 ipsec_busy:
 	kfree(pol_entry);
 	NL_SET_ERR_MSG_MOD(extack, "Device failed to offload this policy");
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_main.c b/drivers/net/ethernet/mellanox/mlx5/core/en_main.c
index c3c2c5720c6f..5cf3362259fc 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/en_main.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/en_main.c
@@ -6382,7 +6382,7 @@ int mlx5e_priv_init(struct mlx5e_priv *priv,
 	node = dev_to_node(mlx5_core_dma_dev(mdev));
 
 	/* priv init */
-	priv->mdev        = mdev;
+	WRITE_ONCE(priv->mdev, mdev);
 	priv->netdev      = netdev;
 	priv->max_nch     = nch;
 	priv->max_opened_tc = 1;
@@ -6467,6 +6467,8 @@ void mlx5e_priv_cleanup(struct mlx5e_priv *priv)
 		mlx5e_mqprio_rl_free(priv->mqprio_rl);
 	}
 
+	/* Clear atomically before memset() for lockless XFRM readers. */
+	WRITE_ONCE(priv->mdev, NULL);
 	memset(priv, 0, sizeof(*priv));
 	if (destroying) /* restore destroying bit, to allow unload */
 		set_bit(MLX5E_STATE_DESTROYING, &priv->state);
-- 
2.44.0

[PATCH net V3 6/8] net/mlx5e: Block E-Switch mode changes during IPsec policy creation

[Tariq Toukan]

From: Cosmin Ratiu <cratiu@nvidia.com>

Eswitch mode changes can tear down the IPsec context while policy
creation is accessing it. The mode-blocking reference acquired when
creating a flow table comes too late: the table lookup already accesses
the IPsec context before taking that reference.

Take a temporary mode-blocking reference before checking the IPsec
context and validating the policy. This gets dropped after successful
setup, when the flow table holds its own reference.

The temporary reservation must allow reuse of existing IPsec tables.
For example:
1. An offloaded inbound IPsec policy is added on the uplink.
2. A TC flower rule is added on a VF representor.
3. Another uplink policy is added using the same RX tables.

The third policy reuses the first policy's tables, so it must not be
rejected merely because esw->user_count is nonzero. Add a check_users
argument to mlx5_eswitch_block_mode(). Pass false for temporary policy
reservations and true when creating the first table (rx_get()/tx_get()).

Temporary reservations take mode_lock and check operation ownership
without requiring eswitch user_count to be zero.

mlx5e_xfrm_add_state() needs the same early mode reservation and is
handled in the next patch.

This was found by Sashiko, during review of the last patch in the
series. No reproduction attempted, as it seems quite a tricky race to
successfully trigger.

Fixes: a5b8ca9471d3 ("net/mlx5e: Add XFRM policy offload logic")
Signed-off-by: Cosmin Ratiu <cratiu@nvidia.com>
Reviewed-by: Jianbo Liu <jianbol@nvidia.com>
Reviewed-by: Dragos Tatulea <dtatulea@nvidia.com>
Signed-off-by: Tariq Toukan <tariqt@nvidia.com>
---
 .../mellanox/mlx5/core/en_accel/ipsec.c       | 23 +++++++++++++++----
 .../mellanox/mlx5/core/en_accel/ipsec_fs.c    |  4 ++--
 .../net/ethernet/mellanox/mlx5/core/eswitch.h |  8 +++++--
 .../mellanox/mlx5/core/eswitch_offloads.c     |  9 +++++---
 4 files changed, 32 insertions(+), 12 deletions(-)

diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_accel/ipsec.c b/drivers/net/ethernet/mellanox/mlx5/core/en_accel/ipsec.c
index 33a7434a7bb9..3cd0b36a02e7 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/en_accel/ipsec.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/en_accel/ipsec.c
@@ -814,7 +814,7 @@ static int mlx5e_xfrm_add_state(struct net_device *dev,
 		goto err_xfrm;
 	}
 
-	err = mlx5_eswitch_block_mode(mdev);
+	err = mlx5_eswitch_block_mode(mdev, true);
 	if (err)
 		goto unblock_ipsec;
 
@@ -1288,18 +1288,28 @@ static int mlx5e_xfrm_add_policy(struct xfrm_policy *x,
 		return -EBUSY;
 	}
 
+	/* Block esw mode changes until the policy holds its own block. */
+	err = mlx5_eswitch_block_mode(mdev, false);
+	if (err) {
+		NL_SET_ERR_MSG_MOD(extack, "Eswitch busy, can't add policy");
+		return err;
+	}
+
 	if (!priv->ipsec) {
 		NL_SET_ERR_MSG_MOD(extack, "Device doesn't support IPsec packet offload");
-		return -EOPNOTSUPP;
+		err = -EOPNOTSUPP;
+		goto unblock_mode;
 	}
 
 	err = mlx5e_xfrm_validate_policy(mdev, x, extack);
 	if (err)
-		return err;
+		goto unblock_mode;
 
 	pol_entry = kzalloc_obj(*pol_entry);
-	if (!pol_entry)
-		return -ENOMEM;
+	if (!pol_entry) {
+		err = -ENOMEM;
+		goto unblock_mode;
+	}
 
 	pol_entry->x = x;
 	pol_entry->ipsec = priv->ipsec;
@@ -1315,6 +1325,7 @@ static int mlx5e_xfrm_add_policy(struct xfrm_policy *x,
 		goto err_fs;
 
 	x->xdo.offload_handle = (unsigned long)pol_entry;
+	mlx5_eswitch_unblock_mode(mdev);
 	return 0;
 
 err_fs:
@@ -1322,6 +1333,8 @@ static int mlx5e_xfrm_add_policy(struct xfrm_policy *x,
 ipsec_busy:
 	kfree(pol_entry);
 	NL_SET_ERR_MSG_MOD(extack, "Device failed to offload this policy");
+unblock_mode:
+	mlx5_eswitch_unblock_mode(mdev);
 	return err;
 }
 
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_accel/ipsec_fs.c b/drivers/net/ethernet/mellanox/mlx5/core/en_accel/ipsec_fs.c
index 1529d4730323..dbe5547d7a49 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/en_accel/ipsec_fs.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/en_accel/ipsec_fs.c
@@ -1134,7 +1134,7 @@ static int rx_get(struct mlx5_core_dev *mdev, struct mlx5e_ipsec *ipsec,
 	if (rx->ft.refcnt)
 		goto skip;
 
-	err = mlx5_eswitch_block_mode(mdev);
+	err = mlx5_eswitch_block_mode(mdev, true);
 	if (err)
 		return err;
 
@@ -1427,7 +1427,7 @@ static int tx_get(struct mlx5_core_dev *mdev, struct mlx5e_ipsec *ipsec,
 	if (tx->ft.refcnt)
 		goto skip;
 
-	err = mlx5_eswitch_block_mode(mdev);
+	err = mlx5_eswitch_block_mode(mdev, true);
 	if (err)
 		return err;
 
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/eswitch.h b/drivers/net/ethernet/mellanox/mlx5/core/eswitch.h
index 1fb67fc6d49a..48eb6acc1e87 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/eswitch.h
+++ b/drivers/net/ethernet/mellanox/mlx5/core/eswitch.h
@@ -971,7 +971,7 @@ bool mlx5_eswitch_is_peer(struct mlx5_eswitch *esw,
 int mlx5_eswitch_block_encap(struct mlx5_core_dev *dev, bool from_fdb);
 void mlx5_eswitch_unblock_encap(struct mlx5_core_dev *dev);
 
-int mlx5_eswitch_block_mode(struct mlx5_core_dev *dev);
+int mlx5_eswitch_block_mode(struct mlx5_core_dev *dev, bool check_users);
 void mlx5_eswitch_unblock_mode(struct mlx5_core_dev *dev);
 
 static inline int mlx5_eswitch_num_vfs(struct mlx5_eswitch *esw)
@@ -1082,7 +1082,11 @@ static inline void mlx5_eswitch_unblock_encap(struct mlx5_core_dev *dev)
 {
 }
 
-static inline int mlx5_eswitch_block_mode(struct mlx5_core_dev *dev) { return 0; }
+static inline int mlx5_eswitch_block_mode(struct mlx5_core_dev *dev,
+					  bool check_users)
+{
+	return 0;
+}
 static inline void mlx5_eswitch_unblock_mode(struct mlx5_core_dev *dev) {}
 static inline bool mlx5_eswitch_block_ipsec(struct mlx5_core_dev *dev)
 {
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/eswitch_offloads.c b/drivers/net/ethernet/mellanox/mlx5/core/eswitch_offloads.c
index 4da89fcf215b..474784131091 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/eswitch_offloads.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/eswitch_offloads.c
@@ -4365,7 +4365,7 @@ static int esw_inline_mode_to_devlink(u8 mlx5_mode, u8 *mode)
 	return 0;
 }
 
-int mlx5_eswitch_block_mode(struct mlx5_core_dev *dev)
+int mlx5_eswitch_block_mode(struct mlx5_core_dev *dev, bool check_users)
 {
 	struct mlx5_eswitch *esw = dev->priv.eswitch;
 	int err;
@@ -4373,8 +4373,11 @@ int mlx5_eswitch_block_mode(struct mlx5_core_dev *dev)
 	if (!mlx5_esw_allowed(esw))
 		return 0;
 
-	/* Take TC into account */
-	err = mlx5_esw_try_lock(esw);
+	/* Temporary reservations may reuse tables while TC users exist. */
+	if (check_users)
+		err = mlx5_esw_try_lock(esw);
+	else
+		err = mlx5_esw_lock(esw);
 	if (err < 0)
 		return err;
 
-- 
2.44.0

[PATCH net V3 7/8] net/mlx5e: Block E-Switch mode changes during IPsec state creation

[Tariq Toukan]

From: Cosmin Ratiu <cratiu@nvidia.com>

mlx5e_xfrm_add_state() reads priv->ipsec before blocking eswitch mode
changes. A concurrent profile change can free the saved IPsec context
and cause use-after-free.

The temporary mode reservation was added in commit 22239eb258bc
("net/mlx5e: Prevent tunnel reformat when tunnel mode not allowed")
after saving priv->ipsec but the race was actually enabled by the
"Fixes" commit.
Extend the reservation to cover obtaining the context as well.

Use mlx5_eswitch_block_mode(mdev, false) for the temporary reservation,
as policy creation does, to check operation ownership under mode_lock
without rejecting existing eswitch users. This allows reusing existing
IPsec tables while TC rules exist on VF representors.

Check the context only after taking the mode reservation, so a
concurrent mode change consistently returns -EBUSY and a missing context
returns -EOPNOTSUPP, preserving software fallback for crypto offload.
Keep the atomic acquire-placeholder path exempt, since it creates no
hardware state and cannot take sleeping locks.

This was found by Sashiko, during review of the last patch in the
series. No reproduction attempted, as it seems quite a tricky race to
successfully trigger.

Fixes: 7a9fb35e8c3a ("net/mlx5e: Do not reload ethernet ports when changing eswitch mode")
Signed-off-by: Cosmin Ratiu <cratiu@nvidia.com>
Reviewed-by: Dragos Tatulea <dtatulea@nvidia.com>
Reviewed-by: Jianbo Liu <jianbol@nvidia.com>
Signed-off-by: Tariq Toukan <tariqt@nvidia.com>
---
 .../mellanox/mlx5/core/en_accel/ipsec.c       | 37 ++++++++++++-------
 1 file changed, 23 insertions(+), 14 deletions(-)

diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_accel/ipsec.c b/drivers/net/ethernet/mellanox/mlx5/core/en_accel/ipsec.c
index 3cd0b36a02e7..fa6cb51fdb93 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/en_accel/ipsec.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/en_accel/ipsec.c
@@ -771,6 +771,7 @@ static int mlx5e_xfrm_add_state(struct net_device *dev,
 				struct xfrm_state *x,
 				struct netlink_ext_ack *extack)
 {
+	bool is_acq = x->xso.flags & XFRM_DEV_OFFLOAD_FLAG_ACQ;
 	struct mlx5e_ipsec_sa_entry *sa_entry = NULL;
 	bool allow_tunnel_mode = false;
 	struct mlx5_core_dev *mdev;
@@ -787,20 +788,31 @@ static int mlx5e_xfrm_add_state(struct net_device *dev,
 		NL_SET_ERR_MSG_MOD(extack, "Eswitch busy, can't add state");
 		return -EBUSY;
 	}
-	if (!priv->ipsec)
-		return -EOPNOTSUPP;
+
+	if (!is_acq) {
+		err = mlx5_eswitch_block_mode(mdev, false);
+		if (err)
+			return err;
+	}
 
 	ipsec = priv->ipsec;
-	gfp = (x->xso.flags & XFRM_DEV_OFFLOAD_FLAG_ACQ) ? GFP_ATOMIC : GFP_KERNEL;
+	if (!ipsec) {
+		err = -EOPNOTSUPP;
+		goto unblock_mode;
+	}
+
+	gfp = is_acq ? GFP_ATOMIC : GFP_KERNEL;
 	sa_entry = kzalloc_obj(*sa_entry, gfp);
-	if (!sa_entry)
-		return -ENOMEM;
+	if (!sa_entry) {
+		err = -ENOMEM;
+		goto unblock_mode;
+	}
 
 	sa_entry->x = x;
 	sa_entry->dev = dev;
 	sa_entry->ipsec = ipsec;
 	/* Check if this SA is originated from acquire flow temporary SA */
-	if (x->xso.flags & XFRM_DEV_OFFLOAD_FLAG_ACQ) {
+	if (is_acq) {
 		x->xso.offload_handle = (unsigned long)sa_entry;
 		return 0;
 	}
@@ -814,21 +826,17 @@ static int mlx5e_xfrm_add_state(struct net_device *dev,
 		goto err_xfrm;
 	}
 
-	err = mlx5_eswitch_block_mode(mdev, true);
-	if (err)
-		goto unblock_ipsec;
-
 	if (x->props.mode == XFRM_MODE_TUNNEL &&
 	    x->xso.type == XFRM_DEV_OFFLOAD_PACKET) {
 		err = mlx5e_ipsec_fs_tunnel_allowed(sa_entry);
 		if (err < 0)
-			goto unblock_mode;
+			goto unblock_ipsec;
 		allow_tunnel_mode = err;
 		if (!allow_tunnel_mode) {
 			NL_SET_ERR_MSG_MOD(extack,
 					   "Packet offload tunnel mode is disabled due to encap settings");
 			err = -EINVAL;
-			goto unblock_mode;
+			goto unblock_ipsec;
 		}
 	}
 
@@ -904,13 +912,14 @@ static int mlx5e_xfrm_add_state(struct net_device *dev,
 unblock_encap:
 	if (allow_tunnel_mode)
 		mlx5_eswitch_unblock_encap(mdev);
-unblock_mode:
-	mlx5_eswitch_unblock_mode(mdev);
 unblock_ipsec:
 	mlx5_eswitch_unblock_ipsec(mdev);
 err_xfrm:
 	kfree(sa_entry);
 	NL_SET_ERR_MSG_WEAK_MOD(extack, "Device failed to offload this state");
+unblock_mode:
+	if (!is_acq)
+		mlx5_eswitch_unblock_mode(mdev);
 	return err;
 }
 
-- 
2.44.0

[PATCH net V3 8/8] net/mlx5e: Serialize TC and IPsec offload exclusion counters

[Tariq Toukan]

From: Cosmin Ratiu <cratiu@nvidia.com>

The counters enforcing TC and IPsec packet offload mutual exclusion are
not consistently serialized. IPsec admission conditionally takes
mode_lock for writing, while TC admission holds it only for read.
Neither release path takes the lock, and devices without an eswitch
cannot rely on it at all.

These are per-core-device counters, separate from the eswitch mode and
encapsulation reservations protected by mode_lock. Move them into
mdev->offload_block and protect their checks, increments and decrements
with a dedicated spinlock that is available even without an eswitch.
A spinlock is used because some of these functions are called from
atomic context and is a good fit because the critical sections are and
will stay really short.

IPsec admission then no longer needs mode_lock to update these counters.
Add warnings for unbalanced releases. Keep mlx5_esw_lock() for eswitch
operation admission and temporary mode/encapsulation reservations.

This was found by upstream Sashiko during review of [1], and spun-off
into a fix from the larger patch.

[1]
https://lore.kernel.org/netdev/178907726586.219967.4695695146458742597@kernel.org/

Fixes: c8e350e62fc5 ("net/mlx5e: Make TC and IPsec offloads mutually exclusive on a netdev")
Signed-off-by: Cosmin Ratiu <cratiu@nvidia.com>
Reviewed-by: Jianbo Liu <jianbol@nvidia.com>
Reviewed-by: Dragos Tatulea <dtatulea@nvidia.com>
Signed-off-by: Tariq Toukan <tariqt@nvidia.com>
---
 .../mellanox/mlx5/core/en_accel/ipsec_fs.c    | 43 ++++++-------------
 .../net/ethernet/mellanox/mlx5/core/en_tc.c   | 18 +++++---
 .../net/ethernet/mellanox/mlx5/core/main.c    |  1 +
 include/linux/mlx5/driver.h                   |  8 +++-
 4 files changed, 31 insertions(+), 39 deletions(-)

diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_accel/ipsec_fs.c b/drivers/net/ethernet/mellanox/mlx5/core/en_accel/ipsec_fs.c
index dbe5547d7a49..55de9abbd72b 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/en_accel/ipsec_fs.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/en_accel/ipsec_fs.c
@@ -2585,45 +2585,26 @@ void mlx5e_accel_ipsec_fs_read_stats(struct mlx5e_priv *priv, void *ipsec_stats)
 	}
 }
 
-#ifdef CONFIG_MLX5_ESWITCH
 static int mlx5e_ipsec_block_tc_offload(struct mlx5_core_dev *mdev)
 {
-	struct mlx5_eswitch *esw = mdev->priv.eswitch;
-	int err = 0;
-
-	if (esw) {
-		err = mlx5_esw_lock(esw);
-		if (err)
-			return err;
-	}
-
-	if (mdev->num_block_ipsec) {
-		err = -EBUSY;
-		goto unlock;
-	}
+	int ret = 0;
 
-	mdev->num_block_tc++;
-
-unlock:
-	if (esw)
-		mlx5_esw_unlock(esw);
-
-	return err;
-}
-#else
-static int mlx5e_ipsec_block_tc_offload(struct mlx5_core_dev *mdev)
-{
-	if (mdev->num_block_ipsec)
-		return -EBUSY;
+	spin_lock_bh(&mdev->offload_block.lock);
+	if (mdev->offload_block.num_block_ipsec)
+		ret = -EBUSY;
+	else
+		mdev->offload_block.num_block_tc++;
+	spin_unlock_bh(&mdev->offload_block.lock);
 
-	mdev->num_block_tc++;
-	return 0;
+	return ret;
 }
-#endif
 
 static void mlx5e_ipsec_unblock_tc_offload(struct mlx5_core_dev *mdev)
 {
-	mdev->num_block_tc--;
+	spin_lock_bh(&mdev->offload_block.lock);
+	if (!WARN_ON_ONCE(!mdev->offload_block.num_block_tc))
+		mdev->offload_block.num_block_tc--;
+	spin_unlock_bh(&mdev->offload_block.lock);
 }
 
 int mlx5e_accel_ipsec_fs_add_rule(struct mlx5e_ipsec_sa_entry *sa_entry)
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_tc.c b/drivers/net/ethernet/mellanox/mlx5/core/en_tc.c
index 708b7987ea4c..9f97c9d680bb 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/en_tc.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/en_tc.c
@@ -4836,16 +4836,19 @@ static bool is_tc_ipsec_order_check_needed(struct net_device *filter, struct mlx
 static int mlx5e_tc_block_ipsec_offload(struct net_device *filter, struct mlx5e_priv *priv)
 {
 	struct mlx5_core_dev *mdev = priv->mdev;
+	int ret = 0;
 
 	if (!is_tc_ipsec_order_check_needed(filter, priv))
 		return 0;
 
-	if (mdev->num_block_tc)
-		return -EBUSY;
-
-	mdev->num_block_ipsec++;
+	spin_lock_bh(&mdev->offload_block.lock);
+	if (mdev->offload_block.num_block_tc)
+		ret = -EBUSY;
+	else
+		mdev->offload_block.num_block_ipsec++;
+	spin_unlock_bh(&mdev->offload_block.lock);
 
-	return 0;
+	return ret;
 }
 
 static void mlx5e_tc_unblock_ipsec_offload(struct net_device *filter, struct mlx5e_priv *priv)
@@ -4853,7 +4856,10 @@ static void mlx5e_tc_unblock_ipsec_offload(struct net_device *filter, struct mlx
 	if (!is_tc_ipsec_order_check_needed(filter, priv))
 		return;
 
-	priv->mdev->num_block_ipsec--;
+	spin_lock_bh(&priv->mdev->offload_block.lock);
+	if (!WARN_ON_ONCE(!priv->mdev->offload_block.num_block_ipsec))
+		priv->mdev->offload_block.num_block_ipsec--;
+	spin_unlock_bh(&priv->mdev->offload_block.lock);
 }
 
 int mlx5e_configure_flower(struct net_device *dev, struct mlx5e_priv *priv,
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/main.c b/drivers/net/ethernet/mellanox/mlx5/core/main.c
index 1a45474ce03d..0439fcfb9ce5 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/main.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/main.c
@@ -1814,6 +1814,7 @@ int mlx5_mdev_init(struct mlx5_core_dev *dev, int profile_idx)
 	lockdep_register_key(&dev->lock_key);
 	mutex_init(&dev->intf_state_mutex);
 	lockdep_set_class(&dev->intf_state_mutex, &dev->lock_key);
+	spin_lock_init(&dev->offload_block.lock);
 	mutex_init(&dev->mlx5e_res.uplink_netdev_lock);
 	mutex_init(&dev->wc_state_lock);
 
diff --git a/include/linux/mlx5/driver.h b/include/linux/mlx5/driver.h
index abb99371ff13..84e9f6bdb7c1 100644
--- a/include/linux/mlx5/driver.h
+++ b/include/linux/mlx5/driver.h
@@ -789,8 +789,12 @@ struct mlx5_core_dev {
 	u32                      vsc_addr;
 	struct mlx5_hv_vhca	*hv_vhca;
 	struct mlx5_hwmon	*hwmon;
-	u64			num_block_tc;
-	u64			num_block_ipsec;
+	struct {
+		/* Protects the fields below. */
+		spinlock_t lock;
+		u64 num_block_tc;
+		u64 num_block_ipsec;
+	} offload_block;
 #ifdef CONFIG_MLX5_MACSEC
 	struct mlx5_macsec_fs *macsec_fs;
 	/* MACsec notifier chain to sync MACsec core and IB database */
-- 
2.44.0