[PWCI] "doc: document telemetry command name restrictions" #141
+73
−29
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
Running rte_exit() in a forked process means that shared memory will be released by the child process before the parent process does the same. This issue has been seen recently when some GHA virtual machine (with some mlx5 devices) runs the debug_autotest unit test. Instead, run rte_panic() and rte_exit() from a new DPDK process spawned like for other recursive unit tests. Bugzilla ID: 1796 Fixes: af75078 ("first public release") Cc: stable@dpdk.org Signed-off-by: David Marchand <david.marchand@redhat.com> Acked-by: Bruce Richardson <bruce.richardson@intel.com> Acked-by: Dariusz Sosnowski <dsosnowski@nvidia.com>
Telemetry command names must contain only alphanumeric characters (`A-Z`, `a-z`, `0-9`), underscore (`_`), and forward slash (`/`). Other characters, including hyphens (`-`), are not allowed and cause registration to fail. This patch updates the documentation to explicitly state these restrictions and highlights the existing invalid example "/l3fwd-power/stats" (in l3fwd-power app). Signed-off-by: Sivaprasad Tummala <sivaprasad.tummala@amd.com> Signed-off-by: 0-day Robot <robot@bytheb.org>
Reviewer's GuideThis PR refactors the debug test suite to replace direct fork()/wait logic with a recursive invocation via process_dup and environment variables, adds Windows support stubs, updates test registration, adjusts header usage and inlining, and enhances the telemetry guide with command name restrictions documentation. File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
WalkthroughThe changeset refactors test debug execution to use process duplication and environment-driven recursion instead of fork/wait, adds new test actions (test_panic, test_exit) and their declarations, adjusts a helper function to be inline, and updates telemetry documentation to clarify command name character constraints. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Tester
participant TestApp as Test App (parent)
participant Proc as process_dup
participant Child as Test App (child)
participant Env as Environment
Tester->>TestApp: run test_debug
Note over TestApp: Build test_args and EAL options
TestApp->>Env: Set RECURSIVE_ENV_VAR=<action>
TestApp->>Proc: spawn with test_args
Proc-->>TestApp: PID/handle
rect rgba(200,230,255,0.3)
Note over Child: On start, read RECURSIVE_ENV_VAR
Child->>Env: Read RECURSIVE_ENV_VAR
alt action == "test_panic"
Child->>Child: test_panic()
Note over Child: trigger panic path
else action == "test_exit"
Child->>Child: test_exit()
Note over Child: call rte_exit(exit_val)
else other
Child->>Child: normal test flow
end
end
Child-->>Proc: exit(code)
Proc-->>TestApp: exit status
TestApp-->>Tester: report result
Note over Tester,Child: On Windows: tests stubbed, return TEST_SKIPPED
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (5)
🧰 Additional context used🧬 Code graph analysis (3)app/test/test.h (1)
app/test/test.c (1)
app/test/test_debug.c (2)
🪛 ast-grep (0.39.5)app/test/test_debug.c[warning] 104-104: The (null-library-function-c) ⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
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}
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.
NOTE: This is an auto submission for "doc: document telemetry command name restrictions".
See "http://patchwork.dpdk.org/project/dpdk/list/?series=36295" for details.
Summary by Sourcery
Refactor the debug test suite to use a new process_dup helper for testing rte_panic and rte_exit in child processes, add cross‐platform support (skipping tests on Windows and handling FreeBSD prefixes), register test_panic and test_exit in the test runner, and document telemetry command name restrictions in the user guide
Enhancements:
Documentation:
Summary by CodeRabbit
New Features
Tests
Documentation