On Tue, 13 Oct 2026 16:49:50 +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]> > Reviewed-by: Dariusz Sosnowski <[email protected]> > --- 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.
