On Tue, 4 Aug 2026 10:13:14 +0200 Mattias Rönnblom <[email protected]> wrote:
> From: Mattias Rönnblom <[email protected]> > > The objective of this series is to make rte_ecpri.h a complete and > correct description of the eCPRI V2.0 message headers, usable for > parsing and building messages, and not only for matching on a few > header fields with rte_flow, which is what it was added for. > > The first two patches fix wire format bugs in rte_ecpri.h: missing > fields in the One-Way Delay Measurement message header, and padding in > the Remote Reset one. Both change struct sizes, hence the release note > entries and the lack of a stable tag. > > The rest completes the eCPRI V2.0 coverage of the header. > > Mattias Rönnblom (5): > net: fix eCPRI delay measurement message header > net: fix eCPRI remote reset message header size > net: add missing eCPRI message field values > net: allow byte swapping eCPRI remote memory access > net: add eCPRI IWF message headers > > doc/guides/rel_notes/release_26_11.rst | 8 ++ > lib/net/rte_ecpri.h | 164 +++++++++++++++++++++++-- > 2 files changed, 159 insertions(+), 13 deletions(-) More detailed AI review (the CI one is really pretty dumb). TLDR: missing/lack of deprecation notices. Applied cleanly on c1a46b9 (26.11.0-rc0). Built with -Dwerror=true and net/dpaa2 + net/bnxt enabled (the two in-tree consumers of the changed structs) plus testpmd: no warnings. Per-commit compile of dpaa2_flow.c, ulp_rte_parser.c and cmdline_flow.c at each of the five commits is also clean, so the series is bisect safe. Measured layouts after the series: rte_ecpri_msg_delay_measure size=20 align=1 msr_id=0 act_type=1 ts_sec=2 ts_nsec=8 comp_val=12 rte_ecpri_msg_remote_reset size= 3 align=1 rst_id=0 rst_op=2 rte_ecpri_msg_rm_access size=12 align=4 addr=4 length=10 rte_ecpri_msg_iwf_up size= 9 align=1 rte_ecpri_msg_iwf_opt size= 4 align=2 rte_ecpri_msg_iwf_map size= 4 align=2 rte_ecpri_msg_iwf_dctrl size=12 align=4 rte_ecpri_combined_msg_hdr size=24 align=4 Every size and offset claim in the commit messages and the release notes holds, including "the layout, the size and the field access are all unchanged" in 4/5 and "keeps its size" in 5/5. dummy[5] bounds the union exactly again. The only in-tree users of dummy[] touch dummy[0], and the offsetof() uses in dpaa2 land on unchanged offsets, so nothing breaks at source level. The Fixes tag is correct: d164c609e70b is the commit that added lib/librte_net/rte_ecpri.h along with the flow item. Patch 1: net: fix eCPRI delay measurement message header Error: ABI break with no deprecation notice. There is no eCPRI entry in doc/guides/rel_notes/deprecation.rst, neither in this series nor in tree. abi_policy.rst is explicit that the .11 breakage window "is *not* permission to circumvent the other aspects of the procedures to make ABI changes ... 3 ACKs of the requirement to break the ABI and the observance of a deprecation notice are still mandatory". The ABI Changes template in the release notes says the same thing ("which was announced in the previous releases"). As posted this targets 27.11, not 26.11, unless the techboard grants an exception. Worth spelling out the concrete breakage in the notice, because this is not just a formality. rte_flow_conv_copy() uses rte_flow_desc_item[].size, which is sizeof(struct rte_flow_item_ecpri). That becomes 24 bytes, so a new library memcpy()s 24 bytes out of a 16-byte spec allocated by an application built against the old header, an 8-byte out-of-bounds read. All existing field offsets are preserved, so that is the specific hazard to cite. Warning: the reason for the missing stable tag is only in the cover letter, which is not preserved in git history. Someone doing LTS triage later sees a Fixes tag with no Cc: [email protected] and no explanation. Please move that reasoning into the commit message, and say plainly that the bug stays live on the LTS branches because the fix cannot be backported. Info: the Compensation Value is documented here as "in units of 2^-16 ns" while delay_a/delay_b in 5/5 are documented as "in 1/16 ns". Those differ by a factor of 4096, so one of them looks like a typo. Info: please add static_assert on the size, and ideally the alignment, of the packed structs. rte_ether.h already does this: static_assert(sizeof(struct rte_ether_hdr) == 14, ...); static_assert(alignof(struct rte_ether_hdr) == 2, ...); Three structs in this series now depend on __rte_packed_begin producing 20, 3 and 9 bytes respectively, and MSVC handles pragma pack and uint8_t bitfields on its own terms. There is no eCPRI test anywhere in app/test, so the wire layout is otherwise entirely unverified. This is the addition I would most like to see. Patch 2: net: fix eCPRI remote reset message header size Error: same missing deprecation notice as 1/5. Warning: same stable tag rationale as 1/5. Patch 3: net: add missing eCPRI message field values Warning: eleven new macros in an installed header with no release note. One "New Features" entry covering the eCPRI V2.0 header completion, for this patch and 5/5 together, would be enough. Patch 5: net: add eCPRI IWF message headers Warning: four new public structs and six new macros with no release note. See 3/5. Info: I could not verify the line_rate:5 / res:1 / s:1 / f:1 bit ordering in rte_ecpri_msg_iwf_up against eCPRI V2.0 figure 31A. Please confirm that byte. The uint8_t bitfields themselves are fine, rte_geneve.h and rte_gtp.h set the precedent. Info: 5/5 says the combined header keeps its size but does not say why iwf_up is packed while the other three IWF structs are not. Without packing it would be 12 bytes rather than 9. A sentence in the commit message would save the next reader the arithmetic.
