From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: <dev-bounces@dpdk.org> Received: from dpdk.org (dpdk.org [92.243.14.124]) by dpdk.space (Postfix) with ESMTP id 9EFBBA00E6 for <public@inbox.dpdk.org>; Tue, 14 May 2019 12:30:03 +0200 (CEST) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id B74D34D3A; Tue, 14 May 2019 12:30:02 +0200 (CEST) Received: from mga02.intel.com (mga02.intel.com [134.134.136.20]) by dpdk.org (Postfix) with ESMTP id F19BD4CA7 for <dev@dpdk.org>; Tue, 14 May 2019 12:30:00 +0200 (CEST) X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from fmsmga003.fm.intel.com ([10.253.24.29]) by orsmga101.jf.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 14 May 2019 03:30:00 -0700 X-ExtLoop1: 1 Received: from irsmsx154.ger.corp.intel.com ([163.33.192.96]) by FMSMGA003.fm.intel.com with ESMTP; 14 May 2019 03:29:58 -0700 Received: from irsmsx105.ger.corp.intel.com ([169.254.7.155]) by IRSMSX154.ger.corp.intel.com ([169.254.12.120]) with mapi id 14.03.0415.000; Tue, 14 May 2019 11:29:57 +0100 From: "Ananyev, Konstantin" <konstantin.ananyev@intel.com> To: Adrien Mazarguil <adrien.mazarguil@6wind.com>, "Smoczynski, MarcinX" <marcinx.smoczynski@intel.com> CC: Thomas Monjalon <thomas@monjalon.net>, "dev@dpdk.org" <dev@dpdk.org>, "Richardson, Bruce" <bruce.richardson@intel.com>, "shahafs@mellanox.com" <shahafs@mellanox.com>, "gaetan.rivet@6wind.com" <gaetan.rivet@6wind.com>, "matan@mellanox.com" <matan@mellanox.com> Thread-Topic: [dpdk-dev] Using _XOPEN_SOURCE macros may break builds on FreeBSD Thread-Index: AdUHUhFuf1vcUQd1TgeecdbKRnBo9QAAiWkAAAA7TgAAhPm0AAABLeYAAAJB0QAAA6nuAAAIRlgwACETcQAAAJ9xAAAC0uBg Date: Tue, 14 May 2019 10:29:57 +0000 Message-ID: <2601191342CEEE43887BDE71AB977258016163294B@irsmsx105.ger.corp.intel.com> References: <2F25558C1648FA498380EAC12A8612624FD953@HASMSX110.ger.corp.intel.com> <9076832.UKkv39EgZr@xps> <2604468.nBFxeWxGDx@xps> <2F25558C1648FA498380EAC12A86126251F3B2@HASMSX110.ger.corp.intel.com> <20190513102510.GJ4284@6wind.com> <2601191342CEEE43887BDE71AB9772580161631159@irsmsx105.ger.corp.intel.com> <20190513131442.GK4284@6wind.com> <2601191342CEEE43887BDE71AB97725801616313F3@irsmsx105.ger.corp.intel.com> <2F25558C1648FA498380EAC12A86126251FDD5@HASMSX110.ger.corp.intel.com> <20190514091632.GO4284@6wind.com> In-Reply-To: <20190514091632.GO4284@6wind.com> Accept-Language: en-IE, en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-titus-metadata-40: eyJDYXRlZ29yeUxhYmVscyI6IiIsIk1ldGFkYXRhIjp7Im5zIjoiaHR0cDpcL1wvd3d3LnRpdHVzLmNvbVwvbnNcL0ludGVsMyIsImlkIjoiY2U3MzFjNmQtY2MxNS00YTc2LTlkZmYtZDI1NGExOWEyMmRkIiwicHJvcHMiOlt7Im4iOiJDVFBDbGFzc2lmaWNhdGlvbiIsInZhbHMiOlt7InZhbHVlIjoiQ1RQX05UIn1dfV19LCJTdWJqZWN0TGFiZWxzIjpbXSwiVE1DVmVyc2lvbiI6IjE3LjEwLjE4MDQuNDkiLCJUcnVzdGVkTGFiZWxIYXNoIjoiMkZjaFE0ZWFlTzZYVEZcL1htSmFjQTU1TWtvSXhxS3VcL2NvY3FISDlidHVXNUptREV5ZmdNZTV0U1YxZFVCZmd1In0= x-ctpclassification: CTP_NT dlp-product: dlpe-windows dlp-version: 11.0.600.7 dlp-reaction: no-action x-originating-ip: [163.33.239.182] Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 Subject: Re: [dpdk-dev] Using _XOPEN_SOURCE macros may break builds on FreeBSD X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions <dev.dpdk.org> List-Unsubscribe: <https://mails.dpdk.org/options/dev>, <mailto:dev-request@dpdk.org?subject=unsubscribe> List-Archive: <http://mails.dpdk.org/archives/dev/> List-Post: <mailto:dev@dpdk.org> List-Help: <mailto:dev-request@dpdk.org?subject=help> List-Subscribe: <https://mails.dpdk.org/listinfo/dev>, <mailto:dev-request@dpdk.org?subject=subscribe> Errors-To: dev-bounces@dpdk.org Sender: "dev" <dev-bounces@dpdk.org> Message-ID: <20190514102957.ciHHbDljK9XaHBK-6Xju4oqEHi2WPU3_jlHuJsvz7Ss@z> Hi Adrien, > On Tue, May 14, 2019 at 08:58:42AM +0000, Smoczynski, MarcinX wrote: > > > > Hey Konstantin, > > > > > > > > On Mon, May 13, 2019 at 10:49:00AM +0000, Ananyev, Konstantin wrote= : > > > > > Hi Adrien, > > > > > > > > > > > > > > > > > On Mon, May 13, 2019 at 09:51:24AM +0000, Smoczynski, MarcinX > > > wrote: > > > > > > > 10/05/2019 20:17, Thomas Monjalon: > > > > > > > > 10/05/2019 19:14, Smoczynski, MarcinX: > > > > > > > > > To summarize we have different visibility sets for Linux = and > > > > > > > > > BSD when using XOPEN_SOURCE or POSIX_C_SOURCE explicitly. > > > To > > > > > > > > > overcome this situation we can either remove problematic > > > > > > > > > XOPEN macros from mk/meson rules (drivers/net/failsafe, > > > > > > > > > drivers/net/mlx4, > > > > > > > > > drivers/net/mlx5) > > > > > > > > > > > > > > > > What is the consequence of removing these macros in mlx and > > > failsafe PMDs? > > > > > > > > > > > > > > The purpose of these *_SOURCE constants is to enable particul= ar > > > > > > > feature sets visibility. As long as we have GNU_SOURCE on Lin= ux > > > > > > > removing it won't have any consequences. On BSD it will unify > > > > > > > feature sets visibility with the rest of sources. Can't think= of any > > > downsides here. > > > > > > > > > > > > > > I believe XOPEN_SOURCE was introduced to extend features not = to > > > restrict them. > > > > > > > > > > > > I confirm that under Linux, all IPPROTO_* (POSIX/XOPEN/RFC1700) > > > > > > are defined regardless (_GNU_SOURCE not even needed), while und= er > > > > > > FreeBSD, the non-POSIX versions are only defined when > > > __BSD_VISIBLE is set. > > > > > > > > > > > > The FreeBSD behavior is more correct in this respect since the > > > > > > purpose of _XOPEN_SOURCE and friends is also to let application= s > > > > > > limit the risk of redefinitions in case they were written for a= n > > > > > > earlier standard (e.g. -D_XOPEN_SOURCE=3D500 vs. - > > > D_XOPEN_SOURCE=3D600). > > > > > > > > > > Still not sure why do you need it for failsafe and mlx PMDs? > > > > > Would something in these PMDs be broken without '- > > > D_XOPEN_SOURCE=3D600'? > > > > > > > > Well, not really. At least not anymore if they compile fine without= it > > > > on all supported targets. I don't mind if they are removed from PMD= s. > > > > > > > > _XOPEN_SOURCE=3D600 was originally added to mlx4 (later inherited b= y > > > > mlx5 and > > > > failsafe) for the following reasons: > > > > > > > > - Out fo habit, since a lot of stuff in unistd.h and fcntl.h depend= s on it > > > > to be exposed. Some affected definitions were likely needed at so= me > > > point. > > > > > > > > - Besides toggling C syntax extensions, forcing a C standard throug= h the > > > > -std parameter (e.g. -std=3Dc99) in order to guarantee a minimum = level of > > > > C compliance disables the implicit presence of nonstandard defini= tions, > > > > which must be re-enabled as needed through the appropriate #defin= es. > > > > > > > > For instance, including unistd.h for getsid() stops working as soon= as > > > > you use -std=3Dc99. On Linux you can get it back through -std=3Dgnu= 99 or > > > > by combining -std=3Dc99 with -D_GNU_SOURCE or -D_XOPEN_SOURCE. The > > > > latter was chosen because it is the standard define supposed to wor= k > > > across OSes. > > > > > > > > Historically mlx4 had to enable -std=3Dc99 to be able to use variou= s > > > > features not present when GCC defaulted to -std=3Dgnu90. It was lat= er > > > > transformed to > > > > -std=3Dc11 for similar reasons (anonymous members in structs/unions= if > > > > memory serves me right). > > > > > > > > > > DPDK applications may also define _XOPEN_SOURCE for their own > > > > > > needs. They should still be able to use rte_ip.h afterward. > > > > > > > > > > I suppose they can, they would just have (on FreeBSD) to add '-D > > > __BSD_VISIBLE' > > > > > themselves. > > > > > > > > Of course, but public headers should be as self sufficient as possi= ble. > > > > Unless they provide really insane compiler flags, if user applicati= ons > > > > get compilation errors after including some header we install on th= e > > > > system, I think the blame is on DPDK. > > > > > > > > > > I think this reason is > > > > > > enough to go with -D__BSD_VISIBLE under FreeBSD without removin= g > > > > > > _XOPEN_SOURCE, as it should work regardless. > > > > > > > > > > So do you suggest to add '-D __BSD_VISIBLE' into mlx/failsafe PM= Ds > > > > > Makefiles/meson.build, or ... ? > > > > > > > > Since headers of our public API potentially require it, it must be > > > > defined globally (unlike _XOPEN_SOURCE which is only local to a few > > > PMDs): > > > > app/meson.build, lib/meson.build, mk/target/generic/rte.vars.mk, > > > > alongside -D_GNU_SOURCE. > > > > > > > > Add it to mlx*/failsafe only if that's not enough. Just make sure > > > > applications inherit this flag. > > > > > > Ok, to summarize, eyour suggestion is: > > > 1. remove -D_XOPEN_SOURCE=3D... from mlx and failsafe PMDs. > > > 2. add '-D __BSD_VISIBLE' into top level make/meson files > > > (app/meson.build, lib/meson.build, mk/target/generic/rte.vars.mk) Sim= ilar > > > to what we doing for -DGNU_SOURCE. > > > > > > If I understand you correctly, then it sounds ok to me. > > > > > > > > > > > > > Looking at the patch [1], I also think there's another, simpler= approach: > > > > > > unless really performance critical, defining > > > > > > rte_ipv6_get_next_ext() in rte_net.c instead of a static inline= in > > > rte_ip.h should address this issue. > > > > > > > > > > It is performance critical, and I think that function call for ea= ch > > > > > ext header is a way too expensive approach. > > > > > Will prefer to keep that function inline. > > > > > > > > OK, a bit cumbersome but since we're heading this way [2], how abou= t > > > > defining our own instead of all the above? > > > > > > > > #define RTE_IPPROTO_HOPOPTS 0 > > > > #define RTE_IPPROTO_ROUTING 43 > > > > ... > > > > > > > > Which could prove handy later as it appears Linux and FreeBSD don't > > > > have the same set of available IPPROTO_* definitions. > > > > > > > > Thoughts? > > > > > > > > [2] "[RFC v2 00/14] prefix network structures" > > > > https://mails.dpdk.org/archives/dev/2019-April/129752.html > > > > > > Yep, that's definitely an option too. > > > But if we going to replace all current references of IPPROTO_ inside = DPDK to > > > RTE_IPROTO_ - the change will be massive. > > > And for sure it is out of scope of this patch series. > > > That's probably need to be done after Olivier RFC will be in and shou= ld be > > > subject of a separate patch series. > > > Konstantin > > > > I agree that we need RTE_IPPROTO* macros but as Konstantin pointed out = this > > would be a huge change and we should do that on top of Oliver's work in > > a separate patch set. > > > > I will propose a patch set with: > > 1. Removed XOPEN_SOURCE macros as they are not needed anymore > > 2. Added BSD_VISIBLE at the top of build system. >=20 > Actually I still suggest to leave the existing _XOPEN_SOURCE for users of > -std=3Dwhatever, even if covered globally by _GNU_SOURCE and __BSD_VISIBL= E. > I think it's useful as a reminder that they did their homework since this= is > macro is itself standard. >=20 If you insist, I don't mind to keep it - less changes for us, again I am not a maintainer, nor a user of these PMDs. Just don't to see much rationale behind it. Ss I understand from your previous letters - with global flags in place, it would build with -std=3D... regardless do we have or not XOPEN_SOURCE=3D... in these PMDs or not. Konstantin > Regarding RTE_IPPROTO*, my suggestion wasn't to convert DPDK entirely, on= ly > to add the missing ones so far only needed by this patch. Given their val= ues > are defined by RFCs, they should be fully compatible and interchangeable > with system definitions. >=20 > I'm fine with either approach in any case. >=20 > -- > Adrien Mazarguil > 6WIND