* [PATCH 1/2] lib/ethdev: support inline calculating masked item value
@ 2025-11-17 7:54 Bing Zhao
2025-11-17 17:25 ` Stephen Hemminger
` (2 more replies)
0 siblings, 3 replies; 12+ messages in thread
From: Bing Zhao @ 2025-11-17 7:54 UTC (permalink / raw)
To: viacheslavo, dev, rasland; +Cc: orika, dsosnowski, suanmingm, matan, thomas
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>
---
lib/ethdev/rte_flow.c | 29 +++++++++++++++++++++++------
lib/ethdev/rte_flow.h | 12 ++++++++++++
2 files changed, 35 insertions(+), 6 deletions(-)
diff --git a/lib/ethdev/rte_flow.c b/lib/ethdev/rte_flow.c
index fe8f43caff..b94f0e304d 100644
--- a/lib/ethdev/rte_flow.c
+++ b/lib/ethdev/rte_flow.c
@@ -831,6 +831,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
@@ -843,12 +845,14 @@ 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;
+ uint8_t *c_spec = NULL, *c_last = NULL;
for (i = 0, off = 0; !num || i != num; ++i, ++src, ++dst) {
/**
@@ -878,9 +882,14 @@ rte_flow_conv_pattern(struct rte_flow_item *dst,
((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 && src->mask)
+ for (j = 0; j < ret; j++)
+ *(c_spec + j) &= *((const uint8_t *)src->mask + j);
}
if (src->last) {
@@ -889,9 +898,14 @@ 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 && src->mask)
+ for (j = 0; j < ret; j++)
+ *(c_last + j) &= *((const uint8_t *)src->mask + j);
}
if (src->mask) {
off = RTE_ALIGN_CEIL(off, sizeof(double));
@@ -1038,7 +1052,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)
@@ -1139,7 +1153,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;
@@ -1154,7 +1168,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);
@@ -1174,6 +1188,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 3d2ccdeb92..85acb26752 100644
--- a/lib/ethdev/rte_flow.h
+++ b/lib/ethdev/rte_flow.h
@@ -4559,6 +4559,18 @@ 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 @p mask to @p spec and @p spec.
+ *
+ * - @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.34.1
^ permalink raw reply related [flat|nested] 12+ messages in thread* Re: [PATCH 1/2] lib/ethdev: support inline calculating masked item value 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-13 13:31 ` [PATCH v2] " Bing Zhao 2 siblings, 1 reply; 12+ messages in thread From: Stephen Hemminger @ 2025-11-17 17:25 UTC (permalink / raw) To: Bing Zhao Cc: viacheslavo, dev, rasland, orika, dsosnowski, suanmingm, matan, thomas On Mon, 17 Nov 2025 09:54:07 +0200 Bing Zhao <bingz@nvidia.com> wrote: > diff --git a/lib/ethdev/rte_flow.c b/lib/ethdev/rte_flow.c > index fe8f43caff..b94f0e304d 100644 > --- a/lib/ethdev/rte_flow.c > +++ b/lib/ethdev/rte_flow.c > @@ -831,6 +831,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 Sorry, this patch is submitted too late to make it into 25.11 release. Missing any release note. This would be a good example of where to use API versioning in a later release. ^ permalink raw reply [flat|nested] 12+ messages in thread
* RE: [PATCH 1/2] lib/ethdev: support inline calculating masked item value 2025-11-17 17:25 ` Stephen Hemminger @ 2026-02-05 8:45 ` Bing Zhao 0 siblings, 0 replies; 12+ messages in thread From: Bing Zhao @ 2026-02-05 8:45 UTC (permalink / raw) To: Stephen Hemminger Cc: Slava Ovsiienko, dev@dpdk.org, Raslan Darawsheh, Ori Kam, Dariusz Sosnowski, Suanming Mou, Matan Azrad, NBU-Contact-Thomas Monjalon (EXTERNAL) Hi Stephen, Thanks for the comments. What do you mean by missing release notes? Should I add some description of the new conversion type? It will not break the current usage of any user. So do you mean I should let the user know that they can do the inline calculation with the new API? Thanks > -----Original Message----- > From: Stephen Hemminger <stephen@networkplumber.org> > Sent: Tuesday, November 18, 2025 1:25 AM > To: Bing Zhao <bingz@nvidia.com> > Cc: Slava Ovsiienko <viacheslavo@nvidia.com>; dev@dpdk.org; Raslan > Darawsheh <rasland@nvidia.com>; Ori Kam <orika@nvidia.com>; Dariusz > Sosnowski <dsosnowski@nvidia.com>; Suanming Mou <suanmingm@nvidia.com>; > Matan Azrad <matan@nvidia.com>; NBU-Contact-Thomas Monjalon (EXTERNAL) > <thomas@monjalon.net> > Subject: Re: [PATCH 1/2] lib/ethdev: support inline calculating masked > item value > > External email: Use caution opening links or attachments > > > On Mon, 17 Nov 2025 09:54:07 +0200 > Bing Zhao <bingz@nvidia.com> wrote: > > > diff --git a/lib/ethdev/rte_flow.c b/lib/ethdev/rte_flow.c index > > fe8f43caff..b94f0e304d 100644 > > --- a/lib/ethdev/rte_flow.c > > +++ b/lib/ethdev/rte_flow.c > > @@ -831,6 +831,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 > > Sorry, this patch is submitted too late to make it into 25.11 release. > Missing any release note. > > This would be a good example of where to use API versioning in a later > release. ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 1/2] lib/ethdev: support inline calculating masked item value 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 16:42 ` Stephen Hemminger 2026-02-09 4:23 ` Bing Zhao 2026-02-13 13:31 ` [PATCH v2] " Bing Zhao 2 siblings, 1 reply; 12+ messages in thread From: Stephen Hemminger @ 2026-02-05 16:42 UTC (permalink / raw) To: Bing Zhao Cc: viacheslavo, dev, rasland, orika, dsosnowski, suanmingm, matan, thomas On Mon, 17 Nov 2025 09:54:07 +0200 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> > --- Looked at this patch again: 1. No API/ABI breakage adding a single flow type is going to work great. rte_flow_conv_pattern is internal so changing is no problem. 2. No need for release note for single flow change. 3. Do need a test to cover the new code. I know rte_flow is way under tested right now. Maybe a chance to use AI to generate a unit test for rte_flow. This is important because it is a bug trap to add code that is not covered. ^ permalink raw reply [flat|nested] 12+ messages in thread
* RE: [PATCH 1/2] lib/ethdev: support inline calculating masked item value 2026-02-05 16:42 ` Stephen Hemminger @ 2026-02-09 4:23 ` Bing Zhao 2026-02-09 14:17 ` Bing Zhao 0 siblings, 1 reply; 12+ messages in thread From: Bing Zhao @ 2026-02-09 4:23 UTC (permalink / raw) To: Stephen Hemminger Cc: Slava Ovsiienko, dev@dpdk.org, Raslan Darawsheh, Ori Kam, Dariusz Sosnowski, Suanming Mou, Matan Azrad, NBU-Contact-Thomas Monjalon (EXTERNAL) Sure. I will add some tests. > -----Original Message----- > From: Stephen Hemminger <stephen@networkplumber.org> > Sent: Friday, February 6, 2026 12:43 AM > To: Bing Zhao <bingz@nvidia.com> > Cc: Slava Ovsiienko <viacheslavo@nvidia.com>; dev@dpdk.org; Raslan > Darawsheh <rasland@nvidia.com>; Ori Kam <orika@nvidia.com>; Dariusz > Sosnowski <dsosnowski@nvidia.com>; Suanming Mou <suanmingm@nvidia.com>; > Matan Azrad <matan@nvidia.com>; NBU-Contact-Thomas Monjalon (EXTERNAL) > <thomas@monjalon.net> > Subject: Re: [PATCH 1/2] lib/ethdev: support inline calculating masked > item value > > External email: Use caution opening links or attachments > > > On Mon, 17 Nov 2025 09:54:07 +0200 > 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> > > --- > > Looked at this patch again: > 1. No API/ABI breakage adding a single flow type is going > to work great. rte_flow_conv_pattern is internal so changing is no > problem. > 2. No need for release note for single flow change. > > 3. Do need a test to cover the new code. I know rte_flow is way under > tested > right now. Maybe a chance to use AI to generate a unit test for > rte_flow. > This is important because it is a bug trap to add code that is not > covered. ^ permalink raw reply [flat|nested] 12+ messages in thread
* RE: [PATCH 1/2] lib/ethdev: support inline calculating masked item value 2026-02-09 4:23 ` Bing Zhao @ 2026-02-09 14:17 ` Bing Zhao 2026-02-09 18:58 ` Stephen Hemminger 0 siblings, 1 reply; 12+ messages in thread From: Bing Zhao @ 2026-02-09 14:17 UTC (permalink / raw) To: Stephen Hemminger Cc: Slava Ovsiienko, dev@dpdk.org, Raslan Darawsheh, Ori Kam, Dariusz Sosnowski, Suanming Mou, Matan Azrad, NBU-Contact-Thomas Monjalon (EXTERNAL) Hi Stephen, Since there is no UT for the flow API. Such application would require some design discussions, and it is too short for -rc1. Could we focus on the change itself and try to close it? Thanks a lot. > -----Original Message----- > From: Bing Zhao <bingz@nvidia.com> > Sent: Monday, February 9, 2026 12:24 PM > To: Stephen Hemminger <stephen@networkplumber.org> > Cc: Slava Ovsiienko <viacheslavo@nvidia.com>; dev@dpdk.org; Raslan > Darawsheh <rasland@nvidia.com>; Ori Kam <orika@nvidia.com>; Dariusz > Sosnowski <dsosnowski@nvidia.com>; Suanming Mou <suanmingm@nvidia.com>; > Matan Azrad <matan@nvidia.com>; NBU-Contact-Thomas Monjalon (EXTERNAL) > <thomas@monjalon.net> > Subject: RE: [PATCH 1/2] lib/ethdev: support inline calculating masked > item value > > Sure. I will add some tests. > > > -----Original Message----- > > From: Stephen Hemminger <stephen@networkplumber.org> > > Sent: Friday, February 6, 2026 12:43 AM > > To: Bing Zhao <bingz@nvidia.com> > > Cc: Slava Ovsiienko <viacheslavo@nvidia.com>; dev@dpdk.org; Raslan > > Darawsheh <rasland@nvidia.com>; Ori Kam <orika@nvidia.com>; Dariusz > > Sosnowski <dsosnowski@nvidia.com>; Suanming Mou > > <suanmingm@nvidia.com>; Matan Azrad <matan@nvidia.com>; > > NBU-Contact-Thomas Monjalon (EXTERNAL) <thomas@monjalon.net> > > Subject: Re: [PATCH 1/2] lib/ethdev: support inline calculating masked > > item value > > > > External email: Use caution opening links or attachments > > > > > > On Mon, 17 Nov 2025 09:54:07 +0200 > > 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> > > > --- > > > > Looked at this patch again: > > 1. No API/ABI breakage adding a single flow type is going > > to work great. rte_flow_conv_pattern is internal so changing is > > no problem. > > 2. No need for release note for single flow change. > > > > 3. Do need a test to cover the new code. I know rte_flow is way > > under tested > > right now. Maybe a chance to use AI to generate a unit test for > > rte_flow. > > This is important because it is a bug trap to add code that is > > not covered. ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 1/2] lib/ethdev: support inline calculating masked item value 2026-02-09 14:17 ` Bing Zhao @ 2026-02-09 18:58 ` Stephen Hemminger 2026-02-09 21:46 ` Thomas Monjalon 0 siblings, 1 reply; 12+ messages in thread From: Stephen Hemminger @ 2026-02-09 18:58 UTC (permalink / raw) To: Bing Zhao Cc: Slava Ovsiienko, dev@dpdk.org, Raslan Darawsheh, Ori Kam, Dariusz Sosnowski, Suanming Mou, Matan Azrad, NBU-Contact-Thomas Monjalon (EXTERNAL) On Mon, 9 Feb 2026 14:17:27 +0000 Bing Zhao <bingz@nvidia.com> wrote: > Hi Stephen, > > Since there is no UT for the flow API. Such application would require some design discussions, and it is too short for -rc1. Could we focus on the change itself and try to close it? Thanks a lot. I have discovered that AI does pretty good job of generating basic tests. That is all we need to get some basic CI coverage. ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 1/2] lib/ethdev: support inline calculating masked item value 2026-02-09 18:58 ` Stephen Hemminger @ 2026-02-09 21:46 ` Thomas Monjalon 0 siblings, 0 replies; 12+ messages in thread From: Thomas Monjalon @ 2026-02-09 21:46 UTC (permalink / raw) To: Stephen Hemminger Cc: Bing Zhao, dev, Slava Ovsiienko, Raslan Darawsheh, Ori Kam, Dariusz Sosnowski, Suanming Mou, Matan Azrad 09/02/2026 19:58, Stephen Hemminger: > On Mon, 9 Feb 2026 14:17:27 +0000 > Bing Zhao <bingz@nvidia.com> wrote: > > > Hi Stephen, > > > > Since there is no UT for the flow API. > > Such application would require some design discussions, > > and it is too short for -rc1. > > Could we focus on the change itself and try to close it? Thanks a lot. > > I have discovered that AI does pretty good job of generating basic tests. > That is all we need to get some basic CI coverage. As you know, we don't have a stub implementation working without specific HW. So far, the decision discussed in the techboard was to implement flow API tests in DTS and run it in UNH. This work is in progress in the DTS team. I don't think we should require something else at this point. ^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH v2] lib/ethdev: support inline calculating masked item value 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 16:42 ` Stephen Hemminger @ 2026-02-13 13:31 ` Bing Zhao 2026-02-13 19:50 ` Stephen Hemminger 2026-06-03 8:19 ` [PATCH v3] ethdev: " Bing Zhao 2 siblings, 2 replies; 12+ messages in thread From: Bing Zhao @ 2026-02-13 13:31 UTC (permalink / raw) To: viacheslavo, dev, rasland Cc: orika, dsosnowski, suanmingm, matan, thomas, stephen 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> --- lib/ethdev/rte_flow.c | 29 +++++++++++++++++++++++------ lib/ethdev/rte_flow.h | 12 ++++++++++++ 2 files changed, 35 insertions(+), 6 deletions(-) diff --git a/lib/ethdev/rte_flow.c b/lib/ethdev/rte_flow.c index fe8f43caff..b94f0e304d 100644 --- a/lib/ethdev/rte_flow.c +++ b/lib/ethdev/rte_flow.c @@ -831,6 +831,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 @@ -843,12 +845,14 @@ 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; + uint8_t *c_spec = NULL, *c_last = NULL; for (i = 0, off = 0; !num || i != num; ++i, ++src, ++dst) { /** @@ -878,9 +882,14 @@ rte_flow_conv_pattern(struct rte_flow_item *dst, ((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 && src->mask) + for (j = 0; j < ret; j++) + *(c_spec + j) &= *((const uint8_t *)src->mask + j); } if (src->last) { @@ -889,9 +898,14 @@ 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 && src->mask) + for (j = 0; j < ret; j++) + *(c_last + j) &= *((const uint8_t *)src->mask + j); } if (src->mask) { off = RTE_ALIGN_CEIL(off, sizeof(double)); @@ -1038,7 +1052,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) @@ -1139,7 +1153,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; @@ -1154,7 +1168,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); @@ -1174,6 +1188,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 ba3bcc89a3..f56b328537 100644 --- a/lib/ethdev/rte_flow.h +++ b/lib/ethdev/rte_flow.h @@ -4558,6 +4558,18 @@ 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 @p mask to @p spec and @p spec. + * + * - @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.34.1 ^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH v2] lib/ethdev: support inline calculating masked item value 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 1 sibling, 0 replies; 12+ messages in thread From: Stephen Hemminger @ 2026-02-13 19:50 UTC (permalink / raw) To: Bing Zhao Cc: viacheslavo, dev, rasland, orika, dsosnowski, suanmingm, matan, thomas On Fri, 13 Feb 2026 15:31:43 +0200 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> I didn't see any direct issue, but AI deep dive did. Please consider these and resubmit (or say why they are false positives). PS: I would just pull the Doxygen comment, this is internal and never gets processed for inclusion in doc. Summary Severity Finding Error Stale c_spec/c_last pointers across loop iterations — can corrupt previously copied item data Error Potential out-of-bounds read on src->mask when ret exceeds actual mask size Error Wrong component prefix (lib/ethdev: → ethdev:) Error Broken Doxygen comment (missing * prefix) and typo (spec → last) Warning New enum value not marked experimental Warning Missing release notes for API change Warning Missing functional tests Info Pointer arithmetic vs array indexing style --- ## Deep Dive Review: `[PATCH v2] lib/ethdev: support inline calculating masked item value` ### Commit Message Analysis **Subject line:** `lib/ethdev: support inline calculating masked item value` **Error — Wrong component prefix.** The subject uses `lib/ethdev:` but per AGENTS.md §Common Prefix Mistakes, the correct prefix for ethdev is `ethdev:` (without `lib/`). Should be: ``` ethdev: support inline calculating masked item value ``` Let me verify the character count of the current subject: `lib/ethdev: support inline calculating masked item value` = 56 characters — within the 60-character limit, but after fixing the prefix to `ethdev:` it would be shorter still. **Commit body** — the description is functional but uses somewhat unclear phrasing: "the reason of speed of light rule insertion rate" is vague and would benefit from clearer technical justification. However, per AGENTS.md guidance, I'll only flag clarity issues if genuinely confusing — this is borderline but understandable in context to a flow API developer. **Tags:** Signed-off-by and Reviewed-by are present and in correct order (Group 2 tags only, no Group 1 tags needed). No issues there. --- ### Correctness Bugs (Highest Priority) **1. Error — `c_spec` / `c_last` not reset per loop iteration: stale pointer may cause masking wrong item's data (~80% confidence)** The variables `c_spec` and `c_last` are declared outside the `for` loop and initialized to `NULL`, but they are **never reset to `NULL`** at the start of each iteration. Consider this scenario: - Iteration N: `src->spec` exists, `size` is sufficient, so `c_spec` is set to a valid pointer. The masking is applied correctly. - Iteration N+1: `src->spec` exists, but `size` is *not* sufficient (the `size && size >= off + ret` check fails). `c_spec` retains the pointer from iteration N. Then the `with_mask && c_spec && src->mask` check passes, and the masking loop applies the **new item's mask** to the **previous item's already-copied spec data**, corrupting it. The same issue applies to `c_last`. **Suggested fix:** Reset both pointers at the top of each loop iteration, or (better) scope them inside the relevant `if` blocks: ```c for (i = 0, off = 0; !num || i != num; ++i, ++src, ++dst) { uint8_t *c_spec = NULL, *c_last = NULL; /* ... rest of loop body ... */ ``` **2. Error — Masking applied to converted data using `ret` as length, but `ret` includes alignment padding (~60% confidence)** The return value `ret` from `rte_flow_conv_pattern` internal call via `rte_flow_conv_item_spec` may include padding from `RTE_ALIGN_CEIL`. The masking loop uses `ret` as the byte count: ```c for (j = 0; j < ret; j++) *(c_spec + j) &= *((const uint8_t *)src->mask + j); ``` If `ret` is larger than the actual item spec size, this reads past the end of `src->mask`, which may only be `sizeof(struct rte_flow_item_<type>)` bytes. This is a potential **out-of-bounds read** on `src->mask`. The mask is user-provided via `src->mask` and its actual size corresponds to the item type, not the aligned/padded conversion size. If `ret` exceeds the true item spec size, the extra bytes of `src->mask` are being read from unknown memory. **Suggested fix:** The masking should use the actual item spec size rather than the conversion return value, or at minimum clamp the loop to `min(ret, item_spec_size)`. **3. Warning — No bounds validation between `ret` and mask size (~50% confidence)** Related to the above: there's no assertion or runtime check that `src->mask` points to at least `ret` bytes of valid data. The `rte_flow_conv` infrastructure knows the item type sizes, but this masking code makes an assumption that the mask buffer is at least as large as the converted spec. If a caller provides a shorter mask, this is undefined behavior. --- ### API / Process Issues **4. Warning — New public enum value `RTE_FLOW_CONV_OP_PATTERN_MASKED` not marked experimental.** A new value is added to the public `enum rte_flow_conv_op` in `rte_flow.h`. Per AGENTS.md, new APIs must be marked as `__rte_experimental`. While enum values can't directly carry the `__rte_experimental` tag, this is a new public API feature exposed through `rte_flow_conv()` and should be documented as experimental, or the corresponding code path should have experimental gating. **5. Warning — Missing release notes.** This patch adds a new conversion operation (`RTE_FLOW_CONV_OP_PATTERN_MASKED`) to the public `rte_flow_conv()` API. Per AGENTS.md, API changes require release notes updates in `doc/guides/rel_notes/`. **6. Warning — Missing tests.** No test code is included for the new `RTE_FLOW_CONV_OP_PATTERN_MASKED` operation. Per AGENTS.md, new API functions/operations should have functional tests. --- ### Documentation Issues **7. Error — Doxygen comment has broken formatting and a typo.** In `rte_flow.h`, the new enum doc block: ```c /** * Convert an entire pattern. * Duplicates all pattern items at once, applying @p mask to @p spec and @p spec. ``` Two problems: - The line `Duplicates all pattern items at once...` is missing the leading ` *` comment continuation prefix — this will break Doxygen parsing. - The text says "applying @p mask to @p spec and @p spec" — the second `@p spec` should be `@p last`. **Suggested fix:** ```c /** * Convert an entire pattern. * * Duplicates all pattern items at once, applying @p mask to @p spec and @p last. ``` --- ### Style Observations **8. Info — Pointer arithmetic style.** The masking uses `*(c_spec + j)` rather than the more idiomatic `c_spec[j]`, and `*((const uint8_t *)src->mask + j)` rather than `((const uint8_t *)src->mask)[j]`. Array indexing notation would be clearer and more consistent with typical DPDK style. --- ### Summary | Severity | Finding | |----------|---------| | **Error** | Stale `c_spec`/`c_last` pointers across loop iterations — can corrupt previously copied item data | | **Error** | Potential out-of-bounds read on `src->mask` when `ret` exceeds actual mask size | | **Error** | Wrong component prefix (`lib/ethdev:` → `ethdev:`) | | **Error** | Broken Doxygen comment (missing `*` prefix) and typo (`spec` → `last`) | | **Warning** | New enum value not marked experimental | | **Warning** | Missing release notes for API change | | **Warning** | Missing functional tests | | **Info** | Pointer arithmetic vs array indexing style | The most critical finding is #1 — the stale pointer bug. It's a logic error that would cause data corruption whenever `rte_flow_conv` is called with `RTE_FLOW_CONV_OP_PATTERN_MASKED` on a pattern containing multiple items where the output buffer is too small for some but not all items. ^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH v3] ethdev: support inline calculating masked item value 2026-02-13 13:31 ` [PATCH v2] " Bing Zhao 2026-02-13 19:50 ` Stephen Hemminger @ 2026-06-03 8:19 ` Bing Zhao 2026-06-03 9:28 ` [PATCH v4] " Bing Zhao 1 sibling, 1 reply; 12+ messages in thread From: Bing Zhao @ 2026-06-03 8:19 UTC (permalink / raw) To: viacheslavo, dev, rasland, stephen Cc: orika, dsosnowski, suanmingm, matan, thomas 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> --- v3: - add test code - fix the issue found by AI --- app/test/test_ethdev_api.c | 76 ++++++++++++++++++++++++++ doc/guides/rel_notes/release_26_07.rst | 6 ++ lib/ethdev/rte_flow.c | 46 ++++++++++++++-- lib/ethdev/rte_flow.h | 13 +++++ 4 files changed, 135 insertions(+), 6 deletions(-) diff --git a/app/test/test_ethdev_api.c b/app/test/test_ethdev_api.c index 76afd0345c..5cae1cdc1d 100644 --- a/app/test/test_ethdev_api.c +++ b/app/test/test_ethdev_api.c @@ -4,6 +4,7 @@ #include <rte_log.h> #include <rte_ethdev.h> +#include <rte_flow.h> #include <rte_test.h> #include "test.h" @@ -15,6 +16,80 @@ #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 struct rte_flow_item pattern[] = { + { + .type = RTE_FLOW_ITEM_TYPE_ETH, + .spec = &spec, + .last = &last, + .mask = &mask, + }, + { .type = RTE_FLOW_ITEM_TYPE_END }, + }; + union { + struct rte_flow_item item; + struct rte_flow_item_eth eth; + double align; + uint8_t raw[256]; + } dst; + const struct rte_flow_item *item; + const struct rte_flow_item_eth *conv_spec; + const struct rte_flow_item_eth *conv_last; + 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.raw), + "Masked pattern conversion needs too much storage"); + + memset(&dst, 0, sizeof(dst)); + ret = rte_flow_conv(RTE_FLOW_CONV_OP_PATTERN_MASKED, dst.raw, + sizeof(dst.raw), pattern, NULL); + TEST_ASSERT(ret > 0, "Masked pattern conversion failed"); + + item = (const struct rte_flow_item *)dst.raw; + 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"); + + return TEST_SUCCESS; +} + static int32_t ethdev_api_queue_status(void) { @@ -167,6 +242,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/doc/guides/rel_notes/release_26_07.rst b/doc/guides/rel_notes/release_26_07.rst index 8b4f8401e2..46a83cfea5 100644 --- a/doc/guides/rel_notes/release_26_07.rst +++ b/doc/guides/rel_notes/release_26_07.rst @@ -116,6 +116,12 @@ API Changes Also, make sure to start the actual text at the margin. ======================================================= +* ethdev: Added masked pattern conversion. + + Added ``RTE_FLOW_CONV_OP_PATTERN_MASKED`` to ``rte_flow_conv()`` + to copy an entire pattern while applying each item's mask to its + ``spec`` and ``last`` fields. + ABI Changes ----------- diff --git a/lib/ethdev/rte_flow.c b/lib/ethdev/rte_flow.c index 7a51b667cf..7cf9f6f6f3 100644 --- a/lib/ethdev/rte_flow.c +++ b/lib/ethdev/rte_flow.c @@ -178,6 +178,14 @@ 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 rte_flow_desc_item[item->type].size; + return sizeof(void *); +} + /** Generate flow_action[] entry. */ #define MK_FLOW_ACTION(t, s) \ [RTE_FLOW_ACTION_TYPE_ ## t] = { \ @@ -835,6 +843,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 @@ -847,12 +857,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) { /** @@ -876,15 +887,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) { @@ -893,9 +916,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)); @@ -1042,7 +1073,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) @@ -1143,7 +1174,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; @@ -1158,7 +1189,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); @@ -1178,6 +1209,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 ba3bcc89a3..f3102f4bb0 100644 --- a/lib/ethdev/rte_flow.h +++ b/lib/ethdev/rte_flow.h @@ -4558,6 +4558,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 @p mask to @p spec + * and @p last. + * + * - @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.34.1 ^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH v4] ethdev: support inline calculating masked item value 2026-06-03 8:19 ` [PATCH v3] ethdev: " Bing Zhao @ 2026-06-03 9:28 ` Bing Zhao 0 siblings, 0 replies; 12+ messages in thread From: Bing Zhao @ 2026-06-03 9:28 UTC (permalink / raw) To: viacheslavo, dev, rasland, stephen Cc: orika, dsosnowski, suanmingm, matan, thomas 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> --- v3: - add test code - fix the issue found by AI v4: reabse on top of the main --- app/test/test_ethdev_api.c | 76 ++++++++++++++++++++++++++ doc/guides/rel_notes/release_26_07.rst | 6 ++ lib/ethdev/rte_flow.c | 46 ++++++++++++++-- lib/ethdev/rte_flow.h | 13 +++++ 4 files changed, 135 insertions(+), 6 deletions(-) diff --git a/app/test/test_ethdev_api.c b/app/test/test_ethdev_api.c index 76afd0345c..5cae1cdc1d 100644 --- a/app/test/test_ethdev_api.c +++ b/app/test/test_ethdev_api.c @@ -4,6 +4,7 @@ #include <rte_log.h> #include <rte_ethdev.h> +#include <rte_flow.h> #include <rte_test.h> #include "test.h" @@ -15,6 +16,80 @@ #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 struct rte_flow_item pattern[] = { + { + .type = RTE_FLOW_ITEM_TYPE_ETH, + .spec = &spec, + .last = &last, + .mask = &mask, + }, + { .type = RTE_FLOW_ITEM_TYPE_END }, + }; + union { + struct rte_flow_item item; + struct rte_flow_item_eth eth; + double align; + uint8_t raw[256]; + } dst; + const struct rte_flow_item *item; + const struct rte_flow_item_eth *conv_spec; + const struct rte_flow_item_eth *conv_last; + 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.raw), + "Masked pattern conversion needs too much storage"); + + memset(&dst, 0, sizeof(dst)); + ret = rte_flow_conv(RTE_FLOW_CONV_OP_PATTERN_MASKED, dst.raw, + sizeof(dst.raw), pattern, NULL); + TEST_ASSERT(ret > 0, "Masked pattern conversion failed"); + + item = (const struct rte_flow_item *)dst.raw; + 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"); + + return TEST_SUCCESS; +} + static int32_t ethdev_api_queue_status(void) { @@ -167,6 +242,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/doc/guides/rel_notes/release_26_07.rst b/doc/guides/rel_notes/release_26_07.rst index b8a3e2ced9..5f01420d2b 100644 --- a/doc/guides/rel_notes/release_26_07.rst +++ b/doc/guides/rel_notes/release_26_07.rst @@ -134,6 +134,12 @@ API Changes Also, make sure to start the actual text at the margin. ======================================================= +* ethdev: Added masked pattern conversion. + + Added ``RTE_FLOW_CONV_OP_PATTERN_MASKED`` to ``rte_flow_conv()`` + to copy an entire pattern while applying each item's mask to its + ``spec`` and ``last`` fields. + ABI Changes ----------- diff --git a/lib/ethdev/rte_flow.c b/lib/ethdev/rte_flow.c index 7a51b667cf..7cf9f6f6f3 100644 --- a/lib/ethdev/rte_flow.c +++ b/lib/ethdev/rte_flow.c @@ -178,6 +178,14 @@ 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 rte_flow_desc_item[item->type].size; + return sizeof(void *); +} + /** Generate flow_action[] entry. */ #define MK_FLOW_ACTION(t, s) \ [RTE_FLOW_ACTION_TYPE_ ## t] = { \ @@ -835,6 +843,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 @@ -847,12 +857,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) { /** @@ -876,15 +887,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) { @@ -893,9 +916,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)); @@ -1042,7 +1073,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) @@ -1143,7 +1174,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; @@ -1158,7 +1189,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); @@ -1178,6 +1209,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 eacfd7c5f7..5f8d4b00f0 100644 --- a/lib/ethdev/rte_flow.h +++ b/lib/ethdev/rte_flow.h @@ -4558,6 +4558,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 @p mask to @p spec + * and @p last. + * + * - @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.34.1 ^ permalink raw reply related [flat|nested] 12+ messages in thread
end of thread, other threads:[~2026-06-03 9:28 UTC | newest] Thread overview: 12+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 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
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.