[PATCH] testpmd: support meter_mark init_color in indirect list configuration

[PATCH] testpmd: support meter_mark init_color in indirect list configuration

[Gregory Etelson]

Flow actions parameters in indirect actions list are created as
read-only and shared between all flows that reference that indirect
list.

If a flow rule needs to apply rule specific actions list parameters it
does it with the indirect actions list conf parameter.

The patch allows flow rule to set meter_mark init_color value when
meter_mark action was created in indirect actions list.

Example:

# create indirect actions list with meter_mark flow action:
testpmd> flow indirect_action 0 create action_id 10 ingress \
  list actions meter_mark mtr_profile 20 \
  mtr_state 1 mtr_color_mode 1 / end

# create a flow specific meter_mark init_color configuration:
testpmd> flow indirect_action 0 create action_id 11 flow_conf \
   actions meter_mark_conf mtr_update_init_color red / end

# queue a flow rule with indirect actions list
# and flow specific configuration:
testpmd> flow queue 0 create 0 template_table 1 pattern_template 0 \
   actions_template 0 postpone no pattern eth / ipv4 / udp / end \
   actions indirect_list handle 10 conf 11 / \
   jump group 10 / end

cc: stable@dpdk.org

Signed-off-by: Gregory Etelson <getelson@nvidia.com>
---
 app/test-pmd/cmdline_flow.c | 52 +++++++++++++++++++++++++++++--------
 1 file changed, 41 insertions(+), 11 deletions(-)

diff --git a/app/test-pmd/cmdline_flow.c b/app/test-pmd/cmdline_flow.c
index e1720e54d7..6add4e56ec 100644
--- a/app/test-pmd/cmdline_flow.c
+++ b/app/test-pmd/cmdline_flow.c
@@ -62,6 +62,7 @@ enum index {
 	COMMON_ACTIONS_TEMPLATE_ID,
 	COMMON_TABLE_ID,
 	COMMON_QUEUE_ID,
+	COMMON_METER_COLOR_NAME,
 
 	/* TOP-level command. */
 	ADD,
@@ -556,7 +557,6 @@ enum index {
 	ITEM_PPP_PROTO_ID,
 	ITEM_METER,
 	ITEM_METER_COLOR,
-	ITEM_METER_COLOR_NAME,
 	ITEM_QUOTA,
 	ITEM_QUOTA_STATE,
 	ITEM_QUOTA_STATE_NAME,
@@ -642,6 +642,8 @@ enum index {
 	ACTION_METER_COLOR_RED,
 	ACTION_METER_ID,
 	ACTION_METER_MARK,
+	ACTION_METER_MARK_CONF,
+	ACTION_METER_MARK_CONF_COLOR,
 	ACTION_METER_PROFILE,
 	ACTION_METER_PROFILE_ID2PTR,
 	ACTION_METER_POLICY,
@@ -2270,6 +2272,7 @@ static const enum index next_action[] = {
 	ACTION_METER,
 	ACTION_METER_COLOR,
 	ACTION_METER_MARK,
+	ACTION_METER_MARK_CONF,
 	ACTION_OF_DEC_NW_TTL,
 	ACTION_OF_POP_VLAN,
 	ACTION_OF_PUSH_VLAN,
@@ -3235,6 +3238,12 @@ static const struct token token_list[] = {
 		.call = parse_int,
 		.comp = comp_queue_id,
 	},
+	[COMMON_METER_COLOR_NAME] = {
+		.name = "color_name",
+		.help = "meter color name",
+		.call = parse_meter_color,
+		.comp = comp_meter_color,
+	},
 	/* Top-level command. */
 	[FLOW] = {
 		.name = "flow",
@@ -6280,17 +6289,11 @@ static const struct token token_list[] = {
 		.name = "color",
 		.help = "meter color",
 		.next = NEXT(item_meter,
-			     NEXT_ENTRY(ITEM_METER_COLOR_NAME),
+			     NEXT_ENTRY(COMMON_METER_COLOR_NAME),
 			     item_param),
 		.args = ARGS(ARGS_ENTRY(struct rte_flow_item_meter_color,
 					color)),
 	},
-	[ITEM_METER_COLOR_NAME] = {
-		.name = "color_name",
-		.help = "meter color name",
-		.call = parse_meter_color,
-		.comp = comp_meter_color,
-	},
 	[ITEM_QUOTA] = {
 		.name = "quota",
 		.help = "match quota",
@@ -6856,6 +6859,23 @@ static const struct token token_list[] = {
 		.next = NEXT(action_meter_mark),
 		.call = parse_vc,
 	},
+	[ACTION_METER_MARK_CONF] = {
+		.name = "meter_mark_conf",
+		.help = "meter mark configuration",
+		.priv = PRIV_ACTION(METER_MARK,
+				    sizeof(struct rte_flow_action_meter_mark)),
+		.next = NEXT(NEXT_ENTRY(ACTION_METER_MARK_CONF_COLOR)),
+		.call = parse_vc,
+	},
+	[ACTION_METER_MARK_CONF_COLOR] = {
+		.name = "mtr_update_init_color",
+		.help = "meter update init color",
+		.next = NEXT(NEXT_ENTRY(ACTION_NEXT),
+			     NEXT_ENTRY(COMMON_METER_COLOR_NAME)),
+		.args = ARGS(ARGS_ENTRY
+			     (struct rte_flow_indirect_update_flow_meter_mark,
+			      init_color)),
+	},
 	[ACTION_METER_PROFILE] = {
 		.name = "mtr_profile",
 		.help = "meter profile id to use",
@@ -12375,8 +12395,8 @@ parse_meter_color(struct context *ctx, const struct token *token,
 		  const char *str, unsigned int len, void *buf,
 		  unsigned int size)
 {
-	struct rte_flow_item_meter_color *meter_color;
 	unsigned int i;
+	struct buffer *out = buf;
 
 	(void)token;
 	(void)buf;
@@ -12388,8 +12408,18 @@ parse_meter_color(struct context *ctx, const struct token *token,
 		return -1;
 	if (!ctx->object)
 		return len;
-	meter_color = ctx->object;
-	meter_color->color = (enum rte_color)i;
+	if (ctx->prev == ACTION_METER_MARK_CONF_COLOR) {
+		struct rte_flow_action *action =
+			out->args.vc.actions + out->args.vc.actions_n - 1;
+		const struct arg *arg = pop_args(ctx);
+
+		if (!arg)
+			return -1;
+		*(int *)RTE_PTR_ADD(action->conf, arg->offset) = i;
+	} else {
+		((struct rte_flow_item_meter_color *)
+			ctx->object)->color = (enum rte_color)i;
+	}
 	return len;
 }
 
-- 
2.45.2

Re: [PATCH] testpmd: support meter_mark init_color in indirect list configuration

[Stephen Hemminger]

On Sun, 16 Feb 2025 16:04:20 +0200
Gregory Etelson <getelson@nvidia.com> wrote:

> Flow actions parameters in indirect actions list are created as
> read-only and shared between all flows that reference that indirect
> list.
> 
> If a flow rule needs to apply rule specific actions list parameters it
> does it with the indirect actions list conf parameter.
> 
> The patch allows flow rule to set meter_mark init_color value when
> meter_mark action was created in indirect actions list.
> 
> Example:
> 
> # create indirect actions list with meter_mark flow action:
> testpmd> flow indirect_action 0 create action_id 10 ingress \  
>   list actions meter_mark mtr_profile 20 \
>   mtr_state 1 mtr_color_mode 1 / end
> 
> # create a flow specific meter_mark init_color configuration:
> testpmd> flow indirect_action 0 create action_id 11 flow_conf \  
>    actions meter_mark_conf mtr_update_init_color red / end
> 
> # queue a flow rule with indirect actions list
> # and flow specific configuration:
> testpmd> flow queue 0 create 0 template_table 1 pattern_template 0 \  
>    actions_template 0 postpone no pattern eth / ipv4 / udp / end \
>    actions indirect_list handle 10 conf 11 / \
>    jump group 10 / end
> 
> cc: stable@dpdk.org
> 
> Signed-off-by: Gregory Etelson <getelson@nvidia.com>

Patches that are cc to stable need a Fixes tag so that stable
maintainers know what to fix.

But this does not look like stable material since it adds new functionality.

Also, current upstream kernel checkpatch doesn't like # as comment

 $ ./devtools/checkpatches.sh 

### [PATCH] testpmd: support meter_mark init_color in indirect list configuration

WARNING:COMMIT_COMMENT_SYMBOL: Commit log lines starting with '#' are dropped by git as comments
#19: 
# create indirect actions list with meter_mark flow action:

WARNING:COMMIT_COMMENT_SYMBOL: Commit log lines starting with '#' are dropped by git as comments
#24: 
# create a flow specific meter_mark init_color configuration:

WARNING:COMMIT_COMMENT_SYMBOL: Commit log lines starting with '#' are dropped by git as comments
#28: 
# queue a flow rule with indirect actions list

WARNING:COMMIT_COMMENT_SYMBOL: Commit log lines starting with '#' are dropped by git as comments
#29: 
# and flow specific configuration:

WARNING:MISSING_FIXES_TAG: The commit message has 'stable@', perhaps it also needs a 'Fixes:' tag?

total: 0 errors, 5 warnings, 111 lines checked

[PATCH v2] testpmd: support meter_mark init_color in indirect list configuration

[Gregory Etelson]

Flow actions parameters in indirect actions list are created as
read-only and shared between all flows that reference that indirect
list.

If a flow rule needs to apply rule specific actions list parameters it
does it with the indirect actions list conf parameter.

The patch allows flow rule to set meter_mark init_color value when
meter_mark action was created in indirect actions list.

Example:

// create indirect actions list with meter_mark flow action:
testpmd> flow indirect_action 0 create action_id 10 ingress \
  list actions meter_mark mtr_profile 20 \
  mtr_state 1 mtr_color_mode 1 / end

// create a flow specific meter_mark init_color configuration:
testpmd> flow indirect_action 0 create action_id 11 flow_conf \
   actions meter_mark_conf mtr_update_init_color red / end

// queue a flow rule with indirect actions list
// and flow specific configuration:
testpmd> flow queue 0 create 0 template_table 1 pattern_template 0 \
   actions_template 0 postpone no pattern eth / ipv4 / udp / end \
   actions indirect_list handle 10 conf 11 / \
   jump group 10 / end

Signed-off-by: Gregory Etelson <getelson@nvidia.com>
---
v2: Fix commit log.
---
 app/test-pmd/cmdline_flow.c | 52 +++++++++++++++++++++++++++++--------
 1 file changed, 41 insertions(+), 11 deletions(-)

diff --git a/app/test-pmd/cmdline_flow.c b/app/test-pmd/cmdline_flow.c
index e1720e54d7..6add4e56ec 100644
--- a/app/test-pmd/cmdline_flow.c
+++ b/app/test-pmd/cmdline_flow.c
@@ -62,6 +62,7 @@ enum index {
 	COMMON_ACTIONS_TEMPLATE_ID,
 	COMMON_TABLE_ID,
 	COMMON_QUEUE_ID,
+	COMMON_METER_COLOR_NAME,
 
 	/* TOP-level command. */
 	ADD,
@@ -556,7 +557,6 @@ enum index {
 	ITEM_PPP_PROTO_ID,
 	ITEM_METER,
 	ITEM_METER_COLOR,
-	ITEM_METER_COLOR_NAME,
 	ITEM_QUOTA,
 	ITEM_QUOTA_STATE,
 	ITEM_QUOTA_STATE_NAME,
@@ -642,6 +642,8 @@ enum index {
 	ACTION_METER_COLOR_RED,
 	ACTION_METER_ID,
 	ACTION_METER_MARK,
+	ACTION_METER_MARK_CONF,
+	ACTION_METER_MARK_CONF_COLOR,
 	ACTION_METER_PROFILE,
 	ACTION_METER_PROFILE_ID2PTR,
 	ACTION_METER_POLICY,
@@ -2270,6 +2272,7 @@ static const enum index next_action[] = {
 	ACTION_METER,
 	ACTION_METER_COLOR,
 	ACTION_METER_MARK,
+	ACTION_METER_MARK_CONF,
 	ACTION_OF_DEC_NW_TTL,
 	ACTION_OF_POP_VLAN,
 	ACTION_OF_PUSH_VLAN,
@@ -3235,6 +3238,12 @@ static const struct token token_list[] = {
 		.call = parse_int,
 		.comp = comp_queue_id,
 	},
+	[COMMON_METER_COLOR_NAME] = {
+		.name = "color_name",
+		.help = "meter color name",
+		.call = parse_meter_color,
+		.comp = comp_meter_color,
+	},
 	/* Top-level command. */
 	[FLOW] = {
 		.name = "flow",
@@ -6280,17 +6289,11 @@ static const struct token token_list[] = {
 		.name = "color",
 		.help = "meter color",
 		.next = NEXT(item_meter,
-			     NEXT_ENTRY(ITEM_METER_COLOR_NAME),
+			     NEXT_ENTRY(COMMON_METER_COLOR_NAME),
 			     item_param),
 		.args = ARGS(ARGS_ENTRY(struct rte_flow_item_meter_color,
 					color)),
 	},
-	[ITEM_METER_COLOR_NAME] = {
-		.name = "color_name",
-		.help = "meter color name",
-		.call = parse_meter_color,
-		.comp = comp_meter_color,
-	},
 	[ITEM_QUOTA] = {
 		.name = "quota",
 		.help = "match quota",
@@ -6856,6 +6859,23 @@ static const struct token token_list[] = {
 		.next = NEXT(action_meter_mark),
 		.call = parse_vc,
 	},
+	[ACTION_METER_MARK_CONF] = {
+		.name = "meter_mark_conf",
+		.help = "meter mark configuration",
+		.priv = PRIV_ACTION(METER_MARK,
+				    sizeof(struct rte_flow_action_meter_mark)),
+		.next = NEXT(NEXT_ENTRY(ACTION_METER_MARK_CONF_COLOR)),
+		.call = parse_vc,
+	},
+	[ACTION_METER_MARK_CONF_COLOR] = {
+		.name = "mtr_update_init_color",
+		.help = "meter update init color",
+		.next = NEXT(NEXT_ENTRY(ACTION_NEXT),
+			     NEXT_ENTRY(COMMON_METER_COLOR_NAME)),
+		.args = ARGS(ARGS_ENTRY
+			     (struct rte_flow_indirect_update_flow_meter_mark,
+			      init_color)),
+	},
 	[ACTION_METER_PROFILE] = {
 		.name = "mtr_profile",
 		.help = "meter profile id to use",
@@ -12375,8 +12395,8 @@ parse_meter_color(struct context *ctx, const struct token *token,
 		  const char *str, unsigned int len, void *buf,
 		  unsigned int size)
 {
-	struct rte_flow_item_meter_color *meter_color;
 	unsigned int i;
+	struct buffer *out = buf;
 
 	(void)token;
 	(void)buf;
@@ -12388,8 +12408,18 @@ parse_meter_color(struct context *ctx, const struct token *token,
 		return -1;
 	if (!ctx->object)
 		return len;
-	meter_color = ctx->object;
-	meter_color->color = (enum rte_color)i;
+	if (ctx->prev == ACTION_METER_MARK_CONF_COLOR) {
+		struct rte_flow_action *action =
+			out->args.vc.actions + out->args.vc.actions_n - 1;
+		const struct arg *arg = pop_args(ctx);
+
+		if (!arg)
+			return -1;
+		*(int *)RTE_PTR_ADD(action->conf, arg->offset) = i;
+	} else {
+		((struct rte_flow_item_meter_color *)
+			ctx->object)->color = (enum rte_color)i;
+	}
 	return len;
 }
 
-- 
2.45.2

[PATCH] config/arm: update NVIDIA BlueField-3 configuration

[Gregory Etelson]

ARM configuration requires explicit `mcpu` specifications in the
implementor description.

The patch adds `mcpu` and `flags` description for the NVIDIA
BlueField-3 configuration.

Signed-off-by: Gregory Etelson <getelson@nvidia.com>
---
 config/arm/meson.build | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/config/arm/meson.build b/config/arm/meson.build
index 5302861e2c..74f4dd4e1f 100644
--- a/config/arm/meson.build
+++ b/config/arm/meson.build
@@ -85,6 +85,11 @@ part_number_config_arm = {
     },
     '0xd42': {
         'march': 'armv8.4-a',
+        'mcpu' : 'cortex-a78ae',
+        'flags': [
+            ['RTE_MAX_LCORE', 16],
+            ['RTE_MAX_NUMA_NODES', 1]
+        ],
     },
     '0xd49': {
         'mcpu': 'neoverse-n2',
-- 
2.45.2

RE: [PATCH] config/arm: update NVIDIA BlueField-3 configuration

[Wathsala Wathawana Vithanage]

> Subject: [PATCH] config/arm: update NVIDIA BlueField-3 configuration
> 
> ARM configuration requires explicit `mcpu` specifications in the implementor
> description.
> 
> The patch adds `mcpu` and `flags` description for the NVIDIA
> BlueField-3 configuration.
> 
> Signed-off-by: Gregory Etelson <getelson@nvidia.com>
> ---
>  config/arm/meson.build | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/config/arm/meson.build b/config/arm/meson.build index
> 5302861e2c..74f4dd4e1f 100644
> --- a/config/arm/meson.build
> +++ b/config/arm/meson.build
> @@ -85,6 +85,11 @@ part_number_config_arm = {
>      },
>      '0xd42': {
>          'march': 'armv8.4-a',
> +        'mcpu' : 'cortex-a78ae',
> +        'flags': [
> +            ['RTE_MAX_LCORE', 16],
> +            ['RTE_MAX_NUMA_NODES', 1]
> +        ],
>      },
>      '0xd49': {
>          'mcpu': 'neoverse-n2',
> --
> 2.45.2

Acked-by: Wathsala Vithanage <wathsala.vithanage@arm.com>

Re: [PATCH] config/arm: update NVIDIA BlueField-3 configuration

[Thomas Monjalon]

19/02/2025 17:35, Wathsala Wathawana Vithanage:
> > Subject: [PATCH] config/arm: update NVIDIA BlueField-3 configuration
> > 
> > ARM configuration requires explicit `mcpu` specifications in the implementor
> > description.
> > 
> > The patch adds `mcpu` and `flags` description for the NVIDIA
> > BlueField-3 configuration.
> > 
> > Signed-off-by: Gregory Etelson <getelson@nvidia.com>
> 
> Acked-by: Wathsala Vithanage <wathsala.vithanage@arm.com>

Adding error message:
"
The build was broken for BlueField-3:
    ERROR: Problem encountered:
    No suitable Arm mcpu name or custom mcpu definition object found.
"

and
Fixes: c02c01dbf907 ("config/arm: prefer strict use of -mcpu if supported")

Applied, thanks.

Re: [PATCH v2] testpmd: support meter_mark init_color in indirect list configuration

[Stephen Hemminger]

On Mon, 17 Feb 2025 19:21:10 +0200
Gregory Etelson <getelson@nvidia.com> wrote:

> Flow actions parameters in indirect actions list are created as
> read-only and shared between all flows that reference that indirect
> list.
> 
> If a flow rule needs to apply rule specific actions list parameters it
> does it with the indirect actions list conf parameter.
> 
> The patch allows flow rule to set meter_mark init_color value when
> meter_mark action was created in indirect actions list.
> 
> Example:
> 
> // create indirect actions list with meter_mark flow action:
> testpmd> flow indirect_action 0 create action_id 10 ingress \  
>   list actions meter_mark mtr_profile 20 \
>   mtr_state 1 mtr_color_mode 1 / end
> 
> // create a flow specific meter_mark init_color configuration:
> testpmd> flow indirect_action 0 create action_id 11 flow_conf \  
>    actions meter_mark_conf mtr_update_init_color red / end
> 
> // queue a flow rule with indirect actions list
> // and flow specific configuration:
> testpmd> flow queue 0 create 0 template_table 1 pattern_template 0 \  
>    actions_template 0 postpone no pattern eth / ipv4 / udp / end \
>    actions indirect_list handle 10 conf 11 / \
>    jump group 10 / end
> 
> Signed-off-by: Gregory Etelson <getelson@nvidia.com>

Looks good, would like a ack from one of the flow maintainers that
looked at the original patch. Those were:

    Signed-off-by: Alexander Kozyrev <akozyrev@nvidia.com>
    Acked-by: Cristian Dumitrescu <cristian.dumitrescu@intel.com>
    Acked-by: Ori Kam <orika@nvidia.com>
    Acked-by: Ajit Khaparde <ajit.khaparde@broadcom.com>
    Acked-by: Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>

RE: [PATCH v2] testpmd: support meter_mark init_color in indirect list configuration

[Ori Kam]

> -----Original Message-----
> From: Gregory Etelson <getelson@nvidia.com>
> Sent: Monday, February 17, 2025 7:21 PM
> 
> Flow actions parameters in indirect actions list are created as
> read-only and shared between all flows that reference that indirect
> list.
> 
> If a flow rule needs to apply rule specific actions list parameters it
> does it with the indirect actions list conf parameter.
> 
> The patch allows flow rule to set meter_mark init_color value when
> meter_mark action was created in indirect actions list.
> 
> Example:
> 
> // create indirect actions list with meter_mark flow action:
> testpmd> flow indirect_action 0 create action_id 10 ingress \
>   list actions meter_mark mtr_profile 20 \
>   mtr_state 1 mtr_color_mode 1 / end
> 
> // create a flow specific meter_mark init_color configuration:
> testpmd> flow indirect_action 0 create action_id 11 flow_conf \
>    actions meter_mark_conf mtr_update_init_color red / end
> 
> // queue a flow rule with indirect actions list
> // and flow specific configuration:
> testpmd> flow queue 0 create 0 template_table 1 pattern_template 0 \
>    actions_template 0 postpone no pattern eth / ipv4 / udp / end \
>    actions indirect_list handle 10 conf 11 / \
>    jump group 10 / end
> 
> Signed-off-by: Gregory Etelson <getelson@nvidia.com>
> ---
> v2: Fix commit log.
> ---

Acked-by: Ori Kam <orika@nvidia.com>
Best,
Ori