[v3] ethdev: add flow rule actions update API

Message ID 20230518214852.2364233-1-akozyrev@nvidia.com (mailing list archive)
State Superseded, archived
Delegated to: Ferruh Yigit
Headers
Series [v3] ethdev: add flow rule actions update API |

Checks

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

Commit Message

Alexander Kozyrev May 18, 2023, 9:48 p.m. UTC
  Introduce the new rte_flow_update() API allowing users
to update the action list in the already existing rule.
Flow rules can be updated now without the need to destroy
the rule first and create a new one instead.
A single API call ensures that no packets are lost by
guaranteeing atomicity and flow state correctness.
The rte_flow_async_update() is added as well.
The matcher is not updated, only the action list is.

Signed-off-by: Alexander Kozyrev <akozyrev@nvidia.com>
---
 doc/guides/prog_guide/rte_flow.rst     | 42 +++++++++++++++++
 doc/guides/rel_notes/release_23_07.rst |  4 ++
 lib/ethdev/ethdev_trace.h              | 29 ++++++++++++
 lib/ethdev/ethdev_trace_points.c       |  6 +++
 lib/ethdev/rte_flow.c                  | 53 ++++++++++++++++++++++
 lib/ethdev/rte_flow.h                  | 62 ++++++++++++++++++++++++++
 lib/ethdev/rte_flow_driver.h           | 16 +++++++
 lib/ethdev/version.map                 |  2 +
 8 files changed, 214 insertions(+)
  

Comments

Ori Kam May 21, 2023, 7:07 p.m. UTC | #1
Hi Alexander,

> -----Original Message-----
> From: Alexander Kozyrev <akozyrev@nvidia.com>
> Sent: Friday, May 19, 2023 12:49 AM
> 
> Introduce the new rte_flow_update() API allowing users
> to update the action list in the already existing rule.
> Flow rules can be updated now without the need to destroy
> the rule first and create a new one instead.
> A single API call ensures that no packets are lost by
> guaranteeing atomicity and flow state correctness.
> The rte_flow_async_update() is added as well.
> The matcher is not updated, only the action list is.
> 
> Signed-off-by: Alexander Kozyrev <akozyrev@nvidia.com>
> ---

Acked-by: Ori Kam <orika@nvidia.com>
Best,
Ori
  
Ferruh Yigit May 22, 2023, 10:28 a.m. UTC | #2
On 5/18/2023 10:48 PM, Alexander Kozyrev wrote:
> Introduce the new rte_flow_update() API allowing users
> to update the action list in the already existing rule.

If the API is only to update actions, does make sense to rename it to
explicitly state this, like:
`rte_flow_action_update()`

Same for async version of the API.

> Flow rules can be updated now without the need to destroy
> the rule first and create a new one instead.
> A single API call ensures that no packets are lost by
> guaranteeing atomicity and flow state correctness.
> The rte_flow_async_update() is added as well.
> The matcher is not updated, only the action list is.
> 
> Signed-off-by: Alexander Kozyrev <akozyrev@nvidia.com>

<...>
  
Ori Kam May 23, 2023, 6:59 a.m. UTC | #3
Hi 

> -----Original Message-----
> From: Ferruh Yigit <ferruh.yigit@amd.com>
> Sent: Monday, May 22, 2023 1:28 PM
> 
> On 5/18/2023 10:48 PM, Alexander Kozyrev wrote:
> > Introduce the new rte_flow_update() API allowing users
> > to update the action list in the already existing rule.
> 
> If the API is only to update actions, does make sense to rename it to
> explicitly state this, like:
> `rte_flow_action_update()`
> 
> Same for async version of the API.
> 

I'm O.K with the suggested name.
Maybe just change action to actions?

Best,
Ori

> > Flow rules can be updated now without the need to destroy
> > the rule first and create a new one instead.
> > A single API call ensures that no packets are lost by
> > guaranteeing atomicity and flow state correctness.
> > The rte_flow_async_update() is added as well.
> > The matcher is not updated, only the action list is.
> >
> > Signed-off-by: Alexander Kozyrev <akozyrev@nvidia.com>
> 
> <...>
  
Ferruh Yigit May 23, 2023, 10:34 a.m. UTC | #4
On 5/23/2023 7:59 AM, Ori Kam wrote:
> Hi 
> 
>> -----Original Message-----
>> From: Ferruh Yigit <ferruh.yigit@amd.com>
>> Sent: Monday, May 22, 2023 1:28 PM
>>
>> On 5/18/2023 10:48 PM, Alexander Kozyrev wrote:
>>> Introduce the new rte_flow_update() API allowing users
>>> to update the action list in the already existing rule.
>>
>> If the API is only to update actions, does make sense to rename it to
>> explicitly state this, like:
>> `rte_flow_action_update()`
>>
>> Same for async version of the API.
>>
> 
> I'm O.K with the suggested name.
> Maybe just change action to actions?
> 

Both OK for me, existing APIs have mixed usage of 'action' vs 'actions',
is there a clear distinction when to use one or other?

> Best,
> Ori
> 
>>> Flow rules can be updated now without the need to destroy
>>> the rule first and create a new one instead.
>>> A single API call ensures that no packets are lost by
>>> guaranteeing atomicity and flow state correctness.
>>> The rte_flow_async_update() is added as well.
>>> The matcher is not updated, only the action list is.
>>>
>>> Signed-off-by: Alexander Kozyrev <akozyrev@nvidia.com>
>>
>> <...>
  
Ori Kam May 23, 2023, 11:20 a.m. UTC | #5
Hi Ferruh,

> -----Original Message-----
> From: Ferruh Yigit <ferruh.yigit@amd.com>
> Sent: Tuesday, May 23, 2023 1:34 PM
> 
> On 5/23/2023 7:59 AM, Ori Kam wrote:
> > Hi
> >
> >> -----Original Message-----
> >> From: Ferruh Yigit <ferruh.yigit@amd.com>
> >> Sent: Monday, May 22, 2023 1:28 PM
> >>
> >> On 5/18/2023 10:48 PM, Alexander Kozyrev wrote:
> >>> Introduce the new rte_flow_update() API allowing users
> >>> to update the action list in the already existing rule.
> >>
> >> If the API is only to update actions, does make sense to rename it to
> >> explicitly state this, like:
> >> `rte_flow_action_update()`
> >>
> >> Same for async version of the API.
> >>
> >
> > I'm O.K with the suggested name.
> > Maybe just change action to actions?
> >
> 
> Both OK for me, existing APIs have mixed usage of 'action' vs 'actions',
> is there a clear distinction when to use one or other?
> 
The idea is that if we have a single action it is action else actions.
For example,
1. Template create - rte_flow_actions_template_create since it has number of actions.
2. create indirect action - rte_flow_async_action_handle_create since is create just one action

> > Best,
> > Ori
> >
> >>> Flow rules can be updated now without the need to destroy
> >>> the rule first and create a new one instead.
> >>> A single API call ensures that no packets are lost by
> >>> guaranteeing atomicity and flow state correctness.
> >>> The rte_flow_async_update() is added as well.
> >>> The matcher is not updated, only the action list is.
> >>>
> >>> Signed-off-by: Alexander Kozyrev <akozyrev@nvidia.com>
> >>
> >> <...>
  

Patch

diff --git a/doc/guides/prog_guide/rte_flow.rst b/doc/guides/prog_guide/rte_flow.rst
index 32fc45516a..0930accfea 100644
--- a/doc/guides/prog_guide/rte_flow.rst
+++ b/doc/guides/prog_guide/rte_flow.rst
@@ -3446,6 +3446,31 @@  Return values:
 
 - 0 on success, a negative errno value otherwise and ``rte_errno`` is set.
 
+Update
+~~~~~~
+
+Update an existing flow rule with a new set of actions.
+
+.. code-block:: c
+
+   struct rte_flow *
+   rte_flow_update(uint16_t port_id,
+                   struct rte_flow *flow,
+                   const struct rte_flow_action *actions[],
+                   struct rte_flow_error *error);
+
+Arguments:
+
+- ``port_id``: port identifier of Ethernet device.
+- ``flow``: flow rule handle to update.
+- ``actions``: associated actions (list terminated by the END action).
+- ``error``: perform verbose error reporting if not NULL. PMDs initialize
+  this structure in case of error only.
+
+Return values:
+
+- 0 on success, a negative errno value otherwise and ``rte_errno`` is set.
+
 Flush
 ~~~~~
 
@@ -3795,6 +3820,23 @@  Enqueueing a flow rule destruction operation is similar to simple destruction.
                           void *user_data,
                           struct rte_flow_error *error);
 
+Enqueue update operation
+~~~~~~~~~~~~~~~~~~~~~~~~~~
+
+Enqueueing a flow rule update operation to replace actions in the existing rule.
+
+.. code-block:: c
+
+   int
+   rte_flow_async_update(uint16_t port_id,
+                         uint32_t queue_id,
+                         const struct rte_flow_op_attr *op_attr,
+                         struct rte_flow *flow,
+                         const struct rte_flow_action actions[],
+                         uint8_t actions_template_index,
+                         void *user_data,
+                         struct rte_flow_error *error);
+
 Enqueue indirect action creation operation
 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
 
diff --git a/doc/guides/rel_notes/release_23_07.rst b/doc/guides/rel_notes/release_23_07.rst
index a9b1293689..94e9f8b3ae 100644
--- a/doc/guides/rel_notes/release_23_07.rst
+++ b/doc/guides/rel_notes/release_23_07.rst
@@ -55,6 +55,10 @@  New Features
      Also, make sure to start the actual text at the margin.
      =======================================================
 
+   * **Added flow rule update to the Flow API.**
+
+     * Added API for updating the action list in the already existing rule.
+       Introduced both rte_flow_update() and rte_flow_async_update() functions.
 
 Removed Items
 -------------
diff --git a/lib/ethdev/ethdev_trace.h b/lib/ethdev/ethdev_trace.h
index 3dc7d028b8..ba7871aa3e 100644
--- a/lib/ethdev/ethdev_trace.h
+++ b/lib/ethdev/ethdev_trace.h
@@ -2220,6 +2220,17 @@  RTE_TRACE_POINT_FP(
 	rte_trace_point_emit_int(ret);
 )
 
+/* Called in loop in app/test-flow-perf */
+RTE_TRACE_POINT_FP(
+	rte_flow_trace_update,
+	RTE_TRACE_POINT_ARGS(uint16_t port_id, const struct rte_flow *flow,
+		const struct rte_flow_action *actions, int ret),
+	rte_trace_point_emit_u16(port_id);
+	rte_trace_point_emit_ptr(flow);
+	rte_trace_point_emit_ptr(actions);
+	rte_trace_point_emit_int(ret);
+)
+
 RTE_TRACE_POINT_FP(
 	rte_flow_trace_query,
 	RTE_TRACE_POINT_ARGS(uint16_t port_id, const struct rte_flow *flow,
@@ -2345,6 +2356,24 @@  RTE_TRACE_POINT_FP(
 	rte_trace_point_emit_ptr(flow);
 )
 
+RTE_TRACE_POINT_FP(
+	rte_flow_trace_async_update,
+	RTE_TRACE_POINT_ARGS(uint16_t port_id, uint32_t queue_id,
+		const struct rte_flow_op_attr *op_attr,
+		const struct rte_flow *flow,
+		const struct rte_flow_action *actions,
+		uint8_t actions_template_index,
+		const void *user_data, int ret),
+	rte_trace_point_emit_u16(port_id);
+	rte_trace_point_emit_u32(queue_id);
+	rte_trace_point_emit_ptr(op_attr);
+	rte_trace_point_emit_ptr(flow);
+	rte_trace_point_emit_ptr(actions);
+	rte_trace_point_emit_u8(actions_template_index);
+	rte_trace_point_emit_ptr(user_data);
+	rte_trace_point_emit_int(ret);
+)
+
 RTE_TRACE_POINT_FP(
 	rte_flow_trace_pull,
 	RTE_TRACE_POINT_ARGS(uint16_t port_id, uint32_t queue_id,
diff --git a/lib/ethdev/ethdev_trace_points.c b/lib/ethdev/ethdev_trace_points.c
index 61010cae56..f7bc6f342b 100644
--- a/lib/ethdev/ethdev_trace_points.c
+++ b/lib/ethdev/ethdev_trace_points.c
@@ -490,6 +490,9 @@  RTE_TRACE_POINT_REGISTER(rte_flow_trace_create,
 RTE_TRACE_POINT_REGISTER(rte_flow_trace_destroy,
 	lib.ethdev.flow.destroy)
 
+RTE_TRACE_POINT_REGISTER(rte_flow_trace_update,
+	lib.ethdev.flow.update)
+
 RTE_TRACE_POINT_REGISTER(rte_flow_trace_flush,
 	lib.ethdev.flow.flush)
 
@@ -580,6 +583,9 @@  RTE_TRACE_POINT_REGISTER(rte_flow_trace_async_create,
 RTE_TRACE_POINT_REGISTER(rte_flow_trace_async_destroy,
 	lib.ethdev.flow.async_destroy)
 
+RTE_TRACE_POINT_REGISTER(rte_flow_trace_async_update,
+	lib.ethdev.flow.async_update)
+
 RTE_TRACE_POINT_REGISTER(rte_flow_trace_push,
 	lib.ethdev.flow.push)
 
diff --git a/lib/ethdev/rte_flow.c b/lib/ethdev/rte_flow.c
index 69e6e749f7..baef9e92ea 100644
--- a/lib/ethdev/rte_flow.c
+++ b/lib/ethdev/rte_flow.c
@@ -441,6 +441,32 @@  rte_flow_destroy(uint16_t port_id,
 				  NULL, rte_strerror(ENOSYS));
 }
 
+int
+rte_flow_update(uint16_t port_id,
+		struct rte_flow *flow,
+		const struct rte_flow_action actions[],
+		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->update)) {
+		fts_enter(dev);
+		ret = ops->update(dev, flow, actions, error);
+		fts_exit(dev);
+
+		rte_flow_trace_update(port_id, flow, actions, ret);
+
+		return flow_err(port_id, ret, error);
+	}
+	return rte_flow_error_set(error, ENOSYS,
+				  RTE_FLOW_ERROR_TYPE_UNSPECIFIED,
+				  NULL, rte_strerror(ENOSYS));
+}
+
 /* Destroy all flow rules associated with a port. */
 int
 rte_flow_flush(uint16_t port_id,
@@ -1985,6 +2011,33 @@  rte_flow_async_destroy(uint16_t port_id,
 	return ret;
 }
 
+int
+rte_flow_async_update(uint16_t port_id,
+		      uint32_t queue_id,
+		      const struct rte_flow_op_attr *op_attr,
+		      struct rte_flow *flow,
+		      const struct rte_flow_action actions[],
+		      uint8_t actions_template_index,
+		      void *user_data,
+		      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;
+
+	ret = flow_err(port_id,
+		       ops->async_update(dev, queue_id, op_attr, flow,
+					 actions, actions_template_index,
+					 user_data, error),
+		       error);
+
+	rte_flow_trace_async_update(port_id, queue_id, op_attr, flow,
+				    actions, actions_template_index,
+				    user_data, ret);
+
+	return ret;
+}
+
 int
 rte_flow_push(uint16_t port_id,
 	      uint32_t queue_id,
diff --git a/lib/ethdev/rte_flow.h b/lib/ethdev/rte_flow.h
index 713ba8b65c..79bfc07a1c 100644
--- a/lib/ethdev/rte_flow.h
+++ b/lib/ethdev/rte_flow.h
@@ -4343,6 +4343,29 @@  rte_flow_destroy(uint16_t port_id,
 		 struct rte_flow *flow,
 		 struct rte_flow_error *error);
 
+/**
+ * Update a flow rule with new actions on a given port.
+ *
+ * @param port_id
+ *   Port identifier of Ethernet device.
+ * @param flow
+ *   Flow rule handle to update.
+ * @param[in] actions
+ *   Associated actions (list terminated by the END action).
+ * @param[out] error
+ *   Perform verbose error reporting if not NULL. PMDs initialize this
+ *   structure in case of error only.
+ *
+ * @return
+ *   0 on success, a negative errno value otherwise and rte_errno is set.
+ */
+__rte_experimental
+int
+rte_flow_update(uint16_t port_id,
+		struct rte_flow *flow,
+		const struct rte_flow_action actions[],
+		struct rte_flow_error *error);
+
 /**
  * Destroy all flow rules associated with a port.
  *
@@ -5770,6 +5793,45 @@  rte_flow_async_destroy(uint16_t port_id,
 		       void *user_data,
 		       struct rte_flow_error *error);
 
+/**
+ * @warning
+ * @b EXPERIMENTAL: this API may change without prior notice.
+ *
+ * Enqueue rule update operation.
+ *
+ * @param port_id
+ *   Port identifier of Ethernet device.
+ * @param queue_id
+ *   Flow queue used to insert the rule.
+ * @param[in] op_attr
+ *   Rule creation operation attributes.
+ * @param[in] flow
+ *   Flow rule to be updated.
+ * @param[in] actions
+ *   List of actions to be used.
+ *   The list order should match the order in the actions template.
+ * @param[in] actions_template_index
+ *   Actions template index in the table.
+ * @param[in] user_data
+ *   The user data that will be returned on the completion events.
+ * @param[out] error
+ *   Perform verbose error reporting if not NULL.
+ *   PMDs initialize this structure in case of error only.
+ *
+ * @return
+ *   0 on success, a negative errno value otherwise and rte_errno is set.
+ */
+__rte_experimental
+int
+rte_flow_async_update(uint16_t port_id,
+		      uint32_t queue_id,
+		      const struct rte_flow_op_attr *op_attr,
+		      struct rte_flow *flow,
+		      const struct rte_flow_action actions[],
+		      uint8_t actions_template_index,
+		      void *user_data,
+		      struct rte_flow_error *error);
+
 /**
  * @warning
  * @b EXPERIMENTAL: this API may change without prior notice.
diff --git a/lib/ethdev/rte_flow_driver.h b/lib/ethdev/rte_flow_driver.h
index a129a4605d..193b09a7d3 100644
--- a/lib/ethdev/rte_flow_driver.h
+++ b/lib/ethdev/rte_flow_driver.h
@@ -302,6 +302,22 @@  struct rte_flow_ops {
 		 const void *update, void *query,
 		 enum rte_flow_query_update_mode qu_mode,
 		 void *user_data, struct rte_flow_error *error);
+	/** See rte_flow_update(). */
+	int (*update)
+		(struct rte_eth_dev *dev,
+		 struct rte_flow *flow,
+		 const struct rte_flow_action actions[],
+		 struct rte_flow_error *error);
+	/** See rte_flow_async_update() */
+	int (*async_update)
+		(struct rte_eth_dev *dev,
+		 uint32_t queue_id,
+		 const struct rte_flow_op_attr *op_attr,
+		 struct rte_flow *flow,
+		 const struct rte_flow_action actions[],
+		 uint8_t actions_template_index,
+		 void *user_data,
+		 struct rte_flow_error *error);
 };
 
 /**
diff --git a/lib/ethdev/version.map b/lib/ethdev/version.map
index 357d1a88c0..d4f49cb918 100644
--- a/lib/ethdev/version.map
+++ b/lib/ethdev/version.map
@@ -299,6 +299,8 @@  EXPERIMENTAL {
 	rte_flow_action_handle_query_update;
 	rte_flow_async_action_handle_query_update;
 	rte_flow_async_create_by_index;
+	rte_flow_update;
+	rte_flow_async_update;
 };
 
 INTERNAL {