[PWCI] "Assign all unit tests to suites" #525
Open
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Starting with NVIDIA ConnectX-9, the future devices will support only hardware steering (HWS) flow engine. The software steering options (legacy Verbs and Direct Verbs) have lower performances, and won't be available for new devices. Both flow APIs (sync and async template) will still be supported with the hardware steering flow engine. Fixes: 1b55eeb ("common/mlx5: add ConnectX-9 SuperNIC") Signed-off-by: Thomas Monjalon <thomas@monjalon.net>
The tool ripgrep allows to find files not ending with a line break: rg -Ul '[^\n]\z' The files with a trailing blank lines are shown with this command: rg -Ul '\n\n\z' Files are fixed to end with a single line break. Signed-off-by: Thomas Monjalon <thomas@monjalon.net>
Signed-off-by: Thomas Monjalon <thomas@monjalon.net>
Start a new release cycle with empty release notes. Bump version and ABI minor. Signed-off-by: David Marchand <david.marchand@redhat.com> Acked-by: Aaron Conole <aconole@redhat.com> Acked-by: Thomas Monjalon <thomas@monjalon.net>
Within GitHub actions, with MSVC 19.44.35219, the following error can be reported: ...\rte_rcu_qsbr.h(566): error C2220: the following warning is treated as an error ...\rte_rcu_qsbr.h(566): warning C4319: '~': zero extending 'unsigned long' to 'uint64_t' of greater size To fix this, replace the "1UL" with RTE_BIT64 to force a 64-bit value on all platforms. Fixes: 64994b5 ("rcu: add RCU library supporting QSBR mechanism") Cc: stable@dpdk.org Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
Disable warning about "zero extending 'unsigned long' to 'u64' of greater size" in base code builds. Cc: stable@dpdk.org Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
For the fast tests, we have two extra parameters specifying when the test can be run without hugepages or using ASan. The true/false nature of these parameters is not very clear, so change things so that they are explicitly specified as NOHUGE_OK/NOHUGE_SKIP and ASAN_OK/ASAN_SKIP instead. Explicitly validate the options in the meson.build files, rather than just checking for one of the pair of options - which can hide errors. Signed-off-by: Bruce Richardson <bruce.richardson@intel.com> Signed-off-by: 0-day Robot <robot@bytheb.org>
The shift of a negative number (or very large positive) is undefined behaviour which causes errors when run with UBSan. Fix this by making the behaviour explicit for the edge case of n being zero in the calculation. Fixes: de3cfa2 ("sched: initial import") Cc: stable@dpdk.org Signed-off-by: Bruce Richardson <bruce.richardson@intel.com> Signed-off-by: 0-day Robot <robot@bytheb.org>
Reduce the timer duration and the primary wait duration in the secondary process timer autotest. This should help ensure it doesn't time out what run in a CI environment. Signed-off-by: Bruce Richardson <bruce.richardson@intel.com> Signed-off-by: 0-day Robot <robot@bytheb.org>
When the secondary process run from timer_secontary_autotest fails, the timer loop is never stopped so the whole process hangs until timeout. Fix this by setting the stop flag before checking for success or failure of the secondary process. Fixes: 50247fe ("test/timer: exercise new APIs in secondary process") Cc: stable@dpdk.org Signed-off-by: Bruce Richardson <bruce.richardson@intel.com> Signed-off-by: 0-day Robot <robot@bytheb.org>
Add APIs for internal DPDK use to allow querying the paths of drivers loaded into the running instance. Signed-off-by: Bruce Richardson <bruce.richardson@intel.com> Signed-off-by: 0-day Robot <robot@bytheb.org>
For unit tests which run secondary processes, allow passing the driver paths used by the primary process to that secondary. This allows use of mempools in those secondary tests. Without this, any tests using mempools in secondary process will fail in shared builds. Signed-off-by: Bruce Richardson <bruce.richardson@intel.com> Signed-off-by: 0-day Robot <robot@bytheb.org>
If the power subsystem cannot be initialized e.g. due to not being root, skip the capabilities test, rather than failing it. Signed-off-by: Bruce Richardson <bruce.richardson@intel.com> Signed-off-by: 0-day Robot <robot@bytheb.org>
There are quite a number of test cases defined which are not present in any test-suite. Meson warns about these on build, so reduce the warnings by adding external-mem, ipsec-sad, red, power-caps and secondary timer test cases to the fast-test suite. Signed-off-by: Bruce Richardson <bruce.richardson@intel.com> Signed-off-by: 0-day Robot <robot@bytheb.org>
A number of eventdev, cryptodev and ethdev tests require devices to be present in order to run so add them to the "driver" suite of tests. Signed-off-by: Bruce Richardson <bruce.richardson@intel.com> Signed-off-by: 0-day Robot <robot@bytheb.org>
If there are no tests which are not assigned to a test suite, omit the line printing the non_suite_tests. This avoid having error messages from meson about test "" not being in any test suite. Fixes: 25065ef ("test: emit warning for orphaned tests") Cc: stable@dpdk.org Signed-off-by: Bruce Richardson <bruce.richardson@intel.com> Signed-off-by: 0-day Robot <robot@bytheb.org>
Reviewer's GuideIntroduce internal APIs to iterate/count EAL driver paths and propagate -d/--driver-path options into child test processes; normalize fast-test registration to use NOHUGE_/ASAN_ flags and wire nearly all unit tests into Meson suites; fix a few correctness/robustness issues (RED rounding UB, RCU bitmask), refine some tests’ behavior and skipping logic, and update CI, docs, and metadata for the v25.11/v26.03 baseline. Sequence diagram for iterating EAL driver paths with cmdline_only filtersequenceDiagram
participant TestProcess
participant EAL
participant solib_list
TestProcess->>EAL: RTE_EAL_DRIVER_PATH_FOREACH(path, true)
activate EAL
loop foreach expansion
EAL->>EAL: rte_eal_driver_path_next(start, true)
EAL->>solib_list: locate start entry
alt start is NULL
EAL->>solib_list: take TAILQ_FIRST
else start not NULL
EAL->>solib_list: advance to TAILQ_NEXT
end
alt cmdline_only == true
EAL->>solib_list: skip entries with from_cmdline == false
end
EAL-->>TestProcess: return next path (or NULL)
opt path is NULL
TestProcess->>EAL: loop terminates
end
end
deactivate EAL
Class diagram for updated EAL driver path managementclassDiagram
class shared_driver {
+char name[PATH_MAX]
+void* lib_handle
+bool from_cmdline
}
class EALPluginManager {
+int eal_plugin_add(const char* path, bool from_cmdline)
+const char* rte_eal_driver_path_next(const char* start, bool cmdline_only)
+unsigned int rte_eal_driver_path_count(bool cmdline_only)
}
class EALArgs {
+struct internal_config internal_cfg
+int eal_parse_args(void)
}
EALPluginManager "1" --> "*" shared_driver : manages
EALArgs "1" --> "*" shared_driver : populates via eal_plugin_add
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
|
Important Review skippedMore than 25% of the files skipped due to max files limit. The review is being skipped to prevent a low-quality review. 69 files out of 176 files are above the max files limit of 100. Please upgrade to Pro plan to get higher limits. You can disable this status message by setting the ✨ 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 |
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey there - I've reviewed your changes - here's some feedback:
- The new rte_eal_driver_path_next() API relies on pointer equality between the
startargument andsolib->name; either explicitly document thatstartmust be a pointer previously returned by this API (rather than an arbitrary string) or switch to string comparison to make this more robust. - In app/test/suites/meson.build the comment about the extraction script running tolower() no longer matches the updated get-test-suites.py (which now preserves case), so the comment should be updated or removed to avoid confusion.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The new rte_eal_driver_path_next() API relies on pointer equality between the `start` argument and `solib->name`; either explicitly document that `start` must be a pointer previously returned by this API (rather than an arbitrary string) or switch to string comparison to make this more robust.
- In app/test/suites/meson.build the comment about the extraction script running tolower() no longer matches the updated get-test-suites.py (which now preserves case), so the comment should be updated or removed to avoid confusion.
## Individual Comments
### Comment 1
<location> `lib/eal/common/eal_common_options.c:766-767` </location>
<code_context>
+ solib = TAILQ_FIRST(&solib_list);
+ } else {
+ /* Find the current entry based on the name string */
+ TAILQ_FOREACH(solib, &solib_list, next) {
+ if (start == solib->name) {
+ solib = TAILQ_NEXT(solib, next);
+ break;
</code_context>
<issue_to_address>
**suggestion (bug_risk):** The iterator relies on pointer identity for locating `start`, which may be brittle for external callers.
This relies on `start == solib->name`, so callers must pass back the exact pointer returned by `rte_eal_driver_path_next()`. If a caller passes a different pointer to an identical string (e.g., copied or reconstructed), iteration will stop early. To make this more robust, either compare with `strcmp(start, solib->name)` (noting the complexity trade-off) or clearly document that `start` must be one of the pointers previously returned by this API.
Suggested implementation:
```c
} else {
/* Find the current entry based on the name string contents */
TAILQ_FOREACH(solib, &solib_list, next) {
if (strcmp(start, solib->name) == 0) {
solib = TAILQ_NEXT(solib, next);
break;
}
}
```
1. At the top of `lib/eal/common/eal_common_options.c`, ensure `#include <string.h>` is present so that `strcmp` is declared. If the file already includes `<string.h>`, no further changes are required.
2. If this API is documented elsewhere (e.g., in a header or external docs) as requiring pointer identity, that documentation should be updated to state that iteration is based on string equality instead.
</issue_to_address>
### Comment 2
<location> `lib/eal/include/rte_eal.h:518-519` </location>
<code_context>
+ * @internal
+ * Iterate over all driver paths.
+ *
+ * This macro provides a convenient way to iterate through all driver paths
+ * that were loaded via -d flags during EAL initialization.
+ *
+ * @param path
</code_context>
<issue_to_address>
**nitpick:** Macro documentation doesn’t align with the `cmdline_only` parameter semantics.
The description for `RTE_EAL_DRIVER_PATH_FOREACH` currently says it iterates only over paths loaded via `-d` flags, but the macro passes the caller’s `cmdline_only` value to `rte_eal_driver_path_next()`, so it can iterate either only `-d` paths or all paths (including directory-expanded ones). Please update the docstring to describe both behaviors, depending on `cmdline_only`.
</issue_to_address>
### Comment 3
<location> `doc/guides/nics/mlx5.rst:1344` </location>
<code_context>
Faster than software steering (SWS),
-hardware steering (HWS) is the only mode supporting the flow template async API.
+hardware steering (HWS) is the only mode supporting the flow template async API,
+and the only mode supported on device ConnectX-9 and later.
Flow rules are managed by the hardware,
</code_context>
<issue_to_address>
**nitpick (typo):** Consider using the plural "devices" in "on device ConnectX-9 and later" for grammatical correctness.
Using singular "device" with "ConnectX-9 and later" reads awkward, since it refers to multiple devices; pluralizing to "devices" better matches the intended range.
```suggestion
and the only mode supported on devices ConnectX-9 and later.
```
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Auto-submission for "http://patchwork.dpdk.org/project/dpdk/list/?series=36843"
Summary by Sourcery
Add iteration APIs for EAL driver paths, propagate driver-path flags into spawned test processes, fully classify unit tests into suites with explicit NOHUGE/ASAN capabilities, and adjust CI and release metadata to a new reference release.
New Features:
Bug Fixes:
Enhancements:
Build:
CI:
Documentation:
Tests:
Chores: