On Sun, Jan 11, 2026 at 05:10:16PM -0800, Stephen Hemminger wrote: > 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. > > ---
Fixed in v3: https://patches.dpdk.org/project/dpdk/patch/[email protected]/ > > ### 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"). >
