From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from dpdk.org (dpdk.org [92.243.14.124]) by inbox.dpdk.org (Postfix) with ESMTP id DB112A04B1; Tue, 24 Nov 2020 14:00:39 +0100 (CET) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id 9718AC912; Tue, 24 Nov 2020 14:00:37 +0100 (CET) Received: from shelob.oktetlabs.ru (shelob.oktetlabs.ru [91.220.146.113]) by dpdk.org (Postfix) with ESMTP id 9A2C8C90E for ; Tue, 24 Nov 2020 14:00:35 +0100 (CET) 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 1C0257F53A; Tue, 24 Nov 2020 16:00:34 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 shelob.oktetlabs.ru 1C0257F53A DKIM-Signature: v=1; a=rsa-sha256; c=simple/simple; d=oktetlabs.ru; s=default; t=1606222834; bh=qyaDsZB5hbpAAaiSFl6otYrqZcoIe7DF6PpQlXCPTt8=; h=Subject:To:Cc:References:From:Date:In-Reply-To; b=PDDgU9Y4/wEI5AQESUxdjK28bpdPAd94agLvpywQZ7A1C+8maUnkThDxR92oYQvJS pheVX+DQdXwHAhuLBvo7pur/SaS9t1fgt8YKfESQHqvDxAl97+z3gQJA9L/OwolfeX 8Onea01iwoH7R1NekvbiDV1VnGCTmlhcJDA1RFU0= To: Ferruh Yigit , Ori Kam , Ray Kinsella , Neil Horman Cc: "dev@dpdk.org" , NBU-Contact-Thomas Monjalon References: <20201123134007.2870297-1-ferruh.yigit@intel.com> <148925d7-2ebd-fcbb-0d7c-6cbe3d15a6de@oktetlabs.ru> <6730d62f-6f18-0137-8e7f-140fa564c2d5@oktetlabs.ru> From: Andrew Rybchenko Organization: OKTET Labs Message-ID: <44812e16-93b2-81c6-71ed-c2a0e79b9f16@oktetlabs.ru> Date: Tue, 24 Nov 2020 16:00:33 +0300 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.4.0 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit Content-Language: en-US Subject: Re: [dpdk-dev] [PATCH] doc: announce flow API matching pattern struct changes X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 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 11/24/20 3:56 PM, Ferruh Yigit wrote: > On 11/24/2020 11:43 AM, Ori Kam wrote: >> Hi >> >>> -----Original Message----- >>> From: Ferruh Yigit >>> Sent: Monday, November 23, 2020 5:51 PM >>> Subject: Re: [PATCH] doc: announce flow API matching pattern struct >>> changes >>> >>> On 11/23/2020 2:25 PM, Andrew Rybchenko wrote: >>>> On 11/23/20 5:17 PM, Ferruh Yigit wrote: >>>>> On 11/23/2020 1:50 PM, Andrew Rybchenko wrote: >>>>>> On 11/23/20 4:40 PM, Ferruh Yigit wrote: >>>>>>> Proposing to replace protocol header fields in the >>>>>>> ``rte_flow_item_*`` >>>>>>> structures with the protocol structs, like: >>>>>>> >>>>>>> Current ``struct rte_flow_item_eth``, >>>>>>> >>>>>>> struct rte_flow_item_eth { >>>>>>>       struct rte_ether_addr dst; >>>>>>>       struct rte_ether_addr src; >>>>>>>       rte_be16_t type; >>>>>>>       uint32_t has_vlan:1; >>>>>>>       uint32_t reserved:31; >>>>>>> } >>>>>>> >>>>>>> will become >>>>>>> >>>>>>> struct rte_flow_item_eth { >>>>>>>       struct rte_ether_hdr hdr; >>>>>>>       uint32_t has_vlan:1; >>>>>>>       uint32_t reserved:31; >>>>>>> } >>>>>>> >>>>>>> This is both for documenting the intention and to be sure >>>>>>> ``rte_flow_item_*`` always starts with complete protocol header. >>>>>>> >>>>>>> Already many ``rte_flow_item_*`` structs implemented to have >>>>>>> protocol >>>>>>> struct, target is convert all to this usage. >>>>>>> >>>>>>> Signed-off-by: Ferruh Yigit >>>>>> >>>>>> Acked-by: Andrew Rybchenko >>>>>> >>>>>> a minor note below >>>>>> >>>>>>> --- >>>>>>> Cc: Thomas Monjalon >>>>>>> Cc: Andrew Rybchenko >>>>>>> Cc: Ori Kam >>>>>>> --- >>>>>>>     doc/guides/rel_notes/deprecation.rst | 7 +++++++ >>>>>>>     1 file changed, 7 insertions(+) >>>>>>> >>>>>>> diff --git a/doc/guides/rel_notes/deprecation.rst >>>>>>> b/doc/guides/rel_notes/deprecation.rst >>>>>>> index 96986fabd598..a2fa0c196472 100644 >>>>>>> --- a/doc/guides/rel_notes/deprecation.rst >>>>>>> +++ b/doc/guides/rel_notes/deprecation.rst >>>>>>> @@ -88,6 +88,13 @@ Deprecation Notices >>>>>>>       will be limited to maximum 256 queues. >>>>>>>       Also compile time flag ``RTE_ETHDEV_QUEUE_STAT_CNTRS`` >>>>>>> will be >>>>>>> removed. >>>>>>>     +* ethdev: The flow API matching pattern structures, ``struct >>>>>>> rte_flow_item_*``, >>>>>>> +  should start with relevant protocol header. >>>>>>> +  Some matching pattern structures implements this by duplicating >>>>>>> protocol header >>>>>>> +  fields in the struct. To clarify the intention and to be sure >>>>>>> protocol header >>>>>>> +  is intact, will replace those fields with relevant protocol >>>>>>> header struct. >>>>>>> +  Target is v21.02 release and this should not change the ABI. >>>>>>> + >>>>>>>     * sched: To allow more traffic classes, flexible mapping of >>>>>>> pipe >>>>>>> queues to >>>>>>>       traffic classes, and subport level configuration of pipes and >>>>>>> queues >>>>>>>       changes will be made to macros, data structures and API >>>>>>> functions defined >>>>>>> >>>>>> >>>>>> Just want to highlight that even API could be kept using >>>>>> unnamed union for hdr and unnamed structure for existing >>>>>> protocol header fields. >>>>>> >>>>> >>>>> Then we may never clean the protocol header fields out of it, >>>>> yes this will impact the user but I believe the impact is small and >>>>> trivial, >>>>> I prefer replacing fields with protocol struct. >>>> >>>> The problem that API breakages are bad and, for example, OvS uses >>>> these >>>> fields. >>>> >>>> May be API breakage should be postponed to 21.11? >>>> >>> >>> Agree but it is not as bad as ABI break, if user is already >>> compiling their >>> code, it is not too bad to adjust the struct for changes, and the >>> changes are >>> straightforward. >>> >> I'm not sure which is worse ABI or API, API is more straight forward >> but all apps must be modified, >> while ABI is hidden and happens only in rare cases. >> In a addition it may result in large number of changes (simple but >> large number) >> >>> But if, somehow, application needs to support multiple version of >>> the DPDK it >>> can be headache. >>> >> >> Agree, >> >>> We may go with your suggestion until 21.11, and do the cleanup on >>> 21.11, will >>> it >>> work? >> +1 also when considering my next line, >> >> One more point to consider what happens to struct that are not >> according to spec, >> for example mpls, geneve where the struct is different than the item. >> > > At least for mpls & geneve, the ABI still looks same so change is > still possible, but a few fields seems merged which means the change > will require more updates in the user application and the drivers. > Anyway, agree to postpone change to the 21.11. > > I will send a v2. I hope it is still possible to add hdr fields without ABI/ABI breakage in 20.02.