[dpdk] Add support for additional drivers#51835
Conversation
8583079 to
88c360f
Compare
|
Thank you for hooking this stuff up! |
No worries - happy to help. I have more work to do on this port. The next stage is to write ports for the remaining dependencies, which is going to be more involved than what I've done so far, so may take a while. |
7effe3c to
aa1b6f0
Compare
There was a problem hiding this comment.
Pull request overview
This PR updates the dpdk port to expose additional DPDK drivers as opt-in vcpkg features, wiring those features into the Meson disable_drivers configuration, and bumps the port version (with corresponding versions database updates).
Changes:
- Add new
dpdkfeatures for additional drivers (e.g., bnx2x, pcap, ISA-L/zlib compression, CCP/OpenSSL crypto, QAT) and connect them to Meson driver disablement. - Bump
dpdkport-versionto 4 and update the versions database/baseline accordingly. - Update the DPDK Meson dependency patching used by the port.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| versions/d-/dpdk.json | Adds the new 26.3 port-version 4 entry. |
| versions/baseline.json | Updates dpdk baseline port-version to 4. |
| ports/dpdk/vcpkg.json | Bumps port-version and introduces new opt-in driver features with deps/supports. |
| ports/dpdk/portfile.cmake | Extends disable_drivers assembly to match the new feature set. |
| ports/dpdk/0002-fix-dependencies.patch | Adjusts Meson dependency discovery/requirements for selected drivers and libs. |
Comments suppressed due to low confidence (8)
ports/dpdk/0002-fix-dependencies.patch:102
- Using
cc.has_header(..., required: true)insideif not cc.has_header(...)is self-contradictory: withrequired: trueMeson will raise an error rather than return false, so thebuild=false/subdir_done()path won’t run. Similarly,cc.find_library(..., required: true)makes the subsequent.found()check redundant. Preferrequired: falsewith the existing soft-disable flow, or remove the conditional logic and rely on Meson’s hard error consistently when this driver is explicitly enabled.
-if not cc.has_header('mlx5devx.h')
+if not cc.has_header('mlx5devx.h', required: true)
build = false
reason = 'missing dependency, "mlx5devx.h"'
subdir_done()
endif
-devxlib = cc.find_library('mlx5devx', required: false)
+devxlib = cc.find_library('mlx5devx', required: true)
if not devxlib.found() or not cc.links(min_c_code, dependencies: devxlib)
build = false
reason = 'missing dependency, "mlx5devx"'
ports/dpdk/0002-fix-dependencies.patch:141
- With
dependency('zlib', required: true, ...), Meson will abort if zlib isn’t found, so the followingif not dep.found()/build=falseblock is dead code. Either revert torequired: false(and keep the soft-disable behavior) or remove the unreachabledep.found()handling and treat zlib as strictly required when this driver is enabled.
-dep = dependency('zlib', required: false, method: 'pkg-config')
+dep = dependency('zlib', required: true, method: 'pkg-config')
if not dep.found()
build = false
reason = 'missing dependency, "zlib"'
ports/dpdk/0002-fix-dependencies.patch:154
dependency('libcrypto', required: true, ...)makes the subsequentif not dep.found()/build=falselogic unreachable (Meson will error beforedep.found()can be false). If the intent is to keep CCP optional unless explicitly enabled, keeprequired: falseand only raise an error when the driver is requested.
-dep = dependency('libcrypto', required: false, method: 'pkg-config')
+dep = dependency('libcrypto', required: true, method: 'pkg-config')
if not dep.found()
build = false
reason = 'missing dependency, "libcrypto"'
ports/dpdk/0002-fix-dependencies.patch:167
dependency('libcrypto', required: true, ...)already hard-fails when missing, so the followingif not dep.found()/build=falseis dead code. If this driver is meant to be optional unless selected, keeprequired: falseand error only when explicitly enabled; otherwise remove the unreachable branch.
-dep = dependency('libcrypto', required: false, method: 'pkg-config')
+dep = dependency('libcrypto', required: true, method: 'pkg-config')
if not dep.found()
build = false
reason = 'missing dependency, "libcrypto"'
ports/dpdk/0002-fix-dependencies.patch:208
dependency(... required: true)makes theif not lib.found()fallback unreachable (Meson will abort before returning an unfound dependency). If you still want thefind_library()fallback, keeprequired: falseand only error after both methods fail.
libnames = ['ibverbs', 'mana']
libs = []
foreach libname:libnames
- lib = dependency('lib' + libname, required:false)
+ lib = dependency('lib' + libname, required:true)
if not lib.found()
lib = cc.find_library(libname, required:false)
endif
ports/dpdk/0002-fix-dependencies.patch:221
- Same pattern as above: setting
required: truefor the pkg-config dependency means the subsequentif not lib.found()+find_library()fallback cannot run. If pkg-config is unavailable but the library is otherwise discoverable, this will fail unnecessarily; consider keepingrequired: falseand handling the failure after the fallback attempt.
libs = []
foreach libname:libnames
lib = dependency('lib' + libname, static:static_ibverbs,
- required:false, method: 'pkg-config')
+ required:true, method: 'pkg-config')
if not lib.found() and not static_ibverbs
lib = cc.find_library(libname, required:false)
endif
ports/dpdk/0002-fix-dependencies.patch:233
- Switching to
error('missing dependency: "libpcap"')changes behavior from skipping the driver to failing the entire configuration whenRTE_HAS_LIBPCAPis unset. If this meson.build is reached even when the driver is disabled viadisable_drivers, this can break default builds; consider restoring the soft-disable (build=false/reason/subdir_done()) and only hard-error when the driver is explicitly enabled.
if not dpdk_conf.has('RTE_HAS_LIBPCAP')
- build = false
- reason = 'missing dependency, "libpcap"'
+ error('missing dependency: "libpcap"')
endif
ports/dpdk/0002-fix-dependencies.patch:256
- For
raw/ifpga, switching missing-libfdthandling toerror()can make an otherwise-disabled driver abort the whole build if this file is evaluated unconditionally. Additionally,cc.find_library('rt', required: true)makes the subsequentrtdep.found()/cc.links()check inconsistent because Meson will error before returning an unfound dependency. Prefer the soft-disable flow when the driver is not selected, and only hard-fail when the driver is explicitly enabled.
@@ -2,14 +2,12 @@
# Copyright(c) 2018 Intel Corporation
if not has_libfdt
- build = false
- reason = 'missing dependency, "libfdt"'
- subdir_done()
+ error('missing dependency: "libfdt"')
endif
rtdep = dependency('librt', required: false)
if not rtdep.found()
- rtdep = cc.find_library('rt', required: false)
+ rtdep = cc.find_library('rt', required: true)
endif
if not rtdep.found() or not cc.links(min_c_code, dependencies: rtdep)
build = false
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
copilot really seems confused in reviewing patches. In fairness so are humans |
./vcpkg x-add-version --alland committing the result.