From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga06.intel.com (mga06.intel.com [134.134.136.31]) by dpdk.org (Postfix) with ESMTP id D7D0C5B3C for ; Wed, 13 Feb 2019 12:48:16 +0100 (CET) X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from fmsmga006.fm.intel.com ([10.253.24.20]) by orsmga104.jf.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 13 Feb 2019 03:48:15 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.58,365,1544515200"; d="scan'208";a="318639752" Received: from fyigit-mobl.ger.corp.intel.com (HELO [10.237.221.114]) ([10.237.221.114]) by fmsmga006.fm.intel.com with ESMTP; 13 Feb 2019 03:48:13 -0800 To: Olivier Matz , Ferruh Yigit Cc: "Wiles, Keith" , Stephen Hemminger , dpdk-dev , "Richardson, Bruce" 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> From: "Yigit, Ferruh" Message-ID: Date: Wed, 13 Feb 2019 11:48:13 +0000 User-Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:60.0) Gecko/20100101 Thunderbird/60.5.0 MIME-Version: 1.0 In-Reply-To: <20181227093530.igimdsn65xmr7qdk@platinum> Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit 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: Wed, 13 Feb 2019 11:48:17 -0000 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? Thanks, ferruh