diff mbox series

ethdev: introduce indirect action APIs

Message ID 1617940481-125528-1-git-send-email-bingz@nvidia.com (mailing list archive)
State Superseded, archived
Delegated to: Ferruh Yigit
Headers
Series ethdev: introduce indirect action APIs |

Checks

Context Check Description
ci/checkpatch warning coding style issues
ci/travis-robot fail travis build: failed
ci/github-robot fail github build: failed
ci/Performance-Testing fail build patch failure
ci/Intel-compilation fail Compilation issues

Commit Message

Bing Zhao April 9, 2021, 3:54 a.m. UTC 
  Right now, rte_flow_shared_action_* APIs are used for some shared
actions, like RSS, count. The shared action shoule be created before
using it inside a flow. These shared actions sometimes are not
really shared but just some indirect actions decoupled from a flow.

The new functions rte_flow_action_handle_* are added to replace
the current shared functions rte_flow_shared_action_*.

There are two types of flow actions:
1. the direct (normal) actions that could be created and stored
   within a flow rule. Such action is tied to its flow rule and
   cannot be reused.
2. the indirect action, in the past, named shared_action. It is
   created from a direct actioni, like count or rss, and then used
   in the flow rules with an object handle. The PMD will take care
   of the retrieve from indirect action to the direct action
   when it is referenced.

The indirect action is accessed (update / query) w/o any flow rule,
just via the action object handle. For example, when querying or
resetting a counter, it could be done out of any flow using this
counter, but only the handle of the counter action object is
required.
The indirect action object could be shared by different flows or
used by a single flow, depending on the direct action type and
the real-life requirements.
The handle of an indirect action object is opaque and defined in
each driver and possibly different per direct action type.

The old name "shared" is improper in a sense and should be replaced.

All the command lines in testpmd application with "shared_action*"
are replaced with "indirect_action*".

The parameter of "update" interface is also changed. A general
pointer will replace the rte_flow_action struct pointer due to the
facts:
1. Some action may not support fields updating. In the example of a
   counter, the only "update" supported should be the reset. So
   passing a rte_flow_action struct pointer is meaningless and
   there is even no such corresponding action struct. What's more,
   if more than one operations should be supported, for some other
   action, such pointer paramter may not meet the need.
2. Some action may need conditional or partial update, the current
   parameter will not provide the ability to indicate which part(s)
   to update.
   For different types of indirect action objects, the pointer could
   either be the same of rte_flow_action* struct - in order not to
   break the current driver implementation, or some wrapper
   structures with bits as masks to indicate which part to be
   updated, depending on real needs of the corresponding direct
   action. For different direct actions, the structures of indirect
   action objects updating will be different.

All the underlayer PMD callbacks will be moved to these new APIs.

The RTE_FLOW_ACTION_TYPE_SHARED is kept for now in order not to
break the ABI. All the implementations are changed by using
RTE_FLOW_ACTION_TYPE_INDIRECT.

Signed-off-by: Bing Zhao <bingz@nvidia.com>
---
 doc/guides/rel_notes/release_21_05.rst |   3 +
 lib/librte_ethdev/rte_flow.c           |  48 +++++------
 lib/librte_ethdev/rte_flow.h           | 114 ++++++++++++++-----------
 lib/librte_ethdev/rte_flow_driver.h    |  26 +++---
 lib/librte_ethdev/version.map          |   8 +-
 5 files changed, 109 insertions(+), 90 deletions(-)

Comments

Bing Zhao April 19, 2021, 2:28 p.m. UTC | #1 
The patch set includes:
  1. API changes
  2. testpmd adaption and guide update
  3. driver update (only net/mlx5 is impacted)
  4. rte_flow documentation update

---
  v2: add adaptions of testpmd and driver part
  v3: squash all patches into one and update rte_flow doc
  v4: fix the doc format error
  v5: deprecate "RTE_FLOW_ACTION_TYPE_SHARED" and update description
---

Bing Zhao (1):
  ethdev: introduce indirect action APIs

 app/test-pmd/cmdline.c                      |  24 +--
 app/test-pmd/cmdline_flow.c                 | 252 ++++++++++++++--------------
 app/test-pmd/config.c                       | 160 +++++++++---------
 app/test-pmd/testpmd.h                      |  28 ++--
 doc/guides/prog_guide/rte_flow.rst          |  29 ++--
 doc/guides/rel_notes/release_21_05.rst      |   7 +
 doc/guides/testpmd_app_ug/testpmd_funcs.rst | 106 ++++++------
 drivers/net/mlx5/mlx5.c                     |   2 +-
 drivers/net/mlx5/mlx5_defs.h                |   4 +-
 drivers/net/mlx5/mlx5_flow.c                | 238 +++++++++++++-------------
 drivers/net/mlx5/mlx5_flow.h                |  24 +--
 drivers/net/mlx5/mlx5_flow_dv.c             |  85 +++++-----
 lib/librte_ethdev/rte_flow.c                |  56 +++----
 lib/librte_ethdev/rte_flow.h                | 119 +++++++------
 lib/librte_ethdev/rte_flow_driver.h         |  26 +--
 lib/librte_ethdev/version.map               |   8 +-
 16 files changed, 602 insertions(+), 566 deletions(-)
diff mbox series

Patch

diff --git a/doc/guides/rel_notes/release_21_05.rst b/doc/guides/rel_notes/release_21_05.rst
index 6f5858c8f6..abdaf90b3f 100644
--- a/doc/guides/rel_notes/release_21_05.rst
+++ b/doc/guides/rel_notes/release_21_05.rst
@@ -166,6 +166,9 @@  API Changes
 * pci: The value ``PCI_ANY_ID`` is marked as deprecated
   and can be replaced with ``RTE_PCI_ANY_ID``.
 
+* ethdev: The experimental shared action APIs in ``rte_flow.h`` has been
+  replaced from ``rte_flow_shared_action_*`` to indirect action APIs named
+  ``rte_flow_action_handle_*``.
 
 ABI Changes
 -----------
diff --git a/lib/librte_ethdev/rte_flow.c b/lib/librte_ethdev/rte_flow.c
index e07e617d74..5ae9397440 100644
--- a/lib/librte_ethdev/rte_flow.c
+++ b/lib/librte_ethdev/rte_flow.c
@@ -1067,53 +1067,53 @@  rte_flow_get_aged_flows(uint16_t port_id, void **contexts,
 				  NULL, rte_strerror(ENOTSUP));
 }
 
-struct rte_flow_shared_action *
-rte_flow_shared_action_create(uint16_t port_id,
-			      const struct rte_flow_shared_action_conf *conf,
+struct rte_flow_action_handle *
+rte_flow_action_handle_create(uint16_t port_id,
+			      const struct rte_flow_indir_action_conf *conf,
 			      const struct rte_flow_action *action,
 			      struct rte_flow_error *error)
 {
-	struct rte_flow_shared_action *shared_action;
+	struct rte_flow_action_handle *handle;
 	const struct rte_flow_ops *ops = rte_flow_ops_get(port_id, error);
 
 	if (unlikely(!ops))
 		return NULL;
-	if (unlikely(!ops->shared_action_create)) {
+	if (unlikely(!ops->action_handle_create)) {
 		rte_flow_error_set(error, ENOSYS,
 				   RTE_FLOW_ERROR_TYPE_UNSPECIFIED, NULL,
 				   rte_strerror(ENOSYS));
 		return NULL;
 	}
-	shared_action = ops->shared_action_create(&rte_eth_devices[port_id],
-						  conf, action, error);
-	if (shared_action == NULL)
+	handle = ops->action_handle_create(&rte_eth_devices[port_id],
+					   conf, action, error);
+	if (handle == NULL)
 		flow_err(port_id, -rte_errno, error);
-	return shared_action;
+	return handle;
 }
 
 int
-rte_flow_shared_action_destroy(uint16_t port_id,
-			      struct rte_flow_shared_action *action,
-			      struct rte_flow_error *error)
+rte_flow_action_handle_destroy(uint16_t port_id,
+			       struct rte_flow_action_handle *handle,
+			       struct rte_flow_error *error)
 {
 	int ret;
 	const struct rte_flow_ops *ops = rte_flow_ops_get(port_id, error);
 
 	if (unlikely(!ops))
 		return -rte_errno;
-	if (unlikely(!ops->shared_action_destroy))
+	if (unlikely(!ops->action_handle_destroy))
 		return rte_flow_error_set(error, ENOSYS,
 					  RTE_FLOW_ERROR_TYPE_UNSPECIFIED,
 					  NULL, rte_strerror(ENOSYS));
-	ret = ops->shared_action_destroy(&rte_eth_devices[port_id], action,
-					 error);
+	ret = ops->action_handle_destroy(&rte_eth_devices[port_id],
+					 handle, error);
 	return flow_err(port_id, ret, error);
 }
 
 int
-rte_flow_shared_action_update(uint16_t port_id,
-			      struct rte_flow_shared_action *action,
-			      const struct rte_flow_action *update,
+rte_flow_action_handle_update(uint16_t port_id,
+			      struct rte_flow_action_handle *handle,
+			      const void *update,
 			      struct rte_flow_error *error)
 {
 	int ret;
@@ -1121,18 +1121,18 @@  rte_flow_shared_action_update(uint16_t port_id,
 
 	if (unlikely(!ops))
 		return -rte_errno;
-	if (unlikely(!ops->shared_action_update))
+	if (unlikely(!ops->action_handle_update))
 		return rte_flow_error_set(error, ENOSYS,
 					  RTE_FLOW_ERROR_TYPE_UNSPECIFIED,
 					  NULL, rte_strerror(ENOSYS));
-	ret = ops->shared_action_update(&rte_eth_devices[port_id], action,
+	ret = ops->action_handle_update(&rte_eth_devices[port_id], handle,
 					update, error);
 	return flow_err(port_id, ret, error);
 }
 
 int
-rte_flow_shared_action_query(uint16_t port_id,
-			     const struct rte_flow_shared_action *action,
+rte_flow_action_handle_query(uint16_t port_id,
+			     const struct rte_flow_action_handle *handle,
 			     void *data,
 			     struct rte_flow_error *error)
 {
@@ -1141,11 +1141,11 @@  rte_flow_shared_action_query(uint16_t port_id,
 
 	if (unlikely(!ops))
 		return -rte_errno;
-	if (unlikely(!ops->shared_action_query))
+	if (unlikely(!ops->action_handle_query))
 		return rte_flow_error_set(error, ENOSYS,
 					  RTE_FLOW_ERROR_TYPE_UNSPECIFIED,
 					  NULL, rte_strerror(ENOSYS));
-	ret = ops->shared_action_query(&rte_eth_devices[port_id], action,
+	ret = ops->action_handle_query(&rte_eth_devices[port_id], handle,
 				       data, error);
 	return flow_err(port_id, ret, error);
 }
diff --git a/lib/librte_ethdev/rte_flow.h b/lib/librte_ethdev/rte_flow.h
index 6cc57136ac..880e0b50fb 100644
--- a/lib/librte_ethdev/rte_flow.h
+++ b/lib/librte_ethdev/rte_flow.h
@@ -2267,6 +2267,16 @@  enum rte_flow_action_type {
 	 * See struct rte_flow_action_modify_field.
 	 */
 	RTE_FLOW_ACTION_TYPE_MODIFY_FIELD,
+
+	/**
+	 * Describe indirect action that could be used by a single flow rule
+	 * or multiple flow rules.
+	 *
+	 * Allow flow rule(s) reference the same action by the indirect action
+	 * handle (see struct rte_flow_action_handle), rules could be on the
+	 * same port or across different ports.
+	 */
+	RTE_FLOW_ACTION_TYPE_INDIRECT,
 };
 
 /**
@@ -2847,17 +2857,23 @@  struct rte_flow_action_set_dscp {
 };
 
 /**
- * RTE_FLOW_ACTION_TYPE_SHARED
+ * @warning
+ * @b EXPERIMENTAL: this structure may change without prior notice
+ *
+ * RTE_FLOW_ACTION_TYPE_INDIRECT
  *
- * Opaque type returned after successfully creating a shared action.
+ * Opaque type returned after successfully creating an indirect action object.
+ * The definition of the object handle will be different per driver or
+ * per immediate action type.
  *
- * This handle can be used to manage and query the related action:
- * - share it across multiple flow rules
- * - update action configuration
- * - query action data
- * - destroy action
+ * This handle can be used to manage and query the related immediate action:
+ * - referenced in single flow rule or across multiple flow rules
+ *   over multiple ports
+ * - update action object configuration
+ * - query action object data
+ * - destroy action object
  */
-struct rte_flow_shared_action;
+struct rte_flow_action_handle;
 
 /**
  * Field IDs for MODIFY_FIELD action.
@@ -3628,25 +3644,22 @@  rte_flow_get_aged_flows(uint16_t port_id, void **contexts,
 			uint32_t nb_contexts, struct rte_flow_error *error);
 
 /**
- * Specify shared action configuration
+ * Specify indirect action object configuration
  */
-struct rte_flow_shared_action_conf {
+struct rte_flow_indir_action_conf {
 	/**
-	 * Flow direction for shared action configuration.
+	 * Flow direction for the indirect action configuration.
 	 *
-	 * Shared action should be valid at least for one flow direction,
+	 * Action should be valid at least for one flow direction,
 	 * otherwise it is invalid for both ingress and egress rules.
 	 */
 	uint32_t ingress:1;
 	/**< Action valid for rules applied to ingress traffic. */
 	uint32_t egress:1;
 	/**< Action valid for rules applied to egress traffic. */
-
 	/**
 	 * When set to 1, indicates that the action is valid for
 	 * transfer traffic; otherwise, for non-transfer traffic.
-	 *
-	 * See struct rte_flow_attr.
 	 */
 	uint32_t transfer:1;
 };
@@ -3655,16 +3668,17 @@  struct rte_flow_shared_action_conf {
  * @warning
  * @b EXPERIMENTAL: this API may change without prior notice.
  *
- * Create shared action for reuse in multiple flow rules.
- * The created shared action has single state and configuration
- * across all flow rules using it.
+ * Create an indirect action object that can be used by flow create, and
+ * could also be shared by different flows.
+ * The created object handle has single state and configuration
+ * across all the flow rules using it.
  *
  * @param[in] port_id
  *    The port identifier of the Ethernet device.
  * @param[in] conf
- *   Shared action configuration.
+ *   Action configuration for the indirect action object creation.
  * @param[in] action
- *   Action configuration for shared action creation.
+ *   Specific configuration of the indirect action object.
  * @param[out] error
  *   Perform verbose error reporting if not NULL. PMDs initialize this
  *   structure in case of error only.
@@ -3678,9 +3692,9 @@  struct rte_flow_shared_action_conf {
  *   - (ENOTSUP) if *action* valid but unsupported.
  */
 __rte_experimental
-struct rte_flow_shared_action *
-rte_flow_shared_action_create(uint16_t port_id,
-			      const struct rte_flow_shared_action_conf *conf,
+struct rte_flow_action_handle *
+rte_flow_action_handle_create(uint16_t port_id,
+			      const struct rte_flow_indir_action_conf *conf,
 			      const struct rte_flow_action *action,
 			      struct rte_flow_error *error);
 
@@ -3688,12 +3702,12 @@  rte_flow_shared_action_create(uint16_t port_id,
  * @warning
  * @b EXPERIMENTAL: this API may change without prior notice.
  *
- * Destroy the shared action by handle.
+ * Destroy indirect action by handle.
  *
  * @param[in] port_id
  *    The port identifier of the Ethernet device.
- * @param[in] action
- *   Handle for the shared action to be destroyed.
+ * @param[in] handle
+ *   Handle for the indirect action object to be destroyed.
  * @param[out] error
  *   Perform verbose error reporting if not NULL. PMDs initialize this
  *   structure in case of error only.
@@ -3708,27 +3722,30 @@  rte_flow_shared_action_create(uint16_t port_id,
  */
 __rte_experimental
 int
-rte_flow_shared_action_destroy(uint16_t port_id,
-			       struct rte_flow_shared_action *action,
+rte_flow_action_handle_destroy(uint16_t port_id,
+			       struct rte_flow_action_handle *handle,
 			       struct rte_flow_error *error);
 
 /**
  * @warning
  * @b EXPERIMENTAL: this API may change without prior notice.
  *
- * Update in-place the shared action configuration pointed by *action* handle
- * with the configuration provided as *update* argument.
- * The update of the shared action configuration effects all flow rules reusing
- * the action via handle.
+ * Update in-place the action configuration and / or state pointed
+ * by action *handle* with the configuration provided as *update* argument.
+ * The update of the action configuration effects all flow rules reusing
+ * the action via *handle*.
+ * The update general pointer provides the ability of partial updating.
  *
  * @param[in] port_id
  *    The port identifier of the Ethernet device.
- * @param[in] action
- *   Handle for the shared action to be updated.
+ * @param[in] handle
+ *   Handle for the indirect action object to be updated.
  * @param[in] update
- *   Action specification used to modify the action pointed by handle.
- *   *update* should be of same type with the action pointed by the *action*
- *   handle argument, otherwise considered as invalid.
+ *   Update profile specification used to modify the action pointed by handle.
+ *   *update* could be with the same type of the immediate action corresponding
+ *   to the *handle* argument when creating, or a wrapper structure includes
+ *   action configuration to be updated and bit fields to indicate the member
+ *   of fields inside the action to update.
  * @param[out] error
  *   Perform verbose error reporting if not NULL. PMDs initialize this
  *   structure in case of error only.
@@ -3739,32 +3756,32 @@  rte_flow_shared_action_destroy(uint16_t port_id,
  *   - (-EIO) if underlying device is removed.
  *   - (-EINVAL) if *update* invalid.
  *   - (-ENOTSUP) if *update* valid but unsupported.
- *   - (-ENOENT) if action pointed by *ctx* was not found.
+ *   - (-ENOENT) if indirect action object pointed by *handle* was not found.
  *   rte_errno is also set.
  */
 __rte_experimental
 int
-rte_flow_shared_action_update(uint16_t port_id,
-			      struct rte_flow_shared_action *action,
-			      const struct rte_flow_action *update,
+rte_flow_action_handle_update(uint16_t port_id,
+			      struct rte_flow_action_handle *handle,
+			      const void *update,
 			      struct rte_flow_error *error);
 
 /**
  * @warning
  * @b EXPERIMENTAL: this API may change without prior notice.
  *
- * Query the shared action by handle.
+ * Query the orginal action by corresponding indirect action object handle.
  *
  * Retrieve action-specific data such as counters.
  * Data is gathered by special action which may be present/referenced in
  * more than one flow rule definition.
  *
- * \see RTE_FLOW_ACTION_TYPE_COUNT
+ * @see RTE_FLOW_ACTION_TYPE_COUNT
  *
  * @param port_id
  *   Port identifier of Ethernet device.
- * @param[in] action
- *   Handle for the shared action to query.
+ * @param[in] handle
+ *   Handle for the action object to query.
  * @param[in, out] data
  *   Pointer to storage for the associated query data type.
  * @param[out] error
@@ -3776,10 +3793,9 @@  rte_flow_shared_action_update(uint16_t port_id,
  */
 __rte_experimental
 int
-rte_flow_shared_action_query(uint16_t port_id,
-			     const struct rte_flow_shared_action *action,
-			     void *data,
-			     struct rte_flow_error *error);
+rte_flow_action_handle_query(uint16_t port_id,
+			     const struct rte_flow_action_handle *handle,
+			     void *data, struct rte_flow_error *error);
 
 /* Tunnel has a type and the key information. */
 struct rte_flow_tunnel {
diff --git a/lib/librte_ethdev/rte_flow_driver.h b/lib/librte_ethdev/rte_flow_driver.h
index da594d9256..8d825eb245 100644
--- a/lib/librte_ethdev/rte_flow_driver.h
+++ b/lib/librte_ethdev/rte_flow_driver.h
@@ -83,27 +83,27 @@  struct rte_flow_ops {
 		 void **context,
 		 uint32_t nb_contexts,
 		 struct rte_flow_error *err);
-	/** See rte_flow_shared_action_create() */
-	struct rte_flow_shared_action *(*shared_action_create)
+	/** See rte_flow_action_handle_create() */
+	struct rte_flow_action_handle *(*action_handle_create)
 		(struct rte_eth_dev *dev,
-		 const struct rte_flow_shared_action_conf *conf,
+		 const struct rte_flow_indir_action_conf *conf,
 		 const struct rte_flow_action *action,
 		 struct rte_flow_error *error);
-	/** See rte_flow_shared_action_destroy() */
-	int (*shared_action_destroy)
+	/** See rte_flow_action_handle_destroy() */
+	int (*action_handle_destroy)
 		(struct rte_eth_dev *dev,
-		 struct rte_flow_shared_action *shared_action,
+		 struct rte_flow_action_handle *handle,
 		 struct rte_flow_error *error);
-	/** See rte_flow_shared_action_update() */
-	int (*shared_action_update)
+	/** See rte_flow_action_handle_update() */
+	int (*action_handle_update)
 		(struct rte_eth_dev *dev,
-		 struct rte_flow_shared_action *shared_action,
-		 const struct rte_flow_action *update,
+		 struct rte_flow_action_handle *handle,
+		 const void *update,
 		 struct rte_flow_error *error);
-	/** See rte_flow_shared_action_query() */
-	int (*shared_action_query)
+	/** See rte_flow_action_handle_query() */
+	int (*action_handle_query)
 		(struct rte_eth_dev *dev,
-		 const struct rte_flow_shared_action *shared_action,
+		 const struct rte_flow_action_handle *handle,
 		 void *data,
 		 struct rte_flow_error *error);
 	/** See rte_flow_tunnel_decap_set() */
diff --git a/lib/librte_ethdev/version.map b/lib/librte_ethdev/version.map
index 93ad388e96..4eb561a89a 100644
--- a/lib/librte_ethdev/version.map
+++ b/lib/librte_ethdev/version.map
@@ -231,10 +231,6 @@  EXPERIMENTAL {
 	rte_eth_fec_get_capability;
 	rte_eth_fec_get;
 	rte_eth_fec_set;
-	rte_flow_shared_action_create;
-	rte_flow_shared_action_destroy;
-	rte_flow_shared_action_query;
-	rte_flow_shared_action_update;
 	rte_flow_tunnel_decap_set;
 	rte_flow_tunnel_match;
 	rte_flow_get_restore_info;
@@ -246,6 +242,10 @@  EXPERIMENTAL {
 
 	# added in 21.05
 	rte_eth_representor_info_get;
+	rte_flow_action_handle_create;
+	rte_flow_action_handle_destroy;
+	rte_flow_action_handle_update;
+	rte_flow_action_handle_query;
 };
 
 INTERNAL {