On Fri, 9 Jan 2026 16:26:07 +0100
Dariusz Sosnowski <[email protected]> wrote:
> Each enqueued async flow operation in testpmd has an associated
> queue_job struct. It is passed in user data and used to determine
> the type of operation when operation results are pulled on a given
> queue. This information informs the necessary additional handling
> (e.g., freeing flow struct or dumping the queried action state).
>
> If async flow operations were enqueued and results were not pulled
> before quitting testpmd, these queue_job structs were leaked as reported
> by ASAN:
>
> Direct leak of 88 byte(s) in 1 object(s) allocated from:
> #0 0x7f7539084a37 in __interceptor_calloc
> ../../../../src/libsanitizer/asan/asan_malloc_linux.cpp:154
> #1 0x55a872c8e512 in port_queue_flow_create
> (/download/dpdk/install/bin/dpdk-testpmd+0x4cd512)
> #2 0x55a872c28414 in cmd_flow_cb
> (/download/dpdk/install/bin/dpdk-testpmd+0x467414)
> #3 0x55a8734fa6a3 in __cmdline_parse
> (/download/dpdk/install/bin/dpdk-testpmd+0xd396a3)
> #4 0x55a8734f6130 in cmdline_valid_buffer
> (/download/dpdk/install/bin/dpdk-testpmd+0xd35130)
> #5 0x55a873503b4f in rdline_char_in
> (/download/dpdk/install/bin/dpdk-testpmd+0xd42b4f)
> #6 0x55a8734f62ba in cmdline_in
> (/download/dpdk/install/bin/dpdk-testpmd+0xd352ba)
> #7 0x55a8734f65eb in cmdline_interact
> (/download/dpdk/install/bin/dpdk-testpmd+0xd355eb)
> #8 0x55a872c19b8e in prompt
> (/download/dpdk/install/bin/dpdk-testpmd+0x458b8e)
> #9 0x55a872be425a in main
> (/download/dpdk/install/bin/dpdk-testpmd+0x42325a)
> #10 0x7f7538756d8f in __libc_start_call_main
> ../sysdeps/nptl/libc_start_call_main.h:58
>
> This patch addresses that by registering all queue_job structs, for a
> given queue, on a linked list. Whenever operation results are pulled
> and result is handled, queue_job struct will be removed from that list
> and freed.
> Before port is closed, during flow flush, testpmd will pull
> all of the expected results
> (based on the number of queue_job on the list).
>
> Fixes: c9dc03840873 ("ethdev: add indirect action async query")
> Fixes: 99231e480b69 ("ethdev: add template table resize")
> Fixes: 77e7939acf1f ("app/testpmd: add flow rule update command")
> Fixes: 3e3edab530a1 ("ethdev: add flow quota")
> Fixes: 966eb55e9a00 ("ethdev: add queue-based API to report aged flow rules")
> Cc: [email protected]
>
Surprisingly AI code spotted a minor bug which I did not see.
---
## DPDK Patch Evaluation: `app/testpmd: fix flow queue job leaks`
### Summary
| Category | Status |
|----------|--------|
| Commit Message | ✅ Pass |
| Tags | ✅ Pass |
| Code Style | ⚠️ Minor issues |
| Code Correctness | ❌ **Bug found** |
---
### Commit Message Analysis
**Subject Line:** `app/testpmd: fix flow queue job leaks`
| Criterion | Result |
|-----------|--------|
| Length (≤60 chars) | ✅ 37 characters |
| Format `component: description` | ✅ Correct |
| Component prefix | ✅ `app/testpmd:` (not `testpmd:`) |
| Lowercase after colon | ✅ Yes |
| Imperative mood | ✅ "fix" |
| No trailing period | ✅ Correct |
| No forbidden punctuation | ✅ Clean |
**Body:**
- ✅ Does not start with "It"
- ✅ Provides clear context (ASAN leak report, explanation of queue_job
lifecycle)
- ✅ Lines appear within 75 character limit
- ✅ Well-structured explanation of the problem and solution
**Tags:**
- ✅ Multiple `Fixes:` tags with 12-character SHAs and quoted subjects
- ✅ `Cc: [email protected]` present (appropriate for bug fix)
- ✅ `Signed-off-by:` with real name and valid email
- ✅ Tag order correct: Fixes → Cc → blank line → Signed-off-by
---
### Code Style Analysis
**Positive:**
- ✅ Function return types on separate lines
- ✅ Explicit comparisons (`res == NULL`, `ret < 0`, `ret == 0`)
- ✅ Proper brace style (single statements without braces)
- ✅ Lowercase function names with underscores
- ✅ Comment style follows guidelines
- ✅ Lines within 100 characters
**Info (minor observations):**
- C99-style loop variable declarations (`for (int i = 0; ...)` and `for
(queueid_t queue_id = 0; ...)`). This is acceptable in DPDK but some
maintainers prefer traditional style.
---
### ❌ Critical Bug Found: Format String Argument Mismatch
There are **two printf format string bugs** where arguments are in the wrong
order:
**Bug 1 (line ~428-429):**
```c
printf("Flushed flow queue %u on port %u (%u failed, %u succeeded).\n",
port_id, queue_id, polled - success, success);
```
Format says "queue %u on port %u" but arguments are `port_id, queue_id` —
**reversed**.
**Bug 2 (line ~437-438):**
```c
TESTPMD_LOG(ERR, "Unable to fully flush flow queue %u on port %u (left ops
%u)\n",
port_id, queue_id, expected_ops);
```
Same issue — format expects queue then port, but arguments are port then queue.
**Correct version should be:**
```c
printf("Flushed flow queue %u on port %u (%u failed, %u succeeded).\n",
queue_id, port_id, polled - success, success);
TESTPMD_LOG(ERR, "Unable to fully flush flow queue %u on port %u (left ops
%u)\n",
queue_id, port_id, expected_ops);
```
Note: Line ~475-476 has the correct order (`queue_id, port_id`), which makes
this inconsistency more apparent.
---
### Verdict
| Severity | Issue |
|----------|-------|
| **Error** | Format string argument order bug (2 instances) — will print
misleading debug output |
| Info | C99 loop variable declarations |
**Recommendation:** Request v3 to fix the printf argument order. The fix itself
is sound in design — tracking queue_job entries on a list and flushing on port
close is the right approach. The ASAN leak is properly addressed. However, the
swapped format arguments would cause confusing output during debugging
("Flushed flow queue 0 on port 1" when it should say "queue 1 on port 0").
