From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga07.intel.com (mga07.intel.com [134.134.136.100]) by dpdk.org (Postfix) with ESMTP id 253883253 for ; Mon, 13 May 2019 18:24:34 +0200 (CEST) X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from fmsmga007.fm.intel.com ([10.253.24.52]) by orsmga105.jf.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 13 May 2019 09:24:33 -0700 X-ExtLoop1: 1 Received: from irsmsx104.ger.corp.intel.com ([163.33.3.159]) by fmsmga007.fm.intel.com with ESMTP; 13 May 2019 09:24:32 -0700 Received: from irsmsx105.ger.corp.intel.com ([169.254.7.155]) by IRSMSX104.ger.corp.intel.com ([169.254.5.93]) with mapi id 14.03.0415.000; Mon, 13 May 2019 17:24:30 +0100 From: "Ananyev, Konstantin" To: Adrien Mazarguil CC: "Smoczynski, MarcinX" , Thomas Monjalon , "dev@dpdk.org" , "Richardson, Bruce" , "shahafs@mellanox.com" , "gaetan.rivet@6wind.com" , "matan@mellanox.com" Thread-Topic: [dpdk-dev] Using _XOPEN_SOURCE macros may break builds on FreeBSD Thread-Index: AdUHUhFuf1vcUQd1TgeecdbKRnBo9QAAiWkAAAA7TgAAhPm0AAABLeYAAAJB0QAAA6nuAAAIRlgw Date: Mon, 13 May 2019 16:24:29 +0000 Message-ID: <2601191342CEEE43887BDE71AB97725801616313F3@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> In-Reply-To: <20190513131442.GK4284@6wind.com> Accept-Language: en-IE, en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-titus-metadata-40: eyJDYXRlZ29yeUxhYmVscyI6IiIsIk1ldGFkYXRhIjp7Im5zIjoiaHR0cDpcL1wvd3d3LnRpdHVzLmNvbVwvbnNcL0ludGVsMyIsImlkIjoiY2YwMDRlZjUtZTk2Ny00NDZmLWEzY2QtMmMyMzBjOTMyZWE5IiwicHJvcHMiOlt7Im4iOiJDVFBDbGFzc2lmaWNhdGlvbiIsInZhbHMiOlt7InZhbHVlIjoiQ1RQX05UIn1dfV19LCJTdWJqZWN0TGFiZWxzIjpbXSwiVE1DVmVyc2lvbiI6IjE3LjEwLjE4MDQuNDkiLCJUcnVzdGVkTGFiZWxIYXNoIjoiTXpvbUZjYSs2RWI2dFZHbmF6eHR6d1lIVkRwZmJVM0hoTnlNQmViVnN0QVBaTTNVRlk1MFpcL3kyZlVXWmZpNmwifQ== x-ctpclassification: CTP_NT dlp-product: dlpe-windows dlp-version: 11.0.600.7 dlp-reaction: no-action x-originating-ip: [163.33.239.180] Content-Type: text/plain; charset="us-ascii" 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 List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 13 May 2019 16:24:35 -0000 =20 > Hey Konstantin, >=20 > 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 BS= D > > > > > > when using XOPEN_SOURCE or POSIX_C_SOURCE explicitly. To overco= me > > > > > > this situation we can either remove problematic XOPEN macros fr= om > > > > > > mk/meson rules (drivers/net/failsafe, drivers/net/mlx4, > > > > > > drivers/net/mlx5) > > > > > > > > > > What is the consequence of removing these macros in mlx and fails= afe PMDs? > > > > > > > > The purpose of these *_SOURCE constants is to enable particular fea= ture sets > > > > visibility. As long as we have GNU_SOURCE on Linux 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 res= trict them. > > > > > > I confirm that under Linux, all IPPROTO_* (POSIX/XOPEN/RFC1700) are d= efined > > > regardless (_GNU_SOURCE not even needed), while under FreeBSD, the no= n-POSIX > > > versions are only defined when __BSD_VISIBLE is set. > > > > > > The FreeBSD behavior is more correct in this respect since the purpos= e of > > > _XOPEN_SOURCE and friends is also to let applications limit the risk = of > > > redefinitions in case they were written for an 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= '? >=20 > 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 PMDs. >=20 > _XOPEN_SOURCE=3D600 was originally added to mlx4 (later inherited by mlx5= and > failsafe) for the following reasons: >=20 > - Out fo habit, since a lot of stuff in unistd.h and fcntl.h depends on i= t > to be exposed. Some affected definitions were likely needed at some poi= nt. >=20 > - Besides toggling C syntax extensions, forcing a C standard through the > -std parameter (e.g. -std=3Dc99) in order to guarantee a minimum level = of > C compliance disables the implicit presence of nonstandard definitions, > which must be re-enabled as needed through the appropriate #defines. >=20 > For instance, including unistd.h for getsid() stops working as soon as yo= u > use -std=3Dc99. On Linux you can get it back through -std=3Dgnu99 or by > combining -std=3Dc99 with -D_GNU_SOURCE or -D_XOPEN_SOURCE. The latter wa= s > chosen because it is the standard define supposed to work across OSes. >=20 > Historically mlx4 had to enable -std=3Dc99 to be able to use various feat= ures > not present when GCC defaulted to -std=3Dgnu90. It was later transformed = to > -std=3Dc11 for similar reasons (anonymous members in structs/unions if me= mory > serves me right). >=20 > > > 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. >=20 > Of course, but public headers should be as self sufficient as possible. > Unless they provide really insane compiler flags, if user applications ge= t > compilation errors after including some header we install on the system, > I think the blame is on DPDK. >=20 > > > I think this reason is > > > enough to go with -D__BSD_VISIBLE under FreeBSD without removing > > > _XOPEN_SOURCE, as it should work regardless. > > > > So do you suggest to add '-D __BSD_VISIBLE' into > > mlx/failsafe PMDs Makefiles/meson.build, or ... ? >=20 > Since headers of our public API potentially require it, it must be define= d > globally (unlike _XOPEN_SOURCE which is only local to a few PMDs): > app/meson.build, lib/meson.build, mk/target/generic/rte.vars.mk, alongsid= e > -D_GNU_SOURCE. >=20 > 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) Similar to what we doing for -DGNU_SOURCE. If I understand you correctly, then it sounds ok to me. >=20 > > > Looking at the patch [1], I also think there's another, simpler appro= ach: > > > 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 each ext header is a way too expensive approach. > > Will prefer to keep that function inline. >=20 > OK, a bit cumbersome but since we're heading this way [2], how about > defining our own instead of all the above? >=20 > #define RTE_IPPROTO_HOPOPTS 0 > #define RTE_IPPROTO_ROUTING 43 > ... >=20 > Which could prove handy later as it appears Linux and FreeBSD don't have = the > same set of available IPPROTO_* definitions. >=20 > Thoughts? >=20 > [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=20 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 should be subject of a separate patch series. Konstantin From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from dpdk.org (dpdk.org [92.243.14.124]) by dpdk.space (Postfix) with ESMTP id 6BB4FA00E6 for ; Mon, 13 May 2019 18:24:37 +0200 (CEST) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id 992954C95; Mon, 13 May 2019 18:24:36 +0200 (CEST) Received: from mga07.intel.com (mga07.intel.com [134.134.136.100]) by dpdk.org (Postfix) with ESMTP id 253883253 for ; Mon, 13 May 2019 18:24:34 +0200 (CEST) X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from fmsmga007.fm.intel.com ([10.253.24.52]) by orsmga105.jf.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 13 May 2019 09:24:33 -0700 X-ExtLoop1: 1 Received: from irsmsx104.ger.corp.intel.com ([163.33.3.159]) by fmsmga007.fm.intel.com with ESMTP; 13 May 2019 09:24:32 -0700 Received: from irsmsx105.ger.corp.intel.com ([169.254.7.155]) by IRSMSX104.ger.corp.intel.com ([169.254.5.93]) with mapi id 14.03.0415.000; Mon, 13 May 2019 17:24:30 +0100 From: "Ananyev, Konstantin" To: Adrien Mazarguil CC: "Smoczynski, MarcinX" , Thomas Monjalon , "dev@dpdk.org" , "Richardson, Bruce" , "shahafs@mellanox.com" , "gaetan.rivet@6wind.com" , "matan@mellanox.com" Thread-Topic: [dpdk-dev] Using _XOPEN_SOURCE macros may break builds on FreeBSD Thread-Index: AdUHUhFuf1vcUQd1TgeecdbKRnBo9QAAiWkAAAA7TgAAhPm0AAABLeYAAAJB0QAAA6nuAAAIRlgw Date: Mon, 13 May 2019 16:24:29 +0000 Message-ID: <2601191342CEEE43887BDE71AB97725801616313F3@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> In-Reply-To: <20190513131442.GK4284@6wind.com> Accept-Language: en-IE, en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-titus-metadata-40: eyJDYXRlZ29yeUxhYmVscyI6IiIsIk1ldGFkYXRhIjp7Im5zIjoiaHR0cDpcL1wvd3d3LnRpdHVzLmNvbVwvbnNcL0ludGVsMyIsImlkIjoiY2YwMDRlZjUtZTk2Ny00NDZmLWEzY2QtMmMyMzBjOTMyZWE5IiwicHJvcHMiOlt7Im4iOiJDVFBDbGFzc2lmaWNhdGlvbiIsInZhbHMiOlt7InZhbHVlIjoiQ1RQX05UIn1dfV19LCJTdWJqZWN0TGFiZWxzIjpbXSwiVE1DVmVyc2lvbiI6IjE3LjEwLjE4MDQuNDkiLCJUcnVzdGVkTGFiZWxIYXNoIjoiTXpvbUZjYSs2RWI2dFZHbmF6eHR6d1lIVkRwZmJVM0hoTnlNQmViVnN0QVBaTTNVRlk1MFpcL3kyZlVXWmZpNmwifQ== x-ctpclassification: CTP_NT dlp-product: dlpe-windows dlp-version: 11.0.600.7 dlp-reaction: no-action x-originating-ip: [163.33.239.180] 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 List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" Message-ID: <20190513162429.W-0MVIvtevJH2zaqhvo1zXuTtC4z3c5EmL5VsPBY_BI@z> =20 > Hey Konstantin, >=20 > 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 BS= D > > > > > > when using XOPEN_SOURCE or POSIX_C_SOURCE explicitly. To overco= me > > > > > > this situation we can either remove problematic XOPEN macros fr= om > > > > > > mk/meson rules (drivers/net/failsafe, drivers/net/mlx4, > > > > > > drivers/net/mlx5) > > > > > > > > > > What is the consequence of removing these macros in mlx and fails= afe PMDs? > > > > > > > > The purpose of these *_SOURCE constants is to enable particular fea= ture sets > > > > visibility. As long as we have GNU_SOURCE on Linux 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 res= trict them. > > > > > > I confirm that under Linux, all IPPROTO_* (POSIX/XOPEN/RFC1700) are d= efined > > > regardless (_GNU_SOURCE not even needed), while under FreeBSD, the no= n-POSIX > > > versions are only defined when __BSD_VISIBLE is set. > > > > > > The FreeBSD behavior is more correct in this respect since the purpos= e of > > > _XOPEN_SOURCE and friends is also to let applications limit the risk = of > > > redefinitions in case they were written for an 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= '? >=20 > 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 PMDs. >=20 > _XOPEN_SOURCE=3D600 was originally added to mlx4 (later inherited by mlx5= and > failsafe) for the following reasons: >=20 > - Out fo habit, since a lot of stuff in unistd.h and fcntl.h depends on i= t > to be exposed. Some affected definitions were likely needed at some poi= nt. >=20 > - Besides toggling C syntax extensions, forcing a C standard through the > -std parameter (e.g. -std=3Dc99) in order to guarantee a minimum level = of > C compliance disables the implicit presence of nonstandard definitions, > which must be re-enabled as needed through the appropriate #defines. >=20 > For instance, including unistd.h for getsid() stops working as soon as yo= u > use -std=3Dc99. On Linux you can get it back through -std=3Dgnu99 or by > combining -std=3Dc99 with -D_GNU_SOURCE or -D_XOPEN_SOURCE. The latter wa= s > chosen because it is the standard define supposed to work across OSes. >=20 > Historically mlx4 had to enable -std=3Dc99 to be able to use various feat= ures > not present when GCC defaulted to -std=3Dgnu90. It was later transformed = to > -std=3Dc11 for similar reasons (anonymous members in structs/unions if me= mory > serves me right). >=20 > > > 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. >=20 > Of course, but public headers should be as self sufficient as possible. > Unless they provide really insane compiler flags, if user applications ge= t > compilation errors after including some header we install on the system, > I think the blame is on DPDK. >=20 > > > I think this reason is > > > enough to go with -D__BSD_VISIBLE under FreeBSD without removing > > > _XOPEN_SOURCE, as it should work regardless. > > > > So do you suggest to add '-D __BSD_VISIBLE' into > > mlx/failsafe PMDs Makefiles/meson.build, or ... ? >=20 > Since headers of our public API potentially require it, it must be define= d > globally (unlike _XOPEN_SOURCE which is only local to a few PMDs): > app/meson.build, lib/meson.build, mk/target/generic/rte.vars.mk, alongsid= e > -D_GNU_SOURCE. >=20 > 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) Similar to what we doing for -DGNU_SOURCE. If I understand you correctly, then it sounds ok to me. >=20 > > > Looking at the patch [1], I also think there's another, simpler appro= ach: > > > 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 each ext header is a way too expensive approach. > > Will prefer to keep that function inline. >=20 > OK, a bit cumbersome but since we're heading this way [2], how about > defining our own instead of all the above? >=20 > #define RTE_IPPROTO_HOPOPTS 0 > #define RTE_IPPROTO_ROUTING 43 > ... >=20 > Which could prove handy later as it appears Linux and FreeBSD don't have = the > same set of available IPPROTO_* definitions. >=20 > Thoughts? >=20 > [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=20 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 should be subject of a separate patch series. Konstantin