Re: [dpdk-dev] [PATCH v3 1/1] ethdev: introduce indirect action APIs

[Bing Zhao <bingz@nvidia.com>]

Hi Ajit,

> -----Original Message-----
> From: Ajit Khaparde <ajit.khaparde@broadcom.com>
> Sent: Friday, April 16, 2021 2:00 AM
> To: Bing Zhao <bingz@nvidia.com>
> Cc: Ori Kam <orika@nvidia.com>; NBU-Contact-Thomas Monjalon
> <thomas@monjalon.net>; Ferruh Yigit <ferruh.yigit@intel.com>; Andrew
> Rybchenko <andrew.rybchenko@oktetlabs.ru>; Matan Azrad
> <matan@nvidia.com>; Slava Ovsiienko <viacheslavo@nvidia.com>; dpdk-
> dev <dev@dpdk.org>; Gregory Etelson <getelson@nvidia.com>; Andrey
> Vesnovaty <andreyv@nvidia.com>
> Subject: Re: [PATCH v3 1/1] ethdev: introduce indirect action APIs
> 
> On Thu, Apr 15, 2021 at 8:52 AM Bing Zhao <bingz@nvidia.com> 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.
> >
> > Since the APIs are changed from "rte_flow_shared_action*" to the
> new
> > "rte_flow_action_handle*", the testpmd application code and
> command
> > line interfaces also need to be updated to do the adaption.
> > The testpmd application user guide is also updated. All the
> "shared
> > action" related parts are replaced with "indirect action" to have
> a
> > correct explanation.
> >
> > 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.
> When I read this somehow indirect did not feel right.
> But I don't have a strong suggestion either.
> Since it is a context of action or actions maybe we use
> action_context?
> 

The original RFC use the naming proposal "action_context", then there were some comments that it was not as good as "shared action".
The "indirect action" may describe the behavior of the action better than "shared" to my understanding.

In the email from Thomas, the comments may explain it better than mine:
" 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?"

What do you think?

> >
> > Since the APIs are changed from "rte_flow_shared_action*" to the
> new
> > "rte_flow_action_handle*" and the "update" interface's 3rd input
> > parameter is changed to generic pointer, the mlx5 PMD that uses
> these
> > APIs needs to do the adaption to the new APIs as well.
> >
> > Signed-off-by: Bing Zhao <bingz@nvidia.com>
> > Acked-by: Andrey Vesnovaty <andreyv@nvidia.com>

BR. Bing