[PWCI] "[v3,1/4] compressdev: support for dictionaries and PDCP checksum" #136
Conversation
The rel_mbufs_type value for tracking the Tx cleanup function is no longer used in the iavf code. Therefore it can be removed. Signed-off-by: Bruce Richardson <bruce.richardson@intel.com> Acked-by: Ciara Loftus <ciara.loftus@intel.com>
Rather than calling the mempool function rte_mempool_get_bulk we update the code to use the mbuf function rte_mbuf_raw_alloc_bulk, which properly supports debug flags and checks - when enabled. Signed-off-by: Bruce Richardson <bruce.richardson@intel.com> Acked-by: Morten Brørup <mb@smartsharesystems.com>
Rather than calling the mempool function rte_pktmbuf_put_bulk directly in drivers, switch to using the rte_mbuf_raw_free_bulk() function which properly supports debug flags and checks, when they are enabled. Signed-off-by: Bruce Richardson <bruce.richardson@intel.com> Acked-by: Morten Brørup <mb@smartsharesystems.com>
Add a check for the AVX512DQ CPU flag to the x86 vector capability function. This flag is required by the idpf and cpfl drivers which will soon use this function during their Rx path selection process. Signed-off-by: Ciara Loftus <ciara.loftus@intel.com> Acked-by: Bruce Richardson <bruce.richardson@intel.com> Tested-by: Shaiq Wani <shaiq.wani@intel.com>
Use the new function for determining the maximum simd bitwidth in the idpf driver. Signed-off-by: Ciara Loftus <ciara.loftus@intel.com> Tested-by: Shaiq Wani <shaiq.wani@intel.com>
Update the common Rx path selection infrastructure to include the feature "single queue" which is relevant for the idpf and cpfl drivers. Replace the existing complicated logic in the idpf driver with the use of the common function. Signed-off-by: Ciara Loftus <ciara.loftus@intel.com> Tested-by: Shaiq Wani <shaiq.wani@intel.com>
Use the new function for determining the maximum simd bitwidth in the cpfl driver. Also, remove unused elements related to AVX capability from the idpf_vport structure, now that neither the idpf or cpfl drivers use them. Signed-off-by: Ciara Loftus <ciara.loftus@intel.com> Tested-by: Shaiq Wani <shaiq.wani@intel.com>
Replace the existing complicated logic with the use of the common function. Signed-off-by: Ciara Loftus <ciara.loftus@intel.com> Tested-by: Shaiq Wani <shaiq.wani@intel.com>
The free tx mbuf on demand feature has been supported since commit ab7cfe1 ("net/ice: cleanup Tx buffers"). Update the list of supported features to reflect this. Fixes: ab7cfe1 ("net/ice: cleanup Tx buffers") Cc: stable@dpdk.org Signed-off-by: Ciara Loftus <ciara.loftus@intel.com> Acked-by: Bruce Richardson <bruce.richardson@intel.com>
The iavf PMD does not report the speed capabilities that the device is capable of in the speed_capa field of the rte_eth_dev_info struct. The documentation incorrectly stated this feature as supported. Fix this. The free tx mbuf on demand feature has been supported since commit 86e4424 ("net/iavf: cleanup Tx buffers"). Update the list of supported features to reflect this. The burst mode info feature has been supported since commit 0d5a856 ("net/iavf: support Rx/Tx burst mode info"). Update the list of supported features to reflect this. The extended statistics feature has been implemented since commit d38a06b ("net/iavf: add extended stats"). Update the list of supported features to reflect this. Fixes: 48de41c ("net/avf: enable link status update") Fixes: 86e4424 ("net/iavf: cleanup Tx buffers") Fixes: 0d5a856 ("net/iavf: support Rx/Tx burst mode info") Fixes: d38a06b ("net/iavf: add extended stats") Cc: stable@dpdk.org Signed-off-by: Ciara Loftus <ciara.loftus@intel.com> Acked-by: Bruce Richardson <bruce.richardson@intel.com>
One issue with the currently imported header is that if a header (external to DPDK) starts including the equivalent system installed uapi header, then when DPDK code later includes its own uapi/ header, the header guard won't match, which may result in DPDK build failure. To address this issue, adjust the include guards in our imported uapi header and make sure those headers are included first before any external header. Update the documentation accordingly. Fixes: cf97dfd ("uapi: import Linux kernel uAPI headers") Fixes: 67b79f6 ("vhost: use imported VDUSE uAPI header") Signed-off-by: David Marchand <david.marchand@redhat.com> Reviewed-by: Maxime Coquelin <maxime.coquelin@redhat.com>
Those drivers sources do nothing related to VFIO. Signed-off-by: David Marchand <david.marchand@redhat.com> Acked-by: Anatoly Burakov <anatoly.burakov@intel.com>
Contrary to what the VFIO_PRESENT macro may suggest, there is no check on whether VFIO is enabled in the (build system) Linux kernel configuration. Replace check on VFIO_PRESENT with RTE_EAL_VFIO (for components built on FreeBSD or Windows), or simply remove it otherwise. Then remove inclusion of rte_vfio.h when the only purpose was checking VFIO_PRESENT. Signed-off-by: David Marchand <david.marchand@redhat.com> Reviewed-by: Rosen Xu <rosen.xu@altera.com>
RTE_EAL_VFIO is directly mapped to is_linux in meson. Replace this intermediate build knob with RTE_EXEC_ENV_LINUX. This is an intermediate state before importing the VFIO uapi header. Note: crypto/bcmfs was functional only for OSes supporting VFIO iow Linux. Stop compiling it on other OSes. Signed-off-by: David Marchand <david.marchand@redhat.com> Acked-by: Anatoly Burakov <anatoly.burakov@intel.com>
The public header defines a number of wrappers that can be removed or hidden internally. Either, those concern old Linux kernel versions that are not supported by dpdk anymore (DPDK now requires Linux v5.4 at least), like: - the request notifier feature, present since Linux v4.0, - the noiommu mode, present since Linux v4.5, - the capability support, present since Linux v4.6, - the msix mapping feature, present since Linux v4.16, Or, those wrappers can be made private as only consumed internally. - VFIO_GET_REGION_IDX() has no equivalent in the Linux uapi, but is only used by the Linux PCI bus code, - VFIO_DEVICE_FEATURE is only used by the CDX bus code, The last VFIO_ Linux specific macros around /dev/vfio/ and the noiommu kmod parameter are kept for convenience but prefixed with RTE_ to avoid polluting the VFIO_ namespace. Signed-off-by: David Marchand <david.marchand@redhat.com>
No need to redefine some VFIO_* macros that look like a define coming from standard linux/vfio.h header. Use RTE_DIM() and remove unneeded checks (like in get_vfio_group_idx() where j can't be >= RTE_DIM(vfio_cfg->vfio_groups)). Signed-off-by: David Marchand <david.marchand@redhat.com> Acked-by: Anatoly Burakov <anatoly.burakov@intel.com>
DPDK now requires Linux v5.4 at least. VFIO_SPAPR_TCE_IOMMU is present in Linux since v3.11. VFIO_SPAPR_TCE_v2_IOMMU is present in Linux since v4.2. Remove wrappers accordingly. Signed-off-by: David Marchand <david.marchand@redhat.com>
The DPDK API is not dependent on the VFIO API itself. Remove inclusion in rte_vfio.h (and bus_fslmc_driver.h) and add explicit inclusion where needed. Signed-off-by: David Marchand <david.marchand@redhat.com> Reviewed-by: Rosen Xu <rosen.xu@altera.com> Acked-by: Anatoly Burakov <anatoly.burakov@intel.com>
Import VFIO header (from v6.16) to be included in many parts of DPDK. Signed-off-by: David Marchand <david.marchand@redhat.com>
Now that we have a v6.16 header in DPDK, we can remove all remaining wrappers around VFIO uapi. Signed-off-by: David Marchand <david.marchand@redhat.com> Reviewed-by: Rosen Xu <rosen.xu@altera.com>
The meson find_library() API call sometimes finds a library that is actually incompatible with the current build, for example, returning a 64-bit library when doing a 32-bit build. To avoid problems with this, check each library returned from a find_library() call and treat the library as unfound if its incompatible. This checking is not necessary (or should not be necessary) for dependencies got using pkg-config, since the .pc files for each build type are stored in a different directory on the system. Signed-off-by: Bruce Richardson <bruce.richardson@intel.com> Acked-by: David Marchand <david.marchand@redhat.com>
rte_acc_cfg.h relies on rte_acc_common_cfg.h. Fixes: 32e8b7e ("baseband/acc100: refactor to segregate common code") Signed-off-by: David Marchand <david.marchand@redhat.com> Acked-by: Bruce Richardson <bruce.richardson@intel.com>
Many classes are exposing driver only headers as public headers. Move them to the driver_sdk_headers list. Signed-off-by: David Marchand <david.marchand@redhat.com> Acked-by: Bruce Richardson <bruce.richardson@intel.com> Acked-by: Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru> Acked-by: Akhil Goyal <gakhil@marvell.com>
The dma adapter header does not require including rte_dmadev_pmd.h which is a driver header. Fixes: 66a30a2 ("eventdev/dma: introduce DMA adapter") Signed-off-by: David Marchand <david.marchand@redhat.com> Acked-by: Amit Prakash Shukla <amitprakashs@marvell.com> Acked-by: Bruce Richardson <bruce.richardson@intel.com>
Use rte_os.h and its RTE_TAILQ_HEAD definition compatible with BSD sys/queue.h Fixes: 18cb075 ("gpudev: add event notification") Signed-off-by: David Marchand <david.marchand@redhat.com> Acked-by: Bruce Richardson <bruce.richardson@intel.com>
Those headers could not be included individually as they were not including their dependencies, were subject to some build warnings, or were not compiling on Windows. Fixes: 831dba4 ("bus/vmbus: add Hyper-V virtual bus support") Fixes: 5b2a1a0 ("crypto/cnxk: fix experimental version for PMD API") Fixes: e5abbee ("crypto/cnxk: add PMD API for getting CPTR") Fixes: 3ca6074 ("crypto/cnxk: add PMD API to flush CTX") Fixes: 8c3495f ("net/dpaa: support loopback API") Fixes: 12b435b ("net/iavf: support flex desc metadata extraction") Fixes: 23f627e ("net/mlx5: add flow sync API") Fixes: f5177bd ("net/mlx5: add GENEVE TLV options parser API") Fixes: 53c7158 ("raw/dpaa2_cmdif: support enqueue/dequeue operations") Fixes: c39d1e0 ("raw/ntb: setup queues") Signed-off-by: David Marchand <david.marchand@redhat.com> Acked-by: Bruce Richardson <bruce.richardson@intel.com>
This was caught while checking ARM arch headers.
In file included from buildtools/chkincs/chkincs-cpp.p/rte_memcpy_32.cpp:1:
/home/runner/work/dpdk/dpdk/lib/eal/arm/include/rte_memcpy_32.h:302:1:
error: expected declaration before ‘}’ token
302 | }
| ^
Fixes: 719834a ("use C linkage where appropriate in headers")
Cc: stable@dpdk.org
Signed-off-by: David Marchand <david.marchand@redhat.com>
Acked-by: Bruce Richardson <bruce.richardson@intel.com>
Acked-by: Morten Brørup <mb@smartsharesystems.com>
Acked-by: Mattias Rönnblom <mattias.ronnblom@ericsson.com>
Gather all headers in global variables, put headers installation in a single location and make the headers check use them instead of dpdk_chkinc_headers. This rework reveals a number of issues: - net/txgbe driver was directly installing its header, bypassing drivers/meson.build and skipping the headers check, - arch headers were not checked except for x86, - some driver headers were not checked, For the last point, a build warning must be disabled (like it is globally in DPDK) for headers from raw drivers. Finally, checkpatches is updated to avoid reintroductions of install_headers(). Signed-off-by: David Marchand <david.marchand@redhat.com> Acked-by: Bruce Richardson <bruce.richardson@intel.com>
A problem with the current headers check is that it relies on meson dependencies objects that come with their include_directories directives, and all of those point at the library / driver sources. This means that we won't detect a public header including a private (as in, not exported) header, or a driver only header. To address this issue, a staging directory is added and every header is copied to it. Drivers and library headers are staged to two different directories and the check is updated accordingly. Signed-off-by: David Marchand <david.marchand@redhat.com> Acked-by: Bruce Richardson <bruce.richardson@intel.com>
power_cpufreq.h, power_common.h and power_uncore_ops.h look like driver only headers, but were included from public headers. Move them to the driver_sdk_headers list. There is one complication though for power_cpufreq.h as it was included from a public header rte_power_cpufreq.h. Move the rte_power_core_capabilities struct definition to the public header, since a (stable) public symbol relies on it. Signed-off-by: David Marchand <david.marchand@redhat.com> Acked-by: Bruce Richardson <bruce.richardson@intel.com>
Adds definitions for PDCP checksums and apis to pass in dictionaries Signed-off-by: Sameer Vaze <svaze@qti.qualcomm.com> Signed-off-by: 0-day Robot <robot@bytheb.org>
Support to calculate PDCP checksums. Includes changes to pass in dictionaries. Adds support for sync flush Signed-off-by: Sameer Vaze <svaze@qti.qualcomm.com> Signed-off-by: 0-day Robot <robot@bytheb.org>
Adds support to read and apply dictionarie Signed-off-by: Sameer Vaze <svaze@qti.qualcomm.com> Signed-off-by: 0-day Robot <robot@bytheb.org>
Adds docs for comp perf dictionary option Signed-off-by: Sameer Vaze <svaze@qti.qualcomm.com> Signed-off-by: 0-day Robot <robot@bytheb.org>
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Sorry @ovsrobot, your pull request is larger than the review limit of 150000 diff characters
WalkthroughAdds dictionary support to compress-perf and zlib paths, new checksum/type APIs, and dictionary handling in tests. Broad VFIO refactor switches to uapi headers, removes VFIO_PRESENT guards, and updates EAL/PCI/driver code accordingly. Meson build gains staged header processing and stricter “found+linkable” dependency checks. Multiple Intel NIC paths migrate to raw mbuf bulk APIs and centralized RX/TX path selection. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant User
participant App as comp-perf app
participant Parser as Options Parser
participant Main as main()
participant Verify as Test Verify
participant PMD as Compress/Decompress PMD
User->>App: run with --dictionary <file>
App->>Parser: parse args
Parser-->>App: dictionary_file set
App->>Main: initialize test data
Main->>Main: load dictionary file into memory
Main-->>App: dictionary_data + size
App->>Verify: enqueue ops (DEFLATE/Inflate)
Verify->>PMD: xform with dictionary + len
PMD-->>Verify: process op
Verify-->>App: results (incl. checksum status)
sequenceDiagram
autonumber
participant Meson
participant Stage as stage-headers.py
participant Targets as staging custom_targets
participant Chkincs as chkincs executables
Meson->>Targets: cleanup_target (deps on header lists)
Targets->>Stage: --cleanup <dir> <stamp>
Stage-->>Targets: cleanup_staging.stamp
Meson->>Targets: arch/headers/generic staging
Targets->>Stage: <dir> <stamp> [headers...]
Stage-->>Targets: *.stamp
Meson->>Chkincs: build chkincs(-drv* , -cpp*)
Chkincs-->>Meson: compile with staged includes
sequenceDiagram
autonumber
participant App as EAL init/cleanup
participant VFIO as VFIO subsystem
App->>VFIO: rte_vfio_enable("vfio") [unconditional]
VFIO-->>App: init done or error
App-->>VFIO: vfio_mp_sync_setup (as needed)
App->>VFIO: vfio_mp_sync_cleanup() [unconditional at exit]
VFIO-->>App: cleanup done
Estimated code review effort🎯 5 (Critical) | ⏱️ ~150 minutes Poem
Pre-merge checks and finishing touches❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 18
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (6)
drivers/net/intel/iavf/rte_pmd_iavf.h (1)
188-251: Runtime crash risk:RTE_VERIFY(false)always fails.When
ALLOW_EXPERIMENTAL_APIis not defined, calling this function triggersRTE_VERIFY(false)at line 250, causing an assertion failure and application crash. Since the function isstatic inline, this crash occurs at every call site.If the intent is to prevent usage of experimental APIs:
- For compile-time enforcement, use
#error "ALLOW_EXPERIMENTAL_API must be defined"in the#elsebranch.- For link-time enforcement, remove
static inlineand provide only a declaration without definition.- For graceful degradation, keep the no-op but remove the assertion:
RTE_SET_USED(m); return;.The current approach causes runtime crashes instead of build failures, which is particularly problematic for a debugging/diagnostic function.
Apply this diff to prevent compilation instead of runtime crashes:
#ifdef ALLOW_EXPERIMENTAL_API union rte_pmd_ifd_proto_xtr_metadata data; if (!rte_pmd_ifd_dynf_proto_xtr_metadata_avail()) return; data.metadata = rte_pmd_ifd_dynf_proto_xtr_metadata_get(m); if (m->ol_flags & RTE_IAVF_PKT_RX_DYNF_PROTO_XTR_VLAN) printf(" - Flexible descriptor's Metadata: [0x%04x:0x%04x]," "vlan,stag=%u:%u:%u,ctag=%u:%u:%u", data.raw.data0, data.raw.data1, data.vlan.stag_pcp, data.vlan.stag_dei, data.vlan.stag_vid, data.vlan.ctag_pcp, data.vlan.ctag_dei, data.vlan.ctag_vid); else if (m->ol_flags & RTE_IAVF_PKT_RX_DYNF_PROTO_XTR_IPV4) printf(" - Flexible descriptor's Metadata: [0x%04x:0x%04x]," "ipv4,ver=%u,hdrlen=%u,tos=%u,ttl=%u,proto=%u", data.raw.data0, data.raw.data1, data.ipv4.version, data.ipv4.ihl, data.ipv4.tos, data.ipv4.ttl, data.ipv4.protocol); else if (m->ol_flags & RTE_IAVF_PKT_RX_DYNF_PROTO_XTR_IPV6) printf(" - Flexible descriptor's Metadata: [0x%04x:0x%04x]," "ipv6,ver=%u,tc=%u,flow_hi4=0x%x,nexthdr=%u,hoplimit=%u", data.raw.data0, data.raw.data1, data.ipv6.version, data.ipv6.tc, data.ipv6.flowhi4, data.ipv6.nexthdr, data.ipv6.hoplimit); else if (m->ol_flags & RTE_IAVF_PKT_RX_DYNF_PROTO_XTR_IPV6_FLOW) printf(" - Flexible descriptor's Metadata: [0x%04x:0x%04x]," "ipv6_flow,ver=%u,tc=%u,flow=0x%x%04x", data.raw.data0, data.raw.data1, data.ipv6_flow.version, data.ipv6_flow.tc, data.ipv6_flow.flowhi4, data.ipv6_flow.flowlo16); else if (m->ol_flags & RTE_IAVF_PKT_RX_DYNF_PROTO_XTR_TCP) printf(" - Flexible descriptor's Metadata: [0x%04x:0x%04x]," "tcp,doff=%u,flags=%s%s%s%s%s%s%s%s", data.raw.data0, data.raw.data1, data.tcp.doff, data.tcp.cwr ? "C" : "", data.tcp.ece ? "E" : "", data.tcp.urg ? "U" : "", data.tcp.ack ? "A" : "", data.tcp.psh ? "P" : "", data.tcp.rst ? "R" : "", data.tcp.syn ? "S" : "", data.tcp.fin ? "F" : ""); else if (m->ol_flags & RTE_IAVF_PKT_RX_DYNF_PROTO_XTR_IP_OFFSET) printf(" - Flexible descriptor's Extraction: ip_offset=%u", data.ip_ofs); #else - RTE_SET_USED(m); - RTE_VERIFY(false); +#error "ALLOW_EXPERIMENTAL_API must be defined to use rte_pmd_ifd_dump_proto_xtr_metadata" #endifAlternatively, for graceful degradation without crashing:
#else RTE_SET_USED(m); - RTE_VERIFY(false); #endifdrivers/raw/ifpga/afu_pmd_n3000.c (1)
1716-1728: Verify graceful degradation when VFIO IRQ setup fails.The removal of
VFIO_PRESENTguards makes IRQ setup unconditional. Ifdma_afu_set_irqsfails (line 1726), the error is logged but initialization continues withevent_fdset to the eventfd value.Potential issues:
- On systems without VFIO support, the ioctl in
dma_afu_set_irqs(line 1507) will fail- The
poll_interruptfunction (lines 682-719) usesevent_fdwithout checking if IRQ setup succeeded- DMA operations may hang or timeout if interrupts are required but not configured
Ensure:
- The driver can operate correctly in polling mode if IRQ setup fails
- Error handling in DMA transfer paths gracefully handles missing interrupts
- Consider propagating IRQ setup failure if interrupts are mandatory
Run the following script to verify interrupt usage and error paths:
#!/bin/bash # Check interrupt dependency and error handling # Find all calls to poll_interrupt echo "=== poll_interrupt call sites ===" ast-grep --pattern $'poll_interrupt($CTX)' # Check if there's fallback when IRQ setup fails echo "=== Checking error propagation from dma_afu_set_irqs ===" rg -n "dma_afu_set_irqs" -A5 -B5 --type=c # Check if event_fd is validated before use echo "=== event_fd validation checks ===" rg -n "event_fd.*<.*0|!.*event_fd" --type=c drivers/raw/ifpga/app/test-compress-perf/comp_perf_test_verify.c (1)
214-228: Keepsrc.lengthconsistent when skipping the PDCP headerWhen we set
src.offset = 1for PDCP UDC,src.lengthis still the full packet length, sooffset + lengthoverruns the buffer. Compressdev PMDs reject these ops as invalid. Recompute the length after changing the offset.- ops[op_id]->src.length = - rte_pktmbuf_pkt_len(input_bufs[buf_id]); + uint32_t src_len = + rte_pktmbuf_pkt_len(input_bufs[buf_id]); + ops[op_id]->src.length = src_len; ... if ((xform.type == RTE_COMP_DECOMPRESS) && (xform.decompress.chksum == RTE_COMP_CHECKSUM_3GPP_PDCP_UDC)) { uint8_t *udc_header = rte_pktmbuf_mtod(ops[op_id]->m_src, uint8_t *); ops[op_id]->input_chksum = *udc_header & 0xf; ops[op_id]->src.offset = 1; + if (src_len == 0) { + res = -1; + goto end; + } + ops[op_id]->src.length = src_len - 1; } else { ops[op_id]->input_chksum = buf_id; ops[op_id]->src.offset = 0; + ops[op_id]->src.length = src_len; }lib/eal/linux/eal_vfio.c (3)
1549-1566: Critical: wrong struct type passed to VFIO_IOMMU_UNMAP_DMAUnmap path uses vfio_iommu_type1_dma_map instead of vfio_iommu_type1_dma_unmap, corrupting iova/size offsets for the ioctl.
Apply this fix:
- struct vfio_iommu_type1_dma_map dma_unmap; + struct vfio_iommu_type1_dma_unmap dma_unmap; - memset(&dma_unmap, 0, sizeof(dma_unmap)); - dma_unmap.argsz = sizeof(struct vfio_iommu_type1_dma_unmap); - dma_unmap.size = len; - dma_unmap.iova = iova; + memset(&dma_unmap, 0, sizeof(dma_unmap)); + dma_unmap.argsz = sizeof(dma_unmap); + dma_unmap.flags = 0; + dma_unmap.iova = iova; + dma_unmap.size = len; ret = ioctl(vfio_container_fd, VFIO_IOMMU_UNMAP_DMA, &dma_unmap);
332-344: Off‑by‑one: first two entries never merged in compactionLoop skips i == 0, so [0] and [1] are never merged. Use a safe reverse traversal that compares (i‑1, i).
- for (i = RTE_DIM(user_mem_maps->maps) - 2; i != 0; i--) { - struct user_mem_map *l, *r; - - l = &user_mem_maps->maps[i]; - r = &user_mem_maps->maps[i + 1]; + for (i = RTE_DIM(user_mem_maps->maps) - 1; i > 0; i--) { + struct user_mem_map *l, *r; + + l = &user_mem_maps->maps[i - 1]; + r = &user_mem_maps->maps[i];
2011-2016: Capacity check rejects valid exactly‑full resultnewlen >= capacity should be a strict > check; exactly filling the table is valid and currently incorrectly returns ENOMEM.
- if (newlen >= RTE_DIM(user_mem_maps->maps)) { + if (newlen > RTE_DIM(user_mem_maps->maps)) { EAL_LOG(ERR, "Not enough space to store partial mapping"); rte_errno = ENOMEM; ret = -1; goto out; }
🧹 Nitpick comments (8)
drivers/baseband/acc/meson.build (1)
8-32: Consider refactoring to avoid redundantrequired: truein find_library calls.The current pattern uses
required: trueincc.find_library(), which will cause Meson to error immediately if a library is not found, making the subsequentcc.links()check and custom error messages unreachable in that scenario. While the linkability verification adds value for detecting broken installations, the pattern can be streamlined.Consider this alternative approach:
- stdcpp_dep = cc.find_library('stdc++', required: true) + stdcpp_dep = cc.find_library('stdc++', required: false) + if not stdcpp_dep.found() + error('library not found, "libstdc++"') + endif if not cc.links(min_c_code, dependencies: stdcpp_dep) error('broken dependency, "libstdc++"') endif ext_deps += stdcpp_depApply the same pattern to the other four libraries (irc, imf, ipps, svml). This allows your custom error messages to distinguish between "not found" and "found but not linkable" scenarios.
drivers/baseband/turbo_sw/meson.build (1)
9-33: Drop redundant linkage checks or adjustrequired:behavior
min_c_codeis defined at meson.build:60, socc.links()works as intended. However,find_library(..., required: true)already errors if the library is missing, making thecc.links()+error()calls in lines 9–33 redundant. Either setrequired: falseonfind_libraryand keepcc.links()for validation, or remove the extra linkage checks.drivers/ml/cnxk/meson.build (1)
75-79: <!-- [scratchpad]
[task review_comment]
[observations]
min_c_codedefined in rootmeson.build(line 60)- Numerous
cc.links(min_c_code, ...)checks across codebase
[analysis]min_c_codeis globally available and its use here is consistent
[action]- Drop the verification request for
min_c_code- Retain and rephrase the optional error-message enhancement
[done]Drop the
min_c_codeexistence check (it’s defined in the rootmeson.buildand used consistently elsewhere).Optional: enhance the error message for clarity:
- error('broken dependency, "libstdc++"') + error('libstdc++ was found but failed to link. Please verify your C++ toolchain installation.')drivers/net/intel/ice/ice_rxtx.c (1)
2046-2061: Make raw alloc/free pointer types explicit; guard struct layoutrte_mbuf_raw_alloc_bulk/free expect struct rte_mbuf**. Passing (void *)rxep works only if ci_rx_entry.mbuf is first and tightly packed. Use the mbuf field explicitly and add a compile-time guard.
Apply this diff:
- diag = rte_mbuf_raw_alloc_bulk(rxq->mp, (void *)rxep, + diag = rte_mbuf_raw_alloc_bulk(rxq->mp, &rxep[0].mbuf, rxq->rx_free_thresh); @@ - diag_pay = rte_mbuf_raw_alloc_bulk(rxq->rxseg[1].mp, - (void *)rxq->sw_split_buf, rxq->rx_free_thresh); + diag_pay = rte_mbuf_raw_alloc_bulk(rxq->rxseg[1].mp, + &rxq->sw_split_buf[0].mbuf, rxq->rx_free_thresh); @@ - rte_mbuf_raw_free_bulk(rxq->mp, (void *)rxep, + rte_mbuf_raw_free_bulk(rxq->mp, &rxep[0].mbuf, rxq->rx_free_thresh);Additionally (outside this hunk), consider:
RTE_BUILD_BUG_ON(offsetof(struct ci_rx_entry, mbuf) != 0);to enforce the layout assumption. Based on relevant code in lib/mbuf/rte_mbuf.h.
drivers/net/intel/idpf/idpf_common_rxtx_avx2.c (1)
23-25: Pass correct type to rte_mbuf_raw_alloc_bulkUse rxep (struct rte_mbuf **) directly; the (void *) cast is a leftover from rte_mempool_get_bulk.
- if (rte_mbuf_raw_alloc_bulk(rxq->mp, - (void *)rxep, - IDPF_RXQ_REARM_THRESH) < 0) { + if (rte_mbuf_raw_alloc_bulk(rxq->mp, + rxep, + IDPF_RXQ_REARM_THRESH) < 0) {Based on lib/mbuf/rte_mbuf.h signature.
buildtools/chkincs/staging/stage-headers.py (3)
1-13: Remove unusedosimport.The
osmodule is imported but not used in the script. Consider removing it to reduce clutter.Apply this diff:
import sys -import os import shutil from pathlib import Path
29-34: Ensure cleanup mode handles non-existent directory gracefully.If
shutil.rmtree(staging_dir)is called when the staging directory doesn't exist, it will raiseFileNotFoundError. Consider checking existence first or usingignore_errors=True.Apply this diff:
- shutil.rmtree(staging_dir) + shutil.rmtree(staging_dir, ignore_errors=True) staging_dir.mkdir(parents=True, exist_ok=True)Or check existence explicitly:
+ if staging_dir.exists(): + shutil.rmtree(staging_dir) staging_dir.mkdir(parents=True, exist_ok=True)
36-48: LGTM: Normal mode logic is sound.The script correctly stages headers by creating the directory (if needed) and copying files with metadata preservation. The
meson_stamp.touch()ensures Meson tracks completion.Optional: Consider wrapping the
shutil.copy2loop in a try-except to provide clearer error messages if a header file is missing, though failing loudly is also reasonable for a build script.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (107)
app/test-compress-perf/comp_perf_options.h(2 hunks)app/test-compress-perf/comp_perf_options_parse.c(5 hunks)app/test-compress-perf/comp_perf_test_verify.c(4 hunks)app/test-compress-perf/main.c(3 hunks)buildtools/chkincs/meson.build(4 hunks)buildtools/chkincs/staging/drivers/meson.build(1 hunks)buildtools/chkincs/staging/meson.build(1 hunks)buildtools/chkincs/staging/stage-headers.py(1 hunks)config/meson.build(5 hunks)devtools/checkpatches.sh(1 hunks)devtools/linux-uapi.sh(1 hunks)doc/api/doxy-api.conf.in(0 hunks)doc/guides/contributing/linux_uapi.rst(1 hunks)doc/guides/cryptodevs/bcmfs.rst(0 hunks)doc/guides/nics/features/iavf.ini(2 hunks)doc/guides/nics/features/ice.ini(1 hunks)doc/guides/tools/comp_perf.rst(1 hunks)drivers/baseband/acc/meson.build(2 hunks)drivers/baseband/turbo_sw/meson.build(1 hunks)drivers/bus/cdx/cdx_vfio.c(3 hunks)drivers/bus/fslmc/bus_fslmc_driver.h(1 hunks)drivers/bus/fslmc/fslmc_bus.c(2 hunks)drivers/bus/fslmc/fslmc_vfio.c(8 hunks)drivers/bus/fslmc/fslmc_vfio.h(0 hunks)drivers/bus/fslmc/portal/dpaa2_hw_dpio.c(1 hunks)drivers/bus/pci/linux/pci.c(0 hunks)drivers/bus/pci/linux/pci_init.h(0 hunks)drivers/bus/pci/linux/pci_vfio.c(6 hunks)drivers/bus/pci/pci_common.c(0 hunks)drivers/bus/platform/bus_platform_driver.h(1 hunks)drivers/bus/platform/platform.c(1 hunks)drivers/bus/platform/platform_params.c(0 hunks)drivers/bus/vmbus/rte_vmbus_reg.h(1 hunks)drivers/common/cnxk/roc_platform.c(1 hunks)drivers/common/mlx5/linux/meson.build(2 hunks)drivers/common/mlx5/windows/meson.build(1 hunks)drivers/common/qat/dev/qat_dev_gen_lce.c(0 hunks)drivers/compress/zlib/zlib_pmd.c(11 hunks)drivers/compress/zlib/zlib_pmd_private.h(2 hunks)drivers/crypto/bcmfs/bcmfs_vfio.c(1 hunks)drivers/crypto/bcmfs/meson.build(1 hunks)drivers/crypto/cnxk/rte_pmd_cnxk_crypto.h(1 hunks)drivers/meson.build(1 hunks)drivers/ml/cnxk/meson.build(1 hunks)drivers/net/af_xdp/meson.build(1 hunks)drivers/net/dpaa/rte_pmd_dpaa.h(1 hunks)drivers/net/hns3/hns3_ethdev_vf.c(0 hunks)drivers/net/intel/common/rx.h(2 hunks)drivers/net/intel/common/rx_vec_arm.h(1 hunks)drivers/net/intel/common/rx_vec_ppc.h(1 hunks)drivers/net/intel/common/rx_vec_x86.h(2 hunks)drivers/net/intel/common/tx.h(1 hunks)drivers/net/intel/cpfl/cpfl_rxtx.c(6 hunks)drivers/net/intel/cpfl/cpfl_rxtx_vec_common.h(2 hunks)drivers/net/intel/fm10k/fm10k_ethdev.c(1 hunks)drivers/net/intel/fm10k/fm10k_rxtx.c(3 hunks)drivers/net/intel/fm10k/fm10k_rxtx_vec.c(2 hunks)drivers/net/intel/i40e/i40e_rxtx.c(3 hunks)drivers/net/intel/iavf/iavf_rxtx.c(1 hunks)drivers/net/intel/iavf/rte_pmd_iavf.h(3 hunks)drivers/net/intel/ice/ice_rxtx.c(1 hunks)drivers/net/intel/idpf/idpf_common_device.h(2 hunks)drivers/net/intel/idpf/idpf_common_rxtx.c(2 hunks)drivers/net/intel/idpf/idpf_common_rxtx.h(3 hunks)drivers/net/intel/idpf/idpf_common_rxtx_avx2.c(1 hunks)drivers/net/intel/idpf/idpf_common_rxtx_avx512.c(2 hunks)drivers/net/intel/idpf/idpf_ethdev.c(1 hunks)drivers/net/intel/idpf/idpf_rxtx.c(5 hunks)drivers/net/intel/idpf/idpf_rxtx_vec_common.h(2 hunks)drivers/net/intel/ixgbe/ixgbe_rxtx.c(2 hunks)drivers/net/intel/ixgbe/ixgbe_rxtx_vec_common.h(1 hunks)drivers/net/mana/meson.build(1 hunks)drivers/net/mlx4/meson.build(1 hunks)drivers/net/mlx5/rte_pmd_mlx5.h(1 hunks)drivers/net/pcap/meson.build(1 hunks)drivers/net/txgbe/meson.build(1 hunks)drivers/net/xsc/xsc_vfio.c(1 hunks)drivers/power/amd_uncore/meson.build(1 hunks)drivers/raw/dpaa2_cmdif/rte_pmd_dpaa2_cmdif.h(1 hunks)drivers/raw/ifpga/afu_pmd_he_hssi.c(0 hunks)drivers/raw/ifpga/afu_pmd_he_lpbk.c(0 hunks)drivers/raw/ifpga/afu_pmd_he_mem.c(0 hunks)drivers/raw/ifpga/afu_pmd_n3000.c(1 hunks)drivers/raw/ifpga/base/ifpga_feature_dev.c(1 hunks)drivers/raw/ifpga/meson.build(1 hunks)drivers/raw/ntb/rte_pmd_ntb.h(1 hunks)drivers/vdpa/ifc/ifcvf_vdpa.c(1 hunks)drivers/vdpa/nfp/nfp_vdpa.c(1 hunks)drivers/vdpa/sfc/sfc_vdpa_ops.c(1 hunks)examples/l2fwd-cat/meson.build(1 hunks)examples/l2fwd-keepalive/meson.build(1 hunks)examples/vm_power_manager/guest_cli/meson.build(1 hunks)examples/vm_power_manager/meson.build(1 hunks)kernel/linux/uapi/linux/vduse.h(2 hunks)kernel/linux/uapi/linux/vfio.h(1 hunks)kernel/linux/uapi/version(1 hunks)lib/bbdev/meson.build(1 hunks)lib/compressdev/rte_comp.h(5 hunks)lib/eal/arm/include/meson.build(1 hunks)lib/eal/arm/include/rte_memcpy_32.h(2 hunks)lib/eal/include/meson.build(1 hunks)lib/eal/include/rte_vfio.h(1 hunks)lib/eal/linux/eal.c(0 hunks)lib/eal/linux/eal_interrupts.c(1 hunks)lib/eal/linux/eal_vfio.c(28 hunks)lib/eal/linux/eal_vfio.h(0 hunks)lib/eal/linux/eal_vfio_mp_sync.c(0 hunks)
⛔ Files not processed due to max files limit (23)
- lib/eal/loongarch/include/meson.build
- lib/eal/ppc/include/meson.build
- lib/eal/riscv/include/meson.build
- lib/eal/windows/meson.build
- lib/eal/x86/include/meson.build
- lib/ethdev/meson.build
- lib/eventdev/rte_event_dma_adapter.h
- lib/gpudev/gpudev.c
- lib/gpudev/gpudev_driver.h
- lib/meson.build
- lib/mldev/meson.build
- lib/power/meson.build
- lib/power/power_cpufreq.h
- lib/power/power_uncore_ops.h
- lib/power/rte_power_cpufreq.c
- lib/power/rte_power_cpufreq.h
- lib/power/rte_power_uncore.c
- lib/power/rte_power_uncore.h
- lib/rawdev/meson.build
- lib/regexdev/meson.build
- lib/security/meson.build
- lib/vhost/vduse.c
- meson.build
💤 Files with no reviewable changes (15)
- doc/guides/cryptodevs/bcmfs.rst
- drivers/bus/pci/linux/pci.c
- lib/eal/linux/eal_vfio_mp_sync.c
- doc/api/doxy-api.conf.in
- drivers/bus/fslmc/fslmc_vfio.h
- drivers/raw/ifpga/afu_pmd_he_hssi.c
- drivers/raw/ifpga/afu_pmd_he_lpbk.c
- drivers/bus/pci/pci_common.c
- drivers/common/qat/dev/qat_dev_gen_lce.c
- drivers/raw/ifpga/afu_pmd_he_mem.c
- drivers/net/hns3/hns3_ethdev_vf.c
- lib/eal/linux/eal_vfio.h
- drivers/bus/platform/platform_params.c
- drivers/bus/pci/linux/pci_init.h
- lib/eal/linux/eal.c
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-09-29T08:37:46.915Z
Learnt from: david-marchand
PR: ovsrobot/dpdk#106
File: buildtools/chkincs/stage-headers.py:23-29
Timestamp: 2025-09-29T08:37:46.915Z
Learning: The buildtools/chkincs/stage-headers.py script is called multiple times per build (once for each header group: arch, regular, generic, and drivers), so cleaning the staging directory inside the script itself would be problematic. Directory cleanup should be handled at a higher level in the meson build system.
Applied to files:
buildtools/chkincs/staging/stage-headers.pybuildtools/chkincs/staging/drivers/meson.buildbuildtools/chkincs/meson.buildbuildtools/chkincs/staging/meson.build
🧬 Code graph analysis (19)
drivers/net/intel/idpf/idpf_common_rxtx_avx2.c (1)
lib/mbuf/rte_mbuf.h (1)
rte_mbuf_raw_alloc_bulk(641-649)
drivers/net/intel/idpf/idpf_rxtx_vec_common.h (1)
drivers/net/intel/common/rx_vec_x86.h (1)
ci_get_x86_max_simd_bitwidth(355-370)
drivers/net/intel/fm10k/fm10k_rxtx.c (1)
lib/mbuf/rte_mbuf.h (2)
rte_mbuf_raw_alloc_bulk(641-649)rte_mbuf_raw_free_bulk(696-707)
drivers/net/intel/ixgbe/ixgbe_rxtx.c (1)
lib/mbuf/rte_mbuf.h (2)
rte_mbuf_raw_free_bulk(696-707)rte_mbuf_raw_alloc_bulk(641-649)
drivers/net/intel/i40e/i40e_rxtx.c (1)
lib/mbuf/rte_mbuf.h (2)
rte_mbuf_raw_alloc_bulk(641-649)rte_mbuf_raw_free_bulk(696-707)
drivers/net/intel/common/rx_vec_x86.h (1)
lib/mbuf/rte_mbuf.h (1)
rte_mbuf_raw_alloc_bulk(641-649)
drivers/net/intel/fm10k/fm10k_ethdev.c (2)
lib/mbuf/rte_mbuf.h (2)
rte_mbuf_raw_alloc_bulk(641-649)rte_mbuf_raw_free_bulk(696-707)drivers/net/intel/fm10k/fm10k.h (2)
fm10k_pktmbuf_reset(259-271)fm10k_addr_alignment_valid(293-315)
drivers/net/intel/ixgbe/ixgbe_rxtx_vec_common.h (1)
lib/mbuf/rte_mbuf.h (1)
rte_mbuf_raw_free_bulk(696-707)
drivers/net/intel/iavf/iavf_rxtx.c (1)
lib/mbuf/rte_mbuf.h (1)
rte_mbuf_raw_alloc_bulk(641-649)
drivers/net/intel/idpf/idpf_common_rxtx.c (2)
drivers/net/intel/idpf/idpf_common_rxtx_avx2.c (2)
RTE_EXPORT_INTERNAL_SYMBOL(478-483)RTE_EXPORT_INTERNAL_SYMBOL(621-642)drivers/net/intel/idpf/idpf_common_rxtx_avx512.c (4)
RTE_EXPORT_INTERNAL_SYMBOL(535-541)RTE_EXPORT_INTERNAL_SYMBOL(993-1000)RTE_EXPORT_INTERNAL_SYMBOL(1162-1168)RTE_EXPORT_INTERNAL_SYMBOL(1364-1370)
drivers/net/intel/idpf/idpf_rxtx.c (4)
drivers/net/intel/idpf/idpf_rxtx_vec_common.h (2)
idpf_rx_vec_dev_check_default(92-116)idpf_get_max_simd_bitwidth(136-140)drivers/net/intel/common/rx.h (1)
ci_rx_path_select(262-326)drivers/net/intel/idpf/idpf_common_rxtx.c (2)
idpf_qc_singleq_rx_vec_setup(1613-1618)idpf_qc_splitq_rx_vec_setup(1621-1626)drivers/net/intel/idpf/idpf_common_rxtx_avx512.c (1)
idpf_qc_tx_vec_avx512_setup(1373-1381)
drivers/net/intel/common/rx_vec_arm.h (1)
lib/mbuf/rte_mbuf.h (1)
rte_mbuf_raw_alloc_bulk(641-649)
drivers/net/intel/cpfl/cpfl_rxtx_vec_common.h (1)
drivers/net/intel/common/rx_vec_x86.h (1)
ci_get_x86_max_simd_bitwidth(355-370)
drivers/net/intel/common/rx_vec_ppc.h (1)
lib/mbuf/rte_mbuf.h (1)
rte_mbuf_raw_alloc_bulk(641-649)
drivers/net/intel/idpf/idpf_common_rxtx_avx512.c (1)
lib/mbuf/rte_mbuf.h (1)
rte_mbuf_raw_alloc_bulk(641-649)
drivers/net/intel/ice/ice_rxtx.c (1)
lib/mbuf/rte_mbuf.h (2)
rte_mbuf_raw_alloc_bulk(641-649)rte_mbuf_raw_free_bulk(696-707)
drivers/net/intel/cpfl/cpfl_rxtx.c (4)
drivers/net/intel/cpfl/cpfl_rxtx_vec_common.h (2)
cpfl_rx_vec_dev_check_default(80-107)cpfl_get_max_simd_bitwidth(129-133)drivers/net/intel/common/rx.h (1)
ci_rx_path_select(262-326)drivers/net/intel/idpf/idpf_common_rxtx.c (2)
idpf_qc_singleq_rx_vec_setup(1613-1618)idpf_qc_splitq_rx_vec_setup(1621-1626)drivers/net/intel/idpf/idpf_common_rxtx_avx512.c (1)
idpf_qc_tx_vec_avx512_setup(1373-1381)
drivers/net/intel/fm10k/fm10k_rxtx_vec.c (1)
lib/mbuf/rte_mbuf.h (2)
rte_mbuf_raw_alloc_bulk(641-649)rte_mbuf_raw_free_bulk(696-707)
drivers/net/intel/common/tx.h (1)
lib/mbuf/rte_mbuf.h (1)
rte_mbuf_raw_free_bulk(696-707)
🪛 Shellcheck (0.11.0)
devtools/checkpatches.sh
[warning] 95-95: Quote this to prevent word splitting.
(SC2046)
[warning] 95-95: Quote this to prevent word splitting.
(SC2046)
🔇 Additional comments (68)
drivers/net/dpaa/rte_pmd_dpaa.h (1)
8-8: Include makes the header self-contained.Adding
<stdint.h>ensures the fixed-width types used below are defined without relying on transitive includes. Nicely done.kernel/linux/uapi/version (1)
1-1: Version string update looks good.Change clean and ready to land.
drivers/net/mlx5/rte_pmd_mlx5.h (1)
8-10: Includes look good.Explicitly pulling in the fixed-width types and byte-order helpers makes the header self-sufficient.
drivers/raw/ntb/rte_pmd_ntb.h (1)
8-9: LGTM! Improves header self-sufficiency.Adding the explicit
<stdint.h>include ensures that fixed-width integer types (uint8_t, uint16_t, uint32_t, uint64_t) used in the structure definitions are properly declared when this header is included standalone, eliminating reliance on transitive includes.drivers/raw/dpaa2_cmdif/rte_pmd_dpaa2_cmdif.h (1)
15-16: LGTM! Header made self-contained.Adding the explicit
#include <stdint.h>is correct since the struct usesuint32_tanduint64_ttypes. This follows best practices by making the header self-contained rather than relying on transitive includes.drivers/net/intel/ixgbe/ixgbe_rxtx_vec_common.h (1)
58-58: LGTM! Appropriate migration to mbuf-specific bulk-free API.The switch from
rte_mempool_put_bulktorte_mbuf_raw_free_bulkis correct and improves type safety. The new API adds debug assertions while maintaining identical performance in release builds.Also applies to: 64-64
drivers/bus/vmbus/rte_vmbus_reg.h (1)
9-13: LGTM! Explicit header inclusions improve robustness.The added headers make implicit dependencies explicit:
stdint.hfor standard integer types used throughoutrte_common.hfor__rte_packed_begin/endmacrosrte_stdatomic.hforRTE_ATOMIC()macro (line 37)rte_uuid.hforrte_uuid_ttype (lines 328-329)This change prevents potential breakage if transitive includes change in the future and follows the standard practice of listing direct dependencies.
lib/bbdev/meson.build (3)
4-7: LGTM!The multi-line formatting with trailing commas improves readability and follows modern Meson best practices.
8-12: LGTM!The header reorganization correctly adds
rte_bbdev_trace_fp.hto the public headers and improves formatting. The removal ofrte_bbdev_pmd.hfrom this list is intentional, as it's now categorized as a driver SDK header (see line 13).
13-13: LGTM!The introduction of
driver_sdk_headersproperly separates driver-facing APIs from public application-facing APIs. Movingrte_bbdev_pmd.h(Poll Mode Driver header) into this category is the correct architectural decision.drivers/common/mlx5/windows/meson.build (1)
11-11: Stricter dependency validation improves build robustness. The addedcc.links(min_c_code, dependencies: devxlib)check catches link-time failures andmin_c_codeis defined at meson.build:60.drivers/net/mlx4/meson.build (1)
34-34: LGTM! Stricter dependency validation.The addition of the linkability check ensures that libraries are not only found but also successfully linkable before being added as dependencies. This prevents downstream build failures and aligns with the stricter dependency validation pattern applied across the codebase.
drivers/common/mlx5/linux/meson.build (2)
26-26: LGTM! Consistent linkability check.The addition of the linkability check for mlx5 and ibverbs libraries mirrors the pattern in mlx4 and ensures that only properly linkable dependencies are included in the build.
50-50: LGTM! Linkability check for optional dependency.The linkability check for the optional libmtcr_ul dependency ensures that it is only included when both present and properly linkable, which is especially important for optional dependencies to avoid unexpected build issues.
drivers/net/af_xdp/meson.build (1)
23-25: Approve robust link verification in drivers/net/af_xdp/meson.build
The link-time check ensuresbpf_depis both found and linkable, disabling non-linkable cases early to prevent downstream failures.drivers/baseband/acc/meson.build (1)
49-49: LGTM!The addition of the new public header
rte_acc_common_cfg.his properly integrated into the headers list.drivers/common/cnxk/roc_platform.c (1)
5-6: LGTM! Consistent uapi/linux/vfio.h usage across all drivers; no remaining<linux/vfio.h>includes.drivers/bus/fslmc/bus_fslmc_driver.h (1)
39-40: LGTM – forward declaration is sufficient here
The typedef in bus_fslmc_driver.h only usesstruct vfio_device_info *, and all full‐definition usages remain in .c files that include the proper header.drivers/raw/ifpga/meson.build (1)
14-14: LGTM — Enhanced dependency validation. The new linkage check ensureslibrtis both found and linkable. Please confirm thatmin_c_codeis defined in your Meson build configuration.drivers/raw/ifpga/base/ifpga_feature_dev.c (1)
5-6: Verify missing VFIO definitions in kernel header.The previous check found some but not all symbols. Confirm that
struct vfio_irq_setandVFIO_PCI_MSIX_IRQ_INDEXare defined inkernel/linux/uapi/linux/vfio.h:#!/bin/bash grep -nE "struct vfio_irq_set|VFIO_PCI_MSIX_IRQ_INDEX" kernel/linux/uapi/linux/vfio.h \ && echo "Found missing symbols in kernel header" \ || echo "Error: struct vfio_irq_set or VFIO_PCI_MSIX_IRQ_INDEX not defined"drivers/net/intel/common/rx.h (1)
134-134: LGTM! Field addition follows existing pattern.The
single_queueboolean field is consistent with other feature flags in the struct and clearly named.drivers/bus/cdx/cdx_vfio.c (3)
14-15: LGTM!The addition of the kernel uapi VFIO header is appropriate for the migration from RTE-specific VFIO constants to standard kernel definitions.
689-701: LGTM!The bus master disable function mirrors the enable function's two-stage approach (probe then set), with the appropriate use of
VFIO_DEVICE_FEATURE_CLEAR_MASTERinstead ofSET_MASTER. The migration to kernel uapi constants is consistent.
644-656: Approve constant migration Verified thatVFIO_DEVICE_FEATURE,VFIO_DEVICE_FEATURE_SET,VFIO_DEVICE_FEATURE_PROBE,VFIO_DEVICE_FEATURE_BUS_MASTER, andVFIO_DEVICE_FEATURE_SET_MASTERare defined inuapi/linux/vfio.hand noRTE_VFIO_DEVICE_FEATUREremains.drivers/net/intel/common/tx.h (1)
205-212: Bulk release path looks solid.Switching to
rte_mbuf_raw_free_bulkpreserves the pool-grouping logic while adding the raw helper’s additional checks, so batched frees remain safe. Nice cleanup.drivers/net/intel/common/rx_vec_ppc.h (3)
26-26: LGTM! Allocation API migration is correct.The migration to
rte_mbuf_raw_alloc_bulk(rxq->mp, rxp, rearm_thresh)is correctly implemented. The function signature matches (mempool pointer, mbuf pointer array, count), and the error handling pattern (checking for< 0) is preserved.
19-19: Approve pointer type change in rx_vec_ppc.h
Matches x86/ARM implementations (struct rte_mbuf **rxp = &rxq->sw_ring[...].mbuf); no further action needed.
30-31: Fake mbuf assignment and DESCS_PER_LOOP chunking are intentional across vector RX backends.
The allocation-failure path’sfor (i = 0; i < CI_VPMD_DESCS_PER_LOOP; i++) rxp[i] = &rxq->fake_mbuf;matches x86 and ARM implementations.drivers/net/intel/common/rx_vec_arm.h (5)
19-19: LGTM: Buffer pointer type updated to mbuf-based representation.The change from
struct ci_rx_entry *tostruct rte_mbuf **correctly reflects the transition to a mbuf-based rearm structure, aligning with similar changes in other architecture-specific headers.
26-26: LGTM: Bulk allocation updated to use raw mbuf API.The call to
rte_mbuf_raw_alloc_bulkcorrectly matches the new mbuf-based pointer type, and the argument types align with the API signature shown in the relevant code snippets.
31-31: LGTM: Fallback initialization correctly updated.The pointer assignment
rxp[i] = &rxq->fake_mbufproperly matches the new mbuf-based representation whererxp[i]is of typestruct rte_mbuf *.
80-103: LGTM: Rearm wrapper function logic preserved.The overall control flow and rearm logic remain correct, assuming the pointer arithmetic issue flagged in
_ci_rxq_rearm_get_bufsis resolved.
46-57: No pointer-arithmetic issue —ci_rx_entryis a single-pointer struct.
ci_rx_entryis defined as onlystruct rte_mbuf *mbuf(drivers/net/intel/common/rx.h), so using&sw_ring[..].mbufasstruct rte_mbuf **and&sw_ring[..]asstruct ci_rx_entry *is safe; no stride mismatch.drivers/net/intel/iavf/iavf_rxtx.c (1)
2245-2245: LGTM: Clean migration to raw mbuf bulk API.The replacement of
rte_mempool_get_bulkwithrte_mbuf_raw_alloc_bulkis correct. The new API is a thin wrapper that adds sanity checks while preserving identical semantics (same parameters, same return values, same error handling). This aligns with the broader ecosystem-wide migration to standardized raw mbuf bulk APIs across Intel drivers.drivers/net/intel/cpfl/cpfl_rxtx_vec_common.h (2)
14-17: LGTM: Proper conditional include for x86 vector support.The addition of
rx_vec_x86.hunder theRTE_ARCH_X86guard is correct and necessary to provide access to x86-specific SIMD capability detection functions. The include path is appropriate and aligns with the driver's architecture-aware vector path support.
128-134: LGTM: Clean wrapper for SIMD capability detection.The new
cpfl_get_max_simd_bitwidth()function provides a driver-specific wrapper over the common x86 SIMD detection utility. The architecture guard is appropriate, and the wrapper pattern enables centralized SIMD width detection for RX/TX path selection logic. This aligns with the broader vector-path enhancements across the driver ecosystem.drivers/net/intel/idpf/idpf_ethdev.c (1)
697-698: Sane default RX path initializationSetting rx_func_type to IDPF_RX_DEFAULT on configure makes the runtime selection predictable. LGTM.
doc/guides/nics/features/ice.ini (1)
17-17: LGTM! Feature flag addition is correct.The addition of the "Free Tx mbuf on demand" feature flag properly documents this capability for the ice driver.
drivers/net/xsc/xsc_vfio.c (1)
5-6: LGTM! Header migration to uapi is correct.The change from
<linux/vfio.h>to<uapi/linux/vfio.h>correctly migrates to the user-space API header, consistent with the broader VFIO refactor across the codebase.drivers/bus/fslmc/portal/dpaa2_hw_dpio.c (1)
7-8: LGTM! VFIO uapi header addition is appropriate.The addition of
<uapi/linux/vfio.h>aligns with the broader VFIO refactor to use user-space API headers consistently across the codebase.doc/guides/nics/features/iavf.ini (1)
12-12: LGTM! Feature flag additions are correct.The addition of three feature flags (Free Tx mbuf on demand, Burst mode info, Extended stats) properly documents these capabilities for the iavf driver. The placement within the Features section follows logical ordering.
Also applies to: 16-16, 43-43
examples/l2fwd-keepalive/meson.build (1)
11-11: LGTM! Enhanced build check improves robustness.The addition of
cc.links(min_c_code, dependencies: librt)ensures the library is not just present but actually linkable, preventing build failures downstream. This aligns with the broader pattern of stricter dependency validation across the build system.drivers/crypto/cnxk/rte_pmd_cnxk_crypto.h (1)
14-14: LGTM! Explicit includes improve header self-sufficiency.The additions of
<stdbool.h>and<rte_compat.h>make dependencies explicit:
stdbool.his required for thebooltype in function parameters (line 134)rte_compat.hprovides the__rte_experimentalmacro used throughoutThis improves header self-sufficiency by not relying on transitive includes.
Also applies to: 17-17
drivers/net/mana/meson.build (1)
30-30: LGTM! Link check strengthens dependency validation.The addition of
and cc.links(min_c_code, dependencies: lib)ensures each required library (ibverbs, mana) is not just found but actually linkable. This prevents build failures and is consistent with the broader pattern of stricter dependency validation in the build system.kernel/linux/uapi/linux/vduse.h (1)
2-3: Confirm include-guard naming consistency in uapi/linux headers.Automated checks found no other
_UAPI_…_H_guards or conflicting_VDUSE_H_macro. Manually verify that every header inkernel/linux/uapi/linux/follows the same guard-naming convention and that no name collisions occur.lib/eal/arm/include/meson.build (1)
4-30: LGTM: Header accumulation change aligns with centralized staging.The switch from
arch_headers = files(...)todpdk_arch_headers += files(...)correctly accumulates arch headers into the project-wide collection, supporting the new staged header workflow.examples/l2fwd-cat/meson.build (1)
10-10: LGTM: Linkage check improves dependency validation.Requiring both
pqos.found()and successful linking prevents false positives when the library is present but not usable.drivers/net/pcap/meson.build (1)
16-20: LGTM: Robust dependency validation for iphlpapi.The two-step approach (find library, verify linkability, then add to ext_deps) ensures the dependency is usable before proceeding.
drivers/net/txgbe/meson.build (1)
34-34: LGTM: Header exposure via centralized file list.Switching from
install_headers()toheaders = files(...)aligns with the project-wide shift to centralized header collections.drivers/power/amd_uncore/meson.build (1)
12-12: LGTM: Linkage check improves dependency validation.Requiring successful linking in addition to library presence ensures the e_smi64 dependency is usable before proceeding.
devtools/linux-uapi.sh (1)
84-86: LGTM: _UAPI guard normalization is correct.The three sed commands correctly strip
_UAPIfrom preprocessor guards in imported uAPI headers, normalizing them for DPDK use.devtools/checkpatches.sh (1)
90-96: LGTM: Forbidden-token check enforces centralized header policy.The new check correctly rejects direct
install_headers()calls in lib and drivers, enforcing the shift to centralized header collections.Note: The Shellcheck warning on line 95 about unquoted
$(dirname ...)is a false positive—the path is correctly constructed and quoted in context.config/meson.build (7)
211-216: LGTM! Enhanced dependency verification.The addition of
cc.links(min_c_code, dependencies: math_dep)ensures the math library is not only found but also actually linkable, preventing false positives in the dependency detection.
239-239: LGTM! Comprehensive NUMA dependency validation.The three-part check (library found + header present + linkable) provides robust verification of NUMA support availability.
249-249: LGTM! Thorough FDT dependency verification.Consistent with the NUMA check pattern, ensuring both header availability and successful linking.
257-257: LGTM! Linkability check for execinfo.Ensures the execinfo library can be successfully linked before adding it to the build.
292-293: LGTM! Complete pcap dependency validation.The multi-condition check ensures libpcap is fully functional before enabling support.
301-303: LGTM! Proper error handling for required libatomic.For 32-bit clang builds, libatomic is mandatory. Raising an error when linking fails is the correct behavior, ensuring build failures are caught early rather than manifesting as runtime issues.
464-464: dpdk_arch_headers properly initialized and consumed
dpdk_arch_headers is initialized and passed to install_headers, so adding rte_config.h here ensures it will be installed as intended.doc/guides/tools/comp_perf.rst (1)
101-102: LGTM! Clear documentation for dictionary option.The documentation clearly describes the new
--dictionaryoption for specifying a pre-defined dictionary file, consistent with the other application options.doc/guides/contributing/linux_uapi.rst (1)
65-76: LGTM! Important clarification on header inclusion order.The documentation now explicitly states that uAPI headers must be included before external headers, with a clear example demonstrating the correct order. This prevents potential conflicts with system headers and aligns with the VFIO refactoring changes throughout the codebase.
drivers/bus/fslmc/fslmc_bus.c (2)
20-20: LGTM! Required include for VFIO API.The addition of
<rte_vfio.h>provides access torte_vfio_noiommu_is_enabled()used in the IOMMU class selection logic below.
585-585: LGTM! Simplified IOVA mode selection.The combined condition elegantly determines IOVA mode:
- Returns
RTE_IOVA_VAwhen all devices support IOVA AND VFIO noiommu is disabled- Falls back to
RTE_IOVA_PAotherwiseThis simplification eliminates intermediate boolean variables while preserving the original logic.
drivers/bus/platform/platform.c (1)
5-6: Linux-only compilation verified Themeson.buildguard (if not is_linux build = false) ensuresplatform.c(and its<uapi/linux/vfio.h>include) is only built on Linux.lib/eal/include/meson.build (1)
67-80: Confirminstall_headersusesdpdk_generic_headers
dpdk_generic_headersis initialized atmeson.build:42and correctly augmented.- Ensure all
install_headers(...)calls referencedpdk_generic_headersinstead ofgeneric_headers, for example:rg -n "install_headers" --glob "*.build" rg -n "install_headers.*generic_headers" --glob "*.build" rg -n "install_headers.*dpdk_generic_headers" --glob "*.build"drivers/vdpa/sfc/sfc_vdpa_ops.c (1)
5-6: Verify kernel VFIO uAPI definitions. This file uses onlystruct vfio_irq_setandvfio_region_infofrom<uapi/linux/vfio.h>, with no<rte_vfio.h>orrte_vfio_*calls remaining. Confirm that<uapi/linux/vfio.h>exports all required VFIO symbols.lib/eal/include/rte_vfio.h (1)
22-31: Approve VFIO macro renames
No remaining references to oldVFIO_*macros orVFIO_PRESENTguard found – refactor is complete.lib/eal/linux/eal_vfio.c (1)
5-5: Switch to <uapi/linux/vfio.h>: verify header precedenceEnsure the build include paths prefer the vendored kernel/linux/uapi tree over system headers to avoid ABI drift/mismatches across distros.
| if (test_data->dictionary_data) | ||
| rte_free(test_data->dictionary_data); | ||
|
|
There was a problem hiding this comment.
Prevent double-free of dictionary buffer
Line 415 frees dictionary_data even on the success path, so the pointer is deallocated before being consumed and then freed again during cleanup (case ST_DICTIONARY_DATA), triggering a double free. Please free this buffer only on the error path and clear the fields when doing so.
- if (test_data->dictionary_data)
- rte_free(test_data->dictionary_data);
+ if (ret != 0) {
+ rte_free(test_data->dictionary_data);
+ test_data->dictionary_data = NULL;
+ test_data->dictionary_data_sz = 0;
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if (test_data->dictionary_data) | |
| rte_free(test_data->dictionary_data); | |
| if (ret != 0) { | |
| rte_free(test_data->dictionary_data); | |
| test_data->dictionary_data = NULL; | |
| test_data->dictionary_data_sz = 0; | |
| } |
🤖 Prompt for AI Agents
In app/test-compress-perf/main.c around lines 415 to 417, the code frees
test_data->dictionary_data unconditionally which causes a double-free when the
success path later hands that buffer to the ST_DICTIONARY_DATA case; change the
logic so the dictionary buffer is freed only on error paths (not on the
success/consume path), and when you free it set test_data->dictionary_data to
NULL and clear any related length/flags fields to avoid later cleanup attempting
to free it again; remove the unconditional free on the success path and add the
guarded free+clear only where error handling occurs.
| deps_drivers += declare_dependency(sources: | ||
| custom_target('drivers_headers_staging', | ||
| output: 'drivers_headers_staging.stamp', | ||
| depends: cleanup_target, | ||
| command: [stage_headers_cmd, meson.current_build_dir(), '@OUTPUT@', | ||
| dpdk_drivers_headers], | ||
| install: false, |
There was a problem hiding this comment.
Fix the staged headers command invocation
stage_headers_cmd is already a list; wrapping it in another list hands Meson a nested array and the target fails to configure (“Invalid type list for argument command”). Flatten the command by concatenating the arguments instead.
- custom_target('drivers_headers_staging',
- output: 'drivers_headers_staging.stamp',
- depends: cleanup_target,
- command: [stage_headers_cmd, meson.current_build_dir(), '@OUTPUT@',
- dpdk_drivers_headers],
+ custom_target('drivers_headers_staging',
+ output: 'drivers_headers_staging.stamp',
+ depends: cleanup_target,
+ command: stage_headers_cmd + [meson.current_build_dir(), '@OUTPUT@',
+ dpdk_drivers_headers],
install: false,
)🤖 Prompt for AI Agents
In buildtools/chkincs/staging/drivers/meson.build around lines 5 to 11, the
custom_target command uses stage_headers_cmd wrapped as an element inside the
command list which creates a nested list and causes Meson to error; replace that
nested-list invocation by flattening/concatenating stage_headers_cmd with the
rest of the command arguments (meson.current_build_dir(), '@OUTPUT@',
dpdk_drivers_headers, etc.) so command is a single flat list of strings rather
than a list containing a list.
| cleanup_target = custom_target('cleanup_staging', | ||
| output: 'cleanup_staging.stamp', | ||
| depend_files: dpdk_arch_headers + dpdk_arch_indirect_headers + dpdk_generic_headers + | ||
| dpdk_headers + dpdk_indirect_headers + dpdk_drivers_headers, | ||
| command: [stage_headers_cmd, '--cleanup', meson.current_build_dir(), '@OUTPUT@'], | ||
| install: false, | ||
| ) |
There was a problem hiding this comment.
Fix Meson variable order: define dpdk_drivers_headers before use
cleanup_target uses dpdk_drivers_headers before subdir('drivers') defines it. Move subdir('drivers') above cleanup_target to avoid an undefined variable at configure time.
Based on learnings
stage_headers_cmd = py3 + files('stage-headers.py')
+subdir('drivers')
+
-cleanup_target = custom_target('cleanup_staging',
+cleanup_target = custom_target('cleanup_staging',
output: 'cleanup_staging.stamp',
depend_files: dpdk_arch_headers + dpdk_arch_indirect_headers + dpdk_generic_headers +
dpdk_headers + dpdk_indirect_headers + dpdk_drivers_headers,
command: [stage_headers_cmd, '--cleanup', meson.current_build_dir(), '@OUTPUT@'],
install: false,
)
-
-subdir('drivers')Also applies to: 14-14
🤖 Prompt for AI Agents
In buildtools/chkincs/staging/meson.build around lines 6 to 12 (and also line
14), dpdk_drivers_headers is referenced before it is defined by
subdir('drivers'); move the subdir('drivers') invocation so it appears before
the cleanup_target definition (and before any other targets that reference
dpdk_drivers_headers) to ensure the variable is created at configure time;
update the file ordering accordingly so cleanup_target and the other affected
lines reference the already-defined dpdk_drivers_headers.
| #include <uapi/linux/vfio.h> | ||
|
|
There was a problem hiding this comment.
Restore portable VFIO header include
Including <uapi/linux/vfio.h> directly breaks builds on mainstream distributions because /usr/include/uapi is not in the default search path. We need to keep the <linux/vfio.h> include (or add a guarded fallback) so compilation succeeds everywhere.
-#include <uapi/linux/vfio.h>
+#if defined(__has_include)
+#if __has_include(<uapi/linux/vfio.h>)
+#include <uapi/linux/vfio.h>
+#else
+#include <linux/vfio.h>
+#endif
+#else
+#include <linux/vfio.h>
+#endif📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| #include <uapi/linux/vfio.h> | |
| #if defined(__has_include) | |
| # if __has_include(<uapi/linux/vfio.h>) | |
| # include <uapi/linux/vfio.h> | |
| # else | |
| # include <linux/vfio.h> | |
| # endif | |
| #else | |
| # include <linux/vfio.h> | |
| #endif |
🤖 Prompt for AI Agents
In drivers/bus/pci/linux/pci_vfio.c around lines 5-6, the code currently
includes the non-portable header <uapi/linux/vfio.h>; replace that include with
the portable standard header <linux/vfio.h> (or add a guarded fallback that
includes <linux/vfio.h> first and only uses <uapi/linux/vfio.h> if the former is
unavailable) so the file builds on mainstream distributions where
/usr/include/uapi is not in the default search path.
| if (deflateGetDictionary(strm, dictionary, &dictionary_len)) { | ||
| ZLIB_PMD_ERR("Unable to fetch dictionary"); | ||
| op->status = RTE_COMP_OP_STATUS_CHECKSUM_VALIDATION_FAILED; | ||
| free(dictionary); | ||
| return; | ||
| } | ||
|
|
||
| uint32_t dictionary_start = (uint32_t)(*dictionary); | ||
| uint32_t dictionary_end = (uint32_t)(*(dictionary + dictionary_len - 4)); | ||
| uint32_t sum = (dictionary_start & BOTTOM_NIBBLE_OF_BYTES_IN_DOUBLE_WORD) | ||
| + (dictionary_start & (TOP_NIBBLE_OF_BYTE_IN_DOUBLE_WORD >> 4)) | ||
| + (dictionary_end & BOTTOM_NIBBLE_OF_BYTES_IN_DOUBLE_WORD) | ||
| + (dictionary_end & (TOP_NIBBLE_OF_BYTE_IN_DOUBLE_WORD >> 4)); | ||
| uint8_t *sum_bytes = (uint8_t *)∑ | ||
|
|
||
| op->output_chksum = ~(sum_bytes[0] + sum_bytes[1] + sum_bytes[2] + sum_bytes[3]) | ||
| & BOTTOM_NIBBLE_OF_BYTE; | ||
| break; |
There was a problem hiding this comment.
Guard against short dictionaries before computing PDCP checksum.
dictionary_len is unsigned. When deflateGetDictionary() reports a length < 4 (perfectly valid for user-supplied dictionaries), dictionary_len - 4 underflows and we read far outside the buffer. Please reject too-short dictionaries (or handle them safely) before touching dictionary[dictionary_len - 4].
A minimal fix would be:
if (deflateGetDictionary(strm, dictionary, &dictionary_len)) {
ZLIB_PMD_ERR("Unable to fetch dictionary");
op->status = RTE_COMP_OP_STATUS_CHECKSUM_VALIDATION_FAILED;
free(dictionary);
return;
}
+ if (dictionary_len < 4) {
+ ZLIB_PMD_ERR("Dictionary too small for PDCP checksum");
+ op->status = RTE_COMP_OP_STATUS_CHECKSUM_VALIDATION_FAILED;
+ free(dictionary);
+ return;
+ }
+
uint32_t dictionary_start = (uint32_t)(*dictionary);
uint32_t dictionary_end = (uint32_t)(*(dictionary + dictionary_len - 4));🤖 Prompt for AI Agents
In drivers/compress/zlib/zlib_pmd.c around lines 41 to 58, the code reads
dictionary[dictionary_len - 4] without checking that dictionary_len >= 4, which
underflows because dictionary_len is unsigned; before computing PDCP checksum,
detect if dictionary_len < 4 and treat it as an error path (free(dictionary),
set op->status to an appropriate failure like
RTE_COMP_OP_STATUS_CHECKSUM_VALIDATION_FAILED or another defined error, log via
ZLIB_PMD_ERR, and return) or otherwise handle the short dictionary safely so no
out-of-bounds access occurs.
| #include <uapi/linux/vfio.h> | ||
|
|
There was a problem hiding this comment.
Build will fail: use <linux/vfio.h> instead.
Including <uapi/linux/vfio.h> requires the kernel source tree (uapi subdir) on the include path; standard userspace builds only ship /usr/include/linux/vfio.h. This change breaks compilation on usual DPDK build environments. Please switch to <linux/vfio.h> (or ensure the build adds the uapi path explicitly).
🤖 Prompt for AI Agents
In drivers/vdpa/ifc/ifcvf_vdpa.c around lines 5-6, the file currently includes
<uapi/linux/vfio.h> which breaks regular userspace builds because they expect
<linux/vfio.h>; replace the include with <linux/vfio.h> (or modify the build to
add the kernel uapi include path), i.e., change the include directive to use the
standard userspace header so typical DPDK build environments can compile.
| * All rights reserved. | ||
| */ | ||
|
|
||
| #include <uapi/linux/vfio.h> |
There was a problem hiding this comment.
Avoid <uapi/> include to keep the build working
Including <uapi/linux/vfio.h> breaks builds on standard user-space setups (e.g., linux-libc-dev) where only <linux/vfio.h> is exported. Unless the build system now injects raw kernel header paths, this include will not resolve. Stick with <linux/vfio.h> (or add the proper include path) to preserve portability.
-#include <uapi/linux/vfio.h>
+#include <linux/vfio.h>📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| #include <uapi/linux/vfio.h> | |
| #include <linux/vfio.h> |
🤖 Prompt for AI Agents
In drivers/vdpa/nfp/nfp_vdpa.c around line 6, the source includes
<uapi/linux/vfio.h> which breaks user-space builds because standard installs
expose <linux/vfio.h>; replace the include with <linux/vfio.h> (or, if you
intentionally need kernel raw-uapi headers, adjust the build to add the kernel
uapi include path) so the header resolves correctly on standard systems.
| build = (opt_dep.found() and cc.links(min_c_code, dependencies: opt_dep)) | ||
| ext_deps += opt_dep |
There was a problem hiding this comment.
🧩 Analysis chain
Same concern: verify min_c_code and ext_deps handling.
This change mirrors the one in examples/vm_power_manager/meson.build (line 41-42). The same concerns apply:
- Ensure
min_c_codeis defined in scope. - Adding
opt_deptoext_depsunconditionally (line 21) may be problematic if the linking check fails.
Consider the same conditional fix as suggested for the parent meson.build file, or verify that the build variable gates the use of this example elsewhere in the build system.
The verification script from the parent meson.build review will cover this file as well, but you can also check this specific file:
🏁 Script executed:
#!/bin/bash
# Check the guest_cli meson.build specifically
rg -n -A 5 "build\s*=.*opt_dep" examples/vm_power_manager/guest_cli/
# Look for how build variable affects the guest_cli example
rg -n "build" examples/vm_power_manager/guest_cli/Length of output: 1352
🏁 Script executed:
#!/bin/bash
# Dump the guest_cli meson.build to inspect definitions
sed -n '1,200p' examples/vm_power_manager/guest_cli/meson.buildLength of output: 559
🏁 Script executed:
#!/bin/bash
# Inspect the parent meson.build for definitions of min_c_code and ext_deps
sed -n '1,200p' examples/vm_power_manager/meson.buildLength of output: 1187
Define min_c_code and guard the optional dependency
The call to cc.links(min_c_code, dependencies: opt_dep) fails because min_c_code isn’t declared in this scope, and ext_deps += opt_dep must be wrapped in if opt_dep.found() to avoid adding an unresolved dependency.
🤖 Prompt for AI Agents
In examples/vm_power_manager/guest_cli/meson.build around lines 20 to 21, the
code references an undeclared identifier min_c_code and unconditionally appends
the optional dependency opt_dep; fix by declaring or passing the correct
variable for the minimum C code target (or replace min_c_code with the actual
target variable name in scope) before calling cc.links(...), and wrap ext_deps
+= opt_dep inside an if opt_dep.found() conditional so the optional dependency
is only added when present.
| build = (opt_dep.found() and cc.links(min_c_code, dependencies: opt_dep)) | ||
| ext_deps += opt_dep |
There was a problem hiding this comment.
🧩 Analysis chain
Verify min_c_code is defined and assess ext_deps handling.
The change tightens the build condition by requiring both discovery and successful linking of the 'virt' library. However, there are two concerns:
- Ensure
min_c_codeis defined in the Meson build system scope. - Line 42 adds
opt_deptoext_depsregardless of whether linking succeeds. If linking fails (build = false), adding an unlinkable dependency toext_depscould cause downstream build issues.
Consider conditionally adding to ext_deps only when the linking check passes:
opt_dep = cc.find_library('virt', required : false)
build = (opt_dep.found() and cc.links(min_c_code, dependencies: opt_dep))
-ext_deps += opt_dep
+if build
+ ext_deps += opt_dep
+endifAlternatively, verify that the build variable is checked elsewhere to skip building this example when false, making the unconditional ext_deps addition safe.
Run the following script to verify the definition of min_c_code and the usage pattern of the build variable:
🏁 Script executed:
#!/bin/bash
# Verify min_c_code is defined in the Meson build system
rg -n "min_c_code\s*="
# Check how the build variable is used in this context
rg -n -A 5 "build\s*=.*opt_dep" examples/vm_power_manager/
# Look for patterns where ext_deps is conditionally added
rg -n -B 2 -A 2 "ext_deps\s*\+=" examples/vm_power_manager/Length of output: 1682
🏁 Script executed:
#!/bin/bash
# Show all ext_deps usage in vm_power_manager example
rg -n -C3 "ext_deps" examples/vm_power_manager/meson.buildLength of output: 328
🏁 Script executed:
#!/bin/bash
# Locate build guard around executable usage
rg -n -C3 "if build" examples/vm_power_manager/meson.build
rg -n -C3 "executable" examples/vm_power_manager/meson.buildLength of output: 0
Conditionally add virt dependency only when linking succeeds
min_c_code is defined, but on line 42 ext_deps += opt_dep runs unconditionally. Wrap it in if build to match the jansson pattern and avoid adding an unlinkable dependency:
build = opt_dep.found() and cc.links(min_c_code, dependencies: opt_dep)
-ext_deps += opt_dep
+if build
+ ext_deps += opt_dep
+endif📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| build = (opt_dep.found() and cc.links(min_c_code, dependencies: opt_dep)) | |
| ext_deps += opt_dep | |
| build = (opt_dep.found() and cc.links(min_c_code, dependencies: opt_dep)) | |
| if build | |
| ext_deps += opt_dep | |
| endif |
🤖 Prompt for AI Agents
In examples/vm_power_manager/meson.build around lines 41 to 42, the optional
dependency `opt_dep` is always appended to `ext_deps` even when the link step
(`build = (opt_dep.found() and cc.links(min_c_code, dependencies: opt_dep))`)
fails; change the unconditional append to be conditional by wrapping `ext_deps
+= opt_dep` in `if build` so `opt_dep` is only added when `build` is true.
| * Copyright(c) 2010-2014 Intel Corporation | ||
| */ | ||
|
|
||
| #include <uapi/linux/vfio.h> |
There was a problem hiding this comment.
Build breaks: <uapi/linux/vfio.h> is not on the default include path.
Most user-space toolchains only ship /usr/include/linux/vfio.h; /usr/include/uapi/... is normally absent, so this include fails to compile out of the box. Keep using <linux/vfio.h> (or add the uapi directory to the include search path) before we can merge.
-#include <uapi/linux/vfio.h>
+#include <linux/vfio.h>📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| #include <uapi/linux/vfio.h> | |
| #include <linux/vfio.h> |
🤖 Prompt for AI Agents
In lib/eal/linux/eal_interrupts.c around line 5 the source includes
<uapi/linux/vfio.h>, which breaks builds because /usr/include/uapi is not on
default include paths; change the include to use the public header
<linux/vfio.h> (or alternatively add the kernel uapi include directory to the
compiler include paths) so the file compiles with standard user-space
toolchains.
NOTE: This is an auto submission for "[v3,1/4] compressdev: support for dictionaries and PDCP checksum".
See "http://patchwork.dpdk.org/project/dpdk/list/?series=36290" for details.
Summary by CodeRabbit
New Features
Improvements
Documentation