[PATCH 1/2] lib/ethdev: support inline calculating masked item value

[PATCH 1/2] lib/ethdev: support inline calculating masked item value

[Bing Zhao]

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

Re: [PATCH 1/2] lib/ethdev: support inline calculating masked item value

[Stephen Hemminger]

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.

RE: [PATCH 1/2] lib/ethdev: support inline calculating masked item value

[Bing Zhao]

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.

Re: [PATCH 1/2] lib/ethdev: support inline calculating masked item value

[Stephen Hemminger]

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.

RE: [PATCH 1/2] lib/ethdev: support inline calculating masked item value

[Bing Zhao]

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.

RE: [PATCH 1/2] lib/ethdev: support inline calculating masked item value

[Bing Zhao]

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.

Re: [PATCH 1/2] lib/ethdev: support inline calculating masked item value

[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.

Re: [PATCH 1/2] lib/ethdev: support inline calculating masked item value

[Thomas Monjalon]

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.

[PATCH v2] lib/ethdev: support inline calculating masked item value

[Bing Zhao]

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

Re: [PATCH v2] lib/ethdev: support inline calculating masked item value

[Stephen Hemminger]

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.