Re: [dpdk-dev] [PATCH] add flow shared action API

[Jerin Jacob <jerinjacobk@gmail.com>]

On Sat, Jul 4, 2020 at 3:40 PM Andrey Vesnovaty
<andrey.vesnovaty@gmail.com> wrote:
>
>
> Thanks,
>
> Andrey Vesnovaty
> (+972)526775512 | Skype: andrey775512
>
>
> On Fri, Jul 3, 2020 at 6:02 PM Jerin Jacob <jerinjacobk@gmail.com> wrote:
>>
>> On Fri, Jul 3, 2020 at 8:07 PM Andrey Vesnovaty <andreyv@mellanox.com> wrote:
>> >
>> > From: Andrey Vesnovaty <andrey.vesnovaty@gmail.com>
>> >
>> > 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 effects 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 of of multiple
>> >   flows
>> >
>> > Change description
>> > ===
>> >
>> > Shared action
>> > ===
>> > In order to represent flow action shared by multiple flows new action
>> > type RTE_FLOW_ACTION_TYPE_SHARED 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 for multiple flows.
>> >
>> > Shared action create/use/destroy
>> > ===
>> > Shared action may be reused by some or none flow rules at any given
>> > moment, IOW shared action reside outside of the context of any flow.
>> > Shared action represent HW resources/objects used for action offloading
>> > implementation. For allocation/release of all HW resources and all
>> > related initializations/cleanups in PMD space required for shared action
>> > implementation added new API
>> > rte_flow_shared_action_create()/rte_flow_shared_action_destroy().
>> > In addition to the above all preparations needed to maintain shared
>> > access to the action resources, configuration and state should be done in
>> > rte_flow_shared_action_create().
>> >
>> > 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 i.e. result of the usage is undefined.
>> >
>> > Shared action re-configuration
>> > ===
>> > Shared action behavior defined by its configuration & and can be updated
>> > via rte_flow_shared_action_update() (see "example" section). The shared
>> > action update operation modifies HW related resources/objects allocated
>> > by the action. The number of operations performed by the update operation
>> > should not be dependent on number of flows sharing the related action.
>> > On return of shared action updated 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 sate (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.
>> >
>> > 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 create {port_id} [index] {action}
>> > flow shared_action update {port_id} {index} {action}
>> > flow shared_action destroy {port_id} {index}
>> > flow shared_action query {port_id} {index}
>> >
>> > testpmd example
>> > ===
>> >
>> > configure rss to queues 1 & 2
>> >
>> > testpmd> flow shared_action create 0 100 rss 1 2
>> >
>> > create flow rule utilizing shared action
>> >
>> > testpmd> flow create 0 ingress \
>> >     pattern eth dst is 0c:42:a1:15:fd:ac / ipv6 / tcp / end \
>> >   actions shared 100 end / end
>> >
>> > add 2 more queues
>> >
>> > testpmd> flow shared_action modify 0 100 rss 1 2 3 4
>> >
>> > example
>> > ===
>> >
>> > struct rte_flow_action actions[2];
>> > struct rte_flow_action action;
>> > /* skipped: initialize action */
>> > struct rte_flow_shared_action *handle = rte_flow_shared_action_create(
>> >                                         port_id, &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.conf,
>> >                                 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@mellanox.com>
>> > Signed-off-by: Andrey Vesnovaty <andrey.vesnovaty@gmail.com>
>> > Signed-off-by: Andrey Vesnovaty <andreyv@mellanox.com>
>>
>> Duplicate Signoffs.
>>
>> # Request to CC all the people who are already giving comments to this patch.
>> # The Following comment is not addressed in this patch.
>> http://mails.dpdk.org/archives/dev/2020-July/172408.html
>
>
> I need to mention the locking issue once again.
> If there is a need to maintain "shared session" in the generic rte_flow layer all
> calls to flow_create() with shared action & all delete need to take sharedsession
> management locks at least for verification. Lock partitioning is also bit problematic
> since one flow may have more than one shared action.

Then, I think better approach would be to introduce
rte_flow_action_update() public
API which can either take "const struct rte_flow_action []" OR shared
context ID, to cater to
both cases or something on similar lines. This would allow HW's
without have  the shared context ID
to use the action update.