[PATCH net 0/3] net/mlx5: LAG bug fixes

[PATCH net 0/3] net/mlx5: LAG bug fixes

[Tariq Toukan]

Hi,

Three bug fixes by Shay in the mlx5 LAG subsystem.

Patch 1 fixes an off-by-one in the error rollback path of
mlx5_lag_create_single_fdb(): the loop started from the failed index i,
potentially operating on uninitialized state or double-tearing-down an
entry that had already self-rolled-back. The rollback should start from
i - 1.

Patch 2 fixes a hang in mlx5_mpesw_work(): when
mlx5_lag_get_devcom_comp() returns NULL the function returned early
without calling complete(), blocking any caller waiting on mpesww->comp
indefinitely.

Patch 3 fixes a kernel crash during teardown when mlx5_lag_get_dev_seq()
returns an error because no device is marked as master or the peer is no
longer in the LAG. The peer flow cleanup is now skipped instead of
proceeding with a bad pointer.

Regards,
Tariq

Shay Drory (3):
  net/mlx5: LAG, Fix off-by-one in single-FDB error rollback
  net/mlx5: LAG, MPESW, Fix missing complete() on devcom error
  net/mlx5e: TC, skip peer flow cleanup when LAG seq is unavailable

 drivers/net/ethernet/mellanox/mlx5/core/en_tc.c     | 6 ++++++
 drivers/net/ethernet/mellanox/mlx5/core/lag/lag.c   | 2 +-
 drivers/net/ethernet/mellanox/mlx5/core/lag/mpesw.c | 7 +++++--
 3 files changed, 12 insertions(+), 3 deletions(-)


base-commit: 0068940907d33217ae01217f84910a5cde606c17
-- 
2.44.0

[PATCH net 1/3] net/mlx5: LAG, Fix off-by-one in single-FDB error rollback

[Tariq Toukan]

From: Shay Drory <shayd@nvidia.com>

On failure at index i, the reverse cleanup loop in
mlx5_lag_create_single_fdb() starts from i, so the failed index
itself is rolled back. That can operate on uninitialized state or
double-tear-down a rule the add_one path already self-rolled-back.

Start the rollback from i - 1 so only successfully-installed entries
are undone.

Fixes: ddbb5ddc43ad ("net/mlx5: LAG, Refactor lag logic")
Signed-off-by: Shay Drory <shayd@nvidia.com>
Reviewed-by: Mark Bloch <mbloch@nvidia.com>
Signed-off-by: Tariq Toukan <tariqt@nvidia.com>
---
 drivers/net/ethernet/mellanox/mlx5/core/lag/lag.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/mellanox/mlx5/core/lag/lag.c b/drivers/net/ethernet/mellanox/mlx5/core/lag/lag.c
index f8e70ac5a85b..6ae1a7781c8a 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/lag/lag.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/lag/lag.c
@@ -845,7 +845,7 @@ static int mlx5_lag_create_single_fdb(struct mlx5_lag *ldev)
 	}
 	return 0;
 err:
-	mlx5_ldev_for_each_reverse(j, i, 0, ldev) {
+	mlx5_ldev_for_each_reverse(j, i - 1, 0, ldev) {
 		if (j == master_idx)
 			continue;
 		mlx5_eswitch_offloads_single_fdb_del_one(master_esw,
-- 
2.44.0

[PATCH net 2/3] net/mlx5: LAG, MPESW, Fix missing complete() on devcom error

[Tariq Toukan]

From: Shay Drory <shayd@nvidia.com>

mlx5_mpesw_work() returned without calling complete() when
mlx5_lag_get_devcom_comp() returned NULL. A caller that queued the
work and waited on mpesww->comp would block indefinitely.

Funnel the early-return path through a new "complete" label so the
waiter is always woken.

Fixes: b430c1b4f63b ("net/mlx5: Replace global mlx5_intf_lock with HCA devcom component lock")
Signed-off-by: Shay Drory <shayd@nvidia.com>
Reviewed-by: Mark Bloch <mbloch@nvidia.com>
Signed-off-by: Tariq Toukan <tariqt@nvidia.com>
---
 drivers/net/ethernet/mellanox/mlx5/core/lag/mpesw.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/mellanox/mlx5/core/lag/mpesw.c b/drivers/net/ethernet/mellanox/mlx5/core/lag/mpesw.c
index 5eea12a6887a..db506ab4fa96 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/lag/mpesw.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/lag/mpesw.c
@@ -140,8 +140,10 @@ static void mlx5_mpesw_work(struct work_struct *work)
 	struct mlx5_lag *ldev = mpesww->lag;
 
 	devcom = mlx5_lag_get_devcom_comp(ldev);
-	if (!devcom)
-		return;
+	if (!devcom) {
+		mpesww->result = -ENODEV;
+		goto complete;
+	}
 
 	mlx5_devcom_comp_lock(devcom);
 	mutex_lock(&ldev->lock);
@@ -157,6 +159,7 @@ static void mlx5_mpesw_work(struct work_struct *work)
 unlock:
 	mutex_unlock(&ldev->lock);
 	mlx5_devcom_comp_unlock(devcom);
+complete:
 	complete(&mpesww->comp);
 }
 
-- 
2.44.0

[PATCH net 3/3] net/mlx5e: TC, skip peer flow cleanup when LAG seq is unavailable

[Tariq Toukan]

From: Shay Drory <shayd@nvidia.com>

mlx5_lag_get_dev_seq() will return error when the peer isn't in the LAG
or when no device is marked as master. Result bad memory access and kernel
crash[1].

Hence, skip the peer when lookup fails.

Note: In case there are peer flows, they are cleaned before LAG cleared
the master mark.

[1]
RIP: 0010:mlx5e_tc_del_fdb_peers_flow+0x3d/0x350 [mlx5_core]
Call Trace:
 <TASK>
 mlx5e_tc_clean_fdb_peer_flows+0xc1/0x130 [mlx5_core]
 mlx5_esw_offloads_unpair+0x3a/0x400 [mlx5_core]
 mlx5_esw_offloads_devcom_event+0xee/0x360 [mlx5_core]
 mlx5_devcom_send_event+0x7a/0x140 [mlx5_core]
 mlx5_esw_offloads_devcom_cleanup+0x2f/0x90 [mlx5_core]
 mlx5e_tc_esw_cleanup+0x28/0xf0 [mlx5_core]
 mlx5e_rep_tc_cleanup+0x19/0x30 [mlx5_core]
 mlx5e_cleanup_uplink_rep_tx+0x36/0x40 [mlx5_core]
 mlx5e_cleanup_rep_tx+0x55/0x60 [mlx5_core]
 mlx5e_detach_netdev+0x96/0xf0 [mlx5_core]
 mlx5e_netdev_change_profile+0x5b/0x120 [mlx5_core]
 mlx5e_netdev_attach_nic_profile+0x1b/0x30 [mlx5_core]
 mlx5e_vport_rep_unload+0xdd/0x110 [mlx5_core]
 __esw_offloads_unload_rep+0x81/0xb0 [mlx5_core]
 mlx5_eswitch_unregister_vport_reps+0x1d7/0x220 [mlx5_core]
 mlx5e_rep_remove+0x22/0x30 [mlx5_core]
 device_release_driver_internal+0x194/0x1f0
 bus_remove_device+0xe8/0x1b0
 device_del+0x159/0x3c0
 mlx5_rescan_drivers_locked+0xbc/0x2d0 [mlx5_core]
 mlx5_unregister_device+0x54/0x80 [mlx5_core]
 mlx5_uninit_one+0x73/0x130 [mlx5_core]
 remove_one+0x78/0xe0 [mlx5_core]
 pci_device_remove+0x39/0xa0

Fixes: 971b28accc09 ("net/mlx5: LAG, replace mlx5_get_dev_index with LAG sequence number")
Signed-off-by: Shay Drory <shayd@nvidia.com>
Reviewed-by: Mark Bloch <mbloch@nvidia.com>
Signed-off-by: Tariq Toukan <tariqt@nvidia.com>
---
 drivers/net/ethernet/mellanox/mlx5/core/en_tc.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_tc.c b/drivers/net/ethernet/mellanox/mlx5/core/en_tc.c
index a9001d1c902f..c6e6534a5e23 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/en_tc.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/en_tc.c
@@ -2155,6 +2155,9 @@ static void mlx5e_tc_del_fdb_peers_flow(struct mlx5e_tc_flow *flow)
 	devcom = flow->priv->mdev->priv.eswitch->devcom;
 	mlx5_devcom_for_each_peer_entry(devcom, peer_esw, pos) {
 		i = mlx5_lag_get_dev_seq(peer_esw->dev);
+		if (i < 0)
+			continue;
+
 		mlx5e_tc_del_fdb_peer_flow(flow, i);
 	}
 }
@@ -5526,6 +5529,9 @@ void mlx5e_tc_clean_fdb_peer_flows(struct mlx5_eswitch *esw)
 
 	mlx5_devcom_for_each_peer_entry(devcom, peer_esw, pos) {
 		i = mlx5_lag_get_dev_seq(peer_esw->dev);
+		if (i < 0)
+			continue;
+
 		list_for_each_entry_safe(flow, tmp, &esw->offloads.peer_flows[i], peer[i])
 			mlx5e_tc_del_fdb_peers_flow(flow);
 	}
-- 
2.44.0