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 CB902A00E6 for ; Tue, 14 May 2019 11:16:36 +0200 (CEST) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id A5B6E5942; Tue, 14 May 2019 11:16:35 +0200 (CEST) Received: from mail-wr1-f68.google.com (mail-wr1-f68.google.com [209.85.221.68]) by dpdk.org (Postfix) with ESMTP id 10FC7568A for ; Tue, 14 May 2019 11:16:34 +0200 (CEST) Received: by mail-wr1-f68.google.com with SMTP id f8so482953wrt.1 for ; Tue, 14 May 2019 02:16:34 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=6wind.com; s=google; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to; bh=kfnEjW9DIMlG7wLLmLi0xAkAVtfMrkmaBwTv9nxiTKM=; b=dYkgUCQ+hqcdhWwMXFuU6wyZ+y1z6/jXXLPO1p6VYmm+jOKxbv8++2Zlc8Q73A+W/I JPi7fjTRLfMNOra1AXA0AlHYr5DXYLJgrQPFws/Fy86wxFohsvtpnBUcXBM06yyU0k3m pNqKnUQ/pQG3Nic7uJAtDrg4+TsB1FUH/RA7esh5Xn9W8eLFp8p8XUc1nc/zZ/6kxCUA rgwbmImu0sBr2lOWVklVswACxeVvxYbva66fn78hzc+MNUGladDuqmstsI3c6oqUDVlT 2eFOeW5q4WiTpUMh4GMCka34oaq0orRayElWWWMXMyWA2dVMHtMOWNPUL+EOpV8w/jPR 2k+Q== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to; bh=kfnEjW9DIMlG7wLLmLi0xAkAVtfMrkmaBwTv9nxiTKM=; b=PIAxp2OZN8NuNjkRglTIyiAW4RmOBVKex3s6eDMJMbMqO6eqlAPgHj19PMKN76aL3I 1BuUykxUysexOtZoBrlOGm2iimerZqsQyGgszrOaSqmSz4YFSmVkg8t7g/5RehWQ70vw 84hjAznGbKsWuXRqaYQydUrHDmpBK4KoGlNibPmtXpcVl23E6DoXEy3/L60vvwsZ3wv1 Inwbk6kxF2N1n0I094vj7/9Vfg3Lj24ImorlGtb17SZjbvZKhHc81IxEHcqzBSqgg2Rr PcDdEh0RIhXyYFxplUqfcV8tEk9NqOBC9H1c4zp7jeYXUhebb+vFtc+JMtY9jHuxC0pz NrIQ== X-Gm-Message-State: APjAAAXvBex2X7157kfFmYej72JPR5Zx/3GeZpyDuKdORHMruRaqdHhP lRQMVduErS0K7jTL/pXChRQKUg== X-Google-Smtp-Source: APXvYqzrVDaP8CYaBTxmqTEh3tDJqX74raduhZ9aDgM1AiC4DTtGCaoopbeRO5AUKyxiTrV8ggfPDA== X-Received: by 2002:a5d:688f:: with SMTP id h15mr10354717wru.44.1557825394708; Tue, 14 May 2019 02:16:34 -0700 (PDT) Received: from 6wind.com (host.78.145.23.62.rev.coltfrance.com. [62.23.145.78]) by smtp.gmail.com with ESMTPSA id r3sm11822786wrn.5.2019.05.14.02.16.33 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Tue, 14 May 2019 02:16:33 -0700 (PDT) Date: Tue, 14 May 2019 11:16:32 +0200 From: Adrien Mazarguil To: "Smoczynski, MarcinX" Cc: "Ananyev, Konstantin" , Thomas Monjalon , "dev@dpdk.org" , "Richardson, Bruce" , "shahafs@mellanox.com" , "gaetan.rivet@6wind.com" , "matan@mellanox.com" Message-ID: <20190514091632.GO4284@6wind.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> MIME-Version: 1.0 Content-Type: text/plain; charset="UTF-8" Content-Disposition: inline In-Reply-To: <2F25558C1648FA498380EAC12A86126251FDD5@HASMSX110.ger.corp.intel.com> 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: <20190514091632.NqdM0o--SAIyUI8TCKj4qpk6cRLlkPxU8vpW9WawVqo@z> 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 particular > > > > > > feature 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 > > restrict them. > > > > > > > > > > I confirm that under Linux, all IPPROTO_* (POSIX/XOPEN/RFC1700) > > > > > are defined regardless (_GNU_SOURCE not even needed), while under > > > > > 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 applications > > > > > limit the risk of redefinitions in case they were written for an > > > > > earlier standard (e.g. -D_XOPEN_SOURCE=500 vs. - > > D_XOPEN_SOURCE=600). > > > > > > > > Still not sure why do you need it for failsafe and mlx PMDs? > > > > Would something in these PMDs be broken without '- > > D_XOPEN_SOURCE=600'? > > > > > > 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. > > > > > > _XOPEN_SOURCE=600 was originally added to mlx4 (later inherited by > > > mlx5 and > > > failsafe) for the following reasons: > > > > > > - Out fo habit, since a lot of stuff in unistd.h and fcntl.h depends on it > > > to be exposed. Some affected definitions were likely needed at some > > point. > > > > > > - Besides toggling C syntax extensions, forcing a C standard through the > > > -std parameter (e.g. -std=c99) 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. > > > > > > For instance, including unistd.h for getsid() stops working as soon as > > > you use -std=c99. On Linux you can get it back through -std=gnu99 or > > > by combining -std=c99 with -D_GNU_SOURCE or -D_XOPEN_SOURCE. The > > > latter was chosen because it is the standard define supposed to work > > across OSes. > > > > > > Historically mlx4 had to enable -std=c99 to be able to use various > > > features not present when GCC defaulted to -std=gnu90. It was later > > > transformed to > > > -std=c11 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 possible. > > > Unless they provide really insane compiler flags, if user applications > > > get compilation errors after including some header we install on the > > > system, I think the blame is on DPDK. > > > > > > > > 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 ... ? > > > > > > 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=... 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. > > > > > > > > > > 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 each > > > > 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 about > > > 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 should 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. Actually I still suggest to leave the existing _XOPEN_SOURCE for users of -std=whatever, even if covered globally by _GNU_SOURCE and __BSD_VISIBLE. I think it's useful as a reminder that they did their homework since this is macro is itself standard. Regarding RTE_IPPROTO*, my suggestion wasn't to convert DPDK entirely, only to add the missing ones so far only needed by this patch. Given their values are defined by RFCs, they should be fully compatible and interchangeable with system definitions. I'm fine with either approach in any case. -- Adrien Mazarguil 6WIND