[PATCH v2 0/3] uapi glibc compat: fix musl libc compatibility

[PATCH v2 0/3] uapi glibc compat: fix musl libc compatibility

[Hauke Mehrtens]

The code from libc-compat.h depends on some glibc specific defines and 
causes compile problems with the musl libc. These patches remove some 
of the glibc dependencies. With these patches the LEDE (OpenWrt) base 
user space applications can be build with unmodified kernel headers and 
musl libc.

This was compile tested with the user space from LEDE (OpenWrt) with 
musl 1.1.16, glibc 2.25 and uClibc-ng 1.0.22.

Changes since v1:
 - fix typo in commit message
 - combine two changes


David Heidelberger (1):
  uapi/if_ether.h: prevent redefinition of struct ethhdr

Hauke Mehrtens (2):
  uapi glibc compat: add libc compat code when not build for kernel
  uapi glibc compat: fix build if libc defines IFF_ECHO

 include/uapi/linux/if_ether.h    |  3 +++
 include/uapi/linux/libc-compat.h | 25 +++++++++++++++++++------
 2 files changed, 22 insertions(+), 6 deletions(-)

-- 
2.11.0

[PATCH v2 1/3] uapi glibc compat: add libc compat code when not build for kernel

[Hauke Mehrtens]

Instead of checking if this header file is used in the glibc, check if
it is not used in kernel context, this way it will also work with other
libc implementations like musl.

The __USE_MISC symbol is glibc specific and not available in musl libc.
Only do this check when glibc is used.

This is based on an older patch from David Heidelberger
<david.heidelberger@ixit.cz>

Acked-by: Mikko Rapeli <mikko.rapeli@iki.fi>
Signed-off-by: Hauke Mehrtens <hauke@hauke-m.de>
---
 include/uapi/linux/libc-compat.h | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/include/uapi/linux/libc-compat.h b/include/uapi/linux/libc-compat.h
index 44b8a6bd5fe1..43a81136ea6e 100644
--- a/include/uapi/linux/libc-compat.h
+++ b/include/uapi/linux/libc-compat.h
@@ -48,11 +48,11 @@
 #ifndef _UAPI_LIBC_COMPAT_H
 #define _UAPI_LIBC_COMPAT_H
 
-/* We have included glibc headers... */
-#if defined(__GLIBC__)
+/* We have included libc headers... */
+#if !defined(__KERNEL__)
 
-/* Coordinate with glibc net/if.h header. */
-#if defined(_NET_IF_H) && defined(__USE_MISC)
+/* Coordinate with libc net/if.h header. */
+#if defined(_NET_IF_H) && (!defined(__GLIBC__) || defined(__USE_MISC))
 
 /* GLIBC headers included first so don't define anything
  * that would already be defined. */
@@ -168,7 +168,7 @@
 /* If we did not see any headers from any supported C libraries,
  * or we are being included in the kernel, then define everything
  * that we need. */
-#else /* !defined(__GLIBC__) */
+#else /* defined(__KERNEL__) */
 
 /* Definitions for if.h */
 #define __UAPI_DEF_IF_IFCONF 1
@@ -208,6 +208,6 @@
 /* Definitions for xattr.h */
 #define __UAPI_DEF_XATTR		1
 
-#endif /* __GLIBC__ */
+#endif /* __KERNEL__ */
 
 #endif /* _UAPI_LIBC_COMPAT_H */
-- 
2.11.0

[PATCH v2 2/3] uapi glibc compat: fix build if libc defines IFF_ECHO

[Hauke Mehrtens]

musl 1.1.15 defines IFF_ECHO and the other net_device_flags options.
When a user application includes linux/if.h and net/if.h the compile
will fail.

Activate __UAPI_DEF_IF_NET_DEVICE_FLAGS_LOWER_UP_DORMANT_ECHO only when
it is needed. This should also make this work in case glibc will add
these defines.

Acked-by: Mikko Rapeli <mikko.rapeli@iki.fi>
Signed-off-by: Hauke Mehrtens <hauke@hauke-m.de>
---
 include/uapi/linux/libc-compat.h | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/include/uapi/linux/libc-compat.h b/include/uapi/linux/libc-compat.h
index 43a81136ea6e..ce2fa8a4ced6 100644
--- a/include/uapi/linux/libc-compat.h
+++ b/include/uapi/linux/libc-compat.h
@@ -64,9 +64,11 @@
 /* Everything up to IFF_DYNAMIC, matches net/if.h until glibc 2.23 */
 #define __UAPI_DEF_IF_NET_DEVICE_FLAGS 0
 /* For the future if glibc adds IFF_LOWER_UP, IFF_DORMANT and IFF_ECHO */
+#ifndef IFF_ECHO
 #ifndef __UAPI_DEF_IF_NET_DEVICE_FLAGS_LOWER_UP_DORMANT_ECHO
 #define __UAPI_DEF_IF_NET_DEVICE_FLAGS_LOWER_UP_DORMANT_ECHO 1
 #endif /* __UAPI_DEF_IF_NET_DEVICE_FLAGS_LOWER_UP_DORMANT_ECHO */
+#endif /* IFF_ECHO */
 
 #else /* _NET_IF_H */
 
-- 
2.11.0

[PATCH v2 3/3] uapi/if_ether.h: prevent redefinition of struct ethhdr

[Hauke Mehrtens]

From: David Heidelberger <david.heidelberger@ixit.cz>

Musl provides its own ethhdr struct definition. Add a guard to prevent
its definition of the appropriate musl header has already been included.

Acked-by: Mikko Rapeli <mikko.rapeli@iki.fi>
Signed-off-by: John Spencer <maillist-linux@barfooze.de>
Tested-by: David Heidelberger <david.heidelberger@ixit.cz>
Signed-off-by: Jonas Gorski <jogo@openwrt.org>
Signed-off-by: Hauke Mehrtens <hauke@hauke-m.de>
---
 include/uapi/linux/if_ether.h    |  3 +++
 include/uapi/linux/libc-compat.h | 11 +++++++++++
 2 files changed, 14 insertions(+)

diff --git a/include/uapi/linux/if_ether.h b/include/uapi/linux/if_ether.h
index 5bc9bfd816b7..fb5ab8c1e753 100644
--- a/include/uapi/linux/if_ether.h
+++ b/include/uapi/linux/if_ether.h
@@ -22,6 +22,7 @@
 #define _UAPI_LINUX_IF_ETHER_H
 
 #include <linux/types.h>
+#include <linux/libc-compat.h>
 
 /*
  *	IEEE 802.3 Ethernet magic constants.  The frame sizes omit the preamble
@@ -142,11 +143,13 @@
  *	This is an Ethernet frame header.
  */
 
+#if __UAPI_DEF_ETHHDR
 struct ethhdr {
 	unsigned char	h_dest[ETH_ALEN];	/* destination eth addr	*/
 	unsigned char	h_source[ETH_ALEN];	/* source ether addr	*/
 	__be16		h_proto;		/* packet type ID field	*/
 } __attribute__((packed));
+#endif
 
 
 #endif /* _UAPI_LINUX_IF_ETHER_H */
diff --git a/include/uapi/linux/libc-compat.h b/include/uapi/linux/libc-compat.h
index ce2fa8a4ced6..c92d32f213d1 100644
--- a/include/uapi/linux/libc-compat.h
+++ b/include/uapi/linux/libc-compat.h
@@ -87,6 +87,14 @@
 
 #endif /* _NET_IF_H */
 
+/* musl defines the ethhdr struct itself in its netinet/if_ether.h.
+ * Glibc just includes the kernel header and uses a different guard. */
+#if defined(_NETINET_IF_ETHER_H)
+#define __UAPI_DEF_ETHHDR		0
+#else
+#define __UAPI_DEF_ETHHDR		1
+#endif
+
 /* Coordinate with glibc netinet/in.h header. */
 #if defined(_NETINET_IN_H)
 
@@ -182,6 +190,9 @@
 /* For the future if glibc adds IFF_LOWER_UP, IFF_DORMANT and IFF_ECHO */
 #define __UAPI_DEF_IF_NET_DEVICE_FLAGS_LOWER_UP_DORMANT_ECHO 1
 
+/* Definitions for if_ether.h */
+#define __UAPI_DEF_ETHHDR 		1
+
 /* Definitions for in.h */
 #define __UAPI_DEF_IN_ADDR		1
 #define __UAPI_DEF_IN_IPPROTO		1
-- 
2.11.0

Re: [PATCH v2 0/3] uapi glibc compat: fix musl libc compatibility

[David Miller]

From: Hauke Mehrtens <hauke@hauke-m.de>
Date: Tue, 18 Apr 2017 23:00:33 +0200

> The code from libc-compat.h depends on some glibc specific defines and 
> causes compile problems with the musl libc. These patches remove some 
> of the glibc dependencies. With these patches the LEDE (OpenWrt) base 
> user space applications can be build with unmodified kernel headers and 
> musl libc.
> 
> This was compile tested with the user space from LEDE (OpenWrt) with 
> musl 1.1.16, glibc 2.25 and uClibc-ng 1.0.22.
> 
> Changes since v1:
>  - fix typo in commit message
>  - combine two changes

I think I have to put the brakes on this patch series, after much
consideration.

It does not scale if we continually add a hodge-podge of different
ifdef tests to the UAPI headers in order to prevent mutliple
definitions.

We will add that IFF_ECHO ifdef for MUSL libc today, and for another
libc we will add another such hack.  And so on and so forth...

Instead, LIBC implementation must adopt the ifdef protections which
have standard names and are being adopted by GLIBC and hopefully
others.

So please instead adjust the musl headers so that they interact
properly with the framework we've designed specifically for this
purpose.

Thank you.

Re: [PATCH v2 0/3] uapi glibc compat: fix musl libc compatibility

[David Woodhouse]

[-- Attachment #1: Type: text/plain, Size: 911 bytes --]

On Thu, 2017-04-20 at 16:07 -0400, David Miller wrote:
> 
> I think I have to put the brakes on this patch series, after much
> consideration.
> 
> It does not scale if we continually add a hodge-podge of different
> ifdef tests to the UAPI headers in order to prevent mutliple
> definitions.
> 
> We will add that IFF_ECHO ifdef for MUSL libc today, and for another
> libc we will add another such hack.  And so on and so forth...
> 
> Instead, LIBC implementation must adopt the ifdef protections which
> have standard names and are being adopted by GLIBC and hopefully
> others.

I agree, except I don't think you're going far enough. Those "standard
names" you mention... some of this stuff actually depends on __GLIBC__,
and *that* isn't right either.

I tried to kill that off completely and make the kernel entirely
agnostic, in https://marc.info/?l=linux-api&m=148898383805658&w=2

[-- Attachment #2: smime.p7s --]
[-- Type: application/x-pkcs7-signature, Size: 4938 bytes --]

Re: [PATCH v2 0/3] uapi glibc compat: fix musl libc compatibility

[David Miller]

From: David Woodhouse <dwmw2@infradead.org>
Date: Thu, 20 Apr 2017 21:14:37 +0100

> I agree, except I don't think you're going far enough. Those "standard
> names" you mention... some of this stuff actually depends on __GLIBC__,
> and *that* isn't right either.

Yep, that's something that needs correcting.

Re: [PATCH v2 0/3] uapi glibc compat: fix musl libc compatibility

[Hauke Mehrtens]

On 04/20/2017 10:36 PM, David Miller wrote:
> From: David Woodhouse <dwmw2@infradead.org>
> Date: Thu, 20 Apr 2017 21:14:37 +0100
> 
>> I agree, except I don't think you're going far enough. Those "standard
>> names" you mention... some of this stuff actually depends on __GLIBC__,
>> and *that* isn't right either.
> 
> Yep, that's something that needs correcting.
> 
Should all libc implementations define __GLIBC__ or could we at least
switch the kernel UAPI to !__KERNEL__ here?

Hauke

Re: [PATCH v2 0/3] uapi glibc compat: fix musl libc compatibility

[David Woodhouse]

[-- Attachment #1: Type: text/plain, Size: 658 bytes --]

On Fri, 2017-04-21 at 15:14 +0200, Hauke Mehrtens wrote:
> 
> On 04/20/2017 10:36 PM, David Miller wrote:
> > 
> > From: David Woodhouse <dwmw2@infradead.org>
> > Date: Thu, 20 Apr 2017 21:14:37 +0100
> > 
> > > 
> > > I agree, except I don't think you're going far enough. Those
> > > "standard
> > > names" you mention... some of this stuff actually depends on
> > > __GLIBC__,
> > > and *that* isn't right either.
> > Yep, that's something that needs correcting.
> > 
> Should all libc implementations define __GLIBC__ or could we at least
> switch the kernel UAPI to !__KERNEL__ here?

I'd start with the patch I referenced yesterday...

[-- Attachment #2: smime.p7s --]
[-- Type: application/x-pkcs7-signature, Size: 4938 bytes --]

Re: [musl] Re: [PATCH v2 0/3] uapi glibc compat: fix musl libc compatibility

[Rich Felker]

On Fri, Apr 21, 2017 at 03:14:21PM +0200, Hauke Mehrtens wrote:
> 
> 
> On 04/20/2017 10:36 PM, David Miller wrote:
> > From: David Woodhouse <dwmw2@infradead.org>
> > Date: Thu, 20 Apr 2017 21:14:37 +0100
> > 
> >> I agree, except I don't think you're going far enough. Those "standard
> >> names" you mention... some of this stuff actually depends on __GLIBC__,
> >> and *that* isn't right either.
> > 
> > Yep, that's something that needs correcting.
> > 
> Should all libc implementations define __GLIBC__

Absolutely not.

Rich