* [regresssion 4.15] Userspace compilation broken by uapi/linux/if_ether.h update
@ 2018-01-25 14:58 Guillaume Nault
2018-01-25 22:21 ` Hauke Mehrtens
0 siblings, 1 reply; 6+ messages in thread
From: Guillaume Nault @ 2018-01-25 14:58 UTC (permalink / raw)
To: netdev; +Cc: Hauke Mehrtens
Hi,
Commit 6926e041a892 ("uapi/if_ether.h: prevent redefinition of struct ethhdr"),
can break compilation of userspace programs (in my case, accel-ppp
(https://accel-ppp.org)).
This happens for userspace programs that end up including
linux/if_ether.h, netinet/in.h and linux/in.h in this order:
# cat test_ifether.c
#include <linux/if_ether.h>
#include <netinet/in.h>
#include <linux/in.h>
int main(void)
{
return 0;
}
# gcc test_ifether.c
In file included from test_ifether.c:2:0:
/usr/include/linux/in.h:29:3: error: redeclaration of enumerator ‘IPPROTO_IP’
IPPROTO_IP = 0, /* Dummy protocol for TCP */
^
/usr/include/netinet/in.h:42:5: note: previous definition of ‘IPPROTO_IP’ was here
IPPROTO_IP = 0, /* Dummy protocol for TCP. */
^~~~~~~~~~
Now that linux/libc-compat.h is included in linux/if_ether.h, it is
processed before netinet/in.h. Therefore, it sets the relevant
__UAPI_DEF_* guards to 1 (as _NETINET_IN_H isn't yet defined).
Then netinet/in.h is included, followed by linux/in.h. The later
doesn't realise that what it defines has already been included by
netinet/in.h because the __UAPI_DEF_* guards were set too early.
Of course the situation is a bit more complicated on real projects, as
these files aren't included directly. For example, in accel-ppp, the
PPPoE module (accel-ppp/accel-pppd/ctrl/pppoe/pppoe.c) uses
#include <net/ethernet.h> /* includes linux/if_ether.h */
#include <arpa/inet.h> /* includes netinet/in.h */
#include <linux/if_pppox.h> /* (through pppoe.h), includes linux/in.h */
I don't have a satisfying solution for now, but I'd really like it if
we could avoid shipping a kernel which forces userspace to play with
include files ordering to keep compiling.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [regresssion 4.15] Userspace compilation broken by uapi/linux/if_ether.h update
2018-01-25 14:58 [regresssion 4.15] Userspace compilation broken by uapi/linux/if_ether.h update Guillaume Nault
@ 2018-01-25 22:21 ` Hauke Mehrtens
2018-01-26 10:51 ` Guillaume Nault
0 siblings, 1 reply; 6+ messages in thread
From: Hauke Mehrtens @ 2018-01-25 22:21 UTC (permalink / raw)
To: Guillaume Nault, netdev; +Cc: linux-api
On 01/25/2018 03:58 PM, Guillaume Nault wrote:
> Hi,
>
> Commit 6926e041a892 ("uapi/if_ether.h: prevent redefinition of struct ethhdr"),
> can break compilation of userspace programs (in my case, accel-ppp
> (https://accel-ppp.org)).
>
> This happens for userspace programs that end up including
> linux/if_ether.h, netinet/in.h and linux/in.h in this order:
>
> # cat test_ifether.c
> #include <linux/if_ether.h>
> #include <netinet/in.h>
> #include <linux/in.h>
>
> int main(void)
> {
> return 0;
> }
>
> # gcc test_ifether.c
> In file included from test_ifether.c:2:0:
> /usr/include/linux/in.h:29:3: error: redeclaration of enumerator ‘IPPROTO_IP’
> IPPROTO_IP = 0, /* Dummy protocol for TCP */
> ^
> /usr/include/netinet/in.h:42:5: note: previous definition of ‘IPPROTO_IP’ was here
> IPPROTO_IP = 0, /* Dummy protocol for TCP. */
> ^~~~~~~~~~
>
>
> Now that linux/libc-compat.h is included in linux/if_ether.h, it is
> processed before netinet/in.h. Therefore, it sets the relevant
> __UAPI_DEF_* guards to 1 (as _NETINET_IN_H isn't yet defined).
> Then netinet/in.h is included, followed by linux/in.h. The later
> doesn't realise that what it defines has already been included by
> netinet/in.h because the __UAPI_DEF_* guards were set too early.
>
> Of course the situation is a bit more complicated on real projects, as
> these files aren't included directly. For example, in accel-ppp, the
> PPPoE module (accel-ppp/accel-pppd/ctrl/pppoe/pppoe.c) uses
> #include <net/ethernet.h> /* includes linux/if_ether.h */
> #include <arpa/inet.h> /* includes netinet/in.h */
> #include <linux/if_pppox.h> /* (through pppoe.h), includes linux/in.h */
>
>
> I don't have a satisfying solution for now, but I'd really like it if
> we could avoid shipping a kernel which forces userspace to play with
> include files ordering to keep compiling.
>
Hi,
This is about this commit:
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=6926e041a8920c8ec27e4e155efa760aa01551fd
On option would be to move this into include/uapi/linux/if_ether.h and
remove the include for libc-compat.h:
#ifndef __UAPI_DEF_ETHHDR
#define __UAPI_DEF_ETHHDR 1
#endif
This will only work if netinet/if_ether.h is included before
linux/if_ether.h, but I think this is very likely.
I think we can do this because we do not need some special libc handling
like it is done for other symbols as __UAPI_DEF_ETHHDR is currently only
needed by musl and not by glibc.
Hauke
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [regresssion 4.15] Userspace compilation broken by uapi/linux/if_ether.h update
2018-01-25 22:21 ` Hauke Mehrtens
@ 2018-01-26 10:51 ` Guillaume Nault
[not found] ` <20180126105138.GU1422-pHk1y4uTXVDytLWWfqlThQ@public.gmane.org>
0 siblings, 1 reply; 6+ messages in thread
From: Guillaume Nault @ 2018-01-26 10:51 UTC (permalink / raw)
To: Hauke Mehrtens; +Cc: netdev, linux-api
On Thu, Jan 25, 2018 at 11:21:34PM +0100, Hauke Mehrtens wrote:
> On 01/25/2018 03:58 PM, Guillaume Nault wrote:
> > Hi,
> >
> > Commit 6926e041a892 ("uapi/if_ether.h: prevent redefinition of struct ethhdr"),
> > can break compilation of userspace programs (in my case, accel-ppp
> > (https://accel-ppp.org)).
> >
> > This happens for userspace programs that end up including
> > linux/if_ether.h, netinet/in.h and linux/in.h in this order:
> >
> > # cat test_ifether.c
> > #include <linux/if_ether.h>
> > #include <netinet/in.h>
> > #include <linux/in.h>
> >
> > int main(void)
> > {
> > return 0;
> > }
> >
> > # gcc test_ifether.c
> > In file included from test_ifether.c:2:0:
> > /usr/include/linux/in.h:29:3: error: redeclaration of enumerator ‘IPPROTO_IP’
> > IPPROTO_IP = 0, /* Dummy protocol for TCP */
> > ^
> > /usr/include/netinet/in.h:42:5: note: previous definition of ‘IPPROTO_IP’ was here
> > IPPROTO_IP = 0, /* Dummy protocol for TCP. */
> > ^~~~~~~~~~
> >
> >
> > Now that linux/libc-compat.h is included in linux/if_ether.h, it is
> > processed before netinet/in.h. Therefore, it sets the relevant
> > __UAPI_DEF_* guards to 1 (as _NETINET_IN_H isn't yet defined).
> > Then netinet/in.h is included, followed by linux/in.h. The later
> > doesn't realise that what it defines has already been included by
> > netinet/in.h because the __UAPI_DEF_* guards were set too early.
> >
> > Of course the situation is a bit more complicated on real projects, as
> > these files aren't included directly. For example, in accel-ppp, the
> > PPPoE module (accel-ppp/accel-pppd/ctrl/pppoe/pppoe.c) uses
> > #include <net/ethernet.h> /* includes linux/if_ether.h */
> > #include <arpa/inet.h> /* includes netinet/in.h */
> > #include <linux/if_pppox.h> /* (through pppoe.h), includes linux/in.h */
> >
> >
> > I don't have a satisfying solution for now, but I'd really like it if
> > we could avoid shipping a kernel which forces userspace to play with
> > include files ordering to keep compiling.
> >
> Hi,
>
> This is about this commit:
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=6926e041a8920c8ec27e4e155efa760aa01551fd
>
> On option would be to move this into include/uapi/linux/if_ether.h and
> remove the include for libc-compat.h:
> #ifndef __UAPI_DEF_ETHHDR
> #define __UAPI_DEF_ETHHDR 1
> #endif
>
> This will only work if netinet/if_ether.h is included before
> linux/if_ether.h, but I think this is very likely.
>
I don't see what makes its likely. That's not directly related to your
point, but for example, glibc guarantees the opposite as it includes
linux/if_ether.h at the beginning of netinet/if_ether.h.
> I think we can do this because we do not need some special libc handling
> like it is done for other symbols as __UAPI_DEF_ETHHDR is currently only
> needed by musl and not by glibc.
>
That's ok for me as long as existing projects keep compiling. But all
__UAPI_DEF_* are currently centralised in libc-compat.h. Adding
__UAPI_DEF_ETHHDR in if_ether.h looks like defeating the purpose of
libc-compat.h and I wonder if that'd be accepted. Maybe with a
different name.
In any case, we're really late in the release cycle. If more discussion
is needed, it's probably better to revert and take time to work on a
solution for the next release.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [regresssion 4.15] Userspace compilation broken by uapi/linux/if_ether.h update
[not found] ` <20180126105138.GU1422-pHk1y4uTXVDytLWWfqlThQ@public.gmane.org>
@ 2018-01-26 17:17 ` Guillaume Nault
[not found] ` <20180126171723.GV1422-pHk1y4uTXVDytLWWfqlThQ@public.gmane.org>
2018-02-06 15:08 ` Guillaume Nault
1 sibling, 1 reply; 6+ messages in thread
From: Guillaume Nault @ 2018-01-26 17:17 UTC (permalink / raw)
To: David Miller, Hauke Mehrtens
Cc: netdev-u79uwXL29TY76Z2rM5mHXA, linux-api-u79uwXL29TY76Z2rM5mHXA
On Fri, Jan 26, 2018 at 11:51:38AM +0100, Guillaume Nault wrote:
> On Thu, Jan 25, 2018 at 11:21:34PM +0100, Hauke Mehrtens wrote:
> > On 01/25/2018 03:58 PM, Guillaume Nault wrote:
> > > Now that linux/libc-compat.h is included in linux/if_ether.h, it is
> > > processed before netinet/in.h. Therefore, it sets the relevant
> > > __UAPI_DEF_* guards to 1 (as _NETINET_IN_H isn't yet defined).
> > > Then netinet/in.h is included, followed by linux/in.h. The later
> > > doesn't realise that what it defines has already been included by
> > > netinet/in.h because the __UAPI_DEF_* guards were set too early.
> > >
> > This is about this commit:
> > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=6926e041a8920c8ec27e4e155efa760aa01551fd
> >
> > On option would be to move this into include/uapi/linux/if_ether.h and
> > remove the include for libc-compat.h:
> > #ifndef __UAPI_DEF_ETHHDR
> > #define __UAPI_DEF_ETHHDR 1
> > #endif
> >
> > This will only work if netinet/if_ether.h is included before
> > linux/if_ether.h, but I think this is very likely.
> >
> I don't see what makes its likely. That's not directly related to your
> point, but for example, glibc guarantees the opposite as it includes
> linux/if_ether.h at the beginning of netinet/if_ether.h.
>
> > I think we can do this because we do not need some special libc handling
> > like it is done for other symbols as __UAPI_DEF_ETHHDR is currently only
> > needed by musl and not by glibc.
> >
> That's ok for me as long as existing projects keep compiling. But all
> __UAPI_DEF_* are currently centralised in libc-compat.h. Adding
> __UAPI_DEF_ETHHDR in if_ether.h looks like defeating the purpose of
> libc-compat.h and I wonder if that'd be accepted. Maybe with a
> different name.
>
> In any case, we're really late in the release cycle. If more discussion
> is needed, it's probably better to revert and take time to work on a
> solution for the next release.
>
Hi David,
I just realise you've sent your last pull request for this release. I
was waiting for feedbacks in order to avoid a revert. Should I send a
revert now or do you prefer to sort this out later and backport a fix
in 4.15.1?
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [regresssion 4.15] Userspace compilation broken by uapi/linux/if_ether.h update
[not found] ` <20180126171723.GV1422-pHk1y4uTXVDytLWWfqlThQ@public.gmane.org>
@ 2018-01-26 19:21 ` David Miller
0 siblings, 0 replies; 6+ messages in thread
From: David Miller @ 2018-01-26 19:21 UTC (permalink / raw)
To: g.nault-pHk1y4uTXVDytLWWfqlThQ
Cc: hauke-5/S+JYg5SzeELgA04lAiVw, netdev-u79uwXL29TY76Z2rM5mHXA,
linux-api-u79uwXL29TY76Z2rM5mHXA
From: Guillaume Nault <g.nault-pHk1y4uTXVDytLWWfqlThQ@public.gmane.org>
Date: Fri, 26 Jan 2018 18:17:23 +0100
> On Fri, Jan 26, 2018 at 11:51:38AM +0100, Guillaume Nault wrote:
>> On Thu, Jan 25, 2018 at 11:21:34PM +0100, Hauke Mehrtens wrote:
>> > On 01/25/2018 03:58 PM, Guillaume Nault wrote:
>> > > Now that linux/libc-compat.h is included in linux/if_ether.h, it is
>> > > processed before netinet/in.h. Therefore, it sets the relevant
>> > > __UAPI_DEF_* guards to 1 (as _NETINET_IN_H isn't yet defined).
>> > > Then netinet/in.h is included, followed by linux/in.h. The later
>> > > doesn't realise that what it defines has already been included by
>> > > netinet/in.h because the __UAPI_DEF_* guards were set too early.
>> > >
>> > This is about this commit:
>> > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=6926e041a8920c8ec27e4e155efa760aa01551fd
>> >
>> > On option would be to move this into include/uapi/linux/if_ether.h and
>> > remove the include for libc-compat.h:
>> > #ifndef __UAPI_DEF_ETHHDR
>> > #define __UAPI_DEF_ETHHDR 1
>> > #endif
>> >
>> > This will only work if netinet/if_ether.h is included before
>> > linux/if_ether.h, but I think this is very likely.
>> >
>> I don't see what makes its likely. That's not directly related to your
>> point, but for example, glibc guarantees the opposite as it includes
>> linux/if_ether.h at the beginning of netinet/if_ether.h.
>>
>> > I think we can do this because we do not need some special libc handling
>> > like it is done for other symbols as __UAPI_DEF_ETHHDR is currently only
>> > needed by musl and not by glibc.
>> >
>> That's ok for me as long as existing projects keep compiling. But all
>> __UAPI_DEF_* are currently centralised in libc-compat.h. Adding
>> __UAPI_DEF_ETHHDR in if_ether.h looks like defeating the purpose of
>> libc-compat.h and I wonder if that'd be accepted. Maybe with a
>> different name.
>>
>> In any case, we're really late in the release cycle. If more discussion
>> is needed, it's probably better to revert and take time to work on a
>> solution for the next release.
>>
> Hi David,
>
> I just realise you've sent your last pull request for this release. I
> was waiting for feedbacks in order to avoid a revert. Should I send a
> revert now or do you prefer to sort this out later and backport a fix
> in 4.15.1?
We can do a -stable backport, and I was planning to help looking into this
as well.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [regresssion 4.15] Userspace compilation broken by uapi/linux/if_ether.h update
[not found] ` <20180126105138.GU1422-pHk1y4uTXVDytLWWfqlThQ@public.gmane.org>
2018-01-26 17:17 ` Guillaume Nault
@ 2018-02-06 15:08 ` Guillaume Nault
1 sibling, 0 replies; 6+ messages in thread
From: Guillaume Nault @ 2018-02-06 15:08 UTC (permalink / raw)
To: Hauke Mehrtens
Cc: netdev-u79uwXL29TY76Z2rM5mHXA, linux-api-u79uwXL29TY76Z2rM5mHXA
On Fri, Jan 26, 2018 at 11:51:38AM +0100, Guillaume Nault wrote:
> On Thu, Jan 25, 2018 at 11:21:34PM +0100, Hauke Mehrtens wrote:
> > On 01/25/2018 03:58 PM, Guillaume Nault wrote:
> > > Hi,
> > >
> > > Commit 6926e041a892 ("uapi/if_ether.h: prevent redefinition of struct ethhdr"),
> > > can break compilation of userspace programs (in my case, accel-ppp
> > > (https://accel-ppp.org)).
> > >
> > > This happens for userspace programs that end up including
> > > linux/if_ether.h, netinet/in.h and linux/in.h in this order:
> > >
> > > # cat test_ifether.c
> > > #include <linux/if_ether.h>
> > > #include <netinet/in.h>
> > > #include <linux/in.h>
> > >
> > > int main(void)
> > > {
> > > return 0;
> > > }
> > >
> > > # gcc test_ifether.c
> > > In file included from test_ifether.c:2:0:
> > > /usr/include/linux/in.h:29:3: error: redeclaration of enumerator ‘IPPROTO_IP’
> > > IPPROTO_IP = 0, /* Dummy protocol for TCP */
> > > ^
> > > /usr/include/netinet/in.h:42:5: note: previous definition of ‘IPPROTO_IP’ was here
> > > IPPROTO_IP = 0, /* Dummy protocol for TCP. */
> > > ^~~~~~~~~~
> > >
> > >
> > > Now that linux/libc-compat.h is included in linux/if_ether.h, it is
> > > processed before netinet/in.h. Therefore, it sets the relevant
> > > __UAPI_DEF_* guards to 1 (as _NETINET_IN_H isn't yet defined).
> > > Then netinet/in.h is included, followed by linux/in.h. The later
> > > doesn't realise that what it defines has already been included by
> > > netinet/in.h because the __UAPI_DEF_* guards were set too early.
> > >
> > > Of course the situation is a bit more complicated on real projects, as
> > > these files aren't included directly. For example, in accel-ppp, the
> > > PPPoE module (accel-ppp/accel-pppd/ctrl/pppoe/pppoe.c) uses
> > > #include <net/ethernet.h> /* includes linux/if_ether.h */
> > > #include <arpa/inet.h> /* includes netinet/in.h */
> > > #include <linux/if_pppox.h> /* (through pppoe.h), includes linux/in.h */
> > >
> > >
> > > I don't have a satisfying solution for now, but I'd really like it if
> > > we could avoid shipping a kernel which forces userspace to play with
> > > include files ordering to keep compiling.
> > >
> > Hi,
> >
> > This is about this commit:
> > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=6926e041a8920c8ec27e4e155efa760aa01551fd
> >
> > On option would be to move this into include/uapi/linux/if_ether.h and
> > remove the include for libc-compat.h:
> > #ifndef __UAPI_DEF_ETHHDR
> > #define __UAPI_DEF_ETHHDR 1
> > #endif
> >
Hauke, please submit a patch implementing this approach and let's see
where it goes.
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2018-02-06 15:08 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-01-25 14:58 [regresssion 4.15] Userspace compilation broken by uapi/linux/if_ether.h update Guillaume Nault
2018-01-25 22:21 ` Hauke Mehrtens
2018-01-26 10:51 ` Guillaume Nault
[not found] ` <20180126105138.GU1422-pHk1y4uTXVDytLWWfqlThQ@public.gmane.org>
2018-01-26 17:17 ` Guillaume Nault
[not found] ` <20180126171723.GV1422-pHk1y4uTXVDytLWWfqlThQ@public.gmane.org>
2018-01-26 19:21 ` David Miller
2018-02-06 15:08 ` Guillaume Nault
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).