Re: [PATCH v7] ethdev: support inline calculating masked item value

[Stephen Hemminger <stephen@networkplumber.org>]

On Tue, 13 Oct 2026 16:49:50 +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>
> Reviewed-by: Dariusz Sosnowski <dsosnowski@nvidia.com>
> ---

Semi-automated AI review had some feedback.
I assume mlx5 will come as followon so complaints about just this patch
are not really valid. And not concerned about its complaints
about going over flow twice when copying.

[PATCH v7] ethdev: support inline calculating masked item value

Applies to main, builds with -Dwerror=true.

Warning

1. GENEVE_OPT option_len is masked, data length is not.

rte_flow_conv_item_mask_size() returns offsetof(..., data), so
option_len is ANDed with the mask. The default
rte_flow_item_geneve_opt_mask has option_len = 0, while the deep
copy of data is sized from the unmasked spec->option_len. The output
is a spec with option_len == 0 pointing at option_len * 4 bytes of
data; the new test asserts exactly that. mlx5 sizes the option data
from spec->option_len (mlx5_flow.c, data_len = spec->option_len * 4),
so a driver switched to this op loses the option length.

option_len is a length, not a match value. Leave it unmasked:

	case RTE_FLOW_ITEM_TYPE_GENEVE_OPT:
		return offsetof(struct rte_flow_item_geneve_opt,
				option_len);

and change the test to assert option_len is preserved.

2. TAG index is a selector, same reasoning as COMPARE.

rte_flow_item_tag.index picks the register, it is not matched
against packet data. mlx5 takes it from the unmasked spec on purpose
(mlx5_flow_dv.c: "When set mask, the index should be from spec").
With a template mask that has index == 0, this op returns index 0,
i.e. a different tag. Exclude it:

	case RTE_FLOW_ITEM_TYPE_TAG:
		return offsetof(struct rte_flow_item_tag, index);

Update the "Not masked" list in rte_flow.h, the comment above
rte_flow_conv_item_mask_size() and the rte_flow_conv_pattern()
doxygen to match, and add a TAG case to the test.

3. No user of the new op.

The commit message refers to "the following fix", but nothing in
this submission or in tree uses RTE_FLOW_CONV_OP_PATTERN_MASKED.
Send it as a series with the driver change that consumes it, so the
semantics above can be checked against a real caller.

4. Commit message needs rewriting. Suggested body:

  Drivers translating async flow rules get const spec/last and must
  copy each item to apply the mask, which needs per-item knowledge
  of the spec size.

  Add RTE_FLOW_CONV_OP_PATTERN_MASKED to rte_flow_conv(). It copies
  the pattern like RTE_FLOW_CONV_OP_PATTERN and ANDs each copied
  spec and last with the item mask. Pointed-to data (RAW pattern,
  GENEVE_OPT data), selector fields, FLEX, COMPARE and PMD private
  items are not masked.

Info

5. RAW pattern bytes are deep-copied into the writable output buffer
and spec.length is already masked, so masking the copied pattern
with mask->pattern for that length is straightforward. If it is left
out on purpose, say why in the commit message.

6. The insertion-rate argument needs numbers. rte_flow_conv() walks
the pattern twice (validate, copy) and masks byte by byte; show it
is not slower than the driver-local copy it replaces, or drop the
claim.