Re: [dpdk-dev] [RFC PATCH 2/2] ethdev: add capability to keep indirect actions on restart

[Ori Kam <orika@nvidia.com>]

Hi

> -----Original Message-----
> From: Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>
> Sent: Tuesday, October 12, 2021 12:15 PM
> Subject: Re: [dpdk-dev] [RFC PATCH 2/2] ethdev: add capability to keep indirect actions on restart
> 
> On 10/11/21 6:53 PM, Ori Kam wrote:
> > Hi Andrew and Ajit,
> >
> >> -----Original Message-----
> >> From: Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>
> >> Sent: Monday, October 11, 2021 4:58 PM
> >> Subject: Re: [dpdk-dev] [RFC PATCH 2/2] ethdev: add capability to
> >> keep indirect actions on restart
> >>
> >> On 10/7/21 11:16 AM, Dmitry Kozlyuk wrote:
> >>>> -----Original Message-----
> >>>> From: Ajit Khaparde <ajit.khaparde@broadcom.com>
> >>>> Sent: 6 октября 2021 г. 20:13
> >>>> To: Dmitry Kozlyuk <dkozlyuk@nvidia.com>
> >>>> Cc: dpdk-dev <dev@dpdk.org>; Matan Azrad <matan@nvidia.com>; Ori
> >>>> Kam <orika@nvidia.com>; NBU-Contact-Thomas Monjalon
> >>>> <thomas@monjalon.net>; Ferruh Yigit <ferruh.yigit@intel.com>;
> >>>> Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>
> >>>> Subject: Re: [dpdk-dev] [RFC PATCH 2/2] ethdev: add capability to
> >>>> keep indirect actions on restart
> >>>>
> >>>> On Wed, Sep 1, 2021 at 1:55 AM Dmitry Kozlyuk <dkozlyuk@nvidia.com> wrote:
> >>>>>
> >>>>> rte_flow_action_handle_create() did not mention what happens with
> >>>>> an indirect action when a device is stopped, possibly
> >>>>> reconfigured, and started again. It is natural for some indirect
> >>>>> actions to be persistent, like counters and meters; keeping others
> >>>>> just saves application time and complexity. However, not all PMDs can support it.
> >>>>> It is proposed to add a device capability to indicate if indirect
> >>>>> actions are kept across the above sequence or implicitly destroyed.
> >>>>>
> >>>>> It may happen that in the future a PMD acquires support for a type
> >>>>> of indirect actions that it cannot keep across a restart. It is
> >>>>> undesirable to stop advertising the capability so that
> >>>>> applications that don't use actions of the problematic type can still take advantage of it.
> >>>>> This is why PMDs are allowed to keep only a subset of indirect
> >>>>> actions provided that the vendor mandatorily documents it.
> >>>> Sorry - I am seeing this late.
> >>>> This could become confusing.
> >>>> May be it is better for the PMDs to specify which actions are persistent.
> >>>> How about adding a bit for the possible actions of interest.
> >>>> And then PMDs can set bits for actions which can be persistent
> >>>> across stop, start and reconfigurations?
> >>>
> >>> This approach was considered, but there is a risk of quickly running
> >>> out of capability bits. Each action
> >> would consume one bit plus as many bits as there are special
> >> conditions for it in all the PMDs, because conditions are likely to
> >> be PMD-specific. And the application will anyway need to consider
> >> specific conditions to know which bit to test, so the meaning of the bits will be PMD-specific. On the
> other hand, PMDs are not expected to exercise this loophole unless absolutely needed.
> >>>
> > Right those bits should be considered as master bits and are not per actions.
> > If there is specific case for a PMD it should solve it by documation or other means.
> 
> Documentation does not solve the problem since it can't be automated. So, it just help to solve case-by-
> case.

I agree that documentation can't be automated, I think this is just like other edge cases that can't be checked
for example you can reconfigure the device after start except the queue number or queue size (just an example)
The metrix of actions/items/pmds I don't think we will ever be able to have an easy way to check capabilities.

Maybe we can say that if PMD reports that it supports keeping the actions, and it can't support just one of the actions
it can fail or issue a special error code when calling stop. To let the application know that something was incorrect.
In this case application can create a sample of the action it requires and then call the stop. If it fails it can try again until
he gets no error, and only then start. What do you think?

Another way is to assume that if the action was created before port start it will be kept after port stop.

And this bit is just for letting the application know if it is worth to check.
 
> 
> >
> >>
> >> May be we should separate at least transfer and non-transfer rules?
> >> Transfer rules are less configuration dependent.
> >
> > May be I'm missing something but jut like stated above those are
> > master bits I don't see much use case where the PMD can store transfer
> > rules but not other rules. I assume  that if the application uses the transfer mode most of the flows will be
> in the transfer domain.
> 
> Most likely different HW blocks are responsible for transfer and non-transfer rules. So, I can easily imagine
> that one could be preserved across restart, but another can't.
> 

I don't know, but in our case this is the same block.
since a lot of the action are the same between the eswitch and the ethdev I would expect that the limitation will be the same.
how is it in your case?

> Anyway, I'm just trying to understand. Not a blocker.
> 
> Also have you considered to make it controllable by the application. I.e. PMD advertises a capability and it
> is responsibility of the application to use it or not.
> May be it is excessive. In theory application can check the flag and do flush before or just after stop if it
> does not want to preserve rules.
> 

I'm not sure I understand this comment, The application is always free to use or not use a capability this is 
just to let the application know that if it doesn't want to destroy the action before stop he doesn't have to
and the action will be saved.


> Andrew.

Ori.