patch 'net/mlx5: fix flow counter race on query completion' has been queued to stable release 24.11.7
luca.boccassi at gmail.com
luca.boccassi at gmail.com
Mon Jul 6 12:20:09 CEST 2026
Hi,
FYI, your patch has been queued to stable release 24.11.7
Note it hasn't been pushed to http://dpdk.org/browse/dpdk-stable yet.
It will be pushed if I get no objections before 07/05/26. So please
shout if anyone has objections.
Also note that after the patch there's a diff of the upstream commit vs the
patch applied to the branch. This will indicate if there was any rebasing
needed to apply to the stable branch. If there were code changes for rebasing
(ie: not only metadata diffs), please double check that the rebase was
correctly done.
Queued patches are on a temporary branch at:
https://github.com/bluca/dpdk-stable
This queued commit can be viewed at:
https://github.com/bluca/dpdk-stable/commit/4b8e73813ccf7034dca2843919eb683b0b195ecc
Thanks.
Luca Boccassi
---
>From 4b8e73813ccf7034dca2843919eb683b0b195ecc Mon Sep 17 00:00:00 2001
From: Linhu Li <lilinhu618 at gmail.com>
Date: Thu, 18 Jun 2026 17:14:50 +0800
Subject: [PATCH] net/mlx5: fix flow counter race on query completion
[ upstream commit 60af6e80b99b07aa992f7738ed920a7b99cbdbdd ]
flow_dv_counter_free() inserts counters into
pool->counters[pool->query_gen] under pool->csl. Meanwhile,
mlx5_flow_async_pool_query_handle() moves counters from
pool->counters[query_gen ^ 1] to the global free list via
TAILQ_CONCAT while holding only cmng->csl, not pool->csl.
The comment in flow_dv_counter_free() claims the lock is not needed
because the query callback and the release function operate on
different lists. That holds only if the free path always observes
the up-to-date query_gen. It can be violated:
1. A counter free thread (non-PMD, e.g. OVS offload thread) reads
pool->query_gen == 0 and is about to insert into counters[0].
2. The free thread is preempted by the OS scheduler; it is a regular
pthread, not pinned to a core.
3. The eal-intr-thread alarm fires: query_gen++ (now 1) and the async
query is sent.
4. Hardware completes the query and the callback runs TAILQ_CONCAT on
counters[0] (= query_gen ^ 1).
5. The free thread resumes and runs TAILQ_INSERT_TAIL on counters[0]
concurrently with step 4 on another core.
Because the two paths take different locks, TAILQ_INSERT_TAIL and
TAILQ_CONCAT run concurrently on the same list with no synchronization
and corrupt it: the pool-local list ends up with a NULL head but a
dangling tqh_last, and the global free list tail no longer points to
the real tail. The just-freed counter and every counter inserted
afterwards become unreachable and are leaked.
Non-PMD threads can be preempted for hundreds of microseconds under
CPU pressure, which is well within the async query round-trip time,
so the window is reachable in practice.
Fix it by taking pool->csl in the query completion callback before
operating on pool->counters[query_gen], serializing the CONCAT with
any concurrent INSERT. The lock is taken once per pool per query
completion in the eal-intr-thread context, not on the datapath, so
the cost is negligible. Lock order is pool->csl then cmng->csl,
matching all other sites.
Also handle the error path: previously the counters accumulated in
pool->counters[query_gen] were abandoned when a query failed. Move
them back to the global free list to avoid a leak on persistent
query failures.
Additionally, fix a second independent race in flow_dv_counter_free().
TAILQ_INSERT_TAIL is passed &pool->counters[pool->query_gen] directly,
but the macro evaluates its head argument multiple times. Since
pool->query_gen is a volatile bit-field, if mlx5_flow_query_alarm()
increments query_gen between two evaluations of the macro, the same
insertion can operate on two different lists: the earlier steps update
counters[0] while the later steps update counters[1], leaving both
lists with inconsistent metadata and leaking the counter. Fix by
caching pool->query_gen into a local variable before calling the macro.
Fixes: ac79183dc6f7 ("net/mlx5: optimize free counter lookup")
Signed-off-by: Linhu Li <lilinhu618 at gmail.com>
Acked-by: Dariusz Sosnowski <dsosnowski at nvidia.com>
---
.mailmap | 1 +
drivers/net/mlx5/mlx5_flow.c | 31 +++++++++++++++++++++++++++++++
drivers/net/mlx5/mlx5_flow_dv.c | 12 +++++++-----
3 files changed, 39 insertions(+), 5 deletions(-)
diff --git a/.mailmap b/.mailmap
index 83165bd717..bf1fdf1941 100644
--- a/.mailmap
+++ b/.mailmap
@@ -894,6 +894,7 @@ Linfan Hu <zhongdahulinfan at 163.com>
Lingli Chen <linglix.chen at intel.com>
Lingyu Liu <lingyu.liu at intel.com>
Lin Li <lilintjpu at bytedance.com> <lilin24 at baidu.com>
+Linhu Li <lilinhu618 at gmail.com>
Linsi Yuan <yuanlinsi01 at baidu.com>
Lior Margalit <lmargalit at nvidia.com>
Li Qiang <liq3ea at 163.com>
diff --git a/drivers/net/mlx5/mlx5_flow.c b/drivers/net/mlx5/mlx5_flow.c
index e296fab6c1..9735c664ad 100644
--- a/drivers/net/mlx5/mlx5_flow.c
+++ b/drivers/net/mlx5/mlx5_flow.c
@@ -9890,6 +9890,13 @@ void
mlx5_flow_async_pool_query_handle(struct mlx5_dev_ctx_shared *sh,
uint64_t async_id, int status)
{
+ /*
+ * Handle async counter pool query completion.
+ * query_gen is flipped each round: freed counters go into [query_gen],
+ * while this callback moves [query_gen ^ 1] to the global free list.
+ * pool->csl must be held when operating on pool->counters[] to serialize
+ * with concurrent free-path insertions.
+ */
struct mlx5_flow_counter_pool *pool =
(struct mlx5_flow_counter_pool *)(uintptr_t)async_id;
struct mlx5_counter_stats_raw *raw_to_free;
@@ -9901,6 +9908,21 @@ mlx5_flow_async_pool_query_handle(struct mlx5_dev_ctx_shared *sh,
if (unlikely(status)) {
raw_to_free = pool->raw_hw;
+ /*
+ * The query failed, so the freed counters accumulated
+ * in the old-gen list would otherwise be stranded.
+ * Move them back to the global free list. This is safe
+ * for both transient and persistent failures: the
+ * counters are still valid and can be reused.
+ */
+ if (!TAILQ_EMPTY(&pool->counters[query_gen])) {
+ rte_spinlock_lock(&pool->csl);
+ rte_spinlock_lock(&cmng->csl[cnt_type]);
+ TAILQ_CONCAT(&cmng->counters[cnt_type],
+ &pool->counters[query_gen], next);
+ rte_spinlock_unlock(&cmng->csl[cnt_type]);
+ rte_spinlock_unlock(&pool->csl);
+ }
} else {
raw_to_free = pool->raw;
if (pool->is_aged)
@@ -9910,11 +9932,20 @@ mlx5_flow_async_pool_query_handle(struct mlx5_dev_ctx_shared *sh,
rte_spinlock_unlock(&pool->sl);
/* Be sure the new raw counters data is updated in memory. */
rte_io_wmb();
+ /*
+ * A counter free thread may have read a stale query_gen
+ * before the generation was flipped and could still be
+ * inserting into this same old-gen list. Hold pool->csl to
+ * serialize TAILQ_CONCAT with that TAILQ_INSERT_TAIL and
+ * avoid corrupting the list.
+ */
if (!TAILQ_EMPTY(&pool->counters[query_gen])) {
+ rte_spinlock_lock(&pool->csl);
rte_spinlock_lock(&cmng->csl[cnt_type]);
TAILQ_CONCAT(&cmng->counters[cnt_type],
&pool->counters[query_gen], next);
rte_spinlock_unlock(&cmng->csl[cnt_type]);
+ rte_spinlock_unlock(&pool->csl);
}
}
LIST_INSERT_HEAD(&sh->sws_cmng.free_stat_raws, raw_to_free, next);
diff --git a/drivers/net/mlx5/mlx5_flow_dv.c b/drivers/net/mlx5/mlx5_flow_dv.c
index 5eb92a7d7a..83850096b3 100644
--- a/drivers/net/mlx5/mlx5_flow_dv.c
+++ b/drivers/net/mlx5/mlx5_flow_dv.c
@@ -7119,6 +7119,7 @@ flow_dv_counter_free(struct rte_eth_dev *dev, uint32_t counter)
struct mlx5_flow_counter_pool *pool = NULL;
struct mlx5_flow_counter *cnt;
enum mlx5_counter_type cnt_type;
+ uint32_t query_gen;
if (!counter)
return;
@@ -7143,16 +7144,17 @@ flow_dv_counter_free(struct rte_eth_dev *dev, uint32_t counter)
cnt->pool = pool;
/*
* Put the counter back to list to be updated in none fallback mode.
- * Currently, we are using two list alternately, while one is in query,
+ * Currently, we are using two lists alternately, while one is in query,
* add the freed counter to the other list based on the pool query_gen
* value. After query finishes, add counter the list to the global
- * container counter list. The list changes while query starts. In
- * this case, lock will not be needed as query callback and release
- * function both operate with the different list.
+ * container counter list. Cache query_gen into a local variable before
+ * TAILQ_INSERT_TAIL, since the macro evaluates its head argument
+ * multiple times and pool->query_gen is a volatile bit-field.
*/
if (!priv->sh->sws_cmng.counter_fallback) {
rte_spinlock_lock(&pool->csl);
- TAILQ_INSERT_TAIL(&pool->counters[pool->query_gen], cnt, next);
+ query_gen = pool->query_gen;
+ TAILQ_INSERT_TAIL(&pool->counters[query_gen], cnt, next);
rte_spinlock_unlock(&pool->csl);
} else {
cnt->dcs_when_free = cnt->dcs_when_active;
--
2.47.3
---
Diff of the applied patch vs upstream commit (please double-check if non-empty:
---
--- - 2026-07-03 12:55:47.357434126 +0100
+++ 0017-net-mlx5-fix-flow-counter-race-on-query-completion.patch 2026-07-03 12:55:46.622572724 +0100
@@ -1 +1 @@
-From 60af6e80b99b07aa992f7738ed920a7b99cbdbdd Mon Sep 17 00:00:00 2001
+From 4b8e73813ccf7034dca2843919eb683b0b195ecc Mon Sep 17 00:00:00 2001
@@ -5,0 +6,2 @@
+[ upstream commit 60af6e80b99b07aa992f7738ed920a7b99cbdbdd ]
+
@@ -62 +63,0 @@
-Cc: stable at dpdk.org
@@ -73 +74 @@
-index b21cc2c287..c5bc728fae 100644
+index 83165bd717..bf1fdf1941 100644
@@ -76,2 +77 @@
-@@ -938,6 +938,7 @@ Lin Li <lilintjpu at bytedance.com> <lilin24 at baidu.com>
- Linfan Hu <zhongdahulinfan at 163.com>
+@@ -894,6 +894,7 @@ Linfan Hu <zhongdahulinfan at 163.com>
@@ -79,0 +80 @@
+ Lin Li <lilintjpu at bytedance.com> <lilin24 at baidu.com>
@@ -83 +84 @@
- Liron Himi <lironh at marvell.com>
+ Li Qiang <liq3ea at 163.com>
@@ -85 +86 @@
-index 4b984df892..e8141d2100 100644
+index e296fab6c1..9735c664ad 100644
@@ -88 +89 @@
-@@ -9893,6 +9893,13 @@ void
+@@ -9890,6 +9890,13 @@ void
@@ -102 +103 @@
-@@ -9904,6 +9911,21 @@ mlx5_flow_async_pool_query_handle(struct mlx5_dev_ctx_shared *sh,
+@@ -9901,6 +9908,21 @@ mlx5_flow_async_pool_query_handle(struct mlx5_dev_ctx_shared *sh,
@@ -124 +125 @@
-@@ -9913,11 +9935,20 @@ mlx5_flow_async_pool_query_handle(struct mlx5_dev_ctx_shared *sh,
+@@ -9910,11 +9932,20 @@ mlx5_flow_async_pool_query_handle(struct mlx5_dev_ctx_shared *sh,
@@ -146 +147 @@
-index 307354c886..58ebcf87eb 100644
+index 5eb92a7d7a..83850096b3 100644
@@ -149 +150 @@
-@@ -7129,6 +7129,7 @@ flow_dv_counter_free(struct rte_eth_dev *dev, uint32_t counter)
+@@ -7119,6 +7119,7 @@ flow_dv_counter_free(struct rte_eth_dev *dev, uint32_t counter)
@@ -157 +158 @@
-@@ -7153,16 +7154,17 @@ flow_dv_counter_free(struct rte_eth_dev *dev, uint32_t counter)
+@@ -7143,16 +7144,17 @@ flow_dv_counter_free(struct rte_eth_dev *dev, uint32_t counter)
More information about the stable
mailing list