DPDK patches and discussions
 help / color / mirror / Atom feed
* [PATCH 1/2] app/testpmd: change rule type
@ 2023-02-22 14:11 Eli Britstein
  2023-02-22 14:11 ` [PATCH 2/2] app/testpmd: user assigned flow ID to flows Eli Britstein
  2023-03-01  1:00 ` [PATCH 1/2] app/testpmd: change " Ferruh Yigit
  0 siblings, 2 replies; 11+ messages in thread
From: Eli Britstein @ 2023-02-22 14:11 UTC (permalink / raw)
  To: dev
  Cc: asafp, Thomas Monjalon, Eli Britstein, Ori Kam, Aman Singh, Yuying Zhang

Change rule type to be uintptr_t (instead of currently uint32_t) to be
able to accomodate larger IDs, as a pre-step towards allowing user-id
to flows.

Signed-off-by: Eli Britstein <elibr@nvidia.com>
---
 app/test-pmd/cmdline_flow.c | 12 ++++++------
 app/test-pmd/config.c       | 34 ++++++++++++++++++----------------
 app/test-pmd/testpmd.h      | 10 +++++-----
 3 files changed, 29 insertions(+), 27 deletions(-)

diff --git a/app/test-pmd/cmdline_flow.c b/app/test-pmd/cmdline_flow.c
index 9309607f11..a2709e8aa9 100644
--- a/app/test-pmd/cmdline_flow.c
+++ b/app/test-pmd/cmdline_flow.c
@@ -1085,16 +1085,16 @@ struct buffer {
 			uint8_t *data;
 		} vc; /**< Validate/create arguments. */
 		struct {
-			uint32_t *rule;
-			uint32_t rule_n;
+			uintptr_t *rule;
+			uintptr_t rule_n;
 		} destroy; /**< Destroy arguments. */
 		struct {
 			char file[128];
 			bool mode;
-			uint32_t rule;
+			uintptr_t rule;
 		} dump; /**< Dump arguments. */
 		struct {
-			uint32_t rule;
+			uintptr_t rule;
 			struct rte_flow_action action;
 		} query; /**< Query arguments. */
 		struct {
@@ -9683,7 +9683,7 @@ parse_qo_destroy(struct context *ctx, const struct token *token,
 		 void *buf, unsigned int size)
 {
 	struct buffer *out = buf;
-	uint32_t *flow_id;
+	uintptr_t *flow_id;
 
 	/* Token name must match. */
 	if (parse_default(ctx, token, str, len, NULL, 0) < 0)
@@ -10899,7 +10899,7 @@ comp_rule_id(struct context *ctx, const struct token *token,
 	port = &ports[ctx->port];
 	for (pf = port->flow_list; pf != NULL; pf = pf->next) {
 		if (buf && i == ent)
-			return snprintf(buf, size, "%u", pf->id);
+			return snprintf(buf, size, "%"PRIu64, pf->id);
 		++i;
 	}
 	if (buf)
diff --git a/app/test-pmd/config.c b/app/test-pmd/config.c
index 4121c5c9bb..167cb246c5 100644
--- a/app/test-pmd/config.c
+++ b/app/test-pmd/config.c
@@ -2723,7 +2723,7 @@ port_queue_flow_create(portid_t port_id, queueid_t queue_id,
 		flow = rte_flow_async_create_by_index(port_id, queue_id, &op_attr, pt->table,
 			rule_idx, actions, actions_idx, job, &error);
 	if (!flow) {
-		uint32_t flow_id = pf->id;
+		uintptr_t flow_id = pf->id;
 		port_queue_flow_destroy(port_id, queue_id, true, 1, &flow_id);
 		free(job);
 		return port_flow_complain(&error);
@@ -2734,14 +2734,14 @@ port_queue_flow_create(portid_t port_id, queueid_t queue_id,
 	pf->flow = flow;
 	job->pf = pf;
 	port->flow_list = pf;
-	printf("Flow rule #%u creation enqueued\n", pf->id);
+	printf("Flow rule #%"PRIu64" creation enqueued\n", pf->id);
 	return 0;
 }
 
 /** Enqueue number of destroy flow rules operations. */
 int
 port_queue_flow_destroy(portid_t port_id, queueid_t queue_id,
-			bool postpone, uint32_t n, const uint32_t *rule)
+			bool postpone, uint32_t n, const uintptr_t *rule)
 {
 	struct rte_flow_op_attr op_attr = { .postpone = postpone };
 	struct rte_port *port;
@@ -2788,7 +2788,8 @@ port_queue_flow_destroy(portid_t port_id, queueid_t queue_id,
 				ret = port_flow_complain(&error);
 				continue;
 			}
-			printf("Flow rule #%u destruction enqueued\n", pf->id);
+			printf("Flow rule #%"PRIu64" destruction enqueued\n",
+			       pf->id);
 			*tmp = pf->next;
 			break;
 		}
@@ -3087,7 +3088,7 @@ port_queue_flow_push(portid_t port_id, queueid_t queue_id)
 /** Pull queue operation results from the queue. */
 static int
 port_queue_aged_flow_destroy(portid_t port_id, queueid_t queue_id,
-			     const uint32_t *rule, int nb_flows)
+			     const uintptr_t *rule, int nb_flows)
 {
 	struct rte_port *port = &ports[port_id];
 	struct rte_flow_op_result *res;
@@ -3150,7 +3151,7 @@ port_queue_flow_aged(portid_t port_id, uint32_t queue_id, uint8_t destroy)
 {
 	void **contexts;
 	int nb_context, total = 0, idx;
-	uint32_t *rules = NULL;
+	uintptr_t *rules = NULL;
 	struct rte_port *port;
 	struct rte_flow_error error;
 	enum age_action_context_type *type;
@@ -3206,7 +3207,7 @@ port_queue_flow_aged(portid_t port_id, uint32_t queue_id, uint8_t destroy)
 		switch (*type) {
 		case ACTION_AGE_CONTEXT_TYPE_FLOW:
 			ctx.pf = container_of(type, struct port_flow, age_type);
-			printf("%-20s\t%" PRIu32 "\t%" PRIu32 "\t%" PRIu32
+			printf("%-20s\t%" PRIuPTR "\t%" PRIu32 "\t%" PRIu32
 								 "\t%c%c%c\t\n",
 			       "Flow",
 			       ctx.pf->id,
@@ -3354,13 +3355,13 @@ port_flow_create(portid_t port_id,
 	port->flow_list = pf;
 	if (tunnel_ops->enabled)
 		port_flow_tunnel_offload_cmd_release(port_id, tunnel_ops, pft);
-	printf("Flow rule #%u created\n", pf->id);
+	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 uint32_t *rule)
+port_flow_destroy(portid_t port_id, uint32_t n, const uintptr_t *rule)
 {
 	struct rte_port *port;
 	struct port_flow **tmp;
@@ -3389,7 +3390,7 @@ port_flow_destroy(portid_t port_id, uint32_t n, const uint32_t *rule)
 				ret = port_flow_complain(&error);
 				continue;
 			}
-			printf("Flow rule #%u destroyed\n", pf->id);
+			printf("Flow rule #%"PRIu64" destroyed\n", pf->id);
 			*tmp = pf->next;
 			free(pf);
 			break;
@@ -3434,7 +3435,7 @@ port_flow_flush(portid_t port_id)
 
 /** Dump flow rules. */
 int
-port_flow_dump(portid_t port_id, bool dump_all, uint32_t rule_id,
+port_flow_dump(portid_t port_id, bool dump_all, uintptr_t rule_id,
 		const char *file_name)
 {
 	int ret = 0;
@@ -3463,7 +3464,8 @@ port_flow_dump(portid_t port_id, bool dump_all, uint32_t rule_id,
 			}
 		}
 		if (found == false) {
-			fprintf(stderr, "Failed to dump to flow %d\n", rule_id);
+			fprintf(stderr, "Failed to dump to flow %"PRIu64"\n",
+				rule_id);
 			return -EINVAL;
 		}
 	}
@@ -3493,7 +3495,7 @@ port_flow_dump(portid_t port_id, bool dump_all, uint32_t rule_id,
 
 /** Query a flow rule. */
 int
-port_flow_query(portid_t port_id, uint32_t rule,
+port_flow_query(portid_t port_id, uintptr_t rule,
 		const struct rte_flow_action *action)
 {
 	struct rte_flow_error error;
@@ -3515,7 +3517,7 @@ port_flow_query(portid_t port_id, uint32_t rule,
 		if (pf->id == rule)
 			break;
 	if (!pf) {
-		fprintf(stderr, "Flow rule #%u not found\n", rule);
+		fprintf(stderr, "Flow rule #%"PRIu64" not found\n", rule);
 		return -ENOENT;
 	}
 	ret = rte_flow_conv(RTE_FLOW_CONV_OP_ACTION_NAME_PTR,
@@ -3622,7 +3624,7 @@ port_flow_aged(portid_t port_id, uint8_t destroy)
 		switch (*type) {
 		case ACTION_AGE_CONTEXT_TYPE_FLOW:
 			ctx.pf = container_of(type, struct port_flow, age_type);
-			printf("%-20s\t%" PRIu32 "\t%" PRIu32 "\t%" PRIu32
+			printf("%-20s\t%" PRIu64 "\t%" PRIu32 "\t%" PRIu32
 								 "\t%c%c%c\t\n",
 			       "Flow",
 			       ctx.pf->id,
@@ -3700,7 +3702,7 @@ port_flow_list(portid_t port_id, uint32_t n, const uint32_t *group)
 		const struct rte_flow_action *action = pf->rule.actions;
 		const char *name;
 
-		printf("%" PRIu32 "\t%" PRIu32 "\t%" PRIu32 "\t%c%c%c\t",
+		printf("%" PRIu64 "\t%" PRIu32 "\t%" PRIu32 "\t%c%c%c\t",
 		       pf->id,
 		       pf->rule.attr->group,
 		       pf->rule.attr->priority,
diff --git a/app/test-pmd/testpmd.h b/app/test-pmd/testpmd.h
index 329a6378a1..ba29d97293 100644
--- a/app/test-pmd/testpmd.h
+++ b/app/test-pmd/testpmd.h
@@ -215,7 +215,7 @@ struct port_table {
 struct port_flow {
 	struct port_flow *next; /**< Next flow in list. */
 	struct port_flow *tmp; /**< Temporary linking. */
-	uint32_t id; /**< Flow rule ID. */
+	uintptr_t id; /**< Flow 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. */
@@ -948,7 +948,7 @@ int port_queue_flow_create(portid_t port_id, queueid_t queue_id,
 			   const struct rte_flow_item *pattern,
 			   const struct rte_flow_action *actions);
 int port_queue_flow_destroy(portid_t port_id, queueid_t queue_id,
-			    bool postpone, uint32_t n, const uint32_t *rule);
+			    bool postpone, uint32_t n, const uintptr_t *rule);
 int port_queue_action_handle_create(portid_t port_id, uint32_t queue_id,
 			bool postpone, uint32_t id,
 			const struct rte_flow_indir_action_conf *conf,
@@ -984,11 +984,11 @@ 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 uint32_t *rule);
+int port_flow_destroy(portid_t port_id, uint32_t n, const uintptr_t *rule);
 int port_flow_flush(portid_t port_id);
 int port_flow_dump(portid_t port_id, bool dump_all,
-			uint32_t rule, const char *file_name);
-int port_flow_query(portid_t port_id, uint32_t rule,
+			uintptr_t rule, const char *file_name);
+int port_flow_query(portid_t port_id, uintptr_t rule,
 		    const struct rte_flow_action *action);
 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);
-- 
2.25.1


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

* [PATCH 2/2] app/testpmd: user assigned flow ID to flows
  2023-02-22 14:11 [PATCH 1/2] app/testpmd: change rule type Eli Britstein
@ 2023-02-22 14:11 ` Eli Britstein
  2023-02-22 16:50   ` Thomas Monjalon
  2023-03-16 14:19   ` [PATCH V2 1/2] app/testpmd: change flow rule type Gregory Etelson
  2023-03-01  1:00 ` [PATCH 1/2] app/testpmd: change " Ferruh Yigit
  1 sibling, 2 replies; 11+ messages in thread
From: Eli Britstein @ 2023-02-22 14:11 UTC (permalink / raw)
  To: dev
  Cc: asafp, Thomas Monjalon, Eli Britstein, Ori Kam, Aman Singh, Yuying Zhang

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(-)

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::
 
-- 
2.25.1


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

* Re: [PATCH 2/2] app/testpmd: user assigned flow ID to flows
  2023-02-22 14:11 ` [PATCH 2/2] app/testpmd: user assigned flow ID to flows Eli Britstein
@ 2023-02-22 16:50   ` Thomas Monjalon
  2023-03-16 14:19   ` [PATCH V2 1/2] app/testpmd: change flow rule type Gregory Etelson
  1 sibling, 0 replies; 11+ messages in thread
From: Thomas Monjalon @ 2023-02-22 16:50 UTC (permalink / raw)
  To: Eli Britstein; +Cc: dev, asafp, Ori Kam, Aman Singh, Yuying Zhang

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.



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

* Re: [PATCH 1/2] app/testpmd: change rule type
  2023-02-22 14:11 [PATCH 1/2] app/testpmd: change rule type Eli Britstein
  2023-02-22 14:11 ` [PATCH 2/2] app/testpmd: user assigned flow ID to flows Eli Britstein
@ 2023-03-01  1:00 ` Ferruh Yigit
  1 sibling, 0 replies; 11+ messages in thread
From: Ferruh Yigit @ 2023-03-01  1:00 UTC (permalink / raw)
  To: Eli Britstein, dev
  Cc: asafp, Thomas Monjalon, Ori Kam, Aman Singh, Yuying Zhang

On 2/22/2023 2:11 PM, Eli Britstein wrote:
> Change rule type to be uintptr_t (instead of currently uint32_t) to be
> able to accomodate larger IDs, as a pre-step towards allowing user-id
> to flows.
> 

No objection to extend id storage type, but I am not clear why allowing
user-id justifies this, will user insert id more than uint32_max, or is
there some intention to partition variable for user provided ids etc?
Can you please elaborate?

> Signed-off-by: Eli Britstein <elibr@nvidia.com>
> ---
>  app/test-pmd/cmdline_flow.c | 12 ++++++------
>  app/test-pmd/config.c       | 34 ++++++++++++++++++----------------
>  app/test-pmd/testpmd.h      | 10 +++++-----
>  3 files changed, 29 insertions(+), 27 deletions(-)
> 
> diff --git a/app/test-pmd/cmdline_flow.c b/app/test-pmd/cmdline_flow.c
> index 9309607f11..a2709e8aa9 100644
> --- a/app/test-pmd/cmdline_flow.c
> +++ b/app/test-pmd/cmdline_flow.c
> @@ -1085,16 +1085,16 @@ struct buffer {
>  			uint8_t *data;
>  		} vc; /**< Validate/create arguments. */
>  		struct {
> -			uint32_t *rule;
> -			uint32_t rule_n;
> +			uintptr_t *rule;

Why not using 'uint64_t'?

As far as I know 'uintptr_t' is logically to hold pointer values, it is
good for usage like:
``
void *val;
uintptr_t ptr = (uintptr_t)val;
``

Having 'uintptr_t' as type for pointer looks confusing to me.

> +			uintptr_t rule_n;

Similarly, why not 'uint64_t' or 'size_t'?

Variable name suggest it is storing a count, if so 'size_t' is more
suitable logically (functionally I guess both are same).

Same comments apply for all below type changes.


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

* [PATCH V2 1/2] app/testpmd: change flow rule type
  2023-02-22 14:11 ` [PATCH 2/2] app/testpmd: user assigned flow ID to flows Eli Britstein
  2023-02-22 16:50   ` Thomas Monjalon
@ 2023-03-16 14:19   ` Gregory Etelson
  2023-03-16 14:19     ` [PATCH V2 2/2] app/testpmd: assign custom ID to flow rules Gregory Etelson
  2023-06-02 20:19     ` [PATCH V2 1/2] app/testpmd: change flow rule type Ferruh Yigit
  1 sibling, 2 replies; 11+ messages in thread
From: Gregory Etelson @ 2023-03-16 14:19 UTC (permalink / raw)
  To: dev; +Cc: elibr, asafp, tmonjalon, Ori Kam, Aman Singh, Yuying Zhang

From: Eli Britstein <elibr@nvidia.com>

Change flow rule type to be uint64_t (instead of currently uint32_t) to
be able to accommodate larger IDs, as a pre-step towards allowing user-id
to flows.

Signed-off-by: Eli Britstein <elibr@nvidia.com>
---
 app/test-pmd/cmdline_flow.c | 12 ++++++------
 app/test-pmd/config.c       | 34 ++++++++++++++++++----------------
 app/test-pmd/testpmd.h      | 10 +++++-----
 3 files changed, 29 insertions(+), 27 deletions(-)

diff --git a/app/test-pmd/cmdline_flow.c b/app/test-pmd/cmdline_flow.c
index 9309607f11..aab0df91c2 100644
--- a/app/test-pmd/cmdline_flow.c
+++ b/app/test-pmd/cmdline_flow.c
@@ -1085,16 +1085,16 @@ struct buffer {
 			uint8_t *data;
 		} vc; /**< Validate/create arguments. */
 		struct {
-			uint32_t *rule;
-			uint32_t rule_n;
+			uint64_t *rule;
+			uint64_t rule_n;
 		} destroy; /**< Destroy arguments. */
 		struct {
 			char file[128];
 			bool mode;
-			uint32_t rule;
+			uint64_t rule;
 		} dump; /**< Dump arguments. */
 		struct {
-			uint32_t rule;
+			uint64_t rule;
 			struct rte_flow_action action;
 		} query; /**< Query arguments. */
 		struct {
@@ -9683,7 +9683,7 @@ parse_qo_destroy(struct context *ctx, const struct token *token,
 		 void *buf, unsigned int size)
 {
 	struct buffer *out = buf;
-	uint32_t *flow_id;
+	uint64_t *flow_id;
 
 	/* Token name must match. */
 	if (parse_default(ctx, token, str, len, NULL, 0) < 0)
@@ -10899,7 +10899,7 @@ comp_rule_id(struct context *ctx, const struct token *token,
 	port = &ports[ctx->port];
 	for (pf = port->flow_list; pf != NULL; pf = pf->next) {
 		if (buf && i == ent)
-			return snprintf(buf, size, "%u", pf->id);
+			return snprintf(buf, size, "%"PRIu64, pf->id);
 		++i;
 	}
 	if (buf)
diff --git a/app/test-pmd/config.c b/app/test-pmd/config.c
index 018536f177..804af98c0e 100644
--- a/app/test-pmd/config.c
+++ b/app/test-pmd/config.c
@@ -2725,7 +2725,7 @@ port_queue_flow_create(portid_t port_id, queueid_t queue_id,
 		flow = rte_flow_async_create_by_index(port_id, queue_id, &op_attr, pt->table,
 			rule_idx, actions, actions_idx, job, &error);
 	if (!flow) {
-		uint32_t flow_id = pf->id;
+		uint64_t flow_id = pf->id;
 		port_queue_flow_destroy(port_id, queue_id, true, 1, &flow_id);
 		free(job);
 		return port_flow_complain(&error);
@@ -2736,14 +2736,14 @@ port_queue_flow_create(portid_t port_id, queueid_t queue_id,
 	pf->flow = flow;
 	job->pf = pf;
 	port->flow_list = pf;
-	printf("Flow rule #%u creation enqueued\n", pf->id);
+	printf("Flow rule #%"PRIu64" creation enqueued\n", pf->id);
 	return 0;
 }
 
 /** Enqueue number of destroy flow rules operations. */
 int
 port_queue_flow_destroy(portid_t port_id, queueid_t queue_id,
-			bool postpone, uint32_t n, const uint32_t *rule)
+			bool postpone, uint32_t n, const uint64_t *rule)
 {
 	struct rte_flow_op_attr op_attr = { .postpone = postpone };
 	struct rte_port *port;
@@ -2790,7 +2790,8 @@ port_queue_flow_destroy(portid_t port_id, queueid_t queue_id,
 				ret = port_flow_complain(&error);
 				continue;
 			}
-			printf("Flow rule #%u destruction enqueued\n", pf->id);
+			printf("Flow rule #%"PRIu64" destruction enqueued\n",
+			       pf->id);
 			*tmp = pf->next;
 			break;
 		}
@@ -3089,7 +3090,7 @@ port_queue_flow_push(portid_t port_id, queueid_t queue_id)
 /** Pull queue operation results from the queue. */
 static int
 port_queue_aged_flow_destroy(portid_t port_id, queueid_t queue_id,
-			     const uint32_t *rule, int nb_flows)
+			     const uint64_t *rule, int nb_flows)
 {
 	struct rte_port *port = &ports[port_id];
 	struct rte_flow_op_result *res;
@@ -3152,7 +3153,7 @@ port_queue_flow_aged(portid_t port_id, uint32_t queue_id, uint8_t destroy)
 {
 	void **contexts;
 	int nb_context, total = 0, idx;
-	uint32_t *rules = NULL;
+	uint64_t *rules = NULL;
 	struct rte_port *port;
 	struct rte_flow_error error;
 	enum age_action_context_type *type;
@@ -3208,7 +3209,7 @@ port_queue_flow_aged(portid_t port_id, uint32_t queue_id, uint8_t destroy)
 		switch (*type) {
 		case ACTION_AGE_CONTEXT_TYPE_FLOW:
 			ctx.pf = container_of(type, struct port_flow, age_type);
-			printf("%-20s\t%" PRIu32 "\t%" PRIu32 "\t%" PRIu32
+			printf("%-20s\t%" PRIu64 "\t%" PRIu32 "\t%" PRIu32
 								 "\t%c%c%c\t\n",
 			       "Flow",
 			       ctx.pf->id,
@@ -3356,13 +3357,13 @@ port_flow_create(portid_t port_id,
 	port->flow_list = pf;
 	if (tunnel_ops->enabled)
 		port_flow_tunnel_offload_cmd_release(port_id, tunnel_ops, pft);
-	printf("Flow rule #%u created\n", pf->id);
+	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 uint32_t *rule)
+port_flow_destroy(portid_t port_id, uint32_t n, const uint64_t *rule)
 {
 	struct rte_port *port;
 	struct port_flow **tmp;
@@ -3391,7 +3392,7 @@ port_flow_destroy(portid_t port_id, uint32_t n, const uint32_t *rule)
 				ret = port_flow_complain(&error);
 				continue;
 			}
-			printf("Flow rule #%u destroyed\n", pf->id);
+			printf("Flow rule #%"PRIu64" destroyed\n", pf->id);
 			*tmp = pf->next;
 			free(pf);
 			break;
@@ -3436,7 +3437,7 @@ port_flow_flush(portid_t port_id)
 
 /** Dump flow rules. */
 int
-port_flow_dump(portid_t port_id, bool dump_all, uint32_t rule_id,
+port_flow_dump(portid_t port_id, bool dump_all, uint64_t rule_id,
 		const char *file_name)
 {
 	int ret = 0;
@@ -3465,7 +3466,8 @@ port_flow_dump(portid_t port_id, bool dump_all, uint32_t rule_id,
 			}
 		}
 		if (found == false) {
-			fprintf(stderr, "Failed to dump to flow %d\n", rule_id);
+			fprintf(stderr, "Failed to dump to flow %"PRIu64"\n",
+				rule_id);
 			return -EINVAL;
 		}
 	}
@@ -3495,7 +3497,7 @@ port_flow_dump(portid_t port_id, bool dump_all, uint32_t rule_id,
 
 /** Query a flow rule. */
 int
-port_flow_query(portid_t port_id, uint32_t rule,
+port_flow_query(portid_t port_id, uint64_t rule,
 		const struct rte_flow_action *action)
 {
 	struct rte_flow_error error;
@@ -3517,7 +3519,7 @@ port_flow_query(portid_t port_id, uint32_t rule,
 		if (pf->id == rule)
 			break;
 	if (!pf) {
-		fprintf(stderr, "Flow rule #%u not found\n", rule);
+		fprintf(stderr, "Flow rule #%"PRIu64" not found\n", rule);
 		return -ENOENT;
 	}
 	ret = rte_flow_conv(RTE_FLOW_CONV_OP_ACTION_NAME_PTR,
@@ -3624,7 +3626,7 @@ port_flow_aged(portid_t port_id, uint8_t destroy)
 		switch (*type) {
 		case ACTION_AGE_CONTEXT_TYPE_FLOW:
 			ctx.pf = container_of(type, struct port_flow, age_type);
-			printf("%-20s\t%" PRIu32 "\t%" PRIu32 "\t%" PRIu32
+			printf("%-20s\t%" PRIu64 "\t%" PRIu32 "\t%" PRIu32
 								 "\t%c%c%c\t\n",
 			       "Flow",
 			       ctx.pf->id,
@@ -3702,7 +3704,7 @@ port_flow_list(portid_t port_id, uint32_t n, const uint32_t *group)
 		const struct rte_flow_action *action = pf->rule.actions;
 		const char *name;
 
-		printf("%" PRIu32 "\t%" PRIu32 "\t%" PRIu32 "\t%c%c%c\t",
+		printf("%" PRIu64 "\t%" PRIu32 "\t%" PRIu32 "\t%c%c%c\t",
 		       pf->id,
 		       pf->rule.attr->group,
 		       pf->rule.attr->priority,
diff --git a/app/test-pmd/testpmd.h b/app/test-pmd/testpmd.h
index bdfbfd36d3..c2b0a0a48b 100644
--- a/app/test-pmd/testpmd.h
+++ b/app/test-pmd/testpmd.h
@@ -216,7 +216,7 @@ struct port_table {
 struct port_flow {
 	struct port_flow *next; /**< Next flow in list. */
 	struct port_flow *tmp; /**< Temporary linking. */
-	uint32_t id; /**< Flow rule ID. */
+	uint64_t id; /**< Flow 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. */
@@ -966,7 +966,7 @@ int port_queue_flow_create(portid_t port_id, queueid_t queue_id,
 			   const struct rte_flow_item *pattern,
 			   const struct rte_flow_action *actions);
 int port_queue_flow_destroy(portid_t port_id, queueid_t queue_id,
-			    bool postpone, uint32_t n, const uint32_t *rule);
+			    bool postpone, uint32_t n, const uint64_t *rule);
 int port_queue_action_handle_create(portid_t port_id, uint32_t queue_id,
 			bool postpone, uint32_t id,
 			const struct rte_flow_indir_action_conf *conf,
@@ -1002,11 +1002,11 @@ 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 uint32_t *rule);
+int port_flow_destroy(portid_t port_id, uint32_t n, const uint64_t *rule);
 int port_flow_flush(portid_t port_id);
 int port_flow_dump(portid_t port_id, bool dump_all,
-			uint32_t rule, const char *file_name);
-int port_flow_query(portid_t port_id, uint32_t rule,
+			uint64_t rule, const char *file_name);
+int port_flow_query(portid_t port_id, uint64_t rule,
 		    const struct rte_flow_action *action);
 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);
-- 
2.25.1


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

* [PATCH V2 2/2] app/testpmd: assign custom ID to flow rules
  2023-03-16 14:19   ` [PATCH V2 1/2] app/testpmd: change flow rule type Gregory Etelson
@ 2023-03-16 14:19     ` Gregory Etelson
  2023-06-02 20:19       ` Ferruh Yigit
  2023-07-04  8:25       ` Ori Kam
  2023-06-02 20:19     ` [PATCH V2 1/2] app/testpmd: change flow rule type Ferruh Yigit
  1 sibling, 2 replies; 11+ messages in thread
From: Gregory Etelson @ 2023-03-16 14:19 UTC (permalink / raw)
  To: dev; +Cc: elibr, asafp, tmonjalon, Ori Kam, Aman Singh, Yuying Zhang

From: Eli Britstein <elibr@nvidia.com>

Upon creation of a flow, testpmd assigns it a flow ID. Later, the
flow ID is used for flow operations (query, destroy, dump).

The testpmd application allows to manage flow rules with its IDs.
The flow ID is known only when the flow is created.
In order to prepare a complete sequence of testpmd commands to
copy/paste, the flow IDs must be predictable.

Allow the user to provide an assigned ID.

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 #0 destroyed, user-id 0x1234

Here, "user_id" is a flag that signifies the "rule" ID is the user-id.

The motivation is from OVS. OVS dumps its "rte_flow_create" calls to the
log in testpmd commands syntax. As the flow ID testpmd would assign is
unkwon, it cannot log valid "flow destroy" commands.

With this enhancement, valid testpmd commands can be created in a
log to copy/paste to testpmd.
The application's flows sequence can then be played back in
testpmd, to enable enhanced dpdk debug capabilities of the
applications's flows in a controlled environment of testpmd
rather than a dynamic, more difficult to debug environment of the
application.

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 | 33 +++++++---
 4 files changed, 121 insertions(+), 30 deletions(-)

diff --git a/app/test-pmd/cmdline_flow.c b/app/test-pmd/cmdline_flow.c
index aab0df91c2..669da31e5d 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 {
 			uint64_t *rule;
 			uint64_t rule_n;
+			bool is_user_id;
 		} destroy; /**< Destroy arguments. */
 		struct {
 			char file[128];
 			bool mode;
 			uint64_t rule;
+			bool is_user_id;
 		} dump; /**< Dump arguments. */
 		struct {
 			uint64_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 804af98c0e..c3aaeaed6b 100644
--- a/app/test-pmd/config.c
+++ b/app/test-pmd/config.c
@@ -3305,7 +3305,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;
@@ -3353,17 +3354,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 uint64_t *rule)
+port_flow_destroy(portid_t port_id, uint32_t n, const uint64_t *rule,
+		  bool is_user_id)
 {
 	struct rte_port *port;
 	struct port_flow **tmp;
@@ -3381,7 +3388,7 @@ port_flow_destroy(portid_t port_id, uint32_t n, const uint64_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
@@ -3392,7 +3399,13 @@ port_flow_destroy(portid_t port_id, uint32_t n, const uint64_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;
@@ -3438,7 +3451,7 @@ port_flow_flush(portid_t port_id)
 /** Dump flow rules. */
 int
 port_flow_dump(portid_t port_id, bool dump_all, uint64_t rule_id,
-		const char *file_name)
+		const char *file_name, bool is_user_id)
 {
 	int ret = 0;
 	FILE *file = stdout;
@@ -3456,7 +3469,8 @@ port_flow_dump(portid_t port_id, bool dump_all, uint64_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;
@@ -3498,7 +3512,7 @@ port_flow_dump(portid_t port_id, bool dump_all, uint64_t rule_id,
 /** Query a flow rule. */
 int
 port_flow_query(portid_t port_id, uint64_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;
@@ -3516,7 +3530,7 @@ port_flow_query(portid_t port_id, uint64_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);
@@ -3636,7 +3650,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 c2b0a0a48b..454f2b0c50 100644
--- a/app/test-pmd/testpmd.h
+++ b/app/test-pmd/testpmd.h
@@ -217,6 +217,7 @@ struct port_flow {
 	struct port_flow *next; /**< Next flow in list. */
 	struct port_flow *tmp; /**< Temporary linking. */
 	uint64_t id; /**< Flow rule ID. */
+	uint64_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. */
@@ -997,17 +998,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 uint64_t *rule);
+int port_flow_destroy(portid_t port_id, uint32_t n, const uint64_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,
-			uint64_t rule, const char *file_name);
+			uint64_t rule, const char *file_name,
+			bool is_user_id);
 int port_flow_query(portid_t port_id, uint64_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 8f23847859..1b2991d94a 100644
--- a/doc/guides/testpmd_app_ug/testpmd_funcs.rst
+++ b/doc/guides/testpmd_app_ug/testpmd_funcs.rst
@@ -3009,13 +3009,14 @@ following sections.
 - Create a flow rule::
 
    flow create {port_id}
-       [group {group_id}] [priority {level}] [ingress] [egress] [transfer]
-       pattern {item} [/ {item} [...]] / end
+       [group {group_id}] [priority {level}] [ingress] [egress]
+       [transfer] [tunnel_set {tunnel_id}] [tunnel_match {tunnel_id}]
+       [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]
 
 - Destroy all flow rules::
 
@@ -3023,7 +3024,7 @@ following sections.
 
 - Query an existing flow rule::
 
-   flow query {port_id} {rule_id} {action}
+   flow query {port_id} {rule_id} {action} [user_id]
 
 - List existing flow rules sorted by priority, filtered by group
   identifiers::
@@ -3036,11 +3037,11 @@ following sections.
 
 - Dump internal representation information of all flows in hardware::
 
-   flow dump {port_id} all {output_file}
+   flow dump {port_id} all {output_file} [user_id]
 
   for one flow::
 
-   flow dump {port_id} rule {rule_id} {output_file}
+   flow dump {port_id} rule {rule_id} {output_file} [user_id]
 
 - List and destroy aged flow rules::
 
@@ -3339,12 +3340,14 @@ to ``rte_flow_create()``::
    flow create {port_id}
       [group {group_id}] [priority {level}] [ingress] [egress] [transfer]
       [tunnel_set {tunnel_id}] [tunnel_match {tunnel_id}]
-      pattern {item} [/ {item} [...]] / end
+      [user_id {user_id}] pattern {item} [/ {item} [...]] / end
       actions {action} [/ {action} [...]] / end
 
 If successful, it will return a flow rule ID usable with other commands::
 
    Flow rule #[...] created
+   Or if user_id is provided:
+   Flow rule #[...] created, user-id [...]
 
 Otherwise it will show an error message of the form::
 
@@ -3354,6 +3357,7 @@ Parameters describe in the following order:
 
 - Attributes (*group*, *priority*, *ingress*, *egress*, *transfer* tokens).
 - Tunnel offload specification (tunnel_set, tunnel_match)
+- User identifier for the flow.
 - A matching pattern, starting with the *pattern* token and terminated by an
   *end* pattern item.
 - Actions, starting with the *actions* token and terminated by an *end*
@@ -4077,12 +4081,16 @@ Destroying flow rules
 by ``flow create``), this command calls ``rte_flow_destroy()`` as many
 times as necessary::
 
-   flow destroy {port_id} rule {rule_id} [...]
+   flow destroy {port_id} rule {rule_id} [...] [user_id]
 
 If successful, it will show::
 
    Flow rule #[...] destroyed
+   Or if user_id flag is provided:
+   Flow rule #[...] destroyed, user-id [...]
 
+Optional [user_id] is a flag that signifies the "rule" ID is the one
+provided by the user at creation.
 It does not report anything for rule IDs that do not exist. The usual error
 message is shown when a rule cannot be destroyed::
 
@@ -4161,8 +4169,10 @@ Querying flow rules
 ability. Such actions collect information that can be reported using this
 command. It is bound to ``rte_flow_query()``::
 
-   flow query {port_id} {rule_id} {action}
+   flow query {port_id} {rule_id} {action} [user_id]
 
+Optional [user_id] is a flag that signifies the "rule" ID is the one
+provided by the user at creation.
 If successful, it will display either the retrieved data for known actions
 or the following message::
 
@@ -4313,7 +4323,7 @@ Dumping HW internal information
 ``flow dump`` dumps the hardware's internal representation information of
 all flows. It is bound to ``rte_flow_dev_dump()``::
 
-   flow dump {port_id} {output_file}
+   flow dump {port_id} {output_file} [user_id]
 
 If successful, it will show::
 
@@ -4323,6 +4333,9 @@ Otherwise, it will complain error occurred::
 
    Caught error type [...] ([...]): [...]
 
+Optional [user_id] is a flag that signifies the "rule" ID is the one
+provided by the user at creation.
+
 Listing and destroying aged flow rules
 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
 
-- 
2.25.1


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

* Re: [PATCH V2 1/2] app/testpmd: change flow rule type
  2023-03-16 14:19   ` [PATCH V2 1/2] app/testpmd: change flow rule type Gregory Etelson
  2023-03-16 14:19     ` [PATCH V2 2/2] app/testpmd: assign custom ID to flow rules Gregory Etelson
@ 2023-06-02 20:19     ` Ferruh Yigit
  1 sibling, 0 replies; 11+ messages in thread
From: Ferruh Yigit @ 2023-06-02 20:19 UTC (permalink / raw)
  To: Gregory Etelson, dev
  Cc: elibr, asafp, tmonjalon, Ori Kam, Aman Singh, Yuying Zhang

On 3/16/2023 2:19 PM, Gregory Etelson wrote:
> From: Eli Britstein <elibr@nvidia.com>
> 
> Change flow rule type to be uint64_t (instead of currently uint32_t) to
> be able to accommodate larger IDs, as a pre-step towards allowing user-id
> to flows.
> 
> Signed-off-by: Eli Britstein <elibr@nvidia.com>
>

Acked-by: Ferruh Yigit <ferruh.yigit@amd.com>


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

* Re: [PATCH V2 2/2] app/testpmd: assign custom ID to flow rules
  2023-03-16 14:19     ` [PATCH V2 2/2] app/testpmd: assign custom ID to flow rules Gregory Etelson
@ 2023-06-02 20:19       ` Ferruh Yigit
  2023-06-30 10:21         ` Ferruh Yigit
  2023-07-04  8:25       ` Ori Kam
  1 sibling, 1 reply; 11+ messages in thread
From: Ferruh Yigit @ 2023-06-02 20:19 UTC (permalink / raw)
  To: Gregory Etelson, dev, Ori Kam
  Cc: elibr, asafp, tmonjalon, Aman Singh, Yuying Zhang

On 3/16/2023 2:19 PM, Gregory Etelson wrote:
> From: Eli Britstein <elibr@nvidia.com>
> 
> Upon creation of a flow, testpmd assigns it a flow ID. Later, the
> flow ID is used for flow operations (query, destroy, dump).
> 
> The testpmd application allows to manage flow rules with its IDs.
> The flow ID is known only when the flow is created.
> In order to prepare a complete sequence of testpmd commands to
> copy/paste, the flow IDs must be predictable.
> 
> Allow the user to provide an assigned ID.
> 
> 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 #0 destroyed, user-id 0x1234
> 
> Here, "user_id" is a flag that signifies the "rule" ID is the user-id.
> 
> The motivation is from OVS. OVS dumps its "rte_flow_create" calls to the
> log in testpmd commands syntax. As the flow ID testpmd would assign is
> unkwon, it cannot log valid "flow destroy" commands.
> 
> With this enhancement, valid testpmd commands can be created in a
> log to copy/paste to testpmd.
> The application's flows sequence can then be played back in
> testpmd, to enable enhanced dpdk debug capabilities of the
> applications's flows in a controlled environment of testpmd
> rather than a dynamic, more difficult to debug environment of the
> application.
> 
> 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 | 33 +++++++---
>  4 files changed, 121 insertions(+), 30 deletions(-)
>

Hi Ori,

Can you please help reviewing this patch?

Thanks,
ferruh


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

* Re: [PATCH V2 2/2] app/testpmd: assign custom ID to flow rules
  2023-06-02 20:19       ` Ferruh Yigit
@ 2023-06-30 10:21         ` Ferruh Yigit
  0 siblings, 0 replies; 11+ messages in thread
From: Ferruh Yigit @ 2023-06-30 10:21 UTC (permalink / raw)
  To: Gregory Etelson, dev, Ori Kam
  Cc: elibr, asafp, tmonjalon, Aman Singh, Yuying Zhang

On 6/2/2023 9:19 PM, Ferruh Yigit wrote:
> On 3/16/2023 2:19 PM, Gregory Etelson wrote:
>> From: Eli Britstein <elibr@nvidia.com>
>>
>> Upon creation of a flow, testpmd assigns it a flow ID. Later, the
>> flow ID is used for flow operations (query, destroy, dump).
>>
>> The testpmd application allows to manage flow rules with its IDs.
>> The flow ID is known only when the flow is created.
>> In order to prepare a complete sequence of testpmd commands to
>> copy/paste, the flow IDs must be predictable.
>>
>> Allow the user to provide an assigned ID.
>>
>> 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 #0 destroyed, user-id 0x1234
>>
>> Here, "user_id" is a flag that signifies the "rule" ID is the user-id.
>>
>> The motivation is from OVS. OVS dumps its "rte_flow_create" calls to the
>> log in testpmd commands syntax. As the flow ID testpmd would assign is
>> unkwon, it cannot log valid "flow destroy" commands.
>>
>> With this enhancement, valid testpmd commands can be created in a
>> log to copy/paste to testpmd.
>> The application's flows sequence can then be played back in
>> testpmd, to enable enhanced dpdk debug capabilities of the
>> applications's flows in a controlled environment of testpmd
>> rather than a dynamic, more difficult to debug environment of the
>> application.
>>
>> 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 | 33 +++++++---
>>  4 files changed, 121 insertions(+), 30 deletions(-)
>>
> 
> Hi Ori,
> 
> Can you please help reviewing this patch?
> 

Reminder for review.


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

* RE: [PATCH V2 2/2] app/testpmd: assign custom ID to flow rules
  2023-03-16 14:19     ` [PATCH V2 2/2] app/testpmd: assign custom ID to flow rules Gregory Etelson
  2023-06-02 20:19       ` Ferruh Yigit
@ 2023-07-04  8:25       ` Ori Kam
  2023-07-04 14:40         ` Ferruh Yigit
  1 sibling, 1 reply; 11+ messages in thread
From: Ori Kam @ 2023-07-04  8:25 UTC (permalink / raw)
  To: Gregory Etelson, dev
  Cc: Eli Britstein, Asaf Penso, Thomas Monjalon, Aman Singh, Yuying Zhang



> -----Original Message-----
> From: Gregory Etelson <getelson@nvidia.com>
> Subject: [PATCH V2 2/2] app/testpmd: assign custom ID to flow rules
> 
> From: Eli Britstein <elibr@nvidia.com>
> 
> Upon creation of a flow, testpmd assigns it a flow ID. Later, the
> flow ID is used for flow operations (query, destroy, dump).
> 
> The testpmd application allows to manage flow rules with its IDs.
> The flow ID is known only when the flow is created.
> In order to prepare a complete sequence of testpmd commands to
> copy/paste, the flow IDs must be predictable.
> 
> Allow the user to provide an assigned ID.
> 
> 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 #0 destroyed, user-id 0x1234
> 
> Here, "user_id" is a flag that signifies the "rule" ID is the user-id.
> 
> The motivation is from OVS. OVS dumps its "rte_flow_create" calls to the
> log in testpmd commands syntax. As the flow ID testpmd would assign is
> unkwon, it cannot log valid "flow destroy" commands.
> 
> With this enhancement, valid testpmd commands can be created in a
> log to copy/paste to testpmd.
> The application's flows sequence can then be played back in
> testpmd, to enable enhanced dpdk debug capabilities of the
> applications's flows in a controlled environment of testpmd
> rather than a dynamic, more difficult to debug environment of the
> application.
> 
> Signed-off-by: Eli Britstein <elibr@nvidia.com>
> ---

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

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

* Re: [PATCH V2 2/2] app/testpmd: assign custom ID to flow rules
  2023-07-04  8:25       ` Ori Kam
@ 2023-07-04 14:40         ` Ferruh Yigit
  0 siblings, 0 replies; 11+ messages in thread
From: Ferruh Yigit @ 2023-07-04 14:40 UTC (permalink / raw)
  To: Ori Kam, Gregory Etelson, dev
  Cc: Eli Britstein, Asaf Penso, Thomas Monjalon, Aman Singh, Yuying Zhang

On 7/4/2023 9:25 AM, Ori Kam wrote:
> 
> 
>> -----Original Message-----
>> From: Gregory Etelson <getelson@nvidia.com>
>> Subject: [PATCH V2 2/2] app/testpmd: assign custom ID to flow rules
>>
>> From: Eli Britstein <elibr@nvidia.com>
>>
>> Upon creation of a flow, testpmd assigns it a flow ID. Later, the
>> flow ID is used for flow operations (query, destroy, dump).
>>
>> The testpmd application allows to manage flow rules with its IDs.
>> The flow ID is known only when the flow is created.
>> In order to prepare a complete sequence of testpmd commands to
>> copy/paste, the flow IDs must be predictable.
>>
>> Allow the user to provide an assigned ID.
>>
>> 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 #0 destroyed, user-id 0x1234
>>
>> Here, "user_id" is a flag that signifies the "rule" ID is the user-id.
>>
>> The motivation is from OVS. OVS dumps its "rte_flow_create" calls to the
>> log in testpmd commands syntax. As the flow ID testpmd would assign is
>> unkwon, it cannot log valid "flow destroy" commands.
>>
>> With this enhancement, valid testpmd commands can be created in a
>> log to copy/paste to testpmd.
>> The application's flows sequence can then be played back in
>> testpmd, to enable enhanced dpdk debug capabilities of the
>> applications's flows in a controlled environment of testpmd
>> rather than a dynamic, more difficult to debug environment of the
>> application.
>>
>> Signed-off-by: Eli Britstein <elibr@nvidia.com>
>> ---
> 
> Best,
> Acked-by: Ori Kam <orika@nvidia.com>
>

Series applied to dpdk-next-net/main, thanks.


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

end of thread, other threads:[~2023-07-04 14:40 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-02-22 14:11 [PATCH 1/2] app/testpmd: change rule type Eli Britstein
2023-02-22 14:11 ` [PATCH 2/2] app/testpmd: user assigned flow ID to flows Eli Britstein
2023-02-22 16:50   ` Thomas Monjalon
2023-03-16 14:19   ` [PATCH V2 1/2] app/testpmd: change flow rule type Gregory Etelson
2023-03-16 14:19     ` [PATCH V2 2/2] app/testpmd: assign custom ID to flow rules Gregory Etelson
2023-06-02 20:19       ` Ferruh Yigit
2023-06-30 10:21         ` Ferruh Yigit
2023-07-04  8:25       ` Ori Kam
2023-07-04 14:40         ` Ferruh Yigit
2023-06-02 20:19     ` [PATCH V2 1/2] app/testpmd: change flow rule type Ferruh Yigit
2023-03-01  1:00 ` [PATCH 1/2] app/testpmd: change " Ferruh Yigit

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