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

[Ferruh Yigit <ferruh.yigit@intel.com>]

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 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

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?