On Wed, 7 Oct 2026 19:57:44 +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]> > Acked-by: Dariusz Sosnowski <[email protected]> > --- Existing AI review done by CI is useless at this point... Running a more detailed review does find some things. Review: [PATCH v6] ethdev: support inline calculating masked item value Error ----- 1. NULL mask leaves spec/last unmasked. rte_flow semantics: a NULL item mask means the default mask (rte_flow_item_<name>_mask) applies, not "match all bits". Here item_mask_size is 0 when src->mask is NULL, so spec and last are copied raw. For ETH the default mask is all ones so nothing shows, but for IPV4 (default mask covers only src/dst addr) tos, ttl, etc. come back unmasked. A driver relying on the new op to get "masked values" gets wrong ones. rte_flow.c has no type -> default mask table, so either add one to rte_flow_desc_data, or document that a NULL mask is not applied and the caller must handle it (see doc text in Warning 3). Warning ------- 1. PMD private items get their opaque data ANDed. rte_flow_conv_item_mask_size() returns sizeof(void *) for (int)type < 0, so the first pointer-size bytes of a private spec are ANDed with the first bytes of its mask. ethdev has no idea what those bytes mean. Leave them alone: if ((int)item->type < 0) return 0; 2. Variable part of RAW and GENEVE_OPT is not masked. item_mask_size stops at offsetof(pattern) / offsetof(data), so the copied pattern bytes and option data are never ANDed with mask->pattern / mask->data. The same holds for FLEX (desc_fn returns 0). Either mask them or document it; the current comment says the mask is applied to the spec, which is not true for these. 3. Doxygen for RTE_FLOW_CONV_OP_PATTERN_MASKED is a copy of OP_PATTERN and does not describe the limits above. Suggest: /** * Convert an entire pattern, applying item masks. * * Same as RTE_FLOW_CONV_OP_PATTERN, except each copied spec * and last is ANDed with the item mask. Not masked: * - items with a NULL mask (default mask is not applied); * - RAW pattern and GENEVE_OPT data contents; * - FLEX and PMD private items. * * - @p src type: * @code const struct rte_flow_item * @endcode * - @p dst type: * @code struct rte_flow_item * @endcode */ 4. Missing release notes. New rte_flow_conv() op is a new API and needs an entry in doc/guides/rel_notes/release_26_11.rst. 5. Commit message is hard to parse ("due to the reason of speed of light rule insertion rate", "support usage of the following fix" refers to a patch not in this series). Suggested body: Add RTE_FLOW_CONV_OP_PATTERN_MASKED to rte_flow_conv(). It copies a pattern like RTE_FLOW_CONV_OP_PATTERN and ANDs each item spec and last with the item mask. Drivers using the async flow API copy patterns on the rule insertion path and need masked values. The copied spec and last are const, so masking them in the driver needs extra memory and per-item length handling. Doing it during the conversion avoids both. Info ---- 1. No in-tree user. Please send the mlx5 consumer in the same series so the API is exercised and reviewed in context. 2. Spec and last masking loops are duplicated, and item_mask_size is computed even when with_mask is false. Factor out: static void rte_flow_conv_apply_mask(uint8_t *buf, const uint8_t *mask, size_t len) { size_t i; for (i = 0; i < len; i++) buf[i] &= mask[i]; } and compute the size only under with_mask && mask. 3. Test does not cover a NULL-mask item or an item with a non-trivial default mask (e.g. IPV4). Add one once Error 1 is settled so the chosen behaviour is locked in.
