> -----Original Message-----
> From: Bing Zhao <bi...@nvidia.com>
> Sent: Saturday, April 10, 2021 5:04 PM
> To: Ori Kam <or...@nvidia.com>; NBU-Contact-Thomas Monjalon
> <tho...@monjalon.net>; ferruh.yi...@intel.com;
> andrew.rybche...@oktetlabs.ru; Matan Azrad <ma...@nvidia.com>; Slava
> Ovsiienko <viachesl...@nvidia.com>
> Cc: dev@dpdk.org; ajit.khapa...@broadcom.com; Gregory Etelson
> <getel...@nvidia.com>; Andrey Vesnovaty <andr...@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 <bi...@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 <andr...@nvidia.com>
