[PWCI] "[v2] dts: testpmd link check on port start" #340
Conversation
When launching proc-info: EAL: '-c <coremask>' option is deprecated, and will be removed in a future release EAL: Use '-l <corelist>' or '--lcores=<corelist>' option instead This commit fixes it by use -l0 other than -c1. Fixes: ce5a6fa ("eal: deprecate coremask-based EAL parameters") Signed-off-by: Chengwen Feng <fengchengwen@huawei.com> Acked-by: Bruce Richardson <bruce.richardson@intel.com>
Missing definition of errnos causes build failure:
In file included from ../lib/dmadev/rte_dmadev_trace.h:16,
from ../lib/dmadev/rte_dmadev_trace_points.c:8:
../lib/dmadev/rte_dmadev.h: In function ‘rte_dma_copy’:
../lib/dmadev/rte_dmadev.h:1183:25: error: ‘EINVAL’ undeclared
(first use in this function)
1183 | return -EINVAL;
| ^~~~~~
Bugzilla ID: 1814
Fixes: 91e581e ("dmadev: add data plane API")
Cc: stable@dpdk.org
Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
Acked-by: Chengwen Feng <fengchengwen@huawei.com>
The variable cdx_addr is a stack-allocated zero-initialized char array, but it is never used anywhere, except as part of error message out. The intention is to display device name, so fix the code to reflect that. Fixes: 45ef232 ("bus/cdx: introduce AMD CDX bus") Cc: stable@dpdk.org Signed-off-by: Anatoly Burakov <anatoly.burakov@intel.com> Acked-by: Nipun Gupta <nipun.gupta@amd.com>
The variable cdx_addr is a stack-allocated zero-initialized char array, but it is never used anywhere except in an error handler to stand it as a device bus address for a VFIO device release call. This is incorrect, as it should have been actual device address, so fix the code to reflect that. Fixes: 45ef232 ("bus/cdx: introduce AMD CDX bus") Cc: stable@dpdk.org Signed-off-by: Anatoly Burakov <anatoly.burakov@intel.com> Acked-by: Nipun Gupta <nipun.gupta@amd.com>
Fix the following warning seen on Fedora 43 with Python 3.14: buildtools/pmdinfogen.py:118: DeprecationWarning: Due to '_pack_', the 'rte_pci_id' Structure will use memory layout compatible with MSVC (Windows). If this is intended, set _layout_ to 'ms'. The implicit default is dep recated and slated to become an error in Python 3.19. Use the struct module which is simpler and assumes everything is packed by default. There is no change in the output of dpdk-pmdinfo.py before and after this patch. Bugzilla ID: 1818 Cc: stable@dpdk.org Link: https://docs.python.org/3/library/struct.html Signed-off-by: Robin Jarry <rjarry@redhat.com> Acked-by: Dmitry Kozlyuk <dmitry.kozliuk@gmail.com>
This was caught with recent clang.
The RTE_TRACE_POINT_ARGS() macro expects at least one parameter.
As a (void) function does not require any special tweaking with
__rte_unused, we can simply replace with a direct (void) token.
Build dir: /export/dpdk-next-net-mrvl/build-clang-static
Project version: 25.11.0-rc1
C compiler for the host machine: ccache clang (clang 21.1.4
"clang version 21.1.4")
In file included from ../lib/eal/common/eal_common_trace_points.c:7:
../lib/eal/include/rte_eal_trace.h:23:2: error: attribute 'unused'
cannot be applied to a 'void' parameter
[-Werror,-Wignored-attributes]
23 | RTE_TRACE_POINT_ARGS(void),
| ^~~~~~~~~~~~~~~~~~~~~~~~~~
../lib/eal/include/rte_trace_point_register.h:46:3: note: expanded
from macro 'RTE_TRACE_POINT_ARGS'
46 | (RTE_TRACE_POINT_ARGS_(RTE_TRACE_POINT_ARGS_COUNT(0,
__VA_ARGS__), __VA_ARGS__))
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
~~~~~~~~~~~~~~~~~~~~~~~~~~~
../lib/eal/include/rte_trace_point_register.h:44:30: note: expanded
from macro 'RTE_TRACE_POINT_ARGS_'
44 | RTE_TRACE_POINT_ARGS_EXPAND(RTE_TRACE_POINT_ARGS_FUNC
(N))(__VA_ARGS__)
| ~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~
~~~~~~~~~~~~~~~~~
../lib/eal/include/rte_trace_point_register.h:41:38: note: expanded
from macro 'RTE_TRACE_POINT_ARGS_FUNC'
41 | #define RTE_TRACE_POINT_ARGS_FUNC(a) RTE_TRACE_POINT_ARGS_ ## a
| ^
note: (skipping 1 expansions in backtrace; use -fmacro-backtrace-limit=0
to see all)
../lib/eal/include/rte_trace_point_register.h:42:42: note: expanded
from macro 'RTE_TRACE_POINT_ARGS_EXPAND'
42 | #define RTE_TRACE_POINT_ARGS_EXPAND(...) __VA_ARGS__
| ^
../lib/eal/include/rte_trace_point_register.h:26:35: note: expanded
from macro 'RTE_TRACE_POINT_ARGS_1'
26 | #define RTE_TRACE_POINT_ARGS_1(a) __rte_unused a
| ^~~~~~~~~~~~
../lib/eal/include/rte_common.h:248:37: note: expanded
from macro '__rte_unused'
248 | #define __rte_unused __attribute__((__unused__))
| ^~~~~~~~~~
1 error generated.
Fixes: 856aef5 ("trace: fix undefined behavior in register")
Cc: stable@dpdk.org
Reported-by: Jerin Jacob <jerinj@marvell.com>
Signed-off-by: David Marchand <david.marchand@redhat.com>
Acked-by: Jerin Jacob <jerinj@marvell.com>
Fix the following error seen with clang 21.1.4 drivers/net/intel/iavf/iavf_vchnl.c:123:38: error: variable 'notify_byte' is uninitialized when passed as a const pointer argument here [-Werror,-Wuninitialized-const-pointer] Fixes: cb5c1b9 ("net/iavf: add thread for event callbacks") Cc: stable@dpdk.org Signed-off-by: Jerin Jacob <jerinj@marvell.com> Acked-by: Vladimir Medvedkin <vladimir.medvedkin@intel.com> Reviewed-by: David Marchand <david.marchand@redhat.com>
Fix the following errors seen with clang 21.1.4 app/test/test_event_dma_adapter.c:532:32: error: variable 'event' is uninitialized when passed as a const pointer argument here [-Werror,-Wuninitialized-const-pointer] app/test/test_trace.c:201:29: error: variable 'tmp' is uninitialized when passed as a const pointer argument here [-Werror,-Wuninitialized-const-pointer] Fixes: 4925e15 ("trace: add trace point emit for blob") Fixes: a276e7c ("test/event: add DMA adapter auto-test") Cc: stable@dpdk.org Signed-off-by: Jerin Jacob <jerinj@marvell.com> Acked-by: Amit Prakash Shukla <amitprakashs@marvell.com> Signed-off-by: David Marchand <david.marchand@redhat.com>
Fix the following error seen with clang 21.1.4 app/test-eventdev/test_perf_common.c:1340:30: error: variable 'event' is uninitialized when passed as a const pointer argument here [-Werror,-Wuninitialized-const-pointer] Fixes: b25a66c ("app/eventdev: add DMA adapter performance test") Cc: stable@dpdk.org Signed-off-by: Jerin Jacob <jerinj@marvell.com> Acked-by: Pavan Nikhilesh <pbhagavatula@marvell.com> Signed-off-by: David Marchand <david.marchand@redhat.com>
Fix 16-bits formatting issues reported by clang 21 on Fedora 43:
../lib/eventdev/rte_event_crypto_adapter.c:1461:4: error:
format specifies type 'unsigned char' but the argument has type
'uint16_t' (aka 'unsigned short') [-Werror,-Wformat]
1459 | RTE_EDEV_LOG_ERR("Failed to get adapter caps dev %" PRIu8
| ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
1460 | " cdev %" PRIu8, adapter->eventdev_id,
| ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
1461 | adapter->next_cdev_id);
| ^~~~~~~~~~~~~~~~~~~~~~
../lib/eventdev/rte_event_crypto_adapter.c:1592:46: error:
format specifies type 'unsigned char' but the argument has type
'uint16_t' (aka 'unsigned short') [-Werror,-Wformat]
1592 | RTE_EDEV_LOG_ERR("Invalid dev_id=%" PRIu8, cdev_id);
| ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~
../lib/eventdev/rte_event_crypto_adapter.c:1613:45: error:
format specifies type 'unsigned char' but the argument has type
'uint16_t' (aka 'unsigned short') [-Werror,-Wformat]
1612 | RTE_EDEV_LOG_ERR("Event vectorization is not supported,"
| ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
1613 | "dev %" PRIu8 " cdev %" PRIu8, dev_id, cdev_id);
| ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~
Fixes: c1749bc ("eventdev: introduce event cryptodev vector type")
Fixes: 04ed18c ("eventdev/crypto: support runtime set/get parameters")
Cc: stable@dpdk.org
Signed-off-by: David Marchand <david.marchand@redhat.com>
Acked-by: Abhinandan Gujjar <abhinandan.gujjar@intel.com>
Fix 16-bits formatting issues reported by clang 21 on Fedora 43:
../lib/rawdev/rte_rawdev.c:429:41: error: format specifies type
'unsigned char' but the argument has type 'uint16_t'
(aka 'unsigned short') [-Werror,-Wformat]
429 | RTE_RDEV_DEBUG("Start dev_id=%" PRIu8, dev_id);
| ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~
../lib/rawdev/rte_rawdev.c:435:9: error: format specifies type
'unsigned char' but the argument has type 'uint16_t'
(aka 'unsigned short') [-Werror,-Wformat]
434 | RTE_RDEV_ERR("Device with dev_id=%" PRIu8 "already started",
| ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
435 | dev_id);
| ^~~~~~~
../lib/rawdev/rte_rawdev.c:457:40: error: format specifies type
'unsigned char' but the argument has type 'uint16_t'
(aka 'unsigned short') [-Werror,-Wformat]
457 | RTE_RDEV_DEBUG("Stop dev_id=%" PRIu8, dev_id);
| ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~
../lib/rawdev/rte_rawdev.c:464:4: error: format specifies type
'unsigned char' but the argument has type 'uint16_t'
(aka 'unsigned short') [-Werror,-Wformat]
463 | RTE_RDEV_ERR("Device with dev_id=%" PRIu8 "already stopped",
| ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
464 | dev_id);
| ^~~~~~~
Fixes: c88b3f2 ("rawdev: introduce raw device library")
Cc: stable@dpdk.org
Signed-off-by: David Marchand <david.marchand@redhat.com>
Fedora 39 is EOL for some time. Switch to Fedora 43 which will be maintained for a year (until December 2026). Signed-off-by: David Marchand <david.marchand@redhat.com> Acked-by: Bruce Richardson <bruce.richardson@intel.com> Acked-by: Aaron Conole <aconole@redhat.com>
Add domain_id field to access group join API, which is required to retrieve handler information associated with the specified domain. This enhancement ensures that domain-specific context is available during group join operations, improving flexibility and traceability in multi-domain environments. Patch aligns the parameters of access group create and join APIs for consistency and clarity, also updates the dmadev.rst file with detailed steps. Signed-off-by: Vamsi Attunuru <vattunuru@marvell.com> Signed-off-by: Chengwen Feng <fengchengwen@huawei.com>
This commit supports SVA copy for dmadev test. Signed-off-by: Chengwen Feng <fengchengwen@huawei.com>
The original hisilicon DMA driver correspond to the internal DMA unit of Kunpeng SoC PCIe subsystem. This commit renames it to hisi_pciep (PCIe internal endpoint) to reflect hardware IP. Signed-off-by: Chengwen Feng <fengchengwen@huawei.com>
chkincs tool produces warnings if experimental functions are called directly. There are multiple ways to quiesce these warnings but common pattern is excluding experimental code from build and triggering abort in runtime. Follow that pattern here. Signed-off-by: Tomasz Duszynski <tduszynski@marvell.com>
Install only architecture specific headers to avoid header pollution. Signed-off-by: Tomasz Duszynski <tduszynski@marvell.com>
Replace static per-architecture ops with a callback-based ops structure. This makes architecture-specific PMU implementations more modular and easier to extend. Signed-off-by: Tomasz Duszynski <tduszynski@marvell.com>
RTE_LIB_PMU is defined by build system automatically if DPDK is build for Linux. Otherwise it's missing. That said all cases where conditional compilation is required can be handled without introducing other macros. Signed-off-by: Tomasz Duszynski <tduszynski@marvell.com>
/proc/sys/kernel/perf_user_access attribute allow user process to access perf counters. Though in order to change it, a process requires elevated capabilities or must be run as root. If that's not the case, counter access remains disabled. Hence, to avoid confusion, log message that warns user about that. Signed-off-by: Tomasz Duszynski <tduszynski@marvell.com>
This patch adds probe and remove operation for Hisilicon accelerator DMA driver. Signed-off-by: Chengwen Feng <fengchengwen@huawei.com>
This commit adds control path ops for Hisilicon accelerator DMA driver. Signed-off-by: Chengwen Feng <fengchengwen@huawei.com>
This commit adds data path ops for Hisilicon accelerator DMA driver. Signed-off-by: Chengwen Feng <fengchengwen@huawei.com>
Adding support for HiSilicon HIP12 platform. Signed-off-by: Chengwen Feng <fengchengwen@huawei.com> Acked-by: Huisong Li <lihuisong@huawei.com>
When running our existing DTS testsuites on a new NIC we observed packets would not transmit from the traffic generator to the system under test even after DPDK testpmd and the NIC under test had indicated readiness. After investigation, we determined that the existing readiness check in DTS for testpmd start (checking that port is started) is insufficient, because on some systems the link will remain down for some measurable time, creating a race condition between the testpmd port's link coming up and the DTS execution reaching the packet transmission step. This change will ensure that testpmd start will block until the port is reporting that its link is up. In addition, the interval in between checking the link state has been reduced in order to speed up the execution. Signed-off-by: Patrick Robb <probb@iol.unh.edu> Tested-by: Patrick Robb <probb@iol.unh.edu> Signed-off-by: 0-day Robot <robot@bytheb.org>
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
WalkthroughThis pull request introduces HiSilicon accelerator DMA device support for HIP12 SoC, refactors the existing HiSilicon DMA driver into separate PCIe and accelerator drivers, updates DMA access pair group API to include domain_id parameter, refactors PMU architecture operations to use an ops table, fixes uninitialized variables in tests, and updates build tooling and documentation. Changes
Sequence Diagram(s)sequenceDiagram
participant App as DPDK App
participant PMU as lib/pmu
participant ArchOps as pmu_arch_ops
participant ARM64 as pmu_arm64
App->>PMU: rte_pmu_init()
PMU->>ArchOps: arch_ops->init()
ArchOps->>ARM64: pmu_arm64_init()
ARM64->>ARM64: read PERF_USER_ACCESS_PATH
ARM64-->>ArchOps: return result
ArchOps-->>PMU: return
PMU-->>App: init complete
rect rgba(100, 150, 200, 0.3)
Note over PMU,ARM64: Static ops table dispatch (new)
PMU->>ArchOps: check arch_ops != NULL
alt ops present
ArchOps->>ARM64: call registered function
else ops absent
ArchOps->>PMU: inline no-op
end
end
App->>PMU: rte_pmu_fini()
PMU->>ArchOps: arch_ops->fini()
ArchOps->>ARM64: pmu_arm64_fini()
ARM64-->>ArchOps: return
ArchOps-->>PMU: return
PMU-->>App: fini complete
sequenceDiagram
participant P1 as Process 1
participant Dev as DMA Device
participant P2 as Process 2
rect rgba(100, 200, 100, 0.3)
Note over P1,Dev: Group Creation Phase (New)
P1->>P1: generate unique domain_id
P1->>Dev: rte_dma_access_pair_group_create()
Dev-->>P1: group_id, token
P1->>P1: store group_id, token, domain_id
end
P1->>P2: share group_id, token, domain_id
rect rgba(150, 200, 100, 0.3)
Note over P2,Dev: Group Join Phase (Updated signature)
P2->>P2: generate own domain_id
P2->>Dev: rte_dma_access_pair_group_join(<br/>dev_id, domain_id, token, group_id, cb)
Note over Dev: New: domain_id param added<br/>Reordered: group_id moved after token
Dev-->>P2: handler info
end
P2->>P1: share own domain_id
rect rgba(200, 150, 100, 0.3)
Note over P1,Dev: DMA Operations
P1->>Dev: rte_dma_access_pair_group_handler_get()
Dev-->>P1: handler
P1->>Dev: configure vchan & transfer
end
rect rgba(150, 100, 200, 0.3)
Note over P2,Dev: Cleanup
P2->>Dev: rte_dma_access_pair_group_leave()
P1->>Dev: rte_dma_access_pair_group_destroy()
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes
Areas requiring extra attention:
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ 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: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
drivers/bus/cdx/cdx_vfio.c (1)
103-103: Fix incorrect variable in error message at line 138.The
cdx_addrbuffer is not unused—it's referenced in an error message at line 138. However, it's declared with zero-initialization but never assigned a value, so the error message will print an empty string. Based on the pattern of other error messages in this function (lines 111, 115, 119), the error should usedev->device.nameinstead of the emptycdx_addrbuffer. Either replacecdx_addrwithdev->device.namein the error message, or remove thecdx_addrdeclaration if it's no longer needed elsewhere.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (37)
.github/workflows/build.yml(2 hunks)MAINTAINERS(1 hunks)app/proc-info/main.c(1 hunks)app/test-eventdev/test_perf_common.c(1 hunks)app/test/test_dmadev.c(5 hunks)app/test/test_event_dma_adapter.c(2 hunks)app/test/test_trace.c(1 hunks)app/test/test_trace.h(1 hunks)buildtools/pmdinfogen.py(2 hunks)config/arm/arm64_hip12_linux_gcc(1 hunks)config/arm/meson.build(5 hunks)doc/guides/dmadevs/hisi_acc.rst(1 hunks)doc/guides/dmadevs/hisi_pciep.rst(3 hunks)doc/guides/dmadevs/index.rst(1 hunks)doc/guides/prog_guide/dmadev.rst(2 hunks)doc/guides/rel_notes/release_25_11.rst(1 hunks)drivers/bus/cdx/cdx_vfio.c(2 hunks)drivers/dma/hisi_acc/hisi_acc_dmadev.c(1 hunks)drivers/dma/hisi_acc/hisi_acc_dmadev.h(1 hunks)drivers/dma/hisi_acc/meson.build(1 hunks)drivers/dma/hisi_pciep/hisi_pciep_dmadev.c(2 hunks)drivers/dma/hisi_pciep/meson.build(1 hunks)drivers/dma/meson.build(1 hunks)drivers/net/intel/iavf/iavf_vchnl.c(1 hunks)dts/api/testpmd/__init__.py(2 hunks)lib/dmadev/rte_dmadev.c(2 hunks)lib/dmadev/rte_dmadev.h(4 hunks)lib/dmadev/rte_dmadev_pmd.h(1 hunks)lib/dmadev/rte_dmadev_trace.h(1 hunks)lib/eal/include/rte_eal_trace.h(1 hunks)lib/eventdev/rte_event_crypto_adapter.c(3 hunks)lib/pmu/meson.build(1 hunks)lib/pmu/pmu.c(1 hunks)lib/pmu/pmu_arm64.c(1 hunks)lib/pmu/pmu_private.h(2 hunks)lib/pmu/rte_pmu.h(3 hunks)lib/rawdev/rte_rawdev.c(2 hunks)
🧰 Additional context used
🧬 Code graph analysis (6)
app/test-eventdev/test_perf_common.c (1)
lib/eventdev/rte_event_dma_adapter.c (1)
rte_event_dma_adapter_vchan_add(992-1104)
lib/dmadev/rte_dmadev.h (1)
lib/dmadev/rte_dmadev.c (1)
rte_dma_access_pair_group_join(877-903)
app/test/test_dmadev.c (1)
lib/dmadev/rte_dmadev.h (2)
rte_dma_copy(1177-1195)rte_dma_completed(1351-1385)
dts/api/testpmd/__init__.py (3)
dts/framework/context.py (1)
get_ctx(87-96)dts/api/capabilities.py (1)
LinkTopology(53-66)dts/framework/exception.py (1)
InteractiveCommandExecutionError(180-184)
app/test/test_trace.c (1)
drivers/bus/platform/platform.c (1)
tmp(101-111)
drivers/dma/hisi_acc/hisi_acc_dmadev.c (1)
lib/dmadev/rte_dmadev.c (1)
rte_dma_pmd_allocate(356-371)
🔇 Additional comments (44)
drivers/net/intel/iavf/iavf_vchnl.c (1)
105-105: Good deterministic fixInitializing
notify_byteguarantees we always notify the pipe with a defined value. Thanks for closing out that uninitialized-read risk.app/proc-info/main.c (1)
2124-2124: LGTM: Modernized EAL core specification.The change from
-c1(hexadecimal core mask) to-l0(logical core list) is functionally equivalent—both select core 0. The-loption is the modern, more readable way to specify cores in DPDK.app/test/test_event_dma_adapter.c (2)
145-152: LGTM! Proper initialization fixes uninitialized variable issue.The scoped event variable with designated initializer correctly addresses the uninitialized variable issue. The pattern properly handles both capability cases: passing an initialized event structure when
RTE_EVENT_DMA_ADAPTER_CAP_INTERNAL_PORT_VCHAN_EV_BINDis present, and NULL otherwise.
530-537: LGTM! Consistent initialization pattern.The implementation mirrors the pattern in
test_dma_adapter_params, correctly initializing the event structure within the appropriate scope and passing NULL when the capability is not present.app/test-eventdev/test_perf_common.c (1)
1337-1345: LGTM! Context-appropriate initialization.The implementation correctly initializes the event structure with the context-specific
p->queue_idvalue, which is more appropriate than a hardcoded value. The explicit else block with braces enhances code clarity. The pattern properly handles both capability cases..github/workflows/build.yml (2)
237-237: Fedora image references verified as consistent.All three Fedora image references in the workflow file have been confirmed to use fedora:43 (lines 237, 286, 288), and no references to fedora:39 or older versions remain elsewhere in the file. The update is complete and consistent.
237-237: Fedora 43 is a stable release; verify package compatibility via CI test run.Fedora Linux 43 is a stable, final release published October 28, 2025. However, package availability testing in the sandbox environment showed all 23 build dependencies as inaccessible, which suggests an environmental limitation rather than actual incompatibility.
Since the build will validate immediately upon CI execution, ensure the workflow runs successfully with these changes. If
dnf installcommands fail in the actual CI job, investigate package availability in Fedora 43's official repositories or check for renamed/replaced packages (e.g.,intel-ipsec-mb-develorlibxdp-develmay have different names or availability status in F43).dts/api/testpmd/__init__.py (3)
91-94: LGTM! Clear documentation of new exception behavior.The docstring accurately reflects the new link status verification that can raise
InteractiveCommandExecutionError.
101-104: LGTM! Robust link status verification.The implementation correctly ensures that all port links are up before proceeding with operations that require started ports. The check appropriately:
- Only runs when topology uses linking
- Provides clear error messages identifying which port failed
- Uses the existing
wait_link_status_upmethod with proper timeout handlingNote: This verification runs each time a decorated method is called, which ensures the precondition always holds even if links go down between calls.
270-270: LGTM! Improved polling responsiveness.Reducing the polling interval from 0.5s to 0.25s makes link status detection more responsive, which complements the new link verification logic well.
app/test/test_dmadev.c (4)
517-549: LGTM! Well-structured SVA copy test.The function correctly implements SVA testing by casting virtual addresses to
rte_iova_t, which is the key difference from regular DMA tests. Error handling and data verification are thorough.
551-574: LGTM! Excellent test coverage for SVA copies.The function tests both heap-allocated and stack-allocated buffers, which provides comprehensive coverage for SVA functionality. Memory management is correct, with proper cleanup on malloc failure.
1266-1279: LGTM! Proper capability checking.The setup function correctly validates SVA support before running tests and provides clear logging when tests are skipped.
1438-1449: LGTM! Proper test suite integration.The SVA test is correctly integrated into the test suite, following the established patterns for enum, parameters, and test case registration.
Also applies to: 1451-1461, 1474-1476
buildtools/pmdinfogen.py (1)
9-9: LGTM: Import addition supports refactoring.The
structimport is necessary for the ctypes-to-struct refactoring in the_load_pci_idsmethod.app/test/test_trace.c (1)
182-182: Good fix: Initializing variable to prevent undefined behavior.The initialization of
tmpis appropriate since it's later used as an argument torte_eal_trace_generic_ptr(&tmp)on line 201. This prevents potential undefined behavior from using an uninitialized variable.lib/rawdev/rte_rawdev.c (1)
429-429: LGTM: Format specifiers corrected to match data types.The format specifier changes from
PRIu8toPRIu16correctly align with theuint16_ttype ofdev_idas declared in the function signatures (lines 424, 453).Also applies to: 434-434, 457-457, 463-463
lib/eal/include/rte_eal_trace.h (1)
23-23: LGTM: Trace point declaration standardization.The change from
RTE_TRACE_POINT_ARGS(void)to(void)standardizes the trace point declaration for void arguments, consistent with similar changes across the codebase (e.g., app/test/test_trace.h).app/test/test_trace.h (1)
16-16: LGTM: Consistent trace point declaration update.The change aligns with the trace point declaration standardization seen in lib/eal/include/rte_eal_trace.h, removing the
RTE_TRACE_POINT_ARGSwrapper for void arguments.lib/eventdev/rte_event_crypto_adapter.c (1)
1460-1460: LGTM: Format specifiers corrected for proper type matching.The format specifier changes from
PRIu8toPRIu16forcdev_idcorrectly align with theuint16_ttype as seen in function signatures (e.g., line 1579:uint16_t cdev_id).Also applies to: 1592-1592, 1613-1613
drivers/dma/hisi_pciep/hisi_pciep_dmadev.c (2)
18-18: LGTM: Header include updated for driver refactoring.The include path change from
"hisi_dmadev.h"to"hisi_pciep_dmadev.h"correctly reflects the driver refactoring that splits the HiSilicon DMA driver into separate PCIe endpoint and accelerator drivers.
995-997: LGTM: PMD registration names updated consistently.The PMD registration identifiers have been consistently updated from
dma_hisilicontodma_hisi_pciepacross all three registration points (driver, PCI table, and kernel module dependency), properly reflecting the PCIe endpoint driver naming after the refactoring.drivers/dma/hisi_pciep/meson.build (1)
18-18: LGTM: Build configuration updated for renamed source file.The source file reference correctly updated to
'hisi_pciep_dmadev.c', aligning with the driver refactoring and the include path changes in the source file.doc/guides/dmadevs/index.rst (1)
17-18: LGTM: Documentation updated for driver split.The toctree correctly reflects the refactored HiSilicon DMA driver split into two separate drivers:
hisi_acc(accelerator) andhisi_pciep(PCIe endpoint), matching the driver registration names in the codebase.drivers/dma/meson.build (1)
8-9: LGTM!The driver list correctly reflects the split of the HiSilicon DMA driver into separate accelerator (hisi_acc) and PCIe internal endpoint (hisi_pciep) drivers.
doc/guides/rel_notes/release_25_11.rst (1)
163-173: LGTM!The release notes clearly document both the driver rename and the new accelerator DMA driver addition, providing appropriate context for each change.
drivers/dma/hisi_acc/meson.build (3)
4-8: LGTM!The Linux-only restriction is appropriate for a UACCE-based driver.
10-14: LGTM!The architecture restriction to x86_64 and aarch64 is appropriate for this accelerator driver.
16-21: LGTM!The dependencies, source file specification, and IOVA flag are correctly configured for the accelerator DMA driver.
config/arm/arm64_hip12_linux_gcc (1)
1-17: LGTM!The ARM64 HIP12 cross-compilation configuration correctly specifies the toolchain, host machine characteristics, and platform property.
drivers/bus/cdx/cdx_vfio.c (2)
457-457: LGTM!The error message correctly uses
dev_namefor better clarity in reporting mapping failures.
538-538: LGTM!The VFIO release call correctly uses
dev_namefor consistency with the rest of the function.doc/guides/dmadevs/hisi_acc.rst (1)
1-68: Well-structured documentation.The documentation provides clear guidance on device setup, probing, configuration, and limitations. The information is appropriate for users integrating the HiSilicon Accelerator DMA driver.
doc/guides/dmadevs/hisi_pciep.rst (3)
4-8: LGTM!The title and description updates correctly reflect the driver's focus on PCIe internal endpoint functionality, aligning with the rename from hisilicon to hisi_pciep.
19-22: LGTM!The device setup section clearly specifies the requirement to bind to a DPDK-supported user-space IO driver like vfio-pci.
36-38: LGTM!The configuration section header updates improve consistency with DPDK documentation formatting standards.
lib/dmadev/rte_dmadev_trace.h (1)
123-129: API consistency verified. Changes are correct.The trace point signature matches the
rte_dma_access_pair_group_joinfunction signature. All parameters (dev_id,domain_id,token,group_id) are correctly emitted according to their types, and the callback parameter is appropriately excluded from tracing.lib/dmadev/rte_dmadev.c (1)
878-903: Domain ID propagation matches the updated contract. Passingdomain_idthrough to the PMD hook keeps the library in sync with the revised join signature, so downstream drivers will see the full context they now expect.MAINTAINERS (1)
1383-1392: MAINTAINERS entries stay consistent with the driver split. The renamed PCIe PMD and the new accelerator PMD are both captured with the right paths and maintainer, so tooling likeget-maintainer.shwill route patches correctly.doc/guides/prog_guide/dmadev.rst (1)
204-252: Workflow update clearly explains the new domain handshake. Calling out how both processes generate and exchangedomain_idbefore join helps applications adopt the revised API without guesswork.lib/dmadev/rte_dmadev.h (1)
884-933: Header signature and docs aligned. The public prototype now mirrors the implementation and the comment reinforces thatdomain_idmust be supplied alongside the token when joining.drivers/dma/hisi_acc/hisi_acc_dmadev.h (1)
1-167: Header captures the accelerator queue state cleanly. The SQE/CQE definitions and queue bookkeeping fields give the PMD everything it needs without leaking implementation details elsewhere.lib/dmadev/rte_dmadev_pmd.h (1)
78-80: Signature update looks consistent.The additional
domain_idparameter and reordered arguments match the revised dmadev API usage. ✔️config/arm/meson.build (1)
258-266: HIP12 profile addition looks good.Part-number flags, mcpu definition, and SoC registry updates line up with the HIP10 template and keep the public list in sync. ✅
| if image.is_big_endian: | ||
| fmt = ">" | ||
| else: | ||
| fmt = "<" | ||
| fmt += "LHHHH" | ||
|
|
||
| result = [] | ||
| while True: | ||
| size = ctypes.sizeof(rte_pci_id) | ||
| size = struct.calcsize(fmt) | ||
| offset = size * len(result) | ||
| data = table_symbol.get_value(offset, size) | ||
| if not data: | ||
| break | ||
| pci_id = rte_pci_id.from_buffer_copy(data) | ||
| if not pci_id.device_id: | ||
| _, vendor, device, ss_vendor, ss_device = struct.unpack_from(fmt, data) | ||
| if not device: | ||
| break | ||
| result.append( | ||
| [ | ||
| pci_id.vendor_id, | ||
| pci_id.device_id, | ||
| pci_id.subsystem_vendor_id, | ||
| pci_id.subsystem_device_id, | ||
| ] | ||
| ) | ||
| result.append((vendor, device, ss_vendor, ss_device)) | ||
|
|
There was a problem hiding this comment.
🧩 Analysis chain
Verify format string matches C struct definition across platforms.
The format string uses 'L' (unsigned long) which is platform-dependent—4 bytes on 32-bit systems, often 8 bytes on 64-bit systems. If the corresponding C structure uses a fixed-width type like uint32_t, this should be 'I' (unsigned int, always 4 bytes) to ensure consistent parsing across architectures.
Run the following script to locate and examine the C structure definition:
After reviewing the C structure:
- If it uses
uint32_tor similar fixed-width type for the first field, change'L'to'I'in the format string - If it uses
unsigned long, the current code is correct but may have portability implications
🏁 Script executed:
#!/bin/bash
# Description: Find the rte_pci_id structure definition to verify field sizes
# Search for the struct definition
rg -n -A 10 'struct\s+rte_pci_id\s*\{' --type=c
# Also search for any typedef of rte_pci_id
rg -n -A 10 'typedef.*rte_pci_id' --type=cLength of output: 690
Change format specifier from 'L' to 'I' for platform-independent uint32_t handling.
The C structure defines the first field as uint32_t class_id (4 bytes fixed), but the format string uses 'L' (unsigned long), which is platform-dependent—4 bytes on 32-bit systems but typically 8 bytes on 64-bit systems. This causes misaligned unpacking on 64-bit architectures.
Replace the format string construction (lines 153-155) to use 'I' instead of 'L':
if image.is_big_endian:
fmt = ">"
else:
fmt = "<"
fmt += "IHHHH" # Changed from "LHHHH" to "IHHHH"🤖 Prompt for AI Agents
In buildtools/pmdinfogen.py around lines 151 to 168, the struct format uses 'L'
which is platform-dependent; replace the unsigned long specifier with a 4-byte
unsigned int specifier so the C uint32_t is always treated as 4 bytes.
Concretely, keep the endianness prefix logic but change the appended format
fragment from "LHHHH" to "IHHHH" so struct.calcsize and struct.unpack_from use a
fixed 4-byte field for class_id, preserving the rest of the H fields.
|
|
||
| Currently, the maximum size of the operation data is limited to 16MB-1B in the driver. | ||
| The device actually supports operations in a larger data size, | ||
| but the driver requires complex operations in the datapth. |
There was a problem hiding this comment.
Fix typo.
There's a typographical error: "datapth" should be "datapath".
Apply this diff to fix the typo:
- but the driver requires complex operations in the datapth.
+ but the driver requires complex operations in the datapath.📝 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.
| but the driver requires complex operations in the datapth. | |
| but the driver requires complex operations in the datapath. |
🤖 Prompt for AI Agents
In doc/guides/dmadevs/hisi_acc.rst around line 67, there's a typo where
"datapth" should be "datapath"; update that single word to "datapath" to correct
the spelling.
| *last_idx = hw->cridx + i - 1; | ||
| if (i > 0) { | ||
| hw->cridx += i; | ||
| hw->sq_head = sq_head; | ||
| hw->completed += i; | ||
| } | ||
|
|
||
| return i; | ||
| } |
There was a problem hiding this comment.
Avoid underflow when reporting last_idx.
When no completions are available (i == 0 / cpl_num == 0), the code still updates *last_idx with hw->cridx + ... - 1, which wraps to UINT16_MAX. Applications relying on the old value get a spurious index even though the function returned zero completions. Please bail out before touching last_idx.
@@
- *last_idx = hw->cridx + i - 1;
- if (i > 0) {
- hw->cridx += i;
- hw->sq_head = sq_head;
- hw->completed += i;
- }
-
- return i;
+ if (i == 0)
+ return 0;
+
+ *last_idx = hw->cridx + i - 1;
+ hw->cridx += i;
+ hw->sq_head = sq_head;
+ hw->completed += i;
+
+ return i;
@@
- *last_idx = hw->cridx + cpl_num - 1;
- if (likely(cpl_num > 0)) {
- hw->cridx += cpl_num;
- hw->sq_head = sq_head;
- hw->completed += cpl_num;
- }
-
- return cpl_num;
+ if (cpl_num == 0)
+ return 0;
+
+ *last_idx = hw->cridx + cpl_num - 1;
+ hw->cridx += cpl_num;
+ hw->sq_head = sq_head;
+ hw->completed += cpl_num;
+
+ return cpl_num;Also applies to: 485-493
🤖 Prompt for AI Agents
In drivers/dma/hisi_acc/hisi_acc_dmadev.c around lines 456-464 (and similarly
485-493), the function writes *last_idx = hw->cridx + i - 1 even when i == 0
which underflows to UINT16_MAX; bail out early when there are no completions by
checking if i == 0 (or cpl_num == 0) and return 0 before modifying *last_idx,
leaving the previous value untouched; keep the existing updates to hw->cridx,
hw->sq_head and hw->completed only inside the i > 0 path.
| #define PMU_SET_ARCH_OPS(ops) \ | ||
| RTE_INIT(rte_pmu_set_arch_ops) \ | ||
| { \ | ||
| arch_ops = &(ops); \ | ||
| } |
There was a problem hiding this comment.
Keep the arch_ops constructor reachable in static builds
With the new PMU_SET_ARCH_OPS macro, pmu_arm64.c (and the x86 equivalent) no longer export any symbol that the rest of librte_pmu references. In a static build, the linker therefore never pulls those objects out of librte_pmu.a, the RTE_INIT constructor never runs, and arch_ops stays NULL. As a result, pmu_arch_init/pmu_arch_fixup_config degrade to no-ops and PMU access breaks on static builds. Previously the direct pmu_arch_* calls guaranteed the object was linked, so this is a regression. Please make sure each architecture TU still provides an externally referenced symbol (for example, keep exported pmu_arch_init/pmu_arch_fini wrappers that set the ops table, or otherwise create a referenced symbol that pmu.c pulls in) so the constructor is reachable in both shared and static builds.
NOTE: This is an auto submission for "[v2] dts: testpmd link check on port start".
See "http://patchwork.dpdk.org/project/dpdk/list/?series=36593" for details.
Summary by CodeRabbit
New Features
Bug Fixes
Documentation
Refactoring
dma/hisilicontodma/hisi_pciep.Chores