infra: add pdump support for live packet capture with tcpdump#561
infra: add pdump support for live packet capture with tcpdump#561vjardin wants to merge 1 commit into
Conversation
Add a --pdump flag that enables DPDK secondary process attachment for packet capture. When set, grout starts without --in-memory (shared hugepages) and calls rte_pdump_init() to register the IPC handler. Stock system tcpdump can then attach as a secondary process and capture live traffic using the grout:N device syntax. A wrapper script (subprojects/packagefiles/libpcap/grout-tcpdump.sh) handles the LD_LIBRARY_PATH setup while libpcap upstream support is pending (the-tcpdump-group/libpcap#1656). The libpcap subproject is built as a shared library with DPDK pdump support via cmake. It produces libpcap.so with both PCAP_SUPPORT_DPDK (standalone dpdk:N devices) and PCAP_SUPPORT_DPDK_PDUMP (secondary grout:N devices) enabled. The DPDK subproject requires a patch to lib/eal/common/eal_common_thread.c that removes an overly conservative rte_mp_disable() call from rte_thread_register(), which blocked secondary process attachment when non-EAL worker threads are registered. Signed-off-by: Vincent Jardin <vjardin@free.fr>
|
dependencies: |
📝 WalkthroughWalkthroughThe pull request adds packet dump (pdump) functionality using DPDK's pdump library. Changes include adding a pdump configuration flag to the gr_config structure, introducing a -P/--pdump command-line option, implementing pdump initialization in dpdk_init after rte_eal_init, updating DPDK build configuration to enable pdump and bpf libraries, building a custom libpcap with DPDK pdump support, and adding a tcpdump wrapper script that uses the custom libpcap for packet capture. 📝 Coding Plan
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment Tip CodeRabbit can generate a title for your PR based on the changes with custom instructions.Set the |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@main/dpdk.c`:
- Around line 190-198: The check for rte_pdump_init() incorrectly negates its
return value (which is -1) and loses the real error in rte_errno; change the
failure branch so that when rte_pdump_init() < 0 you read the actual error code
from rte_errno (e.g. int err = rte_errno), log rte_strerror(err) via LOG(...),
and set ret to -err (to preserve the negative return convention) before jumping
to end; update the block around gr_config.pdump / rte_pdump_init to use
rte_errno for both logging and the returned error value.
In `@meson.build`:
- Around line 168-193: The custom_target libpcap_build runs cmake/make against
libpcap but doesn't declare a build dependency on the DPDK build, causing races
in parallel builds; update the libpcap_build target to add a depends entry
referencing the DPDK build target (for example the dpdk library target from the
dpdk subproject or an existing executable like grout that already depends on
DPDK) so the DPDK artifacts and pkg-config files are guaranteed to exist before
running the cmake command; keep the dependency as the dpdk build target name
already present in your meson project (or grout) so cmake runs only after DPDK
is built.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: b3aa95ca-b07d-4663-9c45-d05796c969ad
📒 Files selected for processing (6)
main/dpdk.cmain/gr_config.hmain/main.cmeson.buildsubprojects/libpcap.wrapsubprojects/packagefiles/libpcap/grout-tcpdump.sh
| if (gr_config.pdump) { | ||
| ret = rte_pdump_init(); | ||
| if (ret < 0) { | ||
| ret = -ret; | ||
| LOG(ERR, "rte_pdump_init: %s", rte_strerror(ret)); | ||
| goto end; | ||
| } | ||
| LOG(NOTICE, "pdump enabled, secondary processes can attach for packet capture"); | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
DPDK rte_pdump_init return value and rte_errno
💡 Result:
The DPDK function rte_pdump_init returns an int value: 0 on success. It returns -1 on failure, in which case rte_errno is set accordingly to indicate the specific error condition. This follows the standard DPDK API convention for functions returning int, as seen in related APIs like rte_pdump_enable and older documentation explicitly stating "Returns: 0 on success, -1 on error, rte_errno is set accordingly." (e.g., DPDK 17.11 docs). No specific error codes unique to rte_pdump_init are documented; rte_errno provides DPDK-specific or system error details (e.g., E_RTE_SECONDARY if called from secondary process). Official DPDK documentation and manpages confirm the signature int rte_pdump_init(void) and usage in primary processes like testpmd for initializing the packet capture framework.
Citations:
- 1: https://manpages.debian.org/unstable/dpdk-doc/rte_pdump_init.3.en.html
- 2: https://doc.dpdk.org/api/rte__pdump_8h.html
- 3: https://doc.dpdk.org/api-17.11/rte__pdump_8h.html
- 4: https://doc.dpdk.org/api-20.05/rte__pdump_8h.html
- 5: https://doc.dpdk.org/guides-24.07/prog_guide/pdump_lib.html
- 6: https://doc.dpdk.org/guides-25.07/prog_guide/pdump_lib.html
Incorrect error code extraction from rte_pdump_init().
rte_pdump_init() returns -1 on failure with the actual error code stored in rte_errno. The current code negates the return value (yielding 1), which loses the real error information.
Proposed fix
if (gr_config.pdump) {
ret = rte_pdump_init();
if (ret < 0) {
- ret = -ret;
+ ret = rte_errno;
LOG(ERR, "rte_pdump_init: %s", rte_strerror(ret));
goto end;
}
LOG(NOTICE, "pdump enabled, secondary processes can attach for packet capture");
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@main/dpdk.c` around lines 190 - 198, The check for rte_pdump_init()
incorrectly negates its return value (which is -1) and loses the real error in
rte_errno; change the failure branch so that when rte_pdump_init() < 0 you read
the actual error code from rte_errno (e.g. int err = rte_errno), log
rte_strerror(err) via LOG(...), and set ret to -err (to preserve the negative
return convention) before jumping to end; update the block around
gr_config.pdump / rte_pdump_init to use rte_errno for both logging and the
returned error value.
| libpcap_build = custom_target( | ||
| 'libpcap', | ||
| output: '.libpcap-stamp', | ||
| command: [ | ||
| 'sh', '-c', | ||
| 'mkdir -p "' + libpcap_builddir + '" && ' + | ||
| 'cd "' + libpcap_builddir + '" && ' + | ||
| 'export PKG_CONFIG_PATH="' + dpdk_pkgconfig_path + ':' + dpdk_pc_path + ':$PKG_CONFIG_PATH" && ' + | ||
| 'DPDK_LIBDIR="' + meson.global_build_root() + '/subprojects/dpdk/lib" && ' + | ||
| 'DPDK_DRVDIR="' + meson.global_build_root() + '/subprojects/dpdk/drivers" && ' + | ||
| 'cmake "' + libpcap_srcdir + '" ' + | ||
| '-DDISABLE_DPDK:BOOL=OFF ' + | ||
| '-DHAVE_RTE_ETH_DEV_COUNT_AVAIL:BOOL=ON ' + | ||
| '-DHAVE_STRUCT_RTE_ETHER_ADDR:BOOL=ON ' + | ||
| '-DDISABLE_DBUS:BOOL=ON ' + | ||
| '-DDISABLE_RDMA:BOOL=ON ' + | ||
| '-DDISABLE_BLUETOOTH:BOOL=ON ' + | ||
| '-DDISABLE_NETMAP:BOOL=ON ' + | ||
| '-DBUILD_SHARED_LIBS:BOOL=ON ' + | ||
| '"-DCMAKE_SHARED_LINKER_FLAGS=-L$DPDK_LIBDIR -L$DPDK_DRVDIR -Wl,-rpath,$DPDK_LIBDIR:$DPDK_DRVDIR" ' + | ||
| '&& make -j pcap && ' + | ||
| 'ln -sf libpcap.so.1 libpcap.so.0.8 && ' + | ||
| 'touch "@OUTPUT@"', | ||
| ], | ||
| build_by_default: true, | ||
| ) |
There was a problem hiding this comment.
Missing build dependency on DPDK could cause parallel build failures.
The custom_target for libpcap relies on DPDK libraries and pkg-config files existing at configure/build time, but doesn't declare a dependency on the DPDK build. In a parallel build (ninja -j$(nproc)), cmake may run before DPDK artifacts are available.
Proposed fix
libpcap_build = custom_target(
'libpcap',
output: '.libpcap-stamp',
+ depends: dpdk_dep.type_name() == 'internal' ? subproject('dpdk').get_variable('dpdk_lib') : [],
command: [Alternatively, if dpdk_lib isn't exposed, you could depend on the grout executable or another target that already depends on DPDK:
libpcap_build = custom_target(
'libpcap',
output: '.libpcap-stamp',
+ depends: [grout_exe],
command: [🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@meson.build` around lines 168 - 193, The custom_target libpcap_build runs
cmake/make against libpcap but doesn't declare a build dependency on the DPDK
build, causing races in parallel builds; update the libpcap_build target to add
a depends entry referencing the DPDK build target (for example the dpdk library
target from the dpdk subproject or an existing executable like grout that
already depends on DPDK) so the DPDK artifacts and pkg-config files are
guaranteed to exist before running the cmake command; keep the dependency as the
dpdk build target name already present in your meson project (or grout) so cmake
runs only after DPDK is built.
|
close, let's have a different approach. I'll open a new serie. |
1 participant
Add a --pdump flag that enables DPDK secondary process attachment for packet capture. When set, grout starts without --in-memory (shared hugepages) and calls rte_pdump_init() to register the IPC handler.
Stock system tcpdump can then attach as a secondary process and capture live traffic using the grout:N device syntax. A wrapper script (subprojects/packagefiles/libpcap/grout-tcpdump.sh) handles the LD_LIBRARY_PATH setup while libpcap upstream support is pending (the-tcpdump-group/libpcap#1656).
The libpcap subproject is built as a shared library with DPDK pdump support via cmake. It produces libpcap.so with both PCAP_SUPPORT_DPDK (standalone dpdk:N devices) and PCAP_SUPPORT_DPDK_PDUMP (secondary grout:N devices) enabled.
The DPDK subproject requires a patch to lib/eal/common/eal_common_thread.c that removes an overly conservative rte_mp_disable() call from rte_thread_register(), which blocked secondary process attachment when non-EAL worker threads are registered.
Packet dump support for tcpdump secondary process attachment
Adds
--pdumpflag to enable DPDK secondary process mode, allowing system tcpdump with DPDK support to attach and capture traffic ongrout:Ndevices.Application changes
main/dpdk.c:
rte_pdump.h--in-memoryflag when pdump is enabled (line:else if (!gr_config.pdump))rte_pdump_init()afterrte_eal_init()if pdump is active, logs errors and NOTICE level message on successmain/gr_config.h:
bool pdumpfield to struct gr_configmain/main.c:
-P/--pdumpcommand-line option with documentationgr_config.pdump = truewhen flag is providedBuild system changes
meson.build:
pdumpandbpflibraries inenable_libsoptionlibpcap-build)PKG_CONFIG_PATHto DPDK pkg-config directoriesDPDK_LIBDIRandDPDK_DRVDIRenvironment variables for CMakelibpcap.so.0.8 → libpcap.so.1subprojects/libpcap.wrap:
vj_add_dpdk_groutbranch (depth=1)Packet capture wrapper
subprojects/packagefiles/libpcap/grout-tcpdump.sh:
LD_LIBRARY_PATHto prioritize local libpcap.so.0.8 and DPDK librariesDPDK_CFGwith secondary process configuration and driver path for secondary process PMD discovery