From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mails.dpdk.org (mails.dpdk.org [217.70.189.124]) by inbox.dpdk.org (Postfix) with ESMTP id EE951A0C47; Tue, 12 Oct 2021 12:41:08 +0200 (CEST) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id BCDAE41143; Tue, 12 Oct 2021 12:41:08 +0200 (CEST) Received: from shelob.oktetlabs.ru (shelob.oktetlabs.ru [91.220.146.113]) by mails.dpdk.org (Postfix) with ESMTP id 3F7C641141 for ; Tue, 12 Oct 2021 12:41:07 +0200 (CEST) Received: from [192.168.38.17] (aros.oktetlabs.ru [192.168.38.17]) (using TLSv1.3 with cipher TLS_AES_128_GCM_SHA256 (128/128 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by shelob.oktetlabs.ru (Postfix) with ESMTPSA id AAFF17F567; Tue, 12 Oct 2021 13:41:06 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 shelob.oktetlabs.ru AAFF17F567 DKIM-Signature: v=1; a=rsa-sha256; c=simple/simple; d=oktetlabs.ru; s=default; t=1634035266; bh=KyH1RI2RpF4N/6z+E2pFzGHgB6Jgp2KeNS26XXbzae8=; h=Subject:To:Cc:References:From:Date:In-Reply-To; b=YWevt/a22zHjgoH+jc25bjSa28NCKvHgZ3JpSP9jONI+UZudzs+SAlI/mODx+S6m2 nUY+u0S2trf8i8dfpsXowG525zSM4v2wQmyHzkKsn406jah78SjDM1kHW425lWqrJn cTo7pk+OsLxM4CVzpCRSYYk1Ar7fr1tnTSqkNo9Q= To: Ori Kam , Dmitry Kozlyuk , Ajit Khaparde Cc: dpdk-dev , Matan Azrad , NBU-Contact-Thomas Monjalon , Ferruh Yigit References: <20210901085516.3647814-1-dkozlyuk@nvidia.com> <20210901085516.3647814-3-dkozlyuk@nvidia.com> <5b4d7e66-80f3-8099-2a81-ea6e20ec70ba@oktetlabs.ru> <28f4a819-a9c7-e363-991b-b78f89fcf162@oktetlabs.ru> From: Andrew Rybchenko Organization: OKTET Labs Message-ID: Date: Tue, 12 Oct 2021 13:41:06 +0300 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.14.0 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 8bit Subject: Re: [dpdk-dev] [RFC PATCH 2/2] ethdev: add capability to keep indirect actions on restart X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" On 10/12/21 1:26 PM, Ori Kam wrote: > Hi > >> -----Original Message----- >> From: Andrew Rybchenko >> 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 >>>> 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 >>>>>> Sent: 6 октября 2021 г. 20:13 >>>>>> To: Dmitry Kozlyuk >>>>>> Cc: dpdk-dev ; Matan Azrad ; Ori >>>>>> Kam ; NBU-Contact-Thomas Monjalon >>>>>> ; Ferruh Yigit ; >>>>>> Andrew Rybchenko >>>>>> 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 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?