[dpdk-dev] [PATCH] app/testpmd: support flow aging

[dpdk-dev] [PATCH] app/testpmd: support flow aging

[Bill Zhou]

Currently, there is no way to check the aging event or to get the current
aged flows in testpmd, this patch include those implements, it's included:
- Registering aging event based on verbose level, when set verbose > 0,
  will register this event, otherwise, remove this event. In this event
  only dump one line of log to user there is one aging event coming.
- Add new command to list all aged flows, meanwhile, we can set parameter
  to destroy it.

Signed-off-by: Bill Zhou <dongz@mellanox.com>
---
 app/test-pmd/cmdline.c      |   4 +
 app/test-pmd/cmdline_flow.c |  61 ++++++++++++++
 app/test-pmd/config.c       | 153 +++++++++++++++++++++++++++++++++---
 app/test-pmd/testpmd.c      |   2 +
 app/test-pmd/testpmd.h      |   9 +++
 5 files changed, 217 insertions(+), 12 deletions(-)

diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c
index 22fb23a92d..01aed7cc1f 100644
--- a/app/test-pmd/cmdline.c
+++ b/app/test-pmd/cmdline.c
@@ -1125,6 +1125,10 @@ static void cmd_help_long_parsed(void *parsed_result,
 			"    Restrict ingress traffic to the defined"
 			" flow rules\n\n"
 
+			"flow aged {port_id} destroy {boolean}\n"
+			"    List and destroy aged flows"
+			" flow rules\n\n"
+
 			"set vxlan ip-version (ipv4|ipv6) vni (vni) udp-src"
 			" (udp-src) udp-dst (udp-dst) ip-src (ip-src) ip-dst"
 			" (ip-dst) eth-src (eth-src) eth-dst (eth-dst)\n"
diff --git a/app/test-pmd/cmdline_flow.c b/app/test-pmd/cmdline_flow.c
index 45bcff3cf5..0349875591 100644
--- a/app/test-pmd/cmdline_flow.c
+++ b/app/test-pmd/cmdline_flow.c
@@ -67,6 +67,7 @@ enum index {
 	DUMP,
 	QUERY,
 	LIST,
+	AGED,
 	ISOLATE,
 
 	/* Destroy arguments. */
@@ -78,6 +79,9 @@ enum index {
 	/* List arguments. */
 	LIST_GROUP,
 
+	/* List aged arguments. */
+	AGED_DESTROY,
+
 	/* Validate/create arguments. */
 	GROUP,
 	PRIORITY,
@@ -664,6 +668,9 @@ struct buffer {
 		struct {
 			int set;
 		} isolate; /**< Isolated mode arguments. */
+		struct {
+			int destroy;
+		} aged; /**< Aged list arguments. */
 	} args; /**< Command arguments. */
 };
 
@@ -719,6 +726,12 @@ static const enum index next_list_attr[] = {
 	ZERO,
 };
 
+static const enum index next_aged_attr[] = {
+	AGED_DESTROY,
+	END,
+	ZERO,
+};
+
 static const enum index item_param[] = {
 	ITEM_PARAM_IS,
 	ITEM_PARAM_SPEC,
@@ -1466,6 +1479,9 @@ static int parse_action(struct context *, const struct token *,
 static int parse_list(struct context *, const struct token *,
 		      const char *, unsigned int,
 		      void *, unsigned int);
+static int parse_aged(struct context *, const struct token *,
+		      const char *, unsigned int,
+		      void *, unsigned int);
 static int parse_isolate(struct context *, const struct token *,
 			 const char *, unsigned int,
 			 void *, unsigned int);
@@ -1649,6 +1665,7 @@ static const struct token token_list[] = {
 			      FLUSH,
 			      DUMP,
 			      LIST,
+			      AGED,
 			      QUERY,
 			      ISOLATE)),
 		.call = parse_init,
@@ -1708,6 +1725,13 @@ static const struct token token_list[] = {
 		.args = ARGS(ARGS_ENTRY(struct buffer, port)),
 		.call = parse_list,
 	},
+	[AGED] = {
+		.name = "aged",
+		.help = "list or destroy aged flows",
+		.next = NEXT(next_aged_attr, NEXT_ENTRY(PORT_ID)),
+		.args = ARGS(ARGS_ENTRY(struct buffer, port)),
+		.call = parse_aged,
+	},
 	[ISOLATE] = {
 		.name = "isolate",
 		.help = "restrict ingress traffic to the defined flow rules",
@@ -1741,6 +1765,13 @@ static const struct token token_list[] = {
 		.args = ARGS(ARGS_ENTRY_PTR(struct buffer, args.list.group)),
 		.call = parse_list,
 	},
+	[AGED_DESTROY] = {
+		.name = "destroy",
+		.help = "specify aged flows need be destroyed",
+		.next = NEXT(NEXT_ENTRY(BOOLEAN)),
+		.args = ARGS(ARGS_ENTRY(struct buffer, args.aged.destroy)),
+		.comp = comp_none,
+	},
 	/* Validate/create attributes. */
 	[GROUP] = {
 		.name = "group",
@@ -5367,6 +5398,33 @@ parse_list(struct context *ctx, const struct token *token,
 	return len;
 }
 
+/** Parse tokens for list all aged flows command. */
+static int
+parse_aged(struct context *ctx, const struct token *token,
+	   const char *str, unsigned int len,
+	   void *buf, unsigned int size)
+{
+	struct buffer *out = buf;
+
+	/* Token name must match. */
+	if (parse_default(ctx, token, str, len, NULL, 0) < 0)
+		return -1;
+	/* Nothing else to do if there is no buffer. */
+	if (!out)
+		return len;
+	if (!out->command) {
+		if (ctx->curr != AGED)
+			return -1;
+		if (sizeof(*out) > size)
+			return -1;
+		out->command = ctx->curr;
+		ctx->objdata = 0;
+		ctx->object = out;
+		ctx->objmask = NULL;
+	}
+	return len;
+}
+
 /** Parse tokens for isolate command. */
 static int
 parse_isolate(struct context *ctx, const struct token *token,
@@ -6367,6 +6425,9 @@ cmd_flow_parsed(const struct buffer *in)
 	case ISOLATE:
 		port_flow_isolate(in->port, in->args.isolate.set);
 		break;
+	case AGED:
+		port_flow_aged(in->port, in->args.aged.destroy);
+		break;
 	default:
 		break;
 	}
diff --git a/app/test-pmd/config.c b/app/test-pmd/config.c
index 72f25d1521..b1133ece84 100644
--- a/app/test-pmd/config.c
+++ b/app/test-pmd/config.c
@@ -1367,6 +1367,26 @@ port_flow_validate(portid_t port_id,
 	return 0;
 }
 
+/** Update age action context by port_flow pointer. */
+void
+update_age_action_context(const struct rte_flow_action *actions,
+			struct port_flow *pf)
+{
+	struct rte_flow_action_age *age = NULL;
+
+	for (; actions->type != RTE_FLOW_ACTION_TYPE_END; actions++) {
+		switch (actions->type) {
+		case RTE_FLOW_ACTION_TYPE_AGE:
+			age = (struct rte_flow_action_age *)
+				(uintptr_t)actions->conf;
+			age->context = pf;
+			return;
+		default:
+			break;
+		}
+	}
+}
+
 /** Create flow rule. */
 int
 port_flow_create(portid_t port_id,
@@ -1377,32 +1397,34 @@ port_flow_create(portid_t port_id,
 	struct rte_flow *flow;
 	struct rte_port *port;
 	struct port_flow *pf;
-	uint32_t id;
+	uint32_t id = 0;
 	struct rte_flow_error error;
 
-	/* Poisoning to make sure PMDs update it in case of error. */
-	memset(&error, 0x22, sizeof(error));
-	flow = rte_flow_create(port_id, attr, pattern, actions, &error);
-	if (!flow)
-		return port_flow_complain(&error);
 	port = &ports[port_id];
 	if (port->flow_list) {
 		if (port->flow_list->id == UINT32_MAX) {
 			printf("Highest rule ID is already assigned, delete"
 			       " it first");
-			rte_flow_destroy(port_id, flow, NULL);
 			return -ENOMEM;
 		}
 		id = port->flow_list->id + 1;
-	} else
-		id = 0;
+	}
 	pf = port_flow_new(attr, pattern, actions, &error);
-	if (!pf) {
-		rte_flow_destroy(port_id, flow, NULL);
+	if (!pf)
+		return port_flow_complain(&error);
+	update_age_action_context(actions, pf);
+	/* Poisoning to make sure PMDs update it in case of error. */
+	memset(&error, 0x22, sizeof(error));
+	flow = rte_flow_create(port_id, attr, pattern, actions, &error);
+	if (!flow) {
+		free(pf);
 		return port_flow_complain(&error);
 	}
-	pf->next = port->flow_list;
 	pf->id = id;
+	pf->prev = NULL;
+	pf->next = port->flow_list;
+	if (pf->next)
+		pf->next->prev = pf;
 	pf->flow = flow;
 	port->flow_list = pf;
 	printf("Flow rule #%u created\n", pf->id);
@@ -1570,6 +1592,64 @@ port_flow_query(portid_t port_id, uint32_t rule,
 	return 0;
 }
 
+/** List simply and destroy all aged flows. */
+void
+port_flow_aged(portid_t port_id, uint8_t destroy)
+{
+	int nb_context, total = 0, idx;
+	void **contexts;
+	struct rte_flow_error error;
+	struct port_flow *pf;
+
+	if (port_id_is_invalid(port_id, ENABLED_WARN) ||
+	    port_id == (portid_t)RTE_PORT_ALL)
+		return;
+	total = rte_flow_get_aged_flows(port_id, NULL, 0, &error);
+	printf("Port %u total aged flows: %d\n", port_id, total);
+	if (total <= 0)
+		return;
+	contexts = malloc(sizeof(void *) * total);
+	printf("ID\tGroup\tPrio\tAttr\n");
+	nb_context = rte_flow_get_aged_flows(port_id, contexts, total, &error);
+	if (nb_context != total) {
+		printf("Port:%d get aged flows count(%d) != total(%d)\n",
+			port_id, nb_context, total);
+		free(contexts);
+		return;
+	}
+	for (idx = 0; idx < nb_context; idx++) {
+		pf = (struct port_flow *)contexts[idx];
+		if (!pf) {
+			printf("Error: get Null context in port %u\n", port_id);
+			continue;
+		}
+		printf("%" PRIu32 "\t%" PRIu32 "\t%" PRIu32 "\t%c%c%c\t\n",
+		       pf->id,
+		       pf->rule.attr->group,
+		       pf->rule.attr->priority,
+		       pf->rule.attr->ingress ? 'i' : '-',
+		       pf->rule.attr->egress ? 'e' : '-',
+		       pf->rule.attr->transfer ? 't' : '-');
+		if (!destroy)
+			continue;
+		if (pf->next)
+			pf->next->prev = pf->prev;
+		if (pf->prev)
+			pf->prev->next = pf->next;
+		else
+			(&ports[port_id])->flow_list = pf->next;
+		/*
+		 * Poisoning to make sure PMDs update it in case
+		 * of error.
+		 */
+		memset(&error, 0x33, sizeof(error));
+		if (rte_flow_destroy(port_id, pf->flow, &error))
+			port_flow_complain(&error);
+		free(pf);
+	}
+	free(contexts);
+}
+
 /** List flow rules. */
 void
 port_flow_list(portid_t port_id, uint32_t n, const uint32_t group[n])
@@ -3162,6 +3242,54 @@ configure_rxtx_dump_callbacks(uint16_t verbose)
 	}
 }
 
+int
+aging_event_callback(uint16_t portid,
+		enum rte_eth_event_type event __rte_unused,
+		void *cb_arg __rte_unused,
+		void *ret_param __rte_unused)
+{
+	printf("port %u RTE_ETH_EVENT_FLOW_AGED triggered\n", portid);
+	return 0;
+}
+
+void
+add_aging_event_callbacks(portid_t portid)
+{
+	if (rte_eth_dev_callback_register(portid,
+					  RTE_ETH_EVENT_FLOW_AGED,
+					  aging_event_callback,
+					  NULL))
+		printf("port %u register RTE_ETH_EVENT_FLOW_AGED fail\n",
+			portid);
+}
+
+void
+remove_aging_event_callbacks(portid_t portid)
+{
+	if (rte_eth_dev_callback_unregister(portid,
+					RTE_ETH_EVENT_FLOW_AGED,
+					aging_event_callback,
+					NULL))
+		printf("port %u unregister RTE_ETH_EVENT_FLOW_AGED fail\n",
+			portid);
+}
+
+void
+configure_aging_event_callbacks(uint16_t verbose)
+{
+	portid_t portid;
+
+	RTE_ETH_FOREACH_DEV(portid)
+	{
+		if (port_id_is_invalid(portid, ENABLED_WARN))
+			continue;
+		if (verbose)
+			add_aging_event_callbacks(portid);
+		else
+			remove_aging_event_callbacks(portid);
+	}
+}
+
 void
 set_verbose_level(uint16_t vb_level)
 {
@@ -3169,6 +3297,7 @@ set_verbose_level(uint16_t vb_level)
 	       (unsigned int) verbose_level, (unsigned int) vb_level);
 	verbose_level = vb_level;
 	configure_rxtx_dump_callbacks(verbose_level);
+	configure_aging_event_callbacks(verbose_level);
 }
 
 void
diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c
index 99bacddbfd..0227ba1c30 100644
--- a/app/test-pmd/testpmd.c
+++ b/app/test-pmd/testpmd.c
@@ -2418,6 +2418,7 @@ start_port(portid_t pid)
 				}
 			}
 			configure_rxtx_dump_callbacks(0);
+			configure_aging_event_callbacks(0);
 			printf("Configuring Port %d (socket %u)\n", pi,
 					port->socket_id);
 			if (nb_hairpinq > 0 &&
@@ -2527,6 +2528,7 @@ start_port(portid_t pid)
 				return -1;
 		}
 		configure_rxtx_dump_callbacks(verbose_level);
+		configure_aging_event_callbacks(verbose_level);
 		if (clear_ptypes) {
 			diag = rte_eth_dev_set_ptypes(pi, RTE_PTYPE_UNKNOWN,
 					NULL, 0);
diff --git a/app/test-pmd/testpmd.h b/app/test-pmd/testpmd.h
index 7ff4c5dba3..64af8fa4be 100644
--- a/app/test-pmd/testpmd.h
+++ b/app/test-pmd/testpmd.h
@@ -146,6 +146,7 @@ struct fwd_stream {
 
 /** Descriptor for a single flow. */
 struct port_flow {
+	struct port_flow *prev; /**< Previous flow in list. */
 	struct port_flow *next; /**< Next flow in list. */
 	struct port_flow *tmp; /**< Temporary linking. */
 	uint32_t id; /**< Flow rule ID. */
@@ -747,12 +748,15 @@ 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);
+void update_age_action_context(const struct rte_flow_action *actions,
+		     struct port_flow *pf);
 int port_flow_destroy(portid_t port_id, uint32_t n, const uint32_t *rule);
 int port_flow_flush(portid_t port_id);
 int port_flow_dump(portid_t port_id, const char *file_name);
 int port_flow_query(portid_t port_id, uint32_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);
 int port_flow_isolate(portid_t port_id, int set);
 
 void rx_ring_desc_display(portid_t port_id, queueid_t rxq_id, uint16_t rxd_id);
@@ -895,6 +899,11 @@ void remove_rx_dump_callbacks(portid_t portid);
 void add_tx_dump_callbacks(portid_t portid);
 void remove_tx_dump_callbacks(portid_t portid);
 void configure_rxtx_dump_callbacks(uint16_t verbose);
+int aging_event_callback(uint16_t portid, enum rte_eth_event_type event,
+		      void *cb_arg, void *ret_param);
+void remove_aging_event_callbacks(portid_t portid);
+void add_aging_event_callbacks(portid_t portid);
+void configure_aging_event_callbacks(uint16_t verbose);
 
 uint16_t tx_pkt_set_md(uint16_t port_id, __rte_unused uint16_t queue,
 		       struct rte_mbuf *pkts[], uint16_t nb_pkts,
-- 
2.21.0

Re: [dpdk-dev] [PATCH] app/testpmd: support flow aging

[Ferruh Yigit]

On 4/24/2020 11:55 AM, Bill Zhou wrote:
> Currently, there is no way to check the aging event or to get the current
> aged flows in testpmd, this patch include those implements, it's included:
> - Registering aging event based on verbose level, when set verbose > 0,
>   will register this event, otherwise, remove this event. In this event
>   only dump one line of log to user there is one aging event coming.
> - Add new command to list all aged flows, meanwhile, we can set parameter
>   to destroy it.

Can you please document new feature and command?

Instead of overloading the 'verbose', what do you think having explicit command
to register aging events? ("flow aged register <port_id>"?) I think many of the
verbose usage won't really interest in the flow aging.

> 
> Signed-off-by: Bill Zhou <dongz@mellanox.com>

<...>

Re: [dpdk-dev] [PATCH] app/testpmd: support flow aging

[Bill Zhou]

> -----Original Message-----
> From: Ferruh Yigit <ferruh.yigit@intel.com>
> Sent: Saturday, April 25, 2020 12:25 AM
> To: Bill Zhou <dongz@mellanox.com>; wenzhuo.lu@intel.com;
> jingjing.wu@intel.com; bernard.iremonger@intel.com; Ori Kam
> <orika@mellanox.com>
> Cc: dev@dpdk.org
> Subject: Re: [dpdk-dev] [PATCH] app/testpmd: support flow aging
> 
> On 4/24/2020 11:55 AM, Bill Zhou wrote:
> > Currently, there is no way to check the aging event or to get the
> > current aged flows in testpmd, this patch include those implements, it's
> included:
> > - Registering aging event based on verbose level, when set verbose > 0,
> >   will register this event, otherwise, remove this event. In this event
> >   only dump one line of log to user there is one aging event coming.
> > - Add new command to list all aged flows, meanwhile, we can set
> parameter
> >   to destroy it.
> 
> Can you please document new feature and command?
> 
> Instead of overloading the 'verbose', what do you think having explicit
> command to register aging events? ("flow aged register <port_id>"?) I think
> many of the verbose usage won't really interest in the flow aging.
> 

Yes, some of the verbose usage indeed not interest in the flow aging. But If we use
register or unregister event to one port, sometime, it will repeat many times for
every ports.
What do you think if we register this event all the time, and introduce new global
var from command to control the event export to application ?
   for example: set aged_flow_event_print_en [0 | 1]

> >
> > Signed-off-by: Bill Zhou <dongz@mellanox.com>
> 
> <...>

Re: [dpdk-dev] [PATCH] app/testpmd: support flow aging

[Ferruh Yigit]

On 4/26/2020 8:23 AM, Bill Zhou wrote:
> 
> 
>> -----Original Message-----
>> From: Ferruh Yigit <ferruh.yigit@intel.com>
>> Sent: Saturday, April 25, 2020 12:25 AM
>> To: Bill Zhou <dongz@mellanox.com>; wenzhuo.lu@intel.com;
>> jingjing.wu@intel.com; bernard.iremonger@intel.com; Ori Kam
>> <orika@mellanox.com>
>> Cc: dev@dpdk.org
>> Subject: Re: [dpdk-dev] [PATCH] app/testpmd: support flow aging
>>
>> On 4/24/2020 11:55 AM, Bill Zhou wrote:
>>> Currently, there is no way to check the aging event or to get the
>>> current aged flows in testpmd, this patch include those implements, it's
>> included:
>>> - Registering aging event based on verbose level, when set verbose > 0,
>>>   will register this event, otherwise, remove this event. In this event
>>>   only dump one line of log to user there is one aging event coming.
>>> - Add new command to list all aged flows, meanwhile, we can set
>> parameter
>>>   to destroy it.
>>
>> Can you please document new feature and command?
>>
>> Instead of overloading the 'verbose', what do you think having explicit
>> command to register aging events? ("flow aged register <port_id>"?) I think
>> many of the verbose usage won't really interest in the flow aging.
>>
> 
> Yes, some of the verbose usage indeed not interest in the flow aging. But If we use
> register or unregister event to one port, sometime, it will repeat many times for
> every ports.
> What do you think if we register this event all the time, and introduce new global
> var from command to control the event export to application ?
>    for example: set aged_flow_event_print_en [0 | 1]

I am not also sure about registering this event always, you can register for all
ports with same command, as we do in many commands:
"flow aged register <port_id>|all"

When argument is "all" it registers for all ports, when it is a specific
port_id, registers for that port id.
And it is good to have 'unregister' command too.

> 
>>>
>>> Signed-off-by: Bill Zhou <dongz@mellanox.com>
>>
>> <...>

Re: [dpdk-dev] [PATCH] app/testpmd: support flow aging

[Matan Azrad]

Hi

From: Ferruh Yigit
> On 4/26/2020 8:23 AM, Bill Zhou wrote:
> >
> >
> >> -----Original Message-----
> >> From: Ferruh Yigit <ferruh.yigit@intel.com>
> >> Sent: Saturday, April 25, 2020 12:25 AM
> >> To: Bill Zhou <dongz@mellanox.com>; wenzhuo.lu@intel.com;
> >> jingjing.wu@intel.com; bernard.iremonger@intel.com; Ori Kam
> >> <orika@mellanox.com>
> >> Cc: dev@dpdk.org
> >> Subject: Re: [dpdk-dev] [PATCH] app/testpmd: support flow aging
> >>
> >> On 4/24/2020 11:55 AM, Bill Zhou wrote:
> >>> Currently, there is no way to check the aging event or to get the
> >>> current aged flows in testpmd, this patch include those implements,
> >>> it's
> >> included:
> >>> - Registering aging event based on verbose level, when set verbose > 0,
> >>>   will register this event, otherwise, remove this event. In this event
> >>>   only dump one line of log to user there is one aging event coming.
> >>> - Add new command to list all aged flows, meanwhile, we can set
> >> parameter
> >>>   to destroy it.
> >>
> >> Can you please document new feature and command?
> >>
> >> Instead of overloading the 'verbose', what do you think having
> >> explicit command to register aging events? ("flow aged register
> >> <port_id>"?) I think many of the verbose usage won't really interest in
> the flow aging.
> >>
> >
> > Yes, some of the verbose usage indeed not interest in the flow aging.
> > But If we use register or unregister event to one port, sometime, it
> > will repeat many times for every ports.
> > What do you think if we register this event all the time, and
> > introduce new global var from command to control the event export to
> application ?
> >    for example: set aged_flow_event_print_en [0 | 1]
> 
> I am not also sure about registering this event always, you can register for all
> ports with same command, as we do in many commands:
> "flow aged register <port_id>|all"
> 
> When argument is "all" it registers for all ports, when it is a specific port_id,
> registers for that port id.
> And it is good to have 'unregister' command too.

As I can see in testpmd default, it all the time registers to all events..
See register_eth_event_callback.

I suggest to catch the age event in callback and print it only when set aged_flow_event_print_en is on.


Matan


> 
> >
> >>>
> >>> Signed-off-by: Bill Zhou <dongz@mellanox.com>
> >>
> >> <...>

Re: [dpdk-dev] [PATCH] app/testpmd: support flow aging

[Ferruh Yigit]

On 4/27/2020 4:12 PM, Matan Azrad wrote:
> Hi
> 
> From: Ferruh Yigit
>> On 4/26/2020 8:23 AM, Bill Zhou wrote:
>>>
>>>
>>>> -----Original Message-----
>>>> From: Ferruh Yigit <ferruh.yigit@intel.com>
>>>> Sent: Saturday, April 25, 2020 12:25 AM
>>>> To: Bill Zhou <dongz@mellanox.com>; wenzhuo.lu@intel.com;
>>>> jingjing.wu@intel.com; bernard.iremonger@intel.com; Ori Kam
>>>> <orika@mellanox.com>
>>>> Cc: dev@dpdk.org
>>>> Subject: Re: [dpdk-dev] [PATCH] app/testpmd: support flow aging
>>>>
>>>> On 4/24/2020 11:55 AM, Bill Zhou wrote:
>>>>> Currently, there is no way to check the aging event or to get the
>>>>> current aged flows in testpmd, this patch include those implements,
>>>>> it's
>>>> included:
>>>>> - Registering aging event based on verbose level, when set verbose > 0,
>>>>>   will register this event, otherwise, remove this event. In this event
>>>>>   only dump one line of log to user there is one aging event coming.
>>>>> - Add new command to list all aged flows, meanwhile, we can set
>>>> parameter
>>>>>   to destroy it.
>>>>
>>>> Can you please document new feature and command?
>>>>
>>>> Instead of overloading the 'verbose', what do you think having
>>>> explicit command to register aging events? ("flow aged register
>>>> <port_id>"?) I think many of the verbose usage won't really interest in
>> the flow aging.
>>>>
>>>
>>> Yes, some of the verbose usage indeed not interest in the flow aging.
>>> But If we use register or unregister event to one port, sometime, it
>>> will repeat many times for every ports.
>>> What do you think if we register this event all the time, and
>>> introduce new global var from command to control the event export to
>> application ?
>>>    for example: set aged_flow_event_print_en [0 | 1]
>>
>> I am not also sure about registering this event always, you can register for all
>> ports with same command, as we do in many commands:
>> "flow aged register <port_id>|all"
>>
>> When argument is "all" it registers for all ports, when it is a specific port_id,
>> registers for that port id.
>> And it is good to have 'unregister' command too.
> 
> As I can see in testpmd default, it all the time registers to all events..
> See register_eth_event_callback.

You are right.

> 
> I suggest to catch the age event in callback and print it only when set aged_flow_event_print_en is on.

+1

> 
> 
> Matan
> 
> 
>>
>>>
>>>>>
>>>>> Signed-off-by: Bill Zhou <dongz@mellanox.com>
>>>>
>>>> <...>
> 

[dpdk-dev] [PATCH v2] app/testpmd: support flow aging

[Bill Zhou]

Currently, there is no way to check the aging event or to get the current
aged flows in testpmd, this patch include those implements, it's included:
- Registering aging event when the testpmd application start, add new
  command to control if the event expose to the applications. If it's be
  set, when new flow be checked age out, there will be one-line output log.
- Add new command to list all aged flows, meanwhile, we can set parameter
  to destroy it.

Signed-off-by: Bill Zhou <dongz@mellanox.com>
---
v2: Update the way of registering aging event, add new command to control
if the event need be print or not. Update the output of the delete aged
flow command format.
---
 app/test-pmd/cmdline.c                      |  46 +++++++++
 app/test-pmd/cmdline_flow.c                 |  62 +++++++++++
 app/test-pmd/config.c                       | 108 ++++++++++++++++++--
 app/test-pmd/testpmd.c                      |  17 +++
 app/test-pmd/testpmd.h                      |   6 +-
 doc/guides/testpmd_app_ug/testpmd_funcs.rst |  77 ++++++++++++++
 6 files changed, 304 insertions(+), 12 deletions(-)

diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c
index 1375f223eb..2b1dc8f8d7 100644
--- a/app/test-pmd/cmdline.c
+++ b/app/test-pmd/cmdline.c
@@ -738,6 +738,9 @@ static void cmd_help_long_parsed(void *parsed_result,
 			"show port (port_id) queue-region\n"
 			"    show all queue region related configuration info\n\n"
 
+			"set aged_flow_event_log (on|off)\n"
+			"	 Enable or disable aged flow event logging\n\n"
+
 			, list_pkt_forwarding_modes()
 		);
 	}
@@ -1125,6 +1128,10 @@ static void cmd_help_long_parsed(void *parsed_result,
 			"    Restrict ingress traffic to the defined"
 			" flow rules\n\n"
 
+			"flow aged {port_id} [destroy]\n"
+			"    List and destroy aged flows"
+			" flow rules\n\n"
+
 			"set vxlan ip-version (ipv4|ipv6) vni (vni) udp-src"
 			" (udp-src) udp-dst (udp-dst) ip-src (ip-src) ip-dst"
 			" (ip-dst) eth-src (eth-src) eth-dst (eth-dst)\n"
@@ -19387,6 +19394,44 @@ cmdline_parse_inst_t cmd_showport_macs = {
 	},
 };
 
+/* Enable/Disable flow aging event log */
+struct cmd_set_aged_flow_event_log_result {
+	cmdline_fixed_string_t set;
+	cmdline_fixed_string_t keyword;
+	cmdline_fixed_string_t enable;
+};
+cmdline_parse_token_string_t cmd_set_aged_flow_event_log_set =
+	TOKEN_STRING_INITIALIZER(struct cmd_set_aged_flow_event_log_result,
+		set, "set");
+cmdline_parse_token_string_t cmd_set_aged_flow_event_log_keyword =
+	TOKEN_STRING_INITIALIZER(struct cmd_set_aged_flow_event_log_result,
+		keyword, "aged_flow_event_log");
+cmdline_parse_token_string_t cmd_set_aged_flow_event_log_enable =
+	TOKEN_STRING_INITIALIZER(struct cmd_set_aged_flow_event_log_result,
+		enable, "on#off");
+
+static void
+cmd_set_aged_flow_event_log_parsed(void *parsed_result,
+				__rte_unused struct cmdline *cl,
+				__rte_unused void *data)
+{
+	struct cmd_set_aged_flow_event_log_result *res = parsed_result;
+	int enable = (strcmp(res->enable, "on") == 0) ? 1 : 0;
+	update_aging_event_log_status(enable);
+}
+
+cmdline_parse_inst_t cmd_set_aged_flow_event_log = {
+	.f = cmd_set_aged_flow_event_log_parsed,
+	.data = NULL,
+	.help_str = "set aged_flow_event_log on|off",
+	.tokens = {
+		(void *)&cmd_set_aged_flow_event_log_set,
+		(void *)&cmd_set_aged_flow_event_log_keyword,
+		(void *)&cmd_set_aged_flow_event_log_enable,
+		NULL,
+	},
+};
+
 /* ******************************************************************************** */
 
 /* list of instructions */
@@ -19684,6 +19729,7 @@ cmdline_parse_ctx_t main_ctx[] = {
 	(cmdline_parse_inst_t *)&cmd_show_set_raw,
 	(cmdline_parse_inst_t *)&cmd_show_set_raw_all,
 	(cmdline_parse_inst_t *)&cmd_config_tx_dynf_specific,
+	(cmdline_parse_inst_t *)&cmd_set_aged_flow_event_log,
 	NULL,
 };
 
diff --git a/app/test-pmd/cmdline_flow.c b/app/test-pmd/cmdline_flow.c
index 45bcff3cf5..4e2006c543 100644
--- a/app/test-pmd/cmdline_flow.c
+++ b/app/test-pmd/cmdline_flow.c
@@ -67,6 +67,7 @@ enum index {
 	DUMP,
 	QUERY,
 	LIST,
+	AGED,
 	ISOLATE,
 
 	/* Destroy arguments. */
@@ -78,6 +79,9 @@ enum index {
 	/* List arguments. */
 	LIST_GROUP,
 
+	/* Destroy aged flow arguments. */
+	AGED_DESTROY,
+
 	/* Validate/create arguments. */
 	GROUP,
 	PRIORITY,
@@ -664,6 +668,9 @@ struct buffer {
 		struct {
 			int set;
 		} isolate; /**< Isolated mode arguments. */
+		struct {
+			int destroy;
+		} aged; /**< Aged arguments. */
 	} args; /**< Command arguments. */
 };
 
@@ -719,6 +726,12 @@ static const enum index next_list_attr[] = {
 	ZERO,
 };
 
+static const enum index next_aged_attr[] = {
+	AGED_DESTROY,
+	END,
+	ZERO,
+};
+
 static const enum index item_param[] = {
 	ITEM_PARAM_IS,
 	ITEM_PARAM_SPEC,
@@ -1466,6 +1479,9 @@ static int parse_action(struct context *, const struct token *,
 static int parse_list(struct context *, const struct token *,
 		      const char *, unsigned int,
 		      void *, unsigned int);
+static int parse_aged(struct context *, const struct token *,
+		      const char *, unsigned int,
+		      void *, unsigned int);
 static int parse_isolate(struct context *, const struct token *,
 			 const char *, unsigned int,
 			 void *, unsigned int);
@@ -1649,6 +1665,7 @@ static const struct token token_list[] = {
 			      FLUSH,
 			      DUMP,
 			      LIST,
+			      AGED,
 			      QUERY,
 			      ISOLATE)),
 		.call = parse_init,
@@ -1708,6 +1725,13 @@ static const struct token token_list[] = {
 		.args = ARGS(ARGS_ENTRY(struct buffer, port)),
 		.call = parse_list,
 	},
+	[AGED] = {
+		.name = "aged",
+		.help = "list and destroy aged flows",
+		.next = NEXT(next_aged_attr, NEXT_ENTRY(PORT_ID)),
+		.args = ARGS(ARGS_ENTRY(struct buffer, port)),
+		.call = parse_aged,
+	},
 	[ISOLATE] = {
 		.name = "isolate",
 		.help = "restrict ingress traffic to the defined flow rules",
@@ -1741,6 +1765,12 @@ static const struct token token_list[] = {
 		.args = ARGS(ARGS_ENTRY_PTR(struct buffer, args.list.group)),
 		.call = parse_list,
 	},
+	[AGED_DESTROY] = {
+		.name = "destroy",
+		.help = "specify aged flows need be destroyed",
+		.call = parse_aged,
+		.comp = comp_none,
+	},
 	/* Validate/create attributes. */
 	[GROUP] = {
 		.name = "group",
@@ -5367,6 +5397,35 @@ parse_list(struct context *ctx, const struct token *token,
 	return len;
 }
 
+/** Parse tokens for list all aged flows command. */
+static int
+parse_aged(struct context *ctx, const struct token *token,
+	   const char *str, unsigned int len,
+	   void *buf, unsigned int size)
+{
+	struct buffer *out = buf;
+
+	/* Token name must match. */
+	if (parse_default(ctx, token, str, len, NULL, 0) < 0)
+		return -1;
+	/* Nothing else to do if there is no buffer. */
+	if (!out)
+		return len;
+	if (!out->command) {
+		if (ctx->curr != AGED)
+			return -1;
+		if (sizeof(*out) > size)
+			return -1;
+		out->command = ctx->curr;
+		ctx->objdata = 0;
+		ctx->object = out;
+		ctx->objmask = NULL;
+	}
+	if (ctx->curr == AGED_DESTROY)
+		out->args.aged.destroy = 1;
+	return len;
+}
+
 /** Parse tokens for isolate command. */
 static int
 parse_isolate(struct context *ctx, const struct token *token,
@@ -6367,6 +6426,9 @@ cmd_flow_parsed(const struct buffer *in)
 	case ISOLATE:
 		port_flow_isolate(in->port, in->args.isolate.set);
 		break;
+	case AGED:
+		port_flow_aged(in->port, in->args.aged.destroy);
+		break;
 	default:
 		break;
 	}
diff --git a/app/test-pmd/config.c b/app/test-pmd/config.c
index 72f25d1521..035d336ab5 100644
--- a/app/test-pmd/config.c
+++ b/app/test-pmd/config.c
@@ -1367,6 +1367,26 @@ port_flow_validate(portid_t port_id,
 	return 0;
 }
 
+/** Update age action context by port_flow pointer. */
+void
+update_age_action_context(const struct rte_flow_action *actions,
+			struct port_flow *pf)
+{
+	struct rte_flow_action_age *age = NULL;
+
+	for (; actions->type != RTE_FLOW_ACTION_TYPE_END; actions++) {
+		switch (actions->type) {
+		case RTE_FLOW_ACTION_TYPE_AGE:
+			age = (struct rte_flow_action_age *)
+				(uintptr_t)actions->conf;
+			age->context = pf;
+			return;
+		default:
+			break;
+		}
+	}
+}
+
 /** Create flow rule. */
 int
 port_flow_create(portid_t port_id,
@@ -1377,28 +1397,27 @@ port_flow_create(portid_t port_id,
 	struct rte_flow *flow;
 	struct rte_port *port;
 	struct port_flow *pf;
-	uint32_t id;
+	uint32_t id = 0;
 	struct rte_flow_error error;
 
-	/* Poisoning to make sure PMDs update it in case of error. */
-	memset(&error, 0x22, sizeof(error));
-	flow = rte_flow_create(port_id, attr, pattern, actions, &error);
-	if (!flow)
-		return port_flow_complain(&error);
 	port = &ports[port_id];
 	if (port->flow_list) {
 		if (port->flow_list->id == UINT32_MAX) {
 			printf("Highest rule ID is already assigned, delete"
 			       " it first");
-			rte_flow_destroy(port_id, flow, NULL);
 			return -ENOMEM;
 		}
 		id = port->flow_list->id + 1;
-	} else
-		id = 0;
+	}
 	pf = port_flow_new(attr, pattern, actions, &error);
-	if (!pf) {
-		rte_flow_destroy(port_id, flow, NULL);
+	if (!pf)
+		return port_flow_complain(&error);
+	update_age_action_context(actions, pf);
+	/* Poisoning to make sure PMDs update it in case of error. */
+	memset(&error, 0x22, sizeof(error));
+	flow = rte_flow_create(port_id, attr, pattern, actions, &error);
+	if (!flow) {
+		free(pf);
 		return port_flow_complain(&error);
 	}
 	pf->next = port->flow_list;
@@ -1570,6 +1589,73 @@ port_flow_query(portid_t port_id, uint32_t rule,
 	return 0;
 }
 
+/** List simply and destroy all aged flows. */
+void
+port_flow_aged(portid_t port_id, uint8_t destroy)
+{
+	void **contexts;
+	int nb_context, total = 0, idx;
+	struct rte_flow_error error;
+	struct port_flow *pf;
+
+	if (port_id_is_invalid(port_id, ENABLED_WARN) ||
+	    port_id == (portid_t)RTE_PORT_ALL)
+		return;
+	total = rte_flow_get_aged_flows(port_id, NULL, 0, &error);
+	printf("Port %u total aged flows: %d\n", port_id, total);
+	if (total < 0) {
+		port_flow_complain(&error);
+		return;
+	}
+	if (total == 0)
+		return;
+	contexts = malloc(sizeof(void *) * total);
+	if (contexts == NULL) {
+		printf("Cannot allocate contexts for aged flow\n");
+		return;
+	}
+	printf("ID\tGroup\tPrio\tAttr\n");
+	nb_context = rte_flow_get_aged_flows(port_id, contexts, total, &error);
+	if (nb_context != total) {
+		printf("Port:%d get aged flows count(%d) != total(%d)\n",
+			port_id, nb_context, total);
+		free(contexts);
+		return;
+	}
+	for (idx = 0; idx < nb_context; idx++) {
+		pf = (struct port_flow *)contexts[idx];
+		if (!pf) {
+			printf("Error: get Null context in port %u\n", port_id);
+			continue;
+		}
+		printf("%" PRIu32 "\t%" PRIu32 "\t%" PRIu32 "\t%c%c%c\t\n",
+		       pf->id,
+		       pf->rule.attr->group,
+		       pf->rule.attr->priority,
+		       pf->rule.attr->ingress ? 'i' : '-',
+		       pf->rule.attr->egress ? 'e' : '-',
+		       pf->rule.attr->transfer ? 't' : '-');
+	}
+	if (destroy) {
+		int ret;
+		uint32_t flow_id;
+
+		total = 0;
+		printf("\n");
+		for (idx = 0; idx < nb_context; idx++) {
+			pf = (struct port_flow *)contexts[idx];
+			if (!pf)
+				continue;
+			flow_id = pf->id;
+			ret = port_flow_destroy(port_id, 1, &flow_id);
+			if (!ret)
+				total++;
+		}
+		printf("%d flows be destroyed\n", total);
+	}
+	free(contexts);
+}
+
 /** List flow rules. */
 void
 port_flow_list(portid_t port_id, uint32_t n, const uint32_t group[n])
diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c
index 99bacddbfd..585f5d156d 100644
--- a/app/test-pmd/testpmd.c
+++ b/app/test-pmd/testpmd.c
@@ -3068,6 +3068,21 @@ rmv_port_callback(void *arg)
 		start_packet_forwarding(0);
 }
 
+static int aged_flow_event_enable;
+void update_aging_event_log_status(int enable)
+{
+	aged_flow_event_enable = enable;
+}
+
+int aging_event_output(uint16_t portid)
+{
+	if (aged_flow_event_enable) {
+		printf("port %u RTE_ETH_EVENT_FLOW_AGED triggered\n", portid);
+		fflush(stdout);
+	}
+	return 0;
+}
+
 /* This function is used by the interrupt thread */
 static int
 eth_event_callback(portid_t port_id, enum rte_eth_event_type type, void *param,
@@ -3098,6 +3113,8 @@ eth_event_callback(portid_t port_id, enum rte_eth_event_type type, void *param,
 				rmv_port_callback, (void *)(intptr_t)port_id))
 			fprintf(stderr, "Could not set up deferred device removal\n");
 		break;
+	case RTE_ETH_EVENT_FLOW_AGED:
+		aging_event_output(port_id);
 	default:
 		break;
 	}
diff --git a/app/test-pmd/testpmd.h b/app/test-pmd/testpmd.h
index 7ff4c5dba3..046bde8c9a 100644
--- a/app/test-pmd/testpmd.h
+++ b/app/test-pmd/testpmd.h
@@ -747,14 +747,16 @@ 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);
+void update_age_action_context(const struct rte_flow_action *actions,
+		     struct port_flow *pf);
 int port_flow_destroy(portid_t port_id, uint32_t n, const uint32_t *rule);
 int port_flow_flush(portid_t port_id);
 int port_flow_dump(portid_t port_id, const char *file_name);
 int port_flow_query(portid_t port_id, uint32_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);
 int port_flow_isolate(portid_t port_id, int set);
-
 void rx_ring_desc_display(portid_t port_id, queueid_t rxq_id, uint16_t rxd_id);
 void tx_ring_desc_display(portid_t port_id, queueid_t txq_id, uint16_t txd_id);
 
@@ -895,6 +897,8 @@ void remove_rx_dump_callbacks(portid_t portid);
 void add_tx_dump_callbacks(portid_t portid);
 void remove_tx_dump_callbacks(portid_t portid);
 void configure_rxtx_dump_callbacks(uint16_t verbose);
+void update_aging_event_log_status(int enable);
+int aging_event_output(uint16_t portid);
 
 uint16_t tx_pkt_set_md(uint16_t port_id, __rte_unused uint16_t queue,
 		       struct rte_mbuf *pkts[], uint16_t nb_pkts,
diff --git a/doc/guides/testpmd_app_ug/testpmd_funcs.rst b/doc/guides/testpmd_app_ug/testpmd_funcs.rst
index a360ecccfd..9ca30eea9b 100644
--- a/doc/guides/testpmd_app_ug/testpmd_funcs.rst
+++ b/doc/guides/testpmd_app_ug/testpmd_funcs.rst
@@ -1062,6 +1062,21 @@ Where:
 
    Check the NIC Datasheet for hardware limits.
 
+aged flow event log set
+~~~~~~~~~~~~~~~~~~~~~~~
+
+Currently, the flow aging event is registered when the testpmd application start, if new flow be checked
+age out, there will be one output log for it. But some applications do not interest this event, use this
+command can set if the event expose to the applications::
+
+   testpmd> set aged_flow_event_log (on|off)
+
+For examine::
+
+   testpmd> set aged_flow_event_log on
+   testpmd> port 0 RTE_ETH_EVENT_FLOW_AGED triggered
+   testpmd> set aged_flow_event_log off
+
 RSS queue region
 ~~~~~~~~~~~~~~~~
 
@@ -3616,6 +3631,10 @@ following sections.
 
    flow dump {port_id} {output_file}
 
+- List and destroy aged flow rules::
+
+   flow aged {port_id} [destroy]
+
 Validating flow rules
 ~~~~~~~~~~~~~~~~~~~~~
 
@@ -4503,6 +4522,64 @@ Otherwise, it will complain error occurred::
 
    Caught error type [...] ([...]): [...]
 
+Listing and destroying aged flow rules
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+
+``flow aged`` simply lists aged flow rules be get from api ``rte_flow_get_aged_flows``,
+and ``destroy`` parameter can be used to destroy those flow rules in PMD.
+
+   flow aged {port_id} [destroy]
+
+Listing current aged flow rules::
+
+   testpmd> flow aged 0
+   Port 0 total aged flows: 0
+   testpmd> flow create 0 ingress pattern eth / ipv4 src is 2.2.2.14 / end
+      actions age timeout 5 / queue index 0 /  end
+   Flow rule #0 created
+   testpmd> flow create 0 ingress pattern eth / ipv4 src is 2.2.2.15 / end
+      actions age timeout 4 / queue index 0 /  end
+   Flow rule #1 created
+   testpmd> flow create 0 ingress pattern eth / ipv4 src is 2.2.2.16 / end
+      actions age timeout 2 / queue index 0 /  end
+   Flow rule #2 created
+   testpmd> flow create 0 ingress pattern eth / ipv4 src is 2.2.2.17 / end
+      actions age timeout 3 / queue index 0 /  end
+   Flow rule #3 created
+
+
+Aged Rules are simply list as command ``flow list {port_id}``, but strip the detail rule
+information, all the aged flows are sorted by the longest timeout time. For example, if
+those rules be configured in the same time, ID 2 will be the first aged out rule, the next
+will be ID 3, ID 1, ID 0::
+
+   testpmd> flow aged 0
+   Port 0 total aged flows: 4
+   ID      Group   Prio    Attr
+   2       0       0       i--
+   3       0       0       i--
+   1       0       0       i--
+   0       0       0       i--
+
+If attach ``destroy`` parameter, the command will destroy all the list aged flow rules.
+
+   testpmd> flow aged 0 destroy
+   Port 0 total aged flows: 4
+   ID      Group   Prio    Attr
+   2       0       0       i--
+   3       0       0       i--
+   1       0       0       i--
+   0       0       0       i--
+
+   Flow rule #2 destroyed
+   Flow rule #3 destroyed
+   Flow rule #1 destroyed
+   Flow rule #0 destroyed
+   4 flows be destroyed
+   testpmd> flow aged 0
+   Port 0 total aged flows: 0
+
+
 Sample QinQ flow rules
 ~~~~~~~~~~~~~~~~~~~~~~
 
-- 
2.21.0

Re: [dpdk-dev] [PATCH v2] app/testpmd: support flow aging

[Ferruh Yigit]

On 4/30/2020 4:53 PM, Bill Zhou wrote:
> Currently, there is no way to check the aging event or to get the current
> aged flows in testpmd, this patch include those implements, it's included:
> - Registering aging event when the testpmd application start, add new
>   command to control if the event expose to the applications. If it's be
>   set, when new flow be checked age out, there will be one-line output log.
> - Add new command to list all aged flows, meanwhile, we can set parameter
>   to destroy it.
> 
> Signed-off-by: Bill Zhou <dongz@mellanox.com>
> ---
> v2: Update the way of registering aging event, add new command to control
> if the event need be print or not. Update the output of the delete aged
> flow command format.

<...>

> @@ -19387,6 +19394,44 @@ cmdline_parse_inst_t cmd_showport_macs = {
>  	},
>  };
>  
> +/* Enable/Disable flow aging event log */
> +struct cmd_set_aged_flow_event_log_result {
> +	cmdline_fixed_string_t set;
> +	cmdline_fixed_string_t keyword;
> +	cmdline_fixed_string_t enable;
> +};
> +cmdline_parse_token_string_t cmd_set_aged_flow_event_log_set =
> +	TOKEN_STRING_INITIALIZER(struct cmd_set_aged_flow_event_log_result,
> +		set, "set");
> +cmdline_parse_token_string_t cmd_set_aged_flow_event_log_keyword =
> +	TOKEN_STRING_INITIALIZER(struct cmd_set_aged_flow_event_log_result,
> +		keyword, "aged_flow_event_log");
> +cmdline_parse_token_string_t cmd_set_aged_flow_event_log_enable =
> +	TOKEN_STRING_INITIALIZER(struct cmd_set_aged_flow_event_log_result,
> +		enable, "on#off");
> +
> +static void
> +cmd_set_aged_flow_event_log_parsed(void *parsed_result,
> +				__rte_unused struct cmdline *cl,
> +				__rte_unused void *data)
> +{
> +	struct cmd_set_aged_flow_event_log_result *res = parsed_result;
> +	int enable = (strcmp(res->enable, "on") == 0) ? 1 : 0;
> +	update_aging_event_log_status(enable);
> +}
> +
> +cmdline_parse_inst_t cmd_set_aged_flow_event_log = {
> +	.f = cmd_set_aged_flow_event_log_parsed,
> +	.data = NULL,
> +	.help_str = "set aged_flow_event_log on|off",

What do you think "set aged_flow_verbose on|off" to be more similar to existing
verbose command?

<...>

> --- a/app/test-pmd/testpmd.c
> +++ b/app/test-pmd/testpmd.c
> @@ -3068,6 +3068,21 @@ rmv_port_callback(void *arg)
>  		start_packet_forwarding(0);
>  }
>  
> +static int aged_flow_event_enable;

Other global config values are at the beginning of the file, with a comment on
them, can you do same with variable?

> +void update_aging_event_log_status(int enable)
> +{
> +	aged_flow_event_enable = enable;
> +}
> +
> +int aging_event_output(uint16_t portid)

This can be a 'static' function.

> +{
> +	if (aged_flow_event_enable) {
> +		printf("port %u RTE_ETH_EVENT_FLOW_AGED triggered\n", portid);
> +		fflush(stdout);
> +	}
> +	return 0;
> +}
> +
>  /* This function is used by the interrupt thread */
>  static int
>  eth_event_callback(portid_t port_id, enum rte_eth_event_type type, void *param,
> @@ -3098,6 +3113,8 @@ eth_event_callback(portid_t port_id, enum rte_eth_event_type type, void *param,
>  				rmv_port_callback, (void *)(intptr_t)port_id))
>  			fprintf(stderr, "Could not set up deferred device removal\n");
>  		break;
> +	case RTE_ETH_EVENT_FLOW_AGED:
> +		aging_event_output(port_id);

Can't this provide more information than port_id, like flow id etc, what
event_process function provides? can we print it too?
And what is the intended usage in real application, when flow aged event
occurred, should application destroy the flow? So it should know the flow, right?

<...>

> --- a/doc/guides/testpmd_app_ug/testpmd_funcs.rst
> +++ b/doc/guides/testpmd_app_ug/testpmd_funcs.rst
> @@ -1062,6 +1062,21 @@ Where:
>  
>     Check the NIC Datasheet for hardware limits.
>  
> +aged flow event log set
> +~~~~~~~~~~~~~~~~~~~~~~~
> +
> +Currently, the flow aging event is registered when the testpmd application start, if new flow be checked
> +age out, there will be one output log for it. But some applications do not interest this event, use this
> +command can set if the event expose to the applications::
> +
> +   testpmd> set aged_flow_event_log (on|off)
> +
> +For examine::
> +
> +   testpmd> set aged_flow_event_log on
> +   testpmd> port 0 RTE_ETH_EVENT_FLOW_AGED triggered
> +   testpmd> set aged_flow_event_log off
> +

This can be moved below the "set verbose" command since they are in similar group.

Re: [dpdk-dev] [PATCH v2] app/testpmd: support flow aging

[Matan Azrad]

Hi Ferruh

From: Ferruh Yigit
> On 4/30/2020 4:53 PM, Bill Zhou wrote:
> > Currently, there is no way to check the aging event or to get the
> > current aged flows in testpmd, this patch include those implements, it's
> included:
> > - Registering aging event when the testpmd application start, add new
> >   command to control if the event expose to the applications. If it's be
> >   set, when new flow be checked age out, there will be one-line output log.
> > - Add new command to list all aged flows, meanwhile, we can set
> parameter
> >   to destroy it.
> >
> > Signed-off-by: Bill Zhou <dongz@mellanox.com>
> > ---
> > v2: Update the way of registering aging event, add new command to
> > control if the event need be print or not. Update the output of the
> > delete aged flow command format.
> 
> <...>
> 
> > @@ -19387,6 +19394,44 @@ cmdline_parse_inst_t cmd_showport_macs =
> {
> >  	},
> >  };
> >
> > +/* Enable/Disable flow aging event log */ struct
> > +cmd_set_aged_flow_event_log_result {
> > +	cmdline_fixed_string_t set;
> > +	cmdline_fixed_string_t keyword;
> > +	cmdline_fixed_string_t enable;
> > +};
> > +cmdline_parse_token_string_t cmd_set_aged_flow_event_log_set =
> > +	TOKEN_STRING_INITIALIZER(struct
> cmd_set_aged_flow_event_log_result,
> > +		set, "set");
> > +cmdline_parse_token_string_t cmd_set_aged_flow_event_log_keyword
> =
> > +	TOKEN_STRING_INITIALIZER(struct
> cmd_set_aged_flow_event_log_result,
> > +		keyword, "aged_flow_event_log");
> > +cmdline_parse_token_string_t cmd_set_aged_flow_event_log_enable =
> > +	TOKEN_STRING_INITIALIZER(struct
> cmd_set_aged_flow_event_log_result,
> > +		enable, "on#off");
> > +
> > +static void
> > +cmd_set_aged_flow_event_log_parsed(void *parsed_result,
> > +				__rte_unused struct cmdline *cl,
> > +				__rte_unused void *data)
> > +{
> > +	struct cmd_set_aged_flow_event_log_result *res = parsed_result;
> > +	int enable = (strcmp(res->enable, "on") == 0) ? 1 : 0;
> > +	update_aging_event_log_status(enable);
> > +}
> > +
> > +cmdline_parse_inst_t cmd_set_aged_flow_event_log = {
> > +	.f = cmd_set_aged_flow_event_log_parsed,
> > +	.data = NULL,
> > +	.help_str = "set aged_flow_event_log on|off",
> 
> What do you think "set aged_flow_verbose on|off" to be more similar to
> existing verbose command?

Please see my comments below regard it.

> <...>
> 
> > --- a/app/test-pmd/testpmd.c
> > +++ b/app/test-pmd/testpmd.c
> > @@ -3068,6 +3068,21 @@ rmv_port_callback(void *arg)
> >  		start_packet_forwarding(0);
> >  }
> >
> > +static int aged_flow_event_enable;
> 
> Other global config values are at the beginning of the file, with a comment on
> them, can you do same with variable?

+1

> > +void update_aging_event_log_status(int enable) {
> > +	aged_flow_event_enable = enable;
> > +}
> > +
> > +int aging_event_output(uint16_t portid)
> 
> This can be a 'static' function.

+1

> > +{
> > +	if (aged_flow_event_enable) {
> > +		printf("port %u RTE_ETH_EVENT_FLOW_AGED triggered\n",
> portid);
> > +		fflush(stdout);
> > +	}
> > +	return 0;
> > +}
> > +
> >  /* This function is used by the interrupt thread */  static int
> > eth_event_callback(portid_t port_id, enum rte_eth_event_type type,
> > void *param, @@ -3098,6 +3113,8 @@ eth_event_callback(portid_t
> port_id, enum rte_eth_event_type type, void *param,
> >  				rmv_port_callback, (void
> *)(intptr_t)port_id))
> >  			fprintf(stderr, "Could not set up deferred device
> removal\n");
> >  		break;
> > +	case RTE_ETH_EVENT_FLOW_AGED:
> > +		aging_event_output(port_id);
> 
> Can't this provide more information than port_id, like flow id etc, what
> event_process function provides? can we print it too?
> And what is the intended usage in real application, when flow aged event
> occurred, should application destroy the flow? So it should know the flow,
> right?

Probably Yes Ferruh, I will add details, maybe it will be clearer:

As described well in rte_flow_get_aged_flows API description, there are 2 suggested options for the application:

1. To call rte_flow_get_aged_flows from the AGED event callback in order to get the aged flow contexts of the triggered port.
2. To call rte_flow_get_aged_flows in any time application wants.

It is probably depends in the way the application wants to synchronize flow APIs calls.

The application just gets the information of the aged-out flows context from the above API and can do any flow operation for it, and yes, the most expected case is to destroy the flows.

Bill added 2 testpmd commands:

1. To use rte_flow_get_aged_flows and to print a list of all the aged-out flows (with option to destroy them directly by the command).
2. A Boolean to indicate the application whether to notify the testpmd user about new aged-out flows(by print).

By this way, the testpmd user can use the 2 approaches with minimum testpmd flow management change.

So, the Boolean var is more like "trigger testpmd user notification", not like regular verbose options.

Matan




> 
> <...>
> 
> > --- a/doc/guides/testpmd_app_ug/testpmd_funcs.rst
> > +++ b/doc/guides/testpmd_app_ug/testpmd_funcs.rst
> > @@ -1062,6 +1062,21 @@ Where:
> >
> >     Check the NIC Datasheet for hardware limits.
> >
> > +aged flow event log set
> > +~~~~~~~~~~~~~~~~~~~~~~~
> > +
> > +Currently, the flow aging event is registered when the testpmd
> > +application start, if new flow be checked age out, there will be one
> > +output log for it. But some applications do not interest this event, use this
> command can set if the event expose to the applications::
> > +
> > +   testpmd> set aged_flow_event_log (on|off)
> > +
> > +For examine::
> > +
> > +   testpmd> set aged_flow_event_log on
> > +   testpmd> port 0 RTE_ETH_EVENT_FLOW_AGED triggered
> > +   testpmd> set aged_flow_event_log off
> > +
> 
> This can be moved below the "set verbose" command since they are in
> similar group.

Re: [dpdk-dev] [PATCH v2] app/testpmd: support flow aging

[Ferruh Yigit]

On 5/1/2020 7:51 AM, Matan Azrad wrote:
> Hi Ferruh
> 
> From: Ferruh Yigit
>> On 4/30/2020 4:53 PM, Bill Zhou wrote:
>>> Currently, there is no way to check the aging event or to get the
>>> current aged flows in testpmd, this patch include those implements, it's
>> included:
>>> - Registering aging event when the testpmd application start, add new
>>>   command to control if the event expose to the applications. If it's be
>>>   set, when new flow be checked age out, there will be one-line output log.
>>> - Add new command to list all aged flows, meanwhile, we can set
>> parameter
>>>   to destroy it.
>>>
>>> Signed-off-by: Bill Zhou <dongz@mellanox.com>
>>> ---
>>> v2: Update the way of registering aging event, add new command to
>>> control if the event need be print or not. Update the output of the
>>> delete aged flow command format.
>>
>> <...>
>>
>>> @@ -19387,6 +19394,44 @@ cmdline_parse_inst_t cmd_showport_macs =
>> {
>>>  	},
>>>  };
>>>
>>> +/* Enable/Disable flow aging event log */ struct
>>> +cmd_set_aged_flow_event_log_result {
>>> +	cmdline_fixed_string_t set;
>>> +	cmdline_fixed_string_t keyword;
>>> +	cmdline_fixed_string_t enable;
>>> +};
>>> +cmdline_parse_token_string_t cmd_set_aged_flow_event_log_set =
>>> +	TOKEN_STRING_INITIALIZER(struct
>> cmd_set_aged_flow_event_log_result,
>>> +		set, "set");
>>> +cmdline_parse_token_string_t cmd_set_aged_flow_event_log_keyword
>> =
>>> +	TOKEN_STRING_INITIALIZER(struct
>> cmd_set_aged_flow_event_log_result,
>>> +		keyword, "aged_flow_event_log");
>>> +cmdline_parse_token_string_t cmd_set_aged_flow_event_log_enable =
>>> +	TOKEN_STRING_INITIALIZER(struct
>> cmd_set_aged_flow_event_log_result,
>>> +		enable, "on#off");
>>> +
>>> +static void
>>> +cmd_set_aged_flow_event_log_parsed(void *parsed_result,
>>> +				__rte_unused struct cmdline *cl,
>>> +				__rte_unused void *data)
>>> +{
>>> +	struct cmd_set_aged_flow_event_log_result *res = parsed_result;
>>> +	int enable = (strcmp(res->enable, "on") == 0) ? 1 : 0;
>>> +	update_aging_event_log_status(enable);
>>> +}
>>> +
>>> +cmdline_parse_inst_t cmd_set_aged_flow_event_log = {
>>> +	.f = cmd_set_aged_flow_event_log_parsed,
>>> +	.data = NULL,
>>> +	.help_str = "set aged_flow_event_log on|off",
>>
>> What do you think "set aged_flow_verbose on|off" to be more similar to
>> existing verbose command?
> 
> Please see my comments below regard it.
> 
>> <...>
>>
>>> --- a/app/test-pmd/testpmd.c
>>> +++ b/app/test-pmd/testpmd.c
>>> @@ -3068,6 +3068,21 @@ rmv_port_callback(void *arg)
>>>  		start_packet_forwarding(0);
>>>  }
>>>
>>> +static int aged_flow_event_enable;
>>
>> Other global config values are at the beginning of the file, with a comment on
>> them, can you do same with variable?
> 
> +1
> 
>>> +void update_aging_event_log_status(int enable) {
>>> +	aged_flow_event_enable = enable;
>>> +}
>>> +
>>> +int aging_event_output(uint16_t portid)
>>
>> This can be a 'static' function.
> 
> +1
> 
>>> +{
>>> +	if (aged_flow_event_enable) {
>>> +		printf("port %u RTE_ETH_EVENT_FLOW_AGED triggered\n",
>> portid);
>>> +		fflush(stdout);
>>> +	}
>>> +	return 0;
>>> +}
>>> +
>>>  /* This function is used by the interrupt thread */  static int
>>> eth_event_callback(portid_t port_id, enum rte_eth_event_type type,
>>> void *param, @@ -3098,6 +3113,8 @@ eth_event_callback(portid_t
>> port_id, enum rte_eth_event_type type, void *param,
>>>  				rmv_port_callback, (void
>> *)(intptr_t)port_id))
>>>  			fprintf(stderr, "Could not set up deferred device
>> removal\n");
>>>  		break;
>>> +	case RTE_ETH_EVENT_FLOW_AGED:
>>> +		aging_event_output(port_id);
>>
>> Can't this provide more information than port_id, like flow id etc, what
>> event_process function provides? can we print it too?
>> And what is the intended usage in real application, when flow aged event
>> occurred, should application destroy the flow? So it should know the flow,
>> right?
> 
> Probably Yes Ferruh, I will add details, maybe it will be clearer:
> 
> As described well in rte_flow_get_aged_flows API description, there are 2 suggested options for the application:
> 
> 1. To call rte_flow_get_aged_flows from the AGED event callback in order to get the aged flow contexts of the triggered port.
> 2. To call rte_flow_get_aged_flows in any time application wants.

I see, for the testpmd implementation what do you think getting the aged flow
list and print them in event callback, this can be good as sample of the
intended usage?

> 
> It is probably depends in the way the application wants to synchronize flow APIs calls.
> 
> The application just gets the information of the aged-out flows context from the above API and can do any flow operation for it, and yes, the most expected case is to destroy the flows.
> 
> Bill added 2 testpmd commands:
> 
> 1. To use rte_flow_get_aged_flows and to print a list of all the aged-out flows (with option to destroy them directly by the command).
> 2. A Boolean to indicate the application whether to notify the testpmd user about new aged-out flows(by print).
> 
> By this way, the testpmd user can use the 2 approaches with minimum testpmd flow management change.
> 
> So, the Boolean var is more like "trigger testpmd user notification", not like regular verbose options.

As you already know, event always registered and callback always called, this
command only defines to print a log in the callback or not, so I believe
'verbose' suits here,
main concern is there are many testpmd commands and it is hard to remember them
(usage is not intuitive, I had need to check source code to find a command many
times), trying to make them as consistent as possible to help usage.

But I think as see your concern, "set aged_flow_verbose on|off" command can be
confused as if changing "flow aged #" command verbosity level.

What do you think about a more generic "set event_verbose on|off" command, which
can control to logging for all event handlers, but right now only for aged events?

> 
> Matan
> 
> 
> 
> 
>>
>> <...>
>>
>>> --- a/doc/guides/testpmd_app_ug/testpmd_funcs.rst
>>> +++ b/doc/guides/testpmd_app_ug/testpmd_funcs.rst
>>> @@ -1062,6 +1062,21 @@ Where:
>>>
>>>     Check the NIC Datasheet for hardware limits.
>>>
>>> +aged flow event log set
>>> +~~~~~~~~~~~~~~~~~~~~~~~
>>> +
>>> +Currently, the flow aging event is registered when the testpmd
>>> +application start, if new flow be checked age out, there will be one
>>> +output log for it. But some applications do not interest this event, use this
>> command can set if the event expose to the applications::
>>> +
>>> +   testpmd> set aged_flow_event_log (on|off)
>>> +
>>> +For examine::
>>> +
>>> +   testpmd> set aged_flow_event_log on
>>> +   testpmd> port 0 RTE_ETH_EVENT_FLOW_AGED triggered
>>> +   testpmd> set aged_flow_event_log off
>>> +
>>
>> This can be moved below the "set verbose" command since they are in
>> similar group.

Re: [dpdk-dev] [PATCH v2] app/testpmd: support flow aging

[Matan Azrad]

Hi Ferruh

From: Ferruh Yigit:
> On 5/1/2020 7:51 AM, Matan Azrad wrote:
> > Hi Ferruh
> >
> > From: Ferruh Yigit
> >> On 4/30/2020 4:53 PM, Bill Zhou wrote:
> >>> Currently, there is no way to check the aging event or to get the
> >>> current aged flows in testpmd, this patch include those implements,
> >>> it's
> >> included:
> >>> - Registering aging event when the testpmd application start, add new
> >>>   command to control if the event expose to the applications. If it's be
> >>>   set, when new flow be checked age out, there will be one-line output
> log.
> >>> - Add new command to list all aged flows, meanwhile, we can set
> >> parameter
> >>>   to destroy it.
> >>>
> >>> Signed-off-by: Bill Zhou <dongz@mellanox.com>
> >>> ---
> >>> v2: Update the way of registering aging event, add new command to
> >>> control if the event need be print or not. Update the output of the
> >>> delete aged flow command format.
> >>
> >> <...>
> >>
> >>> @@ -19387,6 +19394,44 @@ cmdline_parse_inst_t
> cmd_showport_macs =
> >> {
> >>>  	},
> >>>  };
> >>>
> >>> +/* Enable/Disable flow aging event log */ struct
> >>> +cmd_set_aged_flow_event_log_result {
> >>> +	cmdline_fixed_string_t set;
> >>> +	cmdline_fixed_string_t keyword;
> >>> +	cmdline_fixed_string_t enable;
> >>> +};
> >>> +cmdline_parse_token_string_t cmd_set_aged_flow_event_log_set =
> >>> +	TOKEN_STRING_INITIALIZER(struct
> >> cmd_set_aged_flow_event_log_result,
> >>> +		set, "set");
> >>> +cmdline_parse_token_string_t
> cmd_set_aged_flow_event_log_keyword
> >> =
> >>> +	TOKEN_STRING_INITIALIZER(struct
> >> cmd_set_aged_flow_event_log_result,
> >>> +		keyword, "aged_flow_event_log");
> >>> +cmdline_parse_token_string_t
> cmd_set_aged_flow_event_log_enable =
> >>> +	TOKEN_STRING_INITIALIZER(struct
> >> cmd_set_aged_flow_event_log_result,
> >>> +		enable, "on#off");
> >>> +
> >>> +static void
> >>> +cmd_set_aged_flow_event_log_parsed(void *parsed_result,
> >>> +				__rte_unused struct cmdline *cl,
> >>> +				__rte_unused void *data)
> >>> +{
> >>> +	struct cmd_set_aged_flow_event_log_result *res = parsed_result;
> >>> +	int enable = (strcmp(res->enable, "on") == 0) ? 1 : 0;
> >>> +	update_aging_event_log_status(enable);
> >>> +}
> >>> +
> >>> +cmdline_parse_inst_t cmd_set_aged_flow_event_log = {
> >>> +	.f = cmd_set_aged_flow_event_log_parsed,
> >>> +	.data = NULL,
> >>> +	.help_str = "set aged_flow_event_log on|off",
> >>
> >> What do you think "set aged_flow_verbose on|off" to be more similar
> >> to existing verbose command?
> >
> > Please see my comments below regard it.
> >
> >> <...>
> >>
> >>> --- a/app/test-pmd/testpmd.c
> >>> +++ b/app/test-pmd/testpmd.c
> >>> @@ -3068,6 +3068,21 @@ rmv_port_callback(void *arg)
> >>>  		start_packet_forwarding(0);
> >>>  }
> >>>
> >>> +static int aged_flow_event_enable;
> >>
> >> Other global config values are at the beginning of the file, with a
> >> comment on them, can you do same with variable?
> >
> > +1
> >
> >>> +void update_aging_event_log_status(int enable) {
> >>> +	aged_flow_event_enable = enable;
> >>> +}
> >>> +
> >>> +int aging_event_output(uint16_t portid)
> >>
> >> This can be a 'static' function.
> >
> > +1
> >
> >>> +{
> >>> +	if (aged_flow_event_enable) {
> >>> +		printf("port %u RTE_ETH_EVENT_FLOW_AGED triggered\n",
> >> portid);
> >>> +		fflush(stdout);
> >>> +	}
> >>> +	return 0;
> >>> +}
> >>> +
> >>>  /* This function is used by the interrupt thread */  static int
> >>> eth_event_callback(portid_t port_id, enum rte_eth_event_type type,
> >>> void *param, @@ -3098,6 +3113,8 @@ eth_event_callback(portid_t
> >> port_id, enum rte_eth_event_type type, void *param,
> >>>  				rmv_port_callback, (void
> >> *)(intptr_t)port_id))
> >>>  			fprintf(stderr, "Could not set up deferred device
> >> removal\n");
> >>>  		break;
> >>> +	case RTE_ETH_EVENT_FLOW_AGED:
> >>> +		aging_event_output(port_id);
> >>
> >> Can't this provide more information than port_id, like flow id etc,
> >> what event_process function provides? can we print it too?
> >> And what is the intended usage in real application, when flow aged
> >> event occurred, should application destroy the flow? So it should
> >> know the flow, right?
> >
> > Probably Yes Ferruh, I will add details, maybe it will be clearer:
> >
> > As described well in rte_flow_get_aged_flows API description, there are 2
> suggested options for the application:
> >
> > 1. To call rte_flow_get_aged_flows from the AGED event callback in order
> to get the aged flow contexts of the triggered port.
> > 2. To call rte_flow_get_aged_flows in any time application wants.
> 
> I see, for the testpmd implementation what do you think getting the aged
> flow list and print them in event callback, this can be good as sample of the
> intended usage?

Yes, we thought on it and I understand you,
The issue with it is that we need to synchronize all the flow management in Testpmd and to protect any flow operation with a lock.
Because the callback is probably coming from the host thread while other flow operations from different Testpmd thread(command line thread).

It will do the patch very intrusive.

The current approach(like the second suggestion) moves the synchronization to the testpmd user to access flows only from the command line thread while hinting to the user when a port holds some aged-out flows.
I think it is better to stay the implementation simple.

> >
> > It is probably depends in the way the application wants to synchronize flow
> APIs calls.
> >
> > The application just gets the information of the aged-out flows context
> from the above API and can do any flow operation for it, and yes, the most
> expected case is to destroy the flows.
> >
> > Bill added 2 testpmd commands:
> >
> > 1. To use rte_flow_get_aged_flows and to print a list of all the aged-out
> flows (with option to destroy them directly by the command).
> > 2. A Boolean to indicate the application whether to notify the testpmd user
> about new aged-out flows(by print).
> >
> > By this way, the testpmd user can use the 2 approaches with minimum
> testpmd flow management change.
> >
> > So, the Boolean var is more like "trigger testpmd user notification", not like
> regular verbose options.
> 
> As you already know, event always registered and callback always called, this
> command only defines to print a log in the callback or not, so I believe
> 'verbose' suits here, main concern is there are many testpmd commands and
> it is hard to remember them (usage is not intuitive, I had need to check
> source code to find a command many times), trying to make them as
> consistent as possible to help usage.
> 
> But I think as see your concern, "set aged_flow_verbose on|off" command
> can be confused as if changing "flow aged #" command verbosity level.
> 
> What do you think about a more generic "set event_verbose on|off"
> command, which can control to logging for all event handlers, but right now
> only for aged events?

Looks good to me, but maybe instead of on | off it is better to use bitmap in order to indicate the event?

 
> >
> > Matan
> >
> >
> >
> >
> >>
> >> <...>
> >>
> >>> --- a/doc/guides/testpmd_app_ug/testpmd_funcs.rst
> >>> +++ b/doc/guides/testpmd_app_ug/testpmd_funcs.rst
> >>> @@ -1062,6 +1062,21 @@ Where:
> >>>
> >>>     Check the NIC Datasheet for hardware limits.
> >>>
> >>> +aged flow event log set
> >>> +~~~~~~~~~~~~~~~~~~~~~~~
> >>> +
> >>> +Currently, the flow aging event is registered when the testpmd
> >>> +application start, if new flow be checked age out, there will be
> >>> +one output log for it. But some applications do not interest this
> >>> +event, use this
> >> command can set if the event expose to the applications::
> >>> +
> >>> +   testpmd> set aged_flow_event_log (on|off)
> >>> +
> >>> +For examine::
> >>> +
> >>> +   testpmd> set aged_flow_event_log on
> >>> +   testpmd> port 0 RTE_ETH_EVENT_FLOW_AGED triggered
> >>> +   testpmd> set aged_flow_event_log off
> >>> +
> >>
> >> This can be moved below the "set verbose" command since they are in
> >> similar group.

Re: [dpdk-dev] [PATCH v2] app/testpmd: support flow aging

[Ferruh Yigit]

On 5/1/2020 12:28 PM, Matan Azrad wrote:
> 
> Hi Ferruh
> 
> From: Ferruh Yigit:
>> On 5/1/2020 7:51 AM, Matan Azrad wrote:
>>> Hi Ferruh
>>>
>>> From: Ferruh Yigit
>>>> On 4/30/2020 4:53 PM, Bill Zhou wrote:
>>>>> Currently, there is no way to check the aging event or to get the
>>>>> current aged flows in testpmd, this patch include those implements,
>>>>> it's
>>>> included:
>>>>> - Registering aging event when the testpmd application start, add new
>>>>>   command to control if the event expose to the applications. If it's be
>>>>>   set, when new flow be checked age out, there will be one-line output
>> log.
>>>>> - Add new command to list all aged flows, meanwhile, we can set
>>>> parameter
>>>>>   to destroy it.
>>>>>
>>>>> Signed-off-by: Bill Zhou <dongz@mellanox.com>
>>>>> ---
>>>>> v2: Update the way of registering aging event, add new command to
>>>>> control if the event need be print or not. Update the output of the
>>>>> delete aged flow command format.
>>>>
>>>> <...>
>>>>
>>>>> @@ -19387,6 +19394,44 @@ cmdline_parse_inst_t
>> cmd_showport_macs =
>>>> {
>>>>>  	},
>>>>>  };
>>>>>
>>>>> +/* Enable/Disable flow aging event log */ struct
>>>>> +cmd_set_aged_flow_event_log_result {
>>>>> +	cmdline_fixed_string_t set;
>>>>> +	cmdline_fixed_string_t keyword;
>>>>> +	cmdline_fixed_string_t enable;
>>>>> +};
>>>>> +cmdline_parse_token_string_t cmd_set_aged_flow_event_log_set =
>>>>> +	TOKEN_STRING_INITIALIZER(struct
>>>> cmd_set_aged_flow_event_log_result,
>>>>> +		set, "set");
>>>>> +cmdline_parse_token_string_t
>> cmd_set_aged_flow_event_log_keyword
>>>> =
>>>>> +	TOKEN_STRING_INITIALIZER(struct
>>>> cmd_set_aged_flow_event_log_result,
>>>>> +		keyword, "aged_flow_event_log");
>>>>> +cmdline_parse_token_string_t
>> cmd_set_aged_flow_event_log_enable =
>>>>> +	TOKEN_STRING_INITIALIZER(struct
>>>> cmd_set_aged_flow_event_log_result,
>>>>> +		enable, "on#off");
>>>>> +
>>>>> +static void
>>>>> +cmd_set_aged_flow_event_log_parsed(void *parsed_result,
>>>>> +				__rte_unused struct cmdline *cl,
>>>>> +				__rte_unused void *data)
>>>>> +{
>>>>> +	struct cmd_set_aged_flow_event_log_result *res = parsed_result;
>>>>> +	int enable = (strcmp(res->enable, "on") == 0) ? 1 : 0;
>>>>> +	update_aging_event_log_status(enable);
>>>>> +}
>>>>> +
>>>>> +cmdline_parse_inst_t cmd_set_aged_flow_event_log = {
>>>>> +	.f = cmd_set_aged_flow_event_log_parsed,
>>>>> +	.data = NULL,
>>>>> +	.help_str = "set aged_flow_event_log on|off",
>>>>
>>>> What do you think "set aged_flow_verbose on|off" to be more similar
>>>> to existing verbose command?
>>>
>>> Please see my comments below regard it.
>>>
>>>> <...>
>>>>
>>>>> --- a/app/test-pmd/testpmd.c
>>>>> +++ b/app/test-pmd/testpmd.c
>>>>> @@ -3068,6 +3068,21 @@ rmv_port_callback(void *arg)
>>>>>  		start_packet_forwarding(0);
>>>>>  }
>>>>>
>>>>> +static int aged_flow_event_enable;
>>>>
>>>> Other global config values are at the beginning of the file, with a
>>>> comment on them, can you do same with variable?
>>>
>>> +1
>>>
>>>>> +void update_aging_event_log_status(int enable) {
>>>>> +	aged_flow_event_enable = enable;
>>>>> +}
>>>>> +
>>>>> +int aging_event_output(uint16_t portid)
>>>>
>>>> This can be a 'static' function.
>>>
>>> +1
>>>
>>>>> +{
>>>>> +	if (aged_flow_event_enable) {
>>>>> +		printf("port %u RTE_ETH_EVENT_FLOW_AGED triggered\n",
>>>> portid);
>>>>> +		fflush(stdout);
>>>>> +	}
>>>>> +	return 0;
>>>>> +}
>>>>> +
>>>>>  /* This function is used by the interrupt thread */  static int
>>>>> eth_event_callback(portid_t port_id, enum rte_eth_event_type type,
>>>>> void *param, @@ -3098,6 +3113,8 @@ eth_event_callback(portid_t
>>>> port_id, enum rte_eth_event_type type, void *param,
>>>>>  				rmv_port_callback, (void
>>>> *)(intptr_t)port_id))
>>>>>  			fprintf(stderr, "Could not set up deferred device
>>>> removal\n");
>>>>>  		break;
>>>>> +	case RTE_ETH_EVENT_FLOW_AGED:
>>>>> +		aging_event_output(port_id);
>>>>
>>>> Can't this provide more information than port_id, like flow id etc,
>>>> what event_process function provides? can we print it too?
>>>> And what is the intended usage in real application, when flow aged
>>>> event occurred, should application destroy the flow? So it should
>>>> know the flow, right?
>>>
>>> Probably Yes Ferruh, I will add details, maybe it will be clearer:
>>>
>>> As described well in rte_flow_get_aged_flows API description, there are 2
>> suggested options for the application:
>>>
>>> 1. To call rte_flow_get_aged_flows from the AGED event callback in order
>> to get the aged flow contexts of the triggered port.
>>> 2. To call rte_flow_get_aged_flows in any time application wants.
>>
>> I see, for the testpmd implementation what do you think getting the aged
>> flow list and print them in event callback, this can be good as sample of the
>> intended usage?
> 
> Yes, we thought on it and I understand you,
> The issue with it is that we need to synchronize all the flow management in Testpmd and to protect any flow operation with a lock.
> Because the callback is probably coming from the host thread while other flow operations from different Testpmd thread(command line thread).
> 
> It will do the patch very intrusive.
> 
> The current approach(like the second suggestion) moves the synchronization to the testpmd user to access flows only from the command line thread while hinting to the user when a port holds some aged-out flows.
> I think it is better to stay the implementation simple.

OK

> 
>>>
>>> It is probably depends in the way the application wants to synchronize flow
>> APIs calls.
>>>
>>> The application just gets the information of the aged-out flows context
>> from the above API and can do any flow operation for it, and yes, the most
>> expected case is to destroy the flows.
>>>
>>> Bill added 2 testpmd commands:
>>>
>>> 1. To use rte_flow_get_aged_flows and to print a list of all the aged-out
>> flows (with option to destroy them directly by the command).
>>> 2. A Boolean to indicate the application whether to notify the testpmd user
>> about new aged-out flows(by print).
>>>
>>> By this way, the testpmd user can use the 2 approaches with minimum
>> testpmd flow management change.
>>>
>>> So, the Boolean var is more like "trigger testpmd user notification", not like
>> regular verbose options.
>>
>> As you already know, event always registered and callback always called, this
>> command only defines to print a log in the callback or not, so I believe
>> 'verbose' suits here, main concern is there are many testpmd commands and
>> it is hard to remember them (usage is not intuitive, I had need to check
>> source code to find a command many times), trying to make them as
>> consistent as possible to help usage.
>>
>> But I think as see your concern, "set aged_flow_verbose on|off" command
>> can be confused as if changing "flow aged #" command verbosity level.
>>
>> What do you think about a more generic "set event_verbose on|off"
>> command, which can control to logging for all event handlers, but right now
>> only for aged events?
> 
> Looks good to me, but maybe instead of on | off it is better to use bitmap in order to indicate the event?

No objection but not sure that level fine grain needed now, or later, why not
add the basic on/off now and add the bitmap when we need to control for each event.

> 
>  
>>>
>>> Matan
>>>
>>>
>>>
>>>
>>>>
>>>> <...>
>>>>
>>>>> --- a/doc/guides/testpmd_app_ug/testpmd_funcs.rst
>>>>> +++ b/doc/guides/testpmd_app_ug/testpmd_funcs.rst
>>>>> @@ -1062,6 +1062,21 @@ Where:
>>>>>
>>>>>     Check the NIC Datasheet for hardware limits.
>>>>>
>>>>> +aged flow event log set
>>>>> +~~~~~~~~~~~~~~~~~~~~~~~
>>>>> +
>>>>> +Currently, the flow aging event is registered when the testpmd
>>>>> +application start, if new flow be checked age out, there will be
>>>>> +one output log for it. But some applications do not interest this
>>>>> +event, use this
>>>> command can set if the event expose to the applications::
>>>>> +
>>>>> +   testpmd> set aged_flow_event_log (on|off)
>>>>> +
>>>>> +For examine::
>>>>> +
>>>>> +   testpmd> set aged_flow_event_log on
>>>>> +   testpmd> port 0 RTE_ETH_EVENT_FLOW_AGED triggered
>>>>> +   testpmd> set aged_flow_event_log off
>>>>> +
>>>>
>>>> This can be moved below the "set verbose" command since they are in
>>>> similar group.
> 

Re: [dpdk-dev] [PATCH v2] app/testpmd: support flow aging

[Matan Azrad]

From: Ferruh Yigit:
> On 5/1/2020 12:28 PM, Matan Azrad wrote:
> >
> > Hi Ferruh
> >
> > From: Ferruh Yigit:
> >> On 5/1/2020 7:51 AM, Matan Azrad wrote:
> >>> Hi Ferruh
> >>>
> >>> From: Ferruh Yigit
> >>>> On 4/30/2020 4:53 PM, Bill Zhou wrote:
> >>>>> Currently, there is no way to check the aging event or to get the
> >>>>> current aged flows in testpmd, this patch include those
> >>>>> implements, it's
> >>>> included:
> >>>>> - Registering aging event when the testpmd application start, add new
> >>>>>   command to control if the event expose to the applications. If it's be
> >>>>>   set, when new flow be checked age out, there will be one-line
> >>>>> output
> >> log.
> >>>>> - Add new command to list all aged flows, meanwhile, we can set
> >>>> parameter
> >>>>>   to destroy it.
> >>>>>
> >>>>> Signed-off-by: Bill Zhou <dongz@mellanox.com>
> >>>>> ---
> >>>>> v2: Update the way of registering aging event, add new command to
> >>>>> control if the event need be print or not. Update the output of
> >>>>> the delete aged flow command format.
> >>>>
> >>>> <...>
> >>>>
> >>>>> @@ -19387,6 +19394,44 @@ cmdline_parse_inst_t
> >> cmd_showport_macs =
> >>>> {
> >>>>>  	},
> >>>>>  };
> >>>>>
> >>>>> +/* Enable/Disable flow aging event log */ struct
> >>>>> +cmd_set_aged_flow_event_log_result {
> >>>>> +	cmdline_fixed_string_t set;
> >>>>> +	cmdline_fixed_string_t keyword;
> >>>>> +	cmdline_fixed_string_t enable;
> >>>>> +};
> >>>>> +cmdline_parse_token_string_t cmd_set_aged_flow_event_log_set
> =
> >>>>> +	TOKEN_STRING_INITIALIZER(struct
> >>>> cmd_set_aged_flow_event_log_result,
> >>>>> +		set, "set");
> >>>>> +cmdline_parse_token_string_t
> >> cmd_set_aged_flow_event_log_keyword
> >>>> =
> >>>>> +	TOKEN_STRING_INITIALIZER(struct
> >>>> cmd_set_aged_flow_event_log_result,
> >>>>> +		keyword, "aged_flow_event_log");
> cmdline_parse_token_string_t
> >> cmd_set_aged_flow_event_log_enable =
> >>>>> +	TOKEN_STRING_INITIALIZER(struct
> >>>> cmd_set_aged_flow_event_log_result,
> >>>>> +		enable, "on#off");
> >>>>> +
> >>>>> +static void
> >>>>> +cmd_set_aged_flow_event_log_parsed(void *parsed_result,
> >>>>> +				__rte_unused struct cmdline *cl,
> >>>>> +				__rte_unused void *data)
> >>>>> +{
> >>>>> +	struct cmd_set_aged_flow_event_log_result *res =
> parsed_result;
> >>>>> +	int enable = (strcmp(res->enable, "on") == 0) ? 1 : 0;
> >>>>> +	update_aging_event_log_status(enable);
> >>>>> +}
> >>>>> +
> >>>>> +cmdline_parse_inst_t cmd_set_aged_flow_event_log = {
> >>>>> +	.f = cmd_set_aged_flow_event_log_parsed,
> >>>>> +	.data = NULL,
> >>>>> +	.help_str = "set aged_flow_event_log on|off",
> >>>>
> >>>> What do you think "set aged_flow_verbose on|off" to be more similar
> >>>> to existing verbose command?
> >>>
> >>> Please see my comments below regard it.
> >>>
> >>>> <...>
> >>>>
> >>>>> --- a/app/test-pmd/testpmd.c
> >>>>> +++ b/app/test-pmd/testpmd.c
> >>>>> @@ -3068,6 +3068,21 @@ rmv_port_callback(void *arg)
> >>>>>  		start_packet_forwarding(0);
> >>>>>  }
> >>>>>
> >>>>> +static int aged_flow_event_enable;
> >>>>
> >>>> Other global config values are at the beginning of the file, with a
> >>>> comment on them, can you do same with variable?
> >>>
> >>> +1
> >>>
> >>>>> +void update_aging_event_log_status(int enable) {
> >>>>> +	aged_flow_event_enable = enable; }
> >>>>> +
> >>>>> +int aging_event_output(uint16_t portid)
> >>>>
> >>>> This can be a 'static' function.
> >>>
> >>> +1
> >>>
> >>>>> +{
> >>>>> +	if (aged_flow_event_enable) {
> >>>>> +		printf("port %u RTE_ETH_EVENT_FLOW_AGED
> triggered\n",
> >>>> portid);
> >>>>> +		fflush(stdout);
> >>>>> +	}
> >>>>> +	return 0;
> >>>>> +}
> >>>>> +
> >>>>>  /* This function is used by the interrupt thread */  static int
> >>>>> eth_event_callback(portid_t port_id, enum rte_eth_event_type
> type,
> >>>>> void *param, @@ -3098,6 +3113,8 @@ eth_event_callback(portid_t
> >>>> port_id, enum rte_eth_event_type type, void *param,
> >>>>>  				rmv_port_callback, (void
> >>>> *)(intptr_t)port_id))
> >>>>>  			fprintf(stderr, "Could not set up deferred device
> >>>> removal\n");
> >>>>>  		break;
> >>>>> +	case RTE_ETH_EVENT_FLOW_AGED:
> >>>>> +		aging_event_output(port_id);
> >>>>
> >>>> Can't this provide more information than port_id, like flow id etc,
> >>>> what event_process function provides? can we print it too?
> >>>> And what is the intended usage in real application, when flow aged
> >>>> event occurred, should application destroy the flow? So it should
> >>>> know the flow, right?
> >>>
> >>> Probably Yes Ferruh, I will add details, maybe it will be clearer:
> >>>
> >>> As described well in rte_flow_get_aged_flows API description, there
> >>> are 2
> >> suggested options for the application:
> >>>
> >>> 1. To call rte_flow_get_aged_flows from the AGED event callback in
> >>> order
> >> to get the aged flow contexts of the triggered port.
> >>> 2. To call rte_flow_get_aged_flows in any time application wants.
> >>
> >> I see, for the testpmd implementation what do you think getting the
> >> aged flow list and print them in event callback, this can be good as
> >> sample of the intended usage?
> >
> > Yes, we thought on it and I understand you, The issue with it is that
> > we need to synchronize all the flow management in Testpmd and to
> protect any flow operation with a lock.
> > Because the callback is probably coming from the host thread while other
> flow operations from different Testpmd thread(command line thread).
> >
> > It will do the patch very intrusive.
> >
> > The current approach(like the second suggestion) moves the
> synchronization to the testpmd user to access flows only from the command
> line thread while hinting to the user when a port holds some aged-out flows.
> > I think it is better to stay the implementation simple.
> 
> OK
> 
> >
> >>>
> >>> It is probably depends in the way the application wants to
> >>> synchronize flow
> >> APIs calls.
> >>>
> >>> The application just gets the information of the aged-out flows
> >>> context
> >> from the above API and can do any flow operation for it, and yes, the
> >> most expected case is to destroy the flows.
> >>>
> >>> Bill added 2 testpmd commands:
> >>>
> >>> 1. To use rte_flow_get_aged_flows and to print a list of all the
> >>> aged-out
> >> flows (with option to destroy them directly by the command).
> >>> 2. A Boolean to indicate the application whether to notify the
> >>> testpmd user
> >> about new aged-out flows(by print).
> >>>
> >>> By this way, the testpmd user can use the 2 approaches with minimum
> >> testpmd flow management change.
> >>>
> >>> So, the Boolean var is more like "trigger testpmd user
> >>> notification", not like
> >> regular verbose options.
> >>
> >> As you already know, event always registered and callback always
> >> called, this command only defines to print a log in the callback or
> >> not, so I believe 'verbose' suits here, main concern is there are
> >> many testpmd commands and it is hard to remember them (usage is not
> >> intuitive, I had need to check source code to find a command many
> >> times), trying to make them as consistent as possible to help usage.
> >>
> >> But I think as see your concern, "set aged_flow_verbose on|off"
> >> command can be confused as if changing "flow aged #" command
> verbosity level.
> >>
> >> What do you think about a more generic "set event_verbose on|off"
> >> command, which can control to logging for all event handlers, but
> >> right now only for aged events?
> >
> > Looks good to me, but maybe instead of on | off it is better to use bitmap
> in order to indicate the event?
> 
> No objection but not sure that level fine grain needed now, or later, why not
> add the basic on/off now and add the bitmap when we need to control for
> each event.

It will be good to the user to reduce logs of non-interested events (maybe even more often) which makes his log bigger.


> 
> >
> >
> >>>
> >>> Matan
> >>>
> >>>
> >>>
> >>>
> >>>>
> >>>> <...>
> >>>>
> >>>>> --- a/doc/guides/testpmd_app_ug/testpmd_funcs.rst
> >>>>> +++ b/doc/guides/testpmd_app_ug/testpmd_funcs.rst
> >>>>> @@ -1062,6 +1062,21 @@ Where:
> >>>>>
> >>>>>     Check the NIC Datasheet for hardware limits.
> >>>>>
> >>>>> +aged flow event log set
> >>>>> +~~~~~~~~~~~~~~~~~~~~~~~
> >>>>> +
> >>>>> +Currently, the flow aging event is registered when the testpmd
> >>>>> +application start, if new flow be checked age out, there will be
> >>>>> +one output log for it. But some applications do not interest this
> >>>>> +event, use this
> >>>> command can set if the event expose to the applications::
> >>>>> +
> >>>>> +   testpmd> set aged_flow_event_log (on|off)
> >>>>> +
> >>>>> +For examine::
> >>>>> +
> >>>>> +   testpmd> set aged_flow_event_log on
> >>>>> +   testpmd> port 0 RTE_ETH_EVENT_FLOW_AGED triggered
> >>>>> +   testpmd> set aged_flow_event_log off
> >>>>> +
> >>>>
> >>>> This can be moved below the "set verbose" command since they are in
> >>>> similar group.
> >

Re: [dpdk-dev] [PATCH v2] app/testpmd: support flow aging

[Ferruh Yigit]

On 5/1/2020 1:45 PM, Matan Azrad wrote:
> 
> 
> From: Ferruh Yigit:
>> On 5/1/2020 12:28 PM, Matan Azrad wrote:
>>>
>>> Hi Ferruh
>>>
>>> From: Ferruh Yigit:
>>>> On 5/1/2020 7:51 AM, Matan Azrad wrote:
>>>>> Hi Ferruh
>>>>>
>>>>> From: Ferruh Yigit
>>>>>> On 4/30/2020 4:53 PM, Bill Zhou wrote:
>>>>>>> Currently, there is no way to check the aging event or to get the
>>>>>>> current aged flows in testpmd, this patch include those
>>>>>>> implements, it's
>>>>>> included:
>>>>>>> - Registering aging event when the testpmd application start, add new
>>>>>>>   command to control if the event expose to the applications. If it's be
>>>>>>>   set, when new flow be checked age out, there will be one-line
>>>>>>> output
>>>> log.
>>>>>>> - Add new command to list all aged flows, meanwhile, we can set
>>>>>> parameter
>>>>>>>   to destroy it.
>>>>>>>
>>>>>>> Signed-off-by: Bill Zhou <dongz@mellanox.com>
>>>>>>> ---
>>>>>>> v2: Update the way of registering aging event, add new command to
>>>>>>> control if the event need be print or not. Update the output of
>>>>>>> the delete aged flow command format.
>>>>>>
>>>>>> <...>
>>>>>>
>>>>>>> @@ -19387,6 +19394,44 @@ cmdline_parse_inst_t
>>>> cmd_showport_macs =
>>>>>> {
>>>>>>>  	},
>>>>>>>  };
>>>>>>>
>>>>>>> +/* Enable/Disable flow aging event log */ struct
>>>>>>> +cmd_set_aged_flow_event_log_result {
>>>>>>> +	cmdline_fixed_string_t set;
>>>>>>> +	cmdline_fixed_string_t keyword;
>>>>>>> +	cmdline_fixed_string_t enable;
>>>>>>> +};
>>>>>>> +cmdline_parse_token_string_t cmd_set_aged_flow_event_log_set
>> =
>>>>>>> +	TOKEN_STRING_INITIALIZER(struct
>>>>>> cmd_set_aged_flow_event_log_result,
>>>>>>> +		set, "set");
>>>>>>> +cmdline_parse_token_string_t
>>>> cmd_set_aged_flow_event_log_keyword
>>>>>> =
>>>>>>> +	TOKEN_STRING_INITIALIZER(struct
>>>>>> cmd_set_aged_flow_event_log_result,
>>>>>>> +		keyword, "aged_flow_event_log");
>> cmdline_parse_token_string_t
>>>> cmd_set_aged_flow_event_log_enable =
>>>>>>> +	TOKEN_STRING_INITIALIZER(struct
>>>>>> cmd_set_aged_flow_event_log_result,
>>>>>>> +		enable, "on#off");
>>>>>>> +
>>>>>>> +static void
>>>>>>> +cmd_set_aged_flow_event_log_parsed(void *parsed_result,
>>>>>>> +				__rte_unused struct cmdline *cl,
>>>>>>> +				__rte_unused void *data)
>>>>>>> +{
>>>>>>> +	struct cmd_set_aged_flow_event_log_result *res =
>> parsed_result;
>>>>>>> +	int enable = (strcmp(res->enable, "on") == 0) ? 1 : 0;
>>>>>>> +	update_aging_event_log_status(enable);
>>>>>>> +}
>>>>>>> +
>>>>>>> +cmdline_parse_inst_t cmd_set_aged_flow_event_log = {
>>>>>>> +	.f = cmd_set_aged_flow_event_log_parsed,
>>>>>>> +	.data = NULL,
>>>>>>> +	.help_str = "set aged_flow_event_log on|off",
>>>>>>
>>>>>> What do you think "set aged_flow_verbose on|off" to be more similar
>>>>>> to existing verbose command?
>>>>>
>>>>> Please see my comments below regard it.
>>>>>
>>>>>> <...>
>>>>>>
>>>>>>> --- a/app/test-pmd/testpmd.c
>>>>>>> +++ b/app/test-pmd/testpmd.c
>>>>>>> @@ -3068,6 +3068,21 @@ rmv_port_callback(void *arg)
>>>>>>>  		start_packet_forwarding(0);
>>>>>>>  }
>>>>>>>
>>>>>>> +static int aged_flow_event_enable;
>>>>>>
>>>>>> Other global config values are at the beginning of the file, with a
>>>>>> comment on them, can you do same with variable?
>>>>>
>>>>> +1
>>>>>
>>>>>>> +void update_aging_event_log_status(int enable) {
>>>>>>> +	aged_flow_event_enable = enable; }
>>>>>>> +
>>>>>>> +int aging_event_output(uint16_t portid)
>>>>>>
>>>>>> This can be a 'static' function.
>>>>>
>>>>> +1
>>>>>
>>>>>>> +{
>>>>>>> +	if (aged_flow_event_enable) {
>>>>>>> +		printf("port %u RTE_ETH_EVENT_FLOW_AGED
>> triggered\n",
>>>>>> portid);
>>>>>>> +		fflush(stdout);
>>>>>>> +	}
>>>>>>> +	return 0;
>>>>>>> +}
>>>>>>> +
>>>>>>>  /* This function is used by the interrupt thread */  static int
>>>>>>> eth_event_callback(portid_t port_id, enum rte_eth_event_type
>> type,
>>>>>>> void *param, @@ -3098,6 +3113,8 @@ eth_event_callback(portid_t
>>>>>> port_id, enum rte_eth_event_type type, void *param,
>>>>>>>  				rmv_port_callback, (void
>>>>>> *)(intptr_t)port_id))
>>>>>>>  			fprintf(stderr, "Could not set up deferred device
>>>>>> removal\n");
>>>>>>>  		break;
>>>>>>> +	case RTE_ETH_EVENT_FLOW_AGED:
>>>>>>> +		aging_event_output(port_id);
>>>>>>
>>>>>> Can't this provide more information than port_id, like flow id etc,
>>>>>> what event_process function provides? can we print it too?
>>>>>> And what is the intended usage in real application, when flow aged
>>>>>> event occurred, should application destroy the flow? So it should
>>>>>> know the flow, right?
>>>>>
>>>>> Probably Yes Ferruh, I will add details, maybe it will be clearer:
>>>>>
>>>>> As described well in rte_flow_get_aged_flows API description, there
>>>>> are 2
>>>> suggested options for the application:
>>>>>
>>>>> 1. To call rte_flow_get_aged_flows from the AGED event callback in
>>>>> order
>>>> to get the aged flow contexts of the triggered port.
>>>>> 2. To call rte_flow_get_aged_flows in any time application wants.
>>>>
>>>> I see, for the testpmd implementation what do you think getting the
>>>> aged flow list and print them in event callback, this can be good as
>>>> sample of the intended usage?
>>>
>>> Yes, we thought on it and I understand you, The issue with it is that
>>> we need to synchronize all the flow management in Testpmd and to
>> protect any flow operation with a lock.
>>> Because the callback is probably coming from the host thread while other
>> flow operations from different Testpmd thread(command line thread).
>>>
>>> It will do the patch very intrusive.
>>>
>>> The current approach(like the second suggestion) moves the
>> synchronization to the testpmd user to access flows only from the command
>> line thread while hinting to the user when a port holds some aged-out flows.
>>> I think it is better to stay the implementation simple.
>>
>> OK
>>
>>>
>>>>>
>>>>> It is probably depends in the way the application wants to
>>>>> synchronize flow
>>>> APIs calls.
>>>>>
>>>>> The application just gets the information of the aged-out flows
>>>>> context
>>>> from the above API and can do any flow operation for it, and yes, the
>>>> most expected case is to destroy the flows.
>>>>>
>>>>> Bill added 2 testpmd commands:
>>>>>
>>>>> 1. To use rte_flow_get_aged_flows and to print a list of all the
>>>>> aged-out
>>>> flows (with option to destroy them directly by the command).
>>>>> 2. A Boolean to indicate the application whether to notify the
>>>>> testpmd user
>>>> about new aged-out flows(by print).
>>>>>
>>>>> By this way, the testpmd user can use the 2 approaches with minimum
>>>> testpmd flow management change.
>>>>>
>>>>> So, the Boolean var is more like "trigger testpmd user
>>>>> notification", not like
>>>> regular verbose options.
>>>>
>>>> As you already know, event always registered and callback always
>>>> called, this command only defines to print a log in the callback or
>>>> not, so I believe 'verbose' suits here, main concern is there are
>>>> many testpmd commands and it is hard to remember them (usage is not
>>>> intuitive, I had need to check source code to find a command many
>>>> times), trying to make them as consistent as possible to help usage.
>>>>
>>>> But I think as see your concern, "set aged_flow_verbose on|off"
>>>> command can be confused as if changing "flow aged #" command
>> verbosity level.
>>>>
>>>> What do you think about a more generic "set event_verbose on|off"
>>>> command, which can control to logging for all event handlers, but
>>>> right now only for aged events?
>>>
>>> Looks good to me, but maybe instead of on | off it is better to use bitmap
>> in order to indicate the event?
>>
>> No objection but not sure that level fine grain needed now, or later, why not
>> add the basic on/off now and add the bitmap when we need to control for
>> each event.
> 
> It will be good to the user to reduce logs of non-interested events (maybe even more often) which makes his log bigger.

Right now, is there any other event that logs? And how much log do they produce?

> 
> 
>>
>>>
>>>
>>>>>
>>>>> Matan
>>>>>
>>>>>
>>>>>
>>>>>
>>>>>>
>>>>>> <...>
>>>>>>
>>>>>>> --- a/doc/guides/testpmd_app_ug/testpmd_funcs.rst
>>>>>>> +++ b/doc/guides/testpmd_app_ug/testpmd_funcs.rst
>>>>>>> @@ -1062,6 +1062,21 @@ Where:
>>>>>>>
>>>>>>>     Check the NIC Datasheet for hardware limits.
>>>>>>>
>>>>>>> +aged flow event log set
>>>>>>> +~~~~~~~~~~~~~~~~~~~~~~~
>>>>>>> +
>>>>>>> +Currently, the flow aging event is registered when the testpmd
>>>>>>> +application start, if new flow be checked age out, there will be
>>>>>>> +one output log for it. But some applications do not interest this
>>>>>>> +event, use this
>>>>>> command can set if the event expose to the applications::
>>>>>>> +
>>>>>>> +   testpmd> set aged_flow_event_log (on|off)
>>>>>>> +
>>>>>>> +For examine::
>>>>>>> +
>>>>>>> +   testpmd> set aged_flow_event_log on
>>>>>>> +   testpmd> port 0 RTE_ETH_EVENT_FLOW_AGED triggered
>>>>>>> +   testpmd> set aged_flow_event_log off
>>>>>>> +
>>>>>>
>>>>>> This can be moved below the "set verbose" command since they are in
>>>>>> similar group.
>>>
> 

Re: [dpdk-dev] [PATCH v2] app/testpmd: support flow aging

[Matan Azrad]

From: Ferruh Yigit:
> On 5/1/2020 1:45 PM, Matan Azrad wrote:
> >
> >
> > From: Ferruh Yigit:
> >> On 5/1/2020 12:28 PM, Matan Azrad wrote:
> >>>
> >>> Hi Ferruh
> >>>
> >>> From: Ferruh Yigit:
> >>>> On 5/1/2020 7:51 AM, Matan Azrad wrote:
> >>>>> Hi Ferruh
> >>>>>
> >>>>> From: Ferruh Yigit
> >>>>>> On 4/30/2020 4:53 PM, Bill Zhou wrote:
> >>>>>>> Currently, there is no way to check the aging event or to get
> >>>>>>> the current aged flows in testpmd, this patch include those
> >>>>>>> implements, it's
> >>>>>> included:
> >>>>>>> - Registering aging event when the testpmd application start, add
> new
> >>>>>>>   command to control if the event expose to the applications. If it's
> be
> >>>>>>>   set, when new flow be checked age out, there will be one-line
> >>>>>>> output
> >>>> log.
> >>>>>>> - Add new command to list all aged flows, meanwhile, we can set
> >>>>>> parameter
> >>>>>>>   to destroy it.
> >>>>>>>
> >>>>>>> Signed-off-by: Bill Zhou <dongz@mellanox.com>
> >>>>>>> ---
> >>>>>>> v2: Update the way of registering aging event, add new command
> >>>>>>> to control if the event need be print or not. Update the output
> >>>>>>> of the delete aged flow command format.
<...>
> >>>> What do you think about a more generic "set event_verbose on|off"
> >>>> command, which can control to logging for all event handlers, but
> >>>> right now only for aged events?
> >>>
> >>> Looks good to me, but maybe instead of on | off it is better to use
> >>> bitmap
> >> in order to indicate the event?
> >>
> >> No objection but not sure that level fine grain needed now, or later,
> >> why not add the basic on/off now and add the bitmap when we need to
> >> control for each event.
> >
> > It will be good to the user to reduce logs of non-interested events (maybe
> even more often) which makes his log bigger.
> 
> Right now, is there any other event that logs? And how much log do they
> produce?

I don't really familiar with all, but looks like IPSEC, VF_MBOX and AGED may make the log bigger than others.

So, if we open verbose for all, the events log bitmap may be very usable.

<....>

Re: [dpdk-dev] [PATCH v2] app/testpmd: support flow aging

[Ferruh Yigit]

On 5/1/2020 4:14 PM, Matan Azrad wrote:
> 
> 
> From: Ferruh Yigit:
>> On 5/1/2020 1:45 PM, Matan Azrad wrote:
>>>
>>>
>>> From: Ferruh Yigit:
>>>> On 5/1/2020 12:28 PM, Matan Azrad wrote:
>>>>>
>>>>> Hi Ferruh
>>>>>
>>>>> From: Ferruh Yigit:
>>>>>> On 5/1/2020 7:51 AM, Matan Azrad wrote:
>>>>>>> Hi Ferruh
>>>>>>>
>>>>>>> From: Ferruh Yigit
>>>>>>>> On 4/30/2020 4:53 PM, Bill Zhou wrote:
>>>>>>>>> Currently, there is no way to check the aging event or to get
>>>>>>>>> the current aged flows in testpmd, this patch include those
>>>>>>>>> implements, it's
>>>>>>>> included:
>>>>>>>>> - Registering aging event when the testpmd application start, add
>> new
>>>>>>>>>   command to control if the event expose to the applications. If it's
>> be
>>>>>>>>>   set, when new flow be checked age out, there will be one-line
>>>>>>>>> output
>>>>>> log.
>>>>>>>>> - Add new command to list all aged flows, meanwhile, we can set
>>>>>>>> parameter
>>>>>>>>>   to destroy it.
>>>>>>>>>
>>>>>>>>> Signed-off-by: Bill Zhou <dongz@mellanox.com>
>>>>>>>>> ---
>>>>>>>>> v2: Update the way of registering aging event, add new command
>>>>>>>>> to control if the event need be print or not. Update the output
>>>>>>>>> of the delete aged flow command format.
> <...>
>>>>>> What do you think about a more generic "set event_verbose on|off"
>>>>>> command, which can control to logging for all event handlers, but
>>>>>> right now only for aged events?
>>>>>
>>>>> Looks good to me, but maybe instead of on | off it is better to use
>>>>> bitmap
>>>> in order to indicate the event?
>>>>
>>>> No objection but not sure that level fine grain needed now, or later,
>>>> why not add the basic on/off now and add the bitmap when we need to
>>>> control for each event.
>>>
>>> It will be good to the user to reduce logs of non-interested events (maybe
>> even more often) which makes his log bigger.
>>
>> Right now, is there any other event that logs? And how much log do they
>> produce?
> 
> I don't really familiar with all, but looks like IPSEC, VF_MBOX and AGED may make the log bigger than others.
> 
> So, if we open verbose for all, the events log bitmap may be very usable.
> 

I was thinking they are not printing anything at all, but if they do I agree
with you. Thanks.

[dpdk-dev] [PATCH v3] app/testpmd: support flow aging

[Bill Zhou]

Currently, there is no way to check the aging event or to get the current
aged flows in testpmd, this patch include those implements, it's included:
- Registering aging event when the testpmd application start, add one new
  command to set verbose bitmaps for all events. If RTE_ETH_EVENT_FLOW_AGED
  bit is set, when new flow be checked age out, there will be output log
  for it.
- Add new command to list all aged flows, meanwhile, we can set parameter
  to destroy it.

Signed-off-by: Bill Zhou <dongz@mellanox.com>
---
v2: Update the way of registering aging event, add new command to control
if the event need be print or not. Update the output of the delete aged
flow command format.
v3: Change the command from only set aged flow output to set one gloable
verbose bitmap for all events output.
---
 app/test-pmd/cmdline.c                      |  49 +++++++++
 app/test-pmd/cmdline_flow.c                 |  62 +++++++++++
 app/test-pmd/config.c                       | 108 ++++++++++++++++++--
 app/test-pmd/testpmd.c                      |  12 +++
 app/test-pmd/testpmd.h                      |   4 +
 doc/guides/testpmd_app_ug/testpmd_funcs.rst |  79 ++++++++++++++
 6 files changed, 303 insertions(+), 11 deletions(-)

diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c
index 1375f223eb..a257f25e01 100644
--- a/app/test-pmd/cmdline.c
+++ b/app/test-pmd/cmdline.c
@@ -263,6 +263,9 @@ static void cmd_help_long_parsed(void *parsed_result,
 			"set verbose (level)\n"
 			"    Set the debug verbosity level X.\n\n"
 
+			"set event_verbose (bitmap)\n"
+			"    Set the event debug verbose 32 bits bitmap X.\n\n"
+
 			"set log global|(type) (level)\n"
 			"    Set the log level.\n\n"
 
@@ -1125,6 +1128,10 @@ static void cmd_help_long_parsed(void *parsed_result,
 			"    Restrict ingress traffic to the defined"
 			" flow rules\n\n"
 
+			"flow aged {port_id} [destroy]\n"
+			"    List and destroy aged flows"
+			" flow rules\n\n"
+
 			"set vxlan ip-version (ipv4|ipv6) vni (vni) udp-src"
 			" (udp-src) udp-dst (udp-dst) ip-src (ip-src) ip-dst"
 			" (ip-dst) eth-src (eth-src) eth-dst (eth-dst)\n"
@@ -19387,6 +19394,47 @@ cmdline_parse_inst_t cmd_showport_macs = {
 	},
 };
 
+/* Set event verbose bitmap*/
+struct cmd_set_event_verbose_result {
+	cmdline_fixed_string_t set;
+	cmdline_fixed_string_t keyword;
+	uint32_t bitmap;
+};
+cmdline_parse_token_string_t cmd_set_event_verbose_set =
+	TOKEN_STRING_INITIALIZER(struct cmd_set_event_verbose_result,
+		set, "set");
+cmdline_parse_token_string_t cmd_set_event_verbose_keyword =
+	TOKEN_STRING_INITIALIZER(struct cmd_set_event_verbose_result,
+		keyword, "event_verbose");
+cmdline_parse_token_num_t cmd_set_event_verbose_bitmap =
+	TOKEN_NUM_INITIALIZER(struct cmd_set_event_verbose_result,
+		bitmap, UINT32);
+
+static void
+cmd_set_event_verbose_parsed(void *parsed_result,
+				__rte_unused struct cmdline *cl,
+				__rte_unused void *data)
+{
+	struct cmd_set_event_verbose_result *res = parsed_result;
+
+	printf("Change event verbose bitmap from 0x%x to 0x%x\n",
+	       (unsigned int) event_verbose_bitmap,
+	       (unsigned int) res->bitmap);
+	event_verbose_bitmap = res->bitmap;
+}
+
+cmdline_parse_inst_t cmd_set_event_verbose = {
+	.f = cmd_set_event_verbose_parsed,
+	.data = NULL,
+	.help_str = "set event_verbose (bitmap)",
+	.tokens = {
+		(void *)&cmd_set_event_verbose_set,
+		(void *)&cmd_set_event_verbose_keyword,
+		(void *)&cmd_set_event_verbose_bitmap,
+		NULL,
+	},
+};
+
 /* ******************************************************************************** */
 
 /* list of instructions */
@@ -19684,6 +19732,7 @@ cmdline_parse_ctx_t main_ctx[] = {
 	(cmdline_parse_inst_t *)&cmd_show_set_raw,
 	(cmdline_parse_inst_t *)&cmd_show_set_raw_all,
 	(cmdline_parse_inst_t *)&cmd_config_tx_dynf_specific,
+	(cmdline_parse_inst_t *)&cmd_set_event_verbose,
 	NULL,
 };
 
diff --git a/app/test-pmd/cmdline_flow.c b/app/test-pmd/cmdline_flow.c
index 45bcff3cf5..4e2006c543 100644
--- a/app/test-pmd/cmdline_flow.c
+++ b/app/test-pmd/cmdline_flow.c
@@ -67,6 +67,7 @@ enum index {
 	DUMP,
 	QUERY,
 	LIST,
+	AGED,
 	ISOLATE,
 
 	/* Destroy arguments. */
@@ -78,6 +79,9 @@ enum index {
 	/* List arguments. */
 	LIST_GROUP,
 
+	/* Destroy aged flow arguments. */
+	AGED_DESTROY,
+
 	/* Validate/create arguments. */
 	GROUP,
 	PRIORITY,
@@ -664,6 +668,9 @@ struct buffer {
 		struct {
 			int set;
 		} isolate; /**< Isolated mode arguments. */
+		struct {
+			int destroy;
+		} aged; /**< Aged arguments. */
 	} args; /**< Command arguments. */
 };
 
@@ -719,6 +726,12 @@ static const enum index next_list_attr[] = {
 	ZERO,
 };
 
+static const enum index next_aged_attr[] = {
+	AGED_DESTROY,
+	END,
+	ZERO,
+};
+
 static const enum index item_param[] = {
 	ITEM_PARAM_IS,
 	ITEM_PARAM_SPEC,
@@ -1466,6 +1479,9 @@ static int parse_action(struct context *, const struct token *,
 static int parse_list(struct context *, const struct token *,
 		      const char *, unsigned int,
 		      void *, unsigned int);
+static int parse_aged(struct context *, const struct token *,
+		      const char *, unsigned int,
+		      void *, unsigned int);
 static int parse_isolate(struct context *, const struct token *,
 			 const char *, unsigned int,
 			 void *, unsigned int);
@@ -1649,6 +1665,7 @@ static const struct token token_list[] = {
 			      FLUSH,
 			      DUMP,
 			      LIST,
+			      AGED,
 			      QUERY,
 			      ISOLATE)),
 		.call = parse_init,
@@ -1708,6 +1725,13 @@ static const struct token token_list[] = {
 		.args = ARGS(ARGS_ENTRY(struct buffer, port)),
 		.call = parse_list,
 	},
+	[AGED] = {
+		.name = "aged",
+		.help = "list and destroy aged flows",
+		.next = NEXT(next_aged_attr, NEXT_ENTRY(PORT_ID)),
+		.args = ARGS(ARGS_ENTRY(struct buffer, port)),
+		.call = parse_aged,
+	},
 	[ISOLATE] = {
 		.name = "isolate",
 		.help = "restrict ingress traffic to the defined flow rules",
@@ -1741,6 +1765,12 @@ static const struct token token_list[] = {
 		.args = ARGS(ARGS_ENTRY_PTR(struct buffer, args.list.group)),
 		.call = parse_list,
 	},
+	[AGED_DESTROY] = {
+		.name = "destroy",
+		.help = "specify aged flows need be destroyed",
+		.call = parse_aged,
+		.comp = comp_none,
+	},
 	/* Validate/create attributes. */
 	[GROUP] = {
 		.name = "group",
@@ -5367,6 +5397,35 @@ parse_list(struct context *ctx, const struct token *token,
 	return len;
 }
 
+/** Parse tokens for list all aged flows command. */
+static int
+parse_aged(struct context *ctx, const struct token *token,
+	   const char *str, unsigned int len,
+	   void *buf, unsigned int size)
+{
+	struct buffer *out = buf;
+
+	/* Token name must match. */
+	if (parse_default(ctx, token, str, len, NULL, 0) < 0)
+		return -1;
+	/* Nothing else to do if there is no buffer. */
+	if (!out)
+		return len;
+	if (!out->command) {
+		if (ctx->curr != AGED)
+			return -1;
+		if (sizeof(*out) > size)
+			return -1;
+		out->command = ctx->curr;
+		ctx->objdata = 0;
+		ctx->object = out;
+		ctx->objmask = NULL;
+	}
+	if (ctx->curr == AGED_DESTROY)
+		out->args.aged.destroy = 1;
+	return len;
+}
+
 /** Parse tokens for isolate command. */
 static int
 parse_isolate(struct context *ctx, const struct token *token,
@@ -6367,6 +6426,9 @@ cmd_flow_parsed(const struct buffer *in)
 	case ISOLATE:
 		port_flow_isolate(in->port, in->args.isolate.set);
 		break;
+	case AGED:
+		port_flow_aged(in->port, in->args.aged.destroy);
+		break;
 	default:
 		break;
 	}
diff --git a/app/test-pmd/config.c b/app/test-pmd/config.c
index 72f25d1521..035d336ab5 100644
--- a/app/test-pmd/config.c
+++ b/app/test-pmd/config.c
@@ -1367,6 +1367,26 @@ port_flow_validate(portid_t port_id,
 	return 0;
 }
 
+/** Update age action context by port_flow pointer. */
+void
+update_age_action_context(const struct rte_flow_action *actions,
+			struct port_flow *pf)
+{
+	struct rte_flow_action_age *age = NULL;
+
+	for (; actions->type != RTE_FLOW_ACTION_TYPE_END; actions++) {
+		switch (actions->type) {
+		case RTE_FLOW_ACTION_TYPE_AGE:
+			age = (struct rte_flow_action_age *)
+				(uintptr_t)actions->conf;
+			age->context = pf;
+			return;
+		default:
+			break;
+		}
+	}
+}
+
 /** Create flow rule. */
 int
 port_flow_create(portid_t port_id,
@@ -1377,28 +1397,27 @@ port_flow_create(portid_t port_id,
 	struct rte_flow *flow;
 	struct rte_port *port;
 	struct port_flow *pf;
-	uint32_t id;
+	uint32_t id = 0;
 	struct rte_flow_error error;
 
-	/* Poisoning to make sure PMDs update it in case of error. */
-	memset(&error, 0x22, sizeof(error));
-	flow = rte_flow_create(port_id, attr, pattern, actions, &error);
-	if (!flow)
-		return port_flow_complain(&error);
 	port = &ports[port_id];
 	if (port->flow_list) {
 		if (port->flow_list->id == UINT32_MAX) {
 			printf("Highest rule ID is already assigned, delete"
 			       " it first");
-			rte_flow_destroy(port_id, flow, NULL);
 			return -ENOMEM;
 		}
 		id = port->flow_list->id + 1;
-	} else
-		id = 0;
+	}
 	pf = port_flow_new(attr, pattern, actions, &error);
-	if (!pf) {
-		rte_flow_destroy(port_id, flow, NULL);
+	if (!pf)
+		return port_flow_complain(&error);
+	update_age_action_context(actions, pf);
+	/* Poisoning to make sure PMDs update it in case of error. */
+	memset(&error, 0x22, sizeof(error));
+	flow = rte_flow_create(port_id, attr, pattern, actions, &error);
+	if (!flow) {
+		free(pf);
 		return port_flow_complain(&error);
 	}
 	pf->next = port->flow_list;
@@ -1570,6 +1589,73 @@ port_flow_query(portid_t port_id, uint32_t rule,
 	return 0;
 }
 
+/** List simply and destroy all aged flows. */
+void
+port_flow_aged(portid_t port_id, uint8_t destroy)
+{
+	void **contexts;
+	int nb_context, total = 0, idx;
+	struct rte_flow_error error;
+	struct port_flow *pf;
+
+	if (port_id_is_invalid(port_id, ENABLED_WARN) ||
+	    port_id == (portid_t)RTE_PORT_ALL)
+		return;
+	total = rte_flow_get_aged_flows(port_id, NULL, 0, &error);
+	printf("Port %u total aged flows: %d\n", port_id, total);
+	if (total < 0) {
+		port_flow_complain(&error);
+		return;
+	}
+	if (total == 0)
+		return;
+	contexts = malloc(sizeof(void *) * total);
+	if (contexts == NULL) {
+		printf("Cannot allocate contexts for aged flow\n");
+		return;
+	}
+	printf("ID\tGroup\tPrio\tAttr\n");
+	nb_context = rte_flow_get_aged_flows(port_id, contexts, total, &error);
+	if (nb_context != total) {
+		printf("Port:%d get aged flows count(%d) != total(%d)\n",
+			port_id, nb_context, total);
+		free(contexts);
+		return;
+	}
+	for (idx = 0; idx < nb_context; idx++) {
+		pf = (struct port_flow *)contexts[idx];
+		if (!pf) {
+			printf("Error: get Null context in port %u\n", port_id);
+			continue;
+		}
+		printf("%" PRIu32 "\t%" PRIu32 "\t%" PRIu32 "\t%c%c%c\t\n",
+		       pf->id,
+		       pf->rule.attr->group,
+		       pf->rule.attr->priority,
+		       pf->rule.attr->ingress ? 'i' : '-',
+		       pf->rule.attr->egress ? 'e' : '-',
+		       pf->rule.attr->transfer ? 't' : '-');
+	}
+	if (destroy) {
+		int ret;
+		uint32_t flow_id;
+
+		total = 0;
+		printf("\n");
+		for (idx = 0; idx < nb_context; idx++) {
+			pf = (struct port_flow *)contexts[idx];
+			if (!pf)
+				continue;
+			flow_id = pf->id;
+			ret = port_flow_destroy(port_id, 1, &flow_id);
+			if (!ret)
+				total++;
+		}
+		printf("%d flows be destroyed\n", total);
+	}
+	free(contexts);
+}
+
 /** List flow rules. */
 void
 port_flow_list(portid_t port_id, uint32_t n, const uint32_t group[n])
diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c
index 99bacddbfd..689467f736 100644
--- a/app/test-pmd/testpmd.c
+++ b/app/test-pmd/testpmd.c
@@ -81,6 +81,7 @@
 #define EXTBUF_ZONE_SIZE RTE_PGSIZE_2M
 
 uint16_t verbose_level = 0; /**< Silent by default. */
+uint32_t event_verbose_bitmap; /**< Verbose bitmap for all events */
 int testpmd_logtype; /**< Log type for testpmd logs */
 
 /* use master core for command line ? */
@@ -3068,6 +3069,15 @@ rmv_port_callback(void *arg)
 		start_packet_forwarding(0);
 }
 
+static void
+aging_event_output(uint16_t portid)
+{
+	if (event_verbose_bitmap & (1 << RTE_ETH_EVENT_FLOW_AGED)) {
+		printf("port %u RTE_ETH_EVENT_FLOW_AGED triggered\n", portid);
+		fflush(stdout);
+	}
+}
+
 /* This function is used by the interrupt thread */
 static int
 eth_event_callback(portid_t port_id, enum rte_eth_event_type type, void *param,
@@ -3098,6 +3108,8 @@ eth_event_callback(portid_t port_id, enum rte_eth_event_type type, void *param,
 				rmv_port_callback, (void *)(intptr_t)port_id))
 			fprintf(stderr, "Could not set up deferred device removal\n");
 		break;
+	case RTE_ETH_EVENT_FLOW_AGED:
+		aging_event_output(port_id);
 	default:
 		break;
 	}
diff --git a/app/test-pmd/testpmd.h b/app/test-pmd/testpmd.h
index 7ff4c5dba3..674c3670f9 100644
--- a/app/test-pmd/testpmd.h
+++ b/app/test-pmd/testpmd.h
@@ -323,6 +323,7 @@ extern uint8_t xstats_hide_zero; /**< Hide zero values for xstats display */
 
 /* globals used for configuration */
 extern uint16_t verbose_level; /**< Drives messages being displayed, if any. */
+extern uint32_t event_verbose_bitmap; /**< Verbose bitmap for all events */
 extern int testpmd_logtype; /**< Log type for testpmd logs */
 extern uint8_t  interactive;
 extern uint8_t  auto_start;
@@ -747,12 +748,15 @@ 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);
+void update_age_action_context(const struct rte_flow_action *actions,
+		     struct port_flow *pf);
 int port_flow_destroy(portid_t port_id, uint32_t n, const uint32_t *rule);
 int port_flow_flush(portid_t port_id);
 int port_flow_dump(portid_t port_id, const char *file_name);
 int port_flow_query(portid_t port_id, uint32_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);
 int port_flow_isolate(portid_t port_id, int set);
 
 void rx_ring_desc_display(portid_t port_id, queueid_t rxq_id, uint16_t rxd_id);
diff --git a/doc/guides/testpmd_app_ug/testpmd_funcs.rst b/doc/guides/testpmd_app_ug/testpmd_funcs.rst
index a360ecccfd..b5eb59ba46 100644
--- a/doc/guides/testpmd_app_ug/testpmd_funcs.rst
+++ b/doc/guides/testpmd_app_ug/testpmd_funcs.rst
@@ -632,6 +632,23 @@ Available levels are as following:
 * ``2`` fully verbose except for Rx packets.
 * ``> 2`` fully verbose.
 
+set event_verbose
+~~~~~~~~~~~~~~~~~
+
+Set the debug verbosity bitmaps for all events defined in enum rte_eth_event_type,
+the maximum bits of the bitmap is 32::
+
+   testpmd> set event_verbose (bitmap)
+
+For examine, to start the event:RTE_ETH_EVENT_FLOW_AGED log::
+
+   testpmd> set event_verbose 0x400
+   Change event verbose bitmap from 0x0 to 0x400
+
+When aged flow be checkout, there will be one output log for it::
+
+   testpmd> port 0 RTE_ETH_EVENT_FLOW_AGED triggered
+
 set log
 ~~~~~~~
 
@@ -3616,6 +3633,10 @@ following sections.
 
    flow dump {port_id} {output_file}
 
+- List and destroy aged flow rules::
+
+   flow aged {port_id} [destroy]
+
 Validating flow rules
 ~~~~~~~~~~~~~~~~~~~~~
 
@@ -4503,6 +4524,64 @@ Otherwise, it will complain error occurred::
 
    Caught error type [...] ([...]): [...]
 
+Listing and destroying aged flow rules
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+
+``flow aged`` simply lists aged flow rules be get from api ``rte_flow_get_aged_flows``,
+and ``destroy`` parameter can be used to destroy those flow rules in PMD.
+
+   flow aged {port_id} [destroy]
+
+Listing current aged flow rules::
+
+   testpmd> flow aged 0
+   Port 0 total aged flows: 0
+   testpmd> flow create 0 ingress pattern eth / ipv4 src is 2.2.2.14 / end
+      actions age timeout 5 / queue index 0 /  end
+   Flow rule #0 created
+   testpmd> flow create 0 ingress pattern eth / ipv4 src is 2.2.2.15 / end
+      actions age timeout 4 / queue index 0 /  end
+   Flow rule #1 created
+   testpmd> flow create 0 ingress pattern eth / ipv4 src is 2.2.2.16 / end
+      actions age timeout 2 / queue index 0 /  end
+   Flow rule #2 created
+   testpmd> flow create 0 ingress pattern eth / ipv4 src is 2.2.2.17 / end
+      actions age timeout 3 / queue index 0 /  end
+   Flow rule #3 created
+
+
+Aged Rules are simply list as command ``flow list {port_id}``, but strip the detail rule
+information, all the aged flows are sorted by the longest timeout time. For example, if
+those rules be configured in the same time, ID 2 will be the first aged out rule, the next
+will be ID 3, ID 1, ID 0::
+
+   testpmd> flow aged 0
+   Port 0 total aged flows: 4
+   ID      Group   Prio    Attr
+   2       0       0       i--
+   3       0       0       i--
+   1       0       0       i--
+   0       0       0       i--
+
+If attach ``destroy`` parameter, the command will destroy all the list aged flow rules.
+
+   testpmd> flow aged 0 destroy
+   Port 0 total aged flows: 4
+   ID      Group   Prio    Attr
+   2       0       0       i--
+   3       0       0       i--
+   1       0       0       i--
+   0       0       0       i--
+
+   Flow rule #2 destroyed
+   Flow rule #3 destroyed
+   Flow rule #1 destroyed
+   Flow rule #0 destroyed
+   4 flows be destroyed
+   testpmd> flow aged 0
+   Port 0 total aged flows: 0
+
+
 Sample QinQ flow rules
 ~~~~~~~~~~~~~~~~~~~~~~
 
-- 
2.21.0

[dpdk-dev] [PATCH v4] app/testpmd: support flow aging

[Bill Zhou]

Currently, there is no way to check the aging event or to get the current
aged flows in testpmd, this patch include those implements, it's included:

- Add new item "flow_aged" to the current print event command arguments.
- Add new command to list all aged flows, meanwhile, we can set parameter
  to destroy it.

Signed-off-by: Bill Zhou <dongz@mellanox.com>
---
v2: Update the way of registering aging event, add new command to control
if the event need be print or not. Update the output of the delete aged
flow command format.
v3: Change the command from only set aged flow output to set one gloable
verbose bitmap for all events output.
v4: Add the event output to current global print event arguments.
---
 app/test-pmd/cmdline.c                      |   4 +
 app/test-pmd/cmdline_flow.c                 |  62 +++++++++++
 app/test-pmd/config.c                       | 108 ++++++++++++++++++--
 app/test-pmd/parameters.c                   |   6 +-
 app/test-pmd/testpmd.c                      |   4 +-
 app/test-pmd/testpmd.h                      |   3 +
 doc/guides/testpmd_app_ug/testpmd_funcs.rst |  62 +++++++++++
 7 files changed, 235 insertions(+), 14 deletions(-)

diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c
index 1375f223eb..bcf9080c48 100644
--- a/app/test-pmd/cmdline.c
+++ b/app/test-pmd/cmdline.c
@@ -1125,6 +1125,10 @@ static void cmd_help_long_parsed(void *parsed_result,
 			"    Restrict ingress traffic to the defined"
 			" flow rules\n\n"
 
+			"flow aged {port_id} [destroy]\n"
+			"    List and destroy aged flows"
+			" flow rules\n\n"
+
 			"set vxlan ip-version (ipv4|ipv6) vni (vni) udp-src"
 			" (udp-src) udp-dst (udp-dst) ip-src (ip-src) ip-dst"
 			" (ip-dst) eth-src (eth-src) eth-dst (eth-dst)\n"
diff --git a/app/test-pmd/cmdline_flow.c b/app/test-pmd/cmdline_flow.c
index 45bcff3cf5..4e2006c543 100644
--- a/app/test-pmd/cmdline_flow.c
+++ b/app/test-pmd/cmdline_flow.c
@@ -67,6 +67,7 @@ enum index {
 	DUMP,
 	QUERY,
 	LIST,
+	AGED,
 	ISOLATE,
 
 	/* Destroy arguments. */
@@ -78,6 +79,9 @@ enum index {
 	/* List arguments. */
 	LIST_GROUP,
 
+	/* Destroy aged flow arguments. */
+	AGED_DESTROY,
+
 	/* Validate/create arguments. */
 	GROUP,
 	PRIORITY,
@@ -664,6 +668,9 @@ struct buffer {
 		struct {
 			int set;
 		} isolate; /**< Isolated mode arguments. */
+		struct {
+			int destroy;
+		} aged; /**< Aged arguments. */
 	} args; /**< Command arguments. */
 };
 
@@ -719,6 +726,12 @@ static const enum index next_list_attr[] = {
 	ZERO,
 };
 
+static const enum index next_aged_attr[] = {
+	AGED_DESTROY,
+	END,
+	ZERO,
+};
+
 static const enum index item_param[] = {
 	ITEM_PARAM_IS,
 	ITEM_PARAM_SPEC,
@@ -1466,6 +1479,9 @@ static int parse_action(struct context *, const struct token *,
 static int parse_list(struct context *, const struct token *,
 		      const char *, unsigned int,
 		      void *, unsigned int);
+static int parse_aged(struct context *, const struct token *,
+		      const char *, unsigned int,
+		      void *, unsigned int);
 static int parse_isolate(struct context *, const struct token *,
 			 const char *, unsigned int,
 			 void *, unsigned int);
@@ -1649,6 +1665,7 @@ static const struct token token_list[] = {
 			      FLUSH,
 			      DUMP,
 			      LIST,
+			      AGED,
 			      QUERY,
 			      ISOLATE)),
 		.call = parse_init,
@@ -1708,6 +1725,13 @@ static const struct token token_list[] = {
 		.args = ARGS(ARGS_ENTRY(struct buffer, port)),
 		.call = parse_list,
 	},
+	[AGED] = {
+		.name = "aged",
+		.help = "list and destroy aged flows",
+		.next = NEXT(next_aged_attr, NEXT_ENTRY(PORT_ID)),
+		.args = ARGS(ARGS_ENTRY(struct buffer, port)),
+		.call = parse_aged,
+	},
 	[ISOLATE] = {
 		.name = "isolate",
 		.help = "restrict ingress traffic to the defined flow rules",
@@ -1741,6 +1765,12 @@ static const struct token token_list[] = {
 		.args = ARGS(ARGS_ENTRY_PTR(struct buffer, args.list.group)),
 		.call = parse_list,
 	},
+	[AGED_DESTROY] = {
+		.name = "destroy",
+		.help = "specify aged flows need be destroyed",
+		.call = parse_aged,
+		.comp = comp_none,
+	},
 	/* Validate/create attributes. */
 	[GROUP] = {
 		.name = "group",
@@ -5367,6 +5397,35 @@ parse_list(struct context *ctx, const struct token *token,
 	return len;
 }
 
+/** Parse tokens for list all aged flows command. */
+static int
+parse_aged(struct context *ctx, const struct token *token,
+	   const char *str, unsigned int len,
+	   void *buf, unsigned int size)
+{
+	struct buffer *out = buf;
+
+	/* Token name must match. */
+	if (parse_default(ctx, token, str, len, NULL, 0) < 0)
+		return -1;
+	/* Nothing else to do if there is no buffer. */
+	if (!out)
+		return len;
+	if (!out->command) {
+		if (ctx->curr != AGED)
+			return -1;
+		if (sizeof(*out) > size)
+			return -1;
+		out->command = ctx->curr;
+		ctx->objdata = 0;
+		ctx->object = out;
+		ctx->objmask = NULL;
+	}
+	if (ctx->curr == AGED_DESTROY)
+		out->args.aged.destroy = 1;
+	return len;
+}
+
 /** Parse tokens for isolate command. */
 static int
 parse_isolate(struct context *ctx, const struct token *token,
@@ -6367,6 +6426,9 @@ cmd_flow_parsed(const struct buffer *in)
 	case ISOLATE:
 		port_flow_isolate(in->port, in->args.isolate.set);
 		break;
+	case AGED:
+		port_flow_aged(in->port, in->args.aged.destroy);
+		break;
 	default:
 		break;
 	}
diff --git a/app/test-pmd/config.c b/app/test-pmd/config.c
index 72f25d1521..035d336ab5 100644
--- a/app/test-pmd/config.c
+++ b/app/test-pmd/config.c
@@ -1367,6 +1367,26 @@ port_flow_validate(portid_t port_id,
 	return 0;
 }
 
+/** Update age action context by port_flow pointer. */
+void
+update_age_action_context(const struct rte_flow_action *actions,
+			struct port_flow *pf)
+{
+	struct rte_flow_action_age *age = NULL;
+
+	for (; actions->type != RTE_FLOW_ACTION_TYPE_END; actions++) {
+		switch (actions->type) {
+		case RTE_FLOW_ACTION_TYPE_AGE:
+			age = (struct rte_flow_action_age *)
+				(uintptr_t)actions->conf;
+			age->context = pf;
+			return;
+		default:
+			break;
+		}
+	}
+}
+
 /** Create flow rule. */
 int
 port_flow_create(portid_t port_id,
@@ -1377,28 +1397,27 @@ port_flow_create(portid_t port_id,
 	struct rte_flow *flow;
 	struct rte_port *port;
 	struct port_flow *pf;
-	uint32_t id;
+	uint32_t id = 0;
 	struct rte_flow_error error;
 
-	/* Poisoning to make sure PMDs update it in case of error. */
-	memset(&error, 0x22, sizeof(error));
-	flow = rte_flow_create(port_id, attr, pattern, actions, &error);
-	if (!flow)
-		return port_flow_complain(&error);
 	port = &ports[port_id];
 	if (port->flow_list) {
 		if (port->flow_list->id == UINT32_MAX) {
 			printf("Highest rule ID is already assigned, delete"
 			       " it first");
-			rte_flow_destroy(port_id, flow, NULL);
 			return -ENOMEM;
 		}
 		id = port->flow_list->id + 1;
-	} else
-		id = 0;
+	}
 	pf = port_flow_new(attr, pattern, actions, &error);
-	if (!pf) {
-		rte_flow_destroy(port_id, flow, NULL);
+	if (!pf)
+		return port_flow_complain(&error);
+	update_age_action_context(actions, pf);
+	/* Poisoning to make sure PMDs update it in case of error. */
+	memset(&error, 0x22, sizeof(error));
+	flow = rte_flow_create(port_id, attr, pattern, actions, &error);
+	if (!flow) {
+		free(pf);
 		return port_flow_complain(&error);
 	}
 	pf->next = port->flow_list;
@@ -1570,6 +1589,73 @@ port_flow_query(portid_t port_id, uint32_t rule,
 	return 0;
 }
 
+/** List simply and destroy all aged flows. */
+void
+port_flow_aged(portid_t port_id, uint8_t destroy)
+{
+	void **contexts;
+	int nb_context, total = 0, idx;
+	struct rte_flow_error error;
+	struct port_flow *pf;
+
+	if (port_id_is_invalid(port_id, ENABLED_WARN) ||
+	    port_id == (portid_t)RTE_PORT_ALL)
+		return;
+	total = rte_flow_get_aged_flows(port_id, NULL, 0, &error);
+	printf("Port %u total aged flows: %d\n", port_id, total);
+	if (total < 0) {
+		port_flow_complain(&error);
+		return;
+	}
+	if (total == 0)
+		return;
+	contexts = malloc(sizeof(void *) * total);
+	if (contexts == NULL) {
+		printf("Cannot allocate contexts for aged flow\n");
+		return;
+	}
+	printf("ID\tGroup\tPrio\tAttr\n");
+	nb_context = rte_flow_get_aged_flows(port_id, contexts, total, &error);
+	if (nb_context != total) {
+		printf("Port:%d get aged flows count(%d) != total(%d)\n",
+			port_id, nb_context, total);
+		free(contexts);
+		return;
+	}
+	for (idx = 0; idx < nb_context; idx++) {
+		pf = (struct port_flow *)contexts[idx];
+		if (!pf) {
+			printf("Error: get Null context in port %u\n", port_id);
+			continue;
+		}
+		printf("%" PRIu32 "\t%" PRIu32 "\t%" PRIu32 "\t%c%c%c\t\n",
+		       pf->id,
+		       pf->rule.attr->group,
+		       pf->rule.attr->priority,
+		       pf->rule.attr->ingress ? 'i' : '-',
+		       pf->rule.attr->egress ? 'e' : '-',
+		       pf->rule.attr->transfer ? 't' : '-');
+	}
+	if (destroy) {
+		int ret;
+		uint32_t flow_id;
+
+		total = 0;
+		printf("\n");
+		for (idx = 0; idx < nb_context; idx++) {
+			pf = (struct port_flow *)contexts[idx];
+			if (!pf)
+				continue;
+			flow_id = pf->id;
+			ret = port_flow_destroy(port_id, 1, &flow_id);
+			if (!ret)
+				total++;
+		}
+		printf("%d flows be destroyed\n", total);
+	}
+	free(contexts);
+}
+
 /** List flow rules. */
 void
 port_flow_list(portid_t port_id, uint32_t n, const uint32_t group[n])
diff --git a/app/test-pmd/parameters.c b/app/test-pmd/parameters.c
index 30c1753c32..92b5575626 100644
--- a/app/test-pmd/parameters.c
+++ b/app/test-pmd/parameters.c
@@ -187,9 +187,9 @@ usage(char* progname)
 	printf("  --no-rmv-interrupt: disable device removal interrupt.\n");
 	printf("  --bitrate-stats=N: set the logical core N to perform "
 		"bit-rate calculation.\n");
-	printf("  --print-event <unknown|intr_lsc|queue_state|intr_reset|vf_mbox|macsec|intr_rmv|all>: "
+	printf("  --print-event <unknown|intr_lsc|queue_state|intr_reset|vf_mbox|macsec|intr_rmv|flow_aged|all>: "
 	       "enable print of designated event or all of them.\n");
-	printf("  --mask-event <unknown|intr_lsc|queue_state|intr_reset|vf_mbox|macsec|intr_rmv|all>: "
+	printf("  --mask-event <unknown|intr_lsc|queue_state|intr_reset|vf_mbox|macsec|intr_rmv|flow_aged|all>: "
 	       "disable print of designated event or all of them.\n");
 	printf("  --flow-isolate-all: "
 	       "requests flow API isolated mode on all ports at initialization time.\n");
@@ -545,6 +545,8 @@ parse_event_printing_config(const char *optarg, int enable)
 		mask = UINT32_C(1) << RTE_ETH_EVENT_NEW;
 	else if (!strcmp(optarg, "dev_released"))
 		mask = UINT32_C(1) << RTE_ETH_EVENT_DESTROY;
+	else if (!strcmp(optarg, "flow_aged"))
+		mask = UINT32_C(1) << RTE_ETH_EVENT_FLOW_AGED;
 	else if (!strcmp(optarg, "all"))
 		mask = ~UINT32_C(0);
 	else {
diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c
index 99bacddbfd..a2d0be56b3 100644
--- a/app/test-pmd/testpmd.c
+++ b/app/test-pmd/testpmd.c
@@ -375,6 +375,7 @@ static const char * const eth_event_desc[] = {
 	[RTE_ETH_EVENT_INTR_RMV] = "device removal",
 	[RTE_ETH_EVENT_NEW] = "device probed",
 	[RTE_ETH_EVENT_DESTROY] = "device released",
+	[RTE_ETH_EVENT_FLOW_AGED] = "flow aged",
 	[RTE_ETH_EVENT_MAX] = NULL,
 };
 
@@ -388,7 +389,8 @@ uint32_t event_print_mask = (UINT32_C(1) << RTE_ETH_EVENT_UNKNOWN) |
 			    (UINT32_C(1) << RTE_ETH_EVENT_INTR_RESET) |
 			    (UINT32_C(1) << RTE_ETH_EVENT_IPSEC) |
 			    (UINT32_C(1) << RTE_ETH_EVENT_MACSEC) |
-			    (UINT32_C(1) << RTE_ETH_EVENT_INTR_RMV);
+			    (UINT32_C(1) << RTE_ETH_EVENT_INTR_RMV) |
+			    (UINT32_C(1) << RTE_ETH_EVENT_FLOW_AGED);
 /*
  * Decide if all memory are locked for performance.
  */
diff --git a/app/test-pmd/testpmd.h b/app/test-pmd/testpmd.h
index 7ff4c5dba3..fb391672a8 100644
--- a/app/test-pmd/testpmd.h
+++ b/app/test-pmd/testpmd.h
@@ -747,12 +747,15 @@ 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);
+void update_age_action_context(const struct rte_flow_action *actions,
+		     struct port_flow *pf);
 int port_flow_destroy(portid_t port_id, uint32_t n, const uint32_t *rule);
 int port_flow_flush(portid_t port_id);
 int port_flow_dump(portid_t port_id, const char *file_name);
 int port_flow_query(portid_t port_id, uint32_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);
 int port_flow_isolate(portid_t port_id, int set);
 
 void rx_ring_desc_display(portid_t port_id, queueid_t rxq_id, uint16_t rxd_id);
diff --git a/doc/guides/testpmd_app_ug/testpmd_funcs.rst b/doc/guides/testpmd_app_ug/testpmd_funcs.rst
index a360ecccfd..19260cc2d9 100644
--- a/doc/guides/testpmd_app_ug/testpmd_funcs.rst
+++ b/doc/guides/testpmd_app_ug/testpmd_funcs.rst
@@ -3616,6 +3616,10 @@ following sections.
 
    flow dump {port_id} {output_file}
 
+- List and destroy aged flow rules::
+
+   flow aged {port_id} [destroy]
+
 Validating flow rules
 ~~~~~~~~~~~~~~~~~~~~~
 
@@ -4503,6 +4507,64 @@ Otherwise, it will complain error occurred::
 
    Caught error type [...] ([...]): [...]
 
+Listing and destroying aged flow rules
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+
+``flow aged`` simply lists aged flow rules be get from api ``rte_flow_get_aged_flows``,
+and ``destroy`` parameter can be used to destroy those flow rules in PMD.
+
+   flow aged {port_id} [destroy]
+
+Listing current aged flow rules::
+
+   testpmd> flow aged 0
+   Port 0 total aged flows: 0
+   testpmd> flow create 0 ingress pattern eth / ipv4 src is 2.2.2.14 / end
+      actions age timeout 5 / queue index 0 /  end
+   Flow rule #0 created
+   testpmd> flow create 0 ingress pattern eth / ipv4 src is 2.2.2.15 / end
+      actions age timeout 4 / queue index 0 /  end
+   Flow rule #1 created
+   testpmd> flow create 0 ingress pattern eth / ipv4 src is 2.2.2.16 / end
+      actions age timeout 2 / queue index 0 /  end
+   Flow rule #2 created
+   testpmd> flow create 0 ingress pattern eth / ipv4 src is 2.2.2.17 / end
+      actions age timeout 3 / queue index 0 /  end
+   Flow rule #3 created
+
+
+Aged Rules are simply list as command ``flow list {port_id}``, but strip the detail rule
+information, all the aged flows are sorted by the longest timeout time. For example, if
+those rules be configured in the same time, ID 2 will be the first aged out rule, the next
+will be ID 3, ID 1, ID 0::
+
+   testpmd> flow aged 0
+   Port 0 total aged flows: 4
+   ID      Group   Prio    Attr
+   2       0       0       i--
+   3       0       0       i--
+   1       0       0       i--
+   0       0       0       i--
+
+If attach ``destroy`` parameter, the command will destroy all the list aged flow rules.
+
+   testpmd> flow aged 0 destroy
+   Port 0 total aged flows: 4
+   ID      Group   Prio    Attr
+   2       0       0       i--
+   3       0       0       i--
+   1       0       0       i--
+   0       0       0       i--
+
+   Flow rule #2 destroyed
+   Flow rule #3 destroyed
+   Flow rule #1 destroyed
+   Flow rule #0 destroyed
+   4 flows be destroyed
+   testpmd> flow aged 0
+   Port 0 total aged flows: 0
+
+
 Sample QinQ flow rules
 ~~~~~~~~~~~~~~~~~~~~~~
 
-- 
2.21.0

Re: [dpdk-dev] [PATCH v4] app/testpmd: support flow aging

[Matan Azrad]

From: Bill Zhou:
> Currently, there is no way to check the aging event or to get the current aged
> flows in testpmd, this patch include those implements, it's included:
> 
> - Add new item "flow_aged" to the current print event command arguments.
> - Add new command to list all aged flows, meanwhile, we can set parameter
>   to destroy it.
> 
> Signed-off-by: Bill Zhou <dongz@mellanox.com>
Acked-by: Matan Azrad <matan@mellanox.com>

Re: [dpdk-dev] [PATCH v4] app/testpmd: support flow aging

[Ori Kam]

> -----Original Message-----
> From: Bill Zhou <dongz@mellanox.com>
> Subject: [PATCH v4] app/testpmd: support flow aging
> 
> Currently, there is no way to check the aging event or to get the current
> aged flows in testpmd, this patch include those implements, it's included:
> 
> - Add new item "flow_aged" to the current print event command arguments.
> - Add new command to list all aged flows, meanwhile, we can set parameter
>   to destroy it.
> 
> Signed-off-by: Bill Zhou <dongz@mellanox.com>
> ---
> v2: Update the way of registering aging event, add new command to control
> if the event need be print or not. Update the output of the delete aged
> flow command format.
> v3: Change the command from only set aged flow output to set one gloable
> verbose bitmap for all events output.
> v4: Add the event output to current global print event arguments.
> ---

Acked-by: Ori Kam <orika@mellanox.com>
Thanks,
Ori

>  app/test-pmd/cmdline.c                      |   4 +
>  app/test-pmd/cmdline_flow.c                 |  62 +++++++++++
>  app/test-pmd/config.c                       | 108 ++++++++++++++++++--
>  app/test-pmd/parameters.c                   |   6 +-
>  app/test-pmd/testpmd.c                      |   4 +-
>  app/test-pmd/testpmd.h                      |   3 +
>  doc/guides/testpmd_app_ug/testpmd_funcs.rst |  62 +++++++++++
>  7 files changed, 235 insertions(+), 14 deletions(-)
> 
> diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c
> index 1375f223eb..bcf9080c48 100644
> --- a/app/test-pmd/cmdline.c
> +++ b/app/test-pmd/cmdline.c
> @@ -1125,6 +1125,10 @@ static void cmd_help_long_parsed(void
> *parsed_result,
>  			"    Restrict ingress traffic to the defined"
>  			" flow rules\n\n"
> 
> +			"flow aged {port_id} [destroy]\n"
> +			"    List and destroy aged flows"
> +			" flow rules\n\n"
> +
>  			"set vxlan ip-version (ipv4|ipv6) vni (vni) udp-src"
>  			" (udp-src) udp-dst (udp-dst) ip-src (ip-src) ip-dst"
>  			" (ip-dst) eth-src (eth-src) eth-dst (eth-dst)\n"
> diff --git a/app/test-pmd/cmdline_flow.c b/app/test-pmd/cmdline_flow.c
> index 45bcff3cf5..4e2006c543 100644
> --- a/app/test-pmd/cmdline_flow.c
> +++ b/app/test-pmd/cmdline_flow.c
> @@ -67,6 +67,7 @@ enum index {
>  	DUMP,
>  	QUERY,
>  	LIST,
> +	AGED,
>  	ISOLATE,
> 
>  	/* Destroy arguments. */
> @@ -78,6 +79,9 @@ enum index {
>  	/* List arguments. */
>  	LIST_GROUP,
> 
> +	/* Destroy aged flow arguments. */
> +	AGED_DESTROY,
> +
>  	/* Validate/create arguments. */
>  	GROUP,
>  	PRIORITY,
> @@ -664,6 +668,9 @@ struct buffer {
>  		struct {
>  			int set;
>  		} isolate; /**< Isolated mode arguments. */
> +		struct {
> +			int destroy;
> +		} aged; /**< Aged arguments. */
>  	} args; /**< Command arguments. */
>  };
> 
> @@ -719,6 +726,12 @@ static const enum index next_list_attr[] = {
>  	ZERO,
>  };
> 
> +static const enum index next_aged_attr[] = {
> +	AGED_DESTROY,
> +	END,
> +	ZERO,
> +};
> +
>  static const enum index item_param[] = {
>  	ITEM_PARAM_IS,
>  	ITEM_PARAM_SPEC,
> @@ -1466,6 +1479,9 @@ static int parse_action(struct context *, const struct
> token *,
>  static int parse_list(struct context *, const struct token *,
>  		      const char *, unsigned int,
>  		      void *, unsigned int);
> +static int parse_aged(struct context *, const struct token *,
> +		      const char *, unsigned int,
> +		      void *, unsigned int);
>  static int parse_isolate(struct context *, const struct token *,
>  			 const char *, unsigned int,
>  			 void *, unsigned int);
> @@ -1649,6 +1665,7 @@ static const struct token token_list[] = {
>  			      FLUSH,
>  			      DUMP,
>  			      LIST,
> +			      AGED,
>  			      QUERY,
>  			      ISOLATE)),
>  		.call = parse_init,
> @@ -1708,6 +1725,13 @@ static const struct token token_list[] = {
>  		.args = ARGS(ARGS_ENTRY(struct buffer, port)),
>  		.call = parse_list,
>  	},
> +	[AGED] = {
> +		.name = "aged",
> +		.help = "list and destroy aged flows",
> +		.next = NEXT(next_aged_attr, NEXT_ENTRY(PORT_ID)),
> +		.args = ARGS(ARGS_ENTRY(struct buffer, port)),
> +		.call = parse_aged,
> +	},
>  	[ISOLATE] = {
>  		.name = "isolate",
>  		.help = "restrict ingress traffic to the defined flow rules",
> @@ -1741,6 +1765,12 @@ static const struct token token_list[] = {
>  		.args = ARGS(ARGS_ENTRY_PTR(struct buffer, args.list.group)),
>  		.call = parse_list,
>  	},
> +	[AGED_DESTROY] = {
> +		.name = "destroy",
> +		.help = "specify aged flows need be destroyed",
> +		.call = parse_aged,
> +		.comp = comp_none,
> +	},
>  	/* Validate/create attributes. */
>  	[GROUP] = {
>  		.name = "group",
> @@ -5367,6 +5397,35 @@ parse_list(struct context *ctx, const struct token
> *token,
>  	return len;
>  }
> 
> +/** Parse tokens for list all aged flows command. */
> +static int
> +parse_aged(struct context *ctx, const struct token *token,
> +	   const char *str, unsigned int len,
> +	   void *buf, unsigned int size)
> +{
> +	struct buffer *out = buf;
> +
> +	/* Token name must match. */
> +	if (parse_default(ctx, token, str, len, NULL, 0) < 0)
> +		return -1;
> +	/* Nothing else to do if there is no buffer. */
> +	if (!out)
> +		return len;
> +	if (!out->command) {
> +		if (ctx->curr != AGED)
> +			return -1;
> +		if (sizeof(*out) > size)
> +			return -1;
> +		out->command = ctx->curr;
> +		ctx->objdata = 0;
> +		ctx->object = out;
> +		ctx->objmask = NULL;
> +	}
> +	if (ctx->curr == AGED_DESTROY)
> +		out->args.aged.destroy = 1;
> +	return len;
> +}
> +
>  /** Parse tokens for isolate command. */
>  static int
>  parse_isolate(struct context *ctx, const struct token *token,
> @@ -6367,6 +6426,9 @@ cmd_flow_parsed(const struct buffer *in)
>  	case ISOLATE:
>  		port_flow_isolate(in->port, in->args.isolate.set);
>  		break;
> +	case AGED:
> +		port_flow_aged(in->port, in->args.aged.destroy);
> +		break;
>  	default:
>  		break;
>  	}
> diff --git a/app/test-pmd/config.c b/app/test-pmd/config.c
> index 72f25d1521..035d336ab5 100644
> --- a/app/test-pmd/config.c
> +++ b/app/test-pmd/config.c
> @@ -1367,6 +1367,26 @@ port_flow_validate(portid_t port_id,
>  	return 0;
>  }
> 
> +/** Update age action context by port_flow pointer. */
> +void
> +update_age_action_context(const struct rte_flow_action *actions,
> +			struct port_flow *pf)
> +{
> +	struct rte_flow_action_age *age = NULL;
> +
> +	for (; actions->type != RTE_FLOW_ACTION_TYPE_END; actions++) {
> +		switch (actions->type) {
> +		case RTE_FLOW_ACTION_TYPE_AGE:
> +			age = (struct rte_flow_action_age *)
> +				(uintptr_t)actions->conf;
> +			age->context = pf;
> +			return;
> +		default:
> +			break;
> +		}
> +	}
> +}
> +
>  /** Create flow rule. */
>  int
>  port_flow_create(portid_t port_id,
> @@ -1377,28 +1397,27 @@ port_flow_create(portid_t port_id,
>  	struct rte_flow *flow;
>  	struct rte_port *port;
>  	struct port_flow *pf;
> -	uint32_t id;
> +	uint32_t id = 0;
>  	struct rte_flow_error error;
> 
> -	/* Poisoning to make sure PMDs update it in case of error. */
> -	memset(&error, 0x22, sizeof(error));
> -	flow = rte_flow_create(port_id, attr, pattern, actions, &error);
> -	if (!flow)
> -		return port_flow_complain(&error);
>  	port = &ports[port_id];
>  	if (port->flow_list) {
>  		if (port->flow_list->id == UINT32_MAX) {
>  			printf("Highest rule ID is already assigned, delete"
>  			       " it first");
> -			rte_flow_destroy(port_id, flow, NULL);
>  			return -ENOMEM;
>  		}
>  		id = port->flow_list->id + 1;
> -	} else
> -		id = 0;
> +	}
>  	pf = port_flow_new(attr, pattern, actions, &error);
> -	if (!pf) {
> -		rte_flow_destroy(port_id, flow, NULL);
> +	if (!pf)
> +		return port_flow_complain(&error);
> +	update_age_action_context(actions, pf);
> +	/* Poisoning to make sure PMDs update it in case of error. */
> +	memset(&error, 0x22, sizeof(error));
> +	flow = rte_flow_create(port_id, attr, pattern, actions, &error);
> +	if (!flow) {
> +		free(pf);
>  		return port_flow_complain(&error);
>  	}
>  	pf->next = port->flow_list;
> @@ -1570,6 +1589,73 @@ port_flow_query(portid_t port_id, uint32_t rule,
>  	return 0;
>  }
> 
> +/** List simply and destroy all aged flows. */
> +void
> +port_flow_aged(portid_t port_id, uint8_t destroy)
> +{
> +	void **contexts;
> +	int nb_context, total = 0, idx;
> +	struct rte_flow_error error;
> +	struct port_flow *pf;
> +
> +	if (port_id_is_invalid(port_id, ENABLED_WARN) ||
> +	    port_id == (portid_t)RTE_PORT_ALL)
> +		return;
> +	total = rte_flow_get_aged_flows(port_id, NULL, 0, &error);
> +	printf("Port %u total aged flows: %d\n", port_id, total);
> +	if (total < 0) {
> +		port_flow_complain(&error);
> +		return;
> +	}
> +	if (total == 0)
> +		return;
> +	contexts = malloc(sizeof(void *) * total);
> +	if (contexts == NULL) {
> +		printf("Cannot allocate contexts for aged flow\n");
> +		return;
> +	}
> +	printf("ID\tGroup\tPrio\tAttr\n");
> +	nb_context = rte_flow_get_aged_flows(port_id, contexts, total,
> &error);
> +	if (nb_context != total) {
> +		printf("Port:%d get aged flows count(%d) != total(%d)\n",
> +			port_id, nb_context, total);
> +		free(contexts);
> +		return;
> +	}
> +	for (idx = 0; idx < nb_context; idx++) {
> +		pf = (struct port_flow *)contexts[idx];
> +		if (!pf) {
> +			printf("Error: get Null context in port %u\n", port_id);
> +			continue;
> +		}
> +		printf("%" PRIu32 "\t%" PRIu32 "\t%" PRIu32 "\t%c%c%c\t\n",
> +		       pf->id,
> +		       pf->rule.attr->group,
> +		       pf->rule.attr->priority,
> +		       pf->rule.attr->ingress ? 'i' : '-',
> +		       pf->rule.attr->egress ? 'e' : '-',
> +		       pf->rule.attr->transfer ? 't' : '-');
> +	}
> +	if (destroy) {
> +		int ret;
> +		uint32_t flow_id;
> +
> +		total = 0;
> +		printf("\n");
> +		for (idx = 0; idx < nb_context; idx++) {
> +			pf = (struct port_flow *)contexts[idx];
> +			if (!pf)
> +				continue;
> +			flow_id = pf->id;
> +			ret = port_flow_destroy(port_id, 1, &flow_id);
> +			if (!ret)
> +				total++;
> +		}
> +		printf("%d flows be destroyed\n", total);
> +	}
> +	free(contexts);
> +}
> +
>  /** List flow rules. */
>  void
>  port_flow_list(portid_t port_id, uint32_t n, const uint32_t group[n])
> diff --git a/app/test-pmd/parameters.c b/app/test-pmd/parameters.c
> index 30c1753c32..92b5575626 100644
> --- a/app/test-pmd/parameters.c
> +++ b/app/test-pmd/parameters.c
> @@ -187,9 +187,9 @@ usage(char* progname)
>  	printf("  --no-rmv-interrupt: disable device removal interrupt.\n");
>  	printf("  --bitrate-stats=N: set the logical core N to perform "
>  		"bit-rate calculation.\n");
> -	printf("  --print-event
> <unknown|intr_lsc|queue_state|intr_reset|vf_mbox|macsec|intr_rmv|all>: "
> +	printf("  --print-event
> <unknown|intr_lsc|queue_state|intr_reset|vf_mbox|macsec|intr_rmv|flow_a
> ged|all>: "
>  	       "enable print of designated event or all of them.\n");
> -	printf("  --mask-event
> <unknown|intr_lsc|queue_state|intr_reset|vf_mbox|macsec|intr_rmv|all>: "
> +	printf("  --mask-event
> <unknown|intr_lsc|queue_state|intr_reset|vf_mbox|macsec|intr_rmv|flow_a
> ged|all>: "
>  	       "disable print of designated event or all of them.\n");
>  	printf("  --flow-isolate-all: "
>  	       "requests flow API isolated mode on all ports at initialization
> time.\n");
> @@ -545,6 +545,8 @@ parse_event_printing_config(const char *optarg, int
> enable)
>  		mask = UINT32_C(1) << RTE_ETH_EVENT_NEW;
>  	else if (!strcmp(optarg, "dev_released"))
>  		mask = UINT32_C(1) << RTE_ETH_EVENT_DESTROY;
> +	else if (!strcmp(optarg, "flow_aged"))
> +		mask = UINT32_C(1) << RTE_ETH_EVENT_FLOW_AGED;
>  	else if (!strcmp(optarg, "all"))
>  		mask = ~UINT32_C(0);
>  	else {
> diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c
> index 99bacddbfd..a2d0be56b3 100644
> --- a/app/test-pmd/testpmd.c
> +++ b/app/test-pmd/testpmd.c
> @@ -375,6 +375,7 @@ static const char * const eth_event_desc[] = {
>  	[RTE_ETH_EVENT_INTR_RMV] = "device removal",
>  	[RTE_ETH_EVENT_NEW] = "device probed",
>  	[RTE_ETH_EVENT_DESTROY] = "device released",
> +	[RTE_ETH_EVENT_FLOW_AGED] = "flow aged",
>  	[RTE_ETH_EVENT_MAX] = NULL,
>  };
> 
> @@ -388,7 +389,8 @@ uint32_t event_print_mask = (UINT32_C(1) <<
> RTE_ETH_EVENT_UNKNOWN) |
>  			    (UINT32_C(1) << RTE_ETH_EVENT_INTR_RESET) |
>  			    (UINT32_C(1) << RTE_ETH_EVENT_IPSEC) |
>  			    (UINT32_C(1) << RTE_ETH_EVENT_MACSEC) |
> -			    (UINT32_C(1) << RTE_ETH_EVENT_INTR_RMV);
> +			    (UINT32_C(1) << RTE_ETH_EVENT_INTR_RMV) |
> +			    (UINT32_C(1) << RTE_ETH_EVENT_FLOW_AGED);
>  /*
>   * Decide if all memory are locked for performance.
>   */
> diff --git a/app/test-pmd/testpmd.h b/app/test-pmd/testpmd.h
> index 7ff4c5dba3..fb391672a8 100644
> --- a/app/test-pmd/testpmd.h
> +++ b/app/test-pmd/testpmd.h
> @@ -747,12 +747,15 @@ 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);
> +void update_age_action_context(const struct rte_flow_action *actions,
> +		     struct port_flow *pf);
>  int port_flow_destroy(portid_t port_id, uint32_t n, const uint32_t *rule);
>  int port_flow_flush(portid_t port_id);
>  int port_flow_dump(portid_t port_id, const char *file_name);
>  int port_flow_query(portid_t port_id, uint32_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);
>  int port_flow_isolate(portid_t port_id, int set);
> 
>  void rx_ring_desc_display(portid_t port_id, queueid_t rxq_id, uint16_t rxd_id);
> diff --git a/doc/guides/testpmd_app_ug/testpmd_funcs.rst
> b/doc/guides/testpmd_app_ug/testpmd_funcs.rst
> index a360ecccfd..19260cc2d9 100644
> --- a/doc/guides/testpmd_app_ug/testpmd_funcs.rst
> +++ b/doc/guides/testpmd_app_ug/testpmd_funcs.rst
> @@ -3616,6 +3616,10 @@ following sections.
> 
>     flow dump {port_id} {output_file}
> 
> +- List and destroy aged flow rules::
> +
> +   flow aged {port_id} [destroy]
> +
>  Validating flow rules
>  ~~~~~~~~~~~~~~~~~~~~~
> 
> @@ -4503,6 +4507,64 @@ Otherwise, it will complain error occurred::
> 
>     Caught error type [...] ([...]): [...]
> 
> +Listing and destroying aged flow rules
> +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> +
> +``flow aged`` simply lists aged flow rules be get from api
> ``rte_flow_get_aged_flows``,
> +and ``destroy`` parameter can be used to destroy those flow rules in PMD.
> +
> +   flow aged {port_id} [destroy]
> +
> +Listing current aged flow rules::
> +
> +   testpmd> flow aged 0
> +   Port 0 total aged flows: 0
> +   testpmd> flow create 0 ingress pattern eth / ipv4 src is 2.2.2.14 / end
> +      actions age timeout 5 / queue index 0 /  end
> +   Flow rule #0 created
> +   testpmd> flow create 0 ingress pattern eth / ipv4 src is 2.2.2.15 / end
> +      actions age timeout 4 / queue index 0 /  end
> +   Flow rule #1 created
> +   testpmd> flow create 0 ingress pattern eth / ipv4 src is 2.2.2.16 / end
> +      actions age timeout 2 / queue index 0 /  end
> +   Flow rule #2 created
> +   testpmd> flow create 0 ingress pattern eth / ipv4 src is 2.2.2.17 / end
> +      actions age timeout 3 / queue index 0 /  end
> +   Flow rule #3 created
> +
> +
> +Aged Rules are simply list as command ``flow list {port_id}``, but strip the
> detail rule
> +information, all the aged flows are sorted by the longest timeout time. For
> example, if
> +those rules be configured in the same time, ID 2 will be the first aged out rule,
> the next
> +will be ID 3, ID 1, ID 0::
> +
> +   testpmd> flow aged 0
> +   Port 0 total aged flows: 4
> +   ID      Group   Prio    Attr
> +   2       0       0       i--
> +   3       0       0       i--
> +   1       0       0       i--
> +   0       0       0       i--
> +
> +If attach ``destroy`` parameter, the command will destroy all the list aged
> flow rules.
> +
> +   testpmd> flow aged 0 destroy
> +   Port 0 total aged flows: 4
> +   ID      Group   Prio    Attr
> +   2       0       0       i--
> +   3       0       0       i--
> +   1       0       0       i--
> +   0       0       0       i--
> +
> +   Flow rule #2 destroyed
> +   Flow rule #3 destroyed
> +   Flow rule #1 destroyed
> +   Flow rule #0 destroyed
> +   4 flows be destroyed
> +   testpmd> flow aged 0
> +   Port 0 total aged flows: 0
> +
> +
>  Sample QinQ flow rules
>  ~~~~~~~~~~~~~~~~~~~~~~
> 
> --
> 2.21.0

Re: [dpdk-dev] [PATCH v4] app/testpmd: support flow aging

[Ferruh Yigit]

On 5/3/2020 9:59 AM, Bill Zhou wrote:
> Currently, there is no way to check the aging event or to get the current
> aged flows in testpmd, this patch include those implements, it's included:
> 
> - Add new item "flow_aged" to the current print event command arguments.
> - Add new command to list all aged flows, meanwhile, we can set parameter
>   to destroy it.
> 
> Signed-off-by: Bill Zhou <dongz@mellanox.com>
> ---
> v2: Update the way of registering aging event, add new command to control
> if the event need be print or not. Update the output of the delete aged
> flow command format.
> v3: Change the command from only set aged flow output to set one gloable
> verbose bitmap for all events output.
> v4: Add the event output to current global print event arguments.

<...>

> @@ -187,9 +187,9 @@ usage(char* progname)
>  	printf("  --no-rmv-interrupt: disable device removal interrupt.\n");
>  	printf("  --bitrate-stats=N: set the logical core N to perform "
>  		"bit-rate calculation.\n");
> -	printf("  --print-event <unknown|intr_lsc|queue_state|intr_reset|vf_mbox|macsec|intr_rmv|all>: "
> +	printf("  --print-event <unknown|intr_lsc|queue_state|intr_reset|vf_mbox|macsec|intr_rmv|flow_aged|all>: "
>  	       "enable print of designated event or all of them.\n");
> -	printf("  --mask-event <unknown|intr_lsc|queue_state|intr_reset|vf_mbox|macsec|intr_rmv|all>: "
> +	printf("  --mask-event <unknown|intr_lsc|queue_state|intr_reset|vf_mbox|macsec|intr_rmv|flow_aged|all>: "

+1 to '--print-event', can you please update the documentation for the change?

<...>

> @@ -388,7 +389,8 @@ uint32_t event_print_mask = (UINT32_C(1) << RTE_ETH_EVENT_UNKNOWN) |
>  			    (UINT32_C(1) << RTE_ETH_EVENT_INTR_RESET) |
>  			    (UINT32_C(1) << RTE_ETH_EVENT_IPSEC) |
>  			    (UINT32_C(1) << RTE_ETH_EVENT_MACSEC) |
> -			    (UINT32_C(1) << RTE_ETH_EVENT_INTR_RMV);
> +			    (UINT32_C(1) << RTE_ETH_EVENT_INTR_RMV) |
> +			    (UINT32_C(1) << RTE_ETH_EVENT_FLOW_AGED);

This is enabling the event logging by default, we are turning back to original
point, since '--print-event' can be used to enable it, can you please leave it
out by default?

Re: [dpdk-dev] [PATCH v4] app/testpmd: support flow aging

[Matan Azrad]

From: Ferruh Yigit:
> On 5/3/2020 9:59 AM, Bill Zhou wrote:
> > Currently, there is no way to check the aging event or to get the
> > current aged flows in testpmd, this patch include those implements, it's
> included:
> >
> > - Add new item "flow_aged" to the current print event command
> arguments.
> > - Add new command to list all aged flows, meanwhile, we can set
> parameter
> >   to destroy it.
> >
> > Signed-off-by: Bill Zhou <dongz@mellanox.com>
> > ---
> > v2: Update the way of registering aging event, add new command to
> > control if the event need be print or not. Update the output of the
> > delete aged flow command format.
> > v3: Change the command from only set aged flow output to set one
> > gloable verbose bitmap for all events output.
> > v4: Add the event output to current global print event arguments.
> 
> <...>
> 
> > @@ -187,9 +187,9 @@ usage(char* progname)
> >  	printf("  --no-rmv-interrupt: disable device removal interrupt.\n");
> >  	printf("  --bitrate-stats=N: set the logical core N to perform "
> >  		"bit-rate calculation.\n");
> > -	printf("  --print-event
> <unknown|intr_lsc|queue_state|intr_reset|vf_mbox|macsec|intr_rmv|all
> >: "
> > +	printf("  --print-event
> <unknown|intr_lsc|queue_state|intr_reset|vf_mbox|macsec|intr_rmv|flo
> w_aged|all>: "
> >  	       "enable print of designated event or all of them.\n");
> > -	printf("  --mask-event
> <unknown|intr_lsc|queue_state|intr_reset|vf_mbox|macsec|intr_rmv|all
> >: "
> > +	printf("  --mask-event
> <unknown|intr_lsc|queue_state|intr_reset|vf_mbox|macsec|intr_rmv|flo
> w_aged|all>: "
> 
> +1 to '--print-event', can you please update the documentation for the
> change?

 +1

> <...>
> 
> > @@ -388,7 +389,8 @@ uint32_t event_print_mask = (UINT32_C(1) <<
> RTE_ETH_EVENT_UNKNOWN) |
> >  			    (UINT32_C(1) << RTE_ETH_EVENT_INTR_RESET) |
> >  			    (UINT32_C(1) << RTE_ETH_EVENT_IPSEC) |
> >  			    (UINT32_C(1) << RTE_ETH_EVENT_MACSEC) |
> > -			    (UINT32_C(1) << RTE_ETH_EVENT_INTR_RMV);
> > +			    (UINT32_C(1) << RTE_ETH_EVENT_INTR_RMV) |
> > +			    (UINT32_C(1) << RTE_ETH_EVENT_FLOW_AGED);
> 
> This is enabling the event logging by default, we are turning back to original
> point, since '--print-event' can be used to enable it, can you please leave it
> out by default?

The aged event is triggered only when there is at least one flow which is configured with AGE action,
Don't you think it should be printed by default if the user requested aging in a flow?

Re: [dpdk-dev] [PATCH v4] app/testpmd: support flow aging

[Ferruh Yigit]

On 5/5/2020 10:11 AM, Matan Azrad wrote:
> 
> 
> From: Ferruh Yigit:
>> On 5/3/2020 9:59 AM, Bill Zhou wrote:
>>> Currently, there is no way to check the aging event or to get the
>>> current aged flows in testpmd, this patch include those implements, it's
>> included:
>>>
>>> - Add new item "flow_aged" to the current print event command
>> arguments.
>>> - Add new command to list all aged flows, meanwhile, we can set
>> parameter
>>>   to destroy it.
>>>
>>> Signed-off-by: Bill Zhou <dongz@mellanox.com>
>>> ---
>>> v2: Update the way of registering aging event, add new command to
>>> control if the event need be print or not. Update the output of the
>>> delete aged flow command format.
>>> v3: Change the command from only set aged flow output to set one
>>> gloable verbose bitmap for all events output.
>>> v4: Add the event output to current global print event arguments.
>>
>> <...>
>>
>>> @@ -187,9 +187,9 @@ usage(char* progname)
>>>  	printf("  --no-rmv-interrupt: disable device removal interrupt.\n");
>>>  	printf("  --bitrate-stats=N: set the logical core N to perform "
>>>  		"bit-rate calculation.\n");
>>> -	printf("  --print-event
>> <unknown|intr_lsc|queue_state|intr_reset|vf_mbox|macsec|intr_rmv|all
>>> : "
>>> +	printf("  --print-event
>> <unknown|intr_lsc|queue_state|intr_reset|vf_mbox|macsec|intr_rmv|flo
>> w_aged|all>: "
>>>  	       "enable print of designated event or all of them.\n");
>>> -	printf("  --mask-event
>> <unknown|intr_lsc|queue_state|intr_reset|vf_mbox|macsec|intr_rmv|all
>>> : "
>>> +	printf("  --mask-event
>> <unknown|intr_lsc|queue_state|intr_reset|vf_mbox|macsec|intr_rmv|flo
>> w_aged|all>: "
>>
>> +1 to '--print-event', can you please update the documentation for the
>> change?
> 
>  +1
> 
>> <...>
>>
>>> @@ -388,7 +389,8 @@ uint32_t event_print_mask = (UINT32_C(1) <<
>> RTE_ETH_EVENT_UNKNOWN) |
>>>  			    (UINT32_C(1) << RTE_ETH_EVENT_INTR_RESET) |
>>>  			    (UINT32_C(1) << RTE_ETH_EVENT_IPSEC) |
>>>  			    (UINT32_C(1) << RTE_ETH_EVENT_MACSEC) |
>>> -			    (UINT32_C(1) << RTE_ETH_EVENT_INTR_RMV);
>>> +			    (UINT32_C(1) << RTE_ETH_EVENT_INTR_RMV) |
>>> +			    (UINT32_C(1) << RTE_ETH_EVENT_FLOW_AGED);
>>
>> This is enabling the event logging by default, we are turning back to original
>> point, since '--print-event' can be used to enable it, can you please leave it
>> out by default?
> 
> The aged event is triggered only when there is at least one flow which is configured with AGE action,
> Don't you think it should be printed by default if the user requested aging in a flow?
> 
Good point, OK to keep as it is.
So only above doc update please and feel free to keep existing acks.

[dpdk-dev] [PATCH v5] app/testpmd: support flow aging

[Bill Zhou]

Currently, there is no way to check the aging event or to get the current
aged flows in testpmd, this patch include those implements, it's included:

- Add new item "flow_aged" to the current print event command arguments.
- Add new command to list all aged flows, meanwhile, we can set parameter
  to destroy it.

Signed-off-by: Bill Zhou <dongz@mellanox.com>
---
v2: Update the way of registering aging event, add new command to control
if the event need be print or not. Update the output of the delete aged
flow command format.
v3: Change the command from only set aged flow output to set one gloable
verbose bitmap for all events output.
v4: Add the event output to current global print event arguments.
v5: Update the documentation about print event command line change.
 app/test-pmd/cmdline.c                      |   4 +
 app/test-pmd/cmdline_flow.c                 |  62 +++++++++++
 app/test-pmd/config.c                       | 108 ++++++++++++++++++--
 app/test-pmd/parameters.c                   |   6 +-
 app/test-pmd/testpmd.c                      |   4 +-
 app/test-pmd/testpmd.h                      |   3 +
 doc/guides/testpmd_app_ug/run_app.rst       |   4 +-
 doc/guides/testpmd_app_ug/testpmd_funcs.rst |  62 +++++++++++
 8 files changed, 237 insertions(+), 16 deletions(-)

diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c
index 1375f223eb..bcf9080c48 100644
--- a/app/test-pmd/cmdline.c
+++ b/app/test-pmd/cmdline.c
@@ -1125,6 +1125,10 @@ static void cmd_help_long_parsed(void *parsed_result,
 			"    Restrict ingress traffic to the defined"
 			" flow rules\n\n"
 
+			"flow aged {port_id} [destroy]\n"
+			"    List and destroy aged flows"
+			" flow rules\n\n"
+
 			"set vxlan ip-version (ipv4|ipv6) vni (vni) udp-src"
 			" (udp-src) udp-dst (udp-dst) ip-src (ip-src) ip-dst"
 			" (ip-dst) eth-src (eth-src) eth-dst (eth-dst)\n"
diff --git a/app/test-pmd/cmdline_flow.c b/app/test-pmd/cmdline_flow.c
index 45bcff3cf5..4e2006c543 100644
--- a/app/test-pmd/cmdline_flow.c
+++ b/app/test-pmd/cmdline_flow.c
@@ -67,6 +67,7 @@ enum index {
 	DUMP,
 	QUERY,
 	LIST,
+	AGED,
 	ISOLATE,
 
 	/* Destroy arguments. */
@@ -78,6 +79,9 @@ enum index {
 	/* List arguments. */
 	LIST_GROUP,
 
+	/* Destroy aged flow arguments. */
+	AGED_DESTROY,
+
 	/* Validate/create arguments. */
 	GROUP,
 	PRIORITY,
@@ -664,6 +668,9 @@ struct buffer {
 		struct {
 			int set;
 		} isolate; /**< Isolated mode arguments. */
+		struct {
+			int destroy;
+		} aged; /**< Aged arguments. */
 	} args; /**< Command arguments. */
 };
 
@@ -719,6 +726,12 @@ static const enum index next_list_attr[] = {
 	ZERO,
 };
 
+static const enum index next_aged_attr[] = {
+	AGED_DESTROY,
+	END,
+	ZERO,
+};
+
 static const enum index item_param[] = {
 	ITEM_PARAM_IS,
 	ITEM_PARAM_SPEC,
@@ -1466,6 +1479,9 @@ static int parse_action(struct context *, const struct token *,
 static int parse_list(struct context *, const struct token *,
 		      const char *, unsigned int,
 		      void *, unsigned int);
+static int parse_aged(struct context *, const struct token *,
+		      const char *, unsigned int,
+		      void *, unsigned int);
 static int parse_isolate(struct context *, const struct token *,
 			 const char *, unsigned int,
 			 void *, unsigned int);
@@ -1649,6 +1665,7 @@ static const struct token token_list[] = {
 			      FLUSH,
 			      DUMP,
 			      LIST,
+			      AGED,
 			      QUERY,
 			      ISOLATE)),
 		.call = parse_init,
@@ -1708,6 +1725,13 @@ static const struct token token_list[] = {
 		.args = ARGS(ARGS_ENTRY(struct buffer, port)),
 		.call = parse_list,
 	},
+	[AGED] = {
+		.name = "aged",
+		.help = "list and destroy aged flows",
+		.next = NEXT(next_aged_attr, NEXT_ENTRY(PORT_ID)),
+		.args = ARGS(ARGS_ENTRY(struct buffer, port)),
+		.call = parse_aged,
+	},
 	[ISOLATE] = {
 		.name = "isolate",
 		.help = "restrict ingress traffic to the defined flow rules",
@@ -1741,6 +1765,12 @@ static const struct token token_list[] = {
 		.args = ARGS(ARGS_ENTRY_PTR(struct buffer, args.list.group)),
 		.call = parse_list,
 	},
+	[AGED_DESTROY] = {
+		.name = "destroy",
+		.help = "specify aged flows need be destroyed",
+		.call = parse_aged,
+		.comp = comp_none,
+	},
 	/* Validate/create attributes. */
 	[GROUP] = {
 		.name = "group",
@@ -5367,6 +5397,35 @@ parse_list(struct context *ctx, const struct token *token,
 	return len;
 }
 
+/** Parse tokens for list all aged flows command. */
+static int
+parse_aged(struct context *ctx, const struct token *token,
+	   const char *str, unsigned int len,
+	   void *buf, unsigned int size)
+{
+	struct buffer *out = buf;
+
+	/* Token name must match. */
+	if (parse_default(ctx, token, str, len, NULL, 0) < 0)
+		return -1;
+	/* Nothing else to do if there is no buffer. */
+	if (!out)
+		return len;
+	if (!out->command) {
+		if (ctx->curr != AGED)
+			return -1;
+		if (sizeof(*out) > size)
+			return -1;
+		out->command = ctx->curr;
+		ctx->objdata = 0;
+		ctx->object = out;
+		ctx->objmask = NULL;
+	}
+	if (ctx->curr == AGED_DESTROY)
+		out->args.aged.destroy = 1;
+	return len;
+}
+
 /** Parse tokens for isolate command. */
 static int
 parse_isolate(struct context *ctx, const struct token *token,
@@ -6367,6 +6426,9 @@ cmd_flow_parsed(const struct buffer *in)
 	case ISOLATE:
 		port_flow_isolate(in->port, in->args.isolate.set);
 		break;
+	case AGED:
+		port_flow_aged(in->port, in->args.aged.destroy);
+		break;
 	default:
 		break;
 	}
diff --git a/app/test-pmd/config.c b/app/test-pmd/config.c
index 72f25d1521..035d336ab5 100644
--- a/app/test-pmd/config.c
+++ b/app/test-pmd/config.c
@@ -1367,6 +1367,26 @@ port_flow_validate(portid_t port_id,
 	return 0;
 }
 
+/** Update age action context by port_flow pointer. */
+void
+update_age_action_context(const struct rte_flow_action *actions,
+			struct port_flow *pf)
+{
+	struct rte_flow_action_age *age = NULL;
+
+	for (; actions->type != RTE_FLOW_ACTION_TYPE_END; actions++) {
+		switch (actions->type) {
+		case RTE_FLOW_ACTION_TYPE_AGE:
+			age = (struct rte_flow_action_age *)
+				(uintptr_t)actions->conf;
+			age->context = pf;
+			return;
+		default:
+			break;
+		}
+	}
+}
+
 /** Create flow rule. */
 int
 port_flow_create(portid_t port_id,
@@ -1377,28 +1397,27 @@ port_flow_create(portid_t port_id,
 	struct rte_flow *flow;
 	struct rte_port *port;
 	struct port_flow *pf;
-	uint32_t id;
+	uint32_t id = 0;
 	struct rte_flow_error error;
 
-	/* Poisoning to make sure PMDs update it in case of error. */
-	memset(&error, 0x22, sizeof(error));
-	flow = rte_flow_create(port_id, attr, pattern, actions, &error);
-	if (!flow)
-		return port_flow_complain(&error);
 	port = &ports[port_id];
 	if (port->flow_list) {
 		if (port->flow_list->id == UINT32_MAX) {
 			printf("Highest rule ID is already assigned, delete"
 			       " it first");
-			rte_flow_destroy(port_id, flow, NULL);
 			return -ENOMEM;
 		}
 		id = port->flow_list->id + 1;
-	} else
-		id = 0;
+	}
 	pf = port_flow_new(attr, pattern, actions, &error);
-	if (!pf) {
-		rte_flow_destroy(port_id, flow, NULL);
+	if (!pf)
+		return port_flow_complain(&error);
+	update_age_action_context(actions, pf);
+	/* Poisoning to make sure PMDs update it in case of error. */
+	memset(&error, 0x22, sizeof(error));
+	flow = rte_flow_create(port_id, attr, pattern, actions, &error);
+	if (!flow) {
+		free(pf);
 		return port_flow_complain(&error);
 	}
 	pf->next = port->flow_list;
@@ -1570,6 +1589,73 @@ port_flow_query(portid_t port_id, uint32_t rule,
 	return 0;
 }
 
+/** List simply and destroy all aged flows. */
+void
+port_flow_aged(portid_t port_id, uint8_t destroy)
+{
+	void **contexts;
+	int nb_context, total = 0, idx;
+	struct rte_flow_error error;
+	struct port_flow *pf;
+
+	if (port_id_is_invalid(port_id, ENABLED_WARN) ||
+	    port_id == (portid_t)RTE_PORT_ALL)
+		return;
+	total = rte_flow_get_aged_flows(port_id, NULL, 0, &error);
+	printf("Port %u total aged flows: %d\n", port_id, total);
+	if (total < 0) {
+		port_flow_complain(&error);
+		return;
+	}
+	if (total == 0)
+		return;
+	contexts = malloc(sizeof(void *) * total);
+	if (contexts == NULL) {
+		printf("Cannot allocate contexts for aged flow\n");
+		return;
+	}
+	printf("ID\tGroup\tPrio\tAttr\n");
+	nb_context = rte_flow_get_aged_flows(port_id, contexts, total, &error);
+	if (nb_context != total) {
+		printf("Port:%d get aged flows count(%d) != total(%d)\n",
+			port_id, nb_context, total);
+		free(contexts);
+		return;
+	}
+	for (idx = 0; idx < nb_context; idx++) {
+		pf = (struct port_flow *)contexts[idx];
+		if (!pf) {
+			printf("Error: get Null context in port %u\n", port_id);
+			continue;
+		}
+		printf("%" PRIu32 "\t%" PRIu32 "\t%" PRIu32 "\t%c%c%c\t\n",
+		       pf->id,
+		       pf->rule.attr->group,
+		       pf->rule.attr->priority,
+		       pf->rule.attr->ingress ? 'i' : '-',
+		       pf->rule.attr->egress ? 'e' : '-',
+		       pf->rule.attr->transfer ? 't' : '-');
+	}
+	if (destroy) {
+		int ret;
+		uint32_t flow_id;
+
+		total = 0;
+		printf("\n");
+		for (idx = 0; idx < nb_context; idx++) {
+			pf = (struct port_flow *)contexts[idx];
+			if (!pf)
+				continue;
+			flow_id = pf->id;
+			ret = port_flow_destroy(port_id, 1, &flow_id);
+			if (!ret)
+				total++;
+		}
+		printf("%d flows be destroyed\n", total);
+	}
+	free(contexts);
+}
+
 /** List flow rules. */
 void
 port_flow_list(portid_t port_id, uint32_t n, const uint32_t group[n])
diff --git a/app/test-pmd/parameters.c b/app/test-pmd/parameters.c
index 30c1753c32..92b5575626 100644
--- a/app/test-pmd/parameters.c
+++ b/app/test-pmd/parameters.c
@@ -187,9 +187,9 @@ usage(char* progname)
 	printf("  --no-rmv-interrupt: disable device removal interrupt.\n");
 	printf("  --bitrate-stats=N: set the logical core N to perform "
 		"bit-rate calculation.\n");
-	printf("  --print-event <unknown|intr_lsc|queue_state|intr_reset|vf_mbox|macsec|intr_rmv|all>: "
+	printf("  --print-event <unknown|intr_lsc|queue_state|intr_reset|vf_mbox|macsec|intr_rmv|flow_aged|all>: "
 	       "enable print of designated event or all of them.\n");
-	printf("  --mask-event <unknown|intr_lsc|queue_state|intr_reset|vf_mbox|macsec|intr_rmv|all>: "
+	printf("  --mask-event <unknown|intr_lsc|queue_state|intr_reset|vf_mbox|macsec|intr_rmv|flow_aged|all>: "
 	       "disable print of designated event or all of them.\n");
 	printf("  --flow-isolate-all: "
 	       "requests flow API isolated mode on all ports at initialization time.\n");
@@ -545,6 +545,8 @@ parse_event_printing_config(const char *optarg, int enable)
 		mask = UINT32_C(1) << RTE_ETH_EVENT_NEW;
 	else if (!strcmp(optarg, "dev_released"))
 		mask = UINT32_C(1) << RTE_ETH_EVENT_DESTROY;
+	else if (!strcmp(optarg, "flow_aged"))
+		mask = UINT32_C(1) << RTE_ETH_EVENT_FLOW_AGED;
 	else if (!strcmp(optarg, "all"))
 		mask = ~UINT32_C(0);
 	else {
diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c
index 99bacddbfd..a2d0be56b3 100644
--- a/app/test-pmd/testpmd.c
+++ b/app/test-pmd/testpmd.c
@@ -375,6 +375,7 @@ static const char * const eth_event_desc[] = {
 	[RTE_ETH_EVENT_INTR_RMV] = "device removal",
 	[RTE_ETH_EVENT_NEW] = "device probed",
 	[RTE_ETH_EVENT_DESTROY] = "device released",
+	[RTE_ETH_EVENT_FLOW_AGED] = "flow aged",
 	[RTE_ETH_EVENT_MAX] = NULL,
 };
 
@@ -388,7 +389,8 @@ uint32_t event_print_mask = (UINT32_C(1) << RTE_ETH_EVENT_UNKNOWN) |
 			    (UINT32_C(1) << RTE_ETH_EVENT_INTR_RESET) |
 			    (UINT32_C(1) << RTE_ETH_EVENT_IPSEC) |
 			    (UINT32_C(1) << RTE_ETH_EVENT_MACSEC) |
-			    (UINT32_C(1) << RTE_ETH_EVENT_INTR_RMV);
+			    (UINT32_C(1) << RTE_ETH_EVENT_INTR_RMV) |
+			    (UINT32_C(1) << RTE_ETH_EVENT_FLOW_AGED);
 /*
  * Decide if all memory are locked for performance.
  */
diff --git a/app/test-pmd/testpmd.h b/app/test-pmd/testpmd.h
index 7ff4c5dba3..fb391672a8 100644
--- a/app/test-pmd/testpmd.h
+++ b/app/test-pmd/testpmd.h
@@ -747,12 +747,15 @@ 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);
+void update_age_action_context(const struct rte_flow_action *actions,
+		     struct port_flow *pf);
 int port_flow_destroy(portid_t port_id, uint32_t n, const uint32_t *rule);
 int port_flow_flush(portid_t port_id);
 int port_flow_dump(portid_t port_id, const char *file_name);
 int port_flow_query(portid_t port_id, uint32_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);
 int port_flow_isolate(portid_t port_id, int set);
 
 void rx_ring_desc_display(portid_t port_id, queueid_t rxq_id, uint16_t rxd_id);
diff --git a/doc/guides/testpmd_app_ug/run_app.rst b/doc/guides/testpmd_app_ug/run_app.rst
index 727ef52b8f..71213361b4 100644
--- a/doc/guides/testpmd_app_ug/run_app.rst
+++ b/doc/guides/testpmd_app_ug/run_app.rst
@@ -388,12 +388,12 @@ The command line options are:
 
     Set the logical core N to perform bitrate calculation.
 
-*   ``--print-event <unknown|intr_lsc|queue_state|intr_reset|vf_mbox|macsec|intr_rmv|dev_probed|dev_released|all>``
+*   ``--print-event <unknown|intr_lsc|queue_state|intr_reset|vf_mbox|macsec|intr_rmv|dev_probed|dev_released|flow_aged|all>``
 
     Enable printing the occurrence of the designated event. Using all will
     enable all of them.
 
-*   ``--mask-event <unknown|intr_lsc|queue_state|intr_reset|vf_mbox|macsec|intr_rmv|dev_probed|dev_released|all>``
+*   ``--mask-event <unknown|intr_lsc|queue_state|intr_reset|vf_mbox|macsec|intr_rmv|dev_probed|dev_released|flow_aged|all>``
 
     Disable printing the occurrence of the designated event. Using all will
     disable all of them.
diff --git a/doc/guides/testpmd_app_ug/testpmd_funcs.rst b/doc/guides/testpmd_app_ug/testpmd_funcs.rst
index a360ecccfd..19260cc2d9 100644
--- a/doc/guides/testpmd_app_ug/testpmd_funcs.rst
+++ b/doc/guides/testpmd_app_ug/testpmd_funcs.rst
@@ -3616,6 +3616,10 @@ following sections.
 
    flow dump {port_id} {output_file}
 
+- List and destroy aged flow rules::
+
+   flow aged {port_id} [destroy]
+
 Validating flow rules
 ~~~~~~~~~~~~~~~~~~~~~
 
@@ -4503,6 +4507,64 @@ Otherwise, it will complain error occurred::
 
    Caught error type [...] ([...]): [...]
 
+Listing and destroying aged flow rules
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+
+``flow aged`` simply lists aged flow rules be get from api ``rte_flow_get_aged_flows``,
+and ``destroy`` parameter can be used to destroy those flow rules in PMD.
+
+   flow aged {port_id} [destroy]
+
+Listing current aged flow rules::
+
+   testpmd> flow aged 0
+   Port 0 total aged flows: 0
+   testpmd> flow create 0 ingress pattern eth / ipv4 src is 2.2.2.14 / end
+      actions age timeout 5 / queue index 0 /  end
+   Flow rule #0 created
+   testpmd> flow create 0 ingress pattern eth / ipv4 src is 2.2.2.15 / end
+      actions age timeout 4 / queue index 0 /  end
+   Flow rule #1 created
+   testpmd> flow create 0 ingress pattern eth / ipv4 src is 2.2.2.16 / end
+      actions age timeout 2 / queue index 0 /  end
+   Flow rule #2 created
+   testpmd> flow create 0 ingress pattern eth / ipv4 src is 2.2.2.17 / end
+      actions age timeout 3 / queue index 0 /  end
+   Flow rule #3 created
+
+
+Aged Rules are simply list as command ``flow list {port_id}``, but strip the detail rule
+information, all the aged flows are sorted by the longest timeout time. For example, if
+those rules be configured in the same time, ID 2 will be the first aged out rule, the next
+will be ID 3, ID 1, ID 0::
+
+   testpmd> flow aged 0
+   Port 0 total aged flows: 4
+   ID      Group   Prio    Attr
+   2       0       0       i--
+   3       0       0       i--
+   1       0       0       i--
+   0       0       0       i--
+
+If attach ``destroy`` parameter, the command will destroy all the list aged flow rules.
+
+   testpmd> flow aged 0 destroy
+   Port 0 total aged flows: 4
+   ID      Group   Prio    Attr
+   2       0       0       i--
+   3       0       0       i--
+   1       0       0       i--
+   0       0       0       i--
+
+   Flow rule #2 destroyed
+   Flow rule #3 destroyed
+   Flow rule #1 destroyed
+   Flow rule #0 destroyed
+   4 flows be destroyed
+   testpmd> flow aged 0
+   Port 0 total aged flows: 0
+
+
 Sample QinQ flow rules
 ~~~~~~~~~~~~~~~~~~~~~~
 
-- 
2.21.0

Re: [dpdk-dev] [PATCH v5] app/testpmd: support flow aging

[Ori Kam]

> -----Original Message-----
> From: Bill Zhou <dongz@mellanox.com>
> Sent: Tuesday, May 5, 2020 12:49 PM
> Subject: [PATCH v5] app/testpmd: support flow aging
> 
> Currently, there is no way to check the aging event or to get the current
> aged flows in testpmd, this patch include those implements, it's included:
> 
> - Add new item "flow_aged" to the current print event command arguments.
> - Add new command to list all aged flows, meanwhile, we can set parameter
>   to destroy it.
> 
> Signed-off-by: Bill Zhou <dongz@mellanox.com>
> ---
> v2: Update the way of registering aging event, add new command to control
> if the event need be print or not. Update the output of the delete aged
> flow command format.
> v3: Change the command from only set aged flow output to set one gloable
> verbose bitmap for all events output.
> v4: Add the event output to current global print event arguments.
> v5: Update the documentation about print event command line change.
>  app/test-pmd/cmdline.c                      |   4 +
>  app/test-pmd/cmdline_flow.c                 |  62 +++++++++++
>  app/test-pmd/config.c                       | 108 ++++++++++++++++++--
>  app/test-pmd/parameters.c                   |   6 +-
>  app/test-pmd/testpmd.c                      |   4 +-
>  app/test-pmd/testpmd.h                      |   3 +
>  doc/guides/testpmd_app_ug/run_app.rst       |   4 +-
>  doc/guides/testpmd_app_ug/testpmd_funcs.rst |  62 +++++++++++
>  8 files changed, 237 insertions(+), 16 deletions(-)
> 


Acked-by: Ori Kam <orika@mellanox.com>
Thanks,
Ori

Re: [dpdk-dev] [PATCH v5] app/testpmd: support flow aging

[Ferruh Yigit]

On 5/5/2020 11:09 AM, Ori Kam wrote:
> 
> 
>> -----Original Message-----
>> From: Bill Zhou <dongz@mellanox.com>
>> Sent: Tuesday, May 5, 2020 12:49 PM
>> Subject: [PATCH v5] app/testpmd: support flow aging
>>
>> Currently, there is no way to check the aging event or to get the current
>> aged flows in testpmd, this patch include those implements, it's included:
>>
>> - Add new item "flow_aged" to the current print event command arguments.
>> - Add new command to list all aged flows, meanwhile, we can set parameter
>>   to destroy it.
>>
>> Signed-off-by: Bill Zhou <dongz@mellanox.com>
>> ---
>> v2: Update the way of registering aging event, add new command to control
>> if the event need be print or not. Update the output of the delete aged
>> flow command format.
>> v3: Change the command from only set aged flow output to set one gloable
>> verbose bitmap for all events output.
>> v4: Add the event output to current global print event arguments.
>> v5: Update the documentation about print event command line change.
>>  app/test-pmd/cmdline.c                      |   4 +
>>  app/test-pmd/cmdline_flow.c                 |  62 +++++++++++
>>  app/test-pmd/config.c                       | 108 ++++++++++++++++++--
>>  app/test-pmd/parameters.c                   |   6 +-
>>  app/test-pmd/testpmd.c                      |   4 +-
>>  app/test-pmd/testpmd.h                      |   3 +
>>  doc/guides/testpmd_app_ug/run_app.rst       |   4 +-
>>  doc/guides/testpmd_app_ug/testpmd_funcs.rst |  62 +++++++++++
>>  8 files changed, 237 insertions(+), 16 deletions(-)
>>
> 
> Acked-by: Ori Kam <orika@mellanox.com>

Reviewed-by: Ferruh Yigit <ferruh.yigit@intel.com>

Applied to dpdk-next-net/master, thanks.

Re: [dpdk-dev] [PATCH v5] app/testpmd: support flow aging

[Matan Azrad]

From: Ferruh Yigit:
> On 5/5/2020 11:09 AM, Ori Kam wrote:
> >
> >
> >> -----Original Message-----
> >> From: Bill Zhou <dongz@mellanox.com>
> >> Sent: Tuesday, May 5, 2020 12:49 PM
> >> Subject: [PATCH v5] app/testpmd: support flow aging
> >>
> >> Currently, there is no way to check the aging event or to get the
> >> current aged flows in testpmd, this patch include those implements, it's
> included:
> >>
> >> - Add new item "flow_aged" to the current print event command
> arguments.
> >> - Add new command to list all aged flows, meanwhile, we can set
> parameter
> >>   to destroy it.
> >>
> >> Signed-off-by: Bill Zhou <dongz@mellanox.com>
> >> ---
> >> v2: Update the way of registering aging event, add new command to
> >> control if the event need be print or not. Update the output of the
> >> delete aged flow command format.
> >> v3: Change the command from only set aged flow output to set one
> >> gloable verbose bitmap for all events output.
> >> v4: Add the event output to current global print event arguments.
> >> v5: Update the documentation about print event command line change.
> >>  app/test-pmd/cmdline.c                      |   4 +
> >>  app/test-pmd/cmdline_flow.c                 |  62 +++++++++++
> >>  app/test-pmd/config.c                       | 108 ++++++++++++++++++--
> >>  app/test-pmd/parameters.c                   |   6 +-
> >>  app/test-pmd/testpmd.c                      |   4 +-
> >>  app/test-pmd/testpmd.h                      |   3 +
> >>  doc/guides/testpmd_app_ug/run_app.rst       |   4 +-
> >>  doc/guides/testpmd_app_ug/testpmd_funcs.rst |  62 +++++++++++
> >>  8 files changed, 237 insertions(+), 16 deletions(-)
> >>
> >
> > Acked-by: Ori Kam <orika@mellanox.com>
> 
> Reviewed-by: Ferruh Yigit <ferruh.yigit@intel.com>
Acked-by: Matan Azrad <matan@mellanox.com>
> Applied to dpdk-next-net/master, thanks.