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 <vapier@gentoo.org> wrote:
On Wednesday 16 January 2013 22:15:38 David Miller wrote:
From: Carlos O'Donell <carlos@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.

On 01/18/2013 04:22 AM, Carlos O'Donell wrote:
On Thu, Jan 17, 2013 at 11:20 PM, Mike Frysinger <vapier@gentoo.org> wrote:
On Wednesday 16 January 2013 22:15:38 David Miller wrote:
From: Carlos O'Donell <carlos@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

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 <vapier@gentoo.org> wrote:
On Wednesday 16 January 2013 22:15:38 David Miller wrote:
From: Carlos O'Donell <carlos@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.

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 <vapier@gentoo.org> wrote:
On Wednesday 16 January 2013 22:15:38 David Miller wrote:
From: Carlos O'Donell <carlos@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

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

On Fri, Jan 18, 2013 at 9:36 AM, Pedro Alves <palves@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.
participants (3)
-
Carlos O'Donell
-
Pedro Alves
-
YOSHIFUJI Hideaki