From: Stephen Hemminger <stephen@networkplumber.org>
To: Bing Zhao <bingz@nvidia.com>
Cc: <viacheslavo@nvidia.com>, <dev@dpdk.org>, <rasland@nvidia.com>,
<orika@nvidia.com>, <dsosnowski@nvidia.com>,
<suanmingm@nvidia.com>, <matan@nvidia.com>, <thomas@monjalon.net>
Subject: Re: [PATCH v4] ethdev: support inline calculating masked item value
Date: Mon, 8 Jun 2026 08:45:58 -0700 [thread overview]
Message-ID: <20260608084558.4dd4c07c@phoenix.local> (raw)
In-Reply-To: <20260603092805.9837-1-bingz@nvidia.com>
On Wed, 3 Jun 2026 12:28:05 +0300
Bing Zhao <bingz@nvidia.com> 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 <bingz@nvidia.com>
> ---
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 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.
prev parent reply other threads:[~2026-06-08 15:46 UTC|newest]
Thread overview: 14+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-11-17 7:54 [PATCH 1/2] lib/ethdev: support inline calculating masked item value Bing Zhao
2025-11-17 17:25 ` Stephen Hemminger
2026-02-05 8:45 ` Bing Zhao
2026-02-05 16:42 ` Stephen Hemminger
2026-02-09 4:23 ` Bing Zhao
2026-02-09 14:17 ` Bing Zhao
2026-02-09 18:58 ` Stephen Hemminger
2026-02-09 21:46 ` Thomas Monjalon
2026-02-13 13:31 ` [PATCH v2] " Bing Zhao
2026-02-13 19:50 ` Stephen Hemminger
2026-06-03 8:19 ` [PATCH v3] ethdev: " Bing Zhao
2026-06-03 9:28 ` [PATCH v4] " Bing Zhao
2026-06-08 14:49 ` Dariusz Sosnowski
2026-06-08 15:45 ` Stephen Hemminger [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20260608084558.4dd4c07c@phoenix.local \
--to=stephen@networkplumber.org \
--cc=bingz@nvidia.com \
--cc=dev@dpdk.org \
--cc=dsosnowski@nvidia.com \
--cc=matan@nvidia.com \
--cc=orika@nvidia.com \
--cc=rasland@nvidia.com \
--cc=suanmingm@nvidia.com \
--cc=thomas@monjalon.net \
--cc=viacheslavo@nvidia.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox