[PATCH v6] ethdev: support inline calculating masked item value
Bing Zhao
bingz at nvidia.com
Thu Oct 8 09:17:26 CEST 2026
No. The three errors in that AI review are not valid, and the warnings do not block this patch.
rte_flow_conv_item_spec() returns size_t, the number of bytes to copy. It does not return a negative errno. In rte_flow_conv_pattern() the local ret is also size_t, so if (ret < 0) is always false and cannot be the missing check the review describes. That copy pattern was already there for spec, last, and mask. This patch only ANDs the mask onto a buffer after that copy has produced a pointer.
The other notes are process nits:
The with_mask comment is already clear.
A release note belongs in the current release file, not release_25_03.rst. This master has no current notes file, and release_26_07.rst is gone, which is why that hunk was dropped.
RTE_FLOW_CONV_OP_PATTERN_MASKED is one new enumerator at the end of an existing enum. It cannot be tagged __rte_experimental, and existing callers of rte_flow_conv() are unchanged.
https://mails.dpdk.org/archives/test-report/2026-October/1057123.html
@stephen at networkplumber.org, FYI
> -----Original Message-----
> From: Bing Zhao <bingz at nvidia.com>
> Sent: Thursday, October 8, 2026 12:58 AM
> To: Slava Ovsiienko <viacheslavo at nvidia.com>; dev at dpdk.org; Raslan
> Darawsheh <rasland at nvidia.com>; stephen at networkplumber.org
> Cc: Ori Kam <orika at nvidia.com>; Dariusz Sosnowski <dsosnowski at nvidia.com>;
> Suanming Mou <suanmingm at nvidia.com>; Matan Azrad <matan at nvidia.com>; NBU-
> Contact-Thomas Monjalon (EXTERNAL) <thomas at monjalon.net>
> Subject: [PATCH v6] ethdev: support inline calculating masked item value
>
> External email: Use caution opening links or attachments
>
>
> 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 at nvidia.com>
> Acked-by: Dariusz Sosnowski <dsosnowski at nvidia.com>
> ---
> v3:
> - add test code
> - fix the issue found by AI
> v4: reabse on top of the main
> v5: handle some items separately and add test for them
> v6: resend the correct version instead of v5 with AI defects resolved
> ---
> app/test/test_ethdev_api.c | 123 +++++++++++++++++++++++++++++++++++++
> lib/ethdev/rte_flow.c | 55 +++++++++++++++--
> lib/ethdev/rte_flow.h | 13 ++++
> 3 files changed, 185 insertions(+), 6 deletions(-)
>
> diff --git a/app/test/test_ethdev_api.c b/app/test/test_ethdev_api.c index
> 00d6a5c614..ba3e785425 100644
> --- a/app/test/test_ethdev_api.c
> +++ b/app/test/test_ethdev_api.c
> @@ -2,8 +2,11 @@
> * Copyright (C) 2023, Advanced Micro Devices, Inc.
> */
>
> +#include <string.h>
> +
> #include <rte_log.h>
> #include <rte_ethdev.h>
> +#include <rte_flow.h>
>
> #include <rte_test.h>
> #include "test.h"
> @@ -15,6 +18,125 @@
> #define NUM_MBUF 1024
> #define MBUF_CACHE_SIZE 256
>
> +static int32_t
> +ethdev_api_flow_conv_pattern_masked(void)
> +{
> + const struct rte_flow_item_eth spec = {
> + .hdr.dst_addr.addr_bytes = { 0x01, 0x02, 0x03, 0x04, 0x05,
> 0x06 },
> + .hdr.src_addr.addr_bytes = { 0x0a, 0x0b, 0x0c, 0x0d, 0x0e,
> 0x0f },
> + .hdr.ether_type = RTE_BE16(0x1234),
> + };
> + const struct rte_flow_item_eth last = {
> + .hdr.dst_addr.addr_bytes = { 0x11, 0x12, 0x13, 0x14, 0x15,
> 0x16 },
> + .hdr.src_addr.addr_bytes = { 0x1a, 0x1b, 0x1c, 0x1d, 0x1e,
> 0x1f },
> + .hdr.ether_type = RTE_BE16(0x5678),
> + };
> + const struct rte_flow_item_eth mask = {
> + .hdr.dst_addr.addr_bytes = { 0xff, 0xff, 0x00, 0x00, 0xff,
> 0xff },
> + .hdr.src_addr.addr_bytes = { 0xff, 0x00, 0xff, 0x00, 0xff,
> 0x00 },
> + .hdr.ether_type = RTE_BE16(0xffff),
> + };
> + const uint8_t raw_pattern[] = { 0xaa, 0xbb, 0xcc, 0xdd };
> + const struct rte_flow_item_raw raw_spec = {
> + .relative = 1,
> + .search = 1,
> + .offset = 8,
> + .limit = 4,
> + .length = sizeof(raw_pattern),
> + .pattern = raw_pattern,
> + };
> + const struct rte_flow_item_raw raw_mask = {
> + .relative = 1,
> + .search = 0,
> + .reserved = 0x3fffffff,
> + .offset = 0xffffffff,
> + .limit = 0xffff,
> + .length = 0xffff,
> + .pattern = NULL,
> + };
> + const struct rte_flow_item pattern[] = {
> + {
> + .type = RTE_FLOW_ITEM_TYPE_ETH,
> + .spec = &spec,
> + .last = &last,
> + .mask = &mask,
> + },
> + {
> + .type = RTE_FLOW_ITEM_TYPE_RAW,
> + .spec = &raw_spec,
> + .mask = &raw_mask,
> + },
> + { .type = RTE_FLOW_ITEM_TYPE_END },
> + };
> + union {
> + struct rte_flow_item item;
> + struct rte_flow_item_eth eth;
> + struct rte_flow_item_raw raw;
> + double align;
> + uint8_t buf[256];
> + } dst;
> + const struct rte_flow_item *item;
> + const struct rte_flow_item_eth *conv_spec;
> + const struct rte_flow_item_eth *conv_last;
> + const struct rte_flow_item_raw *conv_raw_spec;
> + int ret;
> +
> + ret = rte_flow_conv(RTE_FLOW_CONV_OP_PATTERN_MASKED, NULL, 0,
> pattern, NULL);
> + TEST_ASSERT(ret > 0, "Masked pattern conversion size query
> failed");
> + TEST_ASSERT((size_t)ret <= sizeof(dst.buf),
> + "Masked pattern conversion needs too much storage");
> +
> + memset(&dst, 0, sizeof(dst));
> + ret = rte_flow_conv(RTE_FLOW_CONV_OP_PATTERN_MASKED, dst.buf,
> + sizeof(dst.buf), pattern, NULL);
> + TEST_ASSERT(ret > 0, "Masked pattern conversion failed");
> +
> + item = (const struct rte_flow_item *)dst.buf;
> + conv_spec = item[0].spec;
> + conv_last = item[0].last;
> + TEST_ASSERT_NOT_NULL(conv_spec, "Converted spec must be set");
> + TEST_ASSERT_NOT_NULL(conv_last, "Converted last must be set");
> +
> + TEST_ASSERT_EQUAL(conv_spec->hdr.dst_addr.addr_bytes[0], 0x01,
> + "Masked spec dst byte 0 mismatch");
> + TEST_ASSERT_EQUAL(conv_spec->hdr.dst_addr.addr_bytes[2], 0x00,
> + "Masked spec dst byte 2 mismatch");
> + TEST_ASSERT_EQUAL(conv_spec->hdr.src_addr.addr_bytes[1], 0x00,
> + "Masked spec src byte 1 mismatch");
> + TEST_ASSERT_EQUAL(conv_spec->hdr.ether_type, RTE_BE16(0x1234),
> + "Masked spec ether type mismatch");
> + TEST_ASSERT_EQUAL(conv_last->hdr.dst_addr.addr_bytes[0], 0x11,
> + "Masked last dst byte 0 mismatch");
> + TEST_ASSERT_EQUAL(conv_last->hdr.dst_addr.addr_bytes[2], 0x00,
> + "Masked last dst byte 2 mismatch");
> + TEST_ASSERT_EQUAL(conv_last->hdr.src_addr.addr_bytes[1], 0x00,
> + "Masked last src byte 1 mismatch");
> + TEST_ASSERT_EQUAL(conv_last->hdr.ether_type, RTE_BE16(0x5678),
> + "Masked last ether type mismatch");
> +
> + conv_raw_spec = item[1].spec;
> + TEST_ASSERT_NOT_NULL(conv_raw_spec, "Converted RAW spec must be
> set");
> + TEST_ASSERT_NOT_NULL(conv_raw_spec->pattern,
> + "Converted RAW pattern pointer must be set");
> + TEST_ASSERT(conv_raw_spec->pattern != raw_pattern,
> + "Converted RAW pattern must be deep-copied");
> + TEST_ASSERT_EQUAL(conv_raw_spec->relative, 1,
> + "Masked RAW relative mismatch");
> + TEST_ASSERT_EQUAL(conv_raw_spec->search, 0,
> + "Masked RAW search mismatch");
> + TEST_ASSERT_EQUAL(conv_raw_spec->offset, raw_spec.offset,
> + "Masked RAW offset mismatch");
> + TEST_ASSERT_EQUAL(conv_raw_spec->limit, raw_spec.limit,
> + "Masked RAW limit mismatch");
> + TEST_ASSERT_EQUAL(conv_raw_spec->length, raw_spec.length,
> + "Masked RAW length mismatch");
> + TEST_ASSERT_BUFFERS_ARE_EQUAL(conv_raw_spec->pattern, raw_pattern,
> + sizeof(raw_pattern),
> + "Converted RAW pattern mismatch");
> +
> + return TEST_SUCCESS;
> +}
> +
> static int32_t
> ethdev_api_queue_status(void)
> {
> @@ -167,6 +289,7 @@ static struct unit_test_suite ethdev_api_testsuite = {
> .setup = NULL,
> .teardown = NULL,
> .unit_test_cases = {
> + TEST_CASE(ethdev_api_flow_conv_pattern_masked),
> TEST_CASE(ethdev_api_queue_status),
> /* TODO: Add deferred_start queue status test */
> TEST_CASES_END() /**< NULL terminate unit test array */
> diff --git a/lib/ethdev/rte_flow.c b/lib/ethdev/rte_flow.c index
> ca2f85c3fa..e4efcbf10f 100644
> --- a/lib/ethdev/rte_flow.c
> +++ b/lib/ethdev/rte_flow.c
> @@ -175,6 +175,23 @@ static const struct rte_flow_desc_data
> rte_flow_desc_item[] = {
> MK_FLOW_ITEM(COMPARE, sizeof(struct rte_flow_item_compare)), };
>
> +static inline size_t
> +rte_flow_conv_item_mask_size(const struct rte_flow_item *item) {
> + if ((int)item->type < 0)
> + return sizeof(void *);
> + switch (item->type) {
> + case RTE_FLOW_ITEM_TYPE_RAW:
> + return offsetof(struct rte_flow_item_raw, pattern);
> + case RTE_FLOW_ITEM_TYPE_GENEVE_OPT:
> + return offsetof(struct rte_flow_item_geneve_opt, data);
> + default:
> + if (rte_flow_desc_item[item->type].desc_fn != NULL)
> + return 0;
> + return rte_flow_desc_item[item->type].size;
> + }
> +}
> +
> /** Generate flow_action[] entry. */
> #define MK_FLOW_ACTION(t, s) \
> [RTE_FLOW_ACTION_TYPE_ ## t] = { \ @@ -797,6 +814,8 @@
> rte_flow_conv_action_conf(void *buf, const size_t size,
> * RTE_FLOW_ITEM_TYPE_END is encountered.
> * @param[out] error
> * Perform verbose error reporting if not NULL.
> + * @param[in] with_mask
> + * If true, @p src mask will be applied to spec and last.
> *
> * @return
> * A positive value representing the number of bytes needed to store
> @@ -809,12 +828,13 @@ rte_flow_conv_pattern(struct rte_flow_item *dst,
> const size_t size,
> const struct rte_flow_item *src,
> unsigned int num,
> + bool with_mask,
> struct rte_flow_error *error) {
> uintptr_t data = (uintptr_t)dst;
> size_t off;
> size_t ret;
> - unsigned int i;
> + unsigned int i, j;
>
> for (i = 0, off = 0; !num || i != num; ++i, ++src, ++dst) {
> /**
> @@ -838,15 +858,27 @@ rte_flow_conv_pattern(struct rte_flow_item *dst,
> src -= num;
> dst -= num;
> do {
> + uint8_t *c_spec = NULL, *c_last = NULL;
> + const uint8_t *mask = src->mask;
> + size_t item_mask_size = mask ?
> + rte_flow_conv_item_mask_size(src) : 0;
> +
> if (src->spec) {
> off = RTE_ALIGN_CEIL(off, sizeof(double));
> ret = rte_flow_conv_item_spec
> ((void *)(data + off),
> size > off ? size - off : 0, src,
> RTE_FLOW_CONV_ITEM_SPEC);
> - if (size && size >= off + ret)
> + if (size && size >= off + ret) {
> dst->spec = (void *)(data + off);
> + c_spec = (uint8_t *)(data + off);
> + }
> off += ret;
> + if (with_mask && c_spec && mask) {
> + size_t mask_size = RTE_MIN(ret,
> + item_mask_size);
> +
> + for (j = 0; j < mask_size; j++)
> + c_spec[j] &= mask[j];
> + }
>
> }
> if (src->last) {
> @@ -855,9 +887,17 @@ rte_flow_conv_pattern(struct rte_flow_item *dst,
> ((void *)(data + off),
> size > off ? size - off : 0, src,
> RTE_FLOW_CONV_ITEM_LAST);
> - if (size && size >= off + ret)
> + if (size && size >= off + ret) {
> dst->last = (void *)(data + off);
> + c_last = (uint8_t *)(data + off);
> + }
> off += ret;
> + if (with_mask && c_last && mask) {
> + size_t mask_size = RTE_MIN(ret,
> + item_mask_size);
> +
> + for (j = 0; j < mask_size; j++)
> + c_last[j] &= mask[j];
> + }
> }
> if (src->mask) {
> off = RTE_ALIGN_CEIL(off, sizeof(double)); @@ -
> 1004,7 +1044,7 @@ rte_flow_conv_rule(struct rte_flow_conv_rule *dst,
> off = RTE_ALIGN_CEIL(off, sizeof(double));
> ret = rte_flow_conv_pattern((void *)((uintptr_t)dst +
> off),
> size > off ? size - off : 0,
> - src->pattern_ro, 0, error);
> + src->pattern_ro, 0, false,
> + error);
> if (ret < 0)
> return ret;
> if (size && size >= off + (size_t)ret) @@ -1104,7 +1144,7
> @@ rte_flow_conv(enum rte_flow_conv_op op,
> ret = sizeof(*attr);
> break;
> case RTE_FLOW_CONV_OP_ITEM:
> - ret = rte_flow_conv_pattern(dst, size, src, 1, error);
> + ret = rte_flow_conv_pattern(dst, size, src, 1, false,
> + error);
> break;
> case RTE_FLOW_CONV_OP_ITEM_MASK:
> item = src;
> @@ -1119,7 +1159,7 @@ rte_flow_conv(enum rte_flow_conv_op op,
> ret = rte_flow_conv_actions(dst, size, src, 1, error);
> break;
> case RTE_FLOW_CONV_OP_PATTERN:
> - ret = rte_flow_conv_pattern(dst, size, src, 0, error);
> + ret = rte_flow_conv_pattern(dst, size, src, 0, false,
> + error);
> break;
> case RTE_FLOW_CONV_OP_ACTIONS:
> ret = rte_flow_conv_actions(dst, size, src, 0, error); @@
> -1139,6 +1179,9 @@ rte_flow_conv(enum rte_flow_conv_op op,
> case RTE_FLOW_CONV_OP_ACTION_NAME_PTR:
> ret = rte_flow_conv_name(1, 1, dst, size, src, error);
> break;
> + case RTE_FLOW_CONV_OP_PATTERN_MASKED:
> + ret = rte_flow_conv_pattern(dst, size, src, 0, true,
> error);
> + break;
> default:
> ret = rte_flow_error_set
> (error, ENOTSUP, RTE_FLOW_ERROR_TYPE_UNSPECIFIED, NULL,
> diff --git a/lib/ethdev/rte_flow.h b/lib/ethdev/rte_flow.h index
> f864578f80..2261d1ea06 100644
> --- a/lib/ethdev/rte_flow.h
> +++ b/lib/ethdev/rte_flow.h
> @@ -4544,6 +4544,19 @@ enum rte_flow_conv_op {
> * @code const char ** @endcode
> */
> RTE_FLOW_CONV_OP_ACTION_NAME_PTR,
> +
> + /**
> + * Convert an entire pattern.
> + *
> + * Duplicates all pattern items at once, applying each source item
> mask
> + * to its copied specification and range.
> + *
> + * - @p src type:
> + * @code const struct rte_flow_item * @endcode
> + * - @p dst type:
> + * @code struct rte_flow_item * @endcode
> + */
> + RTE_FLOW_CONV_OP_PATTERN_MASKED,
> };
>
> /**
> --
> 2.43.0
More information about the dev
mailing list