On Mon, Jan 12, 2026 at 05:09:18PM -0800, Stephen Hemminger wrote: > On Mon, 12 Jan 2026 18:36:47 +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] > > > > Signed-off-by: Dariusz Sosnowski <[email protected]> > > --- > > Much better, but there some suggestions. > Yes this AI "bikeshedding" which is a first... > > ## Patch Review: app/testpmd: fix flow queue job leaks > > ### Summary > This patch fixes memory leaks in testpmd's async flow API by tracking > `queue_job` structs in a linked list and ensuring proper cleanup when > flushing flow queues or closing ports. > > --- > > ### ✅ Commit Message Analysis > > | Check | Status | Notes | > |-------|--------|-------| > | Subject ≤60 chars | ✅ Pass | `app/testpmd: fix flow queue job leaks` = 37 > chars | > | Correct prefix | ✅ Pass | `app/testpmd:` is correct | > | Lowercase after colon | ✅ Pass | | > | Imperative mood | ✅ Pass | "fix" is imperative | > | No trailing period | ✅ Pass | | > | No forbidden punctuation | ✅ Pass | | > | Body doesn't start with "It" | ✅ Pass | Starts with "Each enqueued..." | > | `Signed-off-by:` present | ✅ Pass | Real name with valid email | > | `Fixes:` tags present | ✅ Pass | 5 Fixes tags with 12-char SHAs | > | `Cc: [email protected]` present | ✅ Pass | Correctly included for bug fix | > | Blank line before Signed-off-by | ✅ Pass | | > | Tag order | ✅ Pass | Fixes → Cc → blank → Signed-off-by | > > --- > > ### ⚠️ Warnings > > **1. Unnecessary cast of `void *` (line ~420)** > ```c > job = (struct queue_job *)res[i].user_data; > ``` > If `user_data` is `void *`, this cast is unnecessary in C and should be > removed: > ```c > job = res[i].user_data; > ```
Fixed in v4. > > --- > > ### ℹ️ Informational Notes > > **1. Function naming could be clearer** > > The function `port_alloc_queue_job()` doesn't actually allocate anything—it > inserts a job into a linked list. A clearer name might be > `port_register_queue_job()` or `port_track_queue_job()`. False positive - port_alloc_queue_job() function does not exist. > > **2. Commit body line lengths** > > The ASAN stack trace in the commit message appears to have lines that may > approach or exceed 75 characters due to indentation. While the content itself > is useful, you may want to verify line lengths or consider trimming the stack > trace to the most relevant frames. > > --- > > ### ✅ Code Style Verification > > | Check | Status | > |-------|--------| > | Lines ≤100 characters | ✅ Pass | > | Explicit NULL comparisons | ✅ Pass | > | Explicit zero comparisons | ✅ Pass | > | No forbidden APIs | ✅ Pass | > | Function return type on own line | ✅ Pass | > | Proper comment style | ✅ Pass | > | No trailing whitespace (visible) | ✅ Pass | > | Uses DPDK list macros correctly | ✅ Pass | > > --- > > ### Code Quality Notes > > The implementation is sound: > - Properly tracks jobs per queue using `LIST_HEAD`/`LIST_ENTRY` macros > - Bounded iteration count (`FLOW_QUEUE_FLUSH_MAX_ITERS`) prevents infinite > loops > - Graceful error handling with cleanup on failure paths > - Memory is freed both on normal completion and timeout conditions > - Uses `RTE_ASSERT` with explicit NULL comparison for debugging > > --- > > ### Verdict > > **Acceptable with minor suggestions.** The patch follows DPDK coding > standards correctly. The only actionable item is the unnecessary `void *` > cast, which is a warning-level issue. The naming suggestion is purely > stylistic.
