From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail.droids-corp.org (zoll.droids-corp.org [94.23.50.67]) by dpdk.org (Postfix) with ESMTP id F3A48695D for ; Mon, 18 Feb 2019 17:58:47 +0100 (CET) Received: from lfbn-1-5920-128.w90-110.abo.wanadoo.fr ([90.110.126.128] helo=droids-corp.org) by mail.droids-corp.org with esmtpsa (TLS1.0:RSA_AES_256_CBC_SHA1:256) (Exim 4.89) (envelope-from ) id 1gvmHg-0001DF-QJ; Mon, 18 Feb 2019 18:00:50 +0100 Received: by droids-corp.org (sSMTP sendmail emulation); Mon, 18 Feb 2019 17:58:38 +0100 Date: Mon, 18 Feb 2019 17:58:38 +0100 From: Olivier Matz To: Ferruh Yigit Cc: "Yigit, Ferruh" , "Wiles, Keith" , Stephen Hemminger , dpdk-dev , "Richardson, Bruce" Message-ID: <20190218165838.fvwtrxubtdkadifz@platinum> References: <20181024081833.21432-1-olivier.matz@6wind.com> <20181220154834.166aedb6@xeon-e3> <28E73175-3924-49B0-A88A-E52C65D05C65@intel.com> <9b343ff1-5d08-69e1-140b-fc89a4782570@intel.com> <20181227093530.igimdsn65xmr7qdk@platinum> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: User-Agent: NeoMutt/20170113 (1.7.2) Subject: Re: [dpdk-dev] [RFC 00/14] prefix network structures 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: , X-List-Received-Date: Mon, 18 Feb 2019 16:58:48 -0000 On Mon, Feb 18, 2019 at 12:37:41PM +0000, Ferruh Yigit wrote: > On 2/13/2019 11:48 AM, Yigit, Ferruh wrote: > > On 12/27/2018 9:35 AM, Olivier Matz wrote: > >> Hi, > >> > >> On Fri, Dec 21, 2018 at 03:14:29PM +0000, Ferruh Yigit wrote: > >>> On 12/21/2018 2:38 PM, Wiles, Keith wrote: > >>>> > >>>> > >>>>> On Dec 20, 2018, at 5:48 PM, Stephen Hemminger wrote: > >>>>> > >>>>> On Thu, 20 Dec 2018 21:59:37 +0000 > >>>>> Ferruh Yigit wrote: > >>>>> > >>>>>> On 10/24/2018 9:18 AM, olivier.matz at 6wind.com (Olivier Matz) wrote: > >>>>>>> This RFC targets 19.02. > >>>>>>> > >>>>>>> The rte_net headers conflict with the libc headers, because > >>>>>>> some definitions are duplicated, sometimes with few differences. > >>>>>>> This was discussed in [1], and more recently at the techboard. > >>>>>>> > >>>>>>> Before sending the deprecation notice (target for this is 18.11), > >>>>>>> here is a draft that can be discussed. > >>>>>>> > >>>>>>> This RFC adds the rte_ (or RTE_) prefix to all structures, functions > >>>>>>> and defines in rte_net library. This is a big changeset, that will > >>>>>>> break the API of many functions, but not the ABI. > >>>>>>> > >>>>>>> One question I'm asking is how can we manage the transition. > >>>>>>> Initially, I hoped it was possible to have a compat layer during > >>>>>>> one release (supporting both prefixed and unprefixed names), but > >>>>>>> now that the patch is done, it seems the impact is too big, and > >>>>>>> impacts too many libraries. > >>>>>>> > >>>>>>> Few examples: > >>>>>>> - rte_eth_macaddr_get/add/remove() use a (struct rte_ether_addr *) > >>>>>>> - many rte_flow structures use the rte_ prefixed net structures > >>>>>>> - the mac field of virtio_net structure is rte_ether_addr > >>>>>>> - the first arg of rte_thash_load_v6_addrs is (struct rte_ipv6_hdr *) > >>>>>>> ... > >>>>>>> > >>>>>>> Therefore, it is clear that doing this would break the compilation > >>>>>>> of many external applications. > >>>>>>> > >>>>>>> Another drawback we need to take in account: it will make the > >>>>>>> backport of patches more difficult, although this is something > >>>>>>> that could be tempered by a script. > >>>>>>> > >>>>>>> While it is obviously better to have a good namespace convention, > >>>>>>> we need to identify the issues we have today before deciding it's > >>>>>>> worth doing the change. > >>>>>>> > >>>>>>> Comments? > >>>>>> > >>>>>> Is there an consensus about the patchset? There was a decision on techboard to > >>>>>> go with this change (adding rte_ prefix) [1]. > >>>>>> > >>>>>> > >>>>>> This is something that will affect DPDK consumers. Since many APIs are changing > >>>>>> most probably will break API compatibility for many libraries. > >>>>>> > >>>>>> Meanwhile the conflict with the libc headers mentioned a few times in the past, > >>>>>> this is something we need to fix. > >>>>>> > >>>>>> There are a few comments reluctant to this big modification, but what I > >>>>>> understand from Olivier's response both using BSD defines or having > >>>>>> compatibility headers in DPDK won't solve the problem completely. > >>>>>> > >>>>>> And assuming we will continue with this solution, another question is do we > >>>>>> still want to push in 19.02 scope? (And from process point of view I think a > >>>>>> deprecation notice is not merged for this change in 18.11 scope.) > >>>>>> > >>>>>> With the prediction of 19.05 will be big and already break API/ABI for some > >>>>>> libraries, can we push this into 19.05 as an early merge into repo? > >>>>>> > >>>>>> And I think this patch will affect LTS releases and will break auto backporting > >>>>>> for many fixes because it touches many places, so pushing this change even to > >>>>>> next LTS (19.11) can be an option. > >>>>>> > >>>>>> > >>>>>> Olivier, Thomas, > >>>>>> > >>>>>> What do you think about postponing this to 19.05 or even 19.11 ? > >>>>>> > >>>>>> > >>>>>> > >>>>>> [1] > >>>>>> https://mails.dpdk.org/archives/dev/2018-October/116695.html > >>>>>> > >>>>>>> > >>>>>>> > >>>>>>> Things that are missing in RFC: > >>>>>>> - test with FreeBSD > >>>>>>> - manually fix some indentation issues > >>>>>>> > >>>>>>> > >>>>>>> Olivier Matz (14): > >>>>>>> net: add rte prefix to arp structures > >>>>>>> net: add rte prefix to arp defines > >>>>>>> net: add rte prefix to ether structures > >>>>>>> net: add rte prefix to ether functions > >>>>>>> net: add rte prefix to ether defines > >>>>>>> net: add rte prefix to esp structure > >>>>>>> net: add rte prefix to gre structure > >>>>>>> net: add rte prefix to icmp structure > >>>>>>> net: add rte prefix to icmp defines > >>>>>>> net: add rte prefix to ip structure > >>>>>>> net: add rte prefix to ip defines > >>>>>>> net: add rte prefix to sctp structure > >>>>>>> net: add rte prefix to tcp structure > >>>>>>> net: add rte prefix to udp structure > >>>>>> > >>>>>> > >>>>> > >>>>> Sigh. Another case where DPDK has to reinvent something because > >>>>> we can't figure out how to do dependent libraries correctly. > >>>>> I would have rather just using the existing Linux, BSD definitions > >>>>> and fixing the DPDK code. > >> > >> > >> It is not that simple. As I said in [1], there are still some > >> differences between gnu libc and freebsd libc. Unfortunatly, the struct > >> ether_addr is one of the most important in dpdk, because it is widely > >> used in APIs (drivers). > >> > >> We can find others differences, for instance in constant definitions in > >> if_arp.h. I also see that some structures are packed in freebsd but not > >> in glibc (ex: icmp6), this could have performance impact. > >> > >> Many protocols that are currently defined in dpdk are missing in glibc: > >> esp, sctp, gre, mpls, ... so we will at least need rte_ structures for > >> these protocols. > >> > >> Supporting other OSes or libc in the future could also increase the gaps. > >> > >> For these reasons think it is reasonable to have a consistent set of > >> network structures in dpdk. > >> > >> > >> [1] https://mails.dpdk.org/archives/dev/2018-October/117258.html > >> > >> > >>>>> It is probably the only viable compromise, but it adds to long > >>>>> term maintenance, and breaks DPDK applications. Neither of which > >>>>> is a good thing. > >>>>> > >>>>> Should this be done by marking the old structure deprecated > >>>>> first? Ideally, spread over three releases: first, tell the users > >>>>> in the documentation it is coming; second, mark the old structures > >>>>> as deprecated causing compiler warnings; third, remove the old > >>>>> definitions. Doing at once is introducing user pain for no gain. > >>>> > >>>> +1 > >> > >> Annoucing the change before doing it is obvious. Marking the old > >> structures as deprecated before removing them is maybe doable (to be > >> checked that it does not cause conflicts with new structures), but it > >> means the conflict with libc headers that we are trying to solve will > >> remain for one more version, for a limited gain. > >> > >>> With the current timeline, readiness of the patch and comments, at least it > >>> won't able to make this release, I will update the patchset status as 'Deferred' > >>> > >>> Should we discuss this again in techboard? > >> > >> We should surely weigh the pros and cons. Especially the additional > >> backport troubles it can bring. > >> > >> Are many people bothered by the current conflict with the libc headers? > > > > This is still open. > > > > If we will get these patchset, I suggest it getting early in the 19.05, patch is > > mechanical but it is huge and will affect almost all other patches under > > development. So I am not really for pushing this close to RC. > > > > Is there any way to decide on this week, at worst next week? > > This has been discussed in techboard meeting and decided to go with this patch. > But we are missing the deprecation notice for this. > > > Olivier, > > Can you send a deprecation notice for this in the scope of the 19.05? > And can we target the actual patch for very early days of the 19.08? > Hi Ferruh, OK, will do. Thank you. Olivier