All of lore.kernel.org
 help / color / mirror / Atom feed
* [Patch net-next v2] net: sync some IP headers with glibc
@ 2013-08-15  9:28 Cong Wang
  2013-08-15  9:28 ` [GLIBC Patch v2] inet: avoid redefinition of some structs in kernel Cong Wang
  2013-09-04 17:13 ` [Patch net-next v2] net: sync some IP headers with glibc David Miller
  0 siblings, 2 replies; 12+ messages in thread
From: Cong Wang @ 2013-08-15  9:28 UTC (permalink / raw)
  To: netdev
  Cc: David S. Miller, Thomas Backlund, libc-alpha, YOSHIFUJI Hideaki,
	Carlos O'Donell, Cong Wang

From: Carlos O'Donell <carlos@redhat.com>

Solution:
=========

- Synchronize linux's `include/uapi/linux/in6.h' 
  with glibc's `inet/netinet/in.h'.
- Synchronize glibc's `inet/netinet/in.h with linux's
  `include/uapi/linux/in6.h'.
- Allow including the headers in either other.
- First header included defines the structures and macros.

Details:
========

The kernel promises not to break the UAPI ABI so I don't 
see why we can't just have the two userspace headers
coordinate?

If you include the kernel headers first you get those,
and if you include the glibc headers first you get those,
and the following patch arranges a coordination and
synchronization between the two.

Let's handle `include/uapi/linux/in6.h' from linux, 
and `inet/netinet/in.h' from glibc and ensure they compile
in any order and preserve the required ABI.

These two patches pass the following compile tests:

cat >> test1.c <<EOF
#include <netinet/in.h>
#include <linux/in6.h>
int main (void) {
  return 0;
}
EOF
gcc -c test1.c

cat >> test2.c <<EOF
#include <linux/in6.h>
#include <netinet/in.h>
int main (void) {
  return 0;
}
EOF
gcc -c test2.c

One wrinkle is that the kernel has a different name for one of
the members in ipv6_mreq. In the kernel patch we create a macro
to cover the uses of the old name, and while that's not entirely
clean it's one of the best solutions (aside from an anonymous
union which has other issues).

I've reviewed the code and it looks to me like the ABI is
assured and everything matches on both sides.

Notes:
- You want netinet/in.h to include bits/in.h as early as possible,
  but it needs in_addr so define in_addr early.
- You want bits/in.h included as early as possible so you can use
  the linux specific code to define __USE_KERNEL_DEFS based on
  the _UAPI_* macro definition and use those to cull in.h.
- glibc was missing IPPROTO_MH, added here.

Compile tested and inspected.

Reported-by: Thomas Backlund <tmb@mageia.org>
Cc: Thomas Backlund <tmb@mageia.org>
Cc: libc-alpha@sourceware.org
Cc: YOSHIFUJI Hideaki <yoshfuji@linux-ipv6.org>
Cc: David S. Miller <davem@davemloft.net>
Tested-by: Cong Wang <amwang@redhat.com>
Signed-off-by: Carlos O'Donell <carlos@redhat.com>
Signed-off-by: Cong Wang <amwang@redhat.com>

---
v2: reformat some comments

diff --git a/include/uapi/linux/Kbuild b/include/uapi/linux/Kbuild
index 997f9f2..e7c94ee 100644
--- a/include/uapi/linux/Kbuild
+++ b/include/uapi/linux/Kbuild
@@ -227,6 +227,7 @@ header-y += kvm_para.h
 endif
 
 header-y += l2tp.h
+header-y += libc-compat.h
 header-y += limits.h
 header-y += llc.h
 header-y += loop.h
diff --git a/include/uapi/linux/in.h b/include/uapi/linux/in.h
index 9edb441..f9e8e49 100644
--- a/include/uapi/linux/in.h
+++ b/include/uapi/linux/in.h
@@ -24,30 +24,53 @@
 /* Standard well-defined IP protocols.  */
 enum {
   IPPROTO_IP = 0,		/* Dummy protocol for TCP		*/
+#define IPPROTO_IP		IPPROTO_IP
   IPPROTO_ICMP = 1,		/* Internet Control Message Protocol	*/
+#define IPPROTO_ICMP		IPPROTO_ICMP
   IPPROTO_IGMP = 2,		/* Internet Group Management Protocol	*/
+#define IPPROTO_IGMP		IPPROTO_IGMP
   IPPROTO_IPIP = 4,		/* IPIP tunnels (older KA9Q tunnels use 94) */
+#define IPPROTO_IPIP		IPPROTO_IPIP
   IPPROTO_TCP = 6,		/* Transmission Control Protocol	*/
+#define IPPROTO_TCP		IPPROTO_TCP
   IPPROTO_EGP = 8,		/* Exterior Gateway Protocol		*/
+#define IPPROTO_EGP		IPPROTO_EGP
   IPPROTO_PUP = 12,		/* PUP protocol				*/
+#define IPPROTO_PUP		IPPROTO_PUP
   IPPROTO_UDP = 17,		/* User Datagram Protocol		*/
+#define IPPROTO_UDP		IPPROTO_UDP
   IPPROTO_IDP = 22,		/* XNS IDP protocol			*/
+#define IPPROTO_IDP		IPPROTO_IDP
+  IPPROTO_TP = 29,		/* SO Transport Protocol Class 4	*/
+#define IPPROTO_TP		IPPROTO_TP
   IPPROTO_DCCP = 33,		/* Datagram Congestion Control Protocol */
-  IPPROTO_RSVP = 46,		/* RSVP protocol			*/
+#define IPPROTO_DCCP		IPPROTO_DCCP
+  IPPROTO_IPV6 = 41,		/* IPv6-in-IPv4 tunnelling		*/
+#define IPPROTO_IPV6		IPPROTO_IPV6
+  IPPROTO_RSVP = 46,		/* RSVP Protocol			*/
+#define IPPROTO_RSVP		IPPROTO_RSVP
   IPPROTO_GRE = 47,		/* Cisco GRE tunnels (rfc 1701,1702)	*/
-
-  IPPROTO_IPV6	 = 41,		/* IPv6-in-IPv4 tunnelling		*/
-
-  IPPROTO_ESP = 50,            /* Encapsulation Security Payload protocol */
-  IPPROTO_AH = 51,             /* Authentication Header protocol       */
-  IPPROTO_BEETPH = 94,	       /* IP option pseudo header for BEET */
-  IPPROTO_PIM    = 103,		/* Protocol Independent Multicast	*/
-
-  IPPROTO_COMP   = 108,                /* Compression Header protocol */
-  IPPROTO_SCTP   = 132,		/* Stream Control Transport Protocol	*/
+#define IPPROTO_GRE		IPPROTO_GRE
+  IPPROTO_ESP = 50,		/* Encapsulation Security Payload protocol */
+#define IPPROTO_ESP		IPPROTO_ESP
+  IPPROTO_AH = 51,		/* Authentication Header protocol	*/
+#define IPPROTO_AH		IPPROTO_AH
+  IPPROTO_MTP = 92,		/* Multicast Transport Protocol		*/
+#define IPPROTO_MTP		IPPROTO_MTP
+  IPPROTO_BEETPH = 94,		/* IP option pseudo header for BEET	*/
+#define IPPROTO_BEETPH		IPPROTO_BEETPH
+  IPPROTO_ENCAP = 98,		/* Encapsulation Header			*/
+#define IPPROTO_ENCAP		IPPROTO_ENCAP
+  IPPROTO_PIM = 103,		/* Protocol Independent Multicast	*/
+#define IPPROTO_PIM		IPPROTO_PIM
+  IPPROTO_COMP = 108,		/* Compression Header Protocol		*/
+#define IPPROTO_COMP		IPPROTO_COMP
+  IPPROTO_SCTP = 132,		/* Stream Control Transport Protocol	*/
+#define IPPROTO_SCTP		IPPROTO_SCTP
   IPPROTO_UDPLITE = 136,	/* UDP-Lite (RFC 3828)			*/
-
-  IPPROTO_RAW	 = 255,		/* Raw IP packets			*/
+#define IPPROTO_UDPLITE		IPPROTO_UDPLITE
+  IPPROTO_RAW = 255,		/* Raw IP packets			*/
+#define IPPROTO_RAW		IPPROTO_RAW
   IPPROTO_MAX
 };
 
diff --git a/include/uapi/linux/in6.h b/include/uapi/linux/in6.h
index 53b1d56..440d5c4 100644
--- a/include/uapi/linux/in6.h
+++ b/include/uapi/linux/in6.h
@@ -22,22 +22,30 @@
 #define _UAPI_LINUX_IN6_H
 
 #include <linux/types.h>
+#include <linux/libc-compat.h>
 
 /*
  *	IPv6 address structure
  */
 
+#if __UAPI_DEF_IN6_ADDR
 struct in6_addr {
 	union {
 		__u8		u6_addr8[16];
+#if __UAPI_DEF_IN6_ADDR_ALT
 		__be16		u6_addr16[8];
 		__be32		u6_addr32[4];
+#endif
 	} in6_u;
 #define s6_addr			in6_u.u6_addr8
+#if __UAPI_DEF_IN6_ADDR_ALT
 #define s6_addr16		in6_u.u6_addr16
 #define s6_addr32		in6_u.u6_addr32
+#endif
 };
+#endif /* __UAPI_DEF_IN6_ADDR */
 
+#if __UAPI_DEF_SOCKADDR_IN6
 struct sockaddr_in6 {
 	unsigned short int	sin6_family;    /* AF_INET6 */
 	__be16			sin6_port;      /* Transport layer port # */
@@ -45,7 +53,9 @@ struct sockaddr_in6 {
 	struct in6_addr		sin6_addr;      /* IPv6 address */
 	__u32			sin6_scope_id;  /* scope id (new in RFC2553) */
 };
+#endif /* __UAPI_DEF_SOCKADDR_IN6 */
 
+#if __UAPI_DEF_IPV6_MREQ
 struct ipv6_mreq {
 	/* IPv6 multicast address of group */
 	struct in6_addr ipv6mr_multiaddr;
@@ -53,6 +63,7 @@ struct ipv6_mreq {
 	/* local IPv6 address of interface */
 	int		ipv6mr_ifindex;
 };
+#endif /* __UAPI_DEF_IVP6_MREQ */
 
 #define ipv6mr_acaddr	ipv6mr_multiaddr
 
@@ -114,13 +125,24 @@ struct in6_flowlabel_req {
 /*
  *	IPV6 extension headers
  */
-#define IPPROTO_HOPOPTS		0	/* IPv6 hop-by-hop options	*/
-#define IPPROTO_ROUTING		43	/* IPv6 routing header		*/
-#define IPPROTO_FRAGMENT	44	/* IPv6 fragmentation header	*/
-#define IPPROTO_ICMPV6		58	/* ICMPv6			*/
-#define IPPROTO_NONE		59	/* IPv6 no next header		*/
-#define IPPROTO_DSTOPTS		60	/* IPv6 destination options	*/
-#define IPPROTO_MH		135	/* IPv6 mobility header		*/
+#if __UAPI_DEF_IPPROTO_V6
+enum {
+  IPPROTO_HOPOPTS = 0,		/* IPv6 hop-by-hop options      */
+#define IPPROTO_HOPOPTS		IPPROTO_HOPOPTS
+  IPPROTO_ROUTING = 43,		/* IPv6 routing header          */
+#define IPPROTO_ROUTING		IPPROTO_ROUTING
+  IPPROTO_FRAGMENT = 44,	/* IPv6 fragmentation header    */
+#define IPPROTO_FRAGMENT	IPPROTO_FRAGMENT
+  IPPROTO_ICMPV6 = 58,		/* ICMPv6                       */
+#define IPPROTO_ICMPV6		IPPROTO_ICMPV6
+  IPPROTO_NONE = 59,		/* IPv6 no next header          */
+#define IPPROTO_NONE		IPPROTO_NONE
+  IPPROTO_DSTOPTS = 60,		/* IPv6 destination options     */
+#define IPPROTO_DSTOPTS		IPPROTO_DSTOPTS
+  IPPROTO_MH = 135,		/* IPv6 mobility header         */
+#define IPPROTO_MH		IPPROTO_MH
+};
+#endif /* __UAPI_DEF_IPPROTO_V6 */
 
 /*
  *	IPv6 TLV options.
diff --git a/include/uapi/linux/libc-compat.h b/include/uapi/linux/libc-compat.h
new file mode 100644
index 0000000..335e8a7
--- /dev/null
+++ b/include/uapi/linux/libc-compat.h
@@ -0,0 +1,103 @@
+/*
+ * Compatibility interface for userspace libc header coordination:
+ *
+ * Define compatibility macros that are used to control the inclusion or
+ * exclusion of UAPI structures and definitions in coordination with another
+ * userspace C library.
+ *
+ * This header is intended to solve the problem of UAPI definitions that
+ * conflict with userspace definitions. If a UAPI header has such conflicting
+ * definitions then the solution is as follows:
+ *
+ * * Synchronize the UAPI header and the libc headers so either one can be
+ *   used and such that the ABI is preserved. If this is not possible then
+ *   no simple compatibility interface exists (you need to write translating
+ *   wrappers and rename things) and you can't use this interface.
+ *
+ * Then follow this process:
+ *
+ * (a) Include libc-compat.h in the UAPI header.
+ *      e.g. #include <linux/libc-compat.h>
+ *     This include must be as early as possible.
+ *
+ * (b) In libc-compat.h add enough code to detect that the comflicting
+ *     userspace libc header has been included first.
+ *
+ * (c) If the userspace libc header has been included first define a set of
+ *     guard macros of the form __UAPI_DEF_FOO and set their values to 1, else
+ *     set their values to 0.
+ *
+ * (d) Back in the UAPI header with the conflicting definitions, guard the
+ *     definitions with:
+ *     #if __UAPI_DEF_FOO
+ *       ...
+ *     #endif
+ *
+ * This fixes the situation where the linux headers are included *after* the
+ * libc headers. To fix the problem with the inclusion in the other order the
+ * userspace libc headers must be fixed like this:
+ *
+ * * For all definitions that conflict with kernel definitions wrap those
+ *   defines in the following:
+ *   #if !__UAPI_DEF_FOO
+ *     ...
+ *   #endif
+ *
+ * This prevents the redefinition of a construct already defined by the kernel.
+ */
+#ifndef _UAPI_LIBC_COMPAT_H
+#define _UAPI_LIBC_COMPAT_H
+
+/* We have included glibc headers... */
+#if defined(__GLIBC__)
+
+/* Coordinate with glibc netinet/in.h header. */
+#if defined(_NETINET_IN_H)
+
+/* GLIBC headers included first so don't define anything
+ * that would already be defined. */
+#define __UAPI_DEF_IN6_ADDR		0
+/* The exception is the in6_addr macros which must be defined
+ * if the glibc code didn't define them. This guard matches
+ * the guard in glibc/inet/netinet/in.h which defines the
+ * additional in6_addr macros e.g. s6_addr16, and s6_addr32. */
+#if defined(__USE_MISC) || defined (__USE_GNU)
+#define __UAPI_DEF_IN6_ADDR_ALT		0
+#else
+#define __UAPI_DEF_IN6_ADDR_ALT		1
+#endif
+#define __UAPI_DEF_SOCKADDR_IN6		0
+#define __UAPI_DEF_IPV6_MREQ		0
+#define __UAPI_DEF_IPPROTO_V6		0
+
+#else
+
+/* 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_IN6_ADDR		1
+/* We unconditionally define the in6_addr macros and glibc must
+ * coordinate. */
+#define __UAPI_DEF_IN6_ADDR_ALT		1
+#define __UAPI_DEF_SOCKADDR_IN6		1
+#define __UAPI_DEF_IPV6_MREQ		1
+#define __UAPI_DEF_IPPROTO_V6		1
+
+#endif /* _NETINET_IN_H */
+
+
+/* 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__) */
+
+/* Definitions for in6.h */
+#define __UAPI_DEF_IN6_ADDR		1
+#define __UAPI_DEF_IN6_ADDR_ALT		1
+#define __UAPI_DEF_SOCKADDR_IN6		1
+#define __UAPI_DEF_IPV6_MREQ		1
+#define __UAPI_DEF_IPPROTO_V6		1
+
+#endif /* __GLIBC__ */
+
+#endif /* _UAPI_LIBC_COMPAT_H */

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [GLIBC Patch v2] inet: avoid redefinition of some structs in kernel
  2013-08-15  9:28 [Patch net-next v2] net: sync some IP headers with glibc Cong Wang
@ 2013-08-15  9:28 ` Cong Wang
  2013-08-16 15:15   ` Carlos O'Donell
                     ` (3 more replies)
  2013-09-04 17:13 ` [Patch net-next v2] net: sync some IP headers with glibc David Miller
  1 sibling, 4 replies; 12+ messages in thread
From: Cong Wang @ 2013-08-15  9:28 UTC (permalink / raw)
  To: netdev
  Cc: David S. Miller, Thomas Backlund, libc-alpha, YOSHIFUJI Hideaki,
	Carlos O'Donell, Cong Wang

From: Carlos O'Donell <carlos@redhat.com>

- Synchronize linux's `include/uapi/linux/in6.h' 
  with glibc's `inet/netinet/in.h'.
- Synchronize glibc's `inet/netinet/in.h with linux's
  `include/uapi/linux/in6.h'.
- Allow including the headers in either other.
- First header included defines the structures and macros.

Notes:
- You want netinet/in.h to include bits/in.h as early as possible,
  but it needs in_addr so define in_addr early.
- You want bits/in.h included as early as possible so you can use
  the linux specific code to define __USE_KERNEL_DEFS based on
  the _UAPI_* macro definition and use those to cull in.h.
- glibc was missing IPPROTO_MH, added here.

Reported-by: Thomas Backlund <tmb@mageia.org>
Cc: Thomas Backlund <tmb@mageia.org>
Cc: libc-alpha@sourceware.org
Cc: YOSHIFUJI Hideaki <yoshfuji@linux-ipv6.org>
Cc: David S. Miller <davem@davemloft.net>
Signed-off-by: Carlos O'Donell <carlos@redhat.com>
Tested-by: Cong Wang <amwang@redhat.com>
Signed-off-by: Cong Wang <amwang@redhat.com>

2013-08-13  Carlos O'Donell  <carlos@redhat.com>
	    Cong Wang <amwang@redhat.com>

	* sysdeps/unix/sysv/linux/bits/in.h
	[_UAPI_LINUX_IN6_H]: Define __USE_KERNEL_IPV6_DEFS.
	* inet/netinet/in.h: Move in_addr definition and bits/in.h inclusion
	before __USE_KERNEL_IPV6_DEFS uses.
	* inet/netinet/in.h [!__USE_KERNEL_IPV6_DEFS]: Define IPPROTO_MH, and
	IPPROTO_BEETPH.
	[__USE_KERNEL_IPV6_DEFS]: Don't define any of IPPROTO_*, in6_addr,
	sockaddr_in6, or ipv6_mreq.

---

diff --git a/inet/netinet/in.h b/inet/netinet/in.h
index 89e3813..05c77e2 100644
--- a/inet/netinet/in.h
+++ b/inet/netinet/in.h
@@ -26,13 +26,21 @@
 
 __BEGIN_DECLS
 
+/* Internet address.  */
+typedef uint32_t in_addr_t;
+struct in_addr
+  {
+    in_addr_t s_addr;
+  };
+
+/* Get system-specific definitions.  */
+#include <bits/in.h>
+
 /* Standard well-defined IP protocols.  */
 enum
   {
     IPPROTO_IP = 0,	   /* Dummy protocol for TCP.  */
 #define IPPROTO_IP		IPPROTO_IP
-    IPPROTO_HOPOPTS = 0,   /* IPv6 Hop-by-Hop options.  */
-#define IPPROTO_HOPOPTS		IPPROTO_HOPOPTS
     IPPROTO_ICMP = 1,	   /* Internet Control Message Protocol.  */
 #define IPPROTO_ICMP		IPPROTO_ICMP
     IPPROTO_IGMP = 2,	   /* Internet Group Management Protocol. */
@@ -55,10 +63,6 @@ enum
 #define IPPROTO_DCCP		IPPROTO_DCCP
     IPPROTO_IPV6 = 41,     /* IPv6 header.  */
 #define IPPROTO_IPV6		IPPROTO_IPV6
-    IPPROTO_ROUTING = 43,  /* IPv6 routing header.  */
-#define IPPROTO_ROUTING		IPPROTO_ROUTING
-    IPPROTO_FRAGMENT = 44, /* IPv6 fragmentation header.  */
-#define IPPROTO_FRAGMENT	IPPROTO_FRAGMENT
     IPPROTO_RSVP = 46,	   /* Reservation Protocol.  */
 #define IPPROTO_RSVP		IPPROTO_RSVP
     IPPROTO_GRE = 47,	   /* General Routing Encapsulation.  */
@@ -67,14 +71,10 @@ enum
 #define IPPROTO_ESP		IPPROTO_ESP
     IPPROTO_AH = 51,       /* authentication header.  */
 #define IPPROTO_AH		IPPROTO_AH
-    IPPROTO_ICMPV6 = 58,   /* ICMPv6.  */
-#define IPPROTO_ICMPV6		IPPROTO_ICMPV6
-    IPPROTO_NONE = 59,     /* IPv6 no next header.  */
-#define IPPROTO_NONE		IPPROTO_NONE
-    IPPROTO_DSTOPTS = 60,  /* IPv6 destination options.  */
-#define IPPROTO_DSTOPTS		IPPROTO_DSTOPTS
     IPPROTO_MTP = 92,	   /* Multicast Transport Protocol.  */
 #define IPPROTO_MTP		IPPROTO_MTP
+    IPPROTO_BEETPH = 94,   /* IP option pseudo header for BEET.  */
+#define IPPROTO_BEETPH		IPPROTO_BEETPH
     IPPROTO_ENCAP = 98,	   /* Encapsulation Header.  */
 #define IPPROTO_ENCAP		IPPROTO_ENCAP
     IPPROTO_PIM = 103,	   /* Protocol Independent Multicast.  */
@@ -90,6 +90,28 @@ enum
     IPPROTO_MAX
   };
 
+/* If __USER_KERNEL_IPV6_DEFS is defined then the user has included the kernel
+   network headers first and we should use those ABI-identical definitions
+   instead of our own.  */
+#ifndef __USE_KERNEL_IPV6_DEFS
+enum
+  {
+    IPPROTO_HOPOPTS = 0,   /* IPv6 Hop-by-Hop options.  */
+#define IPPROTO_HOPOPTS		IPPROTO_HOPOPTS
+    IPPROTO_ROUTING = 43,  /* IPv6 routing header.  */
+#define IPPROTO_ROUTING		IPPROTO_ROUTING
+    IPPROTO_FRAGMENT = 44, /* IPv6 fragmentation header.  */
+#define IPPROTO_FRAGMENT	IPPROTO_FRAGMENT
+    IPPROTO_ICMPV6 = 58,   /* ICMPv6.  */
+#define IPPROTO_ICMPV6		IPPROTO_ICMPV6
+    IPPROTO_NONE = 59,     /* IPv6 no next header.  */
+#define IPPROTO_NONE		IPPROTO_NONE
+    IPPROTO_DSTOPTS = 60,  /* IPv6 destination options.  */
+#define IPPROTO_DSTOPTS		IPPROTO_DSTOPTS
+    IPPROTO_MH = 135,      /* IPv6 mobility header.  */
+#define IPPROTO_MH		IPPROTO_MH
+  };
+#endif /* !__USE_KERNEL_IPV6_DEFS */
 
 /* Type to represent a port.  */
 typedef uint16_t in_port_t;
@@ -134,15 +156,6 @@ enum
     IPPORT_USERRESERVED = 5000
   };
 
-
-/* Internet address.  */
-typedef uint32_t in_addr_t;
-struct in_addr
-  {
-    in_addr_t s_addr;
-  };
-
-
 /* Definitions of the bits in an Internet address integer.
 
    On subnets, host and network parts are found according to
@@ -191,7 +204,7 @@ struct in_addr
 #define INADDR_ALLRTRS_GROUP    ((in_addr_t) 0xe0000002) /* 224.0.0.2 */
 #define INADDR_MAX_LOCAL_GROUP  ((in_addr_t) 0xe00000ff) /* 224.0.0.255 */
 
-
+#ifndef __USE_KERNEL_IPV6_DEFS
 /* IPv6 address */
 struct in6_addr
   {
@@ -209,6 +222,7 @@ struct in6_addr
 # define s6_addr32		__in6_u.__u6_addr32
 #endif
   };
+#endif /* !__USE_KERNEL_IPV6_DEFS */
 
 extern const struct in6_addr in6addr_any;        /* :: */
 extern const struct in6_addr in6addr_loopback;   /* ::1 */
@@ -233,6 +247,7 @@ struct sockaddr_in
 			   sizeof (struct in_addr)];
   };
 
+#ifndef __USE_KERNEL_IPV6_DEFS
 /* Ditto, for IPv6.  */
 struct sockaddr_in6
   {
@@ -242,7 +257,7 @@ struct sockaddr_in6
     struct in6_addr sin6_addr;	/* IPv6 address */
     uint32_t sin6_scope_id;	/* IPv6 scope-id */
   };
-
+#endif /* !__USE_KERNEL_IPV6_DEFS */
 
 #if defined __USE_MISC || defined __USE_GNU
 /* IPv4 multicast request.  */
@@ -268,7 +283,7 @@ struct ip_mreq_source
   };
 #endif
 
-
+#ifndef __USE_KERNEL_IPV6_DEFS
 /* Likewise, for IPv6.  */
 struct ipv6_mreq
   {
@@ -278,7 +293,7 @@ struct ipv6_mreq
     /* local interface */
     unsigned int ipv6mr_interface;
   };
-
+#endif /* !__USE_KERNEL_IPV6_DEFS */
 
 #if defined __USE_MISC || defined __USE_GNU
 /* Multicast group request.  */
@@ -349,10 +364,6 @@ struct group_filter
 				      * sizeof (struct sockaddr_storage)))
 #endif
 
-
-/* Get system-specific definitions.  */
-#include <bits/in.h>
-
 /* Functions to convert between host and network byte order.
 
    Please note that these functions normally take `unsigned long int' or
diff --git a/sysdeps/unix/sysv/linux/bits/in.h b/sysdeps/unix/sysv/linux/bits/in.h
index e959b33..d763ce9 100644
--- a/sysdeps/unix/sysv/linux/bits/in.h
+++ b/sysdeps/unix/sysv/linux/bits/in.h
@@ -21,6 +21,18 @@
 # error "Never use <bits/in.h> directly; include <netinet/in.h> instead."
 #endif
 
+/* If the application has already included linux/in6.h from a linux-based
+   kernel then we will not define the IPv6 IPPROTO_* defines, in6_addr (nor the
+   defines), sockaddr_in6, or ipv6_mreq.  The ABI used by the linux-kernel and
+   glibc match exactly.  Neither the linux kernel nor glibc should break this
+   ABI without coordination.  */
+#ifdef _UAPI_LINUX_IN6_H
+/* This is not quite the same API since the kernel always defines s6_addr16 and
+   s6_addr32. This is not a violation of POSIX since POSIX says "at least the
+   following member" and that holds true.  */
+# define __USE_KERNEL_IPV6_DEFS
+#endif
+
 /* Options for use with `getsockopt' and `setsockopt' at the IP level.
    The first word in the comment at the right is the data type used;
    "bool" means a boolean value stored in an `int'.  */

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: [GLIBC Patch v2] inet: avoid redefinition of some structs in kernel
  2013-08-15  9:28 ` [GLIBC Patch v2] inet: avoid redefinition of some structs in kernel Cong Wang
@ 2013-08-16 15:15   ` Carlos O'Donell
  2013-08-16 15:32   ` Andreas Jaeger
                     ` (2 subsequent siblings)
  3 siblings, 0 replies; 12+ messages in thread
From: Carlos O'Donell @ 2013-08-16 15:15 UTC (permalink / raw)
  To: Cong Wang, Andreas Jaeger
  Cc: netdev, David S. Miller, Thomas Backlund, libc-alpha, YOSHIFUJI Hideaki

On 08/15/2013 05:28 AM, Cong Wang wrote:
> From: Carlos O'Donell <carlos@redhat.com>
> 
> - Synchronize linux's `include/uapi/linux/in6.h' 
>   with glibc's `inet/netinet/in.h'.
> - Synchronize glibc's `inet/netinet/in.h with linux's
>   `include/uapi/linux/in6.h'.
> - Allow including the headers in either other.
> - First header included defines the structures and macros.
> 
> Notes:
> - You want netinet/in.h to include bits/in.h as early as possible,
>   but it needs in_addr so define in_addr early.
> - You want bits/in.h included as early as possible so you can use
>   the linux specific code to define __USE_KERNEL_DEFS based on
>   the _UAPI_* macro definition and use those to cull in.h.
> - glibc was missing IPPROTO_MH, added here.

Andreas,

Would you mind reviewing this?

I'd like an objective review from another senior glibc reviewer.

The goal here is simple, support including the kernel and glibc
headers in any order and coordinate structure definitions to maintain
the ABI.

The kernel headers often have structures and definitions that are
required by specialized userspace applications, and we'd like those
applications to be easy to write and maintain. Coordinate the headers
is going to make this simpler for everyone.

Cheers,
Carlos.

> Reported-by: Thomas Backlund <tmb@mageia.org>
> Cc: Thomas Backlund <tmb@mageia.org>
> Cc: libc-alpha@sourceware.org
> Cc: YOSHIFUJI Hideaki <yoshfuji@linux-ipv6.org>
> Cc: David S. Miller <davem@davemloft.net>
> Signed-off-by: Carlos O'Donell <carlos@redhat.com>
> Tested-by: Cong Wang <amwang@redhat.com>
> Signed-off-by: Cong Wang <amwang@redhat.com>
> 
> 2013-08-13  Carlos O'Donell  <carlos@redhat.com>
> 	    Cong Wang <amwang@redhat.com>
> 
> 	* sysdeps/unix/sysv/linux/bits/in.h
> 	[_UAPI_LINUX_IN6_H]: Define __USE_KERNEL_IPV6_DEFS.
> 	* inet/netinet/in.h: Move in_addr definition and bits/in.h inclusion
> 	before __USE_KERNEL_IPV6_DEFS uses.
> 	* inet/netinet/in.h [!__USE_KERNEL_IPV6_DEFS]: Define IPPROTO_MH, and
> 	IPPROTO_BEETPH.
> 	[__USE_KERNEL_IPV6_DEFS]: Don't define any of IPPROTO_*, in6_addr,
> 	sockaddr_in6, or ipv6_mreq.
> ---
> 
> diff --git a/inet/netinet/in.h b/inet/netinet/in.h
> index 89e3813..05c77e2 100644
> --- a/inet/netinet/in.h
> +++ b/inet/netinet/in.h
> @@ -26,13 +26,21 @@
>  
>  __BEGIN_DECLS
>  
> +/* Internet address.  */
> +typedef uint32_t in_addr_t;
> +struct in_addr
> +  {
> +    in_addr_t s_addr;
> +  };
> +
> +/* Get system-specific definitions.  */
> +#include <bits/in.h>
> +
>  /* Standard well-defined IP protocols.  */
>  enum
>    {
>      IPPROTO_IP = 0,	   /* Dummy protocol for TCP.  */
>  #define IPPROTO_IP		IPPROTO_IP
> -    IPPROTO_HOPOPTS = 0,   /* IPv6 Hop-by-Hop options.  */
> -#define IPPROTO_HOPOPTS		IPPROTO_HOPOPTS
>      IPPROTO_ICMP = 1,	   /* Internet Control Message Protocol.  */
>  #define IPPROTO_ICMP		IPPROTO_ICMP
>      IPPROTO_IGMP = 2,	   /* Internet Group Management Protocol. */
> @@ -55,10 +63,6 @@ enum
>  #define IPPROTO_DCCP		IPPROTO_DCCP
>      IPPROTO_IPV6 = 41,     /* IPv6 header.  */
>  #define IPPROTO_IPV6		IPPROTO_IPV6
> -    IPPROTO_ROUTING = 43,  /* IPv6 routing header.  */
> -#define IPPROTO_ROUTING		IPPROTO_ROUTING
> -    IPPROTO_FRAGMENT = 44, /* IPv6 fragmentation header.  */
> -#define IPPROTO_FRAGMENT	IPPROTO_FRAGMENT
>      IPPROTO_RSVP = 46,	   /* Reservation Protocol.  */
>  #define IPPROTO_RSVP		IPPROTO_RSVP
>      IPPROTO_GRE = 47,	   /* General Routing Encapsulation.  */
> @@ -67,14 +71,10 @@ enum
>  #define IPPROTO_ESP		IPPROTO_ESP
>      IPPROTO_AH = 51,       /* authentication header.  */
>  #define IPPROTO_AH		IPPROTO_AH
> -    IPPROTO_ICMPV6 = 58,   /* ICMPv6.  */
> -#define IPPROTO_ICMPV6		IPPROTO_ICMPV6
> -    IPPROTO_NONE = 59,     /* IPv6 no next header.  */
> -#define IPPROTO_NONE		IPPROTO_NONE
> -    IPPROTO_DSTOPTS = 60,  /* IPv6 destination options.  */
> -#define IPPROTO_DSTOPTS		IPPROTO_DSTOPTS
>      IPPROTO_MTP = 92,	   /* Multicast Transport Protocol.  */
>  #define IPPROTO_MTP		IPPROTO_MTP
> +    IPPROTO_BEETPH = 94,   /* IP option pseudo header for BEET.  */
> +#define IPPROTO_BEETPH		IPPROTO_BEETPH
>      IPPROTO_ENCAP = 98,	   /* Encapsulation Header.  */
>  #define IPPROTO_ENCAP		IPPROTO_ENCAP
>      IPPROTO_PIM = 103,	   /* Protocol Independent Multicast.  */
> @@ -90,6 +90,28 @@ enum
>      IPPROTO_MAX
>    };
>  
> +/* If __USER_KERNEL_IPV6_DEFS is defined then the user has included the kernel
> +   network headers first and we should use those ABI-identical definitions
> +   instead of our own.  */
> +#ifndef __USE_KERNEL_IPV6_DEFS
> +enum
> +  {
> +    IPPROTO_HOPOPTS = 0,   /* IPv6 Hop-by-Hop options.  */
> +#define IPPROTO_HOPOPTS		IPPROTO_HOPOPTS
> +    IPPROTO_ROUTING = 43,  /* IPv6 routing header.  */
> +#define IPPROTO_ROUTING		IPPROTO_ROUTING
> +    IPPROTO_FRAGMENT = 44, /* IPv6 fragmentation header.  */
> +#define IPPROTO_FRAGMENT	IPPROTO_FRAGMENT
> +    IPPROTO_ICMPV6 = 58,   /* ICMPv6.  */
> +#define IPPROTO_ICMPV6		IPPROTO_ICMPV6
> +    IPPROTO_NONE = 59,     /* IPv6 no next header.  */
> +#define IPPROTO_NONE		IPPROTO_NONE
> +    IPPROTO_DSTOPTS = 60,  /* IPv6 destination options.  */
> +#define IPPROTO_DSTOPTS		IPPROTO_DSTOPTS
> +    IPPROTO_MH = 135,      /* IPv6 mobility header.  */
> +#define IPPROTO_MH		IPPROTO_MH
> +  };
> +#endif /* !__USE_KERNEL_IPV6_DEFS */
>  
>  /* Type to represent a port.  */
>  typedef uint16_t in_port_t;
> @@ -134,15 +156,6 @@ enum
>      IPPORT_USERRESERVED = 5000
>    };
>  
> -
> -/* Internet address.  */
> -typedef uint32_t in_addr_t;
> -struct in_addr
> -  {
> -    in_addr_t s_addr;
> -  };
> -
> -
>  /* Definitions of the bits in an Internet address integer.
>  
>     On subnets, host and network parts are found according to
> @@ -191,7 +204,7 @@ struct in_addr
>  #define INADDR_ALLRTRS_GROUP    ((in_addr_t) 0xe0000002) /* 224.0.0.2 */
>  #define INADDR_MAX_LOCAL_GROUP  ((in_addr_t) 0xe00000ff) /* 224.0.0.255 */
>  
> -
> +#ifndef __USE_KERNEL_IPV6_DEFS
>  /* IPv6 address */
>  struct in6_addr
>    {
> @@ -209,6 +222,7 @@ struct in6_addr
>  # define s6_addr32		__in6_u.__u6_addr32
>  #endif
>    };
> +#endif /* !__USE_KERNEL_IPV6_DEFS */
>  
>  extern const struct in6_addr in6addr_any;        /* :: */
>  extern const struct in6_addr in6addr_loopback;   /* ::1 */
> @@ -233,6 +247,7 @@ struct sockaddr_in
>  			   sizeof (struct in_addr)];
>    };
>  
> +#ifndef __USE_KERNEL_IPV6_DEFS
>  /* Ditto, for IPv6.  */
>  struct sockaddr_in6
>    {
> @@ -242,7 +257,7 @@ struct sockaddr_in6
>      struct in6_addr sin6_addr;	/* IPv6 address */
>      uint32_t sin6_scope_id;	/* IPv6 scope-id */
>    };
> -
> +#endif /* !__USE_KERNEL_IPV6_DEFS */
>  
>  #if defined __USE_MISC || defined __USE_GNU
>  /* IPv4 multicast request.  */
> @@ -268,7 +283,7 @@ struct ip_mreq_source
>    };
>  #endif
>  
> -
> +#ifndef __USE_KERNEL_IPV6_DEFS
>  /* Likewise, for IPv6.  */
>  struct ipv6_mreq
>    {
> @@ -278,7 +293,7 @@ struct ipv6_mreq
>      /* local interface */
>      unsigned int ipv6mr_interface;
>    };
> -
> +#endif /* !__USE_KERNEL_IPV6_DEFS */
>  
>  #if defined __USE_MISC || defined __USE_GNU
>  /* Multicast group request.  */
> @@ -349,10 +364,6 @@ struct group_filter
>  				      * sizeof (struct sockaddr_storage)))
>  #endif
>  
> -
> -/* Get system-specific definitions.  */
> -#include <bits/in.h>
> -
>  /* Functions to convert between host and network byte order.
>  
>     Please note that these functions normally take `unsigned long int' or
> diff --git a/sysdeps/unix/sysv/linux/bits/in.h b/sysdeps/unix/sysv/linux/bits/in.h
> index e959b33..d763ce9 100644
> --- a/sysdeps/unix/sysv/linux/bits/in.h
> +++ b/sysdeps/unix/sysv/linux/bits/in.h
> @@ -21,6 +21,18 @@
>  # error "Never use <bits/in.h> directly; include <netinet/in.h> instead."
>  #endif
>  
> +/* If the application has already included linux/in6.h from a linux-based
> +   kernel then we will not define the IPv6 IPPROTO_* defines, in6_addr (nor the
> +   defines), sockaddr_in6, or ipv6_mreq.  The ABI used by the linux-kernel and
> +   glibc match exactly.  Neither the linux kernel nor glibc should break this
> +   ABI without coordination.  */
> +#ifdef _UAPI_LINUX_IN6_H
> +/* This is not quite the same API since the kernel always defines s6_addr16 and
> +   s6_addr32. This is not a violation of POSIX since POSIX says "at least the
> +   following member" and that holds true.  */
> +# define __USE_KERNEL_IPV6_DEFS
> +#endif
> +
>  /* Options for use with `getsockopt' and `setsockopt' at the IP level.
>     The first word in the comment at the right is the data type used;
>     "bool" means a boolean value stored in an `int'.  */
> 

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [GLIBC Patch v2] inet: avoid redefinition of some structs in kernel
  2013-08-15  9:28 ` [GLIBC Patch v2] inet: avoid redefinition of some structs in kernel Cong Wang
  2013-08-16 15:15   ` Carlos O'Donell
@ 2013-08-16 15:32   ` Andreas Jaeger
  2013-08-16 15:44     ` Carlos O'Donell
  2013-08-16 15:47   ` Carlos O'Donell
  2013-08-26  5:26   ` Mike Frysinger
  3 siblings, 1 reply; 12+ messages in thread
From: Andreas Jaeger @ 2013-08-16 15:32 UTC (permalink / raw)
  To: Cong Wang
  Cc: netdev, David S. Miller, Thomas Backlund, libc-alpha,
	YOSHIFUJI Hideaki, Carlos O'Donell

On 08/15/2013 11:28 AM, Cong Wang wrote:
> From: Carlos O'Donell <carlos@redhat.com>
> 
> - Synchronize linux's `include/uapi/linux/in6.h' 
>   with glibc's `inet/netinet/in.h'.
> - Synchronize glibc's `inet/netinet/in.h with linux's
>   `include/uapi/linux/in6.h'.
> - Allow including the headers in either other.
> - First header included defines the structures and macros.

Let me first say that I really love where this is going and would love
to see more of this work.

Will this work with older kernels as well? Meaning: Can I compile
today's user land programs with a new glibc and Kernel 3.10? My reading
of the patch assumes it does but I would like to hear that you tested it.

The patch itself looks fine, thanks,
Andreas
-- 
 Andreas Jaeger aj@{suse.com,opensuse.org} Twitter/Identica: jaegerandi
  SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
   GF: Jeff Hawn,Jennifer Guild,Felix Imendörffer,HRB16746 (AG Nürnberg)
    GPG fingerprint = 93A3 365E CE47 B889 DF7F  FED1 389A 563C C272 A126

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [GLIBC Patch v2] inet: avoid redefinition of some structs in kernel
  2013-08-16 15:32   ` Andreas Jaeger
@ 2013-08-16 15:44     ` Carlos O'Donell
  0 siblings, 0 replies; 12+ messages in thread
From: Carlos O'Donell @ 2013-08-16 15:44 UTC (permalink / raw)
  To: Andreas Jaeger
  Cc: Cong Wang, netdev, David S. Miller, Thomas Backlund, libc-alpha,
	YOSHIFUJI Hideaki

On 08/16/2013 11:32 AM, Andreas Jaeger wrote:
> On 08/15/2013 11:28 AM, Cong Wang wrote:
>> From: Carlos O'Donell <carlos@redhat.com>
>>
>> - Synchronize linux's `include/uapi/linux/in6.h' 
>>   with glibc's `inet/netinet/in.h'.
>> - Synchronize glibc's `inet/netinet/in.h with linux's
>>   `include/uapi/linux/in6.h'.
>> - Allow including the headers in either other.
>> - First header included defines the structures and macros.
> 
> Let me first say that I really love where this is going and would love
> to see more of this work.

The plan would be to eventually tackle more of the conflicting headers
in a uniform way.

> Will this work with older kernels as well? Meaning: Can I compile
> today's user land programs with a new glibc and Kernel 3.10? My reading
> of the patch assumes it does but I would like to hear that you tested it.

You can indeed take a new glibc, and a new kernel, and old programs
that previously failed to compile will now work. The ABI is maintained
and the headers coordinate.

I did not test this explicitly except through the small test case
I wrote which permutes inclusion order and the use of various
structures defined in the headers.

The only quibble one could have is with Linux. In that if you 
include the Linux headers first you will get more definitions than
required by POSIX e.g. s6_addr16 and s6_add32. However as I note
in the comment this is not a violation of POSIX since POSIX says 
"at least the following member" and that holds true.
 
> The patch itself looks fine, thanks,

Thanks for reviewing.

If the kernel patches get accepted I will check these into
glibc 2.19 for Cong.

Cheers,
Carlos.

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [GLIBC Patch v2] inet: avoid redefinition of some structs in kernel
  2013-08-15  9:28 ` [GLIBC Patch v2] inet: avoid redefinition of some structs in kernel Cong Wang
  2013-08-16 15:15   ` Carlos O'Donell
  2013-08-16 15:32   ` Andreas Jaeger
@ 2013-08-16 15:47   ` Carlos O'Donell
  2013-08-19  1:20     ` Cong Wang
  2013-08-26  5:26   ` Mike Frysinger
  3 siblings, 1 reply; 12+ messages in thread
From: Carlos O'Donell @ 2013-08-16 15:47 UTC (permalink / raw)
  To: Cong Wang
  Cc: netdev, David S. Miller, Thomas Backlund, libc-alpha, YOSHIFUJI Hideaki

On 08/15/2013 05:28 AM, Cong Wang wrote:
> 2013-08-13  Carlos O'Donell  <carlos@redhat.com>
> 	    Cong Wang <amwang@redhat.com>
> 
> 	* sysdeps/unix/sysv/linux/bits/in.h
> 	[_UAPI_LINUX_IN6_H]: Define __USE_KERNEL_IPV6_DEFS.
> 	* inet/netinet/in.h: Move in_addr definition and bits/in.h inclusion
> 	before __USE_KERNEL_IPV6_DEFS uses.
> 	* inet/netinet/in.h [!__USE_KERNEL_IPV6_DEFS]: Define IPPROTO_MH, and
> 	IPPROTO_BEETPH.
> 	[__USE_KERNEL_IPV6_DEFS]: Don't define any of IPPROTO_*, in6_addr,
> 	sockaddr_in6, or ipv6_mreq.

Cong,

Given that this is a user visible change could you please file
a glibc bugzilla bug in sourceware[1] so we can track the commit and so
that future users can reopen the bug to discuss any defects?

Then you need to add the BZ# to the ChangeLog, and whomever
commits this for you will mark it fixed in the NEWS. We should
also write up a NEWS blurb for this since it's the first explicit
header coordination of it's kind and we should highlight that
so developers take note and help us coordinate more headers.

Cheers,
Carlos.

[1] http://sourceware.org/bugzilla/

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [GLIBC Patch v2] inet: avoid redefinition of some structs in kernel
  2013-08-16 15:47   ` Carlos O'Donell
@ 2013-08-19  1:20     ` Cong Wang
  2013-08-19 18:11       ` Carlos O'Donell
  0 siblings, 1 reply; 12+ messages in thread
From: Cong Wang @ 2013-08-19  1:20 UTC (permalink / raw)
  To: Carlos O'Donell
  Cc: netdev, David S. Miller, Thomas Backlund, libc-alpha, YOSHIFUJI Hideaki

On Fri, 2013-08-16 at 11:47 -0400, Carlos O'Donell wrote:
> On 08/15/2013 05:28 AM, Cong Wang wrote:
> > 2013-08-13  Carlos O'Donell  <carlos@redhat.com>
> > 	    Cong Wang <amwang@redhat.com>
> > 
> > 	* sysdeps/unix/sysv/linux/bits/in.h
> > 	[_UAPI_LINUX_IN6_H]: Define __USE_KERNEL_IPV6_DEFS.
> > 	* inet/netinet/in.h: Move in_addr definition and bits/in.h inclusion
> > 	before __USE_KERNEL_IPV6_DEFS uses.
> > 	* inet/netinet/in.h [!__USE_KERNEL_IPV6_DEFS]: Define IPPROTO_MH, and
> > 	IPPROTO_BEETPH.
> > 	[__USE_KERNEL_IPV6_DEFS]: Don't define any of IPPROTO_*, in6_addr,
> > 	sockaddr_in6, or ipv6_mreq.
> 
> Cong,
> 
> Given that this is a user visible change could you please file
> a glibc bugzilla bug in sourceware[1] so we can track the commit and so
> that future users can reopen the bug to discuss any defects?

Done, http://sourceware.org/bugzilla/show_bug.cgi?id=15850


> 
> Then you need to add the BZ# to the ChangeLog, and whomever
> commits this for you will mark it fixed in the NEWS. We should
> also write up a NEWS blurb for this since it's the first explicit
> header coordination of it's kind and we should highlight that
> so developers take note and help us coordinate more headers.
> 

Should I resend this patch with BZ# included?

Thanks!

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [GLIBC Patch v2] inet: avoid redefinition of some structs in kernel
  2013-08-19  1:20     ` Cong Wang
@ 2013-08-19 18:11       ` Carlos O'Donell
  0 siblings, 0 replies; 12+ messages in thread
From: Carlos O'Donell @ 2013-08-19 18:11 UTC (permalink / raw)
  To: Cong Wang
  Cc: netdev, David S. Miller, Thomas Backlund, libc-alpha, YOSHIFUJI Hideaki

On 08/18/2013 09:20 PM, Cong Wang wrote:
> On Fri, 2013-08-16 at 11:47 -0400, Carlos O'Donell wrote:
>> On 08/15/2013 05:28 AM, Cong Wang wrote:
>>> 2013-08-13  Carlos O'Donell  <carlos@redhat.com>
>>> 	    Cong Wang <amwang@redhat.com>
>>>
>>> 	* sysdeps/unix/sysv/linux/bits/in.h
>>> 	[_UAPI_LINUX_IN6_H]: Define __USE_KERNEL_IPV6_DEFS.
>>> 	* inet/netinet/in.h: Move in_addr definition and bits/in.h inclusion
>>> 	before __USE_KERNEL_IPV6_DEFS uses.
>>> 	* inet/netinet/in.h [!__USE_KERNEL_IPV6_DEFS]: Define IPPROTO_MH, and
>>> 	IPPROTO_BEETPH.
>>> 	[__USE_KERNEL_IPV6_DEFS]: Don't define any of IPPROTO_*, in6_addr,
>>> 	sockaddr_in6, or ipv6_mreq.
>>
>> Cong,
>>
>> Given that this is a user visible change could you please file
>> a glibc bugzilla bug in sourceware[1] so we can track the commit and so
>> that future users can reopen the bug to discuss any defects?
> 
> Done, http://sourceware.org/bugzilla/show_bug.cgi?id=15850
 
Perfect.
 
>>
>> Then you need to add the BZ# to the ChangeLog, and whomever
>> commits this for you will mark it fixed in the NEWS. We should
>> also write up a NEWS blurb for this since it's the first explicit
>> header coordination of it's kind and we should highlight that
>> so developers take note and help us coordinate more headers.
>>
> 
> Should I resend this patch with BZ# included?

No need. But the final ChangeLog should have the BZ# in it, and the
committer will add it to the NEWS list of bugs fixed in 2.19.

Cheers,
Carlos.

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [GLIBC Patch v2] inet: avoid redefinition of some structs in kernel
  2013-08-15  9:28 ` [GLIBC Patch v2] inet: avoid redefinition of some structs in kernel Cong Wang
                     ` (2 preceding siblings ...)
  2013-08-16 15:47   ` Carlos O'Donell
@ 2013-08-26  5:26   ` Mike Frysinger
  2013-09-06  4:52     ` Carlos O'Donell
  3 siblings, 1 reply; 12+ messages in thread
From: Mike Frysinger @ 2013-08-26  5:26 UTC (permalink / raw)
  To: libc-alpha
  Cc: Cong Wang, netdev, David S. Miller, Thomas Backlund,
	YOSHIFUJI Hideaki, Carlos O'Donell

[-- Attachment #1: Type: Text/Plain, Size: 985 bytes --]

On Thursday 15 August 2013 05:28:11 Cong Wang wrote:
> From: Carlos O'Donell <carlos@redhat.com>
> 
> - Synchronize linux's `include/uapi/linux/in6.h'
>   with glibc's `inet/netinet/in.h'.
> - Synchronize glibc's `inet/netinet/in.h with linux's
>   `include/uapi/linux/in6.h'.
> - Allow including the headers in either other.
> - First header included defines the structures and macros.
> 
> Notes:
> - You want netinet/in.h to include bits/in.h as early as possible,
>   but it needs in_addr so define in_addr early.
> - You want bits/in.h included as early as possible so you can use
>   the linux specific code to define __USE_KERNEL_DEFS based on
>   the _UAPI_* macro definition and use those to cull in.h.
> - glibc was missing IPPROTO_MH, added here.

can we get something better documented here in a central location ?  having 
the names laid out in a git commit message and in a few leaf headers does not 
lend itself to being easily discoverable.
-mike

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [Patch net-next v2] net: sync some IP headers with glibc
  2013-08-15  9:28 [Patch net-next v2] net: sync some IP headers with glibc Cong Wang
  2013-08-15  9:28 ` [GLIBC Patch v2] inet: avoid redefinition of some structs in kernel Cong Wang
@ 2013-09-04 17:13 ` David Miller
  2013-09-06  5:19   ` Carlos O'Donell
  1 sibling, 1 reply; 12+ messages in thread
From: David Miller @ 2013-09-04 17:13 UTC (permalink / raw)
  To: amwang; +Cc: netdev, tmb, libc-alpha, yoshfuji, carlos

From: Cong Wang <amwang@redhat.com>
Date: Thu, 15 Aug 2013 17:28:10 +0800

> Solution:
> =========
> 
> - Synchronize linux's `include/uapi/linux/in6.h' 
>   with glibc's `inet/netinet/in.h'.
> - Synchronize glibc's `inet/netinet/in.h with linux's
>   `include/uapi/linux/in6.h'.
> - Allow including the headers in either other.
> - First header included defines the structures and macros.

Applied, thanks for being so patient.  I want to spend the past few
weeks making sure this is the right way to handle all of this and
now I am confident that it is.

Thanks!

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [GLIBC Patch v2] inet: avoid redefinition of some structs in kernel
  2013-08-26  5:26   ` Mike Frysinger
@ 2013-09-06  4:52     ` Carlos O'Donell
  0 siblings, 0 replies; 12+ messages in thread
From: Carlos O'Donell @ 2013-09-06  4:52 UTC (permalink / raw)
  To: Mike Frysinger
  Cc: libc-alpha, Cong Wang, netdev, David S. Miller, Thomas Backlund,
	YOSHIFUJI Hideaki

On 08/26/2013 01:26 AM, Mike Frysinger wrote:
> On Thursday 15 August 2013 05:28:11 Cong Wang wrote:
>> From: Carlos O'Donell <carlos@redhat.com>
>>
>> - Synchronize linux's `include/uapi/linux/in6.h'
>>   with glibc's `inet/netinet/in.h'.
>> - Synchronize glibc's `inet/netinet/in.h with linux's
>>   `include/uapi/linux/in6.h'.
>> - Allow including the headers in either other.
>> - First header included defines the structures and macros.
>>
>> Notes:
>> - You want netinet/in.h to include bits/in.h as early as possible,
>>   but it needs in_addr so define in_addr early.
>> - You want bits/in.h included as early as possible so you can use
>>   the linux specific code to define __USE_KERNEL_DEFS based on
>>   the _UAPI_* macro definition and use those to cull in.h.
>> - glibc was missing IPPROTO_MH, added here.
> 
> can we get something better documented here in a central location ?  having 
> the names laid out in a git commit message and in a few leaf headers does not 
> lend itself to being easily discoverable.

Care to suggest something?

Cheers,
Carlos.

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [Patch net-next v2] net: sync some IP headers with glibc
  2013-09-04 17:13 ` [Patch net-next v2] net: sync some IP headers with glibc David Miller
@ 2013-09-06  5:19   ` Carlos O'Donell
  0 siblings, 0 replies; 12+ messages in thread
From: Carlos O'Donell @ 2013-09-06  5:19 UTC (permalink / raw)
  To: David Miller; +Cc: amwang, netdev, tmb, libc-alpha, yoshfuji

On 09/04/2013 01:13 PM, David Miller wrote:
> From: Cong Wang <amwang@redhat.com>
> Date: Thu, 15 Aug 2013 17:28:10 +0800
> 
>> Solution:
>> =========
>>
>> - Synchronize linux's `include/uapi/linux/in6.h' 
>>   with glibc's `inet/netinet/in.h'.
>> - Synchronize glibc's `inet/netinet/in.h with linux's
>>   `include/uapi/linux/in6.h'.
>> - Allow including the headers in either other.
>> - First header included defines the structures and macros.
> 
> Applied, thanks for being so patient.  I want to spend the past few
> weeks making sure this is the right way to handle all of this and
> now I am confident that it is.

Pushed into 2.19.

If you compile glibc with old non-UAPI kernel headers then this code
never triggers and glibc continues to redefine the structures and we
have the same problem as always.

If you compile glibc with new UPAI kernel headers, but without the
kernel side patch, you fix the build issues only in one order of
inclusion i.e. linux header then glibc header. Thus previously your
application didn't compile, now it does, but without the kernel
side patch some of the constants glibc define may be missing. At that
point you need to file a bug against the kernel you are using and
request they add the missing constants (or add them yourself).

If you compile glibc with new UAPI kernel headers, and those headers
have the kernel side patch then everything works in any inclusion order
and the defined constants are the superset of those defined in both
implementations.

Note:
- It could have been possible with more added complexity to handle
  the case where the kernel headers have not been patched. I deemed
  that because the code never originally compiled that it was fine
  to continue not compiling in that intermediate state and only support
  the final state as a complete solution. Feel free to disagree.

commit 6c82a2f8d7c8e21e39237225c819f182ae438db3
Author: Carlos O'Donell <carlos@redhat.com>
Date:   Fri Sep 6 01:02:30 2013 -0400

    Coordinate IPv6 definitions for Linux and glibc
    
    This change synchronizes the glibc headers with the Linux kernel
    headers and arranges to coordinate the definition of structures
    already defined the Linux kernel UAPI headers.
    
    It is now safe to include glibc's netinet/in.h or Linux's linux/in6.h
    in any order in a userspace application and you will get the same
    ABI. The ABI is guaranteed by UAPI and glibc.

Cheers,
Carlos.

^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2013-09-06  5:19 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-08-15  9:28 [Patch net-next v2] net: sync some IP headers with glibc Cong Wang
2013-08-15  9:28 ` [GLIBC Patch v2] inet: avoid redefinition of some structs in kernel Cong Wang
2013-08-16 15:15   ` Carlos O'Donell
2013-08-16 15:32   ` Andreas Jaeger
2013-08-16 15:44     ` Carlos O'Donell
2013-08-16 15:47   ` Carlos O'Donell
2013-08-19  1:20     ` Cong Wang
2013-08-19 18:11       ` Carlos O'Donell
2013-08-26  5:26   ` Mike Frysinger
2013-09-06  4:52     ` Carlos O'Donell
2013-09-04 17:13 ` [Patch net-next v2] net: sync some IP headers with glibc David Miller
2013-09-06  5:19   ` Carlos O'Donell

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.