Re: [dpdk-dev] [PATCH v7 1/2] ethdev: add flow shared action API

[Ajit Khaparde <ajit.khaparde@broadcom.com>]

On Thu, Oct 8, 2020 at 4:51 AM Andrey Vesnovaty <andreyv@nvidia.com> wrote:
>
> This commit introduces extension of DPDK flow action API enabling
> sharing of single rte_flow_action in multiple flows. The API intended for
> PMDs, where multiple HW offloaded flows can reuse the same HW
> essence/object representing flow action and modification of such an
> essence/object affects all the rules using it.
>
> Motivation and example
> ===
> Adding or removing one or more queues to RSS used by multiple flow rules
> imposes per rule toll for current DPDK flow API; the scenario requires
> for each flow sharing cloned RSS action:
> - call `rte_flow_destroy()`
> - call `rte_flow_create()` with modified RSS action
>
> API for sharing action and its in-place update benefits:
> - reduce the overhead of multiple RSS flow rules reconfiguration
> - optimize resource utilization by sharing action across multiple
>   flows
>
> Change description
> ===
>
> Shared action
> ===
> In order to represent flow action shared by multiple flows new action
> type RTE_FLOW_ACTION_TYPE_SHARED is introduced (see `enum
> rte_flow_action_type`).
> Actually the introduced API decouples action from any specific flow and
> enables sharing of single action by its handle across multiple flows.
>
> Shared action create/use/destroy
> ===
> Shared action may be reused by some or none flow rules at any given
> moment, i.e. shared action resides outside of the context of any flow.
> Shared action represent HW resources/objects used for action offloading
> implementation.
> API for shared action create (see `rte_flow_shared_action_create()`):
> - should allocate HW resources and make related initializations required
>   for shared action implementation.
> - make necessary preparations to maintain shared access to
>   the action resources, configuration and state.
> API for shared action destroy (see `rte_flow_shared_action_destroy()`)
> should release HW resources and make related cleanups required for shared
> action implementation.
>
> In order to share some flow action reuse the handle of type
> `struct rte_flow_shared_action` returned by
> rte_flow_shared_action_create() as a `conf` field of
> `struct rte_flow_action` (see "example" section).
>
> If some shared action not used by any flow rule all resources allocated
> by the shared action can be released by rte_flow_shared_action_destroy()
> (see "example" section). The shared action handle passed as argument to
> destroy API should not be used any further i.e. result of the usage is
> undefined.
>
> Shared action re-configuration
> ===
> Shared action behavior defined by its configuration can be updated via
> rte_flow_shared_action_update() (see "example" section). The shared
> action update operation modifies HW related resources/objects allocated
> on the action creation. The number of operations performed by the update
> operation should not depend on the number of flows sharing the related
> action. On return of shared action update API action behavior should be
> according to updated configuration for all flows sharing the action.
>
> Shared action query
> ===
> Provide separate API to query shared action state (see
> rte_flow_shared_action_update()). Taking a counter as an example: query
> returns value aggregating all counter increments across all flow rules
> sharing the counter. This API doesn't query shared action configuration
> since it is controlled by rte_flow_shared_action_create() and
> rte_flow_shared_action_update() APIs and no supposed to change by other
> means.
>
> PMD support
> ===
> The support of introduced API is pure PMD specific design and
> responsibility for each action type (see struct rte_flow_ops).
>
> testpmd
> ===
> In order to utilize introduced API testpmd cli may implement following
> extension
> create/update/destroy/query shared action accordingly
>
> flow shared_action (port) create {action_id (id)} (action) / end
> flow shared_action (port) update (id) (action) / end
> flow shared_action (port) destroy action_id (id) {action_id (id) [...]}
> flow shared_action (port) query (id)
>
> testpmd example
> ===
>
> configure rss to queues 1 & 2
>
> > flow shared_action 0 create action_id 100 rss queues 1 2 end / end
>
> create flow rule utilizing shared action
>
> > flow create 0 ingress \
>     pattern eth dst is 0c:42:a1:15:fd:ac / ipv6 / tcp / end \
>   actions shared 100 / end
>
> add 2 more queues
>
> > flow shared_action 0 modify 100 rss queues 1 2 3 4 end / end
>
> example
> ===
>
> struct rte_flow_action actions[2];
> struct rte_flow_shared_action_conf conf;
> struct rte_flow_action action;
> /* skipped: initialize conf and action */
> struct rte_flow_shared_action *handle =
>         rte_flow_shared_action_create(port_id, &conf, &action, &error);
> actions[0].type = RTE_FLOW_ACTION_TYPE_SHARED;
> actions[0].conf = handle;
> actions[1].type = RTE_FLOW_ACTION_TYPE_END;
> /* skipped: init attr0 & pattern0 args */
> struct rte_flow *flow0 = rte_flow_create(port_id, &attr0, pattern0,
>                                         actions, error);
> /* create more rules reusing shared action */
> struct rte_flow *flow1 = rte_flow_create(port_id, &attr1, pattern1,
>                                         actions, error);
> /* skipped: for flows 2 till N */
> struct rte_flow *flowN = rte_flow_create(port_id, &attrN, patternN,
>                                         actions, error);
> /* update shared action */
> struct rte_flow_action updated_action;
> /*
>  * skipped: initialize updated_action according to desired action
>  * configuration change
>  */
> rte_flow_shared_action_update(port_id, handle, &updated_action, error);
> /*
>  * from now on all flows 1 till N will act according to configuration of
>  * updated_action
>  */
> /* skipped: destroy all flows 1 till N */
> rte_flow_shared_action_destroy(port_id, handle, error);
>
> Signed-off-by: Andrey Vesnovaty <andreyv@nvidia.com>
> Acked-by: Ori Kam <orika@nvidia.com>
Since this is an ethdev patch, the testpmd description is really not required.
Moreover they are not in sync with the direction and other changes you made
in the testpmd patch. Also there is a typo inline. Other than that..

Acked-by: Ajit Khaparde <ajit.khaparde@broadcom.com>

> ---
>  doc/guides/prog_guide/rte_flow.rst       |  19 +++
>  doc/guides/rel_notes/release_20_11.rst   |   9 ++
>  lib/librte_ethdev/rte_ethdev_version.map |   4 +
>  lib/librte_ethdev/rte_flow.c             |  84 +++++++++++
>  lib/librte_ethdev/rte_flow.h             | 169 ++++++++++++++++++++++-
>  lib/librte_ethdev/rte_flow_driver.h      |  23 +++
>  6 files changed, 307 insertions(+), 1 deletion(-)
>
[snip]

> +
> +/**
> + * RTE_FLOW_ACTION_TYPE_SHARED
> + *
> + * Opaque type returned after successfully creating a shared action.
> + *
> + * 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
> + */
> +struct rte_flow_shared_action;
> +
>  /* Mbuf dynamic field offset for metadata. */
>  extern int32_t rte_flow_dynf_metadata_offs;
>
> @@ -3357,6 +3380,150 @@ int
>  rte_flow_get_aged_flows(uint16_t port_id, void **contexts,
>                         uint32_t nb_contexts, struct rte_flow_error *error);
>
> +/**
> + * Specify shared action configuration
> + */
> +struct rte_flow_shared_action_conf {
> +       /**
> +        * Flow direction for shared action configuration.
> +        *
> +        * Shred action should be valid at least for one flow direction,
s/Shred/Shared

> +        * 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. */
> +};
[snip]