[v2,1/4] ethdev: introduce indirect action APIs

  Right now, rte_flow_shared_action_* APIs are used for some shared
actions, like RSS, count. The shared action should 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 parameter 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           |  56 ++++++++--------
 lib/librte_ethdev/rte_flow.h           | 118 +++++++++++++++++++--------------
 lib/librte_ethdev/rte_flow_driver.h    |  26 ++++----
 lib/librte_ethdev/version.map          |   8 +--
 5 files changed, 115 insertions(+), 96 deletions(-)
  

On 4/10/2021 3:03 PM, Bing Zhao wrote:
> Right now, rte_flow_shared_action_* APIs are used for some shared
> actions, like RSS, count. The shared action should 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 parameter 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           |  56 ++++++++--------
>   lib/librte_ethdev/rte_flow.h           | 118 +++++++++++++++++++--------------
>   lib/librte_ethdev/rte_flow_driver.h    |  26 ++++----
>   lib/librte_ethdev/version.map          |   8 +--

Isn't there any documentation to update with this change?

If the shared action API is not documented at all, can you please add 
documentation for the action handle API?
  

Hi Ferruh,

> -----Original Message-----
> From: Ferruh Yigit <ferruh.yigit@intel.com>
> Sent: Tuesday, April 13, 2021 3:42 AM
> To: Bing Zhao <bingz@nvidia.com>; Ori Kam <orika@nvidia.com>; NBU-
> Contact-Thomas Monjalon <thomas@monjalon.net>;
> andrew.rybchenko@oktetlabs.ru; Matan Azrad <matan@nvidia.com>; Slava
> Ovsiienko <viacheslavo@nvidia.com>
> Cc: dev@dpdk.org; ajit.khaparde@broadcom.com; Gregory Etelson
> <getelson@nvidia.com>; Andrey Vesnovaty <andreyv@nvidia.com>
> Subject: Re: [PATCH v2 1/4] ethdev: introduce indirect action APIs
> 
> External email: Use caution opening links or attachments
> 
> 
> On 4/10/2021 3:03 PM, Bing Zhao wrote:
> > Right now, rte_flow_shared_action_* APIs are used for some shared
> > actions, like RSS, count. The shared action should 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 parameter 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           |  56 ++++++++--------
> >   lib/librte_ethdev/rte_flow.h           | 118
> +++++++++++++++++++--------------
> >   lib/librte_ethdev/rte_flow_driver.h    |  26 ++++----
> >   lib/librte_ethdev/version.map          |   8 +--
> 
> Isn't there any documentation to update with this change?

Do you mean the release note? I only updated the 20_01 release note part.

> 
> If the shared action API is not documented at all, can you please
> add documentation for the action handle API?

In the testpmd usage guide, I found some description and update it together with the testpmd update.
I will check if there is another place to update or add.

Thanks

BR. Bing
  

> -----Original Message-----
> From: Bing Zhao <bingz@nvidia.com>
> Sent: Saturday, April 10, 2021 5:04 PM
> To: Ori Kam <orika@nvidia.com>; NBU-Contact-Thomas Monjalon
> <thomas@monjalon.net>; ferruh.yigit@intel.com;
> andrew.rybchenko@oktetlabs.ru; Matan Azrad <matan@nvidia.com>; Slava
> Ovsiienko <viacheslavo@nvidia.com>
> Cc: dev@dpdk.org; ajit.khaparde@broadcom.com; Gregory Etelson
> <getelson@nvidia.com>; Andrey Vesnovaty <andreyv@nvidia.com>
> Subject: [PATCH v2 1/4] ethdev: introduce indirect action APIs
> 
> Right now, rte_flow_shared_action_* APIs are used for some shared
> actions, like RSS, count. The shared action should 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 parameter 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           |  56 ++++++++--------
>  lib/librte_ethdev/rte_flow.h           | 118 +++++++++++++++++++--------------
>  lib/librte_ethdev/rte_flow_driver.h    |  26 ++++----
>  lib/librte_ethdev/version.map          |   8 +--
>  5 files changed, 115 insertions(+), 96 deletions(-)
> 
> diff --git a/doc/guides/rel_notes/release_21_05.rst
> b/doc/guides/rel_notes/release_21_05.rst
> index 374d6d9..6c0ac46 100644
> --- a/doc/guides/rel_notes/release_21_05.rst
> +++ b/doc/guides/rel_notes/release_21_05.rst
> @@ -164,6 +164,9 @@ API Changes
>    from ``rte_thread_tls_*`` to ``rte_thread_*`` to avoid naming redundancy
>    and confusion with the transport layer security term.
> 
> +* 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 e07e617..27a1615 100644
> --- a/lib/librte_ethdev/rte_flow.c
> +++ b/lib/librte_ethdev/rte_flow.c
> @@ -180,12 +180,12 @@ struct rte_flow_desc_data {
>  	MK_FLOW_ACTION(MODIFY_FIELD,
>  		       sizeof(struct rte_flow_action_modify_field)),
>  	/**
> -	 * Shared action represented as handle of type
> -	 * (struct rte_flow_shared action *) stored in conf field (see
> +	 * Indirect action represented as handle of type
> +	 * (struct rte_flow_action_handle *) stored in conf field (see
>  	 * struct rte_flow_action); no need for additional structure to * store
> -	 * shared action handle.
> +	 * indirect action handle.
>  	 */
> -	MK_FLOW_ACTION(SHARED, 0),
> +	MK_FLOW_ACTION(INDIRECT, 0),
>  };
> 
>  int
> @@ -1067,53 +1067,53 @@ enum rte_flow_conv_item_spec_type {
>  				  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 @@ struct rte_flow_shared_action *
> 
>  	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 @@ struct rte_flow_shared_action *
> 
>  	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 6cc5713..91ae25b 100644
> --- a/lib/librte_ethdev/rte_flow.h
> +++ b/lib/librte_ethdev/rte_flow.h
> @@ -1821,7 +1821,7 @@ enum rte_flow_action_type {
>  	 * Enables counters for this flow rule.
>  	 *
>  	 * These counters can be retrieved and reset through rte_flow_query()
> or
> -	 * rte_flow_shared_action_query() if the action provided via handle,
> +	 * rte_flow_action_handle_query() if the action provided via handle,
>  	 * see struct rte_flow_query_count.
>  	 *
>  	 * See struct rte_flow_action_count.
> @@ -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,
>  };
> 
>  /**
> @@ -2357,7 +2367,7 @@ struct rte_flow_query_age {
>   * ``struct rte_flow_query_count``.
>   *
>   * @deprecated Shared attribute is deprecated, use generic
> - * RTE_FLOW_ACTION_TYPE_SHARED action.
> + * RTE_FLOW_ACTION_TYPE_INDIRECT action.
>   *
>   * The shared flag indicates whether the counter is unique to the flow rule the
>   * action is specified with, or whether it is a shared counter.
> @@ -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 @@ struct rte_flow_desc {
>  			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 @@ struct rte_flow_shared_action *
>   * @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 @@ struct rte_flow_shared_action *
>   */
>  __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 @@ struct rte_flow_shared_action *
>   *   - (-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 direct 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 @@ struct rte_flow_shared_action *
>   */
>  __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 da594d9..8d825eb 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 93ad388..4eb561a 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 {
> --
> 1.8.3.1


Acked-by: Andrey Vesnovaty <andreyv@nvidia.com>
  

> -----Original Message-----
> From: Bing Zhao <bingz@nvidia.com>
> Sent: Tuesday, April 13, 2021 4:27 AM
> Subject: RE: [PATCH v2 1/4] ethdev: introduce indirect action APIs
> 
> Hi Ferruh,
> 
> > -----Original Message-----
> > From: Ferruh Yigit <ferruh.yigit@intel.com>
> > Sent: Tuesday, April 13, 2021 3:42 AM
> > To: Bing Zhao <bingz@nvidia.com>; Ori Kam <orika@nvidia.com>; NBU-
> > Contact-Thomas Monjalon <thomas@monjalon.net>;
> > andrew.rybchenko@oktetlabs.ru; Matan Azrad <matan@nvidia.com>; Slava
> > Ovsiienko <viacheslavo@nvidia.com>
> > Cc: dev@dpdk.org; ajit.khaparde@broadcom.com; Gregory Etelson
> > <getelson@nvidia.com>; Andrey Vesnovaty <andreyv@nvidia.com>
> > Subject: Re: [PATCH v2 1/4] ethdev: introduce indirect action APIs
> >
> > External email: Use caution opening links or attachments
> >
> >
> > On 4/10/2021 3:03 PM, Bing Zhao wrote:
> > > Right now, rte_flow_shared_action_* APIs are used for some shared
> > > actions, like RSS, count. The shared action should 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 parameter 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           |  56 ++++++++--------
> > >   lib/librte_ethdev/rte_flow.h           | 118
> > +++++++++++++++++++--------------
> > >   lib/librte_ethdev/rte_flow_driver.h    |  26 ++++----
> > >   lib/librte_ethdev/version.map          |   8 +--
> >
> > Isn't there any documentation to update with this change?
> 
> Do you mean the release note? I only updated the 20_01 release note part.
> 
You also need to update the prog/guid/rte_flow.rst

> >
> > If the shared action API is not documented at all, can you please
> > add documentation for the action handle API?
> 
> In the testpmd usage guide, I found some description and update it together
> with the testpmd update.
> I will check if there is another place to update or add.
> 
> Thanks
> 
> BR. Bing
  

On 4/10/21 5:03 PM, Bing Zhao wrote:
> Right now, rte_flow_shared_action_* APIs are used for some shared
> actions, like RSS, count. The shared action should 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 parameter 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>

Just for the record. I still dislike renaming since "shared" highlights
purpose (what is definitely better), but "indirect" is just a technical
detail on how it is done.
  

15/04/2021 15:55, Andrew Rybchenko:
> On 4/10/21 5:03 PM, Bing Zhao wrote:
> > Right now, rte_flow_shared_action_* APIs are used for some shared
> > actions, like RSS, count. The shared action should 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 parameter 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>
> 
> Just for the record. I still dislike renaming since "shared" highlights
> purpose (what is definitely better), but "indirect" is just a technical
> detail on how it is done.

The difference is that indirect action is not only for sharing.
It allows manipulating an action as an object.
Action object is inserted in flow rules through the indirect action.
Does it make it clearer?
  

On 4/15/21 5:10 PM, Thomas Monjalon wrote:
> 15/04/2021 15:55, Andrew Rybchenko:
>> On 4/10/21 5:03 PM, Bing Zhao wrote:
>>> Right now, rte_flow_shared_action_* APIs are used for some shared
>>> actions, like RSS, count. The shared action should 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 parameter 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>
>>
>> Just for the record. I still dislike renaming since "shared" highlights
>> purpose (what is definitely better), but "indirect" is just a technical
>> detail on how it is done.
> 
> The difference is that indirect action is not only for sharing.
> It allows manipulating an action as an object.
> Action object is inserted in flow rules through the indirect action.
> Does it make it clearer?
> 

Yes, I see thanks.