DPDK patches and discussions
 help / color / mirror / Atom feed
* [PATCH] app/testpmd: support updating flow rule actions
@ 2024-01-26 13:21 Oleksandr Kolomeiets
  2024-02-01  9:59 ` [PATCH v2] " Oleksandr Kolomeiets
  0 siblings, 1 reply; 8+ messages in thread
From: Oleksandr Kolomeiets @ 2024-01-26 13:21 UTC (permalink / raw)
  To: thomas, aman.deep.singh, yuying.zhang, orika, mko-plv, ckm; +Cc: dev

"flow actions_update" updates a flow rule specified by a rule ID with a
new action list by making a call to "rte_flow_actions_update()":

    flow actions_update {port_id} {rule_id}
        actions {action} [/ {action} [...]] / end [user_id]

Creating, updating and destroying a flow rule:

    testpmd> flow create 0 group 1 pattern eth / end actions drop / end
    Flow rule #0 created
    testpmd> flow actions_update 0 0 actions queue index 1 / end
    Flow rule #0 updated with new actions
    testpmd> flow destroy 0 rule 0
    Flow rule #0 destroyed

Signed-off-by: Oleksandr Kolomeiets <okl-plv@napatech.com>
Reviewed-by: Mykola Kostenok <mko-plv@napatech.com>
Reviewed-by: Christian Koue Muf <ckm@napatech.com>
---
 .mailmap                                    |  3 ++
 app/test-pmd/cmdline.c                      |  4 ++
 app/test-pmd/cmdline_flow.c                 | 34 +++++++++++++++-
 app/test-pmd/config.c                       | 43 ++++++++++++++++++++
 app/test-pmd/testpmd.h                      |  3 ++
 doc/guides/testpmd_app_ug/testpmd_funcs.rst | 44 +++++++++++++++++++++
 6 files changed, 130 insertions(+), 1 deletion(-)

diff --git a/.mailmap b/.mailmap
index 6011526cae..44da31c1c2 100644
--- a/.mailmap
+++ b/.mailmap
@@ -232,6 +232,7 @@ Chintu Hetam <rometoroam@gmail.com>
 Choonho Son <choonho.son@gmail.com>
 Chris Metcalf <cmetcalf@mellanox.com>
 Christian Ehrhardt <christian.ehrhardt@canonical.com>
+Christian Koue Muf <ckm@napatech.com>
 Christian Maciocco <christian.maciocco@intel.com>
 Christophe Fontaine <cfontain@redhat.com>
 Christophe Grosse <christophe.grosse@6wind.com>
@@ -986,6 +987,7 @@ Mukesh Dua <mukesh.dua81@gmail.com>
 Murphy Yang <murphyx.yang@intel.com>
 Murthy NSSR <nidadavolu.murthy@caviumnetworks.com>
 Muthurajan Jayakumar <muthurajan.jayakumar@intel.com>
+Mykola Kostenok <mko-plv@napatech.com>
 Nachiketa Prachanda <nprachan@brocade.com> <nprachan@vyatta.att-mail.com>
 Nagadheeraj Rottela <rnagadheeraj@marvell.com>
 Naga Harish K S V <s.v.naga.harish.k@intel.com>
@@ -1040,6 +1042,7 @@ Odi Assli <odia@nvidia.com>
 Ognjen Joldzic <ognjen.joldzic@gmail.com>
 Ola Liljedahl <ola.liljedahl@arm.com>
 Oleg Polyakov <olegp123@walla.co.il>
+Oleksandr Kolomeiets <okl-plv@napatech.com>
 Olga Shern <olgas@nvidia.com> <olgas@mellanox.com>
 Olivier Gournet <ogournet@corp.free.fr>
 Olivier Matz <olivier.matz@6wind.com>
diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c
index f704319771..8249e4eb92 100644
--- a/app/test-pmd/cmdline.c
+++ b/app/test-pmd/cmdline.c
@@ -852,6 +852,10 @@ static void cmd_help_long_parsed(void *parsed_result,
 			"flow destroy {port_id} rule {rule_id} [...]\n"
 			"    Destroy specific flow rules.\n\n"
 
+			"flow actions_update {port_id} {rule_id}"
+			" actions {action} [/ {action} [...]] / end [user_id]\n"
+			"    Update a flow rule with new actions.\n\n"
+
 			"flow flush {port_id}\n"
 			"    Destroy all flow rules.\n\n"
 
diff --git a/app/test-pmd/cmdline_flow.c b/app/test-pmd/cmdline_flow.c
index 359c187b3c..0af8d13121 100644
--- a/app/test-pmd/cmdline_flow.c
+++ b/app/test-pmd/cmdline_flow.c
@@ -91,6 +91,7 @@ enum index {
 	VALIDATE,
 	CREATE,
 	DESTROY,
+	ACTIONS_UPDATE,
 	FLUSH,
 	DUMP,
 	QUERY,
@@ -250,6 +251,7 @@ enum index {
 	VC_TUNNEL_SET,
 	VC_TUNNEL_MATCH,
 	VC_USER_ID,
+	VC_IS_USER_ID,
 
 	/* Dump arguments */
 	DUMP_ALL,
@@ -3084,6 +3086,7 @@ static const struct token token_list[] = {
 			      VALIDATE,
 			      CREATE,
 			      DESTROY,
+			      ACTIONS_UPDATE,
 			      FLUSH,
 			      DUMP,
 			      LIST,
@@ -3888,6 +3891,17 @@ static const struct token token_list[] = {
 		.args = ARGS(ARGS_ENTRY(struct buffer, port)),
 		.call = parse_destroy,
 	},
+	[ACTIONS_UPDATE] = {
+		.name = "actions_update",
+		.help = "update a flow rule with new actions",
+		.next = NEXT(NEXT_ENTRY(VC_IS_USER_ID, END),
+			     NEXT_ENTRY(ACTIONS),
+			     NEXT_ENTRY(COMMON_RULE_ID),
+			     NEXT_ENTRY(COMMON_PORT_ID)),
+		.args = ARGS(ARGS_ENTRY(struct buffer, args.vc.rule_id),
+			     ARGS_ENTRY(struct buffer, port)),
+		.call = parse_vc,
+	},
 	[FLUSH] = {
 		.name = "flush",
 		.help = "destroy all flow rules",
@@ -4134,6 +4148,11 @@ static const struct token token_list[] = {
 		.args = ARGS(ARGS_ENTRY(struct buffer, args.vc.user_id)),
 		.call = parse_vc,
 	},
+	[VC_IS_USER_ID] = {
+		.name = "user_id",
+		.help = "rule identifier is user-id",
+		.call = parse_vc,
+	},
 	/* Validate/create pattern. */
 	[ITEM_PATTERN] = {
 		.name = "pattern",
@@ -8083,8 +8102,13 @@ parse_vc(struct context *ctx, const struct token *token,
 	if (!out->command) {
 		if (ctx->curr != VALIDATE && ctx->curr != CREATE &&
 		    ctx->curr != PATTERN_TEMPLATE_CREATE &&
-		    ctx->curr != ACTIONS_TEMPLATE_CREATE)
+		    ctx->curr != ACTIONS_TEMPLATE_CREATE &&
+		    ctx->curr != ACTIONS_UPDATE)
 			return -1;
+		if (ctx->curr == ACTIONS_UPDATE)
+			out->args.vc.pattern =
+				(void *)RTE_ALIGN_CEIL((uintptr_t)(out + 1),
+						       sizeof(double));
 		if (sizeof(*out) > size)
 			return -1;
 		out->command = ctx->curr;
@@ -8156,6 +8180,9 @@ parse_vc(struct context *ctx, const struct token *token,
 		ctx->object = out->args.vc.actions;
 		ctx->objmask = NULL;
 		return len;
+	case VC_IS_USER_ID:
+		out->args.vc.user_id = true;
+		return len;
 	default:
 		if (!token->priv)
 			return -1;
@@ -12713,6 +12740,11 @@ cmd_flow_parsed(const struct buffer *in)
 				  in->args.destroy.rule,
 				  in->args.destroy.is_user_id);
 		break;
+	case ACTIONS_UPDATE:
+		port_flow_actions_update(in->port, in->args.vc.rule_id,
+					 in->args.vc.actions,
+					 in->args.vc.user_id);
+		break;
 	case FLUSH:
 		port_flow_flush(in->port);
 		break;
diff --git a/app/test-pmd/config.c b/app/test-pmd/config.c
index cad7537bc6..09d44908b0 100644
--- a/app/test-pmd/config.c
+++ b/app/test-pmd/config.c
@@ -3696,6 +3696,49 @@ port_flow_destroy(portid_t port_id, uint32_t n, const uint64_t *rule,
 	return ret;
 }
 
+/** Update a flow rule with new actions. */
+int
+port_flow_actions_update(portid_t port_id, uint32_t rule_id,
+			 const struct rte_flow_action *actions, bool is_user_id)
+{
+	struct rte_port *port;
+	struct port_flow **flow_list;
+
+	if (port_id_is_invalid(port_id, ENABLED_WARN) ||
+	    port_id == (portid_t)RTE_PORT_ALL)
+		return -EINVAL;
+	port = &ports[port_id];
+	flow_list = &port->flow_list;
+	while (*flow_list) {
+		struct port_flow *flow = *flow_list;
+		struct rte_flow_error error;
+
+		if (rule_id != (is_user_id ? flow->user_id : flow->id)) {
+			flow_list = &flow->next;
+			continue;
+		}
+		/*
+		 * Poisoning to make sure PMDs update it in case
+		 * of error.
+		 */
+		memset(&error, 0x33, sizeof(error));
+		if (rte_flow_actions_update(port_id, flow->flow, actions,
+					    &error))
+			return port_flow_complain(&error);
+		if (is_user_id)
+			printf("Flow rule #%"PRIu64" updated with new actions,"
+			       " user-id 0x%"PRIx64"\n",
+			       flow->id, flow->user_id);
+		else
+			printf("Flow rule #%"PRIu64
+			       " updated with new actions\n",
+			       flow->id);
+		return 0;
+	}
+	printf("Failed to find flow %"PRIu32"\n", rule_id);
+	return -EINVAL;
+}
+
 /** Remove all flow rules. */
 int
 port_flow_flush(portid_t port_id)
diff --git a/app/test-pmd/testpmd.h b/app/test-pmd/testpmd.h
index 9b10a9ea1c..7ff76658e7 100644
--- a/app/test-pmd/testpmd.h
+++ b/app/test-pmd/testpmd.h
@@ -1034,6 +1034,9 @@ void update_age_action_context(const struct rte_flow_action *actions,
 int mcast_addr_pool_destroy(portid_t port_id);
 int port_flow_destroy(portid_t port_id, uint32_t n, const uint64_t *rule,
 		      bool is_user_id);
+int port_flow_actions_update(portid_t port_id, uint32_t rule,
+			     const struct rte_flow_action *actions,
+			     bool is_user_id);
 int port_flow_flush(portid_t port_id);
 int port_flow_dump(portid_t port_id, bool dump_all,
 			uint64_t rule, const char *file_name,
diff --git a/doc/guides/testpmd_app_ug/testpmd_funcs.rst b/doc/guides/testpmd_app_ug/testpmd_funcs.rst
index ab18a80b30..274cb96724 100644
--- a/doc/guides/testpmd_app_ug/testpmd_funcs.rst
+++ b/doc/guides/testpmd_app_ug/testpmd_funcs.rst
@@ -3046,6 +3046,11 @@ following sections.
 
    flow destroy {port_id} rule {rule_id} [...] [user_id]
 
+- Update a flow rule with new actions::
+
+   flow actions_update {port_id} {rule_id}
+       actions {action} [/ {action} [...]] / end [user_id]
+
 - Destroy all flow rules::
 
    flow flush {port_id}
@@ -4235,6 +4240,45 @@ Non-existent rule IDs are ignored::
    Flow rule #0 destroyed
    testpmd>
 
+Updating flow rules with new actions
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+
+``flow actions_update`` updates a flow rule specified by a rule ID with a new
+action list by making a call to ``rte_flow_actions_update()``::
+
+   flow actions_update {port_id} {rule_id}
+       actions {action} [/ {action} [...]] / end [user_id]
+
+If successful, it will show::
+
+   Flow rule #[...] updated with new actions
+
+Or if ``user_id`` flag is provided::
+
+   Flow rule #[...] updated with new actions, user-id [...]
+
+If a flow rule can not be found::
+
+   Failed to find flow [...]
+
+Otherwise it will show the usual error message of the form::
+
+   Caught error type [...] ([...]): [...]
+
+Optional ``user_id`` is a flag that signifies the rule ID is the one provided
+by the user at creation.
+
+Action list is identical to the one described for the ``flow create``.
+
+Creating, updating and destroying a flow rule::
+
+   testpmd> flow create 0 group 1 pattern eth / end actions drop / end
+   Flow rule #0 created
+   testpmd> flow actions_update 0 0 actions queue index 1 / end
+   Flow rule #0 updated with new actions
+   testpmd> flow destroy 0 rule 0
+   Flow rule #0 destroyed
+
 Enqueueing destruction of flow rules
 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
 
-- 
2.43.0


^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH v2] app/testpmd: support updating flow rule actions
  2024-01-26 13:21 [PATCH] app/testpmd: support updating flow rule actions Oleksandr Kolomeiets
@ 2024-02-01  9:59 ` Oleksandr Kolomeiets
  2024-02-02  9:55   ` Thomas Monjalon
  2024-02-22  9:04   ` [PATCH v3] " Oleksandr Kolomeiets
  0 siblings, 2 replies; 8+ messages in thread
From: Oleksandr Kolomeiets @ 2024-02-01  9:59 UTC (permalink / raw)
  To: thomas, aman.deep.singh, yuying.zhang, orika, mko-plv, ckm; +Cc: dev

"flow actions_update" updates a flow rule specified by a rule ID with a
new action list by making a call to "rte_flow_actions_update()":

    flow actions_update {port_id} {rule_id}
        actions {action} [/ {action} [...]] / end [user_id]

Creating, updating and destroying a flow rule:

    testpmd> flow create 0 group 1 pattern eth / end actions drop / end
    Flow rule #0 created
    testpmd> flow actions_update 0 0 actions queue index 1 / end
    Flow rule #0 updated with new actions
    testpmd> flow destroy 0 rule 0
    Flow rule #0 destroyed

Signed-off-by: Oleksandr Kolomeiets <okl-plv@napatech.com>
Reviewed-by: Mykola Kostenok <mko-plv@napatech.com>
Reviewed-by: Christian Koue Muf <ckm@napatech.com>
---
v2:
* No changes.
---
 .mailmap                                    |  3 ++
 app/test-pmd/cmdline.c                      |  4 ++
 app/test-pmd/cmdline_flow.c                 | 34 +++++++++++++++-
 app/test-pmd/config.c                       | 43 ++++++++++++++++++++
 app/test-pmd/testpmd.h                      |  3 ++
 doc/guides/testpmd_app_ug/testpmd_funcs.rst | 44 +++++++++++++++++++++
 6 files changed, 130 insertions(+), 1 deletion(-)

diff --git a/.mailmap b/.mailmap
index 6011526cae..44da31c1c2 100644
--- a/.mailmap
+++ b/.mailmap
@@ -232,6 +232,7 @@ Chintu Hetam <rometoroam@gmail.com>
 Choonho Son <choonho.son@gmail.com>
 Chris Metcalf <cmetcalf@mellanox.com>
 Christian Ehrhardt <christian.ehrhardt@canonical.com>
+Christian Koue Muf <ckm@napatech.com>
 Christian Maciocco <christian.maciocco@intel.com>
 Christophe Fontaine <cfontain@redhat.com>
 Christophe Grosse <christophe.grosse@6wind.com>
@@ -986,6 +987,7 @@ Mukesh Dua <mukesh.dua81@gmail.com>
 Murphy Yang <murphyx.yang@intel.com>
 Murthy NSSR <nidadavolu.murthy@caviumnetworks.com>
 Muthurajan Jayakumar <muthurajan.jayakumar@intel.com>
+Mykola Kostenok <mko-plv@napatech.com>
 Nachiketa Prachanda <nprachan@brocade.com> <nprachan@vyatta.att-mail.com>
 Nagadheeraj Rottela <rnagadheeraj@marvell.com>
 Naga Harish K S V <s.v.naga.harish.k@intel.com>
@@ -1040,6 +1042,7 @@ Odi Assli <odia@nvidia.com>
 Ognjen Joldzic <ognjen.joldzic@gmail.com>
 Ola Liljedahl <ola.liljedahl@arm.com>
 Oleg Polyakov <olegp123@walla.co.il>
+Oleksandr Kolomeiets <okl-plv@napatech.com>
 Olga Shern <olgas@nvidia.com> <olgas@mellanox.com>
 Olivier Gournet <ogournet@corp.free.fr>
 Olivier Matz <olivier.matz@6wind.com>
diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c
index f704319771..8249e4eb92 100644
--- a/app/test-pmd/cmdline.c
+++ b/app/test-pmd/cmdline.c
@@ -852,6 +852,10 @@ static void cmd_help_long_parsed(void *parsed_result,
 			"flow destroy {port_id} rule {rule_id} [...]\n"
 			"    Destroy specific flow rules.\n\n"
 
+			"flow actions_update {port_id} {rule_id}"
+			" actions {action} [/ {action} [...]] / end [user_id]\n"
+			"    Update a flow rule with new actions.\n\n"
+
 			"flow flush {port_id}\n"
 			"    Destroy all flow rules.\n\n"
 
diff --git a/app/test-pmd/cmdline_flow.c b/app/test-pmd/cmdline_flow.c
index 359c187b3c..0af8d13121 100644
--- a/app/test-pmd/cmdline_flow.c
+++ b/app/test-pmd/cmdline_flow.c
@@ -91,6 +91,7 @@ enum index {
 	VALIDATE,
 	CREATE,
 	DESTROY,
+	ACTIONS_UPDATE,
 	FLUSH,
 	DUMP,
 	QUERY,
@@ -250,6 +251,7 @@ enum index {
 	VC_TUNNEL_SET,
 	VC_TUNNEL_MATCH,
 	VC_USER_ID,
+	VC_IS_USER_ID,
 
 	/* Dump arguments */
 	DUMP_ALL,
@@ -3084,6 +3086,7 @@ static const struct token token_list[] = {
 			      VALIDATE,
 			      CREATE,
 			      DESTROY,
+			      ACTIONS_UPDATE,
 			      FLUSH,
 			      DUMP,
 			      LIST,
@@ -3888,6 +3891,17 @@ static const struct token token_list[] = {
 		.args = ARGS(ARGS_ENTRY(struct buffer, port)),
 		.call = parse_destroy,
 	},
+	[ACTIONS_UPDATE] = {
+		.name = "actions_update",
+		.help = "update a flow rule with new actions",
+		.next = NEXT(NEXT_ENTRY(VC_IS_USER_ID, END),
+			     NEXT_ENTRY(ACTIONS),
+			     NEXT_ENTRY(COMMON_RULE_ID),
+			     NEXT_ENTRY(COMMON_PORT_ID)),
+		.args = ARGS(ARGS_ENTRY(struct buffer, args.vc.rule_id),
+			     ARGS_ENTRY(struct buffer, port)),
+		.call = parse_vc,
+	},
 	[FLUSH] = {
 		.name = "flush",
 		.help = "destroy all flow rules",
@@ -4134,6 +4148,11 @@ static const struct token token_list[] = {
 		.args = ARGS(ARGS_ENTRY(struct buffer, args.vc.user_id)),
 		.call = parse_vc,
 	},
+	[VC_IS_USER_ID] = {
+		.name = "user_id",
+		.help = "rule identifier is user-id",
+		.call = parse_vc,
+	},
 	/* Validate/create pattern. */
 	[ITEM_PATTERN] = {
 		.name = "pattern",
@@ -8083,8 +8102,13 @@ parse_vc(struct context *ctx, const struct token *token,
 	if (!out->command) {
 		if (ctx->curr != VALIDATE && ctx->curr != CREATE &&
 		    ctx->curr != PATTERN_TEMPLATE_CREATE &&
-		    ctx->curr != ACTIONS_TEMPLATE_CREATE)
+		    ctx->curr != ACTIONS_TEMPLATE_CREATE &&
+		    ctx->curr != ACTIONS_UPDATE)
 			return -1;
+		if (ctx->curr == ACTIONS_UPDATE)
+			out->args.vc.pattern =
+				(void *)RTE_ALIGN_CEIL((uintptr_t)(out + 1),
+						       sizeof(double));
 		if (sizeof(*out) > size)
 			return -1;
 		out->command = ctx->curr;
@@ -8156,6 +8180,9 @@ parse_vc(struct context *ctx, const struct token *token,
 		ctx->object = out->args.vc.actions;
 		ctx->objmask = NULL;
 		return len;
+	case VC_IS_USER_ID:
+		out->args.vc.user_id = true;
+		return len;
 	default:
 		if (!token->priv)
 			return -1;
@@ -12713,6 +12740,11 @@ cmd_flow_parsed(const struct buffer *in)
 				  in->args.destroy.rule,
 				  in->args.destroy.is_user_id);
 		break;
+	case ACTIONS_UPDATE:
+		port_flow_actions_update(in->port, in->args.vc.rule_id,
+					 in->args.vc.actions,
+					 in->args.vc.user_id);
+		break;
 	case FLUSH:
 		port_flow_flush(in->port);
 		break;
diff --git a/app/test-pmd/config.c b/app/test-pmd/config.c
index cad7537bc6..09d44908b0 100644
--- a/app/test-pmd/config.c
+++ b/app/test-pmd/config.c
@@ -3696,6 +3696,49 @@ port_flow_destroy(portid_t port_id, uint32_t n, const uint64_t *rule,
 	return ret;
 }
 
+/** Update a flow rule with new actions. */
+int
+port_flow_actions_update(portid_t port_id, uint32_t rule_id,
+			 const struct rte_flow_action *actions, bool is_user_id)
+{
+	struct rte_port *port;
+	struct port_flow **flow_list;
+
+	if (port_id_is_invalid(port_id, ENABLED_WARN) ||
+	    port_id == (portid_t)RTE_PORT_ALL)
+		return -EINVAL;
+	port = &ports[port_id];
+	flow_list = &port->flow_list;
+	while (*flow_list) {
+		struct port_flow *flow = *flow_list;
+		struct rte_flow_error error;
+
+		if (rule_id != (is_user_id ? flow->user_id : flow->id)) {
+			flow_list = &flow->next;
+			continue;
+		}
+		/*
+		 * Poisoning to make sure PMDs update it in case
+		 * of error.
+		 */
+		memset(&error, 0x33, sizeof(error));
+		if (rte_flow_actions_update(port_id, flow->flow, actions,
+					    &error))
+			return port_flow_complain(&error);
+		if (is_user_id)
+			printf("Flow rule #%"PRIu64" updated with new actions,"
+			       " user-id 0x%"PRIx64"\n",
+			       flow->id, flow->user_id);
+		else
+			printf("Flow rule #%"PRIu64
+			       " updated with new actions\n",
+			       flow->id);
+		return 0;
+	}
+	printf("Failed to find flow %"PRIu32"\n", rule_id);
+	return -EINVAL;
+}
+
 /** Remove all flow rules. */
 int
 port_flow_flush(portid_t port_id)
diff --git a/app/test-pmd/testpmd.h b/app/test-pmd/testpmd.h
index 9b10a9ea1c..7ff76658e7 100644
--- a/app/test-pmd/testpmd.h
+++ b/app/test-pmd/testpmd.h
@@ -1034,6 +1034,9 @@ void update_age_action_context(const struct rte_flow_action *actions,
 int mcast_addr_pool_destroy(portid_t port_id);
 int port_flow_destroy(portid_t port_id, uint32_t n, const uint64_t *rule,
 		      bool is_user_id);
+int port_flow_actions_update(portid_t port_id, uint32_t rule,
+			     const struct rte_flow_action *actions,
+			     bool is_user_id);
 int port_flow_flush(portid_t port_id);
 int port_flow_dump(portid_t port_id, bool dump_all,
 			uint64_t rule, const char *file_name,
diff --git a/doc/guides/testpmd_app_ug/testpmd_funcs.rst b/doc/guides/testpmd_app_ug/testpmd_funcs.rst
index ab18a80b30..274cb96724 100644
--- a/doc/guides/testpmd_app_ug/testpmd_funcs.rst
+++ b/doc/guides/testpmd_app_ug/testpmd_funcs.rst
@@ -3046,6 +3046,11 @@ following sections.
 
    flow destroy {port_id} rule {rule_id} [...] [user_id]
 
+- Update a flow rule with new actions::
+
+   flow actions_update {port_id} {rule_id}
+       actions {action} [/ {action} [...]] / end [user_id]
+
 - Destroy all flow rules::
 
    flow flush {port_id}
@@ -4235,6 +4240,45 @@ Non-existent rule IDs are ignored::
    Flow rule #0 destroyed
    testpmd>
 
+Updating flow rules with new actions
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+
+``flow actions_update`` updates a flow rule specified by a rule ID with a new
+action list by making a call to ``rte_flow_actions_update()``::
+
+   flow actions_update {port_id} {rule_id}
+       actions {action} [/ {action} [...]] / end [user_id]
+
+If successful, it will show::
+
+   Flow rule #[...] updated with new actions
+
+Or if ``user_id`` flag is provided::
+
+   Flow rule #[...] updated with new actions, user-id [...]
+
+If a flow rule can not be found::
+
+   Failed to find flow [...]
+
+Otherwise it will show the usual error message of the form::
+
+   Caught error type [...] ([...]): [...]
+
+Optional ``user_id`` is a flag that signifies the rule ID is the one provided
+by the user at creation.
+
+Action list is identical to the one described for the ``flow create``.
+
+Creating, updating and destroying a flow rule::
+
+   testpmd> flow create 0 group 1 pattern eth / end actions drop / end
+   Flow rule #0 created
+   testpmd> flow actions_update 0 0 actions queue index 1 / end
+   Flow rule #0 updated with new actions
+   testpmd> flow destroy 0 rule 0
+   Flow rule #0 destroyed
+
 Enqueueing destruction of flow rules
 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
 
-- 
2.43.0


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v2] app/testpmd: support updating flow rule actions
  2024-02-01  9:59 ` [PATCH v2] " Oleksandr Kolomeiets
@ 2024-02-02  9:55   ` Thomas Monjalon
  2024-02-05 19:18     ` Dariusz Sosnowski
  2024-02-22  9:04   ` [PATCH v3] " Oleksandr Kolomeiets
  1 sibling, 1 reply; 8+ messages in thread
From: Thomas Monjalon @ 2024-02-02  9:55 UTC (permalink / raw)
  To: mko-plv, ckm, Oleksandr Kolomeiets
  Cc: aman.deep.singh, yuying.zhang, dev, Ori Kam, Dariusz Sosnowski

01/02/2024 10:59, Oleksandr Kolomeiets:
> "flow actions_update" updates a flow rule specified by a rule ID with a
> new action list by making a call to "rte_flow_actions_update()":
> 
>     flow actions_update {port_id} {rule_id}
>         actions {action} [/ {action} [...]] / end [user_id]
> 
> Creating, updating and destroying a flow rule:
> 
>     testpmd> flow create 0 group 1 pattern eth / end actions drop / end
>     Flow rule #0 created
>     testpmd> flow actions_update 0 0 actions queue index 1 / end
>     Flow rule #0 updated with new actions
>     testpmd> flow destroy 0 rule 0
>     Flow rule #0 destroyed

Why not a simple "flow update" command name?



^ permalink raw reply	[flat|nested] 8+ messages in thread

* RE: [PATCH v2] app/testpmd: support updating flow rule actions
  2024-02-02  9:55   ` Thomas Monjalon
@ 2024-02-05 19:18     ` Dariusz Sosnowski
  2024-02-12  8:37       ` Oleksandr Kolomeiets
  0 siblings, 1 reply; 8+ messages in thread
From: Dariusz Sosnowski @ 2024-02-05 19:18 UTC (permalink / raw)
  To: NBU-Contact-Thomas Monjalon (EXTERNAL),
	mko-plv, ckm, Oleksandr Kolomeiets
  Cc: aman.deep.singh, yuying.zhang, dev, Ori Kam

> -----Original Message-----
> From: Thomas Monjalon <thomas@monjalon.net>
> Sent: Friday, February 2, 2024 10:55
> To: mko-plv@napatech.com; ckm@napatech.com; Oleksandr Kolomeiets
> <okl-plv@napatech.com>
> Cc: aman.deep.singh@intel.com; yuying.zhang@intel.com; dev@dpdk.org;
> Ori Kam <orika@nvidia.com>; Dariusz Sosnowski <dsosnowski@nvidia.com>
> Subject: Re: [PATCH v2] app/testpmd: support updating flow rule actions
> 
> 01/02/2024 10:59, Oleksandr Kolomeiets:
> > "flow actions_update" updates a flow rule specified by a rule ID with
> > a new action list by making a call to "rte_flow_actions_update()":
> >
> >     flow actions_update {port_id} {rule_id}
> >         actions {action} [/ {action} [...]] / end [user_id]
> >
> > Creating, updating and destroying a flow rule:
> >
> >     testpmd> flow create 0 group 1 pattern eth / end actions drop / end
> >     Flow rule #0 created
> >     testpmd> flow actions_update 0 0 actions queue index 1 / end
> >     Flow rule #0 updated with new actions
> >     testpmd> flow destroy 0 rule 0
> >     Flow rule #0 destroyed
> 
> Why not a simple "flow update" command name?

+1. This would also make it consistent with async version of this command - "flow queue {port_id} update ...".

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v2] app/testpmd: support updating flow rule actions
  2024-02-05 19:18     ` Dariusz Sosnowski
@ 2024-02-12  8:37       ` Oleksandr Kolomeiets
  2024-02-21 10:32         ` Thomas Monjalon
  0 siblings, 1 reply; 8+ messages in thread
From: Oleksandr Kolomeiets @ 2024-02-12  8:37 UTC (permalink / raw)
  To: Dariusz Sosnowski, NBU-Contact-Thomas Monjalon (EXTERNAL),
	Mykola Kostenok, Christian Koue Muf
  Cc: aman.deep.singh, yuying.zhang, dev, Ori Kam

[-- Attachment #1: Type: text/plain, Size: 2343 bytes --]

Thank you for the review.
Indeed, shortening the command from "flow actions_update" to "flow update" seems more natural.
However, note that the command updates only the actions of a flow rule and leaves all other parameters unchanged.
My concern is that in the future there can be some "flow pattern_update" command, thus making "flow update" command ambiguous.
Also, the name is consistent with the underlying rte_flow_actions_update() function.
With that in mind, please clarify if the name should still be changed.
________________________________
From: Dariusz Sosnowski <dsosnowski@nvidia.com>
Sent: Monday, February 5, 2024 9:18 PM
To: NBU-Contact-Thomas Monjalon (EXTERNAL) <thomas@monjalon.net>; Mykola Kostenok <mko-plv@napatech.com>; Christian Koue Muf <ckm@napatech.com>; Oleksandr Kolomeiets <okl-plv@napatech.com>
Cc: aman.deep.singh@intel.com <aman.deep.singh@intel.com>; yuying.zhang@intel.com <yuying.zhang@intel.com>; dev@dpdk.org <dev@dpdk.org>; Ori Kam <orika@nvidia.com>
Subject: RE: [PATCH v2] app/testpmd: support updating flow rule actions

> -----Original Message-----
> From: Thomas Monjalon <thomas@monjalon.net>
> Sent: Friday, February 2, 2024 10:55
> To: mko-plv@napatech.com; ckm@napatech.com; Oleksandr Kolomeiets
> <okl-plv@napatech.com>
> Cc: aman.deep.singh@intel.com; yuying.zhang@intel.com; dev@dpdk.org;
> Ori Kam <orika@nvidia.com>; Dariusz Sosnowski <dsosnowski@nvidia.com>
> Subject: Re: [PATCH v2] app/testpmd: support updating flow rule actions
>
> 01/02/2024 10:59, Oleksandr Kolomeiets:
> > "flow actions_update" updates a flow rule specified by a rule ID with
> > a new action list by making a call to "rte_flow_actions_update()":
> >
> >     flow actions_update {port_id} {rule_id}
> >         actions {action} [/ {action} [...]] / end [user_id]
> >
> > Creating, updating and destroying a flow rule:
> >
> >     testpmd> flow create 0 group 1 pattern eth / end actions drop / end
> >     Flow rule #0 created
> >     testpmd> flow actions_update 0 0 actions queue index 1 / end
> >     Flow rule #0 updated with new actions
> >     testpmd> flow destroy 0 rule 0
> >     Flow rule #0 destroyed
>
> Why not a simple "flow update" command name?

+1. This would also make it consistent with async version of this command - "flow queue {port_id} update ...".

[-- Attachment #2: Type: text/html, Size: 3730 bytes --]

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v2] app/testpmd: support updating flow rule actions
  2024-02-12  8:37       ` Oleksandr Kolomeiets
@ 2024-02-21 10:32         ` Thomas Monjalon
  0 siblings, 0 replies; 8+ messages in thread
From: Thomas Monjalon @ 2024-02-21 10:32 UTC (permalink / raw)
  To: Oleksandr Kolomeiets
  Cc: Dariusz Sosnowski, Mykola Kostenok, Christian Koue Muf, dev,
	aman.deep.singh, yuying.zhang, dev, Ori Kam

12/02/2024 09:37, Oleksandr Kolomeiets:
> From: Dariusz Sosnowski <dsosnowski@nvidia.com>
> > From: Thomas Monjalon <thomas@monjalon.net>
> > > 01/02/2024 10:59, Oleksandr Kolomeiets:
> > > > "flow actions_update" updates a flow rule specified by a rule ID with
> > > > 
> > > > a new action list by making a call to "rte_flow_actions_update()":
> > > >     flow actions_update {port_id} {rule_id}
> > > >     
> > > >         actions {action} [/ {action} [...]] / end [user_id]
> > > > 
> > > > Creating, updating and destroying a flow rule:
> > > >     testpmd> flow create 0 group 1 pattern eth / end actions drop /
> > > >     end
> > > >     Flow rule #0 created
> > > >     testpmd> flow actions_update 0 0 actions queue index 1 / end
> > > >     Flow rule #0 updated with new actions
> > > >     testpmd> flow destroy 0 rule 0
> > > >     Flow rule #0 destroyed
> > > 
> > > Why not a simple "flow update" command name?
> > 
> > +1. This would also make it consistent with async version of this command
> > - "flow queue {port_id} update ...".
> 
> Indeed, shortening the command from "flow actions_update" to "flow update" seems more natural.
> However, note that the command updates only the actions of a flow rule and leaves all other parameters unchanged.
> My concern is that in the future there can be some "flow pattern_update" command, thus making "flow update" command ambiguous.
> Also, the name is consistent with the underlying rte_flow_actions_update() function.
> With that in mind, please clarify if the name should still be changed.

If a function is added for pattern update, we could still implement it
with the same command prefix "flow update" and call functions as appropriate.
So yes I still think the command should be "flow update".



^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH v3] app/testpmd: support updating flow rule actions
  2024-02-01  9:59 ` [PATCH v2] " Oleksandr Kolomeiets
  2024-02-02  9:55   ` Thomas Monjalon
@ 2024-02-22  9:04   ` Oleksandr Kolomeiets
  2024-02-23  4:19     ` [PATCH v4] " Oleksandr Kolomeiets
  1 sibling, 1 reply; 8+ messages in thread
From: Oleksandr Kolomeiets @ 2024-02-22  9:04 UTC (permalink / raw)
  To: thomas, aman.deep.singh, yuying.zhang, orika, mko-plv, ckm; +Cc: dev

"flow update" updates a flow rule specified by a rule ID with a
new action list by making a call to "rte_flow_actions_update()":

    flow update {port_id} {rule_id}
        actions {action} [/ {action} [...]] / end [user_id]

Creating, updating and destroying a flow rule:

    testpmd> flow create 0 group 1 pattern eth / end actions drop / end
    Flow rule #0 created
    testpmd> flow update 0 0 actions queue index 1 / end
    Flow rule #0 updated with new actions
    testpmd> flow destroy 0 rule 0
    Flow rule #0 destroyed

Signed-off-by: Oleksandr Kolomeiets <okl-plv@napatech.com>
Reviewed-by: Mykola Kostenok <mko-plv@napatech.com>
Reviewed-by: Christian Koue Muf <ckm@napatech.com>
---
v3:
* Renamed "flow actions_update" command to "flow update".

v2:
* No changes.
---
 .mailmap                                    |  3 ++
 app/test-pmd/cmdline.c                      |  4 ++
 app/test-pmd/cmdline_flow.c                 | 33 +++++++++++++++-
 app/test-pmd/config.c                       | 43 ++++++++++++++++++++
 app/test-pmd/testpmd.h                      |  2 +
 doc/guides/testpmd_app_ug/testpmd_funcs.rst | 44 +++++++++++++++++++++
 6 files changed, 128 insertions(+), 1 deletion(-)

diff --git a/.mailmap b/.mailmap
index 12d2875641..c1e7aa9ae1 100644
--- a/.mailmap
+++ b/.mailmap
@@ -236,6 +236,7 @@ Chintu Hetam <rometoroam@gmail.com>
 Choonho Son <choonho.son@gmail.com>
 Chris Metcalf <cmetcalf@mellanox.com>
 Christian Ehrhardt <christian.ehrhardt@canonical.com>
+Christian Koue Muf <ckm@napatech.com>
 Christian Maciocco <christian.maciocco@intel.com>
 Christophe Fontaine <cfontain@redhat.com>
 Christophe Grosse <christophe.grosse@6wind.com>
@@ -992,6 +993,7 @@ Mukesh Dua <mukesh.dua81@gmail.com>
 Murphy Yang <murphyx.yang@intel.com>
 Murthy NSSR <nidadavolu.murthy@caviumnetworks.com>
 Muthurajan Jayakumar <muthurajan.jayakumar@intel.com>
+Mykola Kostenok <mko-plv@napatech.com>
 Nachiketa Prachanda <nprachan@brocade.com> <nprachan@vyatta.att-mail.com>
 Nagadheeraj Rottela <rnagadheeraj@marvell.com>
 Naga Harish K S V <s.v.naga.harish.k@intel.com>
@@ -1046,6 +1048,7 @@ Odi Assli <odia@nvidia.com>
 Ognjen Joldzic <ognjen.joldzic@gmail.com>
 Ola Liljedahl <ola.liljedahl@arm.com>
 Oleg Polyakov <olegp123@walla.co.il>
+Oleksandr Kolomeiets <okl-plv@napatech.com>
 Olga Shern <olgas@nvidia.com> <olgas@mellanox.com>
 Olivier Gournet <ogournet@corp.free.fr>
 Olivier Matz <olivier.matz@6wind.com>
diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c
index 566057caa5..f521a1fe9e 100644
--- a/app/test-pmd/cmdline.c
+++ b/app/test-pmd/cmdline.c
@@ -851,6 +851,10 @@ static void cmd_help_long_parsed(void *parsed_result,
 			"flow destroy {port_id} rule {rule_id} [...]\n"
 			"    Destroy specific flow rules.\n\n"
 
+			"flow update {port_id} {rule_id}"
+			" actions {action} [/ {action} [...]] / end [user_id]\n"
+			"    Update a flow rule with new actions.\n\n"
+
 			"flow flush {port_id}\n"
 			"    Destroy all flow rules.\n\n"
 
diff --git a/app/test-pmd/cmdline_flow.c b/app/test-pmd/cmdline_flow.c
index 8e5eb41d71..d9d261ebd2 100644
--- a/app/test-pmd/cmdline_flow.c
+++ b/app/test-pmd/cmdline_flow.c
@@ -91,6 +91,7 @@ enum index {
 	VALIDATE,
 	CREATE,
 	DESTROY,
+	UPDATE,
 	FLUSH,
 	DUMP,
 	QUERY,
@@ -260,6 +261,7 @@ enum index {
 	VC_TUNNEL_SET,
 	VC_TUNNEL_MATCH,
 	VC_USER_ID,
+	VC_IS_USER_ID,
 
 	/* Dump arguments */
 	DUMP_ALL,
@@ -3200,6 +3202,7 @@ static const struct token token_list[] = {
 			      VALIDATE,
 			      CREATE,
 			      DESTROY,
+			      UPDATE,
 			      FLUSH,
 			      DUMP,
 			      LIST,
@@ -4065,6 +4068,17 @@ static const struct token token_list[] = {
 		.args = ARGS(ARGS_ENTRY(struct buffer, port)),
 		.call = parse_destroy,
 	},
+	[UPDATE] = {
+		.name = "update",
+		.help = "update a flow rule with new actions",
+		.next = NEXT(NEXT_ENTRY(VC_IS_USER_ID, END),
+			     NEXT_ENTRY(ACTIONS),
+			     NEXT_ENTRY(COMMON_RULE_ID),
+			     NEXT_ENTRY(COMMON_PORT_ID)),
+		.args = ARGS(ARGS_ENTRY(struct buffer, args.vc.rule_id),
+			     ARGS_ENTRY(struct buffer, port)),
+		.call = parse_vc,
+	},
 	[FLUSH] = {
 		.name = "flush",
 		.help = "destroy all flow rules",
@@ -4311,6 +4325,11 @@ static const struct token token_list[] = {
 		.args = ARGS(ARGS_ENTRY(struct buffer, args.vc.user_id)),
 		.call = parse_vc,
 	},
+	[VC_IS_USER_ID] = {
+		.name = "user_id",
+		.help = "rule identifier is user-id",
+		.call = parse_vc,
+	},
 	/* Validate/create pattern. */
 	[ITEM_PATTERN] = {
 		.name = "pattern",
@@ -8450,8 +8469,13 @@ parse_vc(struct context *ctx, const struct token *token,
 	if (!out->command) {
 		if (ctx->curr != VALIDATE && ctx->curr != CREATE &&
 		    ctx->curr != PATTERN_TEMPLATE_CREATE &&
-		    ctx->curr != ACTIONS_TEMPLATE_CREATE)
+		    ctx->curr != ACTIONS_TEMPLATE_CREATE &&
+		    ctx->curr != UPDATE)
 			return -1;
+		if (ctx->curr == UPDATE)
+			out->args.vc.pattern =
+				(void *)RTE_ALIGN_CEIL((uintptr_t)(out + 1),
+						       sizeof(double));
 		if (sizeof(*out) > size)
 			return -1;
 		out->command = ctx->curr;
@@ -8523,6 +8547,9 @@ parse_vc(struct context *ctx, const struct token *token,
 		ctx->object = out->args.vc.actions;
 		ctx->objmask = NULL;
 		return len;
+	case VC_IS_USER_ID:
+		out->args.vc.user_id = true;
+		return len;
 	default:
 		if (!token->priv)
 			return -1;
@@ -13256,6 +13283,10 @@ cmd_flow_parsed(const struct buffer *in)
 				  in->args.destroy.rule,
 				  in->args.destroy.is_user_id);
 		break;
+	case UPDATE:
+		port_flow_update(in->port, in->args.vc.rule_id,
+				 in->args.vc.actions, in->args.vc.user_id);
+		break;
 	case FLUSH:
 		port_flow_flush(in->port);
 		break;
diff --git a/app/test-pmd/config.c b/app/test-pmd/config.c
index 08d7d0e2f8..cd2a436cd7 100644
--- a/app/test-pmd/config.c
+++ b/app/test-pmd/config.c
@@ -3829,6 +3829,49 @@ port_flow_destroy(portid_t port_id, uint32_t n, const uint64_t *rule,
 	return ret;
 }
 
+/** Update a flow rule with new actions. */
+int
+port_flow_update(portid_t port_id, uint32_t rule_id,
+		 const struct rte_flow_action *actions, bool is_user_id)
+{
+	struct rte_port *port;
+	struct port_flow **flow_list;
+
+	if (port_id_is_invalid(port_id, ENABLED_WARN) ||
+	    port_id == (portid_t)RTE_PORT_ALL)
+		return -EINVAL;
+	port = &ports[port_id];
+	flow_list = &port->flow_list;
+	while (*flow_list) {
+		struct port_flow *flow = *flow_list;
+		struct rte_flow_error error;
+
+		if (rule_id != (is_user_id ? flow->user_id : flow->id)) {
+			flow_list = &flow->next;
+			continue;
+		}
+		/*
+		 * Poisoning to make sure PMDs update it in case
+		 * of error.
+		 */
+		memset(&error, 0x33, sizeof(error));
+		if (rte_flow_actions_update(port_id, flow->flow, actions,
+					    &error))
+			return port_flow_complain(&error);
+		if (is_user_id)
+			printf("Flow rule #%"PRIu64" updated with new actions,"
+			       " user-id 0x%"PRIx64"\n",
+			       flow->id, flow->user_id);
+		else
+			printf("Flow rule #%"PRIu64
+			       " updated with new actions\n",
+			       flow->id);
+		return 0;
+	}
+	printf("Failed to find flow %"PRIu32"\n", rule_id);
+	return -EINVAL;
+}
+
 /** Remove all flow rules. */
 int
 port_flow_flush(portid_t port_id)
diff --git a/app/test-pmd/testpmd.h b/app/test-pmd/testpmd.h
index 467ef3a8ab..55df12033a 100644
--- a/app/test-pmd/testpmd.h
+++ b/app/test-pmd/testpmd.h
@@ -1043,6 +1043,8 @@ void update_age_action_context(const struct rte_flow_action *actions,
 int mcast_addr_pool_destroy(portid_t port_id);
 int port_flow_destroy(portid_t port_id, uint32_t n, const uint64_t *rule,
 		      bool is_user_id);
+int port_flow_update(portid_t port_id, uint32_t rule,
+		     const struct rte_flow_action *actions, bool is_user_id);
 int port_flow_flush(portid_t port_id);
 int port_flow_dump(portid_t port_id, bool dump_all,
 			uint64_t rule, const char *file_name,
diff --git a/doc/guides/testpmd_app_ug/testpmd_funcs.rst b/doc/guides/testpmd_app_ug/testpmd_funcs.rst
index 9f3949667f..1771115857 100644
--- a/doc/guides/testpmd_app_ug/testpmd_funcs.rst
+++ b/doc/guides/testpmd_app_ug/testpmd_funcs.rst
@@ -3057,6 +3057,11 @@ following sections.
 
    flow destroy {port_id} rule {rule_id} [...] [user_id]
 
+- Update a flow rule with new actions::
+
+   flow update {port_id} {rule_id}
+       actions {action} [/ {action} [...]] / end [user_id]
+
 - Destroy all flow rules::
 
    flow flush {port_id}
@@ -4275,6 +4280,45 @@ Non-existent rule IDs are ignored::
    Flow rule #0 destroyed
    testpmd>
 
+Updating flow rules with new actions
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+
+``flow update`` updates a flow rule specified by a rule ID with a new action
+list by making a call to ``rte_flow_actions_update()``::
+
+   flow update {port_id} {rule_id}
+       actions {action} [/ {action} [...]] / end [user_id]
+
+If successful, it will show::
+
+   Flow rule #[...] updated with new actions
+
+Or if ``user_id`` flag is provided::
+
+   Flow rule #[...] updated with new actions, user-id [...]
+
+If a flow rule can not be found::
+
+   Failed to find flow [...]
+
+Otherwise it will show the usual error message of the form::
+
+   Caught error type [...] ([...]): [...]
+
+Optional ``user_id`` is a flag that signifies the rule ID is the one provided
+by the user at creation.
+
+Action list is identical to the one described for the ``flow create``.
+
+Creating, updating and destroying a flow rule::
+
+   testpmd> flow create 0 group 1 pattern eth / end actions drop / end
+   Flow rule #0 created
+   testpmd> flow update 0 0 actions queue index 1 / end
+   Flow rule #0 updated with new actions
+   testpmd> flow destroy 0 rule 0
+   Flow rule #0 destroyed
+
 Enqueueing destruction of flow rules
 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
 
-- 
2.43.0


^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH v4] app/testpmd: support updating flow rule actions
  2024-02-22  9:04   ` [PATCH v3] " Oleksandr Kolomeiets
@ 2024-02-23  4:19     ` Oleksandr Kolomeiets
  0 siblings, 0 replies; 8+ messages in thread
From: Oleksandr Kolomeiets @ 2024-02-23  4:19 UTC (permalink / raw)
  To: thomas, aman.deep.singh, yuying.zhang, orika, mko-plv, ckm; +Cc: dev

"flow update" updates a flow rule specified by a rule ID with a
new action list by making a call to "rte_flow_actions_update()":

    flow update {port_id} {rule_id}
        actions {action} [/ {action} [...]] / end [user_id]

Creating, updating and destroying a flow rule:

    testpmd> flow create 0 group 1 pattern eth / end actions drop / end
    Flow rule #0 created
    testpmd> flow update 0 0 actions queue index 1 / end
    Flow rule #0 updated with new actions
    testpmd> flow destroy 0 rule 0
    Flow rule #0 destroyed

Signed-off-by: Oleksandr Kolomeiets <okl-plv@napatech.com>
Reviewed-by: Mykola Kostenok <mko-plv@napatech.com>
Reviewed-by: Christian Koue Muf <ckm@napatech.com>
---
v4:
* No changes.

v3:
* Renamed "flow actions_update" command to "flow update".

v2:
* No changes.
---
 .mailmap                                    |  3 ++
 app/test-pmd/cmdline.c                      |  4 ++
 app/test-pmd/cmdline_flow.c                 | 33 +++++++++++++++-
 app/test-pmd/config.c                       | 43 ++++++++++++++++++++
 app/test-pmd/testpmd.h                      |  2 +
 doc/guides/testpmd_app_ug/testpmd_funcs.rst | 44 +++++++++++++++++++++
 6 files changed, 128 insertions(+), 1 deletion(-)

diff --git a/.mailmap b/.mailmap
index 12d2875641..c1e7aa9ae1 100644
--- a/.mailmap
+++ b/.mailmap
@@ -236,6 +236,7 @@ Chintu Hetam <rometoroam@gmail.com>
 Choonho Son <choonho.son@gmail.com>
 Chris Metcalf <cmetcalf@mellanox.com>
 Christian Ehrhardt <christian.ehrhardt@canonical.com>
+Christian Koue Muf <ckm@napatech.com>
 Christian Maciocco <christian.maciocco@intel.com>
 Christophe Fontaine <cfontain@redhat.com>
 Christophe Grosse <christophe.grosse@6wind.com>
@@ -992,6 +993,7 @@ Mukesh Dua <mukesh.dua81@gmail.com>
 Murphy Yang <murphyx.yang@intel.com>
 Murthy NSSR <nidadavolu.murthy@caviumnetworks.com>
 Muthurajan Jayakumar <muthurajan.jayakumar@intel.com>
+Mykola Kostenok <mko-plv@napatech.com>
 Nachiketa Prachanda <nprachan@brocade.com> <nprachan@vyatta.att-mail.com>
 Nagadheeraj Rottela <rnagadheeraj@marvell.com>
 Naga Harish K S V <s.v.naga.harish.k@intel.com>
@@ -1046,6 +1048,7 @@ Odi Assli <odia@nvidia.com>
 Ognjen Joldzic <ognjen.joldzic@gmail.com>
 Ola Liljedahl <ola.liljedahl@arm.com>
 Oleg Polyakov <olegp123@walla.co.il>
+Oleksandr Kolomeiets <okl-plv@napatech.com>
 Olga Shern <olgas@nvidia.com> <olgas@mellanox.com>
 Olivier Gournet <ogournet@corp.free.fr>
 Olivier Matz <olivier.matz@6wind.com>
diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c
index 566057caa5..f521a1fe9e 100644
--- a/app/test-pmd/cmdline.c
+++ b/app/test-pmd/cmdline.c
@@ -851,6 +851,10 @@ static void cmd_help_long_parsed(void *parsed_result,
 			"flow destroy {port_id} rule {rule_id} [...]\n"
 			"    Destroy specific flow rules.\n\n"
 
+			"flow update {port_id} {rule_id}"
+			" actions {action} [/ {action} [...]] / end [user_id]\n"
+			"    Update a flow rule with new actions.\n\n"
+
 			"flow flush {port_id}\n"
 			"    Destroy all flow rules.\n\n"
 
diff --git a/app/test-pmd/cmdline_flow.c b/app/test-pmd/cmdline_flow.c
index 8e5eb41d71..d9d261ebd2 100644
--- a/app/test-pmd/cmdline_flow.c
+++ b/app/test-pmd/cmdline_flow.c
@@ -91,6 +91,7 @@ enum index {
 	VALIDATE,
 	CREATE,
 	DESTROY,
+	UPDATE,
 	FLUSH,
 	DUMP,
 	QUERY,
@@ -260,6 +261,7 @@ enum index {
 	VC_TUNNEL_SET,
 	VC_TUNNEL_MATCH,
 	VC_USER_ID,
+	VC_IS_USER_ID,
 
 	/* Dump arguments */
 	DUMP_ALL,
@@ -3200,6 +3202,7 @@ static const struct token token_list[] = {
 			      VALIDATE,
 			      CREATE,
 			      DESTROY,
+			      UPDATE,
 			      FLUSH,
 			      DUMP,
 			      LIST,
@@ -4065,6 +4068,17 @@ static const struct token token_list[] = {
 		.args = ARGS(ARGS_ENTRY(struct buffer, port)),
 		.call = parse_destroy,
 	},
+	[UPDATE] = {
+		.name = "update",
+		.help = "update a flow rule with new actions",
+		.next = NEXT(NEXT_ENTRY(VC_IS_USER_ID, END),
+			     NEXT_ENTRY(ACTIONS),
+			     NEXT_ENTRY(COMMON_RULE_ID),
+			     NEXT_ENTRY(COMMON_PORT_ID)),
+		.args = ARGS(ARGS_ENTRY(struct buffer, args.vc.rule_id),
+			     ARGS_ENTRY(struct buffer, port)),
+		.call = parse_vc,
+	},
 	[FLUSH] = {
 		.name = "flush",
 		.help = "destroy all flow rules",
@@ -4311,6 +4325,11 @@ static const struct token token_list[] = {
 		.args = ARGS(ARGS_ENTRY(struct buffer, args.vc.user_id)),
 		.call = parse_vc,
 	},
+	[VC_IS_USER_ID] = {
+		.name = "user_id",
+		.help = "rule identifier is user-id",
+		.call = parse_vc,
+	},
 	/* Validate/create pattern. */
 	[ITEM_PATTERN] = {
 		.name = "pattern",
@@ -8450,8 +8469,13 @@ parse_vc(struct context *ctx, const struct token *token,
 	if (!out->command) {
 		if (ctx->curr != VALIDATE && ctx->curr != CREATE &&
 		    ctx->curr != PATTERN_TEMPLATE_CREATE &&
-		    ctx->curr != ACTIONS_TEMPLATE_CREATE)
+		    ctx->curr != ACTIONS_TEMPLATE_CREATE &&
+		    ctx->curr != UPDATE)
 			return -1;
+		if (ctx->curr == UPDATE)
+			out->args.vc.pattern =
+				(void *)RTE_ALIGN_CEIL((uintptr_t)(out + 1),
+						       sizeof(double));
 		if (sizeof(*out) > size)
 			return -1;
 		out->command = ctx->curr;
@@ -8523,6 +8547,9 @@ parse_vc(struct context *ctx, const struct token *token,
 		ctx->object = out->args.vc.actions;
 		ctx->objmask = NULL;
 		return len;
+	case VC_IS_USER_ID:
+		out->args.vc.user_id = true;
+		return len;
 	default:
 		if (!token->priv)
 			return -1;
@@ -13256,6 +13283,10 @@ cmd_flow_parsed(const struct buffer *in)
 				  in->args.destroy.rule,
 				  in->args.destroy.is_user_id);
 		break;
+	case UPDATE:
+		port_flow_update(in->port, in->args.vc.rule_id,
+				 in->args.vc.actions, in->args.vc.user_id);
+		break;
 	case FLUSH:
 		port_flow_flush(in->port);
 		break;
diff --git a/app/test-pmd/config.c b/app/test-pmd/config.c
index 08d7d0e2f8..cd2a436cd7 100644
--- a/app/test-pmd/config.c
+++ b/app/test-pmd/config.c
@@ -3829,6 +3829,49 @@ port_flow_destroy(portid_t port_id, uint32_t n, const uint64_t *rule,
 	return ret;
 }
 
+/** Update a flow rule with new actions. */
+int
+port_flow_update(portid_t port_id, uint32_t rule_id,
+		 const struct rte_flow_action *actions, bool is_user_id)
+{
+	struct rte_port *port;
+	struct port_flow **flow_list;
+
+	if (port_id_is_invalid(port_id, ENABLED_WARN) ||
+	    port_id == (portid_t)RTE_PORT_ALL)
+		return -EINVAL;
+	port = &ports[port_id];
+	flow_list = &port->flow_list;
+	while (*flow_list) {
+		struct port_flow *flow = *flow_list;
+		struct rte_flow_error error;
+
+		if (rule_id != (is_user_id ? flow->user_id : flow->id)) {
+			flow_list = &flow->next;
+			continue;
+		}
+		/*
+		 * Poisoning to make sure PMDs update it in case
+		 * of error.
+		 */
+		memset(&error, 0x33, sizeof(error));
+		if (rte_flow_actions_update(port_id, flow->flow, actions,
+					    &error))
+			return port_flow_complain(&error);
+		if (is_user_id)
+			printf("Flow rule #%"PRIu64" updated with new actions,"
+			       " user-id 0x%"PRIx64"\n",
+			       flow->id, flow->user_id);
+		else
+			printf("Flow rule #%"PRIu64
+			       " updated with new actions\n",
+			       flow->id);
+		return 0;
+	}
+	printf("Failed to find flow %"PRIu32"\n", rule_id);
+	return -EINVAL;
+}
+
 /** Remove all flow rules. */
 int
 port_flow_flush(portid_t port_id)
diff --git a/app/test-pmd/testpmd.h b/app/test-pmd/testpmd.h
index 467ef3a8ab..55df12033a 100644
--- a/app/test-pmd/testpmd.h
+++ b/app/test-pmd/testpmd.h
@@ -1043,6 +1043,8 @@ void update_age_action_context(const struct rte_flow_action *actions,
 int mcast_addr_pool_destroy(portid_t port_id);
 int port_flow_destroy(portid_t port_id, uint32_t n, const uint64_t *rule,
 		      bool is_user_id);
+int port_flow_update(portid_t port_id, uint32_t rule,
+		     const struct rte_flow_action *actions, bool is_user_id);
 int port_flow_flush(portid_t port_id);
 int port_flow_dump(portid_t port_id, bool dump_all,
 			uint64_t rule, const char *file_name,
diff --git a/doc/guides/testpmd_app_ug/testpmd_funcs.rst b/doc/guides/testpmd_app_ug/testpmd_funcs.rst
index 9f3949667f..1771115857 100644
--- a/doc/guides/testpmd_app_ug/testpmd_funcs.rst
+++ b/doc/guides/testpmd_app_ug/testpmd_funcs.rst
@@ -3057,6 +3057,11 @@ following sections.
 
    flow destroy {port_id} rule {rule_id} [...] [user_id]
 
+- Update a flow rule with new actions::
+
+   flow update {port_id} {rule_id}
+       actions {action} [/ {action} [...]] / end [user_id]
+
 - Destroy all flow rules::
 
    flow flush {port_id}
@@ -4275,6 +4280,45 @@ Non-existent rule IDs are ignored::
    Flow rule #0 destroyed
    testpmd>
 
+Updating flow rules with new actions
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+
+``flow update`` updates a flow rule specified by a rule ID with a new action
+list by making a call to ``rte_flow_actions_update()``::
+
+   flow update {port_id} {rule_id}
+       actions {action} [/ {action} [...]] / end [user_id]
+
+If successful, it will show::
+
+   Flow rule #[...] updated with new actions
+
+Or if ``user_id`` flag is provided::
+
+   Flow rule #[...] updated with new actions, user-id [...]
+
+If a flow rule can not be found::
+
+   Failed to find flow [...]
+
+Otherwise it will show the usual error message of the form::
+
+   Caught error type [...] ([...]): [...]
+
+Optional ``user_id`` is a flag that signifies the rule ID is the one provided
+by the user at creation.
+
+Action list is identical to the one described for the ``flow create``.
+
+Creating, updating and destroying a flow rule::
+
+   testpmd> flow create 0 group 1 pattern eth / end actions drop / end
+   Flow rule #0 created
+   testpmd> flow update 0 0 actions queue index 1 / end
+   Flow rule #0 updated with new actions
+   testpmd> flow destroy 0 rule 0
+   Flow rule #0 destroyed
+
 Enqueueing destruction of flow rules
 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
 
-- 
2.43.0


^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2024-02-23  4:19 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-01-26 13:21 [PATCH] app/testpmd: support updating flow rule actions Oleksandr Kolomeiets
2024-02-01  9:59 ` [PATCH v2] " Oleksandr Kolomeiets
2024-02-02  9:55   ` Thomas Monjalon
2024-02-05 19:18     ` Dariusz Sosnowski
2024-02-12  8:37       ` Oleksandr Kolomeiets
2024-02-21 10:32         ` Thomas Monjalon
2024-02-22  9:04   ` [PATCH v3] " Oleksandr Kolomeiets
2024-02-23  4:19     ` [PATCH v4] " Oleksandr Kolomeiets

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).