Re: [libvirt] Redefinition of struct in6_addr in netinet/in.h and linux/in6.h

2013-01-18 Thread YOSHIFUJI Hideaki
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

2013-01-18 Thread Carlos O'Donell
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

2013-01-18 Thread Pedro Alves
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

2013-01-18 Thread Pedro Alves
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

2013-01-18 Thread Carlos O'Donell
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

2013-01-17 Thread Jan Engelhardt
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

2013-01-17 Thread David Miller
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

2013-01-17 Thread Carlos O'Donell
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

2013-01-17 Thread Carlos O'Donell
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

2013-01-16 Thread YOSHIFUJI Hideaki
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

2013-01-16 Thread Ben Hutchings
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

2013-01-16 Thread Ben Hutchings
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

2013-01-16 Thread David Miller
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

2013-01-16 Thread David Miller
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

2013-01-16 Thread David Miller
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

2013-01-16 Thread David Miller
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

2013-01-16 Thread David Miller
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

2013-01-16 Thread YOSHIFUJI Hideaki
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

2013-01-16 Thread David Miller
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

2013-01-16 Thread YOSHIFUJI Hideaki
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

2013-01-16 Thread Cong Wang
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

2013-01-16 Thread Jike Song
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

2013-01-16 Thread Carlos O'Donell
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

2013-01-16 Thread Carlos O'Donell
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

2013-01-16 Thread Cong Wang
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

2013-01-16 Thread Cong Wang


- 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