Please let's continue in the thread for the latest version of the patches: http://inbox.dpdk.org/dev/ch0pr12mb5091792a77cbd1528db7a005b9...@ch0pr12mb5091.namprd12.prod.outlook.com/
Please see my comments there. > -----Original Message----- > From: Andrew Rybchenko <andrew.rybche...@oktetlabs.ru> > Sent: 12 октября 2021 г. 13:41 > To: Ori Kam <or...@nvidia.com>; Dmitry Kozlyuk <dkozl...@nvidia.com>; Ajit > Khaparde <ajit.khapa...@broadcom.com> > Cc: dpdk-dev <dev@dpdk.org>; Matan Azrad <ma...@nvidia.com>; NBU- > Contact-Thomas Monjalon <tho...@monjalon.net>; Ferruh Yigit > <ferruh.yi...@intel.com> > Subject: Re: [dpdk-dev] [RFC PATCH 2/2] ethdev: add capability to keep > indirect > actions on restart > > External email: Use caution opening links or attachments > > > On 10/12/21 1:26 PM, Ori Kam wrote: > > Hi > > > >> -----Original Message----- > >> From: Andrew Rybchenko <andrew.rybche...@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.rybche...@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.khapa...@broadcom.com> > >>>>>> Sent: 6 октября 2021 г. 20:13 > >>>>>> To: Dmitry Kozlyuk <dkozl...@nvidia.com> > >>>>>> Cc: dpdk-dev <dev@dpdk.org>; Matan Azrad <ma...@nvidia.com>; Ori > >>>>>> Kam <or...@nvidia.com>; NBU-Contact-Thomas Monjalon > >>>>>> <tho...@monjalon.net>; Ferruh Yigit <ferruh.yi...@intel.com>; > >>>>>> Andrew Rybchenko <andrew.rybche...@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 <dkozl...@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? > > It all sounds complicated. Do we really need it? > > > Another way is to assume that if the action was created before port start > > it will > be kept after port stop. > > It does not sound like a solution. May be I simply don't know target usecase. > > > 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? > > Actions or rules? QUEUE and RSS are non-transfer actions. > PORT_ID etc are transfer actions which do not make sense in non-transfer case. > DROP, COUNT and MARK make sense in both cases. Packet edits make sense in > both cases as well. > > > > >> 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. > > Hm, who said that application must explicitly destroy rules/actions before > stop?
