uapi glibc compat: fix cases where glibc net/if.h is included before linux/if.h

glibc's net/if.h contains copies of definitions from linux/if.h and these
conflict and cause build failures if both files are included by application
source code. Changes in uapi headers, which fixed header file dependencies to
include linux/if.h when it was needed, made the net/if.h and linux/if.h
incompatibilities visible as build failures for userspace applications
like iproute2.

This patch fixes the cases where glibc net/if.h is included before linux/if.h
by using the linux/libc-compat.h approach.

The cases where linux/if.h is included before net/if.h need a similar fix in
the glibc side, or the order of include files can be changed userspace
code as a workaround.

This change was tested in x86 userspace after 'make headers_install' and
combining these headers with plain glibc and gcc headers. The test included
not just net/if.h but all compiling glibc headers with linux/if.h and it
compiles now without errors and warnings.

 $ cat usr/libc_conflict_test/libc_headers.h
 #include <aio.h>
 #include <aliases.h>
 #include <alloca.h>
 #include <argp.h>
 #include <argz.h>
 #include <ar.h>
 //these conflict with each other
 // #include <arpa/ftp.h>
 // #include <arpa/inet.h>
 // #include <arpa/nameser_compat.h>
 // #include <arpa/nameser.h>
 // #include <arpa/telnet.h>
 // #include <arpa/tftp.h>
 #include <assert.h>
 #include <byteswap.h>
 #include <complex.h>
 #include <cpio.h>
 #include <crypt.h>
 #include <ctype.h>
 #include <dirent.h>

151 more lines

diff --git a/include/uapi/linux/if.h b/include/uapi/linux/if.h
index 9cf2394..5a07a67 100644
--- a/include/uapi/linux/if.h
+++ b/include/uapi/linux/if.h

@@ -19,11 +19,15 @@
 #ifndef _LINUX_IF_H
 #define _LINUX_IF_H
 
+#include <linux/libc-compat.h>          /* for compatibility with glibc */
 #include <linux/types.h>		/* for "__kernel_caddr_t" et al	*/
 #include <linux/socket.h>		/* for "struct sockaddr" et al	*/
 #include <linux/compiler.h>		/* for "__user" et al           */
 
+/* For compatibility with glibc net/if.h */
+#if __UAPI_DEF_IF_IFNAMSIZ
 #define	IFNAMSIZ	16
+#endif /* __UAPI_DEF_IF_IFNAMSIZ */
 #define	IFALIASZ	256
 #include <linux/hdlc/ioctl.h>
 

@@ -68,6 +72,8 @@
  * @IFF_ECHO: echo sent packets. Volatile.
  */
 enum net_device_flags {
+/* for compatibility with glibc net/if.h */
+#if __UAPI_DEF_IF_NET_DEVICE_FLAGS
 	IFF_UP				= 1<<0,  /* sysfs */
 	IFF_BROADCAST			= 1<<1,  /* volatile */
 	IFF_DEBUG			= 1<<2,  /* sysfs */

@@ -84,11 +90,14 @@ enum net_device_flags {
 	IFF_PORTSEL			= 1<<13, /* sysfs */
 	IFF_AUTOMEDIA			= 1<<14, /* sysfs */
 	IFF_DYNAMIC			= 1<<15, /* sysfs */
+#endif /* __UAPI_DEF_IF_NET_DEVICE_FLAGS */
 	IFF_LOWER_UP			= 1<<16, /* volatile */
 	IFF_DORMANT			= 1<<17, /* volatile */
 	IFF_ECHO			= 1<<18, /* volatile */
 };
 
+/* for compatibility with glibc net/if.h */
+#if __UAPI_DEF_IF_NET_DEVICE_FLAGS
 #define IFF_UP				IFF_UP
 #define IFF_BROADCAST			IFF_BROADCAST
 #define IFF_DEBUG			IFF_DEBUG

@@ -105,6 +114,8 @@ enum net_device_flags {
 #define IFF_PORTSEL			IFF_PORTSEL
 #define IFF_AUTOMEDIA			IFF_AUTOMEDIA
 #define IFF_DYNAMIC			IFF_DYNAMIC
+#endif /* __UAPI_DEF_IF_NET_DEVICE_FLAGS */
+
 #define IFF_LOWER_UP			IFF_LOWER_UP
 #define IFF_DORMANT			IFF_DORMANT
 #define IFF_ECHO			IFF_ECHO

@@ -166,6 +177,8 @@ enum {
  *	being very small might be worth keeping for clean configuration.
  */
 
+/* for compatibility with glibc net/if.h */
+#if __UAPI_DEF_IF_IFMAP
 struct ifmap {
 	unsigned long mem_start;
 	unsigned long mem_end;

@@ -175,6 +188,7 @@ struct ifmap {
 	unsigned char port;
 	/* 3 bytes spare */
 };
+#endif /* __UAPI_DEF_IF_IFMAP */
 
 struct if_settings {
 	unsigned int type;	/* Type of physical device or protocol */

@@ -200,6 +214,8 @@ struct if_settings {
  * remainder may be interface specific.
  */
 
+/* for compatibility with glibc net/if.h */
+#if __UAPI_DEF_IF_IFREQ
 struct ifreq {
 #define IFHWADDRLEN	6
 	union

@@ -223,6 +239,7 @@ struct ifreq {
 		struct	if_settings ifru_settings;
 	} ifr_ifru;
 };
+#endif /* __UAPI_DEF_IF_IFREQ */
 
 #define ifr_name	ifr_ifrn.ifrn_name	/* interface name 	*/
 #define ifr_hwaddr	ifr_ifru.ifru_hwaddr	/* MAC address 		*/

@@ -249,6 +266,8 @@ struct ifreq {
  * must know all networks accessible).
  */
 
+/* for compatibility with glibc net/if.h */
+#if __UAPI_DEF_IF_IFCONF
 struct ifconf  {
 	int	ifc_len;			/* size of buffer	*/
 	union {

@@ -256,6 +275,8 @@ struct ifconf  {
 		struct ifreq __user *ifcu_req;
 	} ifc_ifcu;
 };
+#endif /* __UAPI_DEF_IF_IFCONF */
+
 #define	ifc_buf	ifc_ifcu.ifcu_buf		/* buffer address	*/
 #define	ifc_req	ifc_ifcu.ifcu_req		/* array of structures	*/
 

diff --git a/include/uapi/linux/libc-compat.h b/include/uapi/linux/libc-compat.h
index 7d024ce..9211e52 100644
--- a/include/uapi/linux/libc-compat.h
+++ b/include/uapi/linux/libc-compat.h

@@ -51,6 +51,32 @@
 /* We have included glibc headers... */
 #if defined(__GLIBC__)
 
+/* Coordinate with glibc net/if.h header. */
+#if defined(_NET_IF_H)
+
+/* GLIBC headers included first so don't define anything
+ * that would already be defined. */
+
+#define __UAPI_DEF_IF_IFCONF 0
+#define __UAPI_DEF_IF_IFMAP 0
+#define __UAPI_DEF_IF_IFNAMSIZ 0
+#define __UAPI_DEF_IF_IFREQ 0
+#define __UAPI_DEF_IF_NET_DEVICE_FLAGS 0
+
+#else /* _NET_IF_H */
+
+/* Linux headers included first, and we must define everything
+ * we need. The expectation is that glibc will check the
+ * __UAPI_DEF_* defines and adjust appropriately. */
+
+#define __UAPI_DEF_IF_IFCONF 1
+#define __UAPI_DEF_IF_IFMAP 1
+#define __UAPI_DEF_IF_IFNAMSIZ 1
+#define __UAPI_DEF_IF_IFREQ 1
+#define __UAPI_DEF_IF_NET_DEVICE_FLAGS 1
+
+#endif /* _NET_IF_H */
+
 /* Coordinate with glibc netinet/in.h header. */
 #if defined(_NETINET_IN_H)
 

@@ -117,6 +143,13 @@
  * that we need. */
 #else /* !defined(__GLIBC__) */
 
+/* Definitions for if.h */
+#define __UAPI_DEF_IF_IFCONF 1
+#define __UAPI_DEF_IF_IFMAP 1
+#define __UAPI_DEF_IF_IFNAMSIZ 1
+#define __UAPI_DEF_IF_IFREQ 1
+#define __UAPI_DEF_IF_NET_DEVICE_FLAGS 1
+
 /* Definitions for in.h */
 #define __UAPI_DEF_IN_ADDR		1
 #define __UAPI_DEF_IN_IPPROTO		1

-- 
2.7.0.rc3

From: Mikko Rapeli <redacted>
Date: Sun,  7 Feb 2016 16:03:21 +0200

This is going to get messy is IFF_LOWER_UP, IFF_DORMANT, and IFF_ECHO
get added the the glibc header.  Why not just handle it now with
another __UAPI_DEF_FOO guard so that the additions to net/if.h can
deal with this case too.

Likewise.

(Adding libc-alpha list, review of https://lkml.org/lkml/2016/2/7/89 )

On Wed, Feb 17, 2016 at 10:46:20AM -0500, David Miller wrote:

From: Mikko Rapeli <redacted>
Date: Sun,  7 Feb 2016 16:03:21 +0200

quoted

@@ -68,6 +72,8 @@
  * @IFF_ECHO: echo sent packets. Volatile.
  */
 enum net_device_flags {
+/* for compatibility with glibc net/if.h */
+#if __UAPI_DEF_IF_NET_DEVICE_FLAGS
 	IFF_UP				= 1<<0,  /* sysfs */
 	IFF_BROADCAST			= 1<<1,  /* volatile */
 	IFF_DEBUG			= 1<<2,  /* sysfs */

@@ -84,11 +90,14 @@ enum net_device_flags {
 	IFF_PORTSEL			= 1<<13, /* sysfs */
 	IFF_AUTOMEDIA			= 1<<14, /* sysfs */
 	IFF_DYNAMIC			= 1<<15, /* sysfs */
+#endif /* __UAPI_DEF_IF_NET_DEVICE_FLAGS */
 	IFF_LOWER_UP			= 1<<16, /* volatile */
 	IFF_DORMANT			= 1<<17, /* volatile */
 	IFF_ECHO			= 1<<18, /* volatile */
 };

This is going to get messy is IFF_LOWER_UP, IFF_DORMANT, and IFF_ECHO
get added the the glibc header.  Why not just handle it now with
another __UAPI_DEF_FOO guard so that the additions to net/if.h can
deal with this case too.

Do you mean that the enum should be protected with a single guard or
should I have one guard for current conflicts and one for the future
if glibc headers include IFF_LOWER_UP and others?

-Mikko

From: Mikko Rapeli <redacted>
Date: Fri, 26 Feb 2016 09:25:13 +0200

(Adding libc-alpha list, review of https://lkml.org/lkml/2016/2/7/89 )

On Wed, Feb 17, 2016 at 10:46:20AM -0500, David Miller wrote:

quoted

From: Mikko Rapeli <redacted>
Date: Sun,  7 Feb 2016 16:03:21 +0200

quoted

@@ -68,6 +72,8 @@
  * @IFF_ECHO: echo sent packets. Volatile.
  */
 enum net_device_flags {
+/* for compatibility with glibc net/if.h */
+#if __UAPI_DEF_IF_NET_DEVICE_FLAGS
 	IFF_UP				= 1<<0,  /* sysfs */
 	IFF_BROADCAST			= 1<<1,  /* volatile */
 	IFF_DEBUG			= 1<<2,  /* sysfs */

@@ -84,11 +90,14 @@ enum net_device_flags {
 	IFF_PORTSEL			= 1<<13, /* sysfs */
 	IFF_AUTOMEDIA			= 1<<14, /* sysfs */
 	IFF_DYNAMIC			= 1<<15, /* sysfs */
+#endif /* __UAPI_DEF_IF_NET_DEVICE_FLAGS */
 	IFF_LOWER_UP			= 1<<16, /* volatile */
 	IFF_DORMANT			= 1<<17, /* volatile */
 	IFF_ECHO			= 1<<18, /* volatile */
 };

This is going to get messy is IFF_LOWER_UP, IFF_DORMANT, and IFF_ECHO
get added the the glibc header.  Why not just handle it now with
another __UAPI_DEF_FOO guard so that the additions to net/if.h can
deal with this case too.

Do you mean that the enum should be protected with a single guard or
should I have one guard for current conflicts and one for the future
if glibc headers include IFF_LOWER_UP and others?

I'm ambivalent about the mechanism, and I'm more concerned about covering
those three values in your change rather than eliding them.