Re: [libvirt] Redefinition of struct in6_addr in netinet/in.h and linux/in6.h
Carlos O'Donell wrote: On 01/18/2013 05:44 AM, Pedro Alves wrote: On 01/18/2013 04:22 AM, Carlos O'Donell wrote: On Thu, Jan 17, 2013 at 11:20 PM, Mike Frysinger vap...@gentoo.org wrote: On Wednesday 16 January 2013 22:15:38 David Miller wrote: From: Carlos O'Donell car...@systemhalted.org Date: Wed, 16 Jan 2013 21:15:03 -0500 +/* If a glibc-based userspace has already included in.h, then we will not + * define in6_addr (nor the defines), sockaddr_in6, or ipv6_mreq. The + * ABI used by the kernel and by glibc match exactly. Neither the kernel + * nor glibc should break this ABI without coordination. + */ +#ifndef _NETINET_IN_H + I think we should shoot for a non-glibc-centric solution. I can't imagine that other libc's won't have the same exact problem with their netinet/in.h conflicting with the kernel's, redefining structures like in6_addr, that we'd want to provide a protection scheme for here as well. yes, the kernel's use of __GLIBC__ in exported headers has already caused problems in the past. fortunately, it's been reduced down to just one case now (stat.h). let's not balloon it back up. -mike I also see coda.h has grown a __GLIBC__ usage. In the next revision of the patch I created a single libc-compat.h header which encompasses the logic for any libc that wants to coordinate with the kernel headers. It's simple enough to move all of the __GLIBC__ uses into libc-compat.h, then you control userspace libc coordination from one file. How about just deciding on a single macro/symbol both the kernel and libc (any libc that needs this) define? Something like both the kernel and userland doing: #ifndef __IPV6_BITS_DEFINED #define __IPV6_BITS_DEFINED ... define in6_addr, sockaddr_in6, ipv6_mreq, whatnot #endif Hmm, how should we handle future structs/enums then? For example, if I want to have in6_flowlabel_req{} defined in glibc, what should we do? We probably want to have __LIBC_HAS_STRUCT_IN6_FLOWLABEL_REQ defined. --yoshfuji -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] Redefinition of struct in6_addr in netinet/in.h and linux/in6.h
On Fri, Jan 18, 2013 at 9:36 AM, Pedro Alves pal...@redhat.com wrote: On 01/18/2013 02:24 PM, YOSHIFUJI Hideaki wrote: It's simple enough to move all of the __GLIBC__ uses into libc-compat.h, then you control userspace libc coordination from one file. How about just deciding on a single macro/symbol both the kernel and libc (any libc that needs this) define? Something like both the kernel and userland doing: #ifndef __IPV6_BITS_DEFINED #define __IPV6_BITS_DEFINED ... define in6_addr, sockaddr_in6, ipv6_mreq, whatnot #endif Hmm, how should we handle future structs/enums then? For example, if I want to have in6_flowlabel_req{} defined in glibc, what should we do? Include the glibc header first? Or is this some other use case? The point wasn't that you'd have only one macro for all structs/enums (you could split into __IPV6_IN6_ADDR_DEFINED, __IPV6_SOCKADDR_IN6_DEFINED, etc.) but to have the kernel and libc agree on guard macros, instead of having the kernel do #ifdef __GLIBC__ and glibc doing #ifdef _NETINET_IN_H. But as Carlos says, the devil is in the details, and I sure am not qualified on the details here. My plan is to have one _UAPI_DEF_xxx macro to guard each problematic definition in the kernel UAPI headers. Then userspace can check for those macros and act appropriately. The kernel likewise when detecting glibc headers included first can use the _UAPI_DEF_xxx macro guards to disable problematic definitions knowing that glibc has identical ABI and API-compatible versions that the program can use without breaking. Cheers, Carlos. -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] Redefinition of struct in6_addr in netinet/in.h and linux/in6.h
On 01/18/2013 02:24 PM, YOSHIFUJI Hideaki wrote: It's simple enough to move all of the __GLIBC__ uses into libc-compat.h, then you control userspace libc coordination from one file. How about just deciding on a single macro/symbol both the kernel and libc (any libc that needs this) define? Something like both the kernel and userland doing: #ifndef __IPV6_BITS_DEFINED #define __IPV6_BITS_DEFINED ... define in6_addr, sockaddr_in6, ipv6_mreq, whatnot #endif Hmm, how should we handle future structs/enums then? For example, if I want to have in6_flowlabel_req{} defined in glibc, what should we do? Include the glibc header first? Or is this some other use case? The point wasn't that you'd have only one macro for all structs/enums (you could split into __IPV6_IN6_ADDR_DEFINED, __IPV6_SOCKADDR_IN6_DEFINED, etc.) but to have the kernel and libc agree on guard macros, instead of having the kernel do #ifdef __GLIBC__ and glibc doing #ifdef _NETINET_IN_H. But as Carlos says, the devil is in the details, and I sure am not qualified on the details here. -- Pedro Alves -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] Redefinition of struct in6_addr in netinet/in.h and linux/in6.h
On 01/18/2013 04:22 AM, Carlos O'Donell wrote: On Thu, Jan 17, 2013 at 11:20 PM, Mike Frysinger vap...@gentoo.org wrote: On Wednesday 16 January 2013 22:15:38 David Miller wrote: From: Carlos O'Donell car...@systemhalted.org Date: Wed, 16 Jan 2013 21:15:03 -0500 +/* If a glibc-based userspace has already included in.h, then we will not + * define in6_addr (nor the defines), sockaddr_in6, or ipv6_mreq. The + * ABI used by the kernel and by glibc match exactly. Neither the kernel + * nor glibc should break this ABI without coordination. + */ +#ifndef _NETINET_IN_H + I think we should shoot for a non-glibc-centric solution. I can't imagine that other libc's won't have the same exact problem with their netinet/in.h conflicting with the kernel's, redefining structures like in6_addr, that we'd want to provide a protection scheme for here as well. yes, the kernel's use of __GLIBC__ in exported headers has already caused problems in the past. fortunately, it's been reduced down to just one case now (stat.h). let's not balloon it back up. -mike I also see coda.h has grown a __GLIBC__ usage. In the next revision of the patch I created a single libc-compat.h header which encompasses the logic for any libc that wants to coordinate with the kernel headers. It's simple enough to move all of the __GLIBC__ uses into libc-compat.h, then you control userspace libc coordination from one file. How about just deciding on a single macro/symbol both the kernel and libc (any libc that needs this) define? Something like both the kernel and userland doing: #ifndef __IPV6_BITS_DEFINED #define __IPV6_BITS_DEFINED ... define in6_addr, sockaddr_in6, ipv6_mreq, whatnot #endif So whichever the application includes first, wins. Too naive? I didn't see this option being discarded, so not sure it was considered. -- Pedro Alves -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] Redefinition of struct in6_addr in netinet/in.h and linux/in6.h
On 01/18/2013 05:44 AM, Pedro Alves wrote: On 01/18/2013 04:22 AM, Carlos O'Donell wrote: On Thu, Jan 17, 2013 at 11:20 PM, Mike Frysinger vap...@gentoo.org wrote: On Wednesday 16 January 2013 22:15:38 David Miller wrote: From: Carlos O'Donell car...@systemhalted.org Date: Wed, 16 Jan 2013 21:15:03 -0500 +/* If a glibc-based userspace has already included in.h, then we will not + * define in6_addr (nor the defines), sockaddr_in6, or ipv6_mreq. The + * ABI used by the kernel and by glibc match exactly. Neither the kernel + * nor glibc should break this ABI without coordination. + */ +#ifndef _NETINET_IN_H + I think we should shoot for a non-glibc-centric solution. I can't imagine that other libc's won't have the same exact problem with their netinet/in.h conflicting with the kernel's, redefining structures like in6_addr, that we'd want to provide a protection scheme for here as well. yes, the kernel's use of __GLIBC__ in exported headers has already caused problems in the past. fortunately, it's been reduced down to just one case now (stat.h). let's not balloon it back up. -mike I also see coda.h has grown a __GLIBC__ usage. In the next revision of the patch I created a single libc-compat.h header which encompasses the logic for any libc that wants to coordinate with the kernel headers. It's simple enough to move all of the __GLIBC__ uses into libc-compat.h, then you control userspace libc coordination from one file. How about just deciding on a single macro/symbol both the kernel and libc (any libc that needs this) define? Something like both the kernel and userland doing: #ifndef __IPV6_BITS_DEFINED #define __IPV6_BITS_DEFINED ... define in6_addr, sockaddr_in6, ipv6_mreq, whatnot #endif So whichever the application includes first, wins. Too naive? I didn't see this option being discarded, so not sure it was considered. Too naive, but *close* to what my patch does :-) The kernel definitions when included first, and in a glibc userspace, must try to mimic the glibc userspace headers and we need more than one guard macro to do that effectively. The reason I jumped into the code is because this kind of problem is easy to talk about, but the devil is in the details. There are certainly some compromises on both sides, but the solution promises to solve this problem. Honestly without UAPI this would have been an impossible task. Cheers, Carlos. -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] Redefinition of struct in6_addr in netinet/in.h and linux/in6.h
On Thursday 2013-01-17 03:05, David Miller wrote: From: Carlos O'Donell car...@systemhalted.org Date: Wed, 16 Jan 2013 20:58:47 -0500 So I just went down the rabbit hole, and the further I get the closer I get to having two exact copies of the same definitions in both glibc and the kernel and using whichever one was included first. Is anyone opposed to that kind of solution? Sounds interesting, please share :-) iptables has the same issue, and solved it its way. (uapi/)linux/netfilter.h is used to get at things like union nf_inet_addr. This union contains struct in6_addr. There is no include for in6_addr in netfilter.h itself. This may break the standalone compilation test, but at least allows for specifying the environment-specific header for in6_addr in the C file: a. userspace: #include netinet/in.h before linux/netfilter.h b. kernel parts: #include linux/in6.h before linux/netfilter.h -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] Redefinition of struct in6_addr in netinet/in.h and linux/in6.h
From: Mike Frysinger vap...@gentoo.org Date: Thu, 17 Jan 2013 23:14:31 -0500 the kernel already exports many types with a __kernel_ prefix. i changed the kernel headers in Gentoo for a few releases (2.6.28 - 2.6.34) to do the same thing to pretty much all the networking headers. a few packages broke, but the number was low, and trivial to fix (a sed would do it most of the time). it's also trivial for userland packages to detect that they need to do this sort of thing in a local header by using linux/version.h and a set of defines to redirect the new structure name back to the old one. would be a lot cleaner to just break it and be done. We are not at liberty to break something that has legitimately compiled successfully for two decades. Your __kernel_ prefix idea breaks compilation just as equally. One thing you certainly don't have to be is happy about this header file situation, but you cannot use that dissatisfaction as justification to make the situation worse by breaking the build for people outside of the world you directly control. -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] Redefinition of struct in6_addr in netinet/in.h and linux/in6.h
On Thu, Jan 17, 2013 at 11:20 PM, Mike Frysinger vap...@gentoo.org wrote: On Wednesday 16 January 2013 22:15:38 David Miller wrote: From: Carlos O'Donell car...@systemhalted.org Date: Wed, 16 Jan 2013 21:15:03 -0500 +/* If a glibc-based userspace has already included in.h, then we will not + * define in6_addr (nor the defines), sockaddr_in6, or ipv6_mreq. The + * ABI used by the kernel and by glibc match exactly. Neither the kernel + * nor glibc should break this ABI without coordination. + */ +#ifndef _NETINET_IN_H + I think we should shoot for a non-glibc-centric solution. I can't imagine that other libc's won't have the same exact problem with their netinet/in.h conflicting with the kernel's, redefining structures like in6_addr, that we'd want to provide a protection scheme for here as well. yes, the kernel's use of __GLIBC__ in exported headers has already caused problems in the past. fortunately, it's been reduced down to just one case now (stat.h). let's not balloon it back up. -mike I also see coda.h has grown a __GLIBC__ usage. In the next revision of the patch I created a single libc-compat.h header which encompasses the logic for any libc that wants to coordinate with the kernel headers. It's simple enough to move all of the __GLIBC__ uses into libc-compat.h, then you control userspace libc coordination from one file. Cheers, Carlos. -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] Redefinition of struct in6_addr in netinet/in.h and linux/in6.h
On 01/16/2013 10:22 PM, YOSHIFUJI Hideaki wrote: Carlos O'Donell wrote: diff --git a/include/uapi/linux/in6.h b/include/uapi/linux/in6.h index f79c372..a2b16a5 100644 --- a/include/uapi/linux/in6.h +++ b/include/uapi/linux/in6.h @@ -23,6 +23,13 @@ #include linux/types.h +/* If a glibc-based userspace has already included in.h, then we will not + * define in6_addr (nor the defines), sockaddr_in6, or ipv6_mreq. The + * ABI used by the kernel and by glibc match exactly. Neither the kernel + * nor glibc should break this ABI without coordination. + */ +#ifndef _NETINET_IN_H + /* * IPv6 address structure */ This should be #if !defined(__GLIBC__) || !defined(_NETINET_IN_H) Correct. If it's non-glibc we want these defines to be used even if a non-glibc defined _NETINET_IN_H. @@ -30,12 +37,20 @@ struct in6_addr { union { __u8u6_addr8[16]; +#if !defined(__GLIBC__) \ +|| (defined(__GLIBC__) (defined(__USE_MISC) || defined(__USE_GNU))) \ +|| defined(__KERNEL__) __be16 u6_addr16[8]; __be32 u6_addr32[4]; +#endif } in6_u; +#if !defined(__GLIBC__) \ +|| (defined(__GLIBC__) (defined(__USE_MISC) || defined(__USE_GNU))) \ +|| defined(__KERNEL__) #define s6_addr in6_u.u6_addr8 #define s6_addr16 in6_u.u6_addr16 #define s6_addr32 in6_u.u6_addr32 +#endif }; 2nd if be after s6_addr? Correct. We want to unconditinally define s6_addr in this case. I've fixed both of those cases in the next version of the patch. Thanks. Cheers, Carlos. -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] Redefinition of struct in6_addr in netinet/in.h and linux/in6.h
Cong Wang wrote: (Cc'ing some glibc developers...) Hello, In glibc source file inet/netinet/in.h and kernel source file include/uapi/linux/in6.h, both define struct in6_addr, and both are visible to user applications. Thomas reported a conflict below. So, how can we handle this? /me is wondering why we didn't see this before. Thanks. On Tue, 2013-01-15 at 12:55 +0200, Thomas Backlund wrote: Cong Wang skrev 15.1.2013 12:11: Does the following patch help? $ git diff include/uapi/linux/if_bridge.h diff --git a/include/uapi/linux/if_bridge.h b/include/uapi/linux/if_bridge.h index 5db2975..653db23 100644 --- a/include/uapi/linux/if_bridge.h +++ b/include/uapi/linux/if_bridge.h @@ -14,6 +14,7 @@ #define _UAPI_LINUX_IF_BRIDGE_H #include linux/types.h +#include linux/in6.h #define SYSFS_BRIDGE_ATTR bridge #define SYSFS_BRIDGE_FDB brforward Well, I suggested the same fix in the beginning of the thread on netdev and lkml: if_bridge.h: include in6.h for struct in6_addr use as it seemed to fix the libvirt case but then asked it to be ignored after I tried to build connman, and hit this conflict with glibc-2.17: In file included from /usr/include/arpa/inet.h:22:0, from ./include/connman/inet.h:25, from src/connman.h:128, from src/tethering.c:40: /usr/include/netinet/in.h:35:5: error: expected identifier before numeric constant /usr/include/netinet/in.h:197:8: error: redefinition of 'struct in6_addr' In file included from /usr/include/linux/if_bridge.h:17:0, from src/tethering.c:38: /usr/include/linux/in6.h:30:8: note: originally defined here In file included from /usr/include/arpa/inet.h:22:0, from ./include/connman/inet.h:25, from src/connman.h:128, from src/tethering.c:40: /usr/include/netinet/in.h:238:8: error: redefinition of 'struct sockaddr_in6' In file included from /usr/include/linux/if_bridge.h:17:0, from src/tethering.c:38: /usr/include/linux/in6.h:46:8: note: originally defined here In file included from /usr/include/arpa/inet.h:22:0, from ./include/connman/inet.h:25, from src/connman.h:128, from src/tethering.c:40: /usr/include/netinet/in.h:274:8: error: redefinition of 'struct ipv6_mreq' In file included from /usr/include/linux/if_bridge.h:17:0, from src/tethering.c:38: /usr/include/linux/in6.h:54:8: note: originally defined here make[1]: *** [src/src_connmand-tethering.o] Error 1 So I'm not sure it's the right one... This is not a new issue. In addition to this, netinet/in.h also conflits with linux/in.h. We might have #if !defined(__GLIBC__) || !defined(_NETINET_IN_H) : #endif around those conflicting definitions in uapi/linux/in{,6}.h. --yoshfuji -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] Redefinition of struct in6_addr in netinet/in.h and linux/in6.h
On Wed, 2013-01-16 at 23:21 +0900, YOSHIFUJI Hideaki wrote: Cong Wang wrote: (Cc'ing some glibc developers...) Hello, In glibc source file inet/netinet/in.h and kernel source file include/uapi/linux/in6.h, both define struct in6_addr, and both are visible to user applications. Thomas reported a conflict below. So, how can we handle this? /me is wondering why we didn't see this before. [...] This is not a new issue. In addition to this, netinet/in.h also conflits with linux/in.h. We might have #if !defined(__GLIBC__) || !defined(_NETINET_IN_H) : #endif around those conflicting definitions in uapi/linux/in{,6}.h. This only solves half the problem, as netinet/in.h might be included after linux/in.h. Also, not all Linux userland uses glibc. Ben. -- Ben Hutchings, Staff Engineer, Solarflare Not speaking for my employer; that's the marketing department's job. They asked us to note that Solarflare product names are trademarked. -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] Redefinition of struct in6_addr in netinet/in.h and linux/in6.h
On Wed, 2013-01-16 at 12:04 -0500, Mike Frysinger wrote: On Wednesday 16 January 2013 10:47:12 Ben Hutchings wrote: On Wed, 2013-01-16 at 23:21 +0900, YOSHIFUJI Hideaki wrote: Cong Wang wrote: (Cc'ing some glibc developers...) Hello, In glibc source file inet/netinet/in.h and kernel source file include/uapi/linux/in6.h, both define struct in6_addr, and both are visible to user applications. Thomas reported a conflict below. So, how can we handle this? /me is wondering why we didn't see this before. [...] This is not a new issue. In addition to this, netinet/in.h also conflits with linux/in.h. We might have #if !defined(__GLIBC__) || !defined(_NETINET_IN_H) #endif around those conflicting definitions in uapi/linux/in{,6}.h. This only solves half the problem, as netinet/in.h might be included after linux/in.h. Also, not all Linux userland uses glibc. certainly true, but the current expectation is that you don't mix your ABIs. Whose expectation? Which ABIs are being mixed? if you're programming with the C library API, then use the C library headers. if you're banging directly on the kernel, then use the kernel headers. not saying it's a perfect solution, but it works for the vast majority of use cases. In practice most C programs for Linux will use a mixture of thinly wrapped system calls and higher-level APIs from the C library, and never really call the kernel directly (as that requires inline assembler). Userland programmers will work around this historical mess by tweaking the #include order or splitting source files. But they shouldn't have to. Ben. -- Ben Hutchings, Staff Engineer, Solarflare Not speaking for my employer; that's the marketing department's job. They asked us to note that Solarflare product names are trademarked. -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] Redefinition of struct in6_addr in netinet/in.h and linux/in6.h
From: Mike Frysinger vap...@gentoo.org Date: Wed, 16 Jan 2013 14:22:16 -0500 On Wednesday 16 January 2013 13:59:59 David Miller wrote: This has been done for decades, wake up. and it's been broken for just as long. no need to be a dick. By being ignorant and having such a simplistic view of the situation, you're the one who's actually the dick. -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] Redefinition of struct in6_addr in netinet/in.h and linux/in6.h
From: Mike Frysinger vap...@gentoo.org Date: Wed, 16 Jan 2013 12:04:56 -0500 certainly true, but the current expectation is that you don't mix your ABIs. if you're programming with the C library API, then use the C library headers. if you're banging directly on the kernel, then use the kernel headers. not saying it's a perfect solution, but it works for the vast majority of use cases. This isn't how real life works. GLIBC itself brings in some of the kernel headers, as do various library headers for libraries other than glibc. So you can get these conflicting headers included indirectly, and it is of no fault of any of the various parties involved. We have to make them work when included at the same time somehow, and this is totally unavoidable. -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] Redefinition of struct in6_addr in netinet/in.h and linux/in6.h
From: Mike Frysinger vap...@gentoo.org Date: Wed, 16 Jan 2013 12:28:39 -0500 if you're not calling the kernel directly, why are you including the kernel headers ? what is the problem people are actually trying to address here (and no, i want to include both headers is not the answer) ? When GLIBC doesn't provide it's own definition of some networking macros or interfaces that the kernel provides, people include the kernel header. This has been done for decades, wake up. -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] Redefinition of struct in6_addr in netinet/in.h and linux/in6.h
From: Ben Hutchings bhutchi...@solarflare.com Date: Wed, 16 Jan 2013 15:47:12 + On Wed, 2013-01-16 at 23:21 +0900, YOSHIFUJI Hideaki wrote: Cong Wang wrote: (Cc'ing some glibc developers...) Hello, In glibc source file inet/netinet/in.h and kernel source file include/uapi/linux/in6.h, both define struct in6_addr, and both are visible to user applications. Thomas reported a conflict below. So, how can we handle this? /me is wondering why we didn't see this before. [...] This is not a new issue. In addition to this, netinet/in.h also conflits with linux/in.h. We might have #if !defined(__GLIBC__) || !defined(_NETINET_IN_H) : #endif around those conflicting definitions in uapi/linux/in{,6}.h. This only solves half the problem, as netinet/in.h might be included after linux/in.h. Also, not all Linux userland uses glibc. So I've been looking at reasonable ways to fix this. What would be really nice is if GCC treated multiple identical definitions of structures the same way it handles multiple identical definitions of CPP defines. Which is to silently accept them. But that's not the case, so we need a different solution. Another thing to do is to use a new structure for in6_addr in kernel headers exported to userland. If we were to make the structure members and accessor macros identical we could avoid breaking most if not all apps. However the type name is different so: struct in6_addr *p = kern_struct-member; wouldn't work so well. And you couldn't fix up the sources to these kinds of accesses in a way that work cleanly both before and after changing the kernel headers. I'm out of ideas for today. -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] Redefinition of struct in6_addr in netinet/in.h and linux/in6.h
From: Carlos O'Donell car...@systemhalted.org Date: Wed, 16 Jan 2013 20:58:47 -0500 So I just went down the rabbit hole, and the further I get the closer I get to having two exact copies of the same definitions in both glibc and the kernel and using whichever one was included first. Is anyone opposed to that kind of solution? Sounds interesting, please share :-) -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] Redefinition of struct in6_addr in netinet/in.h and linux/in6.h
Carlos O'Donell wrote: diff --git a/include/uapi/linux/in6.h b/include/uapi/linux/in6.h index f79c372..a2b16a5 100644 --- a/include/uapi/linux/in6.h +++ b/include/uapi/linux/in6.h : #define IPV6_PRIORITY_14 0x0e00 #define IPV6_PRIORITY_15 0x0f00 + +#ifndef _NETINET_IN_H +#if defined (__GLIBC__) +/* Include all of the other IPPROTO_* defines for userspace. */ +#include linux/ipproto.h +#endif /* * IPV6 extension headers Overall, it looks good, but why including linux/ipproto.h? --yoshfuji -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] Redefinition of struct in6_addr in netinet/in.h and linux/in6.h
From: Carlos O'Donell car...@systemhalted.org Date: Wed, 16 Jan 2013 21:15:03 -0500 +/* If a glibc-based userspace has already included in.h, then we will not + * define in6_addr (nor the defines), sockaddr_in6, or ipv6_mreq. The + * ABI used by the kernel and by glibc match exactly. Neither the kernel + * nor glibc should break this ABI without coordination. + */ +#ifndef _NETINET_IN_H + I think we should shoot for a non-glibc-centric solution. I can't imagine that other libc's won't have the same exact problem with their netinet/in.h conflicting with the kernel's, redefining structures like in6_addr, that we'd want to provide a protection scheme for here as well. Let's pick some more generic names and themes for the CPP macros and comments we use to protect the header file blocks and describe that protection scheme. Thanks. -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] Redefinition of struct in6_addr in netinet/in.h and linux/in6.h
Carlos O'Donell wrote: diff --git a/include/uapi/linux/in6.h b/include/uapi/linux/in6.h index f79c372..a2b16a5 100644 --- a/include/uapi/linux/in6.h +++ b/include/uapi/linux/in6.h @@ -23,6 +23,13 @@ #include linux/types.h +/* If a glibc-based userspace has already included in.h, then we will not + * define in6_addr (nor the defines), sockaddr_in6, or ipv6_mreq. The + * ABI used by the kernel and by glibc match exactly. Neither the kernel + * nor glibc should break this ABI without coordination. + */ +#ifndef _NETINET_IN_H + /* * IPv6 address structure */ This should be #if !defined(__GLIBC__) || !defined(_NETINET_IN_H) @@ -30,12 +37,20 @@ struct in6_addr { union { __u8u6_addr8[16]; +#if !defined(__GLIBC__) \ +|| (defined(__GLIBC__) (defined(__USE_MISC) || defined(__USE_GNU))) \ +|| defined(__KERNEL__) __be16 u6_addr16[8]; __be32 u6_addr32[4]; +#endif } in6_u; +#if !defined(__GLIBC__) \ +|| (defined(__GLIBC__) (defined(__USE_MISC) || defined(__USE_GNU))) \ +|| defined(__KERNEL__) #define s6_addr in6_u.u6_addr8 #define s6_addr16in6_u.u6_addr16 #define s6_addr32in6_u.u6_addr32 +#endif }; 2nd if be after s6_addr? --yoshfuji -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] Redefinition of struct in6_addr in netinet/in.h and linux/in6.h
On Wed, 2013-01-16 at 14:22 -0500, Mike Frysinger wrote: but this is still too vague. what headers/definitions do people want to see simultaneously included ? changes would be needed on both sides (kernel C library). Hi, Mike, Please take a look at my first email in this thread. The user application includes linux/if_bridge.h and netinet/in.h. linux/if_bridge.h uses struct_in6 but doesn't include linux/in6.h (this is my bad, sorry), an obvious fix is just including linux/in6.h. But this immediately breaks applications which include linux/if_bridge.h and netinet/in.h, just as what Thomas reported. And if_bridge.h is kernel-specific, there is no corresponding glibc one, so you can't blame applications which include both of them. Thanks. -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] Redefinition of struct in6_addr in netinet/in.h and linux/in6.h
On Thu, Jan 17, 2013 at 2:59 AM, David Miller da...@davemloft.net wrote: When GLIBC doesn't provide it's own definition of some networking macros or interfaces that the kernel provides, people include the kernel header. Recently I got a problem when copying a structure from kernel to userspace, after debugging I found: kernel: include/linux/inet.h #define INET6_ADDRSTRLEN(48) glibc: /usr/include/netinet/in.h #define INET6_ADDRSTRLEN 46 Any reason to differentiate them from each other? -- Thanks, Jike -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] Redefinition of struct in6_addr in netinet/in.h and linux/in6.h
On 01/16/2013 04:45 PM, David Miller wrote: From: Ben Hutchings bhutchi...@solarflare.com Date: Wed, 16 Jan 2013 15:47:12 + On Wed, 2013-01-16 at 23:21 +0900, YOSHIFUJI Hideaki wrote: Cong Wang wrote: (Cc'ing some glibc developers...) Hello, In glibc source file inet/netinet/in.h and kernel source file include/uapi/linux/in6.h, both define struct in6_addr, and both are visible to user applications. Thomas reported a conflict below. So, how can we handle this? /me is wondering why we didn't see this before. [...] This is not a new issue. In addition to this, netinet/in.h also conflits with linux/in.h. We might have #if !defined(__GLIBC__) || !defined(_NETINET_IN_H) : #endif around those conflicting definitions in uapi/linux/in{,6}.h. This only solves half the problem, as netinet/in.h might be included after linux/in.h. Also, not all Linux userland uses glibc. So I've been looking at reasonable ways to fix this. What would be really nice is if GCC treated multiple identical definitions of structures the same way it handles multiple identical definitions of CPP defines. Which is to silently accept them. But that's not the case, so we need a different solution. Another thing to do is to use a new structure for in6_addr in kernel headers exported to userland. If we were to make the structure members and accessor macros identical we could avoid breaking most if not all apps. However the type name is different so: struct in6_addr *p = kern_struct-member; wouldn't work so well. And you couldn't fix up the sources to these kinds of accesses in a way that work cleanly both before and after changing the kernel headers. I'm out of ideas for today. So I just went down the rabbit hole, and the further I get the closer I get to having two exact copies of the same definitions in both glibc and the kernel and using whichever one was included first. Is anyone opposed to that kind of solution? There is some ugliness there. Cheers, Carlos. -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] Redefinition of struct in6_addr in netinet/in.h and linux/in6.h
On 01/16/2013 01:57 PM, David Miller wrote: From: Mike Frysinger vap...@gentoo.org Date: Wed, 16 Jan 2013 12:04:56 -0500 certainly true, but the current expectation is that you don't mix your ABIs. if you're programming with the C library API, then use the C library headers. if you're banging directly on the kernel, then use the kernel headers. not saying it's a perfect solution, but it works for the vast majority of use cases. This isn't how real life works. GLIBC itself brings in some of the kernel headers, as do various library headers for libraries other than glibc. So you can get these conflicting headers included indirectly, and it is of no fault of any of the various parties involved. We have to make them work when included at the same time somehow, and this is totally unavoidable. Just to put some code behind the comments I made earlier. 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. Comments? 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. glibc/ 2012-01-16 Carlos O'Donell car...@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..882739d 100644 --- a/inet/netinet/in.h +++ b/inet/netinet/in.h @@ -26,6 +26,20 @@ __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 + +/* 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 /* Standard well-defined IP protocols. */ enum { @@ -75,6 +89,8 @@ enum #define IPPROTO_DSTOPTSIPPROTO_DSTOPTS IPPROTO_MTP = 92, /* Multicast Transport Protocol. */ #define IPPROTO_MTPIPPROTO_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. */ @@ -83,13 +99,15 @@ enum #define IPPROTO_COMP IPPROTO_COMP IPPROTO_SCTP = 132, /* Stream Control Transmission Protocol. */ #define IPPROTO_SCTP IPPROTO_SCTP +IPPROTO_MH = 135, /* IPv6 mobility header. */ +#define IPPROTO_MH IPPROTO_MH IPPROTO_UDPLITE = 136, /* UDP-Lite protocol. */ #define IPPROTO_UDPLITEIPPROTO_UDPLITE IPPROTO_RAW = 255,/* Raw IP packets. */ #define IPPROTO_RAWIPPROTO_RAW IPPROTO_MAX }; - +#endif /* !__USE_KERNEL_IPV6_DEFS */ /* Type to represent a port. */ typedef uint16_t in_port_t; @@ -134,15 +152,6 @@ enum IPPORT_USERRESERVED = 5000
Re: [libvirt] Redefinition of struct in6_addr in netinet/in.h and linux/in6.h
On Thu, 2013-01-17 at 11:55 +0800, Jike Song wrote: On Thu, Jan 17, 2013 at 2:59 AM, David Miller da...@davemloft.net wrote: When GLIBC doesn't provide it's own definition of some networking macros or interfaces that the kernel provides, people include the kernel header. Recently I got a problem when copying a structure from kernel to userspace, after debugging I found: kernel: include/linux/inet.h #define INET6_ADDRSTRLEN(48) glibc: /usr/include/netinet/in.h #define INET6_ADDRSTRLEN 46 Any reason to differentiate them from each other? I see no reason, even although I don't know why it is 46 instead of 40. But include/linux/inet.h is not exported to user-space, AFAIK. -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] Redefinition of struct in6_addr in netinet/in.h and linux/in6.h
- Original Message - I see no reason, even although I don't know why it is 46 instead of 40. Ok, for ::::::255.255.255.255. -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list