Skip to contentSkip to conversation

net/mlx5: Steering misc enhancements

PATCH Applied Tariq Toukan 4 participants 9 messages May 7 – May 11

Subsystems (4)
Networking Network drivers InfiniBand Mellanox mlx5 core
lore
Share & tools
lore

Tariq Toukan

author maintainer
#0 lore raw

cover net/mlx5: Steering misc enhancements

Hi,

This small series by Yevgeny contains a few steering enhancements / cleanups.

Regards,
Tariq

Yevgeny Kliteynik (3):
net/mlx5: HWS, Check if device is down while polling for completion net/mlx5: HWS, Handle destroying table that has a miss table net/mlx5: DR, Remove unused field of struct mlx5dr_matcher_rx_tx

 .../ethernet/mellanox/mlx5/core/steering/hws/bwc.c   | 12 ++++++++++++
 .../ethernet/mellanox/mlx5/core/steering/hws/table.c |  3 +++
 .../mellanox/mlx5/core/steering/sws/dr_types.h       |  1 -
 3 files changed, 15 insertions(+), 1 deletion(-)

base-commit: dacf281771a9aed1a723b196120a0de8637910b9

Signature · 1 line
2.44.0
author maintainer
#1 lore raw

PATCH 1/3 net/mlx5: HWS, Check if device is down while polling for completion

From: Yevgeny Kliteynik <[email protected]>

In case the device is down for any reason (e.g. FLR), the HW will no longer generate completions - no point polling and waiting for timeout.

Signed-off-by Yevgeny Kliteynik <[email protected]>
Reviewed-by Erez Shitrit <[email protected]>
Reviewed-by Shay Drori <[email protected]>
Signed-off-by Tariq Toukan <[email protected]>
---
 .../ethernet/mellanox/mlx5/core/steering/hws/bwc.c   | 12 ++++++++++++
 1 file changed, 12 insertions(+)
drivers/net/ethernet/mellanox/mlx5/core/steering/hws/bwc.c +12 -0
index 6dcd9c2a78aa..eae02bc74221 100644
@@ -422,6 +422,18 @@ int mlx5hws_bwc_queue_poll(struct mlx5hws_context *ctx,
 	if (!got_comp && !drain)
 		return 0;
 
+	if (unlikely(ctx->mdev->state == MLX5_DEVICE_STATE_INTERNAL_ERROR)) {
+		/* If the device is down for any reason (e.g. FLR), the HW will
+		 * no longer generate completions.
+		 * Note that ETIMEDOUT is returned here because the BWC layer
+		 * already has a special handling for timeouts - it breaks the
+		 * rehash / resize / shrink loops to avoid chain of timeouts.
+		 */
+		mlx5_core_warn_once(ctx->mdev,
+				    "BWC poll: device is down, polling for completion aborted\n");
+		return -ETIMEDOUT;
+	}
+
 	queue_full = mlx5hws_send_engine_full(&ctx->send_queue[queue_id]);
 	while (queue_full || ((got_comp || drain) && *pending_rules)) {
 		ret = mlx5hws_send_queue_poll(ctx, queue_id, comp, burst_th);
Signature · 1 line
2.44.0
reviewer
#2 lore raw

Re: [PATCH net-next 1/3] net/mlx5: HWS, Check if device is down while polling for completion

From: 'Simon Horman' <[email protected]>

This is an AI-generated review of your patch. The human sending this email has considered the AI review valid, or at least plausible. Full review at: https://sashiko.dev
---
net/mlx5: HWS, Check if device is down while polling for completion

This commit adds a check to abort polling for hardware completions if the device state indicates an internal error. This returns -ETIMEDOUT early to avoid long timeouts when the hardware stops generating completions.

Quoted 12 lines
drivers/net/ethernet/mellanox/mlx5/core/steering/hws/bwc.c +4 -0
index 6dcd9c2a78aa8..eae02bc742218 100644
@@ -422,6 +422,18 @@ int mlx5hws_bwc_queue_poll(struct mlx5hws_context *ctx,
 	if (!got_comp && !drain)
 		return 0;
 
+	if (unlikely(ctx->mdev->state == MLX5_DEVICE_STATE_INTERNAL_ERROR)) {
+		/* If the device is down for any reason (e.g. FLR), the HW will
+		 * no longer generate completions.
+		 * Note that ETIMEDOUT is returned here because the BWC layer

drivers/net/ethernet/mellanox/mlx5/core/steering/hws/bwc.c int mlx5hws_bwc_queue_poll(struct mlx5hws_context *ctx, · line 438

Quoted 10 lines
drivers/net/ethernet/mellanox/mlx5/core/steering/hws/bwc.c +8 -0
+		 * already has a special handling for timeouts - it breaks the
+		 * rehash / resize / shrink loops to avoid chain of timeouts.
+		 */
+		mlx5_core_warn_once(ctx->mdev,
+				    "BWC poll: device is down, polling for completion aborted\n");
+		return -ETIMEDOUT;
+	}
+
 	queue_full = mlx5hws_send_engine_full(&ctx->send_queue[queue_id]);
 	while (queue_full || ((got_comp || drain) && *pending_rules)) {

Could this lead to a CPU stall if the device goes down during the polling loop?

The hardware state check is placed outside the while loop. The loop has a 60-second timeout (MLX5HWS_BWC_POLLING_TIMEOUT) and lacks cond_resched(), meaning it can peg the CPU while spinning.

If the hardware fails and stops generating completions while we are already inside the loop, the driver's health thread on another CPU might detect the failure and set ctx->mdev->state to MLX5_DEVICE_STATE_INTERNAL_ERROR.

Since the check is outside the loop, we would fail to detect the state change and continue to spin uselessly for the remainder of the 60 seconds, which could trigger a soft lockup panic.

Should the state check be moved inside the polling loop, perhaps using READ_ONCE() on ctx->mdev->state to ensure the compiler does not hoist the lockless read?

Quoted 1 line
drivers/net/ethernet/mellanox/mlx5/core/steering/hws/bwc.c +0 -0
 		ret = mlx5hws_send_queue_poll(ctx, queue_id, comp, burst_th);
#3 lore raw

Re: [PATCH net-next 1/3] net/mlx5: HWS, Check if device is down while polling for completion

Quoted 20 lines · Simon Horman

From: 'Simon Horman' <[email protected]>

This is an AI-generated review of your patch. The human sending this email has considered the AI review valid, or at least plausible. Full review at: https://sashiko.dev
---
net/mlx5: HWS, Check if device is down while polling for completion

This commit adds a check to abort polling for hardware completions if the device state indicates an internal error. This returns -ETIMEDOUT early to avoid long timeouts when the hardware stops generating completions.

Quoted 8 lines
drivers/net/ethernet/mellanox/mlx5/core/steering/hws/bwc.c +0 -0
index 6dcd9c2a78aa8..eae02bc742218 100644
@@ -422,6 +422,18 @@ int mlx5hws_bwc_queue_poll(struct mlx5hws_context *ctx,
  	if (!got_comp && !drain)
  		return 0;
Quoted 16 lines
Quoted 4 lines
drivers/net/ethernet/mellanox/mlx5/core/steering/hws/bwc.c +4 -0
+	if (unlikely(ctx->mdev->state == MLX5_DEVICE_STATE_INTERNAL_ERROR)) {
+		/* If the device is down for any reason (e.g. FLR), the HW will
+		 * no longer generate completions.
+		 * Note that ETIMEDOUT is returned here because the BWC layer

drivers/net/ethernet/mellanox/mlx5/core/steering/hws/bwc.c int mlx5hws_bwc_queue_poll(struct mlx5hws_context *ctx, · line 438

Quoted 10 lines
drivers/net/ethernet/mellanox/mlx5/core/steering/hws/bwc.c +8 -0
+		 * already has a special handling for timeouts - it breaks the
+		 * rehash / resize / shrink loops to avoid chain of timeouts.
+		 */
+		mlx5_core_warn_once(ctx->mdev,
+				    "BWC poll: device is down, polling for completion aborted\n");
+		return -ETIMEDOUT;
+	}
+
  	queue_full = mlx5hws_send_engine_full(&ctx->send_queue[queue_id]);
  	while (queue_full || ((got_comp || drain) && *pending_rules)) {

Could this lead to a CPU stall if the device goes down during the polling loop?

That is correct.
There is the possibility that the device goes down while we're already inside the polling loop, in which case the looping will continue till it reaches the timeout.
But what we're trying to optimize is the "normal" flow. We prefer not to add additional 'if' into the polling loop, which should be as efficient as possible - it affects rules insertion/deletion rate.
So this patch aims to prevent chain of timeouts when device is in error state, which is happening during deletion of all the flows, and during shrinking of the matchers, while it still leaves some probability to see a single initial timeout.

Quoted 17 lines

The hardware state check is placed outside the while loop. The loop has a 60-second timeout (MLX5HWS_BWC_POLLING_TIMEOUT) and lacks cond_resched(), meaning it can peg the CPU while spinning.

If the hardware fails and stops generating completions while we are already inside the loop, the driver's health thread on another CPU might detect the failure and set ctx->mdev->state to MLX5_DEVICE_STATE_INTERNAL_ERROR.

Since the check is outside the loop, we would fail to detect the state change and continue to spin uselessly for the remainder of the 60 seconds, which could trigger a soft lockup panic.

Should the state check be moved inside the polling loop, perhaps using READ_ONCE() on ctx->mdev->state to ensure the compiler does not hoist the lockless read?

Quoted 1 line
drivers/net/ethernet/mellanox/mlx5/core/steering/hws/bwc.c +0 -0
  		ret = mlx5hws_send_queue_poll(ctx, queue_id, comp, burst_th);
author maintainer
#4 lore raw

PATCH 2/3 net/mlx5: HWS, Handle destroying table that has a miss table

In reply to Tariq Toukan

From: Yevgeny Kliteynik <[email protected]>

If a table has a miss table that was created by 'mlx5hws_table_set_default_miss' API function, its miss_tbl keeps the table that points to it in a list.
If such table is deleted, we need to also remove it from the miss_tbl list, otherwise the node in miss_tbl list will contain garbage.

Signed-off-by Erez Shitrit <[email protected]>
Signed-off-by Yevgeny Kliteynik <[email protected]>
Reviewed-by Moshe Shemesh <[email protected]>
Signed-off-by Tariq Toukan <[email protected]>
---
 drivers/net/ethernet/mellanox/mlx5/core/steering/hws/table.c | 3 +++
 1 file changed, 3 insertions(+)
drivers/net/ethernet/mellanox/mlx5/core/steering/hws/table.c +3 -0
index bd292485a25b..dd7927983ab2 100644
@@ -282,6 +282,9 @@ int mlx5hws_table_destroy(struct mlx5hws_table *tbl)
 		goto unlock_err;
 	}
 
+	if (tbl->default_miss.miss_tbl)
+		list_del_init(&tbl->default_miss.next);
+
 	list_del_init(&tbl->tbl_list_node);
 	mutex_unlock(&ctx->ctrl_lock);
Signature · 1 line
2.44.0
reviewer Reviewed-by on v1 2/3
#5 lore raw

Re: [PATCH net-next 2/3] net/mlx5: HWS, Handle destroying table that has a miss table

Quoted 9 lines · Tariq Toukan

From: Yevgeny Kliteynik <[email protected]>

If a table has a miss table that was created by 'mlx5hws_table_set_default_miss' API function, its miss_tbl keeps the table that points to it in a list.
If such table is deleted, we need to also remove it from the miss_tbl list, otherwise the node in miss_tbl list will contain garbage.

Quoted 4 lines
Signed-off-by Erez Shitrit <[email protected]>
Signed-off-by Yevgeny Kliteynik <[email protected]>
Reviewed-by Moshe Shemesh <[email protected]>
Signed-off-by Tariq Toukan <[email protected]>
Reviewed-by Simon Horman <[email protected]>
author maintainer
#6 lore raw

PATCH 3/3 net/mlx5: DR, Remove unused field of struct mlx5dr_matcher_rx_tx

In reply to Tariq Toukan

From: Yevgeny Kliteynik <[email protected]>

Remove a field that was never used.

Signed-off-by Yevgeny Kliteynik <[email protected]>
Reviewed-by Erez Shitrit <[email protected]>
Signed-off-by Tariq Toukan <[email protected]>
---
 drivers/net/ethernet/mellanox/mlx5/core/steering/sws/dr_types.h | 1 -
 1 file changed, 1 deletion(-)
drivers/net/ethernet/mellanox/mlx5/core/steering/sws/dr_types.h +0 -1
index cc328292bf84..e0344707f522 100644
@@ -986,7 +986,6 @@ struct mlx5dr_matcher_rx_tx {
 					       [DR_RULE_MAX_STES];
 	u8 num_of_builders;
 	u8 num_of_builders_arr[DR_RULE_IPV_MAX][DR_RULE_IPV_MAX];
-	u64 default_icm_addr;
 	struct mlx5dr_table_rx_tx *nic_tbl;
 	u32 prio;
 	struct list_head list_node;
Signature · 1 line
2.44.0
CI Applied
#8 lore raw
In reply to Tariq Toukan
This series was applied to netdev/net-next.git (main) by Jakub Kicinski <[email protected]>:

Hello:

This series was applied to netdev/net-next.git (main) by Jakub Kicinski <[email protected]>:

Quoted 9 lines · On Thu, 7 May 2026 20:34:40 +0300 you wrote:

Hi,

This small series by Yevgeny contains a few steering enhancements / cleanups.

Regards,
Tariq

[...]

Here is the summary with links:
- [net-next,1/3] net/mlx5: HWS, Check if device is down while polling for completion

    https://git.kernel.org/netdev/net-next/c/e3ec1570895b
  - [net-next,2/3] net/mlx5: HWS, Handle destroying table that has a miss table
    https://git.kernel.org/netdev/net-next/c/60e9e82f162a
  - [net-next,3/3] net/mlx5: DR, Remove unused field of struct mlx5dr_matcher_rx_tx
    https://git.kernel.org/netdev/net-next/c/6316d40b8509

You are awesome, thank you!

Signature · 2 lines
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html