Re: [dpdk-dev] [RFC 00/14] prefix network structures

[Olivier Matz <olivier.matz@6wind.com>]

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 <stephen@networkplumber.org> wrote:
> >>>>>
> >>>>> On Thu, 20 Dec 2018 21:59:37 +0000
> >>>>> Ferruh Yigit <ferruh.yigit@intel.com> 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