Hi, Let me check and update with v5.
> -----Original Message----- > From: Stephen Hemminger <[email protected]> > Sent: Monday, June 8, 2026 11:46 PM > To: Bing Zhao <[email protected]> > Cc: Slava Ovsiienko <[email protected]>; [email protected]; Raslan > Darawsheh <[email protected]>; Ori Kam <[email protected]>; Dariusz > Sosnowski <[email protected]>; Suanming Mou <[email protected]>; > Matan Azrad <[email protected]>; NBU-Contact-Thomas Monjalon (EXTERNAL) > <[email protected]> > Subject: Re: [PATCH v4] ethdev: support inline calculating masked item > value > > External email: Use caution opening links or attachments > > > On Wed, 3 Jun 2026 12:28:05 +0300 > Bing Zhao <[email protected]> wrote: > > > In the asynchronous API definition and some drivers, the rte_flow_item > > spec value may not be calculated by the driver due to the reason of > > speed of light rule insertion rate and sometimes the input parameters > > will be copied and changed internally. > > > > After copying, the spec and last will be protected by the keyword > > const and cannot be changed in the code itself. And also the driver > > needs some extra memory to do the calculation and extra conditions to > > understand the length of each item spec. This is not efficient. > > > > To solve the issue and support usage of the following fix, a new OP > > was introduced to calculate the spec and last values after applying > > the mask inline. > > > > Signed-off-by: Bing Zhao <[email protected]> > > --- > > More detailed AI review found some things that still need addressing. > > On Wed, 3 Jun 2026 12:28:05 +0300, Bing Zhao wrote: > > Subject: [PATCH v4] ethdev: support inline calculating masked item > > value > > Error: byte-wise masking corrupts embedded pointers in deep-copy item > types (RAW, FLEX, GENEVE_OPT). > > In rte_flow_conv_pattern(), the new mask application runs over the fixed > item struct: > > size_t item_mask_size = mask ? rte_flow_conv_item_mask_size(src) : > 0; > ... > size_t mask_size = RTE_MIN(ret, item_mask_size); > > for (j = 0; j < mask_size; j++) > c_spec[j] &= mask[j]; > > item_mask_size is rte_flow_desc_item[type].size, the size of the fixed > item struct. For RTE_FLOW_ITEM_TYPE_RAW, FLEX, and GENEVE_OPT, that fixed > struct ends in an embedded pointer that rte_flow_conv_item_spec() has just > populated to point at the deep-copied trailing data > (rte_flow_item_raw.pattern, rte_flow_item_flex.pattern, > rte_flow_item_geneve_opt.data). Because the masked range covers the whole > fixed struct, the loop ANDs the bytes of that pointer with the mask's > corresponding bytes (typically a NULL mask pointer), zeroing or garbling > it. > > The converted item's pattern/data pointer is clobbered while the copied > payload it should reference is left unreachable. A consumer that follows > conv->pattern then dereferences NULL or a corrupt address. Plain value > conv->items > (eth, ipv4, ...) are unaffected; only the deep-copy item types break, > which is exactly what the test does not exercise. > > Suggested fix: do not blind-mask the entire fixed struct for items that > carry an embedded pointer / desc_fn deep copy. Either skip masking when > rte_flow_desc_item[type].desc_fn != NULL, or mask only the leading plain- > data region and leave the pointer field (and trailing copied bytes) > intact. > > Warning: the new test validates only an ETH pattern, so the > RAW/FLEX/GENEVE_OPT path above is untested. A RAW item case would have > surfaced the pointer corruption. > > Info: the Doxygen block for RTE_FLOW_CONV_OP_PATTERN_MASKED uses @p mask, > @p spec, @p last, but those are item fields, not parameters of the op; the > neighboring enum entries only document the @p src / @p dst types.
