Skip to content

Conversation

@maxime-leroy
Copy link
Collaborator

@maxime-leroy maxime-leroy commented Oct 21, 2025

Summary by CodeRabbit

  • Chores
    • Updated DPDK dependency to 24.11.3 (minor patch release).
    • Introduced a new build option to enable static linking against DPDK and applied it across build and packaging configurations (build system, Debian packaging, RPM spec, and makefile invocations) to produce consistent static-linked artifacts.

@coderabbitai
Copy link

coderabbitai bot commented Oct 21, 2025

📝 Walkthrough

Walkthrough

Removed the default_library=static default option from the libdpdk dependency in meson.build and replaced it with static: get_option('dpdk_static'). Added a new Meson boolean option dpdk_static (default false) in meson_options.txt. Updated build invocations to pass -Ddpdk_static=true in GNUmakefile, debian/rules, and rpm/grout.spec. Bumped the dpdk subproject wrap from v24.11.1 to v24.11.3. No control-flow or runtime code changes observed.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Changes span multiple build and packaging files with moderate heterogeneity (Meson build, options, wraps, Makefile, packaging). Review requires verifying option propagation, wrap bump, and no unintended build regressions.

Suggested reviewers

  • rjarry

Pre-merge checks

✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check ✅ Passed The title "Dpdk force static link" directly addresses the primary change in this PR: adding support for static linking of DPDK libraries across the build system. The changeset introduces a new configurable dpdk_static Meson option and applies it consistently across multiple build files (GNUmakefile, debian/rules, rpm/grout.spec). While "force" is somewhat imprecise terminology (the option defaults to false and is configurable rather than always enabled), it reasonably conveys the capability to statically link DPDK when needed. The title is clear, specific, and directly summarizes the main change.
Docstring Coverage ✅ Passed No functions found in the changes. Docstring coverage check skipped.

📜 Recent review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 5cf2411 and eda4b32.

📒 Files selected for processing (6)
  • GNUmakefile (1 hunks)
  • debian/rules (1 hunks)
  • meson.build (1 hunks)
  • meson_options.txt (1 hunks)
  • rpm/grout.spec (1 hunks)
  • subprojects/dpdk.wrap (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
  • meson.build
  • subprojects/dpdk.wrap
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (7)
  • GitHub Check: build-and-tests (clang-18, none, debugoptimized, ubuntu-24.04, false)
  • GitHub Check: build-and-tests (clang-16, none, debugoptimized, ubuntu-24.04, false)
  • GitHub Check: build-and-tests (gcc-13, none, debugoptimized, ubuntu-24.04, -Dfrr=enabled, false)
  • GitHub Check: build-and-tests (clang-15, none, debugoptimized, ubuntu-22.04, false)
  • GitHub Check: build-and-tests (gcc-14, address, debug, ubuntu-24.04, -Dfrr=enabled, true)
  • GitHub Check: deb
  • GitHub Check: rpm
🔇 Additional comments (4)
meson_options.txt (1)

19-22: LGTM! Option definition is clean and well-documented.

The new dpdk_static option correctly implements opt-in static linking. The default false allows flexible local builds while official build paths (make, deb, rpm) explicitly enable it to maintain static linking behavior.

rpm/grout.spec (1)

78-78: LGTM! Correctly propagates the dpdk_static option.

The -Ddpdk_static=true flag is properly added to the meson invocation for RPM builds.

debian/rules (1)

12-12: LGTM! Correctly propagates the dpdk_static option.

The -Ddpdk_static=true flag is properly added to the Debian build configuration.

GNUmakefile (1)

57-58: LGTM! Clean restructuring with consistent option propagation.

The meson_opts definition is now more readable, and -Ddpdk_static=true is correctly added alongside the sanitize option. This ensures static DPDK linking for all make-based builds.


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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

🧹 Nitpick comments (1)
subprojects/dpdk.wrap (1)

3-3: Consider pinning to a commit SHA for reproducibility.

Tags can be moved; locking revision to the exact commit for v24.11.3 avoids supply‑chain drift in long‑term builds.

📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 23666dc and 5cf2411.

📒 Files selected for processing (2)
  • meson.build (1 hunks)
  • subprojects/dpdk.wrap (1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (8)
  • GitHub Check: build-and-tests (clang-18, none, debugoptimized, ubuntu-24.04, false)
  • GitHub Check: build-and-tests (gcc-14, address, debug, ubuntu-24.04, -Dfrr=enabled, true)
  • GitHub Check: build-and-tests (clang-16, none, debugoptimized, ubuntu-24.04, false)
  • GitHub Check: build-and-tests (clang-15, none, debugoptimized, ubuntu-22.04, false)
  • GitHub Check: build-and-tests (gcc-13, none, debugoptimized, ubuntu-24.04, -Dfrr=enabled, false)
  • GitHub Check: build-cross-aarch64
  • GitHub Check: rpm
  • GitHub Check: deb
🔇 Additional comments (3)
meson.build (3)

59-63: Good: version bump + static dep.

version: '>= 24.11.3' matches the wrap, and static: true enforces static linking when available. LGTM.


71-72: Heads‑up: static link may pull external static deps (mlx5, RDMA).

Enabling common/mlx5,net/mlx5 under static linking typically requires static rdma‑core (libibverbs, libmlx5) and friends. Ensure those static libs are present in CI/build images or consider gating/removing mlx5 from the static profile.


64-76: No changes needed. All option types and values are correct for DPDK v24.11.3:

  • developer_mode (feature type) properly uses disabled
  • tests, enable_docs, enable_kmods (boolean types) properly use false
  • enable_drivers, enable_libs, disable_apps (string types) properly use string values

Copy link
Collaborator

@rjarry rjarry left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Static linking should be disabled by default. You can add a meson option to enable it if you want.

Also, do not change the minimum required version in meson.build since it does not add any crucial features. You can simply bump the version in subprojects/dpdk.wrap.

This release contains several fixes, update accordingly.

Signed-off-by: Maxime Leroy <[email protected]>
DPDK always builds both shared and static libraries, regardless of the
default_library setting. The default_library option only controls how
applications are linked, not what DPDK produces.

In Buildroot, Meson libraries are built as shared by default and static
linking with glibc is not supported globally. However, linking our
application against the DPDK static library provides better performance.

Add a new meson option to force static linking on the DPDK dependency.

Signed-off-by: Maxime Leroy <[email protected]>
@maxime-leroy maxime-leroy force-pushed the dpdk_force_static_link branch from 5cf2411 to eda4b32 Compare October 21, 2025 12:14
@coderabbitai coderabbitai bot requested a review from rjarry October 21, 2025 12:15
@rjarry rjarry merged commit ec9bf5a into DPDK:main Oct 21, 2025
11 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants