[2/2] app/testpmd: user assigned flow ID to flows

Message ID 20230222141139.3233715-2-elibr@nvidia.com (mailing list archive)
State Changes Requested, archived
Delegated to: Ferruh Yigit
Headers
Series [1/2] app/testpmd: change rule type |

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/loongarch-compilation success Compilation OK
ci/loongarch-unit-testing success Unit Testing PASS
ci/Intel-compilation fail Compilation issues
ci/iol-intel-Functional success Functional Testing PASS
ci/iol-broadcom-Functional success Functional Testing PASS
ci/iol-mellanox-Performance success Performance Testing PASS
ci/intel-Testing success Testing PASS
ci/iol-broadcom-Performance success Performance Testing PASS
ci/iol-intel-Performance success Performance Testing PASS
ci/github-robot: build fail github build: failed
ci/iol-x86_64-unit-testing success Testing PASS
ci/iol-x86_64-compile-testing success Testing PASS
ci/iol-testing warning Testing issues
ci/iol-abi-testing success Testing PASS

Commit Message

Eli Britstein Feb. 22, 2023, 2:11 p.m. UTC
  Currently, testpmd assigns its own IDs, as indices, to created flows.
Later, the flow index is used as the ID for flow operations (query,
destroy, dump).

Allow the user to assign a user-id, to be later used as an alternative
to the flow index testpmd assigns.

Example:

testpmd> flow create 0 ingress user_id 0x1234 pattern eth / end actions
count / drop / end
Flow rule #0 created, user-id 0x1234

testpmd> flow query 0 0x1234 count user_id

testpmd> flow dump 0 user_id rule 0x1234

testpmd> flow destroy 0 rule 0x1234 user_id
Flow rule user_id 0x1234 destroyed

testpmd> flow destroy 0 rule 0x1234 user_id
Flow rule #0 destroyed, user-id 0x1234

Signed-off-by: Eli Britstein <elibr@nvidia.com>
---
 app/test-pmd/cmdline_flow.c                 | 72 +++++++++++++++++++--
 app/test-pmd/config.c                       | 34 +++++++---
 app/test-pmd/testpmd.h                      | 12 ++--
 doc/guides/testpmd_app_ug/testpmd_funcs.rst | 14 ++--
 4 files changed, 108 insertions(+), 24 deletions(-)
  

Comments

Thomas Monjalon Feb. 22, 2023, 4:50 p.m. UTC | #1
22/02/2023 15:11, Eli Britstein:
> Currently, testpmd assigns its own IDs, as indices, to created flows.
> Later, the flow index is used as the ID for flow operations (query,
> destroy, dump).
> 
> Allow the user to assign a user-id, to be later used as an alternative
> to the flow index testpmd assigns.
> 
> Example:
> 
> testpmd> flow create 0 ingress user_id 0x1234 pattern eth / end actions
> count / drop / end
> Flow rule #0 created, user-id 0x1234
> 
> testpmd> flow query 0 0x1234 count user_id
> 
> testpmd> flow dump 0 user_id rule 0x1234
> 
> testpmd> flow destroy 0 rule 0x1234 user_id
> Flow rule user_id 0x1234 destroyed
> 
> testpmd> flow destroy 0 rule 0x1234 user_id
> Flow rule #0 destroyed, user-id 0x1234
> 
> Signed-off-by: Eli Britstein <elibr@nvidia.com>

Please Eli, we need explain why adding this feature.
Could you add the justification done in the RFC?
Thank you.
  

Patch

diff --git a/app/test-pmd/cmdline_flow.c b/app/test-pmd/cmdline_flow.c
index a2709e8aa9..758ca03966 100644
--- a/app/test-pmd/cmdline_flow.c
+++ b/app/test-pmd/cmdline_flow.c
@@ -206,9 +206,11 @@  enum index {
 
 	/* Destroy arguments. */
 	DESTROY_RULE,
+	DESTROY_IS_USER_ID,
 
 	/* Query arguments. */
 	QUERY_ACTION,
+	QUERY_IS_USER_ID,
 
 	/* List arguments. */
 	LIST_GROUP,
@@ -224,10 +226,12 @@  enum index {
 	VC_TRANSFER,
 	VC_TUNNEL_SET,
 	VC_TUNNEL_MATCH,
+	VC_USER_ID,
 
 	/* Dump arguments */
 	DUMP_ALL,
 	DUMP_ONE,
+	DUMP_IS_USER_ID,
 
 	/* Configure arguments */
 	CONFIG_QUEUES_NUMBER,
@@ -1077,6 +1081,7 @@  struct buffer {
 			uint32_t act_templ_id;
 			struct rte_flow_attr attr;
 			struct tunnel_ops tunnel_ops;
+			uintptr_t user_id;
 			struct rte_flow_item *pattern;
 			struct rte_flow_action *actions;
 			struct rte_flow_action *masks;
@@ -1087,15 +1092,18 @@  struct buffer {
 		struct {
 			uintptr_t *rule;
 			uintptr_t rule_n;
+			bool is_user_id;
 		} destroy; /**< Destroy arguments. */
 		struct {
 			char file[128];
 			bool mode;
 			uintptr_t rule;
+			bool is_user_id;
 		} dump; /**< Dump arguments. */
 		struct {
 			uintptr_t rule;
 			struct rte_flow_action action;
+			bool is_user_id;
 		} query; /**< Query arguments. */
 		struct {
 			uint32_t *group;
@@ -1319,6 +1327,7 @@  static const enum index next_ia_qu_attr[] = {
 static const enum index next_dump_subcmd[] = {
 	DUMP_ALL,
 	DUMP_ONE,
+	DUMP_IS_USER_ID,
 	ZERO,
 };
 
@@ -1339,12 +1348,14 @@  static const enum index next_vc_attr[] = {
 	VC_TRANSFER,
 	VC_TUNNEL_SET,
 	VC_TUNNEL_MATCH,
+	VC_USER_ID,
 	ITEM_PATTERN,
 	ZERO,
 };
 
 static const enum index next_destroy_attr[] = {
 	DESTROY_RULE,
+	DESTROY_IS_USER_ID,
 	END,
 	ZERO,
 };
@@ -1355,6 +1366,12 @@  static const enum index next_dump_attr[] = {
 	ZERO,
 };
 
+static const enum index next_query_attr[] = {
+	QUERY_IS_USER_ID,
+	END,
+	ZERO,
+};
+
 static const enum index next_list_attr[] = {
 	LIST_GROUP,
 	END,
@@ -3533,7 +3550,7 @@  static const struct token token_list[] = {
 	[DESTROY] = {
 		.name = "destroy",
 		.help = "destroy specific flow rules",
-		.next = NEXT(NEXT_ENTRY(DESTROY_RULE),
+		.next = NEXT(next_destroy_attr,
 			     NEXT_ENTRY(COMMON_PORT_ID)),
 		.args = ARGS(ARGS_ENTRY(struct buffer, port)),
 		.call = parse_destroy,
@@ -3555,7 +3572,7 @@  static const struct token token_list[] = {
 	[QUERY] = {
 		.name = "query",
 		.help = "query an existing flow rule",
-		.next = NEXT(NEXT_ENTRY(QUERY_ACTION),
+		.next = NEXT(next_query_attr, NEXT_ENTRY(QUERY_ACTION),
 			     NEXT_ENTRY(COMMON_RULE_ID),
 			     NEXT_ENTRY(COMMON_PORT_ID)),
 		.args = ARGS(ARGS_ENTRY(struct buffer, args.query.action.type),
@@ -3674,6 +3691,12 @@  static const struct token token_list[] = {
 		.args = ARGS(ARGS_ENTRY_PTR(struct buffer, args.destroy.rule)),
 		.call = parse_destroy,
 	},
+	[DESTROY_IS_USER_ID] = {
+		.name = "user_id",
+		.help = "rule identifier is user-id",
+		.next = NEXT(next_destroy_attr),
+		.call = parse_destroy,
+	},
 	/* Dump arguments. */
 	[DUMP_ALL] = {
 		.name = "all",
@@ -3690,6 +3713,12 @@  static const struct token token_list[] = {
 				ARGS_ENTRY(struct buffer, args.dump.rule)),
 		.call = parse_dump,
 	},
+	[DUMP_IS_USER_ID] = {
+		.name = "user_id",
+		.help = "rule identifier is user-id",
+		.next = NEXT(next_dump_subcmd),
+		.call = parse_dump,
+	},
 	/* Query arguments. */
 	[QUERY_ACTION] = {
 		.name = "{action}",
@@ -3698,6 +3727,12 @@  static const struct token token_list[] = {
 		.call = parse_action,
 		.comp = comp_action,
 	},
+	[QUERY_IS_USER_ID] = {
+		.name = "user_id",
+		.help = "rule identifier is user-id",
+		.next = NEXT(next_query_attr),
+		.call = parse_query,
+	},
 	/* List arguments. */
 	[LIST_GROUP] = {
 		.name = "group",
@@ -3759,6 +3794,13 @@  static const struct token token_list[] = {
 		.args = ARGS(ARGS_ENTRY(struct tunnel_ops, id)),
 		.call = parse_vc,
 	},
+	[VC_USER_ID] = {
+		.name = "user_id",
+		.help = "specify a user id to create",
+		.next = NEXT(next_vc_attr, NEXT_ENTRY(COMMON_UNSIGNED)),
+		.args = ARGS(ARGS_ENTRY(struct buffer, args.vc.user_id)),
+		.call = parse_vc,
+	},
 	/* Validate/create pattern. */
 	[ITEM_PATTERN] = {
 		.name = "pattern",
@@ -7415,11 +7457,15 @@  parse_vc(struct context *ctx, const struct token *token,
 	case VC_TUNNEL_MATCH:
 		ctx->object = &out->args.vc.tunnel_ops;
 		break;
+	case VC_USER_ID:
+		ctx->object = out;
+		break;
 	}
 	ctx->objmask = NULL;
 	switch (ctx->curr) {
 	case VC_GROUP:
 	case VC_PRIORITY:
+	case VC_USER_ID:
 		return len;
 	case VC_TUNNEL_SET:
 		out->args.vc.tunnel_ops.enabled = 1;
@@ -9109,6 +9155,10 @@  parse_destroy(struct context *ctx, const struct token *token,
 					       sizeof(double));
 		return len;
 	}
+	if (ctx->curr == DESTROY_IS_USER_ID) {
+		out->args.destroy.is_user_id = true;
+		return len;
+	}
 	if (((uint8_t *)(out->args.destroy.rule + out->args.destroy.rule_n) +
 	     sizeof(*out->args.destroy.rule)) > (uint8_t *)out + size)
 		return -1;
@@ -9179,6 +9229,9 @@  parse_dump(struct context *ctx, const struct token *token,
 		ctx->object = out;
 		ctx->objmask = NULL;
 		return len;
+	case DUMP_IS_USER_ID:
+		out->args.dump.is_user_id = true;
+		return len;
 	default:
 		return -1;
 	}
@@ -9208,6 +9261,10 @@  parse_query(struct context *ctx, const struct token *token,
 		ctx->object = out;
 		ctx->objmask = NULL;
 	}
+	if (ctx->curr == QUERY_IS_USER_ID) {
+		out->args.query.is_user_id = true;
+		return len;
+	}
 	return len;
 }
 
@@ -11602,11 +11659,12 @@  cmd_flow_parsed(const struct buffer *in)
 	case CREATE:
 		port_flow_create(in->port, &in->args.vc.attr,
 				 in->args.vc.pattern, in->args.vc.actions,
-				 &in->args.vc.tunnel_ops);
+				 &in->args.vc.tunnel_ops, in->args.vc.user_id);
 		break;
 	case DESTROY:
 		port_flow_destroy(in->port, in->args.destroy.rule_n,
-				  in->args.destroy.rule);
+				  in->args.destroy.rule,
+				  in->args.destroy.is_user_id);
 		break;
 	case FLUSH:
 		port_flow_flush(in->port);
@@ -11614,11 +11672,13 @@  cmd_flow_parsed(const struct buffer *in)
 	case DUMP_ONE:
 	case DUMP_ALL:
 		port_flow_dump(in->port, in->args.dump.mode,
-				in->args.dump.rule, in->args.dump.file);
+				in->args.dump.rule, in->args.dump.file,
+				in->args.dump.is_user_id);
 		break;
 	case QUERY:
 		port_flow_query(in->port, in->args.query.rule,
-				&in->args.query.action);
+				&in->args.query.action,
+				in->args.query.is_user_id);
 		break;
 	case LIST:
 		port_flow_list(in->port, in->args.list.group_n,
diff --git a/app/test-pmd/config.c b/app/test-pmd/config.c
index 167cb246c5..51cc480762 100644
--- a/app/test-pmd/config.c
+++ b/app/test-pmd/config.c
@@ -3303,7 +3303,8 @@  port_flow_create(portid_t port_id,
 		 const struct rte_flow_attr *attr,
 		 const struct rte_flow_item *pattern,
 		 const struct rte_flow_action *actions,
-		 const struct tunnel_ops *tunnel_ops)
+		 const struct tunnel_ops *tunnel_ops,
+		 uintptr_t user_id)
 {
 	struct rte_flow *flow;
 	struct rte_port *port;
@@ -3351,17 +3352,23 @@  port_flow_create(portid_t port_id,
 	}
 	pf->next = port->flow_list;
 	pf->id = id;
+	pf->user_id = user_id;
 	pf->flow = flow;
 	port->flow_list = pf;
 	if (tunnel_ops->enabled)
 		port_flow_tunnel_offload_cmd_release(port_id, tunnel_ops, pft);
-	printf("Flow rule #%"PRIu64" created\n", pf->id);
+	if (user_id)
+		printf("Flow rule #%"PRIu64" created, user-id 0x%"PRIx64"\n",
+		       pf->id, pf->user_id);
+	else
+		printf("Flow rule #%"PRIu64" created\n", pf->id);
 	return 0;
 }
 
 /** Destroy a number of flow rules. */
 int
-port_flow_destroy(portid_t port_id, uint32_t n, const uintptr_t *rule)
+port_flow_destroy(portid_t port_id, uint32_t n, const uintptr_t *rule,
+		  bool is_user_id)
 {
 	struct rte_port *port;
 	struct port_flow **tmp;
@@ -3379,7 +3386,7 @@  port_flow_destroy(portid_t port_id, uint32_t n, const uintptr_t *rule)
 			struct rte_flow_error error;
 			struct port_flow *pf = *tmp;
 
-			if (rule[i] != pf->id)
+			if (rule[i] != (is_user_id ? pf->user_id : pf->id))
 				continue;
 			/*
 			 * Poisoning to make sure PMDs update it in case
@@ -3390,7 +3397,13 @@  port_flow_destroy(portid_t port_id, uint32_t n, const uintptr_t *rule)
 				ret = port_flow_complain(&error);
 				continue;
 			}
-			printf("Flow rule #%"PRIu64" destroyed\n", pf->id);
+			if (is_user_id)
+				printf("Flow rule #%"PRIu64" destroyed, "
+				       "user-id 0x%"PRIx64"\n",
+				       pf->id, pf->user_id);
+			else
+				printf("Flow rule #%"PRIu64" destroyed\n",
+				       pf->id);
 			*tmp = pf->next;
 			free(pf);
 			break;
@@ -3436,7 +3449,7 @@  port_flow_flush(portid_t port_id)
 /** Dump flow rules. */
 int
 port_flow_dump(portid_t port_id, bool dump_all, uintptr_t rule_id,
-		const char *file_name)
+		const char *file_name, bool is_user_id)
 {
 	int ret = 0;
 	FILE *file = stdout;
@@ -3454,7 +3467,8 @@  port_flow_dump(portid_t port_id, bool dump_all, uintptr_t rule_id,
 		port = &ports[port_id];
 		pflow = port->flow_list;
 		while (pflow) {
-			if (rule_id != pflow->id) {
+			if (rule_id !=
+			    (is_user_id ? pflow->user_id : pflow->id)) {
 				pflow = pflow->next;
 			} else {
 				tmpFlow = pflow->flow;
@@ -3496,7 +3510,7 @@  port_flow_dump(portid_t port_id, bool dump_all, uintptr_t rule_id,
 /** Query a flow rule. */
 int
 port_flow_query(portid_t port_id, uintptr_t rule,
-		const struct rte_flow_action *action)
+		const struct rte_flow_action *action, bool is_user_id)
 {
 	struct rte_flow_error error;
 	struct rte_port *port;
@@ -3514,7 +3528,7 @@  port_flow_query(portid_t port_id, uintptr_t rule,
 		return -EINVAL;
 	port = &ports[port_id];
 	for (pf = port->flow_list; pf; pf = pf->next)
-		if (pf->id == rule)
+		if ((is_user_id ? pf->user_id : pf->id) == rule)
 			break;
 	if (!pf) {
 		fprintf(stderr, "Flow rule #%"PRIu64" not found\n", rule);
@@ -3634,7 +3648,7 @@  port_flow_aged(portid_t port_id, uint8_t destroy)
 			       ctx.pf->rule.attr->egress ? 'e' : '-',
 			       ctx.pf->rule.attr->transfer ? 't' : '-');
 			if (destroy && !port_flow_destroy(port_id, 1,
-							  &ctx.pf->id))
+							  &ctx.pf->id, false))
 				total++;
 			break;
 		case ACTION_AGE_CONTEXT_TYPE_INDIRECT_ACTION:
diff --git a/app/test-pmd/testpmd.h b/app/test-pmd/testpmd.h
index ba29d97293..b18ebeaf83 100644
--- a/app/test-pmd/testpmd.h
+++ b/app/test-pmd/testpmd.h
@@ -216,6 +216,7 @@  struct port_flow {
 	struct port_flow *next; /**< Next flow in list. */
 	struct port_flow *tmp; /**< Temporary linking. */
 	uintptr_t id; /**< Flow rule ID. */
+	uintptr_t user_id; /**< User rule ID. */
 	struct rte_flow *flow; /**< Opaque flow object returned by PMD. */
 	struct rte_flow_conv_rule rule; /**< Saved flow rule description. */
 	enum age_action_context_type age_type; /**< Age action context type. */
@@ -979,17 +980,20 @@  int port_flow_create(portid_t port_id,
 		     const struct rte_flow_attr *attr,
 		     const struct rte_flow_item *pattern,
 		     const struct rte_flow_action *actions,
-		     const struct tunnel_ops *tunnel_ops);
+		     const struct tunnel_ops *tunnel_ops,
+		     uintptr_t user_id);
 int port_action_handle_query(portid_t port_id, uint32_t id);
 void update_age_action_context(const struct rte_flow_action *actions,
 		     struct port_flow *pf);
 int mcast_addr_pool_destroy(portid_t port_id);
-int port_flow_destroy(portid_t port_id, uint32_t n, const uintptr_t *rule);
+int port_flow_destroy(portid_t port_id, uint32_t n, const uintptr_t *rule,
+		      bool is_user_id);
 int port_flow_flush(portid_t port_id);
 int port_flow_dump(portid_t port_id, bool dump_all,
-			uintptr_t rule, const char *file_name);
+			uintptr_t rule, const char *file_name,
+			bool is_user_id);
 int port_flow_query(portid_t port_id, uintptr_t rule,
-		    const struct rte_flow_action *action);
+		    const struct rte_flow_action *action, bool is_user_id);
 void port_flow_list(portid_t port_id, uint32_t n, const uint32_t *group);
 void port_flow_aged(portid_t port_id, uint8_t destroy);
 const char *port_flow_tunnel_type(struct rte_flow_tunnel *tunnel);
diff --git a/doc/guides/testpmd_app_ug/testpmd_funcs.rst b/doc/guides/testpmd_app_ug/testpmd_funcs.rst
index 5a88933635..92cf7d5adf 100644
--- a/doc/guides/testpmd_app_ug/testpmd_funcs.rst
+++ b/doc/guides/testpmd_app_ug/testpmd_funcs.rst
@@ -3010,12 +3010,14 @@  following sections.
 
    flow create {port_id}
        [group {group_id}] [priority {level}] [ingress] [egress] [transfer]
-       pattern {item} [/ {item} [...]] / end
+       [user_id {user_id}] pattern {item} [/ {item} [...]] / end
        actions {action} [/ {action} [...]] / end
 
 - Destroy specific flow rules::
 
-   flow destroy {port_id} rule {rule_id} [...]
+   flow destroy {port_id} rule {rule_id} [...] [user_id]
+   [user_id] is used as an optional flag to indicate the rule_id is the
+   user_id assigned in "flow create".
 
 - Destroy all flow rules::
 
@@ -3023,7 +3025,9 @@  following sections.
 
 - Query an existing flow rule::
 
-   flow query {port_id} {rule_id} {action}
+   flow query {port_id} {rule_id} {action} [user_id]
+   [user_id] is used as an optional flag to indicate the rule_id is the
+   user_id assigned in "flow create".
 
 - List existing flow rules sorted by priority, filtered by group
   identifiers::
@@ -3040,7 +3044,9 @@  following sections.
 
   for one flow::
 
-   flow dump {port_id} rule {rule_id} {output_file}
+   flow dump {port_id} rule {rule_id} {output_file} [user_id]
+   [user_id] is used as an optional flag to indicate the rule_id is the
+   user_id assigned in "flow create".
 
 - List and destroy aged flow rules::