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?
