header conflict introduced by change to netfilter_ipv4/ip_tables.h

header conflict introduced by change to netfilter_ipv4/ip_tables.h

[Stephen Hemminger]

This commit breaks compilation of iproute2 with net-next.

commit 1ffad83dffd675cd742286ae82dca7d746cb0da8
Author: Mikko Rapeli <mikko.rapeli@iki.fi>
Date:   Thu Oct 15 07:56:30 2015 +0200

    netfilter: fix include files for compilation
    
    Add missing header dependencies and other small changes so that each file
    compiles alone in userspace.
    
    Signed-off-by: Mikko Rapeli <mikko.rapeli@iki.fi>
    Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>

For iproute2, a copy of kernel headers (from make install_headers) is used.
After this change. the build of x_tables.c fails because IFNAMSIZ is already
defined in net/if.h

gcc -Wall -Wstrict-prototypes  -Wmissing-prototypes -Wmissing-declarations -Wold-style-definition -Wformat=2 -O2 -I../include -DRESOLVE_HOSTNAMES -DLIBDIR=\"/usr/lib\" -DCONFDIR=\"/etc/iproute2\" -D_GNU_SOURCE -D_FILE_OFFSET_BITS=64 -D_LARGEFILE_SOURCE -D_LARGEFILE64_SOURCE  -DHAVE_SETNS -DHAVE_ELF -DCONFIG_GACT -DCONFIG_GACT_PROB -DIPT_LIB_DIR=\"/lib/xtables\" -DYY_NO_INPUT -Wl,-export-dynamic -shared -fpic -o q_atm.so q_atm.c -latm
gcc -Wall -Wstrict-prototypes  -Wmissing-prototypes -Wmissing-declarations -Wold-style-definition -Wformat=2 -O2 -I../include -DRESOLVE_HOSTNAMES -DLIBDIR=\"/usr/lib\" -DCONFDIR=\"/etc/iproute2\" -D_GNU_SOURCE -D_FILE_OFFSET_BITS=64 -D_LARGEFILE_SOURCE -D_LARGEFILE64_SOURCE  -DHAVE_SETNS -DHAVE_ELF -DCONFIG_GACT -DCONFIG_GACT_PROB -DIPT_LIB_DIR=\"/lib/xtables\" -DYY_NO_INPUT -Wl,-export-dynamic -shared -fpic -o m_xt.so m_xt.c $(pkg-config xtables --cflags --libs)
In file included from ../include/linux/netfilter_ipv4/ip_tables.h:20:0,
                 from m_xt.c:20:
../include/linux/if.h:26:0: warning: "IFNAMSIZ" redefined
 #define IFNAMSIZ 16
 ^
In file included from m_xt.c:17:0:
/usr/include/net/if.h:129:0: note: this is the location of the previous definition
 # define IFNAMSIZ IF_NAMESIZE
 ^
../include/linux/if.h:71:2: error: redeclaration of enumerator ‘IFF_UP’
  IFF_UP    = 1<<0,  /* sysfs */
  ^
/usr/include/net/if.h:44:5: note: previous definition of ‘IFF_UP’ was here
     IFF_UP = 0x1,  /* Interface is up.  */
     ^
../include/linux/if.h:72:2: error: redeclaration of enumerator ‘IFF_BROADCAST’
  IFF_BROADCAST   = 1<<1,  /* __volatile__ */
  ^
/usr/include/net/if.h:46:5: note: previous definition of ‘IFF_BROADCAST’ was here
     IFF_BROADCAST = 0x2, /* Broadcast address valid.  */
     ^
../include/linux/if.h:73:2: error: redeclaration of enumerator ‘IFF_DEBUG’
  IFF_DEBUG   = 1<<2,  /* sysfs */

Re: header conflict introduced by change to netfilter_ipv4/ip_tables.h

[Mikko Rapeli]

On Wed, Jan 06, 2016 at 09:20:07AM -0800, Stephen Hemminger wrote:
> This commit breaks compilation of iproute2 with net-next.

Ok, linux/if.h and libc net/if.h have overlapping defines, and this is not
the only one. I saw lots of them in the core dump headers.

How should we handle them? Another ifndef for IFNAMSIZ into kernel uapi
headers?

-Mikko

> commit 1ffad83dffd675cd742286ae82dca7d746cb0da8
> Author: Mikko Rapeli <mikko.rapeli@iki.fi>
> Date:   Thu Oct 15 07:56:30 2015 +0200
> 
>     netfilter: fix include files for compilation
>     
>     Add missing header dependencies and other small changes so that each file
>     compiles alone in userspace.
>     
>     Signed-off-by: Mikko Rapeli <mikko.rapeli@iki.fi>
>     Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
> 
> For iproute2, a copy of kernel headers (from make install_headers) is used.
> After this change. the build of x_tables.c fails because IFNAMSIZ is already
> defined in net/if.h
> 
> gcc -Wall -Wstrict-prototypes  -Wmissing-prototypes -Wmissing-declarations -Wold-style-definition -Wformat=2 -O2 -I../include -DRESOLVE_HOSTNAMES -DLIBDIR=\"/usr/lib\" -DCONFDIR=\"/etc/iproute2\" -D_GNU_SOURCE -D_FILE_OFFSET_BITS=64 -D_LARGEFILE_SOURCE -D_LARGEFILE64_SOURCE  -DHAVE_SETNS -DHAVE_ELF -DCONFIG_GACT -DCONFIG_GACT_PROB -DIPT_LIB_DIR=\"/lib/xtables\" -DYY_NO_INPUT -Wl,-export-dynamic -shared -fpic -o q_atm.so q_atm.c -latm
> gcc -Wall -Wstrict-prototypes  -Wmissing-prototypes -Wmissing-declarations -Wold-style-definition -Wformat=2 -O2 -I../include -DRESOLVE_HOSTNAMES -DLIBDIR=\"/usr/lib\" -DCONFDIR=\"/etc/iproute2\" -D_GNU_SOURCE -D_FILE_OFFSET_BITS=64 -D_LARGEFILE_SOURCE -D_LARGEFILE64_SOURCE  -DHAVE_SETNS -DHAVE_ELF -DCONFIG_GACT -DCONFIG_GACT_PROB -DIPT_LIB_DIR=\"/lib/xtables\" -DYY_NO_INPUT -Wl,-export-dynamic -shared -fpic -o m_xt.so m_xt.c $(pkg-config xtables --cflags --libs)
> In file included from ../include/linux/netfilter_ipv4/ip_tables.h:20:0,
>                  from m_xt.c:20:
> ../include/linux/if.h:26:0: warning: "IFNAMSIZ" redefined
>  #define IFNAMSIZ 16
>  ^
> In file included from m_xt.c:17:0:
> /usr/include/net/if.h:129:0: note: this is the location of the previous definition
>  # define IFNAMSIZ IF_NAMESIZE
>  ^
> ../include/linux/if.h:71:2: error: redeclaration of enumerator ‘IFF_UP’
>   IFF_UP    = 1<<0,  /* sysfs */
>   ^
> /usr/include/net/if.h:44:5: note: previous definition of ‘IFF_UP’ was here
>      IFF_UP = 0x1,  /* Interface is up.  */
>      ^
> ../include/linux/if.h:72:2: error: redeclaration of enumerator ‘IFF_BROADCAST’
>   IFF_BROADCAST   = 1<<1,  /* __volatile__ */
>   ^
> /usr/include/net/if.h:46:5: note: previous definition of ‘IFF_BROADCAST’ was here
>      IFF_BROADCAST = 0x2, /* Broadcast address valid.  */
>      ^
> ../include/linux/if.h:73:2: error: redeclaration of enumerator ‘IFF_DEBUG’
>   IFF_DEBUG   = 1<<2,  /* sysfs */

Re: header conflict introduced by change to netfilter_ipv4/ip_tables.h

[Stephen Hemminger]

On Thu, 7 Jan 2016 07:29:50 +0000
Mikko Rapeli <mikko.rapeli@iki.fi> wrote:

> On Wed, Jan 06, 2016 at 09:20:07AM -0800, Stephen Hemminger wrote:
> > This commit breaks compilation of iproute2 with net-next.
> 
> Ok, linux/if.h and libc net/if.h have overlapping defines, and this is not
> the only one. I saw lots of them in the core dump headers.
> 
> How should we handle them? Another ifndef for IFNAMSIZ into kernel uapi
> headers?
> 
> -Mikko

Probably need to do the same thing that was done previously for these
kind of conflicts.  This makes make linux/if.h change to adapt to net/if.h
being included before it.

Or revert your patch.

Re: header conflict introduced by change to netfilter_ipv4/ip_tables.h

[Mikko Rapeli]

On Thu, Jan 07, 2016 at 10:30:40AM -0800, Stephen Hemminger wrote:
> On Thu, 7 Jan 2016 07:29:50 +0000
> Mikko Rapeli <mikko.rapeli@iki.fi> wrote:
> 
> > On Wed, Jan 06, 2016 at 09:20:07AM -0800, Stephen Hemminger wrote:
> > > This commit breaks compilation of iproute2 with net-next.
> > 
> > Ok, linux/if.h and libc net/if.h have overlapping defines, and this is not
> > the only one. I saw lots of them in the core dump headers.
> > 
> > How should we handle them? Another ifndef for IFNAMSIZ into kernel uapi
> > headers?
> > 
> > -Mikko
> 
> Probably need to do the same thing that was done previously for these
> kind of conflicts.  This makes make linux/if.h change to adapt to net/if.h
> being included before it.

Ok, got it. And found include/uapi/linux/libc-compat.h. Did not know about it
and was looking for solutions to these problems.

But now I feel like writing a test script for mixing of kernel uapi
and libc headers to find out how many other collitions are still there.
Not good for the pile of over 70 patches in my branch
https://github.com/torvalds/linux/compare/master...mcfrisk:headers_test_v05

> Or revert your patch.

I'm fine with this too.

-Mikko

Re: header conflict introduced by change to netfilter_ipv4/ip_tables.h

[Josh Boyer]

On Thu, Jan 7, 2016 at 2:15 PM, Mikko Rapeli <mikko.rapeli@iki.fi> wrote:
> On Thu, Jan 07, 2016 at 10:30:40AM -0800, Stephen Hemminger wrote:
>> On Thu, 7 Jan 2016 07:29:50 +0000
>> Mikko Rapeli <mikko.rapeli@iki.fi> wrote:
>>
>> > On Wed, Jan 06, 2016 at 09:20:07AM -0800, Stephen Hemminger wrote:
>> > > This commit breaks compilation of iproute2 with net-next.
>> >
>> > Ok, linux/if.h and libc net/if.h have overlapping defines, and this is not
>> > the only one. I saw lots of them in the core dump headers.
>> >
>> > How should we handle them? Another ifndef for IFNAMSIZ into kernel uapi
>> > headers?
>> >
>> > -Mikko
>>
>> Probably need to do the same thing that was done previously for these
>> kind of conflicts.  This makes make linux/if.h change to adapt to net/if.h
>> being included before it.
>
> Ok, got it. And found include/uapi/linux/libc-compat.h. Did not know about it
> and was looking for solutions to these problems.
>
> But now I feel like writing a test script for mixing of kernel uapi
> and libc headers to find out how many other collitions are still there.
> Not good for the pile of over 70 patches in my branch
> https://github.com/torvalds/linux/compare/master...mcfrisk:headers_test_v05
>
>> Or revert your patch.
>
> I'm fine with this too.

This is causing a number of build failures in Fedora rawhide now.  Did
anyone submit a revert or patch to fix this issue?

josh

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

[Mikko Rapeli]

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>
 #include <dlfcn.h>
 #include <elf.h>
 #include <endian.h>
 #include <envz.h>
 #include <err.h>
 #include <errno.h>
 #include <error.h>
 #include <execinfo.h>
 #include <fcntl.h>
 #include <features.h>
 #include <fenv.h>
 #include <fmtmsg.h>
 #include <fnmatch.h>
 #include <fstab.h>
 #include <fts.h>
 #include <ftw.h>
 #include <_G_config.h>
 #include <gconv.h>
 #include <getopt.h>
 #include <glob.h>
 #include <gnu-versions.h>
 #include <grp.h>
 #include <gshadow.h>
 #include <iconv.h>
 #include <ifaddrs.h>
 #include <inttypes.h>
 #include <langinfo.h>
 #include <lastlog.h>
 #include <libgen.h>
 #include <libintl.h>
 #include <libio.h>
 #include <limits.h>
 #include <link.h>
 #include <locale.h>
 #include <malloc.h>
 #include <math.h>
 #include <mcheck.h>
 #include <memory.h>
 #include <mntent.h>
 #include <monetary.h>
 #include <mqueue.h>
 #include <netash/ash.h>
 #include <netatalk/at.h>
 #include <netax25/ax25.h>
 #include <netdb.h>
 #include <neteconet/ec.h>
 #include <net/ethernet.h>
 #include <net/if_arp.h>
 #include <net/if.h>
 #include <net/if_packet.h>
 #include <net/if_ppp.h>
 #include <net/if_shaper.h>
 #include <net/if_slip.h>
 #include <netinet/ether.h>
 #include <netinet/icmp6.h>
 #include <netinet/if_ether.h>
 #include <netinet/if_fddi.h>
 #include <netinet/if_tr.h>
 #include <netinet/igmp.h>
 #include <netinet/in.h>
 #include <netinet/in_systm.h>
 #include <netinet/ip6.h>
 #include <netinet/ip.h>
 #include <netinet/ip_icmp.h>
 #include <netinet/tcp.h>
 #include <netinet/udp.h>
 #include <netipx/ipx.h>
 #include <netiucv/iucv.h>
 #include <netpacket/packet.h>
 #include <net/ppp-comp.h>
 #include <net/ppp_defs.h>
 #include <netrom/netrom.h>
 #include <netrose/rose.h>
 #include <net/route.h>
 #include <nl_types.h>
 #include <nss.h>
 #include <obstack.h>
 #include <paths.h>
 #include <poll.h>
 #include <printf.h>
 #include <protocols/routed.h>
 #include <protocols/rwhod.h>
 // conflicts with arap headers
 //#include <protocols/talkd.h>
 #include <protocols/timed.h>
 #include <pthread.h>
 #include <pty.h>
 #include <pwd.h>
 #include <re_comp.h>
 #include <regex.h>
 // conflicts with arpa headers
 //#include <resolv.h>
 #include <sched.h>
 #include <scsi/scsi.h>
 #include <scsi/scsi_ioctl.h>
 #include <scsi/sg.h>
 #include <search.h>
 #include <semaphore.h>
 #include <setjmp.h>
 #include <sgtty.h>
 #include <shadow.h>
 #include <signal.h>
 #include <spawn.h>
 #include <stab.h>
 #include <stdc-predef.h>
 #include <stdint.h>
 #include <stdio_ext.h>
 #include <stdio.h>
 #include <stdlib.h>
 #include <string.h>
 #include <strings.h>
 #include <stropts.h>
 #include <syscall.h>
 #include <sysexits.h>
 #include <syslog.h>
 #include <tar.h>
 #include <termio.h>
 #include <termios.h>
 #include <tgmath.h>
 #include <thread_db.h>
 #include <time.h>
 #include <ttyent.h>
 #include <uchar.h>
 #include <ucontext.h>
 #include <ulimit.h>
 #include <unistd.h>
 #include <ustat.h>
 #include <utime.h>
 #include <utmp.h>
 #include <utmpx.h>
 #include <values.h>
 #include <wait.h>
 #include <wchar.h>
 #include <wctype.h>
 #include <wordexp.h>
 #include <xlocale.h>

 $ cat iproute2_bug.h
 #include <libc_headers.h>
 #include <linux/if.h>

 $ cc -Wall -c -nostdinc -I ../include/ -I /usr/lib/gcc/i586-linux-gnu/5/include -I /usr/lib/gcc/i586-linux-gnu/5/include-fixed -I . -I /home/mcfrisk/src/linux-2.6/usr/headers_compile_test_include.ddxtNf -I /home/mcfrisk/src/linux-2.6/usr/headers_compile_test_include.ddxtNf/i586-linux-gnu -o /dev/null ./iproute2_bug.h

Reported-by: Stephen Hemminger <shemming-43mecJUBy8ZBDgjK7y7TUQ@public.gmane.org>
Reported-by: Josh Boyer <jwboyer-rxtnV0ftBwyoClj4AeEUq9i2O/JbrIOy@public.gmane.org>
Signed-off-by: Mikko Rapeli <mikko.rapeli-X3B1VOXEql0@public.gmane.org>
---
 include/uapi/linux/if.h          | 21 +++++++++++++++++++++
 include/uapi/linux/libc-compat.h | 33 +++++++++++++++++++++++++++++++++
 2 files changed, 54 insertions(+)

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

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

[David Miller]

From: Mikko Rapeli <mikko.rapeli-X3B1VOXEql0@public.gmane.org>
Date: Sun,  7 Feb 2016 16:03:21 +0200

> @@ -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.

> +/* 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

Likewise.

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

[Mikko Rapeli]

(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 <mikko.rapeli-X3B1VOXEql0@public.gmane.org>
> Date: Sun,  7 Feb 2016 16:03:21 +0200
> 
> > @@ -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

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

[David Miller]

From: Mikko Rapeli <mikko.rapeli@iki.fi>
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:
>> From: Mikko Rapeli <mikko.rapeli@iki.fi>
>> Date: Sun,  7 Feb 2016 16:03:21 +0200
>> 
>> > @@ -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.

Re: header conflict introduced by change to netfilter_ipv4/ip_tables.h

[Daniel Borkmann]

On 02/04/2016 08:13 AM, Josh Boyer wrote:
> On Thu, Jan 7, 2016 at 2:15 PM, Mikko Rapeli <mikko.rapeli@iki.fi> wrote:
>> On Thu, Jan 07, 2016 at 10:30:40AM -0800, Stephen Hemminger wrote:
>>> On Thu, 7 Jan 2016 07:29:50 +0000
>>> Mikko Rapeli <mikko.rapeli@iki.fi> wrote:
>>>> On Wed, Jan 06, 2016 at 09:20:07AM -0800, Stephen Hemminger wrote:
>>>>> This commit breaks compilation of iproute2 with net-next.
>>>>
>>>> Ok, linux/if.h and libc net/if.h have overlapping defines, and this is not
>>>> the only one. I saw lots of them in the core dump headers.
>>>>
>>>> How should we handle them? Another ifndef for IFNAMSIZ into kernel uapi
>>>> headers?
>>>>
>>>> -Mikko
>>>
>>> Probably need to do the same thing that was done previously for these
>>> kind of conflicts.  This makes make linux/if.h change to adapt to net/if.h
>>> being included before it.
>>
>> Ok, got it. And found include/uapi/linux/libc-compat.h. Did not know about it
>> and was looking for solutions to these problems.
>>
>> But now I feel like writing a test script for mixing of kernel uapi
>> and libc headers to find out how many other collitions are still there.
>> Not good for the pile of over 70 patches in my branch
>> https://github.com/torvalds/linux/compare/master...mcfrisk:headers_test_v05
>>
>>> Or revert your patch.
>>
>> I'm fine with this too.
>
> This is causing a number of build failures in Fedora rawhide now.  Did
> anyone submit a revert or patch to fix this issue?

Mikko, was there any follow-up patch to fix this? Seems like the build
error is not yet resolved.

Thanks,
Daniel

Re: header conflict introduced by change to netfilter_ipv4/ip_tables.h

[Mikko Rapeli]

On Thu, Feb 25, 2016 at 09:53:32PM +0100, Daniel Borkmann wrote:
> On 02/04/2016 08:13 AM, Josh Boyer wrote:
> >On Thu, Jan 7, 2016 at 2:15 PM, Mikko Rapeli <mikko.rapeli@iki.fi> wrote:
> >>On Thu, Jan 07, 2016 at 10:30:40AM -0800, Stephen Hemminger wrote:
> >>>On Thu, 7 Jan 2016 07:29:50 +0000
> >>>Mikko Rapeli <mikko.rapeli@iki.fi> wrote:
> >>>>On Wed, Jan 06, 2016 at 09:20:07AM -0800, Stephen Hemminger wrote:
> >>>>>This commit breaks compilation of iproute2 with net-next.
> >>>>
> >>>>Ok, linux/if.h and libc net/if.h have overlapping defines, and this is not
> >>>>the only one. I saw lots of them in the core dump headers.
> >>>>
> >>>>How should we handle them? Another ifndef for IFNAMSIZ into kernel uapi
> >>>>headers?
> >>>>
> >>>>-Mikko
> >>>
> >>>Probably need to do the same thing that was done previously for these
> >>>kind of conflicts.  This makes make linux/if.h change to adapt to net/if.h
> >>>being included before it.
> >>
> >>Ok, got it. And found include/uapi/linux/libc-compat.h. Did not know about it
> >>and was looking for solutions to these problems.
> >>
> >>But now I feel like writing a test script for mixing of kernel uapi
> >>and libc headers to find out how many other collitions are still there.
> >>Not good for the pile of over 70 patches in my branch
> >>https://github.com/torvalds/linux/compare/master...mcfrisk:headers_test_v05
> >>
> >>>Or revert your patch.
> >>
> >>I'm fine with this too.
> >
> >This is causing a number of build failures in Fedora rawhide now.  Did
> >anyone submit a revert or patch to fix this issue?
> 
> Mikko, was there any follow-up patch to fix this? Seems like the build
> error is not yet resolved.

First draft of proper fix:

https://lkml.org/lkml/2016/2/7/89

Will fix review comments from David Miller soon and I need to figure out
how to get the needed fixes to the glibc side too.

The problem from my point of view is that kernel uapi header files have
incomplete dependencies, which were hiding kernel uapi and glibc header file
conflicts.

I will try to systematically fix the kernel uapi and glibc header conflicts
but this will take some time. If you want, you can hide the problem
by reverting the needed header file dependency fixes.

-Mikko

Kernel uapi and glibc header conflicts (was Re: header conflict introduced by change to netfilter_ipv4/ip_tables.h )

[Mikko Rapeli]

On Thu, Jan 07, 2016 at 10:30:40AM -0800, Stephen Hemminger wrote:
> On Thu, 7 Jan 2016 07:29:50 +0000
> Mikko Rapeli <mikko.rapeli-X3B1VOXEql0@public.gmane.org> wrote:
> 
> > On Wed, Jan 06, 2016 at 09:20:07AM -0800, Stephen Hemminger wrote:
> > > This commit breaks compilation of iproute2 with net-next.
> > 
> > Ok, linux/if.h and libc net/if.h have overlapping defines, and this is not
> > the only one. I saw lots of them in the core dump headers.
> > 
> > How should we handle them? Another ifndef for IFNAMSIZ into kernel uapi
> > headers?
> > 
> > -Mikko
> 
> Probably need to do the same thing that was done previously for these
> kind of conflicts.  This makes make linux/if.h change to adapt to net/if.h
> being included before it.

So uapi headers now have a libc-compat.h
https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/include/uapi/linux/libc-compat.h?id=refs/tags/v4.5-rc2
which tries to detect and fix incompatibilities between Linux kernel and glibc
headers. Part of the fix is then in the kernel side headers and another part
should be in glibc headers, but glibc git repo does not include any of these
fixes yet.

Has the glibc part of this incompatiblity mess been discussed and agreed
with glibc developers?

Many of the conflics arise from propably old glibc headers which had copied
out definitions from the Linux kernel side before it could export any headers
to userspace. I assume that the glibc headers are not allowed to depend and
include Linux kernel uapi headers in deployments but maybe the Linux kernel
headers could be used at glibc compile time to generate needed glibc side
definitions. That would allow having a single source for definitions like 
FNAMSIZ 16.

Has this been considered before?

I'm drafting a test, similar to the kernel uapi header compile test
https://github.com/mcfrisk/linux/blob/headers_test_v05/scripts/headers_compile_test.sh
for the glibc conflicts too, and of course noticed that also glibc headers
conflict with each other. With some workarounds I can test compile each kernel
uapi header against all compiling glibc headers and see the conflicts as
build failures.

-Mikko

Re: Kernel uapi and glibc header conflicts (was Re: header conflict introduced by change to netfilter_ipv4/ip_tables.h )

[Florian Weimer]

On 02/07/2016 12:31 PM, Mikko Rapeli wrote:
> On Thu, Jan 07, 2016 at 10:30:40AM -0800, Stephen Hemminger wrote:
>> On Thu, 7 Jan 2016 07:29:50 +0000
>> Mikko Rapeli <mikko.rapeli@iki.fi> wrote:
>>
>>> On Wed, Jan 06, 2016 at 09:20:07AM -0800, Stephen Hemminger wrote:
>>>> This commit breaks compilation of iproute2 with net-next.
>>>
>>> Ok, linux/if.h and libc net/if.h have overlapping defines, and this is not
>>> the only one. I saw lots of them in the core dump headers.
>>>
>>> How should we handle them? Another ifndef for IFNAMSIZ into kernel uapi
>>> headers?
>>>
>>> -Mikko
>>
>> Probably need to do the same thing that was done previously for these
>> kind of conflicts.  This makes make linux/if.h change to adapt to net/if.h
>> being included before it.
> 
> So uapi headers now have a libc-compat.h
> https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/include/uapi/linux/libc-compat.h?id=refs/tags/v4.5-rc2
> which tries to detect and fix incompatibilities between Linux kernel and glibc
> headers. Part of the fix is then in the kernel side headers and another part
> should be in glibc headers, but glibc git repo does not include any of these
> fixes yet.
> 
> Has the glibc part of this incompatiblity mess been discussed and agreed
> with glibc developers?

I don't remember any recent discussions on libc-alpha, or any bug
reports about this concrete change.

(Redirecting to libc-alpha, which seems the more appropriate list.)

> Many of the conflics arise from propably old glibc headers which had copied
> out definitions from the Linux kernel side before it could export any headers
> to userspace. I assume that the glibc headers are not allowed to depend and
> include Linux kernel uapi headers in deployments but maybe the Linux kernel
> headers could be used at glibc compile time to generate needed glibc side
> definitions. That would allow having a single source for definitions like 
> FNAMSIZ 16.

My impression is that this inconsistency isn't the only problem.  The
problems start if application developers need functionality which is
only in kernel-provided headers, but they still need to include glibc
headers at the same time.

> I'm drafting a test, similar to the kernel uapi header compile test
> https://github.com/mcfrisk/linux/blob/headers_test_v05/scripts/headers_compile_test.sh
> for the glibc conflicts too, and of course noticed that also glibc headers
> conflict with each other. With some workarounds I can test compile each kernel
> uapi header against all compiling glibc headers and see the conflicts as
> build failures.

That could be helpful.

I'm not familiar with relevant developer practices.  It seems to me that
from an application developer point of view, kernel headers are updated
a bit more frequently than glibc headers.  This likely pushes the
solution into a certain direction (and may be the rationale behind the
kernel's libc-compat.h).

Florian

Re: header conflict introduced by change to netfilter_ipv4/ip_tables.h

[Thomas Graf]

On 01/06/16 at 09:20am, Stephen Hemminger wrote:
> This commit breaks compilation of iproute2 with net-next.
> 
> commit 1ffad83dffd675cd742286ae82dca7d746cb0da8
> Author: Mikko Rapeli <mikko.rapeli@iki.fi>
> Date:   Thu Oct 15 07:56:30 2015 +0200
> 
>     netfilter: fix include files for compilation
>     
>     Add missing header dependencies and other small changes so that each file
>     compiles alone in userspace.
>     
>     Signed-off-by: Mikko Rapeli <mikko.rapeli@iki.fi>
>     Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
> 
> For iproute2, a copy of kernel headers (from make install_headers) is used.
> After this change. the build of x_tables.c fails because IFNAMSIZ is already
> defined in net/if.h

There is another issue with this commit. iptables.h included from m_ipt.c
includes  xtables.h which includes <xtables-version.h> which is not
available on a system without xtables.

gcc -Wall -Wstrict-prototypes  -Wmissing-prototypes -Wmissing-declarations -Wold-style-definition -Wformat=2 -O2 -I../include -DRESOLVE_HOSTNAMES -DLIBDIR=\"/usr/lib\" -DCONFDIR=\"/etc/iproute2\" -D_GNU_SOURCE -D_FILE_OFFSET_BITS=64 -D_LARGEFILE_SOURCE -D_LARGEFILE64_SOURCE  -DHAVE_SETNS -DHAVE_ELF -DCONFIG_GACT -DCONFIG_GACT_PROB -DIPT_LIB_DIR=\"/lib/xtables\" -DYY_NO_INPUT   -c -o m_ipt.o m_ipt.c
In file included from ../include/iptables.h:5:0,
                 from m_ipt.c:17:
../include/xtables.h:34:29: fatal error: xtables-version.h: No such file or directory
 #include <xtables-version.h>

Re: header conflict introduced by change to netfilter_ipv4/ip_tables.h

[Mikko Rapeli]

On Thu, Feb 25, 2016 at 10:08:56PM +0100, Thomas Graf wrote:
> On 01/06/16 at 09:20am, Stephen Hemminger wrote:
> > This commit breaks compilation of iproute2 with net-next.
> > 
> > commit 1ffad83dffd675cd742286ae82dca7d746cb0da8
> > Author: Mikko Rapeli <mikko.rapeli@iki.fi>
> > Date:   Thu Oct 15 07:56:30 2015 +0200
> > 
> >     netfilter: fix include files for compilation
> >     
> >     Add missing header dependencies and other small changes so that each file
> >     compiles alone in userspace.
> >     
> >     Signed-off-by: Mikko Rapeli <mikko.rapeli@iki.fi>
> >     Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
> > 
> > For iproute2, a copy of kernel headers (from make install_headers) is used.
> > After this change. the build of x_tables.c fails because IFNAMSIZ is already
> > defined in net/if.h
> 
> There is another issue with this commit. iptables.h included from m_ipt.c
> includes  xtables.h which includes <xtables-version.h> which is not
> available on a system without xtables.
> 
> gcc -Wall -Wstrict-prototypes  -Wmissing-prototypes -Wmissing-declarations -Wold-style-definition -Wformat=2 -O2 -I../include -DRESOLVE_HOSTNAMES -DLIBDIR=\"/usr/lib\" -DCONFDIR=\"/etc/iproute2\" -D_GNU_SOURCE -D_FILE_OFFSET_BITS=64 -D_LARGEFILE_SOURCE -D_LARGEFILE64_SOURCE  -DHAVE_SETNS -DHAVE_ELF -DCONFIG_GACT -DCONFIG_GACT_PROB -DIPT_LIB_DIR=\"/lib/xtables\" -DYY_NO_INPUT   -c -o m_ipt.o m_ipt.c
> In file included from ../include/iptables.h:5:0,
>                  from m_ipt.c:17:
> ../include/xtables.h:34:29: fatal error: xtables-version.h: No such file or directory
>  #include <xtables-version.h>

I don't see any dependencies from kernel side uapi headers to iptables or
xtables in my tree based on v4.5-rc2. Maybe this is a problem in
iproute2.

-Mikko

Re: header conflict introduced by change to netfilter_ipv4/ip_tables.h

[Thomas Graf]

On 02/26/16 at 09:18am, Mikko Rapeli wrote:
> On Thu, Feb 25, 2016 at 10:08:56PM +0100, Thomas Graf wrote:
> > There is another issue with this commit. iptables.h included from m_ipt.c
> > includes  xtables.h which includes <xtables-version.h> which is not
> > available on a system without xtables.
> > 
> > gcc -Wall -Wstrict-prototypes  -Wmissing-prototypes -Wmissing-declarations -Wold-style-definition -Wformat=2 -O2 -I../include -DRESOLVE_HOSTNAMES -DLIBDIR=\"/usr/lib\" -DCONFDIR=\"/etc/iproute2\" -D_GNU_SOURCE -D_FILE_OFFSET_BITS=64 -D_LARGEFILE_SOURCE -D_LARGEFILE64_SOURCE  -DHAVE_SETNS -DHAVE_ELF -DCONFIG_GACT -DCONFIG_GACT_PROB -DIPT_LIB_DIR=\"/lib/xtables\" -DYY_NO_INPUT   -c -o m_ipt.o m_ipt.c
> > In file included from ../include/iptables.h:5:0,
> >                  from m_ipt.c:17:
> > ../include/xtables.h:34:29: fatal error: xtables-version.h: No such file or directory
> >  #include <xtables-version.h>
> 
> I don't see any dependencies from kernel side uapi headers to iptables or
> xtables in my tree based on v4.5-rc2. Maybe this is a problem in
> iproute2.

Right, it's an iproute2 issue in the commit to backport these original
changes to iproute2:

commit 5cd1adba79d33644debd4ba498bb262c5bebcfba
Author: Stephen Hemminger <stephen@networkplumber.org>
Date:   Sun Jan 3 15:14:27 2016 -0800

    Update to current iptables headers
    
    Keep in sync with current iptables upstream