Repository navigation
(14) dpdk: RX metadata - #1877
daniel-noland wants to merge 1 commit into
Conversation
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configuration
You can disable this status message by setting the Use the checkbox below for a quick retry:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info
📝 Walkthrough
Priority: ⬇️ Low Merge Risk: ⚪ Minimal · up to
|
| Check name | Status | Explanation |
|---|---|---|
| Docstring Coverage | ✅ Passed | Docstring coverage is 83.33% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 6 functions across 1 files. |
| Linked Issues check | ✅ Passed | Check skipped because no linked issues were found for this pull request. |
| Out of Scope Changes check | ✅ Passed | Check skipped because no linked issues were found for this pull request. |
| Title check | ✅ Passed | The title clearly identifies the DPDK receive metadata change, which is the main focus of the pull request. |
| Description check | ✅ Passed | The description explains the exposed mbuf receive values, metadata registration, and validation related to the changeset. |
- Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
Comment @coderabbitai help to get the list of available commands.
| pub fn ol_flags(&self) -> u64 { | ||
| // SAFETY: `self.raw` is a live mbuf for the lifetime of `&self`. | ||
| unsafe { self.raw.as_ref() }.ol_flags | ||
| } |
There was a problem hiding this comment.
this should likely return a stronger type. A bitfield enum or some such
There was a problem hiding this comment.
yeah, was going to ask. That's fine for now.
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
daniel-noland
added
the
area/dpdk
bc9de15 to
56039b8
Compare
56039b8 to
996c9bc
Compare
113667a to
2163a66
Compare
ab54519 to
4d77831
Compare
2163a66 to
984402b
Compare
afe8502 to
d420126
Compare
c498514 to
b4ef0da
Compare
Fredi-raspall
left a comment
Fredi-raspall
left a comment
There was a problem hiding this comment.
I can't spot any issue with this PR, other than the lack for a type to keep the metadata flags and hash. My main question is how we're going to use this info, but that's out of the scope of the PR.
| pub fn ol_flags(&self) -> u64 { | ||
| // SAFETY: `self.raw` is a live mbuf for the lifetime of `&self`. | ||
| unsafe { self.raw.as_ref() }.ol_flags | ||
| } |
There was a problem hiding this comment.
yeah, was going to ask. That's fine for now.
d420126 to
f4e8ced
Compare
bb4fb47 to
6ae5b96
Compare
1c6c248 to
cba901d
Compare
77e730c to
9c442cd
Compare
cba901d to
f071e76
Compare
| //! | ||
| //! RSS, MARK and META have independent validity flags: | ||
| //! | ||
| //! | Value | Storage | Software access | | ||
| //! | --- | --- | --- | | ||
| //! | RSS | `hash.rss`, overlapping `hash.fdir.lo` | [`Mbuf::rss_hash`] | | ||
| //! | MARK | `hash.fdir.hi` | [`Mbuf::rx_mark`] | | ||
| //! | META | Registered dynamic `u32` field | [`Mbuf::rx_meta`] | | ||
| //! | TAG | Indexed hardware steering registers | No mbuf field | | ||
| //! | ||
| //! [RSS and MARK can coexist](https://doc.dpdk.org/api/structrte__flow__action__rss.html). | ||
| //! META uses separate mbuf storage. Supported combinations, widths and cross-domain propagation | ||
| //! still depend on the PMD and its configuration. TAG has a 32-bit API value; usable indices and | ||
| //! register availability are device-specific. It is not delivered to software, and applications | ||
| //! must not assume it survives a software queue round trip or a hardware hairpin boundary. |
There was a problem hiding this comment.
reformat table
9c442cd to
5349632
Compare
f071e76 to
65a9eae
Compare
65a9eae to
aed90ca
Compare
5349632 to
7e10438
Compare
aed90ca to
6c9cf2a
Compare
7e10438 to
0c8e642
Compare
6c9cf2a to
de95cd8
Compare
0c8e642 to
dfa429f
Compare
de95cd8 to
68605fa
Compare
b1b5db5 to
6cb84ce
Compare
68605fa to
8321a55
Compare
Expose optional RSS, MARK and META values using their validity flags. Register META during EAL initialization, before exposing pools or queues; terminate startup if registration fails. Document field coexistence and PMD limits. Test metadata against native DPDK mbufs, including zero values, copies and recycled buffers. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Signed-off-by: Daniel Noland <daniel@githedgehog.com>
8321a55 to
8a76f6d
Compare
6cb84ce to
85d5b3e
Compare
| if ret != 0 { | ||
| Eal::fatal_error(format!( | ||
| "failed to register RX metadata: {}", | ||
| errno::ErrorCode::parse_i32(ret) |
There was a problem hiding this comment.
Note
AI-assisted review: This comment was drafted with Claude and reviewed by @mvachhar.
Small one: rte_flow_dynf_metadata_register returns -rte_errno on failure (lib/ethdev/rte_flow.c), so ret is negative here and ErrorCode::parse_i32 reports it as an unknown errno (e.g. -12) rather than ENOMEM. Negating it gives the readable error:
| errno::ErrorCode::parse_i32(ret) | |
| errno::ErrorCode::parse_i32(-ret) |
3 participants
Expose mbuf offload flags and optional RSS, MARK and META values. Register META during EAL startup, before pools or workers become available, and fail startup if registration fails. All binding mechanics remain inside
dpdk.Preserve the distinction between an absent value and a reported zero. Document RSS/MARK coexistence, mlx5 META zero semantics, and device-dependent widths and TAG availability.
Validation: native DPDK tests pass (148 unit tests, the lifecycle integration test, and doctests). New tests cover validity flags, zero values, simultaneous RSS/MARK delivery, deep copies, and recycled buffers. Clippy passes for
dpdkand the dataplane with warnings denied. Physical-NIC metadata delivery remains untested.