From: Olivier Matz <olivier.matz@6wind.com>
To: Adrien Mazarguil <adrien.mazarguil@6wind.com>
Cc: dev@dpdk.org, Bruce Richardson <bruce.richardson@intel.com>
Subject: Re: [dpdk-dev] [PATCH v1 6/6] net: fix rte_ether conflicts with libc
Date: Tue, 16 Jan 2018 14:04:13 +0100 [thread overview]
Message-ID: <20180116130413.q4alhcwvdbgpo3gc@platinum> (raw)
In-Reply-To: <20171222142527.GQ5377@6wind.com>
Hi Adrien,
On Fri, Dec 22, 2017 at 03:25:27PM +0100, Adrien Mazarguil wrote:
> On Fri, Dec 22, 2017 at 02:34:21PM +0100, Olivier MATZ wrote:
> > On Thu, Dec 21, 2017 at 02:00:06PM +0100, Adrien Mazarguil wrote:
...
> > > +#if defined(__FreeBSD__)
> > > +#define addr_bytes octet
> > > +#else
> > > +#define addr_bytes ether_addr_octet
> > > +#endif
> > > +
> > > #define ETHER_LOCAL_ADMIN_ADDR 0x02 /**< Locally assigned Eth. address. */
> > > #define ETHER_GROUP_ADDR 0x01 /**< Multicast or broadcast Eth. address. */
> >
> > This kind of #define looks a bit dangerous to me: it can trigger
> > strange bugs because it will replace all occurences of addr_bytes
> > after this header is included.
>
> Understandable, I checked before settling on this macro though, there's no
> other usage of addr_bytes inside DPDK.
>
> As for applications, there's no way to be completely sure. If we consider
> they have to explicitly include rte_ether.h to get this definition, there
> are chances addr_bytes is exclusively used with MAC addresses.
>
> This change results in an API change (addr_bytes now documented as a
> reserved macro) but has no ABI impact. I think it's a rather harmless
> workaround pending your next suggestion:
>
> > Wouldn't it be a good opportunity to think about adding the rte_ prefix
> > to all variables/functions of rte_ether.h?
>
> That would be ideal, however since almost all definitions in librte_net
> (rte_ip.h, rte_udp.h etc.) also lack this prefix and can still technically
> trigger conflicts with outside definitions (I'm aware work was done to avoid
> that, but it doesn't change the fact they're not in the official DPDK
> namespace), a massive API change would be needed for consistency.
>
> Such a change is unlikely to be accepted for 18.02 in any case, therefore if
> the proposed workaround is deemed too risky, I offer to remove this patch
> from the series. What do you suggest?
I think the only good long term solution is to have a proper namespace
for all rte_net structures and definitions. If there is no blocking issue
requiring this patch now, we can consider the following:
- 18.02: announce an api change for 18.05
- 18.05:
- add new net structures and definitions with rte_ prefix
- mark the old ones as deprecated
- removes the structures or definitions that conflicts with system headers
- 18.08: remove the old structures and definitions
next prev parent reply other threads:[~2018-01-16 13:04 UTC|newest]
Thread overview: 17+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-12-21 12:59 [dpdk-dev] [PATCH v1 0/6] Address various issues with exported headers Adrien Mazarguil
2017-12-21 12:59 ` [dpdk-dev] [PATCH v1 1/6] devtools: update check-includes exceptions Adrien Mazarguil
2017-12-21 12:59 ` [dpdk-dev] [PATCH v1 2/6] net/i40e: fix issue in exported header Adrien Mazarguil
2017-12-27 6:53 ` Xing, Beilei
2017-12-21 12:59 ` [dpdk-dev] [PATCH v1 3/6] flow_classify: " Adrien Mazarguil
2018-01-02 15:19 ` Iremonger, Bernard
2017-12-21 13:00 ` [dpdk-dev] [PATCH v1 4/6] member: " Adrien Mazarguil
2017-12-21 13:00 ` [dpdk-dev] [PATCH v1 5/6] fix missing includes in exported headers Adrien Mazarguil
2017-12-21 14:12 ` Bruce Richardson
2017-12-21 14:50 ` Adrien Mazarguil
2017-12-21 16:01 ` Bruce Richardson
2017-12-21 13:00 ` [dpdk-dev] [PATCH v1 6/6] net: fix rte_ether conflicts with libc Adrien Mazarguil
2017-12-22 13:34 ` Olivier MATZ
2017-12-22 14:25 ` Adrien Mazarguil
2018-01-16 13:04 ` Olivier Matz [this message]
2018-01-16 23:19 ` Thomas Monjalon
2018-01-16 23:18 ` [dpdk-dev] [PATCH v1 0/6] Address various issues with exported headers Thomas Monjalon
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20180116130413.q4alhcwvdbgpo3gc@platinum \
--to=olivier.matz@6wind.com \
--cc=adrien.mazarguil@6wind.com \
--cc=bruce.richardson@intel.com \
--cc=dev@dpdk.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).