DPDK patches and discussions
 help / color / mirror / Atom feed
* [PATCH 0/3] ethdev: AGE action preparation
@ 2022-09-21 14:54 Michael Baum
  2022-09-21 14:54 ` [PATCH 1/3] ethdev: add strict queue to pre-configuration flow hints Michael Baum
                   ` (4 more replies)
  0 siblings, 5 replies; 32+ messages in thread
From: Michael Baum @ 2022-09-21 14:54 UTC (permalink / raw)
  To: dev; +Cc: Matan Azrad, Raslan Darawsheh, Ori Kam

RFC's:
https://patchwork.dpdk.org/project/dpdk/patch/7a45693f478b1b721b4e05131141b526185a175c.1654063912.git.jackmin@nvidia.com/
https://patchwork.dpdk.org/project/dpdk/patch/608febf8d5d3c434a1eddb2e56f425ebbd6ff0b4.1654063912.git.jackmin@nvidia.com/

Michael Baum (3):
  ethdev: add strict queue to pre-configuration flow hints
  ethdev: add queue-based API to report aged flow rules
  ethdev: add structure for indirect AGE update

 app/test-pmd/cmdline_flow.c                 |  93 +++++++++-
 app/test-pmd/config.c                       | 177 +++++++++++++++++++-
 app/test-pmd/testpmd.h                      |   1 +
 doc/guides/prog_guide/rte_flow.rst          |  25 ++-
 doc/guides/testpmd_app_ug/testpmd_funcs.rst |  90 +++++++++-
 lib/ethdev/rte_flow.c                       |  22 +++
 lib/ethdev/rte_flow.h                       |  89 +++++++++-
 lib/ethdev/rte_flow_driver.h                |   7 +
 lib/ethdev/version.map                      |   3 +
 9 files changed, 491 insertions(+), 16 deletions(-)

-- 
2.25.1


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

* [PATCH 1/3] ethdev: add strict queue to pre-configuration flow hints
  2022-09-21 14:54 [PATCH 0/3] ethdev: AGE action preparation Michael Baum
@ 2022-09-21 14:54 ` Michael Baum
  2022-09-29 12:04   ` Ori Kam
  2022-09-21 14:54 ` [PATCH 2/3] ethdev: add queue-based API to report aged flow rules Michael Baum
                   ` (3 subsequent siblings)
  4 siblings, 1 reply; 32+ messages in thread
From: Michael Baum @ 2022-09-21 14:54 UTC (permalink / raw)
  To: dev; +Cc: Matan Azrad, Raslan Darawsheh, Ori Kam

The data-path focused flow rule management can manage flow rules in more
optimized way than traditional one by using hints provided by
application in initialization phase.

In addition to the current hints we have in port attr, more hints could
be provided by application about its behaviour.

One example is how the application do with the same flow rule ?
A. create/destroy flow on same queue but query flow on different queue
   or queue-less way (i.e, counter query)
B. All flow operations will be exactly on the same queue, by which PMD
   could be in more optimized way then A because resource could be
   isolated and access based on queue, without lock, for example.

This patch add flag about above situation and could be extended to cover
more situations.

Signed-off-by: Michael Baum <michaelba@nvidia.com>
---
 app/test-pmd/cmdline_flow.c                 | 10 ++++++++++
 doc/guides/testpmd_app_ug/testpmd_funcs.rst |  4 ++--
 lib/ethdev/rte_flow.h                       | 14 ++++++++++++++
 3 files changed, 26 insertions(+), 2 deletions(-)

diff --git a/app/test-pmd/cmdline_flow.c b/app/test-pmd/cmdline_flow.c
index 7f50028eb7..a982083d27 100644
--- a/app/test-pmd/cmdline_flow.c
+++ b/app/test-pmd/cmdline_flow.c
@@ -219,6 +219,7 @@ enum index {
 	CONFIG_COUNTERS_NUMBER,
 	CONFIG_AGING_OBJECTS_NUMBER,
 	CONFIG_METERS_NUMBER,
+	CONFIG_FLAGS,
 
 	/* Indirect action arguments */
 	INDIRECT_ACTION_CREATE,
@@ -1081,6 +1082,7 @@ static const enum index next_config_attr[] = {
 	CONFIG_COUNTERS_NUMBER,
 	CONFIG_AGING_OBJECTS_NUMBER,
 	CONFIG_METERS_NUMBER,
+	CONFIG_FLAGS,
 	END,
 	ZERO,
 };
@@ -2667,6 +2669,14 @@ static const struct token token_list[] = {
 		.args = ARGS(ARGS_ENTRY(struct buffer,
 					args.configure.port_attr.nb_meters)),
 	},
+	[CONFIG_FLAGS] = {
+		.name = "flags",
+		.help = "configuration flags",
+		.next = NEXT(next_config_attr,
+			     NEXT_ENTRY(COMMON_UNSIGNED)),
+		.args = ARGS(ARGS_ENTRY(struct buffer,
+					args.configure.port_attr.flags)),
+	},
 	/* Top-level command. */
 	[PATTERN_TEMPLATE] = {
 		.name = "pattern_template",
diff --git a/doc/guides/testpmd_app_ug/testpmd_funcs.rst b/doc/guides/testpmd_app_ug/testpmd_funcs.rst
index 330e34427d..6c12e0286c 100644
--- a/doc/guides/testpmd_app_ug/testpmd_funcs.rst
+++ b/doc/guides/testpmd_app_ug/testpmd_funcs.rst
@@ -3082,7 +3082,7 @@ following sections.
        [queues_number {number}] [queues_size {size}]
        [counters_number {number}]
        [aging_counters_number {number}]
-       [meters_number {number}]
+       [meters_number {number}] [flags {number}]
 
 - Create a pattern template::
    flow pattern_template {port_id} create [pattern_template_id {id}]
@@ -3233,7 +3233,7 @@ for asynchronous flow creation/destruction operations. It is bound to
        [queues_number {number}] [queues_size {size}]
        [counters_number {number}]
        [aging_counters_number {number}]
-       [meters_number {number}]
+       [meters_number {number}] [flags {number}]
 
 If successful, it will show::
 
diff --git a/lib/ethdev/rte_flow.h b/lib/ethdev/rte_flow.h
index a79f1e7ef0..c552771472 100644
--- a/lib/ethdev/rte_flow.h
+++ b/lib/ethdev/rte_flow.h
@@ -4874,6 +4874,12 @@ rte_flow_flex_item_release(uint16_t port_id,
 			   const struct rte_flow_item_flex_handle *handle,
 			   struct rte_flow_error *error);
 
+/**
+ * Indicate all operations for a given flow rule will _strictly_
+ * happen on the same queue (create/destroy/query/update).
+ */
+#define RTE_FLOW_PORT_FLAG_STRICT_QUEUE RTE_BIT32(0)
+
 /**
  * @warning
  * @b EXPERIMENTAL: this API may change without prior notice.
@@ -4902,6 +4908,10 @@ struct rte_flow_port_info {
 	 * @see RTE_FLOW_ACTION_TYPE_METER
 	 */
 	uint32_t max_nb_meters;
+	/**
+	 * Port supported flags (RTE_FLOW_PORT_FLAG_*).
+	 */
+	uint32_t supported_flags;
 };
 
 /**
@@ -4971,6 +4981,10 @@ struct rte_flow_port_attr {
 	 * @see RTE_FLOW_ACTION_TYPE_METER
 	 */
 	uint32_t nb_meters;
+	/**
+	 * Port flags (RTE_FLOW_PORT_FLAG_*).
+	 */
+	uint32_t flags;
 };
 
 /**
-- 
2.25.1


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

* [PATCH 2/3] ethdev: add queue-based API to report aged flow rules
  2022-09-21 14:54 [PATCH 0/3] ethdev: AGE action preparation Michael Baum
  2022-09-21 14:54 ` [PATCH 1/3] ethdev: add strict queue to pre-configuration flow hints Michael Baum
@ 2022-09-21 14:54 ` Michael Baum
  2022-09-29 12:04   ` Ori Kam
  2022-09-21 14:54 ` [PATCH 3/3] ethdev: add structure for indirect AGE update Michael Baum
                   ` (2 subsequent siblings)
  4 siblings, 1 reply; 32+ messages in thread
From: Michael Baum @ 2022-09-21 14:54 UTC (permalink / raw)
  To: dev; +Cc: Matan Azrad, Raslan Darawsheh, Ori Kam

When application use queue-based flow rule management and operate the
same flow rule on the same queue, e.g create/destroy/query, API of
querying aged flow rules should also have queue id parameter just like
other queue-based flow APIs.

By this way, PMD can work in more optimized way since resources are
isolated by queue and needn't synchronize.

If application do use queue-based flow management but configure port
without RTE_FLOW_PORT_FLAG_STRICT_QUEUE, which means application operate
a given flow rule on different queues, the queue id parameter will
be ignored.

Signed-off-by: Michael Baum <michaelba@nvidia.com>
---
 app/test-pmd/cmdline_flow.c                 |  17 ++-
 app/test-pmd/config.c                       | 159 +++++++++++++++++++-
 app/test-pmd/testpmd.h                      |   1 +
 doc/guides/testpmd_app_ug/testpmd_funcs.rst |  86 ++++++++++-
 lib/ethdev/rte_flow.c                       |  22 +++
 lib/ethdev/rte_flow.h                       |  48 +++++-
 lib/ethdev/rte_flow_driver.h                |   7 +
 lib/ethdev/version.map                      |   3 +
 8 files changed, 333 insertions(+), 10 deletions(-)

diff --git a/app/test-pmd/cmdline_flow.c b/app/test-pmd/cmdline_flow.c
index a982083d27..4fb90a92cb 100644
--- a/app/test-pmd/cmdline_flow.c
+++ b/app/test-pmd/cmdline_flow.c
@@ -127,6 +127,7 @@ enum index {
 	/* Queue arguments. */
 	QUEUE_CREATE,
 	QUEUE_DESTROY,
+	QUEUE_AGED,
 	QUEUE_INDIRECT_ACTION,
 
 	/* Queue create arguments. */
@@ -1159,6 +1160,7 @@ static const enum index next_table_destroy_attr[] = {
 static const enum index next_queue_subcmd[] = {
 	QUEUE_CREATE,
 	QUEUE_DESTROY,
+	QUEUE_AGED,
 	QUEUE_INDIRECT_ACTION,
 	ZERO,
 };
@@ -2942,6 +2944,13 @@ static const struct token token_list[] = {
 		.args = ARGS(ARGS_ENTRY(struct buffer, queue)),
 		.call = parse_qo_destroy,
 	},
+	[QUEUE_AGED] = {
+		.name = "aged",
+		.help = "list and destroy aged flows",
+		.next = NEXT(next_aged_attr, NEXT_ENTRY(COMMON_QUEUE_ID)),
+		.args = ARGS(ARGS_ENTRY(struct buffer, queue)),
+		.call = parse_aged,
+	},
 	[QUEUE_INDIRECT_ACTION] = {
 		.name = "indirect_action",
 		.help = "queue indirect actions",
@@ -8640,8 +8649,8 @@ parse_aged(struct context *ctx, const struct token *token,
 	/* Nothing else to do if there is no buffer. */
 	if (!out)
 		return len;
-	if (!out->command) {
-		if (ctx->curr != AGED)
+	if (!out->command || out->command == QUEUE) {
+		if (ctx->curr != AGED && ctx->curr != QUEUE_AGED)
 			return -1;
 		if (sizeof(*out) > size)
 			return -1;
@@ -10496,6 +10505,10 @@ cmd_flow_parsed(const struct buffer *in)
 	case PULL:
 		port_queue_flow_pull(in->port, in->queue);
 		break;
+	case QUEUE_AGED:
+		port_queue_flow_aged(in->port, in->queue,
+				     in->args.aged.destroy);
+		break;
 	case QUEUE_INDIRECT_ACTION_CREATE:
 		port_queue_action_handle_create(
 				in->port, in->queue, in->postpone,
diff --git a/app/test-pmd/config.c b/app/test-pmd/config.c
index a2939867c4..31952467fb 100644
--- a/app/test-pmd/config.c
+++ b/app/test-pmd/config.c
@@ -2662,6 +2662,7 @@ port_queue_flow_create(portid_t port_id, queueid_t queue_id,
 		       const struct rte_flow_action *actions)
 {
 	struct rte_flow_op_attr op_attr = { .postpone = postpone };
+	struct rte_flow_attr flow_attr = { 0 };
 	struct rte_flow *flow;
 	struct rte_port *port;
 	struct port_flow *pf;
@@ -2713,7 +2714,7 @@ port_queue_flow_create(portid_t port_id, queueid_t queue_id,
 		return -EINVAL;
 	}
 
-	pf = port_flow_new(NULL, pattern, actions, &error);
+	pf = port_flow_new(&flow_attr, pattern, actions, &error);
 	if (!pf)
 		return port_flow_complain(&error);
 	if (age) {
@@ -2950,6 +2951,162 @@ port_queue_flow_push(portid_t port_id, queueid_t queue_id)
 	return ret;
 }
 
+/** Pull queue operation results from the queue. */
+static int
+port_queue_aged_flow_destroy(portid_t port_id, queueid_t queue_id,
+			     const uint32_t *rule, int nb_flows)
+{
+	struct rte_port *port = &ports[port_id];
+	struct rte_flow_op_result *res;
+	struct rte_flow_error error;
+	uint32_t n = nb_flows;
+	int ret = 0;
+	int i;
+
+	res = calloc(port->queue_sz, sizeof(struct rte_flow_op_result));
+	if (!res) {
+		printf("Failed to allocate memory for pulled results\n");
+		return -ENOMEM;
+	}
+
+	memset(&error, 0x66, sizeof(error));
+	while (nb_flows > 0) {
+		int success = 0;
+
+		if (n > port->queue_sz)
+			n = port->queue_sz;
+		ret = port_queue_flow_destroy(port_id, queue_id, true, n, rule);
+		if (ret < 0) {
+			free(res);
+			return ret;
+		}
+		ret = rte_flow_push(port_id, queue_id, &error);
+		if (ret < 0) {
+			printf("Failed to push operations in the queue: %s\n",
+			       strerror(-ret));
+			free(res);
+			return ret;
+		}
+		while (success < nb_flows) {
+			ret = rte_flow_pull(port_id, queue_id, res,
+					    port->queue_sz, &error);
+			if (ret < 0) {
+				printf("Failed to pull a operation results: %s\n",
+				       strerror(-ret));
+				free(res);
+				return ret;
+			}
+
+			for (i = 0; i < ret; i++) {
+				if (res[i].status == RTE_FLOW_OP_SUCCESS)
+					success++;
+			}
+		}
+		rule += n;
+		nb_flows -= n;
+		n = nb_flows;
+	}
+
+	free(res);
+	return ret;
+}
+
+/** List simply and destroy all aged flows per queue. */
+void
+port_queue_flow_aged(portid_t port_id, uint32_t queue_id, uint8_t destroy)
+{
+	void **contexts;
+	int nb_context, total = 0, idx;
+	uint32_t *rules = NULL;
+	struct rte_port *port;
+	struct rte_flow_error error;
+	enum age_action_context_type *type;
+	union {
+		struct port_flow *pf;
+		struct port_indirect_action *pia;
+	} ctx;
+
+	if (port_id_is_invalid(port_id, ENABLED_WARN) ||
+	    port_id == (portid_t)RTE_PORT_ALL)
+		return;
+	port = &ports[port_id];
+	if (queue_id >= port->queue_nb) {
+		printf("Error: queue #%u is invalid\n", queue_id);
+		return;
+	}
+	total = rte_flow_get_q_aged_flows(port_id, queue_id, NULL, 0, &error);
+	if (total < 0) {
+		port_flow_complain(&error);
+		return;
+	}
+	printf("Port %u queue %u total aged flows: %d\n",
+	       port_id, queue_id, total);
+	if (total == 0)
+		return;
+	contexts = calloc(total, sizeof(void *));
+	if (contexts == NULL) {
+		printf("Cannot allocate contexts for aged flow\n");
+		return;
+	}
+	printf("%-20s\tID\tGroup\tPrio\tAttr\n", "Type");
+	nb_context = rte_flow_get_q_aged_flows(port_id, queue_id, contexts,
+					       total, &error);
+	if (nb_context > total) {
+		printf("Port %u queue %u get aged flows count(%d) > total(%d)\n",
+		       port_id, queue_id, nb_context, total);
+		free(contexts);
+		return;
+	}
+	if (destroy) {
+		rules = malloc(sizeof(uint32_t) * nb_context);
+		if (rules == NULL)
+			printf("Cannot allocate memory for destroy aged flow\n");
+	}
+	total = 0;
+	for (idx = 0; idx < nb_context; idx++) {
+		if (!contexts[idx]) {
+			printf("Error: get Null context in port %u queue %u\n",
+			       port_id, queue_id);
+			continue;
+		}
+		type = (enum age_action_context_type *)contexts[idx];
+		switch (*type) {
+		case ACTION_AGE_CONTEXT_TYPE_FLOW:
+			ctx.pf = container_of(type, struct port_flow, age_type);
+			printf("%-20s\t%" PRIu32 "\t%" PRIu32 "\t%" PRIu32
+								 "\t%c%c%c\t\n",
+			       "Flow",
+			       ctx.pf->id,
+			       ctx.pf->rule.attr->group,
+			       ctx.pf->rule.attr->priority,
+			       ctx.pf->rule.attr->ingress ? 'i' : '-',
+			       ctx.pf->rule.attr->egress ? 'e' : '-',
+			       ctx.pf->rule.attr->transfer ? 't' : '-');
+			if (rules != NULL) {
+				rules[total] = ctx.pf->id;
+				total++;
+			}
+			break;
+		case ACTION_AGE_CONTEXT_TYPE_INDIRECT_ACTION:
+			ctx.pia = container_of(type,
+					       struct port_indirect_action,
+					       age_type);
+			printf("%-20s\t%" PRIu32 "\n", "Indirect action",
+			       ctx.pia->id);
+			break;
+		default:
+			printf("Error: invalid context type %u\n", port_id);
+			break;
+		}
+	}
+	if (rules != NULL) {
+		port_queue_aged_flow_destroy(port_id, queue_id, rules, total);
+		free(rules);
+	}
+	printf("\n%d flows destroyed\n", total);
+	free(contexts);
+}
+
 /** Pull queue operation results from the queue. */
 int
 port_queue_flow_pull(portid_t port_id, queueid_t queue_id)
diff --git a/app/test-pmd/testpmd.h b/app/test-pmd/testpmd.h
index fb2f5195d3..4e24dd9ee0 100644
--- a/app/test-pmd/testpmd.h
+++ b/app/test-pmd/testpmd.h
@@ -982,6 +982,7 @@ int port_queue_action_handle_update(portid_t port_id, uint32_t queue_id,
 				    const struct rte_flow_action *action);
 int port_queue_flow_push(portid_t port_id, queueid_t queue_id);
 int port_queue_flow_pull(portid_t port_id, queueid_t queue_id);
+void port_queue_flow_aged(portid_t port_id, uint32_t queue_id, uint8_t destroy);
 int port_flow_validate(portid_t port_id,
 		       const struct rte_flow_attr *attr,
 		       const struct rte_flow_item *pattern,
diff --git a/doc/guides/testpmd_app_ug/testpmd_funcs.rst b/doc/guides/testpmd_app_ug/testpmd_funcs.rst
index 6c12e0286c..e68b852e29 100644
--- a/doc/guides/testpmd_app_ug/testpmd_funcs.rst
+++ b/doc/guides/testpmd_app_ug/testpmd_funcs.rst
@@ -3085,9 +3085,10 @@ following sections.
        [meters_number {number}] [flags {number}]
 
 - Create a pattern template::
+
    flow pattern_template {port_id} create [pattern_template_id {id}]
        [relaxed {boolean}] [ingress] [egress] [transfer]
-	   template {item} [/ {item} [...]] / end
+       template {item} [/ {item} [...]] / end
 
 - Destroy a pattern template::
 
@@ -3186,6 +3187,10 @@ following sections.
 
    flow aged {port_id} [destroy]
 
+- Enqueue list and destroy aged flow rules::
+
+   flow queue {port_id} aged {queue_id} [destroy]
+
 - Tunnel offload - create a tunnel stub::
 
    flow tunnel create {port_id} type {tunnel_type}
@@ -4427,7 +4432,7 @@ Disabling isolated mode::
  testpmd>
 
 Dumping HW internal information
-~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
 
 ``flow dump`` dumps the hardware's internal representation information of
 all flows. It is bound to ``rte_flow_dev_dump()``::
@@ -4443,10 +4448,10 @@ 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.
+and ``destroy`` parameter can be used to destroy those flow rules in PMD::
 
    flow aged {port_id} [destroy]
 
@@ -4481,7 +4486,7 @@ will be ID 3, ID 1, ID 0::
    1       0       0       i--
    0       0       0       i--
 
-If attach ``destroy`` parameter, the command will destroy all the list aged flow rules.
+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
@@ -4499,6 +4504,77 @@ If attach ``destroy`` parameter, the command will destroy all the list aged flow
    testpmd> flow aged 0
    Port 0 total aged flows: 0
 
+
+Enqueueing listing and destroying aged flow rules
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+
+``flow queue aged`` simply lists aged flow rules be get from
+``rte_flow_get_q_aged_flows`` API, and ``destroy`` parameter can be used to
+destroy those flow rules in PMD::
+
+   flow queue {port_id} aged {queue_id} [destroy]
+
+Listing current aged flow rules::
+
+   testpmd> flow queue 0 aged 0
+   Port 0 queue 0 total aged flows: 0
+   testpmd> flow queue 0 create 0 ingress tanle 0 item_template 0 action_template 0
+      pattern eth / ipv4 src is 2.2.2.14 / end
+      actions age timeout 5 / queue index 0 /  end
+   Flow rule #0 creation enqueued
+   testpmd> flow queue 0 create 0 ingress tanle 0 item_template 0 action_template 0
+      pattern eth / ipv4 src is 2.2.2.15 / end
+      actions age timeout 4 / queue index 0 /  end
+   Flow rule #1 creation enqueued
+   testpmd> flow queue 0 create 0 ingress tanle 0 item_template 0 action_template 0
+      pattern eth / ipv4 src is 2.2.2.16 / end
+      actions age timeout 4 / queue index 0 /  end
+   Flow rule #2 creation enqueued
+   testpmd> flow queue 0 create 0 ingress tanle 0 item_template 0 action_template 0
+      pattern eth / ipv4 src is 2.2.2.17 / end
+      actions age timeout 4 / queue index 0 /  end
+   Flow rule #3 creation enqueued
+   testpmd> flow pull 0 queue 0
+   Queue #0 pulled 4 operations (0 failed, 4 succeeded)
+
+Aged Rules are simply list as command ``flow queue {port_id} list {queue_id}``,
+but strip the detail rule information, all the aged flows are sorted by the
+longest timeout time. For example, if those rules is 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 queue 0 aged 0
+   Port 0 queue 0 total aged flows: 4
+   ID      Group   Prio    Attr
+   2       0       0       ---
+   3       0       0       ---
+   1       0       0       ---
+   0       0       0       ---
+
+   0 flows destroyed
+
+If attach ``destroy`` parameter, the command will destroy all the list aged flow rules::
+
+   testpmd> flow queue 0 aged 0 destroy
+   Port 0 queue 0 total aged flows: 4
+   ID      Group   Prio    Attr
+   2       0       0       ---
+   3       0       0       ---
+   1       0       0       ---
+   0       0       0       ---
+   Flow rule #2 destruction enqueued
+   Flow rule #3 destruction enqueued
+   Flow rule #1 destruction enqueued
+   Flow rule #0 destruction enqueued
+
+   4 flows destroyed
+   testpmd> flow queue 0 aged 0
+   Port 0 total aged flows: 0
+
+.. note::
+
+   The queue must be empty before attaching ``destroy`` parameter.
+
+
 Creating indirect actions
 ~~~~~~~~~~~~~~~~~~~~~~~~~
 
diff --git a/lib/ethdev/rte_flow.c b/lib/ethdev/rte_flow.c
index 501be9d602..5c95ac7f8b 100644
--- a/lib/ethdev/rte_flow.c
+++ b/lib/ethdev/rte_flow.c
@@ -1133,6 +1133,28 @@ rte_flow_get_aged_flows(uint16_t port_id, void **contexts,
 				  NULL, rte_strerror(ENOTSUP));
 }
 
+int
+rte_flow_get_q_aged_flows(uint16_t port_id, uint32_t queue_id, void **contexts,
+			  uint32_t nb_contexts, struct rte_flow_error *error)
+{
+	struct rte_eth_dev *dev = &rte_eth_devices[port_id];
+	const struct rte_flow_ops *ops = rte_flow_ops_get(port_id, error);
+	int ret;
+
+	if (unlikely(!ops))
+		return -rte_errno;
+	if (likely(!!ops->get_q_aged_flows)) {
+		fts_enter(dev);
+		ret = ops->get_q_aged_flows(dev, queue_id, contexts,
+					    nb_contexts, error);
+		fts_exit(dev);
+		return flow_err(port_id, ret, error);
+	}
+	return rte_flow_error_set(error, ENOTSUP,
+				  RTE_FLOW_ERROR_TYPE_UNSPECIFIED,
+				  NULL, rte_strerror(ENOTSUP));
+}
+
 struct rte_flow_action_handle *
 rte_flow_action_handle_create(uint16_t port_id,
 			      const struct rte_flow_indir_action_conf *conf,
diff --git a/lib/ethdev/rte_flow.h b/lib/ethdev/rte_flow.h
index c552771472..d830b02321 100644
--- a/lib/ethdev/rte_flow.h
+++ b/lib/ethdev/rte_flow.h
@@ -2930,8 +2930,8 @@ struct rte_flow_action_queue {
  * on the flow. RTE_ETH_EVENT_FLOW_AGED event is triggered when a
  * port detects new aged-out flows.
  *
- * The flow context and the flow handle will be reported by the
- * rte_flow_get_aged_flows API.
+ * The flow context and the flow handle will be reported by the either
+ * rte_flow_get_aged_flows or rte_flow_get_q_aged_flows APIs.
  */
 struct rte_flow_action_age {
 	uint32_t timeout:24; /**< Time in seconds. */
@@ -4443,6 +4443,50 @@ int
 rte_flow_get_aged_flows(uint16_t port_id, void **contexts,
 			uint32_t nb_contexts, struct rte_flow_error *error);
 
+/**
+ * @warning
+ * @b EXPERIMENTAL: this API may change without prior notice.
+ *
+ * Get aged-out flows of a given port on the given flow queue.
+ *
+ * If application configure port attribute with RTE_FLOW_PORT_FLAG_STRICT_QUEUE,
+ * there is no RTE_ETH_EVENT_FLOW_AGED event and this function must be called to
+ * get the aged flows synchronously.
+ *
+ * If application configure port attribute without
+ * RTE_FLOW_PORT_FLAG_STRICT_QUEUE, RTE_ETH_EVENT_FLOW_AGED event will be
+ * triggered at least one new aged out flow was detected on any flow queue after
+ * the last call to rte_flow_get_q_aged_flows.
+ * In addition, the @p queue_id will be ignored.
+ * This function can be called to get the aged flows asynchronously from the
+ * event callback or synchronously regardless the event.
+ *
+ * @param[in] port_id
+ *   Port identifier of Ethernet device.
+ * @param[in] queue_id
+ *   Flow queue to query. Ignored when RTE_FLOW_PORT_FLAG_STRICT_QUEUE not set.
+ * @param[in, out] contexts
+ *   The address of an array of pointers to the aged-out flows contexts.
+ * @param[in] nb_contexts
+ *   The length of context array pointers.
+ * @param[out] error
+ *   Perform verbose error reporting if not NULL. Initialized in case of
+ *   error only.
+ *
+ * @return
+ *   if nb_contexts is 0, return the amount of all aged contexts.
+ *   if nb_contexts is not 0 , return the amount of aged flows reported
+ *   in the context array, otherwise negative errno value.
+ *
+ * @see rte_flow_action_age
+ * @see RTE_ETH_EVENT_FLOW_AGED
+ * @see rte_flow_port_flag
+ */
+__rte_experimental
+int
+rte_flow_get_q_aged_flows(uint16_t port_id, uint32_t queue_id, void **contexts,
+			  uint32_t nb_contexts, struct rte_flow_error *error);
+
 /**
  * Specify indirect action object configuration
  */
diff --git a/lib/ethdev/rte_flow_driver.h b/lib/ethdev/rte_flow_driver.h
index 2bff732d6a..f0a03bf149 100644
--- a/lib/ethdev/rte_flow_driver.h
+++ b/lib/ethdev/rte_flow_driver.h
@@ -84,6 +84,13 @@ struct rte_flow_ops {
 		 void **context,
 		 uint32_t nb_contexts,
 		 struct rte_flow_error *err);
+	/** See rte_flow_get_q_aged_flows() */
+	int (*get_q_aged_flows)
+		(struct rte_eth_dev *dev,
+		 uint32_t queue_id,
+		 void **contexts,
+		 uint32_t nb_contexts,
+		 struct rte_flow_error *error);
 	/** See rte_flow_action_handle_create() */
 	struct rte_flow_action_handle *(*action_handle_create)
 		(struct rte_eth_dev *dev,
diff --git a/lib/ethdev/version.map b/lib/ethdev/version.map
index 03f52fee91..4a40d24d8f 100644
--- a/lib/ethdev/version.map
+++ b/lib/ethdev/version.map
@@ -285,6 +285,9 @@ EXPERIMENTAL {
 	rte_mtr_color_in_protocol_priority_get;
 	rte_mtr_color_in_protocol_set;
 	rte_mtr_meter_vlan_table_update;
+
+	# added in 22.11
+	rte_flow_get_q_aged_flows;
 };
 
 INTERNAL {
-- 
2.25.1


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

* [PATCH 3/3] ethdev: add structure for indirect AGE update
  2022-09-21 14:54 [PATCH 0/3] ethdev: AGE action preparation Michael Baum
  2022-09-21 14:54 ` [PATCH 1/3] ethdev: add strict queue to pre-configuration flow hints Michael Baum
  2022-09-21 14:54 ` [PATCH 2/3] ethdev: add queue-based API to report aged flow rules Michael Baum
@ 2022-09-21 14:54 ` Michael Baum
  2022-09-29 12:39   ` Ori Kam
  2022-10-03  8:03   ` Andrew Rybchenko
  2022-09-29  8:40 ` [PATCH 0/3] ethdev: AGE action preparation Andrew Rybchenko
  2022-10-19 13:12 ` [PATCH v2 " Michael Baum
  4 siblings, 2 replies; 32+ messages in thread
From: Michael Baum @ 2022-09-21 14:54 UTC (permalink / raw)
  To: dev; +Cc: Matan Azrad, Raslan Darawsheh, Ori Kam

Add a new structure for indirect AGE update.

This new structure enables:
1. Update timeout value.
2. Stop AGE checking.
3. Start AGE checking.
4. restart AGE checking.

Signed-off-by: Michael Baum <michaelba@nvidia.com>
---
 app/test-pmd/cmdline_flow.c        | 66 ++++++++++++++++++++++++++++++
 app/test-pmd/config.c              | 18 +++++++-
 doc/guides/prog_guide/rte_flow.rst | 25 +++++++++--
 lib/ethdev/rte_flow.h              | 27 ++++++++++++
 4 files changed, 132 insertions(+), 4 deletions(-)

diff --git a/app/test-pmd/cmdline_flow.c b/app/test-pmd/cmdline_flow.c
index 4fb90a92cb..a315fd9ded 100644
--- a/app/test-pmd/cmdline_flow.c
+++ b/app/test-pmd/cmdline_flow.c
@@ -583,6 +583,9 @@ enum index {
 	ACTION_SET_IPV6_DSCP_VALUE,
 	ACTION_AGE,
 	ACTION_AGE_TIMEOUT,
+	ACTION_AGE_UPDATE,
+	ACTION_AGE_UPDATE_TIMEOUT,
+	ACTION_AGE_UPDATE_TOUCH,
 	ACTION_SAMPLE,
 	ACTION_SAMPLE_RATIO,
 	ACTION_SAMPLE_INDEX,
@@ -1869,6 +1872,7 @@ static const enum index next_action[] = {
 	ACTION_SET_IPV4_DSCP,
 	ACTION_SET_IPV6_DSCP,
 	ACTION_AGE,
+	ACTION_AGE_UPDATE,
 	ACTION_SAMPLE,
 	ACTION_INDIRECT,
 	ACTION_MODIFY_FIELD,
@@ -2113,6 +2117,14 @@ static const enum index action_age[] = {
 	ZERO,
 };
 
+static const enum index action_age_update[] = {
+	ACTION_AGE_UPDATE,
+	ACTION_AGE_UPDATE_TIMEOUT,
+	ACTION_AGE_UPDATE_TOUCH,
+	ACTION_NEXT,
+	ZERO,
+};
+
 static const enum index action_sample[] = {
 	ACTION_SAMPLE,
 	ACTION_SAMPLE_RATIO,
@@ -2191,6 +2203,9 @@ static int parse_vc_spec(struct context *, const struct token *,
 			 const char *, unsigned int, void *, unsigned int);
 static int parse_vc_conf(struct context *, const struct token *,
 			 const char *, unsigned int, void *, unsigned int);
+static int parse_vc_conf_timeout(struct context *, const struct token *,
+				 const char *, unsigned int, void *,
+				 unsigned int);
 static int parse_vc_item_ecpri_type(struct context *, const struct token *,
 				    const char *, unsigned int,
 				    void *, unsigned int);
@@ -6194,6 +6209,30 @@ static const struct token token_list[] = {
 		.next = NEXT(action_age, NEXT_ENTRY(COMMON_UNSIGNED)),
 		.call = parse_vc_conf,
 	},
+	[ACTION_AGE_UPDATE] = {
+		.name = "age_update",
+		.help = "update aging parameter",
+		.next = NEXT(action_age_update),
+		.priv = PRIV_ACTION(AGE,
+				    sizeof(struct rte_flow_update_age)),
+		.call = parse_vc,
+	},
+	[ACTION_AGE_UPDATE_TIMEOUT] = {
+		.name = "timeout",
+		.help = "age timeout update value",
+		.args = ARGS(ARGS_ENTRY_BF(struct rte_flow_update_age,
+					   timeout, 24)),
+		.next = NEXT(action_age_update, NEXT_ENTRY(COMMON_UNSIGNED)),
+		.call = parse_vc_conf_timeout,
+	},
+	[ACTION_AGE_UPDATE_TOUCH] = {
+		.name = "touch",
+		.help = "this flow is touched",
+		.next = NEXT(action_age_update, NEXT_ENTRY(COMMON_BOOLEAN)),
+		.args = ARGS(ARGS_ENTRY_BF(struct rte_flow_update_age,
+					   touch, 1)),
+		.call = parse_vc_conf,
+	},
 	[ACTION_SAMPLE] = {
 		.name = "sample",
 		.help = "set a sample action",
@@ -7031,6 +7070,33 @@ parse_vc_conf(struct context *ctx, const struct token *token,
 	return len;
 }
 
+/** Parse action configuration field. */
+static int
+parse_vc_conf_timeout(struct context *ctx, const struct token *token,
+		      const char *str, unsigned int len,
+		      void *buf, unsigned int size)
+{
+	struct buffer *out = buf;
+	struct rte_flow_update_age *update;
+
+	(void)size;
+	if (ctx->curr != ACTION_AGE_UPDATE_TIMEOUT)
+		return -1;
+	/* 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;
+	/* Point to selected object. */
+	ctx->object = out->args.vc.data;
+	ctx->objmask = NULL;
+	/* Update the timeout is valid. */
+	update = (struct rte_flow_update_age *)out->args.vc.data;
+	update->timeout_valid = 1;
+	return len;
+}
+
 /** Parse eCPRI common header type field. */
 static int
 parse_vc_item_ecpri_type(struct context *ctx, const struct token *token,
diff --git a/app/test-pmd/config.c b/app/test-pmd/config.c
index 31952467fb..45495385d7 100644
--- a/app/test-pmd/config.c
+++ b/app/test-pmd/config.c
@@ -2065,6 +2065,7 @@ port_action_handle_update(portid_t port_id, uint32_t id,
 	if (!pia)
 		return -EINVAL;
 	switch (pia->type) {
+	case RTE_FLOW_ACTION_TYPE_AGE:
 	case RTE_FLOW_ACTION_TYPE_CONNTRACK:
 		update = action->conf;
 		break;
@@ -2904,6 +2905,8 @@ port_queue_action_handle_update(portid_t port_id,
 	struct rte_port *port;
 	struct rte_flow_error error;
 	struct rte_flow_action_handle *action_handle;
+	struct port_indirect_action *pia;
+	const void *update;
 
 	action_handle = port_action_handle_get_by_id(port_id, id);
 	if (!action_handle)
@@ -2915,8 +2918,21 @@ port_queue_action_handle_update(portid_t port_id,
 		return -EINVAL;
 	}
 
+	pia = action_get_by_id(port_id, id);
+	if (!pia)
+		return -EINVAL;
+
+	switch (pia->type) {
+	case RTE_FLOW_ACTION_TYPE_AGE:
+		update = action->conf;
+		break;
+	default:
+		update = action;
+		break;
+	}
+
 	if (rte_flow_async_action_handle_update(port_id, queue_id, &attr,
-				    action_handle, action, NULL, &error)) {
+				    action_handle, update, NULL, &error)) {
 		return port_flow_complain(&error);
 	}
 	printf("Indirect action #%u update queued\n", id);
diff --git a/doc/guides/prog_guide/rte_flow.rst b/doc/guides/prog_guide/rte_flow.rst
index 588914b231..dae9121279 100644
--- a/doc/guides/prog_guide/rte_flow.rst
+++ b/doc/guides/prog_guide/rte_flow.rst
@@ -2958,7 +2958,7 @@ Otherwise, RTE_FLOW_ERROR_TYPE_ACTION error will be returned.
 Action: ``AGE``
 ^^^^^^^^^^^^^^^
 
-Set ageing timeout configuration to a flow.
+Set aging timeout configuration to a flow.
 
 Event RTE_ETH_EVENT_FLOW_AGED will be reported if
 timeout passed without any matching on the flow.
@@ -2977,8 +2977,8 @@ timeout passed without any matching on the flow.
    | ``context``  | user input flow context         |
    +--------------+---------------------------------+
 
-Query structure to retrieve ageing status information of a
-shared AGE action, or a flow rule using the AGE action:
+Query structure to retrieve aging status information of an
+indirect AGE action, or a flow rule using the AGE action:
 
 .. _table_rte_flow_query_age:
 
@@ -2994,6 +2994,25 @@ shared AGE action, or a flow rule using the AGE action:
    | ``sec_since_last_hit``       | out | Seconds since last traffic hit         |
    +------------------------------+-----+----------------------------------------+
 
+Update structure to modify the parameters of an indirect AGE action.
+The update structure is used by ``rte_flow_action_handle_update()`` function.
+
+.. _table_rte_flow_update_age:
+
+.. table:: AGE update
+
+   +-------------------+--------------------------------------------------------------+
+   | Field             | Value                                                        |
+   +===================+==============================================================+
+   | ``timeout``       | 24 bits timeout value                                        |
+   +-------------------+--------------------------------------------------------------+
+   | ``timeout_valid`` | 1 bit, timeout value is valid                                |
+   +-------------------+--------------------------------------------------------------+
+   | ``touch``         | 1 bit, touch the AGE action to set ``sec_since_last_hit`` 0  |
+   +-------------------+--------------------------------------------------------------+
+   | ``reserved``      | 6 bits reserved, must be zero                                |
+   +-------------------+--------------------------------------------------------------+
+
 Action: ``SAMPLE``
 ^^^^^^^^^^^^^^^^^^
 
diff --git a/lib/ethdev/rte_flow.h b/lib/ethdev/rte_flow.h
index d830b02321..a21d437cf8 100644
--- a/lib/ethdev/rte_flow.h
+++ b/lib/ethdev/rte_flow.h
@@ -2954,6 +2954,33 @@ struct rte_flow_query_age {
 	uint32_t sec_since_last_hit:24; /**< Seconds since last traffic hit. */
 };
 
+/**
+ * @warning
+ * @b EXPERIMENTAL: this structure may change without prior notice
+ *
+ * RTE_FLOW_ACTION_TYPE_AGE
+ *
+ * Update indirect AGE action attributes:
+ *  - Timeout can be updated including stop/start action:
+ *     +-------------+-------------+------------------------------+
+ *     | Old Timeout | New Timeout | Updating                     |
+ *     +=============+=============+==============================+
+ *     | 0           | positive    | Start aging with new value   |
+ *     +-------------+-------------+------------------------------+
+ *     | positive    | 0           | Stop aging			  |
+ *     +-------------+-------------+------------------------------+
+ *     | positive    | positive    | Change timeout to new value  |
+ *     +-------------+-------------+------------------------------+
+ *  - sec_since_last_hit can be reset.
+ */
+struct rte_flow_update_age {
+	uint32_t reserved:6; /**< Reserved, must be zero. */
+	uint32_t timeout_valid:1; /**< The timeout is valid for update. */
+	uint32_t timeout:24; /**< Time in seconds. */
+	uint32_t touch:1;
+	/**< Means that aging should assume packet passed the aging. */
+};
+
 /**
  * @warning
  * @b EXPERIMENTAL: this structure may change without prior notice
-- 
2.25.1


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

* Re: [PATCH 0/3] ethdev: AGE action preparation
  2022-09-21 14:54 [PATCH 0/3] ethdev: AGE action preparation Michael Baum
                   ` (2 preceding siblings ...)
  2022-09-21 14:54 ` [PATCH 3/3] ethdev: add structure for indirect AGE update Michael Baum
@ 2022-09-29  8:40 ` Andrew Rybchenko
  2022-10-19 13:12 ` [PATCH v2 " Michael Baum
  4 siblings, 0 replies; 32+ messages in thread
From: Andrew Rybchenko @ 2022-09-29  8:40 UTC (permalink / raw)
  To: Ori Kam; +Cc: Matan Azrad, Raslan Darawsheh, Michael Baum, dev

@Ori, could you take a look at the patch series.

On 9/21/22 17:54, Michael Baum wrote:
> RFC's:
> https://patchwork.dpdk.org/project/dpdk/patch/7a45693f478b1b721b4e05131141b526185a175c.1654063912.git.jackmin@nvidia.com/
> https://patchwork.dpdk.org/project/dpdk/patch/608febf8d5d3c434a1eddb2e56f425ebbd6ff0b4.1654063912.git.jackmin@nvidia.com/
> 
> Michael Baum (3):
>    ethdev: add strict queue to pre-configuration flow hints
>    ethdev: add queue-based API to report aged flow rules
>    ethdev: add structure for indirect AGE update
> 
>   app/test-pmd/cmdline_flow.c                 |  93 +++++++++-
>   app/test-pmd/config.c                       | 177 +++++++++++++++++++-
>   app/test-pmd/testpmd.h                      |   1 +
>   doc/guides/prog_guide/rte_flow.rst          |  25 ++-
>   doc/guides/testpmd_app_ug/testpmd_funcs.rst |  90 +++++++++-
>   lib/ethdev/rte_flow.c                       |  22 +++
>   lib/ethdev/rte_flow.h                       |  89 +++++++++-
>   lib/ethdev/rte_flow_driver.h                |   7 +
>   lib/ethdev/version.map                      |   3 +
>   9 files changed, 491 insertions(+), 16 deletions(-)
> 


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

* RE: [PATCH 1/3] ethdev: add strict queue to pre-configuration flow hints
  2022-09-21 14:54 ` [PATCH 1/3] ethdev: add strict queue to pre-configuration flow hints Michael Baum
@ 2022-09-29 12:04   ` Ori Kam
  0 siblings, 0 replies; 32+ messages in thread
From: Ori Kam @ 2022-09-29 12:04 UTC (permalink / raw)
  To: Michael Baum, dev; +Cc: Matan Azrad, Raslan Darawsheh

Hi Michael,

> -----Original Message-----
> From: Michael Baum <michaelba@nvidia.com>
> Sent: Wednesday, 21 September 2022 17:54
> 
> The data-path focused flow rule management can manage flow rules in more
> optimized way than traditional one by using hints provided by
> application in initialization phase.
> 
> In addition to the current hints we have in port attr, more hints could
> be provided by application about its behaviour.
> 
> One example is how the application do with the same flow rule ?
> A. create/destroy flow on same queue but query flow on different queue
>    or queue-less way (i.e, counter query)
> B. All flow operations will be exactly on the same queue, by which PMD
>    could be in more optimized way then A because resource could be
>    isolated and access based on queue, without lock, for example.
> 
> This patch add flag about above situation and could be extended to cover
> more situations.
> 
> Signed-off-by: Michael Baum <michaelba@nvidia.com>
> ---

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

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

* RE: [PATCH 2/3] ethdev: add queue-based API to report aged flow rules
  2022-09-21 14:54 ` [PATCH 2/3] ethdev: add queue-based API to report aged flow rules Michael Baum
@ 2022-09-29 12:04   ` Ori Kam
  0 siblings, 0 replies; 32+ messages in thread
From: Ori Kam @ 2022-09-29 12:04 UTC (permalink / raw)
  To: Michael Baum, dev; +Cc: Matan Azrad, Raslan Darawsheh

Hi Michael,

> -----Original Message-----
> From: Michael Baum <michaelba@nvidia.com>
> Sent: Wednesday, 21 September 2022 17:54
> 
> When application use queue-based flow rule management and operate the
> same flow rule on the same queue, e.g create/destroy/query, API of
> querying aged flow rules should also have queue id parameter just like
> other queue-based flow APIs.
> 
> By this way, PMD can work in more optimized way since resources are
> isolated by queue and needn't synchronize.
> 
> If application do use queue-based flow management but configure port
> without RTE_FLOW_PORT_FLAG_STRICT_QUEUE, which means application
> operate
> a given flow rule on different queues, the queue id parameter will
> be ignored.
> 
> Signed-off-by: Michael Baum <michaelba@nvidia.com>
> ---

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

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

* RE: [PATCH 3/3] ethdev: add structure for indirect AGE update
  2022-09-21 14:54 ` [PATCH 3/3] ethdev: add structure for indirect AGE update Michael Baum
@ 2022-09-29 12:39   ` Ori Kam
  2022-10-03  8:03   ` Andrew Rybchenko
  1 sibling, 0 replies; 32+ messages in thread
From: Ori Kam @ 2022-09-29 12:39 UTC (permalink / raw)
  To: Michael Baum, dev; +Cc: Matan Azrad, Raslan Darawsheh

Hi Michael,


> -----Original Message-----
> From: Michael Baum <michaelba@nvidia.com>
> Sent: Wednesday, 21 September 2022 17:54
> 
> Add a new structure for indirect AGE update.
> 
> This new structure enables:
> 1. Update timeout value.
> 2. Stop AGE checking.
> 3. Start AGE checking.
> 4. restart AGE checking.
> 
> Signed-off-by: Michael Baum <michaelba@nvidia.com>
> ---

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

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

* Re: [PATCH 3/3] ethdev: add structure for indirect AGE update
  2022-09-21 14:54 ` [PATCH 3/3] ethdev: add structure for indirect AGE update Michael Baum
  2022-09-29 12:39   ` Ori Kam
@ 2022-10-03  8:03   ` Andrew Rybchenko
  2022-10-03  8:30     ` Ori Kam
  1 sibling, 1 reply; 32+ messages in thread
From: Andrew Rybchenko @ 2022-10-03  8:03 UTC (permalink / raw)
  To: Michael Baum, dev; +Cc: Matan Azrad, Raslan Darawsheh, Ori Kam

On 9/21/22 17:54, Michael Baum wrote:
> Add a new structure for indirect AGE update.
> 
> This new structure enables:
> 1. Update timeout value.
> 2. Stop AGE checking.
> 3. Start AGE checking.
> 4. restart AGE checking.
> 
> Signed-off-by: Michael Baum <michaelba@nvidia.com>
> ---
>   app/test-pmd/cmdline_flow.c        | 66 ++++++++++++++++++++++++++++++
>   app/test-pmd/config.c              | 18 +++++++-
>   doc/guides/prog_guide/rte_flow.rst | 25 +++++++++--
>   lib/ethdev/rte_flow.h              | 27 ++++++++++++
>   4 files changed, 132 insertions(+), 4 deletions(-)
> 
> diff --git a/app/test-pmd/cmdline_flow.c b/app/test-pmd/cmdline_flow.c
> index 4fb90a92cb..a315fd9ded 100644
> --- a/app/test-pmd/cmdline_flow.c
> +++ b/app/test-pmd/cmdline_flow.c
> @@ -583,6 +583,9 @@ enum index {
>   	ACTION_SET_IPV6_DSCP_VALUE,
>   	ACTION_AGE,
>   	ACTION_AGE_TIMEOUT,
> +	ACTION_AGE_UPDATE,
> +	ACTION_AGE_UPDATE_TIMEOUT,
> +	ACTION_AGE_UPDATE_TOUCH,
>   	ACTION_SAMPLE,
>   	ACTION_SAMPLE_RATIO,
>   	ACTION_SAMPLE_INDEX,
> @@ -1869,6 +1872,7 @@ static const enum index next_action[] = {
>   	ACTION_SET_IPV4_DSCP,
>   	ACTION_SET_IPV6_DSCP,
>   	ACTION_AGE,
> +	ACTION_AGE_UPDATE,
>   	ACTION_SAMPLE,
>   	ACTION_INDIRECT,
>   	ACTION_MODIFY_FIELD,
> @@ -2113,6 +2117,14 @@ static const enum index action_age[] = {
>   	ZERO,
>   };
>   
> +static const enum index action_age_update[] = {
> +	ACTION_AGE_UPDATE,
> +	ACTION_AGE_UPDATE_TIMEOUT,
> +	ACTION_AGE_UPDATE_TOUCH,
> +	ACTION_NEXT,
> +	ZERO,
> +};
> +
>   static const enum index action_sample[] = {
>   	ACTION_SAMPLE,
>   	ACTION_SAMPLE_RATIO,
> @@ -2191,6 +2203,9 @@ static int parse_vc_spec(struct context *, const struct token *,
>   			 const char *, unsigned int, void *, unsigned int);
>   static int parse_vc_conf(struct context *, const struct token *,
>   			 const char *, unsigned int, void *, unsigned int);
> +static int parse_vc_conf_timeout(struct context *, const struct token *,
> +				 const char *, unsigned int, void *,
> +				 unsigned int);
>   static int parse_vc_item_ecpri_type(struct context *, const struct token *,
>   				    const char *, unsigned int,
>   				    void *, unsigned int);
> @@ -6194,6 +6209,30 @@ static const struct token token_list[] = {
>   		.next = NEXT(action_age, NEXT_ENTRY(COMMON_UNSIGNED)),
>   		.call = parse_vc_conf,
>   	},
> +	[ACTION_AGE_UPDATE] = {
> +		.name = "age_update",
> +		.help = "update aging parameter",
> +		.next = NEXT(action_age_update),
> +		.priv = PRIV_ACTION(AGE,
> +				    sizeof(struct rte_flow_update_age)),
> +		.call = parse_vc,
> +	},
> +	[ACTION_AGE_UPDATE_TIMEOUT] = {
> +		.name = "timeout",
> +		.help = "age timeout update value",
> +		.args = ARGS(ARGS_ENTRY_BF(struct rte_flow_update_age,
> +					   timeout, 24)),
> +		.next = NEXT(action_age_update, NEXT_ENTRY(COMMON_UNSIGNED)),
> +		.call = parse_vc_conf_timeout,
> +	},
> +	[ACTION_AGE_UPDATE_TOUCH] = {
> +		.name = "touch",
> +		.help = "this flow is touched",
> +		.next = NEXT(action_age_update, NEXT_ENTRY(COMMON_BOOLEAN)),
> +		.args = ARGS(ARGS_ENTRY_BF(struct rte_flow_update_age,
> +					   touch, 1)),
> +		.call = parse_vc_conf,
> +	},
>   	[ACTION_SAMPLE] = {
>   		.name = "sample",
>   		.help = "set a sample action",
> @@ -7031,6 +7070,33 @@ parse_vc_conf(struct context *ctx, const struct token *token,
>   	return len;
>   }
>   
> +/** Parse action configuration field. */
> +static int
> +parse_vc_conf_timeout(struct context *ctx, const struct token *token,
> +		      const char *str, unsigned int len,
> +		      void *buf, unsigned int size)
> +{
> +	struct buffer *out = buf;
> +	struct rte_flow_update_age *update;
> +
> +	(void)size;
> +	if (ctx->curr != ACTION_AGE_UPDATE_TIMEOUT)
> +		return -1;
> +	/* 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;
> +	/* Point to selected object. */
> +	ctx->object = out->args.vc.data;
> +	ctx->objmask = NULL;
> +	/* Update the timeout is valid. */
> +	update = (struct rte_flow_update_age *)out->args.vc.data;
> +	update->timeout_valid = 1;
> +	return len;
> +}
> +
>   /** Parse eCPRI common header type field. */
>   static int
>   parse_vc_item_ecpri_type(struct context *ctx, const struct token *token,
> diff --git a/app/test-pmd/config.c b/app/test-pmd/config.c
> index 31952467fb..45495385d7 100644
> --- a/app/test-pmd/config.c
> +++ b/app/test-pmd/config.c
> @@ -2065,6 +2065,7 @@ port_action_handle_update(portid_t port_id, uint32_t id,
>   	if (!pia)
>   		return -EINVAL;
>   	switch (pia->type) {
> +	case RTE_FLOW_ACTION_TYPE_AGE:
>   	case RTE_FLOW_ACTION_TYPE_CONNTRACK:
>   		update = action->conf;
>   		break;
> @@ -2904,6 +2905,8 @@ port_queue_action_handle_update(portid_t port_id,
>   	struct rte_port *port;
>   	struct rte_flow_error error;
>   	struct rte_flow_action_handle *action_handle;
> +	struct port_indirect_action *pia;
> +	const void *update;
>   
>   	action_handle = port_action_handle_get_by_id(port_id, id);
>   	if (!action_handle)
> @@ -2915,8 +2918,21 @@ port_queue_action_handle_update(portid_t port_id,
>   		return -EINVAL;
>   	}
>   
> +	pia = action_get_by_id(port_id, id);
> +	if (!pia)
> +		return -EINVAL;
> +
> +	switch (pia->type) {
> +	case RTE_FLOW_ACTION_TYPE_AGE:
> +		update = action->conf;
> +		break;
> +	default:
> +		update = action;
> +		break;
> +	}
> +
>   	if (rte_flow_async_action_handle_update(port_id, queue_id, &attr,
> -				    action_handle, action, NULL, &error)) {
> +				    action_handle, update, NULL, &error)) {
>   		return port_flow_complain(&error);
>   	}
>   	printf("Indirect action #%u update queued\n", id);
> diff --git a/doc/guides/prog_guide/rte_flow.rst b/doc/guides/prog_guide/rte_flow.rst
> index 588914b231..dae9121279 100644
> --- a/doc/guides/prog_guide/rte_flow.rst
> +++ b/doc/guides/prog_guide/rte_flow.rst
> @@ -2958,7 +2958,7 @@ Otherwise, RTE_FLOW_ERROR_TYPE_ACTION error will be returned.
>   Action: ``AGE``
>   ^^^^^^^^^^^^^^^
>   
> -Set ageing timeout configuration to a flow.
> +Set aging timeout configuration to a flow.
>   
>   Event RTE_ETH_EVENT_FLOW_AGED will be reported if
>   timeout passed without any matching on the flow.
> @@ -2977,8 +2977,8 @@ timeout passed without any matching on the flow.
>      | ``context``  | user input flow context         |
>      +--------------+---------------------------------+
>   
> -Query structure to retrieve ageing status information of a
> -shared AGE action, or a flow rule using the AGE action:
> +Query structure to retrieve aging status information of an
> +indirect AGE action, or a flow rule using the AGE action:
>   
>   .. _table_rte_flow_query_age:
>   
> @@ -2994,6 +2994,25 @@ shared AGE action, or a flow rule using the AGE action:
>      | ``sec_since_last_hit``       | out | Seconds since last traffic hit         |
>      +------------------------------+-----+----------------------------------------+
>   
> +Update structure to modify the parameters of an indirect AGE action.
> +The update structure is used by ``rte_flow_action_handle_update()`` function.
> +
> +.. _table_rte_flow_update_age:
> +
> +.. table:: AGE update
> +
> +   +-------------------+--------------------------------------------------------------+
> +   | Field             | Value                                                        |
> +   +===================+==============================================================+
> +   | ``timeout``       | 24 bits timeout value                                        |
> +   +-------------------+--------------------------------------------------------------+
> +   | ``timeout_valid`` | 1 bit, timeout value is valid                                |
> +   +-------------------+--------------------------------------------------------------+
> +   | ``touch``         | 1 bit, touch the AGE action to set ``sec_since_last_hit`` 0  |
> +   +-------------------+--------------------------------------------------------------+
> +   | ``reserved``      | 6 bits reserved, must be zero                                |
> +   +-------------------+--------------------------------------------------------------+
> +
>   Action: ``SAMPLE``
>   ^^^^^^^^^^^^^^^^^^
>   
> diff --git a/lib/ethdev/rte_flow.h b/lib/ethdev/rte_flow.h
> index d830b02321..a21d437cf8 100644
> --- a/lib/ethdev/rte_flow.h
> +++ b/lib/ethdev/rte_flow.h
> @@ -2954,6 +2954,33 @@ struct rte_flow_query_age {
>   	uint32_t sec_since_last_hit:24; /**< Seconds since last traffic hit. */
>   };
>   
> +/**
> + * @warning
> + * @b EXPERIMENTAL: this structure may change without prior notice
> + *
> + * RTE_FLOW_ACTION_TYPE_AGE
> + *
> + * Update indirect AGE action attributes:
> + *  - Timeout can be updated including stop/start action:
> + *     +-------------+-------------+------------------------------+
> + *     | Old Timeout | New Timeout | Updating                     |
> + *     +=============+=============+==============================+
> + *     | 0           | positive    | Start aging with new value   |
> + *     +-------------+-------------+------------------------------+
> + *     | positive    | 0           | Stop aging			  |
> + *     +-------------+-------------+------------------------------+
> + *     | positive    | positive    | Change timeout to new value  |
> + *     +-------------+-------------+------------------------------+
> + *  - sec_since_last_hit can be reset.
> + */
> +struct rte_flow_update_age {

I think it worse to mention the structure in
RTE_FLOW_ACTION_TYPE_AGE description.

> +	uint32_t reserved:6; /**< Reserved, must be zero. */
> +	uint32_t timeout_valid:1; /**< The timeout is valid for update. */
> +	uint32_t timeout:24; /**< Time in seconds. */
> +	uint32_t touch:1;
> +	/**< Means that aging should assume packet passed the aging. */

Documentation after code makes sense if it is in the same line.
If the documentation is in its own line, it should be before
the code and use /** prefix.

One more question: why order in the documentation above does
not match order here?

> +};
> +
>   /**
>    * @warning
>    * @b EXPERIMENTAL: this structure may change without prior notice


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

* RE: [PATCH 3/3] ethdev: add structure for indirect AGE update
  2022-10-03  8:03   ` Andrew Rybchenko
@ 2022-10-03  8:30     ` Ori Kam
  2022-10-03 13:17       ` Andrew Rybchenko
  0 siblings, 1 reply; 32+ messages in thread
From: Ori Kam @ 2022-10-03  8:30 UTC (permalink / raw)
  To: Andrew Rybchenko, Michael Baum, dev; +Cc: Matan Azrad, Raslan Darawsheh

Hi Andrew,

> -----Original Message-----
> From: Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>
> Sent: Monday, 3 October 2022 11:04
> 
> On 9/21/22 17:54, Michael Baum wrote:
> > Add a new structure for indirect AGE update.
> >
> > This new structure enables:
> > 1. Update timeout value.
> > 2. Stop AGE checking.
> > 3. Start AGE checking.
> > 4. restart AGE checking.
> >
> > Signed-off-by: Michael Baum <michaelba@nvidia.com>
> > ---
> >   app/test-pmd/cmdline_flow.c        | 66
> ++++++++++++++++++++++++++++++
> >   app/test-pmd/config.c              | 18 +++++++-
> >   doc/guides/prog_guide/rte_flow.rst | 25 +++++++++--
> >   lib/ethdev/rte_flow.h              | 27 ++++++++++++
> >   4 files changed, 132 insertions(+), 4 deletions(-)
> >
> > diff --git a/app/test-pmd/cmdline_flow.c b/app/test-pmd/cmdline_flow.c
> > index 4fb90a92cb..a315fd9ded 100644
> > --- a/app/test-pmd/cmdline_flow.c
> > +++ b/app/test-pmd/cmdline_flow.c
> > @@ -583,6 +583,9 @@ enum index {
> >   	ACTION_SET_IPV6_DSCP_VALUE,
> >   	ACTION_AGE,
> >   	ACTION_AGE_TIMEOUT,
> > +	ACTION_AGE_UPDATE,
> > +	ACTION_AGE_UPDATE_TIMEOUT,
> > +	ACTION_AGE_UPDATE_TOUCH,
> >   	ACTION_SAMPLE,
> >   	ACTION_SAMPLE_RATIO,
> >   	ACTION_SAMPLE_INDEX,
> > @@ -1869,6 +1872,7 @@ static const enum index next_action[] = {
> >   	ACTION_SET_IPV4_DSCP,
> >   	ACTION_SET_IPV6_DSCP,
> >   	ACTION_AGE,
> > +	ACTION_AGE_UPDATE,
> >   	ACTION_SAMPLE,
> >   	ACTION_INDIRECT,
> >   	ACTION_MODIFY_FIELD,
> > @@ -2113,6 +2117,14 @@ static const enum index action_age[] = {
> >   	ZERO,
> >   };
> >
> > +static const enum index action_age_update[] = {
> > +	ACTION_AGE_UPDATE,
> > +	ACTION_AGE_UPDATE_TIMEOUT,
> > +	ACTION_AGE_UPDATE_TOUCH,
> > +	ACTION_NEXT,
> > +	ZERO,
> > +};
> > +
> >   static const enum index action_sample[] = {
> >   	ACTION_SAMPLE,
> >   	ACTION_SAMPLE_RATIO,
> > @@ -2191,6 +2203,9 @@ static int parse_vc_spec(struct context *, const
> struct token *,
> >   			 const char *, unsigned int, void *, unsigned int);
> >   static int parse_vc_conf(struct context *, const struct token *,
> >   			 const char *, unsigned int, void *, unsigned int);
> > +static int parse_vc_conf_timeout(struct context *, const struct token *,
> > +				 const char *, unsigned int, void *,
> > +				 unsigned int);
> >   static int parse_vc_item_ecpri_type(struct context *, const struct token *,
> >   				    const char *, unsigned int,
> >   				    void *, unsigned int);
> > @@ -6194,6 +6209,30 @@ static const struct token token_list[] = {
> >   		.next = NEXT(action_age,
> NEXT_ENTRY(COMMON_UNSIGNED)),
> >   		.call = parse_vc_conf,
> >   	},
> > +	[ACTION_AGE_UPDATE] = {
> > +		.name = "age_update",
> > +		.help = "update aging parameter",
> > +		.next = NEXT(action_age_update),
> > +		.priv = PRIV_ACTION(AGE,
> > +				    sizeof(struct rte_flow_update_age)),
> > +		.call = parse_vc,
> > +	},
> > +	[ACTION_AGE_UPDATE_TIMEOUT] = {
> > +		.name = "timeout",
> > +		.help = "age timeout update value",
> > +		.args = ARGS(ARGS_ENTRY_BF(struct rte_flow_update_age,
> > +					   timeout, 24)),
> > +		.next = NEXT(action_age_update,
> NEXT_ENTRY(COMMON_UNSIGNED)),
> > +		.call = parse_vc_conf_timeout,
> > +	},
> > +	[ACTION_AGE_UPDATE_TOUCH] = {
> > +		.name = "touch",
> > +		.help = "this flow is touched",
> > +		.next = NEXT(action_age_update,
> NEXT_ENTRY(COMMON_BOOLEAN)),
> > +		.args = ARGS(ARGS_ENTRY_BF(struct rte_flow_update_age,
> > +					   touch, 1)),
> > +		.call = parse_vc_conf,
> > +	},
> >   	[ACTION_SAMPLE] = {
> >   		.name = "sample",
> >   		.help = "set a sample action",
> > @@ -7031,6 +7070,33 @@ parse_vc_conf(struct context *ctx, const struct
> token *token,
> >   	return len;
> >   }
> >
> > +/** Parse action configuration field. */
> > +static int
> > +parse_vc_conf_timeout(struct context *ctx, const struct token *token,
> > +		      const char *str, unsigned int len,
> > +		      void *buf, unsigned int size)
> > +{
> > +	struct buffer *out = buf;
> > +	struct rte_flow_update_age *update;
> > +
> > +	(void)size;
> > +	if (ctx->curr != ACTION_AGE_UPDATE_TIMEOUT)
> > +		return -1;
> > +	/* 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;
> > +	/* Point to selected object. */
> > +	ctx->object = out->args.vc.data;
> > +	ctx->objmask = NULL;
> > +	/* Update the timeout is valid. */
> > +	update = (struct rte_flow_update_age *)out->args.vc.data;
> > +	update->timeout_valid = 1;
> > +	return len;
> > +}
> > +
> >   /** Parse eCPRI common header type field. */
> >   static int
> >   parse_vc_item_ecpri_type(struct context *ctx, const struct token *token,
> > diff --git a/app/test-pmd/config.c b/app/test-pmd/config.c
> > index 31952467fb..45495385d7 100644
> > --- a/app/test-pmd/config.c
> > +++ b/app/test-pmd/config.c
> > @@ -2065,6 +2065,7 @@ port_action_handle_update(portid_t port_id,
> uint32_t id,
> >   	if (!pia)
> >   		return -EINVAL;
> >   	switch (pia->type) {
> > +	case RTE_FLOW_ACTION_TYPE_AGE:
> >   	case RTE_FLOW_ACTION_TYPE_CONNTRACK:
> >   		update = action->conf;
> >   		break;
> > @@ -2904,6 +2905,8 @@ port_queue_action_handle_update(portid_t
> port_id,
> >   	struct rte_port *port;
> >   	struct rte_flow_error error;
> >   	struct rte_flow_action_handle *action_handle;
> > +	struct port_indirect_action *pia;
> > +	const void *update;
> >
> >   	action_handle = port_action_handle_get_by_id(port_id, id);
> >   	if (!action_handle)
> > @@ -2915,8 +2918,21 @@ port_queue_action_handle_update(portid_t
> port_id,
> >   		return -EINVAL;
> >   	}
> >
> > +	pia = action_get_by_id(port_id, id);
> > +	if (!pia)
> > +		return -EINVAL;
> > +
> > +	switch (pia->type) {
> > +	case RTE_FLOW_ACTION_TYPE_AGE:
> > +		update = action->conf;
> > +		break;
> > +	default:
> > +		update = action;
> > +		break;
> > +	}
> > +
> >   	if (rte_flow_async_action_handle_update(port_id, queue_id, &attr,
> > -				    action_handle, action, NULL, &error)) {
> > +				    action_handle, update, NULL, &error)) {
> >   		return port_flow_complain(&error);
> >   	}
> >   	printf("Indirect action #%u update queued\n", id);
> > diff --git a/doc/guides/prog_guide/rte_flow.rst
> b/doc/guides/prog_guide/rte_flow.rst
> > index 588914b231..dae9121279 100644
> > --- a/doc/guides/prog_guide/rte_flow.rst
> > +++ b/doc/guides/prog_guide/rte_flow.rst
> > @@ -2958,7 +2958,7 @@ Otherwise, RTE_FLOW_ERROR_TYPE_ACTION
> error will be returned.
> >   Action: ``AGE``
> >   ^^^^^^^^^^^^^^^
> >
> > -Set ageing timeout configuration to a flow.
> > +Set aging timeout configuration to a flow.
> >
> >   Event RTE_ETH_EVENT_FLOW_AGED will be reported if
> >   timeout passed without any matching on the flow.
> > @@ -2977,8 +2977,8 @@ timeout passed without any matching on the
> flow.
> >      | ``context``  | user input flow context         |
> >      +--------------+---------------------------------+
> >
> > -Query structure to retrieve ageing status information of a
> > -shared AGE action, or a flow rule using the AGE action:
> > +Query structure to retrieve aging status information of an
> > +indirect AGE action, or a flow rule using the AGE action:
> >
> >   .. _table_rte_flow_query_age:
> >
> > @@ -2994,6 +2994,25 @@ shared AGE action, or a flow rule using the AGE
> action:
> >      | ``sec_since_last_hit``       | out | Seconds since last traffic hit         |
> >      +------------------------------+-----+----------------------------------------+
> >
> > +Update structure to modify the parameters of an indirect AGE action.
> > +The update structure is used by ``rte_flow_action_handle_update()``
> function.
> > +
> > +.. _table_rte_flow_update_age:
> > +
> > +.. table:: AGE update
> > +
> > +   +-------------------+--------------------------------------------------------------+
> > +   | Field             | Value                                                        |
> > +
> +===================+=====================================
> =========================+
> > +   | ``timeout``       | 24 bits timeout value                                        |
> > +   +-------------------+--------------------------------------------------------------+
> > +   | ``timeout_valid`` | 1 bit, timeout value is valid                                |
> > +   +-------------------+--------------------------------------------------------------+
> > +   | ``touch``         | 1 bit, touch the AGE action to set ``sec_since_last_hit`` 0
> |
> > +   +-------------------+--------------------------------------------------------------+
> > +   | ``reserved``      | 6 bits reserved, must be zero                                |
> > +   +-------------------+--------------------------------------------------------------+
> > +
> >   Action: ``SAMPLE``
> >   ^^^^^^^^^^^^^^^^^^
> >
> > diff --git a/lib/ethdev/rte_flow.h b/lib/ethdev/rte_flow.h
> > index d830b02321..a21d437cf8 100644
> > --- a/lib/ethdev/rte_flow.h
> > +++ b/lib/ethdev/rte_flow.h
> > @@ -2954,6 +2954,33 @@ struct rte_flow_query_age {
> >   	uint32_t sec_since_last_hit:24; /**< Seconds since last traffic hit. */
> >   };
> >
> > +/**
> > + * @warning
> > + * @b EXPERIMENTAL: this structure may change without prior notice
> > + *
> > + * RTE_FLOW_ACTION_TYPE_AGE
> > + *
> > + * Update indirect AGE action attributes:
> > + *  - Timeout can be updated including stop/start action:
> > + *     +-------------+-------------+------------------------------+
> > + *     | Old Timeout | New Timeout | Updating                     |
> > + *
> +=============+=============+=============================
> =+
> > + *     | 0           | positive    | Start aging with new value   |
> > + *     +-------------+-------------+------------------------------+
> > + *     | positive    | 0           | Stop aging			  |
> > + *     +-------------+-------------+------------------------------+
> > + *     | positive    | positive    | Change timeout to new value  |
> > + *     +-------------+-------------+------------------------------+
> > + *  - sec_since_last_hit can be reset.
> > + */
> > +struct rte_flow_update_age {
> 
> I think it worse to mention the structure in
> RTE_FLOW_ACTION_TYPE_AGE description.

What do you mean?

> 
> > +	uint32_t reserved:6; /**< Reserved, must be zero. */
> > +	uint32_t timeout_valid:1; /**< The timeout is valid for update. */
> > +	uint32_t timeout:24; /**< Time in seconds. */
> > +	uint32_t touch:1;
> > +	/**< Means that aging should assume packet passed the aging. */
> 
> Documentation after code makes sense if it is in the same line.
> If the documentation is in its own line, it should be before
> the code and use /** prefix.
> 

+1

> One more question: why order in the documentation above does
> not match order here?
> 

I think we can remove it from the documentation since it doesn't add anything

> > +};
> > +
> >   /**
> >    * @warning
> >    * @b EXPERIMENTAL: this structure may change without prior notice


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

* Re: [PATCH 3/3] ethdev: add structure for indirect AGE update
  2022-10-03  8:30     ` Ori Kam
@ 2022-10-03 13:17       ` Andrew Rybchenko
  2022-10-03 15:13         ` Ori Kam
  0 siblings, 1 reply; 32+ messages in thread
From: Andrew Rybchenko @ 2022-10-03 13:17 UTC (permalink / raw)
  To: Ori Kam, Michael Baum, dev; +Cc: Matan Azrad, Raslan Darawsheh

On 10/3/22 11:30, Ori Kam wrote:
> Hi Andrew,
> 
>> -----Original Message-----
>> From: Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>
>> Sent: Monday, 3 October 2022 11:04
>>
>> On 9/21/22 17:54, Michael Baum wrote:
>>> Add a new structure for indirect AGE update.
>>>
>>> This new structure enables:
>>> 1. Update timeout value.
>>> 2. Stop AGE checking.
>>> 3. Start AGE checking.
>>> 4. restart AGE checking.
>>>
>>> Signed-off-by: Michael Baum <michaelba@nvidia.com>
>>> ---
>>>    app/test-pmd/cmdline_flow.c        | 66
>> ++++++++++++++++++++++++++++++
>>>    app/test-pmd/config.c              | 18 +++++++-
>>>    doc/guides/prog_guide/rte_flow.rst | 25 +++++++++--
>>>    lib/ethdev/rte_flow.h              | 27 ++++++++++++
>>>    4 files changed, 132 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/app/test-pmd/cmdline_flow.c b/app/test-pmd/cmdline_flow.c
>>> index 4fb90a92cb..a315fd9ded 100644
>>> --- a/app/test-pmd/cmdline_flow.c
>>> +++ b/app/test-pmd/cmdline_flow.c
>>> @@ -583,6 +583,9 @@ enum index {
>>>    	ACTION_SET_IPV6_DSCP_VALUE,
>>>    	ACTION_AGE,
>>>    	ACTION_AGE_TIMEOUT,
>>> +	ACTION_AGE_UPDATE,
>>> +	ACTION_AGE_UPDATE_TIMEOUT,
>>> +	ACTION_AGE_UPDATE_TOUCH,
>>>    	ACTION_SAMPLE,
>>>    	ACTION_SAMPLE_RATIO,
>>>    	ACTION_SAMPLE_INDEX,
>>> @@ -1869,6 +1872,7 @@ static const enum index next_action[] = {
>>>    	ACTION_SET_IPV4_DSCP,
>>>    	ACTION_SET_IPV6_DSCP,
>>>    	ACTION_AGE,
>>> +	ACTION_AGE_UPDATE,
>>>    	ACTION_SAMPLE,
>>>    	ACTION_INDIRECT,
>>>    	ACTION_MODIFY_FIELD,
>>> @@ -2113,6 +2117,14 @@ static const enum index action_age[] = {
>>>    	ZERO,
>>>    };
>>>
>>> +static const enum index action_age_update[] = {
>>> +	ACTION_AGE_UPDATE,
>>> +	ACTION_AGE_UPDATE_TIMEOUT,
>>> +	ACTION_AGE_UPDATE_TOUCH,
>>> +	ACTION_NEXT,
>>> +	ZERO,
>>> +};
>>> +
>>>    static const enum index action_sample[] = {
>>>    	ACTION_SAMPLE,
>>>    	ACTION_SAMPLE_RATIO,
>>> @@ -2191,6 +2203,9 @@ static int parse_vc_spec(struct context *, const
>> struct token *,
>>>    			 const char *, unsigned int, void *, unsigned int);
>>>    static int parse_vc_conf(struct context *, const struct token *,
>>>    			 const char *, unsigned int, void *, unsigned int);
>>> +static int parse_vc_conf_timeout(struct context *, const struct token *,
>>> +				 const char *, unsigned int, void *,
>>> +				 unsigned int);
>>>    static int parse_vc_item_ecpri_type(struct context *, const struct token *,
>>>    				    const char *, unsigned int,
>>>    				    void *, unsigned int);
>>> @@ -6194,6 +6209,30 @@ static const struct token token_list[] = {
>>>    		.next = NEXT(action_age,
>> NEXT_ENTRY(COMMON_UNSIGNED)),
>>>    		.call = parse_vc_conf,
>>>    	},
>>> +	[ACTION_AGE_UPDATE] = {
>>> +		.name = "age_update",
>>> +		.help = "update aging parameter",
>>> +		.next = NEXT(action_age_update),
>>> +		.priv = PRIV_ACTION(AGE,
>>> +				    sizeof(struct rte_flow_update_age)),
>>> +		.call = parse_vc,
>>> +	},
>>> +	[ACTION_AGE_UPDATE_TIMEOUT] = {
>>> +		.name = "timeout",
>>> +		.help = "age timeout update value",
>>> +		.args = ARGS(ARGS_ENTRY_BF(struct rte_flow_update_age,
>>> +					   timeout, 24)),
>>> +		.next = NEXT(action_age_update,
>> NEXT_ENTRY(COMMON_UNSIGNED)),
>>> +		.call = parse_vc_conf_timeout,
>>> +	},
>>> +	[ACTION_AGE_UPDATE_TOUCH] = {
>>> +		.name = "touch",
>>> +		.help = "this flow is touched",
>>> +		.next = NEXT(action_age_update,
>> NEXT_ENTRY(COMMON_BOOLEAN)),
>>> +		.args = ARGS(ARGS_ENTRY_BF(struct rte_flow_update_age,
>>> +					   touch, 1)),
>>> +		.call = parse_vc_conf,
>>> +	},
>>>    	[ACTION_SAMPLE] = {
>>>    		.name = "sample",
>>>    		.help = "set a sample action",
>>> @@ -7031,6 +7070,33 @@ parse_vc_conf(struct context *ctx, const struct
>> token *token,
>>>    	return len;
>>>    }
>>>
>>> +/** Parse action configuration field. */
>>> +static int
>>> +parse_vc_conf_timeout(struct context *ctx, const struct token *token,
>>> +		      const char *str, unsigned int len,
>>> +		      void *buf, unsigned int size)
>>> +{
>>> +	struct buffer *out = buf;
>>> +	struct rte_flow_update_age *update;
>>> +
>>> +	(void)size;
>>> +	if (ctx->curr != ACTION_AGE_UPDATE_TIMEOUT)
>>> +		return -1;
>>> +	/* 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;
>>> +	/* Point to selected object. */
>>> +	ctx->object = out->args.vc.data;
>>> +	ctx->objmask = NULL;
>>> +	/* Update the timeout is valid. */
>>> +	update = (struct rte_flow_update_age *)out->args.vc.data;
>>> +	update->timeout_valid = 1;
>>> +	return len;
>>> +}
>>> +
>>>    /** Parse eCPRI common header type field. */
>>>    static int
>>>    parse_vc_item_ecpri_type(struct context *ctx, const struct token *token,
>>> diff --git a/app/test-pmd/config.c b/app/test-pmd/config.c
>>> index 31952467fb..45495385d7 100644
>>> --- a/app/test-pmd/config.c
>>> +++ b/app/test-pmd/config.c
>>> @@ -2065,6 +2065,7 @@ port_action_handle_update(portid_t port_id,
>> uint32_t id,
>>>    	if (!pia)
>>>    		return -EINVAL;
>>>    	switch (pia->type) {
>>> +	case RTE_FLOW_ACTION_TYPE_AGE:
>>>    	case RTE_FLOW_ACTION_TYPE_CONNTRACK:
>>>    		update = action->conf;
>>>    		break;
>>> @@ -2904,6 +2905,8 @@ port_queue_action_handle_update(portid_t
>> port_id,
>>>    	struct rte_port *port;
>>>    	struct rte_flow_error error;
>>>    	struct rte_flow_action_handle *action_handle;
>>> +	struct port_indirect_action *pia;
>>> +	const void *update;
>>>
>>>    	action_handle = port_action_handle_get_by_id(port_id, id);
>>>    	if (!action_handle)
>>> @@ -2915,8 +2918,21 @@ port_queue_action_handle_update(portid_t
>> port_id,
>>>    		return -EINVAL;
>>>    	}
>>>
>>> +	pia = action_get_by_id(port_id, id);
>>> +	if (!pia)
>>> +		return -EINVAL;
>>> +
>>> +	switch (pia->type) {
>>> +	case RTE_FLOW_ACTION_TYPE_AGE:
>>> +		update = action->conf;
>>> +		break;
>>> +	default:
>>> +		update = action;
>>> +		break;
>>> +	}
>>> +
>>>    	if (rte_flow_async_action_handle_update(port_id, queue_id, &attr,
>>> -				    action_handle, action, NULL, &error)) {
>>> +				    action_handle, update, NULL, &error)) {
>>>    		return port_flow_complain(&error);
>>>    	}
>>>    	printf("Indirect action #%u update queued\n", id);
>>> diff --git a/doc/guides/prog_guide/rte_flow.rst
>> b/doc/guides/prog_guide/rte_flow.rst
>>> index 588914b231..dae9121279 100644
>>> --- a/doc/guides/prog_guide/rte_flow.rst
>>> +++ b/doc/guides/prog_guide/rte_flow.rst
>>> @@ -2958,7 +2958,7 @@ Otherwise, RTE_FLOW_ERROR_TYPE_ACTION
>> error will be returned.
>>>    Action: ``AGE``
>>>    ^^^^^^^^^^^^^^^
>>>
>>> -Set ageing timeout configuration to a flow.
>>> +Set aging timeout configuration to a flow.
>>>
>>>    Event RTE_ETH_EVENT_FLOW_AGED will be reported if
>>>    timeout passed without any matching on the flow.
>>> @@ -2977,8 +2977,8 @@ timeout passed without any matching on the
>> flow.
>>>       | ``context``  | user input flow context         |
>>>       +--------------+---------------------------------+
>>>
>>> -Query structure to retrieve ageing status information of a
>>> -shared AGE action, or a flow rule using the AGE action:
>>> +Query structure to retrieve aging status information of an
>>> +indirect AGE action, or a flow rule using the AGE action:
>>>
>>>    .. _table_rte_flow_query_age:
>>>
>>> @@ -2994,6 +2994,25 @@ shared AGE action, or a flow rule using the AGE
>> action:
>>>       | ``sec_since_last_hit``       | out | Seconds since last traffic hit         |
>>>       +------------------------------+-----+----------------------------------------+
>>>
>>> +Update structure to modify the parameters of an indirect AGE action.
>>> +The update structure is used by ``rte_flow_action_handle_update()``
>> function.
>>> +
>>> +.. _table_rte_flow_update_age:
>>> +
>>> +.. table:: AGE update
>>> +
>>> +   +-------------------+--------------------------------------------------------------+
>>> +   | Field             | Value                                                        |
>>> +
>> +===================+=====================================
>> =========================+
>>> +   | ``timeout``       | 24 bits timeout value                                        |
>>> +   +-------------------+--------------------------------------------------------------+
>>> +   | ``timeout_valid`` | 1 bit, timeout value is valid                                |
>>> +   +-------------------+--------------------------------------------------------------+
>>> +   | ``touch``         | 1 bit, touch the AGE action to set ``sec_since_last_hit`` 0
>> |
>>> +   +-------------------+--------------------------------------------------------------+
>>> +   | ``reserved``      | 6 bits reserved, must be zero                                |
>>> +   +-------------------+--------------------------------------------------------------+
>>> +
>>>    Action: ``SAMPLE``
>>>    ^^^^^^^^^^^^^^^^^^
>>>
>>> diff --git a/lib/ethdev/rte_flow.h b/lib/ethdev/rte_flow.h
>>> index d830b02321..a21d437cf8 100644
>>> --- a/lib/ethdev/rte_flow.h
>>> +++ b/lib/ethdev/rte_flow.h
>>> @@ -2954,6 +2954,33 @@ struct rte_flow_query_age {
>>>    	uint32_t sec_since_last_hit:24; /**< Seconds since last traffic hit. */
>>>    };
>>>
>>> +/**
>>> + * @warning
>>> + * @b EXPERIMENTAL: this structure may change without prior notice
>>> + *
>>> + * RTE_FLOW_ACTION_TYPE_AGE
>>> + *
>>> + * Update indirect AGE action attributes:
>>> + *  - Timeout can be updated including stop/start action:
>>> + *     +-------------+-------------+------------------------------+
>>> + *     | Old Timeout | New Timeout | Updating                     |
>>> + *
>> +=============+=============+=============================
>> =+
>>> + *     | 0           | positive    | Start aging with new value   |
>>> + *     +-------------+-------------+------------------------------+
>>> + *     | positive    | 0           | Stop aging			  |
>>> + *     +-------------+-------------+------------------------------+
>>> + *     | positive    | positive    | Change timeout to new value  |
>>> + *     +-------------+-------------+------------------------------+
>>> + *  - sec_since_last_hit can be reset.
>>> + */
>>> +struct rte_flow_update_age {
>>
>> I think it worse to mention the structure in
>> RTE_FLOW_ACTION_TYPE_AGE description.
> 
> What do you mean?

RTE_FLOW_ACTION_TYPE_AGE has a description as enum member.
Typically configuration structures are mentioned there.
However, I think that it would be useful to mention specific
update structure if any. Like this one. Just to make it clear
that if a user wants to update the action configuration it
should use specific structure. Or am I missing something?
Misunderstand something?

> 
>>
>>> +	uint32_t reserved:6; /**< Reserved, must be zero. */
>>> +	uint32_t timeout_valid:1; /**< The timeout is valid for update. */
>>> +	uint32_t timeout:24; /**< Time in seconds. */
>>> +	uint32_t touch:1;
>>> +	/**< Means that aging should assume packet passed the aging. */
>>
>> Documentation after code makes sense if it is in the same line.
>> If the documentation is in its own line, it should be before
>> the code and use /** prefix.
>>
> 
> +1
> 
>> One more question: why order in the documentation above does
>> not match order here?
>>
> 
> I think we can remove it from the documentation since it doesn't add anything
> 
>>> +};
>>> +
>>>    /**
>>>     * @warning
>>>     * @b EXPERIMENTAL: this structure may change without prior notice
> 


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

* RE: [PATCH 3/3] ethdev: add structure for indirect AGE update
  2022-10-03 13:17       ` Andrew Rybchenko
@ 2022-10-03 15:13         ` Ori Kam
  2022-10-04  6:36           ` Andrew Rybchenko
  0 siblings, 1 reply; 32+ messages in thread
From: Ori Kam @ 2022-10-03 15:13 UTC (permalink / raw)
  To: Andrew Rybchenko, Michael Baum, dev; +Cc: Matan Azrad, Raslan Darawsheh

Hi Andrew,

> -----Original Message-----
> From: Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>
> Sent: Monday, 3 October 2022 16:18
> 
> On 10/3/22 11:30, Ori Kam wrote:
> > Hi Andrew,
> >
> >> -----Original Message-----
> >> From: Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>
> >> Sent: Monday, 3 October 2022 11:04
> >>
> >> On 9/21/22 17:54, Michael Baum wrote:
> >>> Add a new structure for indirect AGE update.
> >>>
> >>> This new structure enables:
> >>> 1. Update timeout value.
> >>> 2. Stop AGE checking.
> >>> 3. Start AGE checking.
> >>> 4. restart AGE checking.
> >>>
> >>> Signed-off-by: Michael Baum <michaelba@nvidia.com>
> >>> ---
> >>>    app/test-pmd/cmdline_flow.c        | 66
> >> ++++++++++++++++++++++++++++++
> >>>    app/test-pmd/config.c              | 18 +++++++-
> >>>    doc/guides/prog_guide/rte_flow.rst | 25 +++++++++--
> >>>    lib/ethdev/rte_flow.h              | 27 ++++++++++++
> >>>    4 files changed, 132 insertions(+), 4 deletions(-)
> >>>
> >>> diff --git a/app/test-pmd/cmdline_flow.c b/app/test-
> pmd/cmdline_flow.c
> >>> index 4fb90a92cb..a315fd9ded 100644
> >>> --- a/app/test-pmd/cmdline_flow.c
> >>> +++ b/app/test-pmd/cmdline_flow.c
> >>> @@ -583,6 +583,9 @@ enum index {
> >>>    	ACTION_SET_IPV6_DSCP_VALUE,
> >>>    	ACTION_AGE,
> >>>    	ACTION_AGE_TIMEOUT,
> >>> +	ACTION_AGE_UPDATE,
> >>> +	ACTION_AGE_UPDATE_TIMEOUT,
> >>> +	ACTION_AGE_UPDATE_TOUCH,
> >>>    	ACTION_SAMPLE,
> >>>    	ACTION_SAMPLE_RATIO,
> >>>    	ACTION_SAMPLE_INDEX,
> >>> @@ -1869,6 +1872,7 @@ static const enum index next_action[] = {
> >>>    	ACTION_SET_IPV4_DSCP,
> >>>    	ACTION_SET_IPV6_DSCP,
> >>>    	ACTION_AGE,
> >>> +	ACTION_AGE_UPDATE,
> >>>    	ACTION_SAMPLE,
> >>>    	ACTION_INDIRECT,
> >>>    	ACTION_MODIFY_FIELD,
> >>> @@ -2113,6 +2117,14 @@ static const enum index action_age[] = {
> >>>    	ZERO,
> >>>    };
> >>>
> >>> +static const enum index action_age_update[] = {
> >>> +	ACTION_AGE_UPDATE,
> >>> +	ACTION_AGE_UPDATE_TIMEOUT,
> >>> +	ACTION_AGE_UPDATE_TOUCH,
> >>> +	ACTION_NEXT,
> >>> +	ZERO,
> >>> +};
> >>> +
> >>>    static const enum index action_sample[] = {
> >>>    	ACTION_SAMPLE,
> >>>    	ACTION_SAMPLE_RATIO,
> >>> @@ -2191,6 +2203,9 @@ static int parse_vc_spec(struct context *, const
> >> struct token *,
> >>>    			 const char *, unsigned int, void *, unsigned int);
> >>>    static int parse_vc_conf(struct context *, const struct token *,
> >>>    			 const char *, unsigned int, void *, unsigned int);
> >>> +static int parse_vc_conf_timeout(struct context *, const struct token *,
> >>> +				 const char *, unsigned int, void *,
> >>> +				 unsigned int);
> >>>    static int parse_vc_item_ecpri_type(struct context *, const struct token
> *,
> >>>    				    const char *, unsigned int,
> >>>    				    void *, unsigned int);
> >>> @@ -6194,6 +6209,30 @@ static const struct token token_list[] = {
> >>>    		.next = NEXT(action_age,
> >> NEXT_ENTRY(COMMON_UNSIGNED)),
> >>>    		.call = parse_vc_conf,
> >>>    	},
> >>> +	[ACTION_AGE_UPDATE] = {
> >>> +		.name = "age_update",
> >>> +		.help = "update aging parameter",
> >>> +		.next = NEXT(action_age_update),
> >>> +		.priv = PRIV_ACTION(AGE,
> >>> +				    sizeof(struct rte_flow_update_age)),
> >>> +		.call = parse_vc,
> >>> +	},
> >>> +	[ACTION_AGE_UPDATE_TIMEOUT] = {
> >>> +		.name = "timeout",
> >>> +		.help = "age timeout update value",
> >>> +		.args = ARGS(ARGS_ENTRY_BF(struct rte_flow_update_age,
> >>> +					   timeout, 24)),
> >>> +		.next = NEXT(action_age_update,
> >> NEXT_ENTRY(COMMON_UNSIGNED)),
> >>> +		.call = parse_vc_conf_timeout,
> >>> +	},
> >>> +	[ACTION_AGE_UPDATE_TOUCH] = {
> >>> +		.name = "touch",
> >>> +		.help = "this flow is touched",
> >>> +		.next = NEXT(action_age_update,
> >> NEXT_ENTRY(COMMON_BOOLEAN)),
> >>> +		.args = ARGS(ARGS_ENTRY_BF(struct rte_flow_update_age,
> >>> +					   touch, 1)),
> >>> +		.call = parse_vc_conf,
> >>> +	},
> >>>    	[ACTION_SAMPLE] = {
> >>>    		.name = "sample",
> >>>    		.help = "set a sample action",
> >>> @@ -7031,6 +7070,33 @@ parse_vc_conf(struct context *ctx, const
> struct
> >> token *token,
> >>>    	return len;
> >>>    }
> >>>
> >>> +/** Parse action configuration field. */
> >>> +static int
> >>> +parse_vc_conf_timeout(struct context *ctx, const struct token *token,
> >>> +		      const char *str, unsigned int len,
> >>> +		      void *buf, unsigned int size)
> >>> +{
> >>> +	struct buffer *out = buf;
> >>> +	struct rte_flow_update_age *update;
> >>> +
> >>> +	(void)size;
> >>> +	if (ctx->curr != ACTION_AGE_UPDATE_TIMEOUT)
> >>> +		return -1;
> >>> +	/* 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;
> >>> +	/* Point to selected object. */
> >>> +	ctx->object = out->args.vc.data;
> >>> +	ctx->objmask = NULL;
> >>> +	/* Update the timeout is valid. */
> >>> +	update = (struct rte_flow_update_age *)out->args.vc.data;
> >>> +	update->timeout_valid = 1;
> >>> +	return len;
> >>> +}
> >>> +
> >>>    /** Parse eCPRI common header type field. */
> >>>    static int
> >>>    parse_vc_item_ecpri_type(struct context *ctx, const struct token
> *token,
> >>> diff --git a/app/test-pmd/config.c b/app/test-pmd/config.c
> >>> index 31952467fb..45495385d7 100644
> >>> --- a/app/test-pmd/config.c
> >>> +++ b/app/test-pmd/config.c
> >>> @@ -2065,6 +2065,7 @@ port_action_handle_update(portid_t port_id,
> >> uint32_t id,
> >>>    	if (!pia)
> >>>    		return -EINVAL;
> >>>    	switch (pia->type) {
> >>> +	case RTE_FLOW_ACTION_TYPE_AGE:
> >>>    	case RTE_FLOW_ACTION_TYPE_CONNTRACK:
> >>>    		update = action->conf;
> >>>    		break;
> >>> @@ -2904,6 +2905,8 @@ port_queue_action_handle_update(portid_t
> >> port_id,
> >>>    	struct rte_port *port;
> >>>    	struct rte_flow_error error;
> >>>    	struct rte_flow_action_handle *action_handle;
> >>> +	struct port_indirect_action *pia;
> >>> +	const void *update;
> >>>
> >>>    	action_handle = port_action_handle_get_by_id(port_id, id);
> >>>    	if (!action_handle)
> >>> @@ -2915,8 +2918,21 @@ port_queue_action_handle_update(portid_t
> >> port_id,
> >>>    		return -EINVAL;
> >>>    	}
> >>>
> >>> +	pia = action_get_by_id(port_id, id);
> >>> +	if (!pia)
> >>> +		return -EINVAL;
> >>> +
> >>> +	switch (pia->type) {
> >>> +	case RTE_FLOW_ACTION_TYPE_AGE:
> >>> +		update = action->conf;
> >>> +		break;
> >>> +	default:
> >>> +		update = action;
> >>> +		break;
> >>> +	}
> >>> +
> >>>    	if (rte_flow_async_action_handle_update(port_id, queue_id, &attr,
> >>> -				    action_handle, action, NULL, &error)) {
> >>> +				    action_handle, update, NULL, &error)) {
> >>>    		return port_flow_complain(&error);
> >>>    	}
> >>>    	printf("Indirect action #%u update queued\n", id);
> >>> diff --git a/doc/guides/prog_guide/rte_flow.rst
> >> b/doc/guides/prog_guide/rte_flow.rst
> >>> index 588914b231..dae9121279 100644
> >>> --- a/doc/guides/prog_guide/rte_flow.rst
> >>> +++ b/doc/guides/prog_guide/rte_flow.rst
> >>> @@ -2958,7 +2958,7 @@ Otherwise, RTE_FLOW_ERROR_TYPE_ACTION
> >> error will be returned.
> >>>    Action: ``AGE``
> >>>    ^^^^^^^^^^^^^^^
> >>>
> >>> -Set ageing timeout configuration to a flow.
> >>> +Set aging timeout configuration to a flow.
> >>>
> >>>    Event RTE_ETH_EVENT_FLOW_AGED will be reported if
> >>>    timeout passed without any matching on the flow.
> >>> @@ -2977,8 +2977,8 @@ timeout passed without any matching on the
> >> flow.
> >>>       | ``context``  | user input flow context         |
> >>>       +--------------+---------------------------------+
> >>>
> >>> -Query structure to retrieve ageing status information of a
> >>> -shared AGE action, or a flow rule using the AGE action:
> >>> +Query structure to retrieve aging status information of an
> >>> +indirect AGE action, or a flow rule using the AGE action:
> >>>
> >>>    .. _table_rte_flow_query_age:
> >>>
> >>> @@ -2994,6 +2994,25 @@ shared AGE action, or a flow rule using the
> AGE
> >> action:
> >>>       | ``sec_since_last_hit``       | out | Seconds since last traffic hit         |
> >>>       +------------------------------+-----+----------------------------------------+
> >>>
> >>> +Update structure to modify the parameters of an indirect AGE action.
> >>> +The update structure is used by ``rte_flow_action_handle_update()``
> >> function.
> >>> +
> >>> +.. _table_rte_flow_update_age:
> >>> +
> >>> +.. table:: AGE update
> >>> +
> >>> +   +-------------------+--------------------------------------------------------------+
> >>> +   | Field             | Value                                                        |
> >>> +
> >> +===================+=====================================
> >> =========================+
> >>> +   | ``timeout``       | 24 bits timeout value                                        |
> >>> +   +-------------------+--------------------------------------------------------------+
> >>> +   | ``timeout_valid`` | 1 bit, timeout value is valid                                |
> >>> +   +-------------------+--------------------------------------------------------------+
> >>> +   | ``touch``         | 1 bit, touch the AGE action to set
> ``sec_since_last_hit`` 0
> >> |
> >>> +   +-------------------+--------------------------------------------------------------+
> >>> +   | ``reserved``      | 6 bits reserved, must be zero                                |
> >>> +   +-------------------+--------------------------------------------------------------+
> >>> +
> >>>    Action: ``SAMPLE``
> >>>    ^^^^^^^^^^^^^^^^^^
> >>>
> >>> diff --git a/lib/ethdev/rte_flow.h b/lib/ethdev/rte_flow.h
> >>> index d830b02321..a21d437cf8 100644
> >>> --- a/lib/ethdev/rte_flow.h
> >>> +++ b/lib/ethdev/rte_flow.h
> >>> @@ -2954,6 +2954,33 @@ struct rte_flow_query_age {
> >>>    	uint32_t sec_since_last_hit:24; /**< Seconds since last traffic hit. */
> >>>    };
> >>>
> >>> +/**
> >>> + * @warning
> >>> + * @b EXPERIMENTAL: this structure may change without prior notice
> >>> + *
> >>> + * RTE_FLOW_ACTION_TYPE_AGE
> >>> + *
> >>> + * Update indirect AGE action attributes:
> >>> + *  - Timeout can be updated including stop/start action:
> >>> + *     +-------------+-------------+------------------------------+
> >>> + *     | Old Timeout | New Timeout | Updating                     |
> >>> + *
> >> +=============+=============+=============================
> >> =+
> >>> + *     | 0           | positive    | Start aging with new value   |
> >>> + *     +-------------+-------------+------------------------------+
> >>> + *     | positive    | 0           | Stop aging			  |
> >>> + *     +-------------+-------------+------------------------------+
> >>> + *     | positive    | positive    | Change timeout to new value  |
> >>> + *     +-------------+-------------+------------------------------+
> >>> + *  - sec_since_last_hit can be reset.
> >>> + */
> >>> +struct rte_flow_update_age {
> >>
> >> I think it worse to mention the structure in
> >> RTE_FLOW_ACTION_TYPE_AGE description.
> >
> > What do you mean?
> 
> RTE_FLOW_ACTION_TYPE_AGE has a description as enum member.
> Typically configuration structures are mentioned there.
> However, I think that it would be useful to mention specific
> update structure if any. Like this one. Just to make it clear
> that if a user wants to update the action configuration it
> should use specific structure. Or am I missing something?
> Misunderstand something?
> 

Do you mean something like:
* see enum RTE_ETH_EVENT_FLOW_AGED
 * See struct rte_flow_query_age
+ See struct rte_flow_update_age
 */
RTE_FLOW_ACTION_TYPE_AGE,
 If so, I fully agree.

> >
> >>
> >>> +	uint32_t reserved:6; /**< Reserved, must be zero. */
> >>> +	uint32_t timeout_valid:1; /**< The timeout is valid for update. */
> >>> +	uint32_t timeout:24; /**< Time in seconds. */
> >>> +	uint32_t touch:1;
> >>> +	/**< Means that aging should assume packet passed the aging. */
> >>
> >> Documentation after code makes sense if it is in the same line.
> >> If the documentation is in its own line, it should be before
> >> the code and use /** prefix.
> >>
> >
> > +1
> >
> >> One more question: why order in the documentation above does
> >> not match order here?
> >>
> >
> > I think we can remove it from the documentation since it doesn't add
> anything
> >
> >>> +};
> >>> +
> >>>    /**
> >>>     * @warning
> >>>     * @b EXPERIMENTAL: this structure may change without prior notice
> >


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

* Re: [PATCH 3/3] ethdev: add structure for indirect AGE update
  2022-10-03 15:13         ` Ori Kam
@ 2022-10-04  6:36           ` Andrew Rybchenko
  2022-10-19 13:09             ` Michael Baum
  0 siblings, 1 reply; 32+ messages in thread
From: Andrew Rybchenko @ 2022-10-04  6:36 UTC (permalink / raw)
  To: Ori Kam, Michael Baum, dev; +Cc: Matan Azrad, Raslan Darawsheh

On 10/3/22 18:13, Ori Kam wrote:
> Hi Andrew,
> 
>> -----Original Message-----
>> From: Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>
>> Sent: Monday, 3 October 2022 16:18
>>
>> On 10/3/22 11:30, Ori Kam wrote:
>>> Hi Andrew,
>>>
>>>> -----Original Message-----
>>>> From: Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>
>>>> Sent: Monday, 3 October 2022 11:04
>>>>
>>>> On 9/21/22 17:54, Michael Baum wrote:
>>>>> Add a new structure for indirect AGE update.
>>>>>
>>>>> This new structure enables:
>>>>> 1. Update timeout value.
>>>>> 2. Stop AGE checking.
>>>>> 3. Start AGE checking.
>>>>> 4. restart AGE checking.
>>>>>
>>>>> Signed-off-by: Michael Baum <michaelba@nvidia.com>
>>>>> ---
>>>>>     app/test-pmd/cmdline_flow.c        | 66
>>>> ++++++++++++++++++++++++++++++
>>>>>     app/test-pmd/config.c              | 18 +++++++-
>>>>>     doc/guides/prog_guide/rte_flow.rst | 25 +++++++++--
>>>>>     lib/ethdev/rte_flow.h              | 27 ++++++++++++
>>>>>     4 files changed, 132 insertions(+), 4 deletions(-)
>>>>>
>>>>> diff --git a/app/test-pmd/cmdline_flow.c b/app/test-
>> pmd/cmdline_flow.c
>>>>> index 4fb90a92cb..a315fd9ded 100644
>>>>> --- a/app/test-pmd/cmdline_flow.c
>>>>> +++ b/app/test-pmd/cmdline_flow.c
>>>>> @@ -583,6 +583,9 @@ enum index {
>>>>>     	ACTION_SET_IPV6_DSCP_VALUE,
>>>>>     	ACTION_AGE,
>>>>>     	ACTION_AGE_TIMEOUT,
>>>>> +	ACTION_AGE_UPDATE,
>>>>> +	ACTION_AGE_UPDATE_TIMEOUT,
>>>>> +	ACTION_AGE_UPDATE_TOUCH,
>>>>>     	ACTION_SAMPLE,
>>>>>     	ACTION_SAMPLE_RATIO,
>>>>>     	ACTION_SAMPLE_INDEX,
>>>>> @@ -1869,6 +1872,7 @@ static const enum index next_action[] = {
>>>>>     	ACTION_SET_IPV4_DSCP,
>>>>>     	ACTION_SET_IPV6_DSCP,
>>>>>     	ACTION_AGE,
>>>>> +	ACTION_AGE_UPDATE,
>>>>>     	ACTION_SAMPLE,
>>>>>     	ACTION_INDIRECT,
>>>>>     	ACTION_MODIFY_FIELD,
>>>>> @@ -2113,6 +2117,14 @@ static const enum index action_age[] = {
>>>>>     	ZERO,
>>>>>     };
>>>>>
>>>>> +static const enum index action_age_update[] = {
>>>>> +	ACTION_AGE_UPDATE,
>>>>> +	ACTION_AGE_UPDATE_TIMEOUT,
>>>>> +	ACTION_AGE_UPDATE_TOUCH,
>>>>> +	ACTION_NEXT,
>>>>> +	ZERO,
>>>>> +};
>>>>> +
>>>>>     static const enum index action_sample[] = {
>>>>>     	ACTION_SAMPLE,
>>>>>     	ACTION_SAMPLE_RATIO,
>>>>> @@ -2191,6 +2203,9 @@ static int parse_vc_spec(struct context *, const
>>>> struct token *,
>>>>>     			 const char *, unsigned int, void *, unsigned int);
>>>>>     static int parse_vc_conf(struct context *, const struct token *,
>>>>>     			 const char *, unsigned int, void *, unsigned int);
>>>>> +static int parse_vc_conf_timeout(struct context *, const struct token *,
>>>>> +				 const char *, unsigned int, void *,
>>>>> +				 unsigned int);
>>>>>     static int parse_vc_item_ecpri_type(struct context *, const struct token
>> *,
>>>>>     				    const char *, unsigned int,
>>>>>     				    void *, unsigned int);
>>>>> @@ -6194,6 +6209,30 @@ static const struct token token_list[] = {
>>>>>     		.next = NEXT(action_age,
>>>> NEXT_ENTRY(COMMON_UNSIGNED)),
>>>>>     		.call = parse_vc_conf,
>>>>>     	},
>>>>> +	[ACTION_AGE_UPDATE] = {
>>>>> +		.name = "age_update",
>>>>> +		.help = "update aging parameter",
>>>>> +		.next = NEXT(action_age_update),
>>>>> +		.priv = PRIV_ACTION(AGE,
>>>>> +				    sizeof(struct rte_flow_update_age)),
>>>>> +		.call = parse_vc,
>>>>> +	},
>>>>> +	[ACTION_AGE_UPDATE_TIMEOUT] = {
>>>>> +		.name = "timeout",
>>>>> +		.help = "age timeout update value",
>>>>> +		.args = ARGS(ARGS_ENTRY_BF(struct rte_flow_update_age,
>>>>> +					   timeout, 24)),
>>>>> +		.next = NEXT(action_age_update,
>>>> NEXT_ENTRY(COMMON_UNSIGNED)),
>>>>> +		.call = parse_vc_conf_timeout,
>>>>> +	},
>>>>> +	[ACTION_AGE_UPDATE_TOUCH] = {
>>>>> +		.name = "touch",
>>>>> +		.help = "this flow is touched",
>>>>> +		.next = NEXT(action_age_update,
>>>> NEXT_ENTRY(COMMON_BOOLEAN)),
>>>>> +		.args = ARGS(ARGS_ENTRY_BF(struct rte_flow_update_age,
>>>>> +					   touch, 1)),
>>>>> +		.call = parse_vc_conf,
>>>>> +	},
>>>>>     	[ACTION_SAMPLE] = {
>>>>>     		.name = "sample",
>>>>>     		.help = "set a sample action",
>>>>> @@ -7031,6 +7070,33 @@ parse_vc_conf(struct context *ctx, const
>> struct
>>>> token *token,
>>>>>     	return len;
>>>>>     }
>>>>>
>>>>> +/** Parse action configuration field. */
>>>>> +static int
>>>>> +parse_vc_conf_timeout(struct context *ctx, const struct token *token,
>>>>> +		      const char *str, unsigned int len,
>>>>> +		      void *buf, unsigned int size)
>>>>> +{
>>>>> +	struct buffer *out = buf;
>>>>> +	struct rte_flow_update_age *update;
>>>>> +
>>>>> +	(void)size;
>>>>> +	if (ctx->curr != ACTION_AGE_UPDATE_TIMEOUT)
>>>>> +		return -1;
>>>>> +	/* 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;
>>>>> +	/* Point to selected object. */
>>>>> +	ctx->object = out->args.vc.data;
>>>>> +	ctx->objmask = NULL;
>>>>> +	/* Update the timeout is valid. */
>>>>> +	update = (struct rte_flow_update_age *)out->args.vc.data;
>>>>> +	update->timeout_valid = 1;
>>>>> +	return len;
>>>>> +}
>>>>> +
>>>>>     /** Parse eCPRI common header type field. */
>>>>>     static int
>>>>>     parse_vc_item_ecpri_type(struct context *ctx, const struct token
>> *token,
>>>>> diff --git a/app/test-pmd/config.c b/app/test-pmd/config.c
>>>>> index 31952467fb..45495385d7 100644
>>>>> --- a/app/test-pmd/config.c
>>>>> +++ b/app/test-pmd/config.c
>>>>> @@ -2065,6 +2065,7 @@ port_action_handle_update(portid_t port_id,
>>>> uint32_t id,
>>>>>     	if (!pia)
>>>>>     		return -EINVAL;
>>>>>     	switch (pia->type) {
>>>>> +	case RTE_FLOW_ACTION_TYPE_AGE:
>>>>>     	case RTE_FLOW_ACTION_TYPE_CONNTRACK:
>>>>>     		update = action->conf;
>>>>>     		break;
>>>>> @@ -2904,6 +2905,8 @@ port_queue_action_handle_update(portid_t
>>>> port_id,
>>>>>     	struct rte_port *port;
>>>>>     	struct rte_flow_error error;
>>>>>     	struct rte_flow_action_handle *action_handle;
>>>>> +	struct port_indirect_action *pia;
>>>>> +	const void *update;
>>>>>
>>>>>     	action_handle = port_action_handle_get_by_id(port_id, id);
>>>>>     	if (!action_handle)
>>>>> @@ -2915,8 +2918,21 @@ port_queue_action_handle_update(portid_t
>>>> port_id,
>>>>>     		return -EINVAL;
>>>>>     	}
>>>>>
>>>>> +	pia = action_get_by_id(port_id, id);
>>>>> +	if (!pia)
>>>>> +		return -EINVAL;
>>>>> +
>>>>> +	switch (pia->type) {
>>>>> +	case RTE_FLOW_ACTION_TYPE_AGE:
>>>>> +		update = action->conf;
>>>>> +		break;
>>>>> +	default:
>>>>> +		update = action;
>>>>> +		break;
>>>>> +	}
>>>>> +
>>>>>     	if (rte_flow_async_action_handle_update(port_id, queue_id, &attr,
>>>>> -				    action_handle, action, NULL, &error)) {
>>>>> +				    action_handle, update, NULL, &error)) {
>>>>>     		return port_flow_complain(&error);
>>>>>     	}
>>>>>     	printf("Indirect action #%u update queued\n", id);
>>>>> diff --git a/doc/guides/prog_guide/rte_flow.rst
>>>> b/doc/guides/prog_guide/rte_flow.rst
>>>>> index 588914b231..dae9121279 100644
>>>>> --- a/doc/guides/prog_guide/rte_flow.rst
>>>>> +++ b/doc/guides/prog_guide/rte_flow.rst
>>>>> @@ -2958,7 +2958,7 @@ Otherwise, RTE_FLOW_ERROR_TYPE_ACTION
>>>> error will be returned.
>>>>>     Action: ``AGE``
>>>>>     ^^^^^^^^^^^^^^^
>>>>>
>>>>> -Set ageing timeout configuration to a flow.
>>>>> +Set aging timeout configuration to a flow.
>>>>>
>>>>>     Event RTE_ETH_EVENT_FLOW_AGED will be reported if
>>>>>     timeout passed without any matching on the flow.
>>>>> @@ -2977,8 +2977,8 @@ timeout passed without any matching on the
>>>> flow.
>>>>>        | ``context``  | user input flow context         |
>>>>>        +--------------+---------------------------------+
>>>>>
>>>>> -Query structure to retrieve ageing status information of a
>>>>> -shared AGE action, or a flow rule using the AGE action:
>>>>> +Query structure to retrieve aging status information of an
>>>>> +indirect AGE action, or a flow rule using the AGE action:
>>>>>
>>>>>     .. _table_rte_flow_query_age:
>>>>>
>>>>> @@ -2994,6 +2994,25 @@ shared AGE action, or a flow rule using the
>> AGE
>>>> action:
>>>>>        | ``sec_since_last_hit``       | out | Seconds since last traffic hit         |
>>>>>        +------------------------------+-----+----------------------------------------+
>>>>>
>>>>> +Update structure to modify the parameters of an indirect AGE action.
>>>>> +The update structure is used by ``rte_flow_action_handle_update()``
>>>> function.
>>>>> +
>>>>> +.. _table_rte_flow_update_age:
>>>>> +
>>>>> +.. table:: AGE update
>>>>> +
>>>>> +   +-------------------+--------------------------------------------------------------+
>>>>> +   | Field             | Value                                                        |
>>>>> +
>>>> +===================+=====================================
>>>> =========================+
>>>>> +   | ``timeout``       | 24 bits timeout value                                        |
>>>>> +   +-------------------+--------------------------------------------------------------+
>>>>> +   | ``timeout_valid`` | 1 bit, timeout value is valid                                |
>>>>> +   +-------------------+--------------------------------------------------------------+
>>>>> +   | ``touch``         | 1 bit, touch the AGE action to set
>> ``sec_since_last_hit`` 0
>>>> |
>>>>> +   +-------------------+--------------------------------------------------------------+
>>>>> +   | ``reserved``      | 6 bits reserved, must be zero                                |
>>>>> +   +-------------------+--------------------------------------------------------------+
>>>>> +
>>>>>     Action: ``SAMPLE``
>>>>>     ^^^^^^^^^^^^^^^^^^
>>>>>
>>>>> diff --git a/lib/ethdev/rte_flow.h b/lib/ethdev/rte_flow.h
>>>>> index d830b02321..a21d437cf8 100644
>>>>> --- a/lib/ethdev/rte_flow.h
>>>>> +++ b/lib/ethdev/rte_flow.h
>>>>> @@ -2954,6 +2954,33 @@ struct rte_flow_query_age {
>>>>>     	uint32_t sec_since_last_hit:24; /**< Seconds since last traffic hit. */
>>>>>     };
>>>>>
>>>>> +/**
>>>>> + * @warning
>>>>> + * @b EXPERIMENTAL: this structure may change without prior notice
>>>>> + *
>>>>> + * RTE_FLOW_ACTION_TYPE_AGE
>>>>> + *
>>>>> + * Update indirect AGE action attributes:
>>>>> + *  - Timeout can be updated including stop/start action:
>>>>> + *     +-------------+-------------+------------------------------+
>>>>> + *     | Old Timeout | New Timeout | Updating                     |
>>>>> + *
>>>> +=============+=============+=============================
>>>> =+
>>>>> + *     | 0           | positive    | Start aging with new value   |
>>>>> + *     +-------------+-------------+------------------------------+
>>>>> + *     | positive    | 0           | Stop aging			  |
>>>>> + *     +-------------+-------------+------------------------------+
>>>>> + *     | positive    | positive    | Change timeout to new value  |
>>>>> + *     +-------------+-------------+------------------------------+
>>>>> + *  - sec_since_last_hit can be reset.
>>>>> + */
>>>>> +struct rte_flow_update_age {
>>>>
>>>> I think it worse to mention the structure in
>>>> RTE_FLOW_ACTION_TYPE_AGE description.
>>>
>>> What do you mean?
>>
>> RTE_FLOW_ACTION_TYPE_AGE has a description as enum member.
>> Typically configuration structures are mentioned there.
>> However, I think that it would be useful to mention specific
>> update structure if any. Like this one. Just to make it clear
>> that if a user wants to update the action configuration it
>> should use specific structure. Or am I missing something?
>> Misunderstand something?
>>
> 
> Do you mean something like:
> * see enum RTE_ETH_EVENT_FLOW_AGED
>   * See struct rte_flow_query_age
> + See struct rte_flow_update_age
>   */
> RTE_FLOW_ACTION_TYPE_AGE,
>   If so, I fully agree.

Yes, exactly.

> 
>>>
>>>>
>>>>> +	uint32_t reserved:6; /**< Reserved, must be zero. */
>>>>> +	uint32_t timeout_valid:1; /**< The timeout is valid for update. */
>>>>> +	uint32_t timeout:24; /**< Time in seconds. */
>>>>> +	uint32_t touch:1;
>>>>> +	/**< Means that aging should assume packet passed the aging. */
>>>>
>>>> Documentation after code makes sense if it is in the same line.
>>>> If the documentation is in its own line, it should be before
>>>> the code and use /** prefix.
>>>>
>>>
>>> +1
>>>
>>>> One more question: why order in the documentation above does
>>>> not match order here?
>>>>
>>>
>>> I think we can remove it from the documentation since it doesn't add
>> anything
>>>
>>>>> +};
>>>>> +
>>>>>     /**
>>>>>      * @warning
>>>>>      * @b EXPERIMENTAL: this structure may change without prior notice
>>>
> 


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

* RE: [PATCH 3/3] ethdev: add structure for indirect AGE update
  2022-10-04  6:36           ` Andrew Rybchenko
@ 2022-10-19 13:09             ` Michael Baum
  0 siblings, 0 replies; 32+ messages in thread
From: Michael Baum @ 2022-10-19 13:09 UTC (permalink / raw)
  To: Andrew Rybchenko, Ori Kam, dev; +Cc: Matan Azrad, Raslan Darawsheh

Hi Andrew and Ori,

On 10/4/22 9:37, Andrew Rybchenko wrote: 
> 
> On 10/3/22 18:13, Ori Kam wrote:
> > Hi Andrew,
> >
> >> -----Original Message-----
> >> From: Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>
> >> Sent: Monday, 3 October 2022 16:18
> >>
> >> On 10/3/22 11:30, Ori Kam wrote:
> >>> Hi Andrew,
> >>>
> >>>> -----Original Message-----
> >>>> From: Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>
> >>>> Sent: Monday, 3 October 2022 11:04
> >>>>
> >>>> On 9/21/22 17:54, Michael Baum wrote:
> >>>>> Add a new structure for indirect AGE update.
> >>>>>
> >>>>> This new structure enables:
> >>>>> 1. Update timeout value.
> >>>>> 2. Stop AGE checking.
> >>>>> 3. Start AGE checking.
> >>>>> 4. restart AGE checking.
> >>>>>
> >>>>> Signed-off-by: Michael Baum <michaelba@nvidia.com>
> >>>>> ---
> >>>>>     app/test-pmd/cmdline_flow.c        | 66
> >>>> ++++++++++++++++++++++++++++++
> >>>>>     app/test-pmd/config.c              | 18 +++++++-
> >>>>>     doc/guides/prog_guide/rte_flow.rst | 25 +++++++++--
> >>>>>     lib/ethdev/rte_flow.h              | 27 ++++++++++++
> >>>>>     4 files changed, 132 insertions(+), 4 deletions(-)
> >>>>>
> >>>>> diff --git a/app/test-pmd/cmdline_flow.c b/app/test-
> >> pmd/cmdline_flow.c
> >>>>> index 4fb90a92cb..a315fd9ded 100644
> >>>>> --- a/app/test-pmd/cmdline_flow.c
> >>>>> +++ b/app/test-pmd/cmdline_flow.c
> >>>>> @@ -583,6 +583,9 @@ enum index {
> >>>>>           ACTION_SET_IPV6_DSCP_VALUE,
> >>>>>           ACTION_AGE,
> >>>>>           ACTION_AGE_TIMEOUT,
> >>>>> + ACTION_AGE_UPDATE,
> >>>>> + ACTION_AGE_UPDATE_TIMEOUT,
> >>>>> + ACTION_AGE_UPDATE_TOUCH,
> >>>>>           ACTION_SAMPLE,
> >>>>>           ACTION_SAMPLE_RATIO,
> >>>>>           ACTION_SAMPLE_INDEX,
> >>>>> @@ -1869,6 +1872,7 @@ static const enum index next_action[] = {
> >>>>>           ACTION_SET_IPV4_DSCP,
> >>>>>           ACTION_SET_IPV6_DSCP,
> >>>>>           ACTION_AGE,
> >>>>> + ACTION_AGE_UPDATE,
> >>>>>           ACTION_SAMPLE,
> >>>>>           ACTION_INDIRECT,
> >>>>>           ACTION_MODIFY_FIELD,
> >>>>> @@ -2113,6 +2117,14 @@ static const enum index action_age[] = {
> >>>>>           ZERO,
> >>>>>     };
> >>>>>
> >>>>> +static const enum index action_age_update[] = {
> >>>>> +ACTION_AGE_UPDATE,  ACTION_AGE_UPDATE_TIMEOUT,
> >>>>> +ACTION_AGE_UPDATE_TOUCH,  ACTION_NEXT,  ZERO, };
> >>>>> +
> >>>>>     static const enum index action_sample[] = {
> >>>>>           ACTION_SAMPLE,
> >>>>>           ACTION_SAMPLE_RATIO,
> >>>>> @@ -2191,6 +2203,9 @@ static int parse_vc_spec(struct context *,
> >>>>> const
> >>>> struct token *,
> >>>>>                            const char *, unsigned int, void *, unsigned int);
> >>>>>     static int parse_vc_conf(struct context *, const struct token *,
> >>>>>                            const char *, unsigned int, void *,
> >>>>> unsigned int);
> >>>>> +static int parse_vc_conf_timeout(struct context *, const struct token
> *,
> >>>>> +                          const char *, unsigned int, void *,
> >>>>> +                          unsigned int);
> >>>>>     static int parse_vc_item_ecpri_type(struct context *, const
> >>>>> struct token
> >> *,
> >>>>>                                       const char *, unsigned int,
> >>>>>                                       void *, unsigned int); @@
> >>>>> -6194,6 +6209,30 @@ static const struct token token_list[] = {
> >>>>>                   .next = NEXT(action_age,
> >>>> NEXT_ENTRY(COMMON_UNSIGNED)),
> >>>>>                   .call = parse_vc_conf,
> >>>>>           },
> >>>>> + [ACTION_AGE_UPDATE] = {
> >>>>> +         .name = "age_update",
> >>>>> +         .help = "update aging parameter",
> >>>>> +         .next = NEXT(action_age_update),
> >>>>> +         .priv = PRIV_ACTION(AGE,
> >>>>> +                             sizeof(struct rte_flow_update_age)),
> >>>>> +         .call = parse_vc,
> >>>>> + },
> >>>>> + [ACTION_AGE_UPDATE_TIMEOUT] = {
> >>>>> +         .name = "timeout",
> >>>>> +         .help = "age timeout update value",
> >>>>> +         .args = ARGS(ARGS_ENTRY_BF(struct rte_flow_update_age,
> >>>>> +                                    timeout, 24)),
> >>>>> +         .next = NEXT(action_age_update,
> >>>> NEXT_ENTRY(COMMON_UNSIGNED)),
> >>>>> +         .call = parse_vc_conf_timeout, },
> >>>>> + [ACTION_AGE_UPDATE_TOUCH] = {
> >>>>> +         .name = "touch",
> >>>>> +         .help = "this flow is touched",
> >>>>> +         .next = NEXT(action_age_update,
> >>>> NEXT_ENTRY(COMMON_BOOLEAN)),
> >>>>> +         .args = ARGS(ARGS_ENTRY_BF(struct rte_flow_update_age,
> >>>>> +                                    touch, 1)),
> >>>>> +         .call = parse_vc_conf,
> >>>>> + },
> >>>>>           [ACTION_SAMPLE] = {
> >>>>>                   .name = "sample",
> >>>>>                   .help = "set a sample action", @@ -7031,6
> >>>>> +7070,33 @@ parse_vc_conf(struct context *ctx, const
> >> struct
> >>>> token *token,
> >>>>>           return len;
> >>>>>     }
> >>>>>
> >>>>> +/** Parse action configuration field. */ static int
> >>>>> +parse_vc_conf_timeout(struct context *ctx, const struct token
> *token,
> >>>>> +               const char *str, unsigned int len,
> >>>>> +               void *buf, unsigned int size) {  struct buffer
> >>>>> +*out = buf;  struct rte_flow_update_age *update;
> >>>>> +
> >>>>> + (void)size;
> >>>>> + if (ctx->curr != ACTION_AGE_UPDATE_TIMEOUT)
> >>>>> +         return -1;
> >>>>> + /* 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;
> >>>>> + /* Point to selected object. */
> >>>>> + ctx->object = out->args.vc.data; objmask = NULL;
> >>>>> + /* Update the timeout is valid. */ update = (struct
> >>>>> + rte_flow_update_age *)out->args.vc.data;
> >>>>> + update->timeout_valid = 1;
> >>>>> + return len;
> >>>>> +}
> >>>>> +
> >>>>>     /** Parse eCPRI common header type field. */
> >>>>>     static int
> >>>>>     parse_vc_item_ecpri_type(struct context *ctx, const struct
> >>>>> token
> >> *token,
> >>>>> diff --git a/app/test-pmd/config.c b/app/test-pmd/config.c index
> >>>>> 31952467fb..45495385d7 100644
> >>>>> --- a/app/test-pmd/config.c
> >>>>> +++ b/app/test-pmd/config.c
> >>>>> @@ -2065,6 +2065,7 @@ port_action_handle_update(portid_t
> port_id,
> >>>> uint32_t id,
> >>>>>           if (!pia)
> >>>>>                   return -EINVAL;
> >>>>>           switch (pia->type) {
> >>>>> + case RTE_FLOW_ACTION_TYPE_AGE:
> >>>>>           case RTE_FLOW_ACTION_TYPE_CONNTRACK:
> >>>>>                   update = action->conf;
> >>>>>                   break;
> >>>>> @@ -2904,6 +2905,8 @@
> port_queue_action_handle_update(portid_t
> >>>> port_id,
> >>>>>           struct rte_port *port;
> >>>>>           struct rte_flow_error error;
> >>>>>           struct rte_flow_action_handle *action_handle;
> >>>>> + struct port_indirect_action *pia; const void *update;
> >>>>>
> >>>>>           action_handle = port_action_handle_get_by_id(port_id, id);
> >>>>>           if (!action_handle)
> >>>>> @@ -2915,8 +2918,21 @@
> port_queue_action_handle_update(portid_t
> >>>> port_id,
> >>>>>                   return -EINVAL;
> >>>>>           }
> >>>>>
> >>>>> + pia = action_get_by_id(port_id, id); if (!pia)
> >>>>> +         return -EINVAL;
> >>>>> +
> >>>>> + switch (pia->type) {
> >>>>> + case RTE_FLOW_ACTION_TYPE_AGE:
> >>>>> +         update = action->conf;
> >>>>> +         break;
> >>>>> + default:
> >>>>> +         update = action;
> >>>>> +         break;
> >>>>> + }
> >>>>> +
> >>>>>           if (rte_flow_async_action_handle_update(port_id, queue_id,
> &attr,
> >>>>> -                             action_handle, action, NULL, &error)) {
> >>>>> +                             action_handle, update, NULL,
> >>>>> + &error)) {
> >>>>>                   return port_flow_complain(&error);
> >>>>>           }
> >>>>>           printf("Indirect action #%u update queued\n", id); diff
> >>>>> --git a/doc/guides/prog_guide/rte_flow.rst
> >>>> b/doc/guides/prog_guide/rte_flow.rst
> >>>>> index 588914b231..dae9121279 100644
> >>>>> --- a/doc/guides/prog_guide/rte_flow.rst
> >>>>> +++ b/doc/guides/prog_guide/rte_flow.rst
> >>>>> @@ -2958,7 +2958,7 @@ Otherwise,
> RTE_FLOW_ERROR_TYPE_ACTION
> >>>> error will be returned.
> >>>>>     Action: ``AGE``
> >>>>>     ^^^^^^^^^^^^^^^
> >>>>>
> >>>>> -Set ageing timeout configuration to a flow.
> >>>>> +Set aging timeout configuration to a flow.
> >>>>>
> >>>>>     Event RTE_ETH_EVENT_FLOW_AGED will be reported if
> >>>>>     timeout passed without any matching on the flow.
> >>>>> @@ -2977,8 +2977,8 @@ timeout passed without any matching on
> the
> >>>> flow.
> >>>>>        | ``context``  | user input flow context         |
> >>>>>        +--------------+---------------------------------+
> >>>>>
> >>>>> -Query structure to retrieve ageing status information of a
> >>>>> -shared AGE action, or a flow rule using the AGE action:
> >>>>> +Query structure to retrieve aging status information of an
> >>>>> +indirect AGE action, or a flow rule using the AGE action:
> >>>>>
> >>>>>     .. _table_rte_flow_query_age:
> >>>>>
> >>>>> @@ -2994,6 +2994,25 @@ shared AGE action, or a flow rule using the
> >> AGE
> >>>> action:
> >>>>>        | ``sec_since_last_hit``       | out | Seconds since last traffic hit         |
> >>>>>
> >>>>> +------------------------------+-----+----------------------------
> >>>>> ------------+
> >>>>>
> >>>>> +Update structure to modify the parameters of an indirect AGE
> action.
> >>>>> +The update structure is used by
> >>>>> +``rte_flow_action_handle_update()``
> >>>> function.
> >>>>> +
> >>>>> +.. _table_rte_flow_update_age:
> >>>>> +
> >>>>> +.. table:: AGE update
> >>>>> +
> >>>>> +   +-------------------+-----------------------------------------------------------
> ---+
> >>>>> +   | Field             | Value                                                        |
> >>>>> +
> >>>>
> +===================+=====================================
> >>>> =========================+
> >>>>> +   | ``timeout``       | 24 bits timeout value                                        |
> >>>>> +   +-------------------+-----------------------------------------------------------
> ---+
> >>>>> +   | ``timeout_valid`` | 1 bit, timeout value is valid                                |
> >>>>> +   +-------------------+-----------------------------------------------------------
> ---+
> >>>>> +   | ``touch``         | 1 bit, touch the AGE action to set
> >> ``sec_since_last_hit`` 0
> >>>> |
> >>>>> +   +-------------------+-----------------------------------------------------------
> ---+
> >>>>> +   | ``reserved``      | 6 bits reserved, must be zero                                |
> >>>>> +
> >>>>> + +-------------------+-------------------------------------------
> >>>>> + -------------------+
> >>>>> +
> >>>>>     Action: ``SAMPLE``
> >>>>>     ^^^^^^^^^^^^^^^^^^
> >>>>>
> >>>>> diff --git a/lib/ethdev/rte_flow.h b/lib/ethdev/rte_flow.h index
> >>>>> d830b02321..a21d437cf8 100644
> >>>>> --- a/lib/ethdev/rte_flow.h
> >>>>> +++ b/lib/ethdev/rte_flow.h
> >>>>> @@ -2954,6 +2954,33 @@ struct rte_flow_query_age {
> >>>>>           uint32_t sec_since_last_hit:24; /**< Seconds since last traffic hit.
> */
> >>>>>     };
> >>>>>
> >>>>> +/**
> >>>>> + * @warning
> >>>>> + * @b EXPERIMENTAL: this structure may change without prior
> >>>>> +notice
> >>>>> + *
> >>>>> + * RTE_FLOW_ACTION_TYPE_AGE
> >>>>> + *
> >>>>> + * Update indirect AGE action attributes:
> >>>>> + *  - Timeout can be updated including stop/start action:
> >>>>> + *     +-------------+-------------+------------------------------+
> >>>>> + *     | Old Timeout | New Timeout | Updating                     |
> >>>>> + *
> >>>>
> +=============+=============+=============================
> >>>> =+
> >>>>> + *     | 0           | positive    | Start aging with new value   |
> >>>>> + *     +-------------+-------------+------------------------------+
> >>>>> + *     | positive    | 0           | Stop aging                    |
> >>>>> + *     +-------------+-------------+------------------------------+
> >>>>> + *     | positive    | positive    | Change timeout to new value  |
> >>>>> + *     +-------------+-------------+------------------------------+
> >>>>> + *  - sec_since_last_hit can be reset.
> >>>>> + */
> >>>>> +struct rte_flow_update_age {
> >>>>
> >>>> I think it worse to mention the structure in
> >>>> RTE_FLOW_ACTION_TYPE_AGE description.
> >>>
> >>> What do you mean?
> >>
> >> RTE_FLOW_ACTION_TYPE_AGE has a description as enum member.
> >> Typically configuration structures are mentioned there.
> >> However, I think that it would be useful to mention specific update
> >> structure if any. Like this one. Just to make it clear that if a user
> >> wants to update the action configuration it should use specific
> >> structure. Or am I missing something?
> >> Misunderstand something?
> >>
> >
> > Do you mean something like:
> > * see enum RTE_ETH_EVENT_FLOW_AGED
> >   * See struct rte_flow_query_age
> > + See struct rte_flow_update_age
> >   */
> > RTE_FLOW_ACTION_TYPE_AGE,
> >   If so, I fully agree.
> 
> Yes, exactly.

I have updated it according your suggestion, thank you.

> 
> >
> >>>
> >>>>
> >>>>> + uint32_t reserved:6; /**< Reserved, must be zero. */ uint32_t
> >>>>> + timeout_valid:1; /**< The timeout is valid for update. */
> >>>>> + uint32_t timeout:24; /**< Time in seconds. */ uint32_t touch:1;
> >>>>> + /**< Means that aging should assume packet passed the aging. */
> >>>>
> >>>> Documentation after code makes sense if it is in the same line.
> >>>> If the documentation is in its own line, it should be before the
> >>>> code and use /** prefix.

Updated, thanks.

> >>>
> >>> +1
> >>>
> >>>> One more question: why order in the documentation above does not
> >>>> match order here?

I updated the documentation to be align to this order.

> >>>
> >>> I think we can remove it from the documentation since it doesn't add
> >> anything
> >>>
> >>>>> +};
> >>>>> +
> >>>>>     /**
> >>>>>      * @warning
> >>>>>      * @b EXPERIMENTAL: this structure may change without prior
> >>>>> notice
> >>>
> >


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

* [PATCH v2 0/3] ethdev: AGE action preparation
  2022-09-21 14:54 [PATCH 0/3] ethdev: AGE action preparation Michael Baum
                   ` (3 preceding siblings ...)
  2022-09-29  8:40 ` [PATCH 0/3] ethdev: AGE action preparation Andrew Rybchenko
@ 2022-10-19 13:12 ` Michael Baum
  2022-10-19 13:12   ` [PATCH v2 1/3] ethdev: add strict queue to pre-configuration flow hints Michael Baum
                     ` (3 more replies)
  4 siblings, 4 replies; 32+ messages in thread
From: Michael Baum @ 2022-10-19 13:12 UTC (permalink / raw)
  To: dev; +Cc: Matan Azrad, Raslan Darawsheh, Ori Kam

RFC's:
https://patchwork.dpdk.org/project/dpdk/patch/7a45693f478b1b721b4e05131141b526185a175c.1654063912.git.jackmin@nvidia.com/
https://patchwork.dpdk.org/project/dpdk/patch/608febf8d5d3c434a1eddb2e56f425ebbd6ff0b4.1654063912.git.jackmin@nvidia.com/

v2:
- rebase.
- Add reference to "rte_flow_update_age" structure in
  "RTE_FLOW_ACTION_TYPE_AGE" definition.
- Add reference to "rte_flow_get_q_aged_flows" function in
  "RTE_FLOW_ACTION_TYPE_AGE" definition.
- Change the order of "rte_flow_update_age" structure members in
  documentation, to be aligned with the structure definition.
- Place long comment before struct member definition.

Michael Baum (3):
  ethdev: add strict queue to pre-configuration flow hints
  ethdev: add queue-based API to report aged flow rules
  ethdev: add structure for indirect AGE update

 app/test-pmd/cmdline_flow.c                 |  93 +++++++++-
 app/test-pmd/config.c                       | 177 +++++++++++++++++++-
 app/test-pmd/testpmd.h                      |   1 +
 doc/guides/prog_guide/rte_flow.rst          |  25 ++-
 doc/guides/testpmd_app_ug/testpmd_funcs.rst |  90 +++++++++-
 lib/ethdev/rte_flow.c                       |  22 +++
 lib/ethdev/rte_flow.h                       |  91 +++++++++-
 lib/ethdev/rte_flow_driver.h                |   7 +
 lib/ethdev/version.map                      |   1 +
 9 files changed, 486 insertions(+), 21 deletions(-)

-- 
2.25.1


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

* [PATCH v2 1/3] ethdev: add strict queue to pre-configuration flow hints
  2022-10-19 13:12 ` [PATCH v2 " Michael Baum
@ 2022-10-19 13:12   ` Michael Baum
  2022-10-19 13:12   ` [PATCH v2 2/3] ethdev: add queue-based API to report aged flow rules Michael Baum
                     ` (2 subsequent siblings)
  3 siblings, 0 replies; 32+ messages in thread
From: Michael Baum @ 2022-10-19 13:12 UTC (permalink / raw)
  To: dev; +Cc: Matan Azrad, Raslan Darawsheh, Ori Kam

The data-path focused flow rule management can manage flow rules in more
optimized way than traditional one by using hints provided by
application in initialization phase.

In addition to the current hints we have in port attr, more hints could
be provided by application about its behaviour.

One example is how the application do with the same flow rule ?
A. create/destroy flow on same queue but query flow on different queue
   or queue-less way (i.e, counter query)
B. All flow operations will be exactly on the same queue, by which PMD
   could be in more optimized way then A because resource could be
   isolated and access based on queue, without lock, for example.

This patch add flag about above situation and could be extended to cover
more situations.

Signed-off-by: Michael Baum <michaelba@nvidia.com>
Acked-by: Ori Kam <orika@nvidia.com>
---
 app/test-pmd/cmdline_flow.c                 | 10 ++++++++++
 doc/guides/testpmd_app_ug/testpmd_funcs.rst |  4 ++--
 lib/ethdev/rte_flow.h                       | 14 ++++++++++++++
 3 files changed, 26 insertions(+), 2 deletions(-)

diff --git a/app/test-pmd/cmdline_flow.c b/app/test-pmd/cmdline_flow.c
index 810dfb9854..59829371d4 100644
--- a/app/test-pmd/cmdline_flow.c
+++ b/app/test-pmd/cmdline_flow.c
@@ -226,6 +226,7 @@ enum index {
 	CONFIG_AGING_OBJECTS_NUMBER,
 	CONFIG_METERS_NUMBER,
 	CONFIG_CONN_TRACK_NUMBER,
+	CONFIG_FLAGS,
 
 	/* Indirect action arguments */
 	INDIRECT_ACTION_CREATE,
@@ -1092,6 +1093,7 @@ static const enum index next_config_attr[] = {
 	CONFIG_AGING_OBJECTS_NUMBER,
 	CONFIG_METERS_NUMBER,
 	CONFIG_CONN_TRACK_NUMBER,
+	CONFIG_FLAGS,
 	END,
 	ZERO,
 };
@@ -2692,6 +2694,14 @@ static const struct token token_list[] = {
 		.args = ARGS(ARGS_ENTRY(struct buffer,
 					args.configure.port_attr.nb_conn_tracks)),
 	},
+	[CONFIG_FLAGS] = {
+		.name = "flags",
+		.help = "configuration flags",
+		.next = NEXT(next_config_attr,
+			     NEXT_ENTRY(COMMON_UNSIGNED)),
+		.args = ARGS(ARGS_ENTRY(struct buffer,
+					args.configure.port_attr.flags)),
+	},
 	/* Top-level command. */
 	[PATTERN_TEMPLATE] = {
 		.name = "pattern_template",
diff --git a/doc/guides/testpmd_app_ug/testpmd_funcs.rst b/doc/guides/testpmd_app_ug/testpmd_funcs.rst
index b3f31df69a..a8b99c8c19 100644
--- a/doc/guides/testpmd_app_ug/testpmd_funcs.rst
+++ b/doc/guides/testpmd_app_ug/testpmd_funcs.rst
@@ -2891,7 +2891,7 @@ following sections.
        [queues_number {number}] [queues_size {size}]
        [counters_number {number}]
        [aging_counters_number {number}]
-       [meters_number {number}]
+       [meters_number {number}] [flags {number}]
 
 - Create a pattern template::
    flow pattern_template {port_id} create [pattern_template_id {id}]
@@ -3042,7 +3042,7 @@ for asynchronous flow creation/destruction operations. It is bound to
        [queues_number {number}] [queues_size {size}]
        [counters_number {number}]
        [aging_counters_number {number}]
-       [meters_number {number}]
+       [meters_number {number}] [flags {number}]
 
 If successful, it will show::
 
diff --git a/lib/ethdev/rte_flow.h b/lib/ethdev/rte_flow.h
index cddbe74c33..a93ec796cb 100644
--- a/lib/ethdev/rte_flow.h
+++ b/lib/ethdev/rte_flow.h
@@ -4741,6 +4741,12 @@ rte_flow_flex_item_release(uint16_t port_id,
 			   const struct rte_flow_item_flex_handle *handle,
 			   struct rte_flow_error *error);
 
+/**
+ * Indicate all operations for a given flow rule will _strictly_
+ * happen on the same queue (create/destroy/query/update).
+ */
+#define RTE_FLOW_PORT_FLAG_STRICT_QUEUE RTE_BIT32(0)
+
 /**
  * @warning
  * @b EXPERIMENTAL: this API may change without prior notice.
@@ -4774,6 +4780,10 @@ struct rte_flow_port_info {
 	 * @see RTE_FLOW_ACTION_TYPE_CONNTRACK
 	 */
 	uint32_t max_nb_conn_tracks;
+	/**
+	 * Port supported flags (RTE_FLOW_PORT_FLAG_*).
+	 */
+	uint32_t supported_flags;
 };
 
 /**
@@ -4848,6 +4858,10 @@ struct rte_flow_port_attr {
 	 * @see RTE_FLOW_ACTION_TYPE_CONNTRACK
 	 */
 	uint32_t nb_conn_tracks;
+	/**
+	 * Port flags (RTE_FLOW_PORT_FLAG_*).
+	 */
+	uint32_t flags;
 };
 
 /**
-- 
2.25.1


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

* [PATCH v2 2/3] ethdev: add queue-based API to report aged flow rules
  2022-10-19 13:12 ` [PATCH v2 " Michael Baum
  2022-10-19 13:12   ` [PATCH v2 1/3] ethdev: add strict queue to pre-configuration flow hints Michael Baum
@ 2022-10-19 13:12   ` Michael Baum
  2022-10-19 13:12   ` [PATCH v2 3/3] ethdev: add structure for indirect AGE update Michael Baum
  2022-10-19 14:49   ` [PATCH v3 0/3] ethdev: AGE action preparation Michael Baum
  3 siblings, 0 replies; 32+ messages in thread
From: Michael Baum @ 2022-10-19 13:12 UTC (permalink / raw)
  To: dev; +Cc: Matan Azrad, Raslan Darawsheh, Ori Kam

When application use queue-based flow rule management and operate the
same flow rule on the same queue, e.g create/destroy/query, API of
querying aged flow rules should also have queue id parameter just like
other queue-based flow APIs.

By this way, PMD can work in more optimized way since resources are
isolated by queue and needn't synchronize.

If application do use queue-based flow management but configure port
without RTE_FLOW_PORT_FLAG_STRICT_QUEUE, which means application operate
a given flow rule on different queues, the queue id parameter will
be ignored.

Signed-off-by: Michael Baum <michaelba@nvidia.com>
Acked-by: Ori Kam <orika@nvidia.com>
---
 app/test-pmd/cmdline_flow.c                 |  17 ++-
 app/test-pmd/config.c                       | 159 +++++++++++++++++++-
 app/test-pmd/testpmd.h                      |   1 +
 doc/guides/testpmd_app_ug/testpmd_funcs.rst |  86 ++++++++++-
 lib/ethdev/rte_flow.c                       |  22 +++
 lib/ethdev/rte_flow.h                       |  49 +++++-
 lib/ethdev/rte_flow_driver.h                |   7 +
 lib/ethdev/version.map                      |   1 +
 8 files changed, 332 insertions(+), 10 deletions(-)

diff --git a/app/test-pmd/cmdline_flow.c b/app/test-pmd/cmdline_flow.c
index 59829371d4..992aeb95b3 100644
--- a/app/test-pmd/cmdline_flow.c
+++ b/app/test-pmd/cmdline_flow.c
@@ -129,6 +129,7 @@ enum index {
 	/* Queue arguments. */
 	QUEUE_CREATE,
 	QUEUE_DESTROY,
+	QUEUE_AGED,
 	QUEUE_INDIRECT_ACTION,
 
 	/* Queue create arguments. */
@@ -1170,6 +1171,7 @@ static const enum index next_table_destroy_attr[] = {
 static const enum index next_queue_subcmd[] = {
 	QUEUE_CREATE,
 	QUEUE_DESTROY,
+	QUEUE_AGED,
 	QUEUE_INDIRECT_ACTION,
 	ZERO,
 };
@@ -2967,6 +2969,13 @@ static const struct token token_list[] = {
 		.args = ARGS(ARGS_ENTRY(struct buffer, queue)),
 		.call = parse_qo_destroy,
 	},
+	[QUEUE_AGED] = {
+		.name = "aged",
+		.help = "list and destroy aged flows",
+		.next = NEXT(next_aged_attr, NEXT_ENTRY(COMMON_QUEUE_ID)),
+		.args = ARGS(ARGS_ENTRY(struct buffer, queue)),
+		.call = parse_aged,
+	},
 	[QUEUE_INDIRECT_ACTION] = {
 		.name = "indirect_action",
 		.help = "queue indirect actions",
@@ -8654,8 +8663,8 @@ parse_aged(struct context *ctx, const struct token *token,
 	/* Nothing else to do if there is no buffer. */
 	if (!out)
 		return len;
-	if (!out->command) {
-		if (ctx->curr != AGED)
+	if (!out->command || out->command == QUEUE) {
+		if (ctx->curr != AGED && ctx->curr != QUEUE_AGED)
 			return -1;
 		if (sizeof(*out) > size)
 			return -1;
@@ -10610,6 +10619,10 @@ cmd_flow_parsed(const struct buffer *in)
 	case PULL:
 		port_queue_flow_pull(in->port, in->queue);
 		break;
+	case QUEUE_AGED:
+		port_queue_flow_aged(in->port, in->queue,
+				     in->args.aged.destroy);
+		break;
 	case QUEUE_INDIRECT_ACTION_CREATE:
 		port_queue_action_handle_create(
 				in->port, in->queue, in->postpone,
diff --git a/app/test-pmd/config.c b/app/test-pmd/config.c
index 0f7dbd698f..18f3543887 100644
--- a/app/test-pmd/config.c
+++ b/app/test-pmd/config.c
@@ -2509,6 +2509,7 @@ port_queue_flow_create(portid_t port_id, queueid_t queue_id,
 		       const struct rte_flow_action *actions)
 {
 	struct rte_flow_op_attr op_attr = { .postpone = postpone };
+	struct rte_flow_attr flow_attr = { 0 };
 	struct rte_flow *flow;
 	struct rte_port *port;
 	struct port_flow *pf;
@@ -2568,7 +2569,7 @@ port_queue_flow_create(portid_t port_id, queueid_t queue_id,
 	}
 	job->type = QUEUE_JOB_TYPE_FLOW_CREATE;
 
-	pf = port_flow_new(NULL, pattern, actions, &error);
+	pf = port_flow_new(&flow_attr, pattern, actions, &error);
 	if (!pf) {
 		free(job);
 		return port_flow_complain(&error);
@@ -2905,6 +2906,162 @@ port_queue_flow_push(portid_t port_id, queueid_t queue_id)
 	return ret;
 }
 
+/** Pull queue operation results from the queue. */
+static int
+port_queue_aged_flow_destroy(portid_t port_id, queueid_t queue_id,
+			     const uint32_t *rule, int nb_flows)
+{
+	struct rte_port *port = &ports[port_id];
+	struct rte_flow_op_result *res;
+	struct rte_flow_error error;
+	uint32_t n = nb_flows;
+	int ret = 0;
+	int i;
+
+	res = calloc(port->queue_sz, sizeof(struct rte_flow_op_result));
+	if (!res) {
+		printf("Failed to allocate memory for pulled results\n");
+		return -ENOMEM;
+	}
+
+	memset(&error, 0x66, sizeof(error));
+	while (nb_flows > 0) {
+		int success = 0;
+
+		if (n > port->queue_sz)
+			n = port->queue_sz;
+		ret = port_queue_flow_destroy(port_id, queue_id, true, n, rule);
+		if (ret < 0) {
+			free(res);
+			return ret;
+		}
+		ret = rte_flow_push(port_id, queue_id, &error);
+		if (ret < 0) {
+			printf("Failed to push operations in the queue: %s\n",
+			       strerror(-ret));
+			free(res);
+			return ret;
+		}
+		while (success < nb_flows) {
+			ret = rte_flow_pull(port_id, queue_id, res,
+					    port->queue_sz, &error);
+			if (ret < 0) {
+				printf("Failed to pull a operation results: %s\n",
+				       strerror(-ret));
+				free(res);
+				return ret;
+			}
+
+			for (i = 0; i < ret; i++) {
+				if (res[i].status == RTE_FLOW_OP_SUCCESS)
+					success++;
+			}
+		}
+		rule += n;
+		nb_flows -= n;
+		n = nb_flows;
+	}
+
+	free(res);
+	return ret;
+}
+
+/** List simply and destroy all aged flows per queue. */
+void
+port_queue_flow_aged(portid_t port_id, uint32_t queue_id, uint8_t destroy)
+{
+	void **contexts;
+	int nb_context, total = 0, idx;
+	uint32_t *rules = NULL;
+	struct rte_port *port;
+	struct rte_flow_error error;
+	enum age_action_context_type *type;
+	union {
+		struct port_flow *pf;
+		struct port_indirect_action *pia;
+	} ctx;
+
+	if (port_id_is_invalid(port_id, ENABLED_WARN) ||
+	    port_id == (portid_t)RTE_PORT_ALL)
+		return;
+	port = &ports[port_id];
+	if (queue_id >= port->queue_nb) {
+		printf("Error: queue #%u is invalid\n", queue_id);
+		return;
+	}
+	total = rte_flow_get_q_aged_flows(port_id, queue_id, NULL, 0, &error);
+	if (total < 0) {
+		port_flow_complain(&error);
+		return;
+	}
+	printf("Port %u queue %u total aged flows: %d\n",
+	       port_id, queue_id, total);
+	if (total == 0)
+		return;
+	contexts = calloc(total, sizeof(void *));
+	if (contexts == NULL) {
+		printf("Cannot allocate contexts for aged flow\n");
+		return;
+	}
+	printf("%-20s\tID\tGroup\tPrio\tAttr\n", "Type");
+	nb_context = rte_flow_get_q_aged_flows(port_id, queue_id, contexts,
+					       total, &error);
+	if (nb_context > total) {
+		printf("Port %u queue %u get aged flows count(%d) > total(%d)\n",
+		       port_id, queue_id, nb_context, total);
+		free(contexts);
+		return;
+	}
+	if (destroy) {
+		rules = malloc(sizeof(uint32_t) * nb_context);
+		if (rules == NULL)
+			printf("Cannot allocate memory for destroy aged flow\n");
+	}
+	total = 0;
+	for (idx = 0; idx < nb_context; idx++) {
+		if (!contexts[idx]) {
+			printf("Error: get Null context in port %u queue %u\n",
+			       port_id, queue_id);
+			continue;
+		}
+		type = (enum age_action_context_type *)contexts[idx];
+		switch (*type) {
+		case ACTION_AGE_CONTEXT_TYPE_FLOW:
+			ctx.pf = container_of(type, struct port_flow, age_type);
+			printf("%-20s\t%" PRIu32 "\t%" PRIu32 "\t%" PRIu32
+								 "\t%c%c%c\t\n",
+			       "Flow",
+			       ctx.pf->id,
+			       ctx.pf->rule.attr->group,
+			       ctx.pf->rule.attr->priority,
+			       ctx.pf->rule.attr->ingress ? 'i' : '-',
+			       ctx.pf->rule.attr->egress ? 'e' : '-',
+			       ctx.pf->rule.attr->transfer ? 't' : '-');
+			if (rules != NULL) {
+				rules[total] = ctx.pf->id;
+				total++;
+			}
+			break;
+		case ACTION_AGE_CONTEXT_TYPE_INDIRECT_ACTION:
+			ctx.pia = container_of(type,
+					       struct port_indirect_action,
+					       age_type);
+			printf("%-20s\t%" PRIu32 "\n", "Indirect action",
+			       ctx.pia->id);
+			break;
+		default:
+			printf("Error: invalid context type %u\n", port_id);
+			break;
+		}
+	}
+	if (rules != NULL) {
+		port_queue_aged_flow_destroy(port_id, queue_id, rules, total);
+		free(rules);
+	}
+	printf("\n%d flows destroyed\n", total);
+	free(contexts);
+}
+
 /** Pull queue operation results from the queue. */
 int
 port_queue_flow_pull(portid_t port_id, queueid_t queue_id)
diff --git a/app/test-pmd/testpmd.h b/app/test-pmd/testpmd.h
index acdb7e855d..918c2377d8 100644
--- a/app/test-pmd/testpmd.h
+++ b/app/test-pmd/testpmd.h
@@ -941,6 +941,7 @@ int port_queue_action_handle_query(portid_t port_id, uint32_t queue_id,
 				   bool postpone, uint32_t id);
 int port_queue_flow_push(portid_t port_id, queueid_t queue_id);
 int port_queue_flow_pull(portid_t port_id, queueid_t queue_id);
+void port_queue_flow_aged(portid_t port_id, uint32_t queue_id, uint8_t destroy);
 int port_flow_validate(portid_t port_id,
 		       const struct rte_flow_attr *attr,
 		       const struct rte_flow_item *pattern,
diff --git a/doc/guides/testpmd_app_ug/testpmd_funcs.rst b/doc/guides/testpmd_app_ug/testpmd_funcs.rst
index a8b99c8c19..8e21b2a5b7 100644
--- a/doc/guides/testpmd_app_ug/testpmd_funcs.rst
+++ b/doc/guides/testpmd_app_ug/testpmd_funcs.rst
@@ -2894,9 +2894,10 @@ following sections.
        [meters_number {number}] [flags {number}]
 
 - Create a pattern template::
+
    flow pattern_template {port_id} create [pattern_template_id {id}]
        [relaxed {boolean}] [ingress] [egress] [transfer]
-	   template {item} [/ {item} [...]] / end
+       template {item} [/ {item} [...]] / end
 
 - Destroy a pattern template::
 
@@ -2995,6 +2996,10 @@ following sections.
 
    flow aged {port_id} [destroy]
 
+- Enqueue list and destroy aged flow rules::
+
+   flow queue {port_id} aged {queue_id} [destroy]
+
 - Tunnel offload - create a tunnel stub::
 
    flow tunnel create {port_id} type {tunnel_type}
@@ -4236,7 +4241,7 @@ Disabling isolated mode::
  testpmd>
 
 Dumping HW internal information
-~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
 
 ``flow dump`` dumps the hardware's internal representation information of
 all flows. It is bound to ``rte_flow_dev_dump()``::
@@ -4252,10 +4257,10 @@ 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.
+and ``destroy`` parameter can be used to destroy those flow rules in PMD::
 
    flow aged {port_id} [destroy]
 
@@ -4290,7 +4295,7 @@ will be ID 3, ID 1, ID 0::
    1       0       0       i--
    0       0       0       i--
 
-If attach ``destroy`` parameter, the command will destroy all the list aged flow rules.
+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
@@ -4308,6 +4313,77 @@ If attach ``destroy`` parameter, the command will destroy all the list aged flow
    testpmd> flow aged 0
    Port 0 total aged flows: 0
 
+
+Enqueueing listing and destroying aged flow rules
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+
+``flow queue aged`` simply lists aged flow rules be get from
+``rte_flow_get_q_aged_flows`` API, and ``destroy`` parameter can be used to
+destroy those flow rules in PMD::
+
+   flow queue {port_id} aged {queue_id} [destroy]
+
+Listing current aged flow rules::
+
+   testpmd> flow queue 0 aged 0
+   Port 0 queue 0 total aged flows: 0
+   testpmd> flow queue 0 create 0 ingress tanle 0 item_template 0 action_template 0
+      pattern eth / ipv4 src is 2.2.2.14 / end
+      actions age timeout 5 / queue index 0 /  end
+   Flow rule #0 creation enqueued
+   testpmd> flow queue 0 create 0 ingress tanle 0 item_template 0 action_template 0
+      pattern eth / ipv4 src is 2.2.2.15 / end
+      actions age timeout 4 / queue index 0 /  end
+   Flow rule #1 creation enqueued
+   testpmd> flow queue 0 create 0 ingress tanle 0 item_template 0 action_template 0
+      pattern eth / ipv4 src is 2.2.2.16 / end
+      actions age timeout 4 / queue index 0 /  end
+   Flow rule #2 creation enqueued
+   testpmd> flow queue 0 create 0 ingress tanle 0 item_template 0 action_template 0
+      pattern eth / ipv4 src is 2.2.2.17 / end
+      actions age timeout 4 / queue index 0 /  end
+   Flow rule #3 creation enqueued
+   testpmd> flow pull 0 queue 0
+   Queue #0 pulled 4 operations (0 failed, 4 succeeded)
+
+Aged Rules are simply list as command ``flow queue {port_id} list {queue_id}``,
+but strip the detail rule information, all the aged flows are sorted by the
+longest timeout time. For example, if those rules is 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 queue 0 aged 0
+   Port 0 queue 0 total aged flows: 4
+   ID      Group   Prio    Attr
+   2       0       0       ---
+   3       0       0       ---
+   1       0       0       ---
+   0       0       0       ---
+
+   0 flows destroyed
+
+If attach ``destroy`` parameter, the command will destroy all the list aged flow rules::
+
+   testpmd> flow queue 0 aged 0 destroy
+   Port 0 queue 0 total aged flows: 4
+   ID      Group   Prio    Attr
+   2       0       0       ---
+   3       0       0       ---
+   1       0       0       ---
+   0       0       0       ---
+   Flow rule #2 destruction enqueued
+   Flow rule #3 destruction enqueued
+   Flow rule #1 destruction enqueued
+   Flow rule #0 destruction enqueued
+
+   4 flows destroyed
+   testpmd> flow queue 0 aged 0
+   Port 0 total aged flows: 0
+
+.. note::
+
+   The queue must be empty before attaching ``destroy`` parameter.
+
+
 Creating indirect actions
 ~~~~~~~~~~~~~~~~~~~~~~~~~
 
diff --git a/lib/ethdev/rte_flow.c b/lib/ethdev/rte_flow.c
index d11ba270db..7d0c24366c 100644
--- a/lib/ethdev/rte_flow.c
+++ b/lib/ethdev/rte_flow.c
@@ -1132,6 +1132,28 @@ rte_flow_get_aged_flows(uint16_t port_id, void **contexts,
 				  NULL, rte_strerror(ENOTSUP));
 }
 
+int
+rte_flow_get_q_aged_flows(uint16_t port_id, uint32_t queue_id, void **contexts,
+			  uint32_t nb_contexts, struct rte_flow_error *error)
+{
+	struct rte_eth_dev *dev = &rte_eth_devices[port_id];
+	const struct rte_flow_ops *ops = rte_flow_ops_get(port_id, error);
+	int ret;
+
+	if (unlikely(!ops))
+		return -rte_errno;
+	if (likely(!!ops->get_q_aged_flows)) {
+		fts_enter(dev);
+		ret = ops->get_q_aged_flows(dev, queue_id, contexts,
+					    nb_contexts, error);
+		fts_exit(dev);
+		return flow_err(port_id, ret, error);
+	}
+	return rte_flow_error_set(error, ENOTSUP,
+				  RTE_FLOW_ERROR_TYPE_UNSPECIFIED,
+				  NULL, rte_strerror(ENOTSUP));
+}
+
 struct rte_flow_action_handle *
 rte_flow_action_handle_create(uint16_t port_id,
 			      const struct rte_flow_indir_action_conf *conf,
diff --git a/lib/ethdev/rte_flow.h b/lib/ethdev/rte_flow.h
index a93ec796cb..64ec8f0903 100644
--- a/lib/ethdev/rte_flow.h
+++ b/lib/ethdev/rte_flow.h
@@ -2639,6 +2639,7 @@ enum rte_flow_action_type {
 	 * flow.
 	 *
 	 * See struct rte_flow_action_age.
+	 * See function rte_flow_get_q_aged_flows
 	 * See function rte_flow_get_aged_flows
 	 * see enum RTE_ETH_EVENT_FLOW_AGED
 	 * See struct rte_flow_query_age
@@ -2784,8 +2785,8 @@ struct rte_flow_action_queue {
  * on the flow. RTE_ETH_EVENT_FLOW_AGED event is triggered when a
  * port detects new aged-out flows.
  *
- * The flow context and the flow handle will be reported by the
- * rte_flow_get_aged_flows API.
+ * The flow context and the flow handle will be reported by the either
+ * rte_flow_get_aged_flows or rte_flow_get_q_aged_flows APIs.
  */
 struct rte_flow_action_age {
 	uint32_t timeout:24; /**< Time in seconds. */
@@ -4314,6 +4315,50 @@ int
 rte_flow_get_aged_flows(uint16_t port_id, void **contexts,
 			uint32_t nb_contexts, struct rte_flow_error *error);
 
+/**
+ * @warning
+ * @b EXPERIMENTAL: this API may change without prior notice.
+ *
+ * Get aged-out flows of a given port on the given flow queue.
+ *
+ * If application configure port attribute with RTE_FLOW_PORT_FLAG_STRICT_QUEUE,
+ * there is no RTE_ETH_EVENT_FLOW_AGED event and this function must be called to
+ * get the aged flows synchronously.
+ *
+ * If application configure port attribute without
+ * RTE_FLOW_PORT_FLAG_STRICT_QUEUE, RTE_ETH_EVENT_FLOW_AGED event will be
+ * triggered at least one new aged out flow was detected on any flow queue after
+ * the last call to rte_flow_get_q_aged_flows.
+ * In addition, the @p queue_id will be ignored.
+ * This function can be called to get the aged flows asynchronously from the
+ * event callback or synchronously regardless the event.
+ *
+ * @param[in] port_id
+ *   Port identifier of Ethernet device.
+ * @param[in] queue_id
+ *   Flow queue to query. Ignored when RTE_FLOW_PORT_FLAG_STRICT_QUEUE not set.
+ * @param[in, out] contexts
+ *   The address of an array of pointers to the aged-out flows contexts.
+ * @param[in] nb_contexts
+ *   The length of context array pointers.
+ * @param[out] error
+ *   Perform verbose error reporting if not NULL. Initialized in case of
+ *   error only.
+ *
+ * @return
+ *   if nb_contexts is 0, return the amount of all aged contexts.
+ *   if nb_contexts is not 0 , return the amount of aged flows reported
+ *   in the context array, otherwise negative errno value.
+ *
+ * @see rte_flow_action_age
+ * @see RTE_ETH_EVENT_FLOW_AGED
+ * @see rte_flow_port_flag
+ */
+__rte_experimental
+int
+rte_flow_get_q_aged_flows(uint16_t port_id, uint32_t queue_id, void **contexts,
+			  uint32_t nb_contexts, struct rte_flow_error *error);
+
 /**
  * Specify indirect action object configuration
  */
diff --git a/lib/ethdev/rte_flow_driver.h b/lib/ethdev/rte_flow_driver.h
index 7289deb538..c7d0699c91 100644
--- a/lib/ethdev/rte_flow_driver.h
+++ b/lib/ethdev/rte_flow_driver.h
@@ -84,6 +84,13 @@ struct rte_flow_ops {
 		 void **context,
 		 uint32_t nb_contexts,
 		 struct rte_flow_error *err);
+	/** See rte_flow_get_q_aged_flows() */
+	int (*get_q_aged_flows)
+		(struct rte_eth_dev *dev,
+		 uint32_t queue_id,
+		 void **contexts,
+		 uint32_t nb_contexts,
+		 struct rte_flow_error *error);
 	/** See rte_flow_action_handle_create() */
 	struct rte_flow_action_handle *(*action_handle_create)
 		(struct rte_eth_dev *dev,
diff --git a/lib/ethdev/version.map b/lib/ethdev/version.map
index e749678b96..17201fbe0f 100644
--- a/lib/ethdev/version.map
+++ b/lib/ethdev/version.map
@@ -295,6 +295,7 @@ EXPERIMENTAL {
 	rte_eth_rx_descriptor_dump;
 	rte_eth_tx_descriptor_dump;
 	rte_flow_async_action_handle_query;
+	rte_flow_get_q_aged_flows;
 	rte_mtr_meter_policy_get;
 	rte_mtr_meter_profile_get;
 };
-- 
2.25.1


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

* [PATCH v2 3/3] ethdev: add structure for indirect AGE update
  2022-10-19 13:12 ` [PATCH v2 " Michael Baum
  2022-10-19 13:12   ` [PATCH v2 1/3] ethdev: add strict queue to pre-configuration flow hints Michael Baum
  2022-10-19 13:12   ` [PATCH v2 2/3] ethdev: add queue-based API to report aged flow rules Michael Baum
@ 2022-10-19 13:12   ` Michael Baum
  2022-10-19 14:49   ` [PATCH v3 0/3] ethdev: AGE action preparation Michael Baum
  3 siblings, 0 replies; 32+ messages in thread
From: Michael Baum @ 2022-10-19 13:12 UTC (permalink / raw)
  To: dev; +Cc: Matan Azrad, Raslan Darawsheh, Ori Kam

Add a new structure for indirect AGE update.

This new structure enables:
1. Update timeout value.
2. Stop AGE checking.
3. Start AGE checking.
4. restart AGE checking.

Signed-off-by: Michael Baum <michaelba@nvidia.com>
Acked-by: Ori Kam <orika@nvidia.com>
---
 app/test-pmd/cmdline_flow.c        | 66 ++++++++++++++++++++++++++++++
 app/test-pmd/config.c              | 18 +++++---
 doc/guides/prog_guide/rte_flow.rst | 25 +++++++++--
 lib/ethdev/rte_flow.h              | 28 +++++++++++++
 4 files changed, 128 insertions(+), 9 deletions(-)

diff --git a/app/test-pmd/cmdline_flow.c b/app/test-pmd/cmdline_flow.c
index 992aeb95b3..88108498e0 100644
--- a/app/test-pmd/cmdline_flow.c
+++ b/app/test-pmd/cmdline_flow.c
@@ -586,6 +586,9 @@ enum index {
 	ACTION_SET_IPV6_DSCP_VALUE,
 	ACTION_AGE,
 	ACTION_AGE_TIMEOUT,
+	ACTION_AGE_UPDATE,
+	ACTION_AGE_UPDATE_TIMEOUT,
+	ACTION_AGE_UPDATE_TOUCH,
 	ACTION_SAMPLE,
 	ACTION_SAMPLE_RATIO,
 	ACTION_SAMPLE_INDEX,
@@ -1874,6 +1877,7 @@ static const enum index next_action[] = {
 	ACTION_SET_IPV4_DSCP,
 	ACTION_SET_IPV6_DSCP,
 	ACTION_AGE,
+	ACTION_AGE_UPDATE,
 	ACTION_SAMPLE,
 	ACTION_INDIRECT,
 	ACTION_MODIFY_FIELD,
@@ -2110,6 +2114,14 @@ static const enum index action_age[] = {
 	ZERO,
 };
 
+static const enum index action_age_update[] = {
+	ACTION_AGE_UPDATE,
+	ACTION_AGE_UPDATE_TIMEOUT,
+	ACTION_AGE_UPDATE_TOUCH,
+	ACTION_NEXT,
+	ZERO,
+};
+
 static const enum index action_sample[] = {
 	ACTION_SAMPLE,
 	ACTION_SAMPLE_RATIO,
@@ -2188,6 +2200,9 @@ static int parse_vc_spec(struct context *, const struct token *,
 			 const char *, unsigned int, void *, unsigned int);
 static int parse_vc_conf(struct context *, const struct token *,
 			 const char *, unsigned int, void *, unsigned int);
+static int parse_vc_conf_timeout(struct context *, const struct token *,
+				 const char *, unsigned int, void *,
+				 unsigned int);
 static int parse_vc_item_ecpri_type(struct context *, const struct token *,
 				    const char *, unsigned int,
 				    void *, unsigned int);
@@ -6206,6 +6221,30 @@ static const struct token token_list[] = {
 		.next = NEXT(action_age, NEXT_ENTRY(COMMON_UNSIGNED)),
 		.call = parse_vc_conf,
 	},
+	[ACTION_AGE_UPDATE] = {
+		.name = "age_update",
+		.help = "update aging parameter",
+		.next = NEXT(action_age_update),
+		.priv = PRIV_ACTION(AGE,
+				    sizeof(struct rte_flow_update_age)),
+		.call = parse_vc,
+	},
+	[ACTION_AGE_UPDATE_TIMEOUT] = {
+		.name = "timeout",
+		.help = "age timeout update value",
+		.args = ARGS(ARGS_ENTRY_BF(struct rte_flow_update_age,
+					   timeout, 24)),
+		.next = NEXT(action_age_update, NEXT_ENTRY(COMMON_UNSIGNED)),
+		.call = parse_vc_conf_timeout,
+	},
+	[ACTION_AGE_UPDATE_TOUCH] = {
+		.name = "touch",
+		.help = "this flow is touched",
+		.next = NEXT(action_age_update, NEXT_ENTRY(COMMON_BOOLEAN)),
+		.args = ARGS(ARGS_ENTRY_BF(struct rte_flow_update_age,
+					   touch, 1)),
+		.call = parse_vc_conf,
+	},
 	[ACTION_SAMPLE] = {
 		.name = "sample",
 		.help = "set a sample action",
@@ -7045,6 +7084,33 @@ parse_vc_conf(struct context *ctx, const struct token *token,
 	return len;
 }
 
+/** Parse action configuration field. */
+static int
+parse_vc_conf_timeout(struct context *ctx, const struct token *token,
+		      const char *str, unsigned int len,
+		      void *buf, unsigned int size)
+{
+	struct buffer *out = buf;
+	struct rte_flow_update_age *update;
+
+	(void)size;
+	if (ctx->curr != ACTION_AGE_UPDATE_TIMEOUT)
+		return -1;
+	/* 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;
+	/* Point to selected object. */
+	ctx->object = out->args.vc.data;
+	ctx->objmask = NULL;
+	/* Update the timeout is valid. */
+	update = (struct rte_flow_update_age *)out->args.vc.data;
+	update->timeout_valid = 1;
+	return len;
+}
+
 /** Parse eCPRI common header type field. */
 static int
 parse_vc_item_ecpri_type(struct context *ctx, const struct token *token,
diff --git a/app/test-pmd/config.c b/app/test-pmd/config.c
index 18f3543887..d036fff095 100644
--- a/app/test-pmd/config.c
+++ b/app/test-pmd/config.c
@@ -1886,6 +1886,7 @@ port_action_handle_update(portid_t port_id, uint32_t id,
 	if (!pia)
 		return -EINVAL;
 	switch (pia->type) {
+	case RTE_FLOW_ACTION_TYPE_AGE:
 	case RTE_FLOW_ACTION_TYPE_CONNTRACK:
 		update = action->conf;
 		break;
@@ -2816,17 +2817,22 @@ port_queue_action_handle_update(portid_t port_id,
 		return -EINVAL;
 	}
 
-	if (pia->type == RTE_FLOW_ACTION_TYPE_METER_MARK) {
+	switch (pia->type) {
+	case RTE_FLOW_ACTION_TYPE_AGE:
+		update = action->conf;
+		break;
+	case RTE_FLOW_ACTION_TYPE_METER_MARK:
 		rte_memcpy(&mtr_update.meter_mark, action->conf,
 			sizeof(struct rte_flow_action_meter_mark));
 		mtr_update.profile_valid = 1;
-		mtr_update.policy_valid  = 1;
-		mtr_update.color_mode_valid  = 1;
-		mtr_update.init_color_valid  = 1;
-		mtr_update.state_valid  = 1;
+		mtr_update.policy_valid = 1;
+		mtr_update.color_mode_valid = 1;
+		mtr_update.init_color_valid = 1;
+		mtr_update.state_valid = 1;
 		update = &mtr_update;
-	} else {
+	default:
 		update = action;
+		break;
 	}
 
 	if (rte_flow_async_action_handle_update(port_id, queue_id, &attr,
diff --git a/doc/guides/prog_guide/rte_flow.rst b/doc/guides/prog_guide/rte_flow.rst
index 565868aeea..1ce0277e65 100644
--- a/doc/guides/prog_guide/rte_flow.rst
+++ b/doc/guides/prog_guide/rte_flow.rst
@@ -2737,7 +2737,7 @@ Otherwise, RTE_FLOW_ERROR_TYPE_ACTION error will be returned.
 Action: ``AGE``
 ^^^^^^^^^^^^^^^
 
-Set ageing timeout configuration to a flow.
+Set aging timeout configuration to a flow.
 
 Event RTE_ETH_EVENT_FLOW_AGED will be reported if
 timeout passed without any matching on the flow.
@@ -2756,8 +2756,8 @@ timeout passed without any matching on the flow.
    | ``context``  | user input flow context         |
    +--------------+---------------------------------+
 
-Query structure to retrieve ageing status information of a
-shared AGE action, or a flow rule using the AGE action:
+Query structure to retrieve aging status information of an
+indirect AGE action, or a flow rule using the AGE action:
 
 .. _table_rte_flow_query_age:
 
@@ -2773,6 +2773,25 @@ shared AGE action, or a flow rule using the AGE action:
    | ``sec_since_last_hit``       | out | Seconds since last traffic hit         |
    +------------------------------+-----+----------------------------------------+
 
+Update structure to modify the parameters of an indirect AGE action.
+The update structure is used by ``rte_flow_action_handle_update()`` function.
+
+.. _table_rte_flow_update_age:
+
+.. table:: AGE update
+
+   +-------------------+--------------------------------------------------------------+
+   | Field             | Value                                                        |
+   +===================+==============================================================+
+   | ``reserved``      | 6 bits reserved, must be zero                                |
+   +-------------------+--------------------------------------------------------------+
+   | ``timeout_valid`` | 1 bit, timeout value is valid                                |
+   +-------------------+--------------------------------------------------------------+
+   | ``timeout``       | 24 bits timeout value                                        |
+   +-------------------+--------------------------------------------------------------+
+   | ``touch``         | 1 bit, touch the AGE action to set ``sec_since_last_hit`` 0  |
+   +-------------------+--------------------------------------------------------------+
+
 Action: ``SAMPLE``
 ^^^^^^^^^^^^^^^^^^
 
diff --git a/lib/ethdev/rte_flow.h b/lib/ethdev/rte_flow.h
index 64ec8f0903..a2101e0e11 100644
--- a/lib/ethdev/rte_flow.h
+++ b/lib/ethdev/rte_flow.h
@@ -2643,6 +2643,7 @@ enum rte_flow_action_type {
 	 * See function rte_flow_get_aged_flows
 	 * see enum RTE_ETH_EVENT_FLOW_AGED
 	 * See struct rte_flow_query_age
+	 * See struct rte_flow_update_age
 	 */
 	RTE_FLOW_ACTION_TYPE_AGE,
 
@@ -2809,6 +2810,33 @@ struct rte_flow_query_age {
 	uint32_t sec_since_last_hit:24; /**< Seconds since last traffic hit. */
 };
 
+/**
+ * @warning
+ * @b EXPERIMENTAL: this structure may change without prior notice
+ *
+ * RTE_FLOW_ACTION_TYPE_AGE
+ *
+ * Update indirect AGE action attributes:
+ *  - Timeout can be updated including stop/start action:
+ *     +-------------+-------------+------------------------------+
+ *     | Old Timeout | New Timeout | Updating                     |
+ *     +=============+=============+==============================+
+ *     | 0           | positive    | Start aging with new value   |
+ *     +-------------+-------------+------------------------------+
+ *     | positive    | 0           | Stop aging			  |
+ *     +-------------+-------------+------------------------------+
+ *     | positive    | positive    | Change timeout to new value  |
+ *     +-------------+-------------+------------------------------+
+ *  - sec_since_last_hit can be reset.
+ */
+struct rte_flow_update_age {
+	uint32_t reserved:6; /**< Reserved, must be zero. */
+	uint32_t timeout_valid:1; /**< The timeout is valid for update. */
+	uint32_t timeout:24; /**< Time in seconds. */
+	/**< Means that aging should assume packet passed the aging. */
+	uint32_t touch:1;
+};
+
 /**
  * @warning
  * @b EXPERIMENTAL: this structure may change without prior notice
-- 
2.25.1


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

* [PATCH v3 0/3] ethdev: AGE action preparation
  2022-10-19 13:12 ` [PATCH v2 " Michael Baum
                     ` (2 preceding siblings ...)
  2022-10-19 13:12   ` [PATCH v2 3/3] ethdev: add structure for indirect AGE update Michael Baum
@ 2022-10-19 14:49   ` Michael Baum
  2022-10-19 14:49     ` [PATCH v3 1/3] ethdev: add strict queue to pre-configuration flow hints Michael Baum
                       ` (3 more replies)
  3 siblings, 4 replies; 32+ messages in thread
From: Michael Baum @ 2022-10-19 14:49 UTC (permalink / raw)
  To: dev; +Cc: Matan Azrad, Raslan Darawsheh, Ori Kam

RFC's:
https://patchwork.dpdk.org/project/dpdk/patch/7a45693f478b1b721b4e05131141b526185a175c.1654063912.git.jackmin@nvidia.com/
https://patchwork.dpdk.org/project/dpdk/patch/608febf8d5d3c434a1eddb2e56f425ebbd6ff0b4.1654063912.git.jackmin@nvidia.com/

v2:
- rebase.
- Add reference to "rte_flow_update_age" structure in
  "RTE_FLOW_ACTION_TYPE_AGE" definition.
- Add reference to "rte_flow_get_q_aged_flows" function in
  "RTE_FLOW_ACTION_TYPE_AGE" definition.
- Change the order of "rte_flow_update_age" structure members in
  documentation, to be aligned with the structure definition.
- Place long comment before struct member definition.

v3:
- Fix miss "break" in indirect action update switch-case.


Michael Baum (3):
  ethdev: add strict queue to pre-configuration flow hints
  ethdev: add queue-based API to report aged flow rules
  ethdev: add structure for indirect AGE update

 app/test-pmd/cmdline_flow.c                 |  93 +++++++++-
 app/test-pmd/config.c                       | 178 +++++++++++++++++++-
 app/test-pmd/testpmd.h                      |   1 +
 doc/guides/prog_guide/rte_flow.rst          |  25 ++-
 doc/guides/testpmd_app_ug/testpmd_funcs.rst |  90 +++++++++-
 lib/ethdev/rte_flow.c                       |  22 +++
 lib/ethdev/rte_flow.h                       |  91 +++++++++-
 lib/ethdev/rte_flow_driver.h                |   7 +
 lib/ethdev/version.map                      |   1 +
 9 files changed, 487 insertions(+), 21 deletions(-)

-- 
2.25.1


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

* [PATCH v3 1/3] ethdev: add strict queue to pre-configuration flow hints
  2022-10-19 14:49   ` [PATCH v3 0/3] ethdev: AGE action preparation Michael Baum
@ 2022-10-19 14:49     ` Michael Baum
  2022-10-26 19:10       ` Andrew Rybchenko
  2022-10-19 14:49     ` [PATCH v3 2/3] ethdev: add queue-based API to report aged flow rules Michael Baum
                       ` (2 subsequent siblings)
  3 siblings, 1 reply; 32+ messages in thread
From: Michael Baum @ 2022-10-19 14:49 UTC (permalink / raw)
  To: dev; +Cc: Matan Azrad, Raslan Darawsheh, Ori Kam

The data-path focused flow rule management can manage flow rules in more
optimized way than traditional one by using hints provided by
application in initialization phase.

In addition to the current hints we have in port attr, more hints could
be provided by application about its behaviour.

One example is how the application do with the same flow rule ?
A. create/destroy flow on same queue but query flow on different queue
   or queue-less way (i.e, counter query)
B. All flow operations will be exactly on the same queue, by which PMD
   could be in more optimized way then A because resource could be
   isolated and access based on queue, without lock, for example.

This patch add flag about above situation and could be extended to cover
more situations.

Signed-off-by: Michael Baum <michaelba@nvidia.com>
Acked-by: Ori Kam <orika@nvidia.com>
---
 app/test-pmd/cmdline_flow.c                 | 10 ++++++++++
 doc/guides/testpmd_app_ug/testpmd_funcs.rst |  4 ++--
 lib/ethdev/rte_flow.h                       | 14 ++++++++++++++
 3 files changed, 26 insertions(+), 2 deletions(-)

diff --git a/app/test-pmd/cmdline_flow.c b/app/test-pmd/cmdline_flow.c
index 810dfb9854..59829371d4 100644
--- a/app/test-pmd/cmdline_flow.c
+++ b/app/test-pmd/cmdline_flow.c
@@ -226,6 +226,7 @@ enum index {
 	CONFIG_AGING_OBJECTS_NUMBER,
 	CONFIG_METERS_NUMBER,
 	CONFIG_CONN_TRACK_NUMBER,
+	CONFIG_FLAGS,
 
 	/* Indirect action arguments */
 	INDIRECT_ACTION_CREATE,
@@ -1092,6 +1093,7 @@ static const enum index next_config_attr[] = {
 	CONFIG_AGING_OBJECTS_NUMBER,
 	CONFIG_METERS_NUMBER,
 	CONFIG_CONN_TRACK_NUMBER,
+	CONFIG_FLAGS,
 	END,
 	ZERO,
 };
@@ -2692,6 +2694,14 @@ static const struct token token_list[] = {
 		.args = ARGS(ARGS_ENTRY(struct buffer,
 					args.configure.port_attr.nb_conn_tracks)),
 	},
+	[CONFIG_FLAGS] = {
+		.name = "flags",
+		.help = "configuration flags",
+		.next = NEXT(next_config_attr,
+			     NEXT_ENTRY(COMMON_UNSIGNED)),
+		.args = ARGS(ARGS_ENTRY(struct buffer,
+					args.configure.port_attr.flags)),
+	},
 	/* Top-level command. */
 	[PATTERN_TEMPLATE] = {
 		.name = "pattern_template",
diff --git a/doc/guides/testpmd_app_ug/testpmd_funcs.rst b/doc/guides/testpmd_app_ug/testpmd_funcs.rst
index b3f31df69a..a8b99c8c19 100644
--- a/doc/guides/testpmd_app_ug/testpmd_funcs.rst
+++ b/doc/guides/testpmd_app_ug/testpmd_funcs.rst
@@ -2891,7 +2891,7 @@ following sections.
        [queues_number {number}] [queues_size {size}]
        [counters_number {number}]
        [aging_counters_number {number}]
-       [meters_number {number}]
+       [meters_number {number}] [flags {number}]
 
 - Create a pattern template::
    flow pattern_template {port_id} create [pattern_template_id {id}]
@@ -3042,7 +3042,7 @@ for asynchronous flow creation/destruction operations. It is bound to
        [queues_number {number}] [queues_size {size}]
        [counters_number {number}]
        [aging_counters_number {number}]
-       [meters_number {number}]
+       [meters_number {number}] [flags {number}]
 
 If successful, it will show::
 
diff --git a/lib/ethdev/rte_flow.h b/lib/ethdev/rte_flow.h
index cddbe74c33..a93ec796cb 100644
--- a/lib/ethdev/rte_flow.h
+++ b/lib/ethdev/rte_flow.h
@@ -4741,6 +4741,12 @@ rte_flow_flex_item_release(uint16_t port_id,
 			   const struct rte_flow_item_flex_handle *handle,
 			   struct rte_flow_error *error);
 
+/**
+ * Indicate all operations for a given flow rule will _strictly_
+ * happen on the same queue (create/destroy/query/update).
+ */
+#define RTE_FLOW_PORT_FLAG_STRICT_QUEUE RTE_BIT32(0)
+
 /**
  * @warning
  * @b EXPERIMENTAL: this API may change without prior notice.
@@ -4774,6 +4780,10 @@ struct rte_flow_port_info {
 	 * @see RTE_FLOW_ACTION_TYPE_CONNTRACK
 	 */
 	uint32_t max_nb_conn_tracks;
+	/**
+	 * Port supported flags (RTE_FLOW_PORT_FLAG_*).
+	 */
+	uint32_t supported_flags;
 };
 
 /**
@@ -4848,6 +4858,10 @@ struct rte_flow_port_attr {
 	 * @see RTE_FLOW_ACTION_TYPE_CONNTRACK
 	 */
 	uint32_t nb_conn_tracks;
+	/**
+	 * Port flags (RTE_FLOW_PORT_FLAG_*).
+	 */
+	uint32_t flags;
 };
 
 /**
-- 
2.25.1


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

* [PATCH v3 2/3] ethdev: add queue-based API to report aged flow rules
  2022-10-19 14:49   ` [PATCH v3 0/3] ethdev: AGE action preparation Michael Baum
  2022-10-19 14:49     ` [PATCH v3 1/3] ethdev: add strict queue to pre-configuration flow hints Michael Baum
@ 2022-10-19 14:49     ` Michael Baum
  2022-10-26 19:15       ` Andrew Rybchenko
  2022-10-19 14:49     ` [PATCH v3 3/3] ethdev: add structure for indirect AGE update Michael Baum
  2022-10-26 21:49     ` [PATCH v4 0/3] ethdev: AGE action preparation Michael Baum
  3 siblings, 1 reply; 32+ messages in thread
From: Michael Baum @ 2022-10-19 14:49 UTC (permalink / raw)
  To: dev; +Cc: Matan Azrad, Raslan Darawsheh, Ori Kam

When application use queue-based flow rule management and operate the
same flow rule on the same queue, e.g create/destroy/query, API of
querying aged flow rules should also have queue id parameter just like
other queue-based flow APIs.

By this way, PMD can work in more optimized way since resources are
isolated by queue and needn't synchronize.

If application do use queue-based flow management but configure port
without RTE_FLOW_PORT_FLAG_STRICT_QUEUE, which means application operate
a given flow rule on different queues, the queue id parameter will
be ignored.

Signed-off-by: Michael Baum <michaelba@nvidia.com>
Acked-by: Ori Kam <orika@nvidia.com>
---
 app/test-pmd/cmdline_flow.c                 |  17 ++-
 app/test-pmd/config.c                       | 159 +++++++++++++++++++-
 app/test-pmd/testpmd.h                      |   1 +
 doc/guides/testpmd_app_ug/testpmd_funcs.rst |  86 ++++++++++-
 lib/ethdev/rte_flow.c                       |  22 +++
 lib/ethdev/rte_flow.h                       |  49 +++++-
 lib/ethdev/rte_flow_driver.h                |   7 +
 lib/ethdev/version.map                      |   1 +
 8 files changed, 332 insertions(+), 10 deletions(-)

diff --git a/app/test-pmd/cmdline_flow.c b/app/test-pmd/cmdline_flow.c
index 59829371d4..992aeb95b3 100644
--- a/app/test-pmd/cmdline_flow.c
+++ b/app/test-pmd/cmdline_flow.c
@@ -129,6 +129,7 @@ enum index {
 	/* Queue arguments. */
 	QUEUE_CREATE,
 	QUEUE_DESTROY,
+	QUEUE_AGED,
 	QUEUE_INDIRECT_ACTION,
 
 	/* Queue create arguments. */
@@ -1170,6 +1171,7 @@ static const enum index next_table_destroy_attr[] = {
 static const enum index next_queue_subcmd[] = {
 	QUEUE_CREATE,
 	QUEUE_DESTROY,
+	QUEUE_AGED,
 	QUEUE_INDIRECT_ACTION,
 	ZERO,
 };
@@ -2967,6 +2969,13 @@ static const struct token token_list[] = {
 		.args = ARGS(ARGS_ENTRY(struct buffer, queue)),
 		.call = parse_qo_destroy,
 	},
+	[QUEUE_AGED] = {
+		.name = "aged",
+		.help = "list and destroy aged flows",
+		.next = NEXT(next_aged_attr, NEXT_ENTRY(COMMON_QUEUE_ID)),
+		.args = ARGS(ARGS_ENTRY(struct buffer, queue)),
+		.call = parse_aged,
+	},
 	[QUEUE_INDIRECT_ACTION] = {
 		.name = "indirect_action",
 		.help = "queue indirect actions",
@@ -8654,8 +8663,8 @@ parse_aged(struct context *ctx, const struct token *token,
 	/* Nothing else to do if there is no buffer. */
 	if (!out)
 		return len;
-	if (!out->command) {
-		if (ctx->curr != AGED)
+	if (!out->command || out->command == QUEUE) {
+		if (ctx->curr != AGED && ctx->curr != QUEUE_AGED)
 			return -1;
 		if (sizeof(*out) > size)
 			return -1;
@@ -10610,6 +10619,10 @@ cmd_flow_parsed(const struct buffer *in)
 	case PULL:
 		port_queue_flow_pull(in->port, in->queue);
 		break;
+	case QUEUE_AGED:
+		port_queue_flow_aged(in->port, in->queue,
+				     in->args.aged.destroy);
+		break;
 	case QUEUE_INDIRECT_ACTION_CREATE:
 		port_queue_action_handle_create(
 				in->port, in->queue, in->postpone,
diff --git a/app/test-pmd/config.c b/app/test-pmd/config.c
index 0f7dbd698f..18f3543887 100644
--- a/app/test-pmd/config.c
+++ b/app/test-pmd/config.c
@@ -2509,6 +2509,7 @@ port_queue_flow_create(portid_t port_id, queueid_t queue_id,
 		       const struct rte_flow_action *actions)
 {
 	struct rte_flow_op_attr op_attr = { .postpone = postpone };
+	struct rte_flow_attr flow_attr = { 0 };
 	struct rte_flow *flow;
 	struct rte_port *port;
 	struct port_flow *pf;
@@ -2568,7 +2569,7 @@ port_queue_flow_create(portid_t port_id, queueid_t queue_id,
 	}
 	job->type = QUEUE_JOB_TYPE_FLOW_CREATE;
 
-	pf = port_flow_new(NULL, pattern, actions, &error);
+	pf = port_flow_new(&flow_attr, pattern, actions, &error);
 	if (!pf) {
 		free(job);
 		return port_flow_complain(&error);
@@ -2905,6 +2906,162 @@ port_queue_flow_push(portid_t port_id, queueid_t queue_id)
 	return ret;
 }
 
+/** Pull queue operation results from the queue. */
+static int
+port_queue_aged_flow_destroy(portid_t port_id, queueid_t queue_id,
+			     const uint32_t *rule, int nb_flows)
+{
+	struct rte_port *port = &ports[port_id];
+	struct rte_flow_op_result *res;
+	struct rte_flow_error error;
+	uint32_t n = nb_flows;
+	int ret = 0;
+	int i;
+
+	res = calloc(port->queue_sz, sizeof(struct rte_flow_op_result));
+	if (!res) {
+		printf("Failed to allocate memory for pulled results\n");
+		return -ENOMEM;
+	}
+
+	memset(&error, 0x66, sizeof(error));
+	while (nb_flows > 0) {
+		int success = 0;
+
+		if (n > port->queue_sz)
+			n = port->queue_sz;
+		ret = port_queue_flow_destroy(port_id, queue_id, true, n, rule);
+		if (ret < 0) {
+			free(res);
+			return ret;
+		}
+		ret = rte_flow_push(port_id, queue_id, &error);
+		if (ret < 0) {
+			printf("Failed to push operations in the queue: %s\n",
+			       strerror(-ret));
+			free(res);
+			return ret;
+		}
+		while (success < nb_flows) {
+			ret = rte_flow_pull(port_id, queue_id, res,
+					    port->queue_sz, &error);
+			if (ret < 0) {
+				printf("Failed to pull a operation results: %s\n",
+				       strerror(-ret));
+				free(res);
+				return ret;
+			}
+
+			for (i = 0; i < ret; i++) {
+				if (res[i].status == RTE_FLOW_OP_SUCCESS)
+					success++;
+			}
+		}
+		rule += n;
+		nb_flows -= n;
+		n = nb_flows;
+	}
+
+	free(res);
+	return ret;
+}
+
+/** List simply and destroy all aged flows per queue. */
+void
+port_queue_flow_aged(portid_t port_id, uint32_t queue_id, uint8_t destroy)
+{
+	void **contexts;
+	int nb_context, total = 0, idx;
+	uint32_t *rules = NULL;
+	struct rte_port *port;
+	struct rte_flow_error error;
+	enum age_action_context_type *type;
+	union {
+		struct port_flow *pf;
+		struct port_indirect_action *pia;
+	} ctx;
+
+	if (port_id_is_invalid(port_id, ENABLED_WARN) ||
+	    port_id == (portid_t)RTE_PORT_ALL)
+		return;
+	port = &ports[port_id];
+	if (queue_id >= port->queue_nb) {
+		printf("Error: queue #%u is invalid\n", queue_id);
+		return;
+	}
+	total = rte_flow_get_q_aged_flows(port_id, queue_id, NULL, 0, &error);
+	if (total < 0) {
+		port_flow_complain(&error);
+		return;
+	}
+	printf("Port %u queue %u total aged flows: %d\n",
+	       port_id, queue_id, total);
+	if (total == 0)
+		return;
+	contexts = calloc(total, sizeof(void *));
+	if (contexts == NULL) {
+		printf("Cannot allocate contexts for aged flow\n");
+		return;
+	}
+	printf("%-20s\tID\tGroup\tPrio\tAttr\n", "Type");
+	nb_context = rte_flow_get_q_aged_flows(port_id, queue_id, contexts,
+					       total, &error);
+	if (nb_context > total) {
+		printf("Port %u queue %u get aged flows count(%d) > total(%d)\n",
+		       port_id, queue_id, nb_context, total);
+		free(contexts);
+		return;
+	}
+	if (destroy) {
+		rules = malloc(sizeof(uint32_t) * nb_context);
+		if (rules == NULL)
+			printf("Cannot allocate memory for destroy aged flow\n");
+	}
+	total = 0;
+	for (idx = 0; idx < nb_context; idx++) {
+		if (!contexts[idx]) {
+			printf("Error: get Null context in port %u queue %u\n",
+			       port_id, queue_id);
+			continue;
+		}
+		type = (enum age_action_context_type *)contexts[idx];
+		switch (*type) {
+		case ACTION_AGE_CONTEXT_TYPE_FLOW:
+			ctx.pf = container_of(type, struct port_flow, age_type);
+			printf("%-20s\t%" PRIu32 "\t%" PRIu32 "\t%" PRIu32
+								 "\t%c%c%c\t\n",
+			       "Flow",
+			       ctx.pf->id,
+			       ctx.pf->rule.attr->group,
+			       ctx.pf->rule.attr->priority,
+			       ctx.pf->rule.attr->ingress ? 'i' : '-',
+			       ctx.pf->rule.attr->egress ? 'e' : '-',
+			       ctx.pf->rule.attr->transfer ? 't' : '-');
+			if (rules != NULL) {
+				rules[total] = ctx.pf->id;
+				total++;
+			}
+			break;
+		case ACTION_AGE_CONTEXT_TYPE_INDIRECT_ACTION:
+			ctx.pia = container_of(type,
+					       struct port_indirect_action,
+					       age_type);
+			printf("%-20s\t%" PRIu32 "\n", "Indirect action",
+			       ctx.pia->id);
+			break;
+		default:
+			printf("Error: invalid context type %u\n", port_id);
+			break;
+		}
+	}
+	if (rules != NULL) {
+		port_queue_aged_flow_destroy(port_id, queue_id, rules, total);
+		free(rules);
+	}
+	printf("\n%d flows destroyed\n", total);
+	free(contexts);
+}
+
 /** Pull queue operation results from the queue. */
 int
 port_queue_flow_pull(portid_t port_id, queueid_t queue_id)
diff --git a/app/test-pmd/testpmd.h b/app/test-pmd/testpmd.h
index acdb7e855d..918c2377d8 100644
--- a/app/test-pmd/testpmd.h
+++ b/app/test-pmd/testpmd.h
@@ -941,6 +941,7 @@ int port_queue_action_handle_query(portid_t port_id, uint32_t queue_id,
 				   bool postpone, uint32_t id);
 int port_queue_flow_push(portid_t port_id, queueid_t queue_id);
 int port_queue_flow_pull(portid_t port_id, queueid_t queue_id);
+void port_queue_flow_aged(portid_t port_id, uint32_t queue_id, uint8_t destroy);
 int port_flow_validate(portid_t port_id,
 		       const struct rte_flow_attr *attr,
 		       const struct rte_flow_item *pattern,
diff --git a/doc/guides/testpmd_app_ug/testpmd_funcs.rst b/doc/guides/testpmd_app_ug/testpmd_funcs.rst
index a8b99c8c19..8e21b2a5b7 100644
--- a/doc/guides/testpmd_app_ug/testpmd_funcs.rst
+++ b/doc/guides/testpmd_app_ug/testpmd_funcs.rst
@@ -2894,9 +2894,10 @@ following sections.
        [meters_number {number}] [flags {number}]
 
 - Create a pattern template::
+
    flow pattern_template {port_id} create [pattern_template_id {id}]
        [relaxed {boolean}] [ingress] [egress] [transfer]
-	   template {item} [/ {item} [...]] / end
+       template {item} [/ {item} [...]] / end
 
 - Destroy a pattern template::
 
@@ -2995,6 +2996,10 @@ following sections.
 
    flow aged {port_id} [destroy]
 
+- Enqueue list and destroy aged flow rules::
+
+   flow queue {port_id} aged {queue_id} [destroy]
+
 - Tunnel offload - create a tunnel stub::
 
    flow tunnel create {port_id} type {tunnel_type}
@@ -4236,7 +4241,7 @@ Disabling isolated mode::
  testpmd>
 
 Dumping HW internal information
-~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
 
 ``flow dump`` dumps the hardware's internal representation information of
 all flows. It is bound to ``rte_flow_dev_dump()``::
@@ -4252,10 +4257,10 @@ 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.
+and ``destroy`` parameter can be used to destroy those flow rules in PMD::
 
    flow aged {port_id} [destroy]
 
@@ -4290,7 +4295,7 @@ will be ID 3, ID 1, ID 0::
    1       0       0       i--
    0       0       0       i--
 
-If attach ``destroy`` parameter, the command will destroy all the list aged flow rules.
+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
@@ -4308,6 +4313,77 @@ If attach ``destroy`` parameter, the command will destroy all the list aged flow
    testpmd> flow aged 0
    Port 0 total aged flows: 0
 
+
+Enqueueing listing and destroying aged flow rules
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+
+``flow queue aged`` simply lists aged flow rules be get from
+``rte_flow_get_q_aged_flows`` API, and ``destroy`` parameter can be used to
+destroy those flow rules in PMD::
+
+   flow queue {port_id} aged {queue_id} [destroy]
+
+Listing current aged flow rules::
+
+   testpmd> flow queue 0 aged 0
+   Port 0 queue 0 total aged flows: 0
+   testpmd> flow queue 0 create 0 ingress tanle 0 item_template 0 action_template 0
+      pattern eth / ipv4 src is 2.2.2.14 / end
+      actions age timeout 5 / queue index 0 /  end
+   Flow rule #0 creation enqueued
+   testpmd> flow queue 0 create 0 ingress tanle 0 item_template 0 action_template 0
+      pattern eth / ipv4 src is 2.2.2.15 / end
+      actions age timeout 4 / queue index 0 /  end
+   Flow rule #1 creation enqueued
+   testpmd> flow queue 0 create 0 ingress tanle 0 item_template 0 action_template 0
+      pattern eth / ipv4 src is 2.2.2.16 / end
+      actions age timeout 4 / queue index 0 /  end
+   Flow rule #2 creation enqueued
+   testpmd> flow queue 0 create 0 ingress tanle 0 item_template 0 action_template 0
+      pattern eth / ipv4 src is 2.2.2.17 / end
+      actions age timeout 4 / queue index 0 /  end
+   Flow rule #3 creation enqueued
+   testpmd> flow pull 0 queue 0
+   Queue #0 pulled 4 operations (0 failed, 4 succeeded)
+
+Aged Rules are simply list as command ``flow queue {port_id} list {queue_id}``,
+but strip the detail rule information, all the aged flows are sorted by the
+longest timeout time. For example, if those rules is 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 queue 0 aged 0
+   Port 0 queue 0 total aged flows: 4
+   ID      Group   Prio    Attr
+   2       0       0       ---
+   3       0       0       ---
+   1       0       0       ---
+   0       0       0       ---
+
+   0 flows destroyed
+
+If attach ``destroy`` parameter, the command will destroy all the list aged flow rules::
+
+   testpmd> flow queue 0 aged 0 destroy
+   Port 0 queue 0 total aged flows: 4
+   ID      Group   Prio    Attr
+   2       0       0       ---
+   3       0       0       ---
+   1       0       0       ---
+   0       0       0       ---
+   Flow rule #2 destruction enqueued
+   Flow rule #3 destruction enqueued
+   Flow rule #1 destruction enqueued
+   Flow rule #0 destruction enqueued
+
+   4 flows destroyed
+   testpmd> flow queue 0 aged 0
+   Port 0 total aged flows: 0
+
+.. note::
+
+   The queue must be empty before attaching ``destroy`` parameter.
+
+
 Creating indirect actions
 ~~~~~~~~~~~~~~~~~~~~~~~~~
 
diff --git a/lib/ethdev/rte_flow.c b/lib/ethdev/rte_flow.c
index d11ba270db..7d0c24366c 100644
--- a/lib/ethdev/rte_flow.c
+++ b/lib/ethdev/rte_flow.c
@@ -1132,6 +1132,28 @@ rte_flow_get_aged_flows(uint16_t port_id, void **contexts,
 				  NULL, rte_strerror(ENOTSUP));
 }
 
+int
+rte_flow_get_q_aged_flows(uint16_t port_id, uint32_t queue_id, void **contexts,
+			  uint32_t nb_contexts, struct rte_flow_error *error)
+{
+	struct rte_eth_dev *dev = &rte_eth_devices[port_id];
+	const struct rte_flow_ops *ops = rte_flow_ops_get(port_id, error);
+	int ret;
+
+	if (unlikely(!ops))
+		return -rte_errno;
+	if (likely(!!ops->get_q_aged_flows)) {
+		fts_enter(dev);
+		ret = ops->get_q_aged_flows(dev, queue_id, contexts,
+					    nb_contexts, error);
+		fts_exit(dev);
+		return flow_err(port_id, ret, error);
+	}
+	return rte_flow_error_set(error, ENOTSUP,
+				  RTE_FLOW_ERROR_TYPE_UNSPECIFIED,
+				  NULL, rte_strerror(ENOTSUP));
+}
+
 struct rte_flow_action_handle *
 rte_flow_action_handle_create(uint16_t port_id,
 			      const struct rte_flow_indir_action_conf *conf,
diff --git a/lib/ethdev/rte_flow.h b/lib/ethdev/rte_flow.h
index a93ec796cb..64ec8f0903 100644
--- a/lib/ethdev/rte_flow.h
+++ b/lib/ethdev/rte_flow.h
@@ -2639,6 +2639,7 @@ enum rte_flow_action_type {
 	 * flow.
 	 *
 	 * See struct rte_flow_action_age.
+	 * See function rte_flow_get_q_aged_flows
 	 * See function rte_flow_get_aged_flows
 	 * see enum RTE_ETH_EVENT_FLOW_AGED
 	 * See struct rte_flow_query_age
@@ -2784,8 +2785,8 @@ struct rte_flow_action_queue {
  * on the flow. RTE_ETH_EVENT_FLOW_AGED event is triggered when a
  * port detects new aged-out flows.
  *
- * The flow context and the flow handle will be reported by the
- * rte_flow_get_aged_flows API.
+ * The flow context and the flow handle will be reported by the either
+ * rte_flow_get_aged_flows or rte_flow_get_q_aged_flows APIs.
  */
 struct rte_flow_action_age {
 	uint32_t timeout:24; /**< Time in seconds. */
@@ -4314,6 +4315,50 @@ int
 rte_flow_get_aged_flows(uint16_t port_id, void **contexts,
 			uint32_t nb_contexts, struct rte_flow_error *error);
 
+/**
+ * @warning
+ * @b EXPERIMENTAL: this API may change without prior notice.
+ *
+ * Get aged-out flows of a given port on the given flow queue.
+ *
+ * If application configure port attribute with RTE_FLOW_PORT_FLAG_STRICT_QUEUE,
+ * there is no RTE_ETH_EVENT_FLOW_AGED event and this function must be called to
+ * get the aged flows synchronously.
+ *
+ * If application configure port attribute without
+ * RTE_FLOW_PORT_FLAG_STRICT_QUEUE, RTE_ETH_EVENT_FLOW_AGED event will be
+ * triggered at least one new aged out flow was detected on any flow queue after
+ * the last call to rte_flow_get_q_aged_flows.
+ * In addition, the @p queue_id will be ignored.
+ * This function can be called to get the aged flows asynchronously from the
+ * event callback or synchronously regardless the event.
+ *
+ * @param[in] port_id
+ *   Port identifier of Ethernet device.
+ * @param[in] queue_id
+ *   Flow queue to query. Ignored when RTE_FLOW_PORT_FLAG_STRICT_QUEUE not set.
+ * @param[in, out] contexts
+ *   The address of an array of pointers to the aged-out flows contexts.
+ * @param[in] nb_contexts
+ *   The length of context array pointers.
+ * @param[out] error
+ *   Perform verbose error reporting if not NULL. Initialized in case of
+ *   error only.
+ *
+ * @return
+ *   if nb_contexts is 0, return the amount of all aged contexts.
+ *   if nb_contexts is not 0 , return the amount of aged flows reported
+ *   in the context array, otherwise negative errno value.
+ *
+ * @see rte_flow_action_age
+ * @see RTE_ETH_EVENT_FLOW_AGED
+ * @see rte_flow_port_flag
+ */
+__rte_experimental
+int
+rte_flow_get_q_aged_flows(uint16_t port_id, uint32_t queue_id, void **contexts,
+			  uint32_t nb_contexts, struct rte_flow_error *error);
+
 /**
  * Specify indirect action object configuration
  */
diff --git a/lib/ethdev/rte_flow_driver.h b/lib/ethdev/rte_flow_driver.h
index 7289deb538..c7d0699c91 100644
--- a/lib/ethdev/rte_flow_driver.h
+++ b/lib/ethdev/rte_flow_driver.h
@@ -84,6 +84,13 @@ struct rte_flow_ops {
 		 void **context,
 		 uint32_t nb_contexts,
 		 struct rte_flow_error *err);
+	/** See rte_flow_get_q_aged_flows() */
+	int (*get_q_aged_flows)
+		(struct rte_eth_dev *dev,
+		 uint32_t queue_id,
+		 void **contexts,
+		 uint32_t nb_contexts,
+		 struct rte_flow_error *error);
 	/** See rte_flow_action_handle_create() */
 	struct rte_flow_action_handle *(*action_handle_create)
 		(struct rte_eth_dev *dev,
diff --git a/lib/ethdev/version.map b/lib/ethdev/version.map
index e749678b96..17201fbe0f 100644
--- a/lib/ethdev/version.map
+++ b/lib/ethdev/version.map
@@ -295,6 +295,7 @@ EXPERIMENTAL {
 	rte_eth_rx_descriptor_dump;
 	rte_eth_tx_descriptor_dump;
 	rte_flow_async_action_handle_query;
+	rte_flow_get_q_aged_flows;
 	rte_mtr_meter_policy_get;
 	rte_mtr_meter_profile_get;
 };
-- 
2.25.1


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

* [PATCH v3 3/3] ethdev: add structure for indirect AGE update
  2022-10-19 14:49   ` [PATCH v3 0/3] ethdev: AGE action preparation Michael Baum
  2022-10-19 14:49     ` [PATCH v3 1/3] ethdev: add strict queue to pre-configuration flow hints Michael Baum
  2022-10-19 14:49     ` [PATCH v3 2/3] ethdev: add queue-based API to report aged flow rules Michael Baum
@ 2022-10-19 14:49     ` Michael Baum
  2022-10-26 19:18       ` Andrew Rybchenko
  2022-10-26 21:49     ` [PATCH v4 0/3] ethdev: AGE action preparation Michael Baum
  3 siblings, 1 reply; 32+ messages in thread
From: Michael Baum @ 2022-10-19 14:49 UTC (permalink / raw)
  To: dev; +Cc: Matan Azrad, Raslan Darawsheh, Ori Kam

Add a new structure for indirect AGE update.

This new structure enables:
1. Update timeout value.
2. Stop AGE checking.
3. Start AGE checking.
4. restart AGE checking.

Signed-off-by: Michael Baum <michaelba@nvidia.com>
Acked-by: Ori Kam <orika@nvidia.com>
---
 app/test-pmd/cmdline_flow.c        | 66 ++++++++++++++++++++++++++++++
 app/test-pmd/config.c              | 19 ++++++---
 doc/guides/prog_guide/rte_flow.rst | 25 +++++++++--
 lib/ethdev/rte_flow.h              | 28 +++++++++++++
 4 files changed, 129 insertions(+), 9 deletions(-)

diff --git a/app/test-pmd/cmdline_flow.c b/app/test-pmd/cmdline_flow.c
index 992aeb95b3..88108498e0 100644
--- a/app/test-pmd/cmdline_flow.c
+++ b/app/test-pmd/cmdline_flow.c
@@ -586,6 +586,9 @@ enum index {
 	ACTION_SET_IPV6_DSCP_VALUE,
 	ACTION_AGE,
 	ACTION_AGE_TIMEOUT,
+	ACTION_AGE_UPDATE,
+	ACTION_AGE_UPDATE_TIMEOUT,
+	ACTION_AGE_UPDATE_TOUCH,
 	ACTION_SAMPLE,
 	ACTION_SAMPLE_RATIO,
 	ACTION_SAMPLE_INDEX,
@@ -1874,6 +1877,7 @@ static const enum index next_action[] = {
 	ACTION_SET_IPV4_DSCP,
 	ACTION_SET_IPV6_DSCP,
 	ACTION_AGE,
+	ACTION_AGE_UPDATE,
 	ACTION_SAMPLE,
 	ACTION_INDIRECT,
 	ACTION_MODIFY_FIELD,
@@ -2110,6 +2114,14 @@ static const enum index action_age[] = {
 	ZERO,
 };
 
+static const enum index action_age_update[] = {
+	ACTION_AGE_UPDATE,
+	ACTION_AGE_UPDATE_TIMEOUT,
+	ACTION_AGE_UPDATE_TOUCH,
+	ACTION_NEXT,
+	ZERO,
+};
+
 static const enum index action_sample[] = {
 	ACTION_SAMPLE,
 	ACTION_SAMPLE_RATIO,
@@ -2188,6 +2200,9 @@ static int parse_vc_spec(struct context *, const struct token *,
 			 const char *, unsigned int, void *, unsigned int);
 static int parse_vc_conf(struct context *, const struct token *,
 			 const char *, unsigned int, void *, unsigned int);
+static int parse_vc_conf_timeout(struct context *, const struct token *,
+				 const char *, unsigned int, void *,
+				 unsigned int);
 static int parse_vc_item_ecpri_type(struct context *, const struct token *,
 				    const char *, unsigned int,
 				    void *, unsigned int);
@@ -6206,6 +6221,30 @@ static const struct token token_list[] = {
 		.next = NEXT(action_age, NEXT_ENTRY(COMMON_UNSIGNED)),
 		.call = parse_vc_conf,
 	},
+	[ACTION_AGE_UPDATE] = {
+		.name = "age_update",
+		.help = "update aging parameter",
+		.next = NEXT(action_age_update),
+		.priv = PRIV_ACTION(AGE,
+				    sizeof(struct rte_flow_update_age)),
+		.call = parse_vc,
+	},
+	[ACTION_AGE_UPDATE_TIMEOUT] = {
+		.name = "timeout",
+		.help = "age timeout update value",
+		.args = ARGS(ARGS_ENTRY_BF(struct rte_flow_update_age,
+					   timeout, 24)),
+		.next = NEXT(action_age_update, NEXT_ENTRY(COMMON_UNSIGNED)),
+		.call = parse_vc_conf_timeout,
+	},
+	[ACTION_AGE_UPDATE_TOUCH] = {
+		.name = "touch",
+		.help = "this flow is touched",
+		.next = NEXT(action_age_update, NEXT_ENTRY(COMMON_BOOLEAN)),
+		.args = ARGS(ARGS_ENTRY_BF(struct rte_flow_update_age,
+					   touch, 1)),
+		.call = parse_vc_conf,
+	},
 	[ACTION_SAMPLE] = {
 		.name = "sample",
 		.help = "set a sample action",
@@ -7045,6 +7084,33 @@ parse_vc_conf(struct context *ctx, const struct token *token,
 	return len;
 }
 
+/** Parse action configuration field. */
+static int
+parse_vc_conf_timeout(struct context *ctx, const struct token *token,
+		      const char *str, unsigned int len,
+		      void *buf, unsigned int size)
+{
+	struct buffer *out = buf;
+	struct rte_flow_update_age *update;
+
+	(void)size;
+	if (ctx->curr != ACTION_AGE_UPDATE_TIMEOUT)
+		return -1;
+	/* 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;
+	/* Point to selected object. */
+	ctx->object = out->args.vc.data;
+	ctx->objmask = NULL;
+	/* Update the timeout is valid. */
+	update = (struct rte_flow_update_age *)out->args.vc.data;
+	update->timeout_valid = 1;
+	return len;
+}
+
 /** Parse eCPRI common header type field. */
 static int
 parse_vc_item_ecpri_type(struct context *ctx, const struct token *token,
diff --git a/app/test-pmd/config.c b/app/test-pmd/config.c
index 18f3543887..e8a1b77c2a 100644
--- a/app/test-pmd/config.c
+++ b/app/test-pmd/config.c
@@ -1886,6 +1886,7 @@ port_action_handle_update(portid_t port_id, uint32_t id,
 	if (!pia)
 		return -EINVAL;
 	switch (pia->type) {
+	case RTE_FLOW_ACTION_TYPE_AGE:
 	case RTE_FLOW_ACTION_TYPE_CONNTRACK:
 		update = action->conf;
 		break;
@@ -2816,17 +2817,23 @@ port_queue_action_handle_update(portid_t port_id,
 		return -EINVAL;
 	}
 
-	if (pia->type == RTE_FLOW_ACTION_TYPE_METER_MARK) {
+	switch (pia->type) {
+	case RTE_FLOW_ACTION_TYPE_AGE:
+		update = action->conf;
+		break;
+	case RTE_FLOW_ACTION_TYPE_METER_MARK:
 		rte_memcpy(&mtr_update.meter_mark, action->conf,
 			sizeof(struct rte_flow_action_meter_mark));
 		mtr_update.profile_valid = 1;
-		mtr_update.policy_valid  = 1;
-		mtr_update.color_mode_valid  = 1;
-		mtr_update.init_color_valid  = 1;
-		mtr_update.state_valid  = 1;
+		mtr_update.policy_valid = 1;
+		mtr_update.color_mode_valid = 1;
+		mtr_update.init_color_valid = 1;
+		mtr_update.state_valid = 1;
 		update = &mtr_update;
-	} else {
+		break;
+	default:
 		update = action;
+		break;
 	}
 
 	if (rte_flow_async_action_handle_update(port_id, queue_id, &attr,
diff --git a/doc/guides/prog_guide/rte_flow.rst b/doc/guides/prog_guide/rte_flow.rst
index 565868aeea..1ce0277e65 100644
--- a/doc/guides/prog_guide/rte_flow.rst
+++ b/doc/guides/prog_guide/rte_flow.rst
@@ -2737,7 +2737,7 @@ Otherwise, RTE_FLOW_ERROR_TYPE_ACTION error will be returned.
 Action: ``AGE``
 ^^^^^^^^^^^^^^^
 
-Set ageing timeout configuration to a flow.
+Set aging timeout configuration to a flow.
 
 Event RTE_ETH_EVENT_FLOW_AGED will be reported if
 timeout passed without any matching on the flow.
@@ -2756,8 +2756,8 @@ timeout passed without any matching on the flow.
    | ``context``  | user input flow context         |
    +--------------+---------------------------------+
 
-Query structure to retrieve ageing status information of a
-shared AGE action, or a flow rule using the AGE action:
+Query structure to retrieve aging status information of an
+indirect AGE action, or a flow rule using the AGE action:
 
 .. _table_rte_flow_query_age:
 
@@ -2773,6 +2773,25 @@ shared AGE action, or a flow rule using the AGE action:
    | ``sec_since_last_hit``       | out | Seconds since last traffic hit         |
    +------------------------------+-----+----------------------------------------+
 
+Update structure to modify the parameters of an indirect AGE action.
+The update structure is used by ``rte_flow_action_handle_update()`` function.
+
+.. _table_rte_flow_update_age:
+
+.. table:: AGE update
+
+   +-------------------+--------------------------------------------------------------+
+   | Field             | Value                                                        |
+   +===================+==============================================================+
+   | ``reserved``      | 6 bits reserved, must be zero                                |
+   +-------------------+--------------------------------------------------------------+
+   | ``timeout_valid`` | 1 bit, timeout value is valid                                |
+   +-------------------+--------------------------------------------------------------+
+   | ``timeout``       | 24 bits timeout value                                        |
+   +-------------------+--------------------------------------------------------------+
+   | ``touch``         | 1 bit, touch the AGE action to set ``sec_since_last_hit`` 0  |
+   +-------------------+--------------------------------------------------------------+
+
 Action: ``SAMPLE``
 ^^^^^^^^^^^^^^^^^^
 
diff --git a/lib/ethdev/rte_flow.h b/lib/ethdev/rte_flow.h
index 64ec8f0903..8858b56428 100644
--- a/lib/ethdev/rte_flow.h
+++ b/lib/ethdev/rte_flow.h
@@ -2643,6 +2643,7 @@ enum rte_flow_action_type {
 	 * See function rte_flow_get_aged_flows
 	 * see enum RTE_ETH_EVENT_FLOW_AGED
 	 * See struct rte_flow_query_age
+	 * See struct rte_flow_update_age
 	 */
 	RTE_FLOW_ACTION_TYPE_AGE,
 
@@ -2809,6 +2810,33 @@ struct rte_flow_query_age {
 	uint32_t sec_since_last_hit:24; /**< Seconds since last traffic hit. */
 };
 
+/**
+ * @warning
+ * @b EXPERIMENTAL: this structure may change without prior notice
+ *
+ * RTE_FLOW_ACTION_TYPE_AGE
+ *
+ * Update indirect AGE action attributes:
+ *  - Timeout can be updated including stop/start action:
+ *     +-------------+-------------+------------------------------+
+ *     | Old Timeout | New Timeout | Updating                     |
+ *     +=============+=============+==============================+
+ *     | 0           | positive    | Start aging with new value   |
+ *     +-------------+-------------+------------------------------+
+ *     | positive    | 0           | Stop aging			  |
+ *     +-------------+-------------+------------------------------+
+ *     | positive    | positive    | Change timeout to new value  |
+ *     +-------------+-------------+------------------------------+
+ *  - sec_since_last_hit can be reset.
+ */
+struct rte_flow_update_age {
+	uint32_t reserved:6; /**< Reserved, must be zero. */
+	uint32_t timeout_valid:1; /**< The timeout is valid for update. */
+	uint32_t timeout:24; /**< Time in seconds. */
+	/** Means that aging should assume packet passed the aging. */
+	uint32_t touch:1;
+};
+
 /**
  * @warning
  * @b EXPERIMENTAL: this structure may change without prior notice
-- 
2.25.1


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

* Re: [PATCH v3 1/3] ethdev: add strict queue to pre-configuration flow hints
  2022-10-19 14:49     ` [PATCH v3 1/3] ethdev: add strict queue to pre-configuration flow hints Michael Baum
@ 2022-10-26 19:10       ` Andrew Rybchenko
  0 siblings, 0 replies; 32+ messages in thread
From: Andrew Rybchenko @ 2022-10-26 19:10 UTC (permalink / raw)
  To: Michael Baum, dev; +Cc: Matan Azrad, Raslan Darawsheh, Ori Kam

On 10/19/22 17:49, Michael Baum wrote:
> The data-path focused flow rule management can manage flow rules in more
> optimized way than traditional one by using hints provided by
> application in initialization phase.
> 
> In addition to the current hints we have in port attr, more hints could
> be provided by application about its behaviour.
> 
> One example is how the application do with the same flow rule ?
> A. create/destroy flow on same queue but query flow on different queue
>     or queue-less way (i.e, counter query)
> B. All flow operations will be exactly on the same queue, by which PMD
>     could be in more optimized way then A because resource could be
>     isolated and access based on queue, without lock, for example.
> 
> This patch add flag about above situation and could be extended to cover
> more situations.
> 
> Signed-off-by: Michael Baum <michaelba@nvidia.com>
> Acked-by: Ori Kam <orika@nvidia.com>

Acked-by: Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>



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

* Re: [PATCH v3 2/3] ethdev: add queue-based API to report aged flow rules
  2022-10-19 14:49     ` [PATCH v3 2/3] ethdev: add queue-based API to report aged flow rules Michael Baum
@ 2022-10-26 19:15       ` Andrew Rybchenko
  2022-10-26 21:17         ` Michael Baum
  0 siblings, 1 reply; 32+ messages in thread
From: Andrew Rybchenko @ 2022-10-26 19:15 UTC (permalink / raw)
  To: Michael Baum, dev; +Cc: Matan Azrad, Raslan Darawsheh, Ori Kam

On 10/19/22 17:49, Michael Baum wrote:
> When application use queue-based flow rule management and operate the
> same flow rule on the same queue, e.g create/destroy/query, API of
> querying aged flow rules should also have queue id parameter just like
> other queue-based flow APIs.
> 
> By this way, PMD can work in more optimized way since resources are
> isolated by queue and needn't synchronize.
> 
> If application do use queue-based flow management but configure port
> without RTE_FLOW_PORT_FLAG_STRICT_QUEUE, which means application operate
> a given flow rule on different queues, the queue id parameter will
> be ignored.
> 
> Signed-off-by: Michael Baum <michaelba@nvidia.com>
> Acked-by: Ori Kam <orika@nvidia.com>

Few minor notes below, other than that

Acked-by: Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>

[snip]

> diff --git a/doc/guides/testpmd_app_ug/testpmd_funcs.rst b/doc/guides/testpmd_app_ug/testpmd_funcs.rst
> index a8b99c8c19..8e21b2a5b7 100644
> --- a/doc/guides/testpmd_app_ug/testpmd_funcs.rst
> +++ b/doc/guides/testpmd_app_ug/testpmd_funcs.rst
> @@ -2894,9 +2894,10 @@ following sections.
>          [meters_number {number}] [flags {number}]
>   
>   - Create a pattern template::
> +

unrelated style fixes, should be in a separate patch

>      flow pattern_template {port_id} create [pattern_template_id {id}]
>          [relaxed {boolean}] [ingress] [egress] [transfer]
> -	   template {item} [/ {item} [...]] / end
> +       template {item} [/ {item} [...]] / end

unrelated style fixes

>   
>   - Destroy a pattern template::
>   
> @@ -2995,6 +2996,10 @@ following sections.
>   
>      flow aged {port_id} [destroy]
>   
> +- Enqueue list and destroy aged flow rules::
> +
> +   flow queue {port_id} aged {queue_id} [destroy]
> +
>   - Tunnel offload - create a tunnel stub::
>   
>      flow tunnel create {port_id} type {tunnel_type}
> @@ -4236,7 +4241,7 @@ Disabling isolated mode::
>    testpmd>
>   
>   Dumping HW internal information
> -~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

unrelated changes

>   
>   ``flow dump`` dumps the hardware's internal representation information of
>   all flows. It is bound to ``rte_flow_dev_dump()``::
> @@ -4252,10 +4257,10 @@ 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.
> +and ``destroy`` parameter can be used to destroy those flow rules in PMD::

unrelated style fixes

>   
>      flow aged {port_id} [destroy]
>   
> @@ -4290,7 +4295,7 @@ will be ID 3, ID 1, ID 0::
>      1       0       0       i--
>      0       0       0       i--
>   
> -If attach ``destroy`` parameter, the command will destroy all the list aged flow rules.
> +If attach ``destroy`` parameter, the command will destroy all the list aged flow rules::

unrelated style fixes

[snip]


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

* Re: [PATCH v3 3/3] ethdev: add structure for indirect AGE update
  2022-10-19 14:49     ` [PATCH v3 3/3] ethdev: add structure for indirect AGE update Michael Baum
@ 2022-10-26 19:18       ` Andrew Rybchenko
  2022-10-26 21:19         ` Michael Baum
  0 siblings, 1 reply; 32+ messages in thread
From: Andrew Rybchenko @ 2022-10-26 19:18 UTC (permalink / raw)
  To: Michael Baum, dev; +Cc: Matan Azrad, Raslan Darawsheh, Ori Kam

On 10/19/22 17:49, Michael Baum wrote:
> Add a new structure for indirect AGE update.
> 
> This new structure enables:
> 1. Update timeout value.
> 2. Stop AGE checking.
> 3. Start AGE checking.
> 4. restart AGE checking.
> 
> Signed-off-by: Michael Baum <michaelba@nvidia.com>
> Acked-by: Ori Kam <orika@nvidia.com>

Few minor notes below, other than that

Acked-by: Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>

> diff --git a/app/test-pmd/config.c b/app/test-pmd/config.c
> diff --git a/doc/guides/prog_guide/rte_flow.rst b/doc/guides/prog_guide/rte_flow.rst
> index 565868aeea..1ce0277e65 100644
> --- a/doc/guides/prog_guide/rte_flow.rst
> +++ b/doc/guides/prog_guide/rte_flow.rst
> @@ -2737,7 +2737,7 @@ Otherwise, RTE_FLOW_ERROR_TYPE_ACTION error will be returned.
>   Action: ``AGE``
>   ^^^^^^^^^^^^^^^
>   
> -Set ageing timeout configuration to a flow.
> +Set aging timeout configuration to a flow.

Unrelated fixes

>   
>   Event RTE_ETH_EVENT_FLOW_AGED will be reported if
>   timeout passed without any matching on the flow.
> @@ -2756,8 +2756,8 @@ timeout passed without any matching on the flow.
>      | ``context``  | user input flow context         |
>      +--------------+---------------------------------+
>   
> -Query structure to retrieve ageing status information of a
> -shared AGE action, or a flow rule using the AGE action:
> +Query structure to retrieve aging status information of an
> +indirect AGE action, or a flow rule using the AGE action:

Unrelated fixes


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

* RE: [PATCH v3 2/3] ethdev: add queue-based API to report aged flow rules
  2022-10-26 19:15       ` Andrew Rybchenko
@ 2022-10-26 21:17         ` Michael Baum
  0 siblings, 0 replies; 32+ messages in thread
From: Michael Baum @ 2022-10-26 21:17 UTC (permalink / raw)
  To: Andrew Rybchenko, dev; +Cc: Matan Azrad, Raslan Darawsheh, Ori Kam


On 10/26/22 22:16, Andrew Rybchenko wrote:
> 
> On 10/19/22 17:49, Michael Baum wrote:
> > When application use queue-based flow rule management and operate the
> > same flow rule on the same queue, e.g create/destroy/query, API of
> > querying aged flow rules should also have queue id parameter just like
> > other queue-based flow APIs.
> >
> > By this way, PMD can work in more optimized way since resources are
> > isolated by queue and needn't synchronize.
> >
> > If application do use queue-based flow management but configure port
> > without RTE_FLOW_PORT_FLAG_STRICT_QUEUE, which means application
> > operate a given flow rule on different queues, the queue id parameter
> > will be ignored.
> >
> > Signed-off-by: Michael Baum <michaelba@nvidia.com>
> > Acked-by: Ori Kam <orika@nvidia.com>
> 
> Few minor notes below, other than that
> 
> Acked-by: Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>
> 

Thank you, I'm sending a new version without unrelated fixes.
I'll send them later in a different patch set.

> [snip]
> 
> > diff --git a/doc/guides/testpmd_app_ug/testpmd_funcs.rst
> > b/doc/guides/testpmd_app_ug/testpmd_funcs.rst
> > index a8b99c8c19..8e21b2a5b7 100644
> > --- a/doc/guides/testpmd_app_ug/testpmd_funcs.rst
> > +++ b/doc/guides/testpmd_app_ug/testpmd_funcs.rst
> > @@ -2894,9 +2894,10 @@ following sections.
> >          [meters_number {number}] [flags {number}]
> >
> >   - Create a pattern template::
> > +
> 
> unrelated style fixes, should be in a separate patch
> 
> >      flow pattern_template {port_id} create [pattern_template_id {id}]
> >          [relaxed {boolean}] [ingress] [egress] [transfer]
> > -        template {item} [/ {item} [...]] / end
> > +       template {item} [/ {item} [...]] / end
> 
> unrelated style fixes
> 
> >
> >   - Destroy a pattern template::
> >
> > @@ -2995,6 +2996,10 @@ following sections.
> >
> >      flow aged {port_id} [destroy]
> >
> > +- Enqueue list and destroy aged flow rules::
> > +
> > +   flow queue {port_id} aged {queue_id} [destroy]
> > +
> >   - Tunnel offload - create a tunnel stub::
> >
> >      flow tunnel create {port_id} type {tunnel_type} @@ -4236,7
> > +4241,7 @@ Disabling isolated mode::
> >    testpmd>
> >
> >   Dumping HW internal information
> > -~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> > +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> 
> unrelated changes
> 
> >
> >   ``flow dump`` dumps the hardware's internal representation information of
> >   all flows. It is bound to ``rte_flow_dev_dump()``::
> > @@ -4252,10 +4257,10 @@ 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.
> > +and ``destroy`` parameter can be used to destroy those flow rules in PMD::
> 
> unrelated style fixes
> 
> >
> >      flow aged {port_id} [destroy]
> >
> > @@ -4290,7 +4295,7 @@ will be ID 3, ID 1, ID 0::
> >      1       0       0       i--
> >      0       0       0       i--
> >
> > -If attach ``destroy`` parameter, the command will destroy all the list aged
> flow rules.
> > +If attach ``destroy`` parameter, the command will destroy all the list aged
> flow rules::
> 
> unrelated style fixes
> 
> [snip]


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

* RE: [PATCH v3 3/3] ethdev: add structure for indirect AGE update
  2022-10-26 19:18       ` Andrew Rybchenko
@ 2022-10-26 21:19         ` Michael Baum
  0 siblings, 0 replies; 32+ messages in thread
From: Michael Baum @ 2022-10-26 21:19 UTC (permalink / raw)
  To: Andrew Rybchenko, dev; +Cc: Matan Azrad, Raslan Darawsheh, Ori Kam


On 10/26/22 22:18, Andrew Rybchenko wrote: 
> 
> On 10/19/22 17:49, Michael Baum wrote:
> > Add a new structure for indirect AGE update.
> >
> > This new structure enables:
> > 1. Update timeout value.
> > 2. Stop AGE checking.
> > 3. Start AGE checking.
> > 4. restart AGE checking.
> >
> > Signed-off-by: Michael Baum <michaelba@nvidia.com>
> > Acked-by: Ori Kam <orika@nvidia.com>
> 
> Few minor notes below, other than that
> 
> Acked-by: Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>
> 

Thank you, I'm sending a new version without unrelated fixes.
I'll send them later in a different patch set.

> > diff --git a/app/test-pmd/config.c b/app/test-pmd/config.c diff --git
> > a/doc/guides/prog_guide/rte_flow.rst
> > b/doc/guides/prog_guide/rte_flow.rst
> > index 565868aeea..1ce0277e65 100644
> > --- a/doc/guides/prog_guide/rte_flow.rst
> > +++ b/doc/guides/prog_guide/rte_flow.rst
> > @@ -2737,7 +2737,7 @@ Otherwise, RTE_FLOW_ERROR_TYPE_ACTION error
> will be returned.
> >   Action: ``AGE``
> >   ^^^^^^^^^^^^^^^
> >
> > -Set ageing timeout configuration to a flow.
> > +Set aging timeout configuration to a flow.
> 
> Unrelated fixes
> 
> >
> >   Event RTE_ETH_EVENT_FLOW_AGED will be reported if
> >   timeout passed without any matching on the flow.
> > @@ -2756,8 +2756,8 @@ timeout passed without any matching on the flow.
> >      | ``context``  | user input flow context         |
> >      +--------------+---------------------------------+
> >
> > -Query structure to retrieve ageing status information of a -shared
> > AGE action, or a flow rule using the AGE action:
> > +Query structure to retrieve aging status information of an indirect
> > +AGE action, or a flow rule using the AGE action:
> 
> Unrelated fixes


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

* [PATCH v4 0/3] ethdev: AGE action preparation
  2022-10-19 14:49   ` [PATCH v3 0/3] ethdev: AGE action preparation Michael Baum
                       ` (2 preceding siblings ...)
  2022-10-19 14:49     ` [PATCH v3 3/3] ethdev: add structure for indirect AGE update Michael Baum
@ 2022-10-26 21:49     ` Michael Baum
  2022-10-26 21:49       ` [PATCH v4 1/3] ethdev: add strict queue to pre-configuration flow hints Michael Baum
                         ` (3 more replies)
  3 siblings, 4 replies; 32+ messages in thread
From: Michael Baum @ 2022-10-26 21:49 UTC (permalink / raw)
  To: dev; +Cc: Matan Azrad, Raslan Darawsheh, Ori Kam

RFC's:
https://patchwork.dpdk.org/project/dpdk/patch/7a45693f478b1b721b4e05131141b526185a175c.1654063912.git.jackmin@nvidia.com/
https://patchwork.dpdk.org/project/dpdk/patch/608febf8d5d3c434a1eddb2e56f425ebbd6ff0b4.1654063912.git.jackmin@nvidia.com/

v2:
- rebase.
- Add reference to "rte_flow_update_age" structure in
  "RTE_FLOW_ACTION_TYPE_AGE" definition.
- Add reference to "rte_flow_get_q_aged_flows" function in
  "RTE_FLOW_ACTION_TYPE_AGE" definition.
- Change the order of "rte_flow_update_age" structure members in
  documentation, to be aligned with the structure definition.
- Place long comment before struct member definition.

v3:
- Fix miss "break" in indirect action update switch-case.

v4:
- Remove unrelated doc fixes.

Michael Baum (3):
  ethdev: add strict queue to pre-configuration flow hints
  ethdev: add queue-based API to report aged flow rules
  ethdev: add structure for indirect AGE update

 app/test-pmd/cmdline_flow.c                 |  93 +++++++++-
 app/test-pmd/config.c                       | 178 +++++++++++++++++++-
 app/test-pmd/testpmd.h                      |   1 +
 doc/guides/prog_guide/rte_flow.rst          |  19 +++
 doc/guides/testpmd_app_ug/testpmd_funcs.rst |  79 ++++++++-
 lib/ethdev/rte_flow.c                       |  22 +++
 lib/ethdev/rte_flow.h                       |  91 +++++++++-
 lib/ethdev/rte_flow_driver.h                |   7 +
 lib/ethdev/version.map                      |   1 +
 9 files changed, 478 insertions(+), 13 deletions(-)

-- 
2.25.1


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

* [PATCH v4 1/3] ethdev: add strict queue to pre-configuration flow hints
  2022-10-26 21:49     ` [PATCH v4 0/3] ethdev: AGE action preparation Michael Baum
@ 2022-10-26 21:49       ` Michael Baum
  2022-10-26 21:49       ` [PATCH v4 2/3] ethdev: add queue-based API to report aged flow rules Michael Baum
                         ` (2 subsequent siblings)
  3 siblings, 0 replies; 32+ messages in thread
From: Michael Baum @ 2022-10-26 21:49 UTC (permalink / raw)
  To: dev; +Cc: Matan Azrad, Raslan Darawsheh, Ori Kam, Andrew Rybchenko

The data-path focused flow rule management can manage flow rules in more
optimized way than traditional one by using hints provided by
application in initialization phase.

In addition to the current hints we have in port attr, more hints could
be provided by application about its behaviour.

One example is how the application do with the same flow rule ?
A. create/destroy flow on same queue but query flow on different queue
   or queue-less way (i.e, counter query)
B. All flow operations will be exactly on the same queue, by which PMD
   could be in more optimized way then A because resource could be
   isolated and access based on queue, without lock, for example.

This patch add flag about above situation and could be extended to cover
more situations.

Signed-off-by: Michael Baum <michaelba@nvidia.com>
Acked-by: Ori Kam <orika@nvidia.com>
Acked-by: Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>
---
 app/test-pmd/cmdline_flow.c                 | 10 ++++++++++
 doc/guides/testpmd_app_ug/testpmd_funcs.rst |  4 ++--
 lib/ethdev/rte_flow.h                       | 14 ++++++++++++++
 3 files changed, 26 insertions(+), 2 deletions(-)

diff --git a/app/test-pmd/cmdline_flow.c b/app/test-pmd/cmdline_flow.c
index 810dfb9854..59829371d4 100644
--- a/app/test-pmd/cmdline_flow.c
+++ b/app/test-pmd/cmdline_flow.c
@@ -226,6 +226,7 @@ enum index {
 	CONFIG_AGING_OBJECTS_NUMBER,
 	CONFIG_METERS_NUMBER,
 	CONFIG_CONN_TRACK_NUMBER,
+	CONFIG_FLAGS,
 
 	/* Indirect action arguments */
 	INDIRECT_ACTION_CREATE,
@@ -1092,6 +1093,7 @@ static const enum index next_config_attr[] = {
 	CONFIG_AGING_OBJECTS_NUMBER,
 	CONFIG_METERS_NUMBER,
 	CONFIG_CONN_TRACK_NUMBER,
+	CONFIG_FLAGS,
 	END,
 	ZERO,
 };
@@ -2692,6 +2694,14 @@ static const struct token token_list[] = {
 		.args = ARGS(ARGS_ENTRY(struct buffer,
 					args.configure.port_attr.nb_conn_tracks)),
 	},
+	[CONFIG_FLAGS] = {
+		.name = "flags",
+		.help = "configuration flags",
+		.next = NEXT(next_config_attr,
+			     NEXT_ENTRY(COMMON_UNSIGNED)),
+		.args = ARGS(ARGS_ENTRY(struct buffer,
+					args.configure.port_attr.flags)),
+	},
 	/* Top-level command. */
 	[PATTERN_TEMPLATE] = {
 		.name = "pattern_template",
diff --git a/doc/guides/testpmd_app_ug/testpmd_funcs.rst b/doc/guides/testpmd_app_ug/testpmd_funcs.rst
index d0fe73dff6..81e502b369 100644
--- a/doc/guides/testpmd_app_ug/testpmd_funcs.rst
+++ b/doc/guides/testpmd_app_ug/testpmd_funcs.rst
@@ -2891,7 +2891,7 @@ following sections.
        [queues_number {number}] [queues_size {size}]
        [counters_number {number}]
        [aging_counters_number {number}]
-       [meters_number {number}]
+       [meters_number {number}] [flags {number}]
 
 - Create a pattern template::
    flow pattern_template {port_id} create [pattern_template_id {id}]
@@ -3042,7 +3042,7 @@ for asynchronous flow creation/destruction operations. It is bound to
        [queues_number {number}] [queues_size {size}]
        [counters_number {number}]
        [aging_counters_number {number}]
-       [meters_number {number}]
+       [meters_number {number}] [flags {number}]
 
 If successful, it will show::
 
diff --git a/lib/ethdev/rte_flow.h b/lib/ethdev/rte_flow.h
index cddbe74c33..a93ec796cb 100644
--- a/lib/ethdev/rte_flow.h
+++ b/lib/ethdev/rte_flow.h
@@ -4741,6 +4741,12 @@ rte_flow_flex_item_release(uint16_t port_id,
 			   const struct rte_flow_item_flex_handle *handle,
 			   struct rte_flow_error *error);
 
+/**
+ * Indicate all operations for a given flow rule will _strictly_
+ * happen on the same queue (create/destroy/query/update).
+ */
+#define RTE_FLOW_PORT_FLAG_STRICT_QUEUE RTE_BIT32(0)
+
 /**
  * @warning
  * @b EXPERIMENTAL: this API may change without prior notice.
@@ -4774,6 +4780,10 @@ struct rte_flow_port_info {
 	 * @see RTE_FLOW_ACTION_TYPE_CONNTRACK
 	 */
 	uint32_t max_nb_conn_tracks;
+	/**
+	 * Port supported flags (RTE_FLOW_PORT_FLAG_*).
+	 */
+	uint32_t supported_flags;
 };
 
 /**
@@ -4848,6 +4858,10 @@ struct rte_flow_port_attr {
 	 * @see RTE_FLOW_ACTION_TYPE_CONNTRACK
 	 */
 	uint32_t nb_conn_tracks;
+	/**
+	 * Port flags (RTE_FLOW_PORT_FLAG_*).
+	 */
+	uint32_t flags;
 };
 
 /**
-- 
2.25.1


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

* [PATCH v4 2/3] ethdev: add queue-based API to report aged flow rules
  2022-10-26 21:49     ` [PATCH v4 0/3] ethdev: AGE action preparation Michael Baum
  2022-10-26 21:49       ` [PATCH v4 1/3] ethdev: add strict queue to pre-configuration flow hints Michael Baum
@ 2022-10-26 21:49       ` Michael Baum
  2022-10-26 21:49       ` [PATCH v4 3/3] ethdev: add structure for indirect AGE update Michael Baum
  2022-10-28 10:56       ` [PATCH v4 0/3] ethdev: AGE action preparation Andrew Rybchenko
  3 siblings, 0 replies; 32+ messages in thread
From: Michael Baum @ 2022-10-26 21:49 UTC (permalink / raw)
  To: dev; +Cc: Matan Azrad, Raslan Darawsheh, Ori Kam, Andrew Rybchenko

When application use queue-based flow rule management and operate the
same flow rule on the same queue, e.g create/destroy/query, API of
querying aged flow rules should also have queue id parameter just like
other queue-based flow APIs.

By this way, PMD can work in more optimized way since resources are
isolated by queue and needn't synchronize.

If application do use queue-based flow management but configure port
without RTE_FLOW_PORT_FLAG_STRICT_QUEUE, which means application operate
a given flow rule on different queues, the queue id parameter will
be ignored.

Signed-off-by: Michael Baum <michaelba@nvidia.com>
Acked-by: Ori Kam <orika@nvidia.com>
Acked-by: Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>
---
 app/test-pmd/cmdline_flow.c                 |  17 ++-
 app/test-pmd/config.c                       | 159 +++++++++++++++++++-
 app/test-pmd/testpmd.h                      |   1 +
 doc/guides/testpmd_app_ug/testpmd_funcs.rst |  75 +++++++++
 lib/ethdev/rte_flow.c                       |  22 +++
 lib/ethdev/rte_flow.h                       |  49 +++++-
 lib/ethdev/rte_flow_driver.h                |   7 +
 lib/ethdev/version.map                      |   1 +
 8 files changed, 326 insertions(+), 5 deletions(-)

diff --git a/app/test-pmd/cmdline_flow.c b/app/test-pmd/cmdline_flow.c
index 59829371d4..992aeb95b3 100644
--- a/app/test-pmd/cmdline_flow.c
+++ b/app/test-pmd/cmdline_flow.c
@@ -129,6 +129,7 @@ enum index {
 	/* Queue arguments. */
 	QUEUE_CREATE,
 	QUEUE_DESTROY,
+	QUEUE_AGED,
 	QUEUE_INDIRECT_ACTION,
 
 	/* Queue create arguments. */
@@ -1170,6 +1171,7 @@ static const enum index next_table_destroy_attr[] = {
 static const enum index next_queue_subcmd[] = {
 	QUEUE_CREATE,
 	QUEUE_DESTROY,
+	QUEUE_AGED,
 	QUEUE_INDIRECT_ACTION,
 	ZERO,
 };
@@ -2967,6 +2969,13 @@ static const struct token token_list[] = {
 		.args = ARGS(ARGS_ENTRY(struct buffer, queue)),
 		.call = parse_qo_destroy,
 	},
+	[QUEUE_AGED] = {
+		.name = "aged",
+		.help = "list and destroy aged flows",
+		.next = NEXT(next_aged_attr, NEXT_ENTRY(COMMON_QUEUE_ID)),
+		.args = ARGS(ARGS_ENTRY(struct buffer, queue)),
+		.call = parse_aged,
+	},
 	[QUEUE_INDIRECT_ACTION] = {
 		.name = "indirect_action",
 		.help = "queue indirect actions",
@@ -8654,8 +8663,8 @@ parse_aged(struct context *ctx, const struct token *token,
 	/* Nothing else to do if there is no buffer. */
 	if (!out)
 		return len;
-	if (!out->command) {
-		if (ctx->curr != AGED)
+	if (!out->command || out->command == QUEUE) {
+		if (ctx->curr != AGED && ctx->curr != QUEUE_AGED)
 			return -1;
 		if (sizeof(*out) > size)
 			return -1;
@@ -10610,6 +10619,10 @@ cmd_flow_parsed(const struct buffer *in)
 	case PULL:
 		port_queue_flow_pull(in->port, in->queue);
 		break;
+	case QUEUE_AGED:
+		port_queue_flow_aged(in->port, in->queue,
+				     in->args.aged.destroy);
+		break;
 	case QUEUE_INDIRECT_ACTION_CREATE:
 		port_queue_action_handle_create(
 				in->port, in->queue, in->postpone,
diff --git a/app/test-pmd/config.c b/app/test-pmd/config.c
index 0f7dbd698f..18f3543887 100644
--- a/app/test-pmd/config.c
+++ b/app/test-pmd/config.c
@@ -2509,6 +2509,7 @@ port_queue_flow_create(portid_t port_id, queueid_t queue_id,
 		       const struct rte_flow_action *actions)
 {
 	struct rte_flow_op_attr op_attr = { .postpone = postpone };
+	struct rte_flow_attr flow_attr = { 0 };
 	struct rte_flow *flow;
 	struct rte_port *port;
 	struct port_flow *pf;
@@ -2568,7 +2569,7 @@ port_queue_flow_create(portid_t port_id, queueid_t queue_id,
 	}
 	job->type = QUEUE_JOB_TYPE_FLOW_CREATE;
 
-	pf = port_flow_new(NULL, pattern, actions, &error);
+	pf = port_flow_new(&flow_attr, pattern, actions, &error);
 	if (!pf) {
 		free(job);
 		return port_flow_complain(&error);
@@ -2905,6 +2906,162 @@ port_queue_flow_push(portid_t port_id, queueid_t queue_id)
 	return ret;
 }
 
+/** Pull queue operation results from the queue. */
+static int
+port_queue_aged_flow_destroy(portid_t port_id, queueid_t queue_id,
+			     const uint32_t *rule, int nb_flows)
+{
+	struct rte_port *port = &ports[port_id];
+	struct rte_flow_op_result *res;
+	struct rte_flow_error error;
+	uint32_t n = nb_flows;
+	int ret = 0;
+	int i;
+
+	res = calloc(port->queue_sz, sizeof(struct rte_flow_op_result));
+	if (!res) {
+		printf("Failed to allocate memory for pulled results\n");
+		return -ENOMEM;
+	}
+
+	memset(&error, 0x66, sizeof(error));
+	while (nb_flows > 0) {
+		int success = 0;
+
+		if (n > port->queue_sz)
+			n = port->queue_sz;
+		ret = port_queue_flow_destroy(port_id, queue_id, true, n, rule);
+		if (ret < 0) {
+			free(res);
+			return ret;
+		}
+		ret = rte_flow_push(port_id, queue_id, &error);
+		if (ret < 0) {
+			printf("Failed to push operations in the queue: %s\n",
+			       strerror(-ret));
+			free(res);
+			return ret;
+		}
+		while (success < nb_flows) {
+			ret = rte_flow_pull(port_id, queue_id, res,
+					    port->queue_sz, &error);
+			if (ret < 0) {
+				printf("Failed to pull a operation results: %s\n",
+				       strerror(-ret));
+				free(res);
+				return ret;
+			}
+
+			for (i = 0; i < ret; i++) {
+				if (res[i].status == RTE_FLOW_OP_SUCCESS)
+					success++;
+			}
+		}
+		rule += n;
+		nb_flows -= n;
+		n = nb_flows;
+	}
+
+	free(res);
+	return ret;
+}
+
+/** List simply and destroy all aged flows per queue. */
+void
+port_queue_flow_aged(portid_t port_id, uint32_t queue_id, uint8_t destroy)
+{
+	void **contexts;
+	int nb_context, total = 0, idx;
+	uint32_t *rules = NULL;
+	struct rte_port *port;
+	struct rte_flow_error error;
+	enum age_action_context_type *type;
+	union {
+		struct port_flow *pf;
+		struct port_indirect_action *pia;
+	} ctx;
+
+	if (port_id_is_invalid(port_id, ENABLED_WARN) ||
+	    port_id == (portid_t)RTE_PORT_ALL)
+		return;
+	port = &ports[port_id];
+	if (queue_id >= port->queue_nb) {
+		printf("Error: queue #%u is invalid\n", queue_id);
+		return;
+	}
+	total = rte_flow_get_q_aged_flows(port_id, queue_id, NULL, 0, &error);
+	if (total < 0) {
+		port_flow_complain(&error);
+		return;
+	}
+	printf("Port %u queue %u total aged flows: %d\n",
+	       port_id, queue_id, total);
+	if (total == 0)
+		return;
+	contexts = calloc(total, sizeof(void *));
+	if (contexts == NULL) {
+		printf("Cannot allocate contexts for aged flow\n");
+		return;
+	}
+	printf("%-20s\tID\tGroup\tPrio\tAttr\n", "Type");
+	nb_context = rte_flow_get_q_aged_flows(port_id, queue_id, contexts,
+					       total, &error);
+	if (nb_context > total) {
+		printf("Port %u queue %u get aged flows count(%d) > total(%d)\n",
+		       port_id, queue_id, nb_context, total);
+		free(contexts);
+		return;
+	}
+	if (destroy) {
+		rules = malloc(sizeof(uint32_t) * nb_context);
+		if (rules == NULL)
+			printf("Cannot allocate memory for destroy aged flow\n");
+	}
+	total = 0;
+	for (idx = 0; idx < nb_context; idx++) {
+		if (!contexts[idx]) {
+			printf("Error: get Null context in port %u queue %u\n",
+			       port_id, queue_id);
+			continue;
+		}
+		type = (enum age_action_context_type *)contexts[idx];
+		switch (*type) {
+		case ACTION_AGE_CONTEXT_TYPE_FLOW:
+			ctx.pf = container_of(type, struct port_flow, age_type);
+			printf("%-20s\t%" PRIu32 "\t%" PRIu32 "\t%" PRIu32
+								 "\t%c%c%c\t\n",
+			       "Flow",
+			       ctx.pf->id,
+			       ctx.pf->rule.attr->group,
+			       ctx.pf->rule.attr->priority,
+			       ctx.pf->rule.attr->ingress ? 'i' : '-',
+			       ctx.pf->rule.attr->egress ? 'e' : '-',
+			       ctx.pf->rule.attr->transfer ? 't' : '-');
+			if (rules != NULL) {
+				rules[total] = ctx.pf->id;
+				total++;
+			}
+			break;
+		case ACTION_AGE_CONTEXT_TYPE_INDIRECT_ACTION:
+			ctx.pia = container_of(type,
+					       struct port_indirect_action,
+					       age_type);
+			printf("%-20s\t%" PRIu32 "\n", "Indirect action",
+			       ctx.pia->id);
+			break;
+		default:
+			printf("Error: invalid context type %u\n", port_id);
+			break;
+		}
+	}
+	if (rules != NULL) {
+		port_queue_aged_flow_destroy(port_id, queue_id, rules, total);
+		free(rules);
+	}
+	printf("\n%d flows destroyed\n", total);
+	free(contexts);
+}
+
 /** Pull queue operation results from the queue. */
 int
 port_queue_flow_pull(portid_t port_id, queueid_t queue_id)
diff --git a/app/test-pmd/testpmd.h b/app/test-pmd/testpmd.h
index 7fef96f9b1..e1efd34be9 100644
--- a/app/test-pmd/testpmd.h
+++ b/app/test-pmd/testpmd.h
@@ -941,6 +941,7 @@ int port_queue_action_handle_query(portid_t port_id, uint32_t queue_id,
 				   bool postpone, uint32_t id);
 int port_queue_flow_push(portid_t port_id, queueid_t queue_id);
 int port_queue_flow_pull(portid_t port_id, queueid_t queue_id);
+void port_queue_flow_aged(portid_t port_id, uint32_t queue_id, uint8_t destroy);
 int port_flow_validate(portid_t port_id,
 		       const struct rte_flow_attr *attr,
 		       const struct rte_flow_item *pattern,
diff --git a/doc/guides/testpmd_app_ug/testpmd_funcs.rst b/doc/guides/testpmd_app_ug/testpmd_funcs.rst
index 81e502b369..96c5ae0fe4 100644
--- a/doc/guides/testpmd_app_ug/testpmd_funcs.rst
+++ b/doc/guides/testpmd_app_ug/testpmd_funcs.rst
@@ -2995,6 +2995,10 @@ following sections.
 
    flow aged {port_id} [destroy]
 
+- Enqueue list and destroy aged flow rules::
+
+   flow queue {port_id} aged {queue_id} [destroy]
+
 - Tunnel offload - create a tunnel stub::
 
    flow tunnel create {port_id} type {tunnel_type}
@@ -4308,6 +4312,77 @@ If attach ``destroy`` parameter, the command will destroy all the list aged flow
    testpmd> flow aged 0
    Port 0 total aged flows: 0
 
+
+Enqueueing listing and destroying aged flow rules
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+
+``flow queue aged`` simply lists aged flow rules be get from
+``rte_flow_get_q_aged_flows`` API, and ``destroy`` parameter can be used to
+destroy those flow rules in PMD::
+
+   flow queue {port_id} aged {queue_id} [destroy]
+
+Listing current aged flow rules::
+
+   testpmd> flow queue 0 aged 0
+   Port 0 queue 0 total aged flows: 0
+   testpmd> flow queue 0 create 0 ingress tanle 0 item_template 0 action_template 0
+      pattern eth / ipv4 src is 2.2.2.14 / end
+      actions age timeout 5 / queue index 0 /  end
+   Flow rule #0 creation enqueued
+   testpmd> flow queue 0 create 0 ingress tanle 0 item_template 0 action_template 0
+      pattern eth / ipv4 src is 2.2.2.15 / end
+      actions age timeout 4 / queue index 0 /  end
+   Flow rule #1 creation enqueued
+   testpmd> flow queue 0 create 0 ingress tanle 0 item_template 0 action_template 0
+      pattern eth / ipv4 src is 2.2.2.16 / end
+      actions age timeout 4 / queue index 0 /  end
+   Flow rule #2 creation enqueued
+   testpmd> flow queue 0 create 0 ingress tanle 0 item_template 0 action_template 0
+      pattern eth / ipv4 src is 2.2.2.17 / end
+      actions age timeout 4 / queue index 0 /  end
+   Flow rule #3 creation enqueued
+   testpmd> flow pull 0 queue 0
+   Queue #0 pulled 4 operations (0 failed, 4 succeeded)
+
+Aged Rules are simply list as command ``flow queue {port_id} list {queue_id}``,
+but strip the detail rule information, all the aged flows are sorted by the
+longest timeout time. For example, if those rules is 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 queue 0 aged 0
+   Port 0 queue 0 total aged flows: 4
+   ID      Group   Prio    Attr
+   2       0       0       ---
+   3       0       0       ---
+   1       0       0       ---
+   0       0       0       ---
+
+   0 flows destroyed
+
+If attach ``destroy`` parameter, the command will destroy all the list aged flow rules::
+
+   testpmd> flow queue 0 aged 0 destroy
+   Port 0 queue 0 total aged flows: 4
+   ID      Group   Prio    Attr
+   2       0       0       ---
+   3       0       0       ---
+   1       0       0       ---
+   0       0       0       ---
+   Flow rule #2 destruction enqueued
+   Flow rule #3 destruction enqueued
+   Flow rule #1 destruction enqueued
+   Flow rule #0 destruction enqueued
+
+   4 flows destroyed
+   testpmd> flow queue 0 aged 0
+   Port 0 total aged flows: 0
+
+.. note::
+
+   The queue must be empty before attaching ``destroy`` parameter.
+
+
 Creating indirect actions
 ~~~~~~~~~~~~~~~~~~~~~~~~~
 
diff --git a/lib/ethdev/rte_flow.c b/lib/ethdev/rte_flow.c
index d11ba270db..7d0c24366c 100644
--- a/lib/ethdev/rte_flow.c
+++ b/lib/ethdev/rte_flow.c
@@ -1132,6 +1132,28 @@ rte_flow_get_aged_flows(uint16_t port_id, void **contexts,
 				  NULL, rte_strerror(ENOTSUP));
 }
 
+int
+rte_flow_get_q_aged_flows(uint16_t port_id, uint32_t queue_id, void **contexts,
+			  uint32_t nb_contexts, struct rte_flow_error *error)
+{
+	struct rte_eth_dev *dev = &rte_eth_devices[port_id];
+	const struct rte_flow_ops *ops = rte_flow_ops_get(port_id, error);
+	int ret;
+
+	if (unlikely(!ops))
+		return -rte_errno;
+	if (likely(!!ops->get_q_aged_flows)) {
+		fts_enter(dev);
+		ret = ops->get_q_aged_flows(dev, queue_id, contexts,
+					    nb_contexts, error);
+		fts_exit(dev);
+		return flow_err(port_id, ret, error);
+	}
+	return rte_flow_error_set(error, ENOTSUP,
+				  RTE_FLOW_ERROR_TYPE_UNSPECIFIED,
+				  NULL, rte_strerror(ENOTSUP));
+}
+
 struct rte_flow_action_handle *
 rte_flow_action_handle_create(uint16_t port_id,
 			      const struct rte_flow_indir_action_conf *conf,
diff --git a/lib/ethdev/rte_flow.h b/lib/ethdev/rte_flow.h
index a93ec796cb..64ec8f0903 100644
--- a/lib/ethdev/rte_flow.h
+++ b/lib/ethdev/rte_flow.h
@@ -2639,6 +2639,7 @@ enum rte_flow_action_type {
 	 * flow.
 	 *
 	 * See struct rte_flow_action_age.
+	 * See function rte_flow_get_q_aged_flows
 	 * See function rte_flow_get_aged_flows
 	 * see enum RTE_ETH_EVENT_FLOW_AGED
 	 * See struct rte_flow_query_age
@@ -2784,8 +2785,8 @@ struct rte_flow_action_queue {
  * on the flow. RTE_ETH_EVENT_FLOW_AGED event is triggered when a
  * port detects new aged-out flows.
  *
- * The flow context and the flow handle will be reported by the
- * rte_flow_get_aged_flows API.
+ * The flow context and the flow handle will be reported by the either
+ * rte_flow_get_aged_flows or rte_flow_get_q_aged_flows APIs.
  */
 struct rte_flow_action_age {
 	uint32_t timeout:24; /**< Time in seconds. */
@@ -4314,6 +4315,50 @@ int
 rte_flow_get_aged_flows(uint16_t port_id, void **contexts,
 			uint32_t nb_contexts, struct rte_flow_error *error);
 
+/**
+ * @warning
+ * @b EXPERIMENTAL: this API may change without prior notice.
+ *
+ * Get aged-out flows of a given port on the given flow queue.
+ *
+ * If application configure port attribute with RTE_FLOW_PORT_FLAG_STRICT_QUEUE,
+ * there is no RTE_ETH_EVENT_FLOW_AGED event and this function must be called to
+ * get the aged flows synchronously.
+ *
+ * If application configure port attribute without
+ * RTE_FLOW_PORT_FLAG_STRICT_QUEUE, RTE_ETH_EVENT_FLOW_AGED event will be
+ * triggered at least one new aged out flow was detected on any flow queue after
+ * the last call to rte_flow_get_q_aged_flows.
+ * In addition, the @p queue_id will be ignored.
+ * This function can be called to get the aged flows asynchronously from the
+ * event callback or synchronously regardless the event.
+ *
+ * @param[in] port_id
+ *   Port identifier of Ethernet device.
+ * @param[in] queue_id
+ *   Flow queue to query. Ignored when RTE_FLOW_PORT_FLAG_STRICT_QUEUE not set.
+ * @param[in, out] contexts
+ *   The address of an array of pointers to the aged-out flows contexts.
+ * @param[in] nb_contexts
+ *   The length of context array pointers.
+ * @param[out] error
+ *   Perform verbose error reporting if not NULL. Initialized in case of
+ *   error only.
+ *
+ * @return
+ *   if nb_contexts is 0, return the amount of all aged contexts.
+ *   if nb_contexts is not 0 , return the amount of aged flows reported
+ *   in the context array, otherwise negative errno value.
+ *
+ * @see rte_flow_action_age
+ * @see RTE_ETH_EVENT_FLOW_AGED
+ * @see rte_flow_port_flag
+ */
+__rte_experimental
+int
+rte_flow_get_q_aged_flows(uint16_t port_id, uint32_t queue_id, void **contexts,
+			  uint32_t nb_contexts, struct rte_flow_error *error);
+
 /**
  * Specify indirect action object configuration
  */
diff --git a/lib/ethdev/rte_flow_driver.h b/lib/ethdev/rte_flow_driver.h
index 7289deb538..c7d0699c91 100644
--- a/lib/ethdev/rte_flow_driver.h
+++ b/lib/ethdev/rte_flow_driver.h
@@ -84,6 +84,13 @@ struct rte_flow_ops {
 		 void **context,
 		 uint32_t nb_contexts,
 		 struct rte_flow_error *err);
+	/** See rte_flow_get_q_aged_flows() */
+	int (*get_q_aged_flows)
+		(struct rte_eth_dev *dev,
+		 uint32_t queue_id,
+		 void **contexts,
+		 uint32_t nb_contexts,
+		 struct rte_flow_error *error);
 	/** See rte_flow_action_handle_create() */
 	struct rte_flow_action_handle *(*action_handle_create)
 		(struct rte_eth_dev *dev,
diff --git a/lib/ethdev/version.map b/lib/ethdev/version.map
index e749678b96..17201fbe0f 100644
--- a/lib/ethdev/version.map
+++ b/lib/ethdev/version.map
@@ -295,6 +295,7 @@ EXPERIMENTAL {
 	rte_eth_rx_descriptor_dump;
 	rte_eth_tx_descriptor_dump;
 	rte_flow_async_action_handle_query;
+	rte_flow_get_q_aged_flows;
 	rte_mtr_meter_policy_get;
 	rte_mtr_meter_profile_get;
 };
-- 
2.25.1


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

* [PATCH v4 3/3] ethdev: add structure for indirect AGE update
  2022-10-26 21:49     ` [PATCH v4 0/3] ethdev: AGE action preparation Michael Baum
  2022-10-26 21:49       ` [PATCH v4 1/3] ethdev: add strict queue to pre-configuration flow hints Michael Baum
  2022-10-26 21:49       ` [PATCH v4 2/3] ethdev: add queue-based API to report aged flow rules Michael Baum
@ 2022-10-26 21:49       ` Michael Baum
  2022-10-28 10:56       ` [PATCH v4 0/3] ethdev: AGE action preparation Andrew Rybchenko
  3 siblings, 0 replies; 32+ messages in thread
From: Michael Baum @ 2022-10-26 21:49 UTC (permalink / raw)
  To: dev; +Cc: Matan Azrad, Raslan Darawsheh, Ori Kam, Andrew Rybchenko

Add a new structure for indirect AGE update.

This new structure enables:
1. Update timeout value.
2. Stop AGE checking.
3. Start AGE checking.
4. restart AGE checking.

Signed-off-by: Michael Baum <michaelba@nvidia.com>
Acked-by: Ori Kam <orika@nvidia.com>
Acked-by: Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>
---
 app/test-pmd/cmdline_flow.c        | 66 ++++++++++++++++++++++++++++++
 app/test-pmd/config.c              | 19 ++++++---
 doc/guides/prog_guide/rte_flow.rst | 19 +++++++++
 lib/ethdev/rte_flow.h              | 28 +++++++++++++
 4 files changed, 126 insertions(+), 6 deletions(-)

diff --git a/app/test-pmd/cmdline_flow.c b/app/test-pmd/cmdline_flow.c
index 992aeb95b3..88108498e0 100644
--- a/app/test-pmd/cmdline_flow.c
+++ b/app/test-pmd/cmdline_flow.c
@@ -586,6 +586,9 @@ enum index {
 	ACTION_SET_IPV6_DSCP_VALUE,
 	ACTION_AGE,
 	ACTION_AGE_TIMEOUT,
+	ACTION_AGE_UPDATE,
+	ACTION_AGE_UPDATE_TIMEOUT,
+	ACTION_AGE_UPDATE_TOUCH,
 	ACTION_SAMPLE,
 	ACTION_SAMPLE_RATIO,
 	ACTION_SAMPLE_INDEX,
@@ -1874,6 +1877,7 @@ static const enum index next_action[] = {
 	ACTION_SET_IPV4_DSCP,
 	ACTION_SET_IPV6_DSCP,
 	ACTION_AGE,
+	ACTION_AGE_UPDATE,
 	ACTION_SAMPLE,
 	ACTION_INDIRECT,
 	ACTION_MODIFY_FIELD,
@@ -2110,6 +2114,14 @@ static const enum index action_age[] = {
 	ZERO,
 };
 
+static const enum index action_age_update[] = {
+	ACTION_AGE_UPDATE,
+	ACTION_AGE_UPDATE_TIMEOUT,
+	ACTION_AGE_UPDATE_TOUCH,
+	ACTION_NEXT,
+	ZERO,
+};
+
 static const enum index action_sample[] = {
 	ACTION_SAMPLE,
 	ACTION_SAMPLE_RATIO,
@@ -2188,6 +2200,9 @@ static int parse_vc_spec(struct context *, const struct token *,
 			 const char *, unsigned int, void *, unsigned int);
 static int parse_vc_conf(struct context *, const struct token *,
 			 const char *, unsigned int, void *, unsigned int);
+static int parse_vc_conf_timeout(struct context *, const struct token *,
+				 const char *, unsigned int, void *,
+				 unsigned int);
 static int parse_vc_item_ecpri_type(struct context *, const struct token *,
 				    const char *, unsigned int,
 				    void *, unsigned int);
@@ -6206,6 +6221,30 @@ static const struct token token_list[] = {
 		.next = NEXT(action_age, NEXT_ENTRY(COMMON_UNSIGNED)),
 		.call = parse_vc_conf,
 	},
+	[ACTION_AGE_UPDATE] = {
+		.name = "age_update",
+		.help = "update aging parameter",
+		.next = NEXT(action_age_update),
+		.priv = PRIV_ACTION(AGE,
+				    sizeof(struct rte_flow_update_age)),
+		.call = parse_vc,
+	},
+	[ACTION_AGE_UPDATE_TIMEOUT] = {
+		.name = "timeout",
+		.help = "age timeout update value",
+		.args = ARGS(ARGS_ENTRY_BF(struct rte_flow_update_age,
+					   timeout, 24)),
+		.next = NEXT(action_age_update, NEXT_ENTRY(COMMON_UNSIGNED)),
+		.call = parse_vc_conf_timeout,
+	},
+	[ACTION_AGE_UPDATE_TOUCH] = {
+		.name = "touch",
+		.help = "this flow is touched",
+		.next = NEXT(action_age_update, NEXT_ENTRY(COMMON_BOOLEAN)),
+		.args = ARGS(ARGS_ENTRY_BF(struct rte_flow_update_age,
+					   touch, 1)),
+		.call = parse_vc_conf,
+	},
 	[ACTION_SAMPLE] = {
 		.name = "sample",
 		.help = "set a sample action",
@@ -7045,6 +7084,33 @@ parse_vc_conf(struct context *ctx, const struct token *token,
 	return len;
 }
 
+/** Parse action configuration field. */
+static int
+parse_vc_conf_timeout(struct context *ctx, const struct token *token,
+		      const char *str, unsigned int len,
+		      void *buf, unsigned int size)
+{
+	struct buffer *out = buf;
+	struct rte_flow_update_age *update;
+
+	(void)size;
+	if (ctx->curr != ACTION_AGE_UPDATE_TIMEOUT)
+		return -1;
+	/* 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;
+	/* Point to selected object. */
+	ctx->object = out->args.vc.data;
+	ctx->objmask = NULL;
+	/* Update the timeout is valid. */
+	update = (struct rte_flow_update_age *)out->args.vc.data;
+	update->timeout_valid = 1;
+	return len;
+}
+
 /** Parse eCPRI common header type field. */
 static int
 parse_vc_item_ecpri_type(struct context *ctx, const struct token *token,
diff --git a/app/test-pmd/config.c b/app/test-pmd/config.c
index 18f3543887..e8a1b77c2a 100644
--- a/app/test-pmd/config.c
+++ b/app/test-pmd/config.c
@@ -1886,6 +1886,7 @@ port_action_handle_update(portid_t port_id, uint32_t id,
 	if (!pia)
 		return -EINVAL;
 	switch (pia->type) {
+	case RTE_FLOW_ACTION_TYPE_AGE:
 	case RTE_FLOW_ACTION_TYPE_CONNTRACK:
 		update = action->conf;
 		break;
@@ -2816,17 +2817,23 @@ port_queue_action_handle_update(portid_t port_id,
 		return -EINVAL;
 	}
 
-	if (pia->type == RTE_FLOW_ACTION_TYPE_METER_MARK) {
+	switch (pia->type) {
+	case RTE_FLOW_ACTION_TYPE_AGE:
+		update = action->conf;
+		break;
+	case RTE_FLOW_ACTION_TYPE_METER_MARK:
 		rte_memcpy(&mtr_update.meter_mark, action->conf,
 			sizeof(struct rte_flow_action_meter_mark));
 		mtr_update.profile_valid = 1;
-		mtr_update.policy_valid  = 1;
-		mtr_update.color_mode_valid  = 1;
-		mtr_update.init_color_valid  = 1;
-		mtr_update.state_valid  = 1;
+		mtr_update.policy_valid = 1;
+		mtr_update.color_mode_valid = 1;
+		mtr_update.init_color_valid = 1;
+		mtr_update.state_valid = 1;
 		update = &mtr_update;
-	} else {
+		break;
+	default:
 		update = action;
+		break;
 	}
 
 	if (rte_flow_async_action_handle_update(port_id, queue_id, &attr,
diff --git a/doc/guides/prog_guide/rte_flow.rst b/doc/guides/prog_guide/rte_flow.rst
index 565868aeea..3e6242803d 100644
--- a/doc/guides/prog_guide/rte_flow.rst
+++ b/doc/guides/prog_guide/rte_flow.rst
@@ -2773,6 +2773,25 @@ shared AGE action, or a flow rule using the AGE action:
    | ``sec_since_last_hit``       | out | Seconds since last traffic hit         |
    +------------------------------+-----+----------------------------------------+
 
+Update structure to modify the parameters of an indirect AGE action.
+The update structure is used by ``rte_flow_action_handle_update()`` function.
+
+.. _table_rte_flow_update_age:
+
+.. table:: AGE update
+
+   +-------------------+--------------------------------------------------------------+
+   | Field             | Value                                                        |
+   +===================+==============================================================+
+   | ``reserved``      | 6 bits reserved, must be zero                                |
+   +-------------------+--------------------------------------------------------------+
+   | ``timeout_valid`` | 1 bit, timeout value is valid                                |
+   +-------------------+--------------------------------------------------------------+
+   | ``timeout``       | 24 bits timeout value                                        |
+   +-------------------+--------------------------------------------------------------+
+   | ``touch``         | 1 bit, touch the AGE action to set ``sec_since_last_hit`` 0  |
+   +-------------------+--------------------------------------------------------------+
+
 Action: ``SAMPLE``
 ^^^^^^^^^^^^^^^^^^
 
diff --git a/lib/ethdev/rte_flow.h b/lib/ethdev/rte_flow.h
index 64ec8f0903..8858b56428 100644
--- a/lib/ethdev/rte_flow.h
+++ b/lib/ethdev/rte_flow.h
@@ -2643,6 +2643,7 @@ enum rte_flow_action_type {
 	 * See function rte_flow_get_aged_flows
 	 * see enum RTE_ETH_EVENT_FLOW_AGED
 	 * See struct rte_flow_query_age
+	 * See struct rte_flow_update_age
 	 */
 	RTE_FLOW_ACTION_TYPE_AGE,
 
@@ -2809,6 +2810,33 @@ struct rte_flow_query_age {
 	uint32_t sec_since_last_hit:24; /**< Seconds since last traffic hit. */
 };
 
+/**
+ * @warning
+ * @b EXPERIMENTAL: this structure may change without prior notice
+ *
+ * RTE_FLOW_ACTION_TYPE_AGE
+ *
+ * Update indirect AGE action attributes:
+ *  - Timeout can be updated including stop/start action:
+ *     +-------------+-------------+------------------------------+
+ *     | Old Timeout | New Timeout | Updating                     |
+ *     +=============+=============+==============================+
+ *     | 0           | positive    | Start aging with new value   |
+ *     +-------------+-------------+------------------------------+
+ *     | positive    | 0           | Stop aging			  |
+ *     +-------------+-------------+------------------------------+
+ *     | positive    | positive    | Change timeout to new value  |
+ *     +-------------+-------------+------------------------------+
+ *  - sec_since_last_hit can be reset.
+ */
+struct rte_flow_update_age {
+	uint32_t reserved:6; /**< Reserved, must be zero. */
+	uint32_t timeout_valid:1; /**< The timeout is valid for update. */
+	uint32_t timeout:24; /**< Time in seconds. */
+	/** Means that aging should assume packet passed the aging. */
+	uint32_t touch:1;
+};
+
 /**
  * @warning
  * @b EXPERIMENTAL: this structure may change without prior notice
-- 
2.25.1


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

* Re: [PATCH v4 0/3] ethdev: AGE action preparation
  2022-10-26 21:49     ` [PATCH v4 0/3] ethdev: AGE action preparation Michael Baum
                         ` (2 preceding siblings ...)
  2022-10-26 21:49       ` [PATCH v4 3/3] ethdev: add structure for indirect AGE update Michael Baum
@ 2022-10-28 10:56       ` Andrew Rybchenko
  3 siblings, 0 replies; 32+ messages in thread
From: Andrew Rybchenko @ 2022-10-28 10:56 UTC (permalink / raw)
  To: Michael Baum, dev; +Cc: Matan Azrad, Raslan Darawsheh, Ori Kam

On 10/27/22 00:49, Michael Baum wrote:
> RFC's:
> https://patchwork.dpdk.org/project/dpdk/patch/7a45693f478b1b721b4e05131141b526185a175c.1654063912.git.jackmin@nvidia.com/
> https://patchwork.dpdk.org/project/dpdk/patch/608febf8d5d3c434a1eddb2e56f425ebbd6ff0b4.1654063912.git.jackmin@nvidia.com/
> 
> v2:
> - rebase.
> - Add reference to "rte_flow_update_age" structure in
>    "RTE_FLOW_ACTION_TYPE_AGE" definition.
> - Add reference to "rte_flow_get_q_aged_flows" function in
>    "RTE_FLOW_ACTION_TYPE_AGE" definition.
> - Change the order of "rte_flow_update_age" structure members in
>    documentation, to be aligned with the structure definition.
> - Place long comment before struct member definition.
> 
> v3:
> - Fix miss "break" in indirect action update switch-case.
> 
> v4:
> - Remove unrelated doc fixes.

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


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

end of thread, other threads:[~2022-10-28 10:56 UTC | newest]

Thread overview: 32+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-09-21 14:54 [PATCH 0/3] ethdev: AGE action preparation Michael Baum
2022-09-21 14:54 ` [PATCH 1/3] ethdev: add strict queue to pre-configuration flow hints Michael Baum
2022-09-29 12:04   ` Ori Kam
2022-09-21 14:54 ` [PATCH 2/3] ethdev: add queue-based API to report aged flow rules Michael Baum
2022-09-29 12:04   ` Ori Kam
2022-09-21 14:54 ` [PATCH 3/3] ethdev: add structure for indirect AGE update Michael Baum
2022-09-29 12:39   ` Ori Kam
2022-10-03  8:03   ` Andrew Rybchenko
2022-10-03  8:30     ` Ori Kam
2022-10-03 13:17       ` Andrew Rybchenko
2022-10-03 15:13         ` Ori Kam
2022-10-04  6:36           ` Andrew Rybchenko
2022-10-19 13:09             ` Michael Baum
2022-09-29  8:40 ` [PATCH 0/3] ethdev: AGE action preparation Andrew Rybchenko
2022-10-19 13:12 ` [PATCH v2 " Michael Baum
2022-10-19 13:12   ` [PATCH v2 1/3] ethdev: add strict queue to pre-configuration flow hints Michael Baum
2022-10-19 13:12   ` [PATCH v2 2/3] ethdev: add queue-based API to report aged flow rules Michael Baum
2022-10-19 13:12   ` [PATCH v2 3/3] ethdev: add structure for indirect AGE update Michael Baum
2022-10-19 14:49   ` [PATCH v3 0/3] ethdev: AGE action preparation Michael Baum
2022-10-19 14:49     ` [PATCH v3 1/3] ethdev: add strict queue to pre-configuration flow hints Michael Baum
2022-10-26 19:10       ` Andrew Rybchenko
2022-10-19 14:49     ` [PATCH v3 2/3] ethdev: add queue-based API to report aged flow rules Michael Baum
2022-10-26 19:15       ` Andrew Rybchenko
2022-10-26 21:17         ` Michael Baum
2022-10-19 14:49     ` [PATCH v3 3/3] ethdev: add structure for indirect AGE update Michael Baum
2022-10-26 19:18       ` Andrew Rybchenko
2022-10-26 21:19         ` Michael Baum
2022-10-26 21:49     ` [PATCH v4 0/3] ethdev: AGE action preparation Michael Baum
2022-10-26 21:49       ` [PATCH v4 1/3] ethdev: add strict queue to pre-configuration flow hints Michael Baum
2022-10-26 21:49       ` [PATCH v4 2/3] ethdev: add queue-based API to report aged flow rules Michael Baum
2022-10-26 21:49       ` [PATCH v4 3/3] ethdev: add structure for indirect AGE update Michael Baum
2022-10-28 10:56       ` [PATCH v4 0/3] ethdev: AGE action preparation Andrew Rybchenko

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