* if_bridge.h: include in6.h for struct in6_addr use @ 2013-01-13 18:38 Thomas Backlund 2013-01-13 20:05 ` Thomas Backlund 0 siblings, 1 reply; 45+ messages in thread From: Thomas Backlund @ 2013-01-13 18:38 UTC (permalink / raw) To: netdev; +Cc: linux-kernel [-- Attachment #1: Type: text/plain, Size: 879 bytes --] patch both inline and attached as thunderbird tends to mess up ... ----- if_bridge.h uses struct in6_addr ip6; but does not include the in6.h header. Found by trying to build libvirt and connman against 3.8-rc3 headers. Reported-by: Colin Guthrie <colin@mageia.org> Reported-by: Christiaan Welvaart <cjw@daneel.dyndns.org> Signed-off-by: Thomas Backlund <tmb@mageia.org> -- diff -Nurp linux-3.8-rc3/include/uapi/linux/if_bridge.h linux-3.8-rc3.fix/include/uapi/linux/if_bridge.h --- linux-3.8-rc3/include/uapi/linux/if_bridge.h 2013-01-13 20:09:54.257271755 +0200 +++ linux-3.8-rc3.fix/include/uapi/linux/if_bridge.h 2013-01-13 20:15:04.153676151 +0200 @@ -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" ----- Thomas [-- Attachment #2: if_bridge.h-include-in6.h-for-struct-in6_addr-use.patch --] [-- Type: text/plain, Size: 764 bytes --] if_bridge.h uses struct in6_addr ip6; but does not include the in6.h header. Found by trying to build libvirt and connman against 3.8-rc3 headers. Reported-by: Colin Guthrie <colin@mageia.org> Reported-by: Christiaan Welvaart <cjw@daneel.dyndns.org> Signed-off-by: Thomas Backlund <tmb@mageia.org> -- diff -Nurp linux-3.8-rc3/include/uapi/linux/if_bridge.h linux-3.8-rc3.fix/include/uapi/linux/if_bridge.h --- linux-3.8-rc3/include/uapi/linux/if_bridge.h 2013-01-13 20:09:54.257271755 +0200 +++ linux-3.8-rc3.fix/include/uapi/linux/if_bridge.h 2013-01-13 20:15:04.153676151 +0200 @@ -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" ^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: if_bridge.h: include in6.h for struct in6_addr use 2013-01-13 18:38 if_bridge.h: include in6.h for struct in6_addr use Thomas Backlund @ 2013-01-13 20:05 ` Thomas Backlund 2013-01-14 23:57 ` [libvirt] " Eric Blake 0 siblings, 1 reply; 45+ messages in thread From: Thomas Backlund @ 2013-01-13 20:05 UTC (permalink / raw) To: netdev; +Cc: linux-kernel, libvirt-list Thomas Backlund skrev 13.1.2013 20:38: > patch both inline and attached as thunderbird tends to mess up ... > ----- > > if_bridge.h uses struct in6_addr ip6; but does not include the in6.h > header. > > Found by trying to build libvirt and connman against 3.8-rc3 headers. > Ok, ignore this patch, it's not the correct fix as it introduces redefinitions... Btw, the error that I hit that made me suggest this fix was libvirt config check bailing out: config.log:/usr/include/linux/if_bridge.h:173:20: error: field 'ip6' has incomplete type > Reported-by: Colin Guthrie <colin@mageia.org> > Reported-by: Christiaan Welvaart <cjw@daneel.dyndns.org> > Signed-off-by: Thomas Backlund <tmb@mageia.org> > > -- > > diff -Nurp linux-3.8-rc3/include/uapi/linux/if_bridge.h > linux-3.8-rc3.fix/include/uapi/linux/if_bridge.h > --- linux-3.8-rc3/include/uapi/linux/if_bridge.h 2013-01-13 > 20:09:54.257271755 +0200 > +++ linux-3.8-rc3.fix/include/uapi/linux/if_bridge.h 2013-01-13 > 20:15:04.153676151 +0200 > @@ -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" > > > ----- > Thomas > -- Thomas ^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: [libvirt] if_bridge.h: include in6.h for struct in6_addr use 2013-01-13 20:05 ` Thomas Backlund @ 2013-01-14 23:57 ` Eric Blake 2013-01-15 10:03 ` [libvirt] the patch "bridge: export multicast database via netlink" broke kernel 3.8 uapi (was: Re: if_bridge.h: include in6.h for struct in6_addr use) Thomas Backlund 2013-03-13 15:17 ` [libvirt] if_bridge.h: include in6.h for struct in6_addr use Kumar Gala 0 siblings, 2 replies; 45+ messages in thread From: Eric Blake @ 2013-01-14 23:57 UTC (permalink / raw) To: Thomas Backlund; +Cc: netdev, linux-kernel, libvirt-list [-- Attachment #1: Type: text/plain, Size: 1335 bytes --] On 01/13/2013 01:05 PM, Thomas Backlund wrote: > Thomas Backlund skrev 13.1.2013 20:38: >> patch both inline and attached as thunderbird tends to mess up ... > >> ----- >> >> if_bridge.h uses struct in6_addr ip6; but does not include the in6.h >> header. >> >> Found by trying to build libvirt and connman against 3.8-rc3 headers. >> > > Ok, > ignore this patch, it's not the correct fix as it introduces > redefinitions... > > Btw, the error that I hit that made me suggest this fix was libvirt > config check bailing out: > > config.log:/usr/include/linux/if_bridge.h:173:20: error: field 'ip6' has > incomplete type Hmm, just now noticing this thread, after independently hitting and and having to work around the same problem in libvirt: https://www.redhat.com/archives/libvir-list/2013-January/msg00930.html https://bugzilla.redhat.com/show_bug.cgi?id=895141 >> --- linux-3.8-rc3/include/uapi/linux/if_bridge.h 2013-01-13 >> 20:09:54.257271755 +0200 >> +++ linux-3.8-rc3.fix/include/uapi/linux/if_bridge.h 2013-01-13 >> 20:15:04.153676151 +0200 >> @@ -14,6 +14,7 @@ >> #define _UAPI_LINUX_IF_BRIDGE_H >> >> #include <linux/types.h> >> +#include <linux/in6.h> >> -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org [-- Attachment #2: OpenPGP digital signature --] [-- Type: application/pgp-signature, Size: 621 bytes --] ^ permalink raw reply [flat|nested] 45+ messages in thread
* [libvirt] the patch "bridge: export multicast database via netlink" broke kernel 3.8 uapi (was: Re: if_bridge.h: include in6.h for struct in6_addr use) 2013-01-14 23:57 ` [libvirt] " Eric Blake @ 2013-01-15 10:03 ` Thomas Backlund 2013-01-15 10:11 ` the patch "bridge: export multicast database via netlink" broke kernel 3.8 uapi (was: Re: [libvirt] " Cong Wang 2013-03-13 15:17 ` [libvirt] if_bridge.h: include in6.h for struct in6_addr use Kumar Gala 1 sibling, 1 reply; 45+ messages in thread From: Thomas Backlund @ 2013-01-15 10:03 UTC (permalink / raw) To: amwang; +Cc: tgraf, netdev, linux-kernel, libvirt-list Eric Blake skrev 15.1.2013 01:57: > On 01/13/2013 01:05 PM, Thomas Backlund wrote: >> Thomas Backlund skrev 13.1.2013 20:38: >>> patch both inline and attached as thunderbird tends to mess up ... >> >>> ----- >>> >>> if_bridge.h uses struct in6_addr ip6; but does not include the in6.h >>> header. >>> >>> Found by trying to build libvirt and connman against 3.8-rc3 headers. >>> >> >> Ok, >> ignore this patch, it's not the correct fix as it introduces >> redefinitions... >> >> Btw, the error that I hit that made me suggest this fix was libvirt >> config check bailing out: >> >> config.log:/usr/include/linux/if_bridge.h:173:20: error: field 'ip6' has >> incomplete type > > Hmm, just now noticing this thread, after independently hitting and and > having to work around the same problem in libvirt: > https://www.redhat.com/archives/libvir-list/2013-January/msg00930.html > https://bugzilla.redhat.com/show_bug.cgi?id=895141 Yep, and the commit breaking uapi headers is by using "struct in6_addr ip6" is: From ee07c6e7a6f8a25c18f0a6b18152fbd7499245f6 Mon Sep 17 00:00:00 2001 From: Cong Wang <amwang@redhat.com> Date: Fri, 7 Dec 2012 00:04:48 +0000 Subject: [PATCH] bridge: export multicast database via netlink V5: fix two bugs pointed out by Thomas remove seq check for now, mark it as TODO V4: remove some useless #include some coding style fix V3: drop debugging printk's update selinux perm table as well V2: drop patch 1/2, export ifindex directly Redesign netlink attributes Improve netlink seq check Handle IPv6 addr as well This patch exports bridge multicast database via netlink message type RTM_GETMDB. Similar to fdb, but currently bridge-specific. We may need to support modify multicast database too (RTM_{ADD,DEL}MDB). (Thanks to Thomas for patient reviews) Cc: Herbert Xu <herbert@gondor.apana.org.au> Cc: Stephen Hemminger <shemminger@vyatta.com> Cc: "David S. Miller" <davem@davemloft.net> Cc: Thomas Graf <tgraf@suug.ch> Cc: Jesper Dangaard Brouer <brouer@redhat.com> Signed-off-by: Cong Wang <amwang@redhat.com> Acked-by: Thomas Graf <tgraf@suug.ch> Signed-off-by: David S. Miller <davem@davemloft.net> --- include/uapi/linux/if_bridge.h | 55 ++++++++++++++ include/uapi/linux/rtnetlink.h | 3 + net/bridge/Makefile | 2 +- net/bridge/br_mdb.c | 163 ++++++++++++++++++++++++++++++++++++++++ net/bridge/br_multicast.c | 1 + net/bridge/br_private.h | 1 + security/selinux/nlmsgtab.c | 1 + 7 files changed, 225 insertions(+), 1 deletions(-) create mode 100644 net/bridge/br_mdb.c -- Thomas ^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: the patch "bridge: export multicast database via netlink" broke kernel 3.8 uapi (was: Re: [libvirt] if_bridge.h: include in6.h for struct in6_addr use) 2013-01-15 10:03 ` [libvirt] the patch "bridge: export multicast database via netlink" broke kernel 3.8 uapi (was: Re: if_bridge.h: include in6.h for struct in6_addr use) Thomas Backlund @ 2013-01-15 10:11 ` Cong Wang 2013-01-15 10:55 ` the patch "bridge: export multicast database via netlink" broke kernel 3.8 uapi Thomas Backlund 0 siblings, 1 reply; 45+ messages in thread From: Cong Wang @ 2013-01-15 10:11 UTC (permalink / raw) To: Thomas Backlund; +Cc: Eric Blake, netdev, linux-kernel, libvirt-list, tgraf On Tue, 2013-01-15 at 12:03 +0200, Thomas Backlund wrote: > Eric Blake skrev 15.1.2013 01:57: > > On 01/13/2013 01:05 PM, Thomas Backlund wrote: > >> Thomas Backlund skrev 13.1.2013 20:38: > >>> patch both inline and attached as thunderbird tends to mess up ... > >> > >>> ----- > >>> > >>> if_bridge.h uses struct in6_addr ip6; but does not include the in6.h > >>> header. > >>> > >>> Found by trying to build libvirt and connman against 3.8-rc3 headers. > >>> > >> > >> Ok, > >> ignore this patch, it's not the correct fix as it introduces > >> redefinitions... > >> > >> Btw, the error that I hit that made me suggest this fix was libvirt > >> config check bailing out: > >> > >> config.log:/usr/include/linux/if_bridge.h:173:20: error: field 'ip6' has > >> incomplete type > > > > Hmm, just now noticing this thread, after independently hitting and and > > having to work around the same problem in libvirt: > > https://www.redhat.com/archives/libvir-list/2013-January/msg00930.html > > https://bugzilla.redhat.com/show_bug.cgi?id=895141 > > > Yep, > > and the commit breaking uapi headers is by using "struct in6_addr ip6" is: > > > From ee07c6e7a6f8a25c18f0a6b18152fbd7499245f6 Mon Sep 17 00:00:00 2001 > From: Cong Wang <amwang@redhat.com> > Date: Fri, 7 Dec 2012 00:04:48 +0000 > Subject: [PATCH] bridge: export multicast database via netlink 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" ^ permalink raw reply related [flat|nested] 45+ messages in thread
* Re: the patch "bridge: export multicast database via netlink" broke kernel 3.8 uapi 2013-01-15 10:11 ` the patch "bridge: export multicast database via netlink" broke kernel 3.8 uapi (was: Re: [libvirt] " Cong Wang @ 2013-01-15 10:55 ` Thomas Backlund 2013-01-16 5:51 ` Cong Wang 2013-01-16 6:06 ` Redefinition of struct in6_addr in <netinet/in.h> and <linux/in6.h> Cong Wang 0 siblings, 2 replies; 45+ messages in thread From: Thomas Backlund @ 2013-01-15 10:55 UTC (permalink / raw) To: Cong Wang Cc: Thomas Backlund, Eric Blake, netdev, linux-kernel, libvirt-list, tgraf Cong Wang skrev 15.1.2013 12:11: > On Tue, 2013-01-15 at 12:03 +0200, Thomas Backlund wrote: >> Eric Blake skrev 15.1.2013 01:57: >>> On 01/13/2013 01:05 PM, Thomas Backlund wrote: >>>> Thomas Backlund skrev 13.1.2013 20:38: >>>>> patch both inline and attached as thunderbird tends to mess up ... >>>> >>>>> ----- >>>>> >>>>> if_bridge.h uses struct in6_addr ip6; but does not include the in6.h >>>>> header. >>>>> >>>>> Found by trying to build libvirt and connman against 3.8-rc3 headers. >>>>> >>>> >>>> Ok, >>>> ignore this patch, it's not the correct fix as it introduces >>>> redefinitions... >>>> >>>> Btw, the error that I hit that made me suggest this fix was libvirt >>>> config check bailing out: >>>> >>>> config.log:/usr/include/linux/if_bridge.h:173:20: error: field 'ip6' has >>>> incomplete type >>> >>> Hmm, just now noticing this thread, after independently hitting and and >>> having to work around the same problem in libvirt: >>> https://www.redhat.com/archives/libvir-list/2013-January/msg00930.html >>> https://bugzilla.redhat.com/show_bug.cgi?id=895141 >> >> >> Yep, >> >> and the commit breaking uapi headers is by using "struct in6_addr ip6" is: >> >> >> From ee07c6e7a6f8a25c18f0a6b18152fbd7499245f6 Mon Sep 17 00:00:00 2001 >> From: Cong Wang <amwang@redhat.com> >> Date: Fri, 7 Dec 2012 00:04:48 +0000 >> Subject: [PATCH] bridge: export multicast database via netlink > > 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... -- Thomas ^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: the patch "bridge: export multicast database via netlink" broke kernel 3.8 uapi 2013-01-15 10:55 ` the patch "bridge: export multicast database via netlink" broke kernel 3.8 uapi Thomas Backlund @ 2013-01-16 5:51 ` Cong Wang 2013-01-16 6:06 ` Redefinition of struct in6_addr in <netinet/in.h> and <linux/in6.h> Cong Wang 1 sibling, 0 replies; 45+ messages in thread From: Cong Wang @ 2013-01-16 5:51 UTC (permalink / raw) To: Thomas Backlund; +Cc: Eric Blake, netdev, linux-kernel, libvirt-list, tgraf On Tue, 2013-01-15 at 12:55 +0200, Thomas Backlund wrote: > > 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... % rpm -q --whatprovides /usr/include/netinet/in.h glibc-headers-2.14.90-24.fc16.9.x86_64 Seems we have conflicts with glibc headers... ^ permalink raw reply [flat|nested] 45+ messages in thread
* Redefinition of struct in6_addr in <netinet/in.h> and <linux/in6.h> 2013-01-15 10:55 ` the patch "bridge: export multicast database via netlink" broke kernel 3.8 uapi Thomas Backlund 2013-01-16 5:51 ` Cong Wang @ 2013-01-16 6:06 ` Cong Wang 2013-01-16 14:21 ` YOSHIFUJI Hideaki 1 sibling, 1 reply; 45+ messages in thread From: Cong Wang @ 2013-01-16 6:06 UTC (permalink / raw) To: Thomas Backlund Cc: Eric Blake, netdev, linux-kernel, libvirt-list, tgraf, David Miller, libc-alpha, schwab, carlos (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... > ^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: Redefinition of struct in6_addr in <netinet/in.h> and <linux/in6.h> 2013-01-16 6:06 ` Redefinition of struct in6_addr in <netinet/in.h> and <linux/in6.h> Cong Wang @ 2013-01-16 14:21 ` YOSHIFUJI Hideaki 2013-01-16 15:47 ` Ben Hutchings 0 siblings, 1 reply; 45+ messages in thread From: YOSHIFUJI Hideaki @ 2013-01-16 14:21 UTC (permalink / raw) To: Cong Wang Cc: Thomas Backlund, Eric Blake, netdev, linux-kernel, libvirt-list, tgraf, David Miller, libc-alpha, schwab, carlos, 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 ^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: Redefinition of struct in6_addr in <netinet/in.h> and <linux/in6.h> 2013-01-16 14:21 ` YOSHIFUJI Hideaki @ 2013-01-16 15:47 ` Ben Hutchings 2013-01-16 17:04 ` Mike Frysinger 2013-01-16 21:45 ` David Miller 0 siblings, 2 replies; 45+ messages in thread From: Ben Hutchings @ 2013-01-16 15:47 UTC (permalink / raw) To: YOSHIFUJI Hideaki Cc: Cong Wang, Thomas Backlund, Eric Blake, netdev, linux-kernel, libvirt-list, tgraf, David Miller, libc-alpha, schwab, carlos 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. ^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: Redefinition of struct in6_addr in <netinet/in.h> and <linux/in6.h> 2013-01-16 15:47 ` Ben Hutchings @ 2013-01-16 17:04 ` Mike Frysinger 2013-01-16 17:10 ` Ben Hutchings 2013-01-16 18:57 ` David Miller 2013-01-16 21:45 ` David Miller 1 sibling, 2 replies; 45+ messages in thread From: Mike Frysinger @ 2013-01-16 17:04 UTC (permalink / raw) To: libc-alpha Cc: Ben Hutchings, YOSHIFUJI Hideaki, Cong Wang, Thomas Backlund, Eric Blake, netdev, linux-kernel, libvirt-list, tgraf, David Miller, schwab, carlos [-- Attachment #1: Type: Text/Plain, Size: 1302 bytes --] 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. 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. -mike [-- Attachment #2: This is a digitally signed message part. --] [-- Type: application/pgp-signature, Size: 836 bytes --] ^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: Redefinition of struct in6_addr in <netinet/in.h> and <linux/in6.h> 2013-01-16 17:04 ` Mike Frysinger @ 2013-01-16 17:10 ` Ben Hutchings 2013-01-16 17:28 ` Mike Frysinger 2013-01-16 18:57 ` David Miller 1 sibling, 1 reply; 45+ messages in thread From: Ben Hutchings @ 2013-01-16 17:10 UTC (permalink / raw) To: Mike Frysinger Cc: libc-alpha, YOSHIFUJI Hideaki, Cong Wang, Thomas Backlund, Eric Blake, netdev, linux-kernel, libvirt-list, tgraf, David Miller, schwab, carlos 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. ^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: Redefinition of struct in6_addr in <netinet/in.h> and <linux/in6.h> 2013-01-16 17:10 ` Ben Hutchings @ 2013-01-16 17:28 ` Mike Frysinger 2013-01-16 18:59 ` David Miller 0 siblings, 1 reply; 45+ messages in thread From: Mike Frysinger @ 2013-01-16 17:28 UTC (permalink / raw) To: Ben Hutchings Cc: libc-alpha, YOSHIFUJI Hideaki, Cong Wang, Thomas Backlund, Eric Blake, netdev, linux-kernel, libvirt-list, tgraf, David Miller, schwab, carlos [-- Attachment #1: Type: Text/Plain, Size: 2898 bytes --] On Wednesday 16 January 2013 12:10:11 Ben Hutchings wrote: > 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? the kernel's view of the world and the C library's view of the world. there is a lot of overlap, but it's the C library's job to provide a POSIX compliant interface (and then some). obvious examples: - the stat structures are not the same and require translation - ptrace structures are not the same and you really need to use the C lib one - the readdir function is completely different but to this specific question, if you're calling the kernel's network functions directly, then you need to include the kernel's headers for its definition of how things are passed. if you're calling the C library's network functions, then use the C library's headers. > > 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. 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) ? -mike [-- Attachment #2: This is a digitally signed message part. --] [-- Type: application/pgp-signature, Size: 836 bytes --] ^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: Redefinition of struct in6_addr in <netinet/in.h> and <linux/in6.h> 2013-01-16 17:28 ` Mike Frysinger @ 2013-01-16 18:59 ` David Miller 2013-01-16 19:22 ` Mike Frysinger 2013-01-17 3:55 ` [libvirt] " Jike Song 0 siblings, 2 replies; 45+ messages in thread From: David Miller @ 2013-01-16 18:59 UTC (permalink / raw) To: vapier Cc: bhutchings, libc-alpha, yoshfuji, amwang, tmb, eblake, netdev, linux-kernel, libvirt-list, tgraf, schwab, carlos From: Mike Frysinger <vapier@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. ^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: Redefinition of struct in6_addr in <netinet/in.h> and <linux/in6.h> 2013-01-16 18:59 ` David Miller @ 2013-01-16 19:22 ` Mike Frysinger 2013-01-16 19:25 ` David Miller 2013-01-17 3:40 ` Cong Wang 2013-01-17 3:55 ` [libvirt] " Jike Song 1 sibling, 2 replies; 45+ messages in thread From: Mike Frysinger @ 2013-01-16 19:22 UTC (permalink / raw) To: David Miller Cc: bhutchings, libc-alpha, yoshfuji, amwang, tmb, eblake, netdev, linux-kernel, libvirt-list, tgraf, schwab, carlos [-- Attachment #1: Type: Text/Plain, Size: 895 bytes --] On Wednesday 16 January 2013 13:59:59 David Miller wrote: > From: Mike Frysinger <vapier@gentoo.org> > > 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. sounds like glibc's headers are out of date and we should update them to include the missing definitions 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). > This has been done for decades, wake up. and it's been broken for just as long. no need to be a dick. -mike [-- Attachment #2: This is a digitally signed message part. --] [-- Type: application/pgp-signature, Size: 836 bytes --] ^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: Redefinition of struct in6_addr in <netinet/in.h> and <linux/in6.h> 2013-01-16 19:22 ` Mike Frysinger @ 2013-01-16 19:25 ` David Miller 2013-01-17 3:40 ` Cong Wang 1 sibling, 0 replies; 45+ messages in thread From: David Miller @ 2013-01-16 19:25 UTC (permalink / raw) To: vapier Cc: bhutchings, libc-alpha, yoshfuji, amwang, tmb, eblake, netdev, linux-kernel, libvirt-list, tgraf, schwab, carlos From: Mike Frysinger <vapier@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. ^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: Redefinition of struct in6_addr in <netinet/in.h> and <linux/in6.h> 2013-01-16 19:22 ` Mike Frysinger 2013-01-16 19:25 ` David Miller @ 2013-01-17 3:40 ` Cong Wang 1 sibling, 0 replies; 45+ messages in thread From: Cong Wang @ 2013-01-17 3:40 UTC (permalink / raw) To: Mike Frysinger Cc: David Miller, bhutchings, libc-alpha, yoshfuji, tmb, eblake, netdev, linux-kernel, libvirt-list, tgraf, schwab, carlos 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. ^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: [libvirt] Redefinition of struct in6_addr in <netinet/in.h> and <linux/in6.h> 2013-01-16 18:59 ` David Miller 2013-01-16 19:22 ` Mike Frysinger @ 2013-01-17 3:55 ` Jike Song 2013-01-17 6:59 ` Cong Wang 1 sibling, 1 reply; 45+ messages in thread From: Jike Song @ 2013-01-17 3:55 UTC (permalink / raw) To: David Miller Cc: libc-alpha, amwang, yoshfuji, netdev, linux-kernel, libvirt-list, tgraf, carlos, schwab, bhutchings, vapier, tmb [-- Attachment #1.1: Type: text/plain, Size: 539 bytes --] On Thu, Jan 17, 2013 at 2:59 AM, David Miller <davem@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 [-- Attachment #1.2: Type: text/html, Size: 1198 bytes --] [-- Attachment #2: Type: text/plain, Size: 0 bytes --] ^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: Redefinition of struct in6_addr in <netinet/in.h> and <linux/in6.h> 2013-01-17 3:55 ` [libvirt] " Jike Song @ 2013-01-17 6:59 ` Cong Wang 2013-01-17 7:02 ` Cong Wang 0 siblings, 1 reply; 45+ messages in thread From: Cong Wang @ 2013-01-17 6:59 UTC (permalink / raw) To: Jike Song Cc: David Miller, vapier, bhutchings, libc-alpha, yoshfuji, tmb, eblake, netdev, linux-kernel, libvirt-list, tgraf, schwab, carlos On Thu, 2013-01-17 at 11:55 +0800, Jike Song wrote: > On Thu, Jan 17, 2013 at 2:59 AM, David Miller <davem@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. ^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: Redefinition of struct in6_addr in <netinet/in.h> and <linux/in6.h> 2013-01-17 6:59 ` Cong Wang @ 2013-01-17 7:02 ` Cong Wang 0 siblings, 0 replies; 45+ messages in thread From: Cong Wang @ 2013-01-17 7:02 UTC (permalink / raw) To: Jike Song Cc: David Miller, vapier, bhutchings, libc-alpha, yoshfuji, tmb, eblake, netdev, linux-kernel, libvirt-list, tgraf, schwab, carlos ----- Original Message ----- > > I see no reason, even although I don't know why it is 46 instead of > 40. Ok, for "0000:0000:0000:0000:0000:0000:255.255.255.255". ^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: Redefinition of struct in6_addr in <netinet/in.h> and <linux/in6.h> 2013-01-16 17:04 ` Mike Frysinger 2013-01-16 17:10 ` Ben Hutchings @ 2013-01-16 18:57 ` David Miller 2013-01-16 19:29 ` Mike Frysinger 2013-01-17 2:15 ` Carlos O'Donell 1 sibling, 2 replies; 45+ messages in thread From: David Miller @ 2013-01-16 18:57 UTC (permalink / raw) To: vapier Cc: libc-alpha, bhutchings, yoshfuji, amwang, tmb, eblake, netdev, linux-kernel, libvirt-list, tgraf, schwab, carlos From: Mike Frysinger <vapier@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. ^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: Redefinition of struct in6_addr in <netinet/in.h> and <linux/in6.h> 2013-01-16 18:57 ` David Miller @ 2013-01-16 19:29 ` Mike Frysinger 2013-01-17 2:15 ` Carlos O'Donell 1 sibling, 0 replies; 45+ messages in thread From: Mike Frysinger @ 2013-01-16 19:29 UTC (permalink / raw) To: David Miller Cc: libc-alpha, bhutchings, yoshfuji, amwang, tmb, eblake, netdev, linux-kernel, libvirt-list, tgraf, schwab, carlos [-- Attachment #1: Type: Text/Plain, Size: 1916 bytes --] On Wednesday 16 January 2013 13:57:44 David Miller wrote: > From: Mike Frysinger <vapier@gentoo.org> > > 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. the headers glibc includes tend to be pretty stand alone specifically so that it doesn't matter > We have to make them work when included at the same time somehow, and > this is totally unavoidable. "them" is vague. saying that every kernel header has to be usable in the same compilation unit as every C library header regardless of order is unrealistic (at least it is today). there are cases where they define the same structure different because the structure as the C library expects is different from what the kernel syscall expects. you could avoid that on the kernel side by giving them all prefixes (like __kernel_), but that didn't seem entirely palpable to the kernel folks. i couldn't even get them to remove crap that breaks non- glibc C libraries (e.g. uapi/linux/stat.h -- looks like someone inadvertently fixed uapi/linux/socket.h finally). for many networking headers, the C library will provide enums & defines while the kernel only provides enums. including the kernel after the C library one leads to parsing errors as the defines expand in the enum and kill it. like linux/in.h and netinet/in.h and IPPROTO_*. -mike [-- Attachment #2: This is a digitally signed message part. --] [-- Type: application/pgp-signature, Size: 836 bytes --] ^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: Redefinition of struct in6_addr in <netinet/in.h> and <linux/in6.h> 2013-01-16 18:57 ` David Miller 2013-01-16 19:29 ` Mike Frysinger @ 2013-01-17 2:15 ` Carlos O'Donell 2013-01-17 3:10 ` YOSHIFUJI Hideaki ` (2 more replies) 1 sibling, 3 replies; 45+ messages in thread From: Carlos O'Donell @ 2013-01-17 2:15 UTC (permalink / raw) To: David Miller Cc: vapier, libc-alpha, bhutchings, yoshfuji, amwang, tmb, eblake, netdev, linux-kernel, libvirt-list, tgraf, schwab On 01/16/2013 01:57 PM, David Miller wrote: > From: Mike Frysinger <vapier@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 <carlos@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_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. */ @@ -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_UDPLITE IPPROTO_UDPLITE IPPROTO_RAW = 255, /* Raw IP packets. */ #define IPPROTO_RAW IPPROTO_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 }; - -/* 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 +200,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 +218,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 */ @@ -218,7 +228,6 @@ extern const struct in6_addr in6addr_loopback; /* ::1 */ #define INET_ADDRSTRLEN 16 #define INET6_ADDRSTRLEN 46 - /* Structure describing an Internet socket address. */ struct sockaddr_in { @@ -233,6 +242,7 @@ struct sockaddr_in sizeof (struct in_addr)]; }; +#ifndef __USE_KERNEL_IPV6_DEFS /* Ditto, for IPv6. */ struct sockaddr_in6 { @@ -242,7 +252,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 +278,7 @@ struct ip_mreq_source }; #endif - +#ifndef __USE_KERNEL_IPV6_DEFS /* Likewise, for IPv6. */ struct ipv6_mreq { @@ -278,7 +288,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 +359,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..36bb72b 100644 --- a/sysdeps/unix/sysv/linux/bits/in.h +++ b/sysdeps/unix/sysv/linux/bits/in.h @@ -21,6 +21,15 @@ # 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 IPPROTO_* defines, in6_addr (nor the + defines), sockaddr_in6, or ip_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 +# 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'. */ --- linux/ Compile tested and inspected. Signed-off-by: Carlos O'Donell <carlos@redhat.com> include/uapi/linux/in.h | 32 +----------------------- include/uapi/linux/in6.h | 50 +++++++++++++++++++++++++++++++------ include/uapi/linux/ipproto.h | 0 include/uapi/linux/ipproto.h | 57 +++++++++++++++++++++++++++++++++++++++++++ 4 files changed, 101 insertions(+), 38 deletions(-) diff --git a/include/uapi/linux/in.h b/include/uapi/linux/in.h index 9edb441..998ecb2 100644 --- a/include/uapi/linux/in.h +++ b/include/uapi/linux/in.h @@ -21,36 +21,8 @@ #include <linux/types.h> #include <linux/socket.h> -/* Standard well-defined IP protocols. */ -enum { - IPPROTO_IP = 0, /* Dummy protocol for TCP */ - IPPROTO_ICMP = 1, /* Internet Control Message Protocol */ - IPPROTO_IGMP = 2, /* Internet Group Management Protocol */ - IPPROTO_IPIP = 4, /* IPIP tunnels (older KA9Q tunnels use 94) */ - IPPROTO_TCP = 6, /* Transmission Control Protocol */ - IPPROTO_EGP = 8, /* Exterior Gateway Protocol */ - IPPROTO_PUP = 12, /* PUP protocol */ - IPPROTO_UDP = 17, /* User Datagram Protocol */ - IPPROTO_IDP = 22, /* XNS IDP protocol */ - IPPROTO_DCCP = 33, /* Datagram Congestion Control Protocol */ - IPPROTO_RSVP = 46, /* RSVP protocol */ - 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 */ - IPPROTO_UDPLITE = 136, /* UDP-Lite (RFC 3828) */ - - IPPROTO_RAW = 255, /* Raw IP packets */ - IPPROTO_MAX -}; - +/* Include IPPROTO_* defines. */ +#include <linux/ipproto.h> /* Internet address. */ struct in_addr { 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 */ @@ -30,12 +37,20 @@ struct in6_addr { union { __u8 u6_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 }; /* IPv6 Wildcard Address (::) and Loopback Address (::1) defined in RFC2553 @@ -56,9 +71,12 @@ struct ipv6_mreq { struct in6_addr ipv6mr_multiaddr; /* local IPv6 address of interface */ - int ipv6mr_ifindex; + int ipv6mr_interface; +#define ipv6mr_ifindex ipv6mr_interface }; +#endif /* !_NET_INET_H */ + #define ipv6mr_acaddr ipv6mr_multiaddr struct in6_flowlabel_req { @@ -116,16 +134,32 @@ struct in6_flowlabel_req { #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 */ -#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 */ +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 /* !_NETINET_IN_H */ /* * IPv6 TLV options. diff --git a/include/uapi/linux/ipproto.h b/include/uapi/linux/ipproto.h new file mode 100644 index 0000000..5a183e6 --- /dev/null +++ b/include/uapi/linux/ipproto.h @@ -0,0 +1,57 @@ +#ifndef _UAPI_LINUX_IPPROTO_H +#define _UAPI_LINUX_IPPROTO_H + +/* 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. */ +#define IPPROTO_DCCP IPPROTO_DCCP + IPPROTO_IPV6 = 41, /* IPv6 header. */ +#define IPPROTO_IPV6 IPPROTO_IPV6 + IPPROTO_RSVP = 46, /* Reservation Protocol. */ +#define IPPROTO_RSVP IPPROTO_RSVP + IPPROTO_GRE = 47, /* General Routing Encapsulation. */ +#define IPPROTO_GRE IPPROTO_GRE + IPPROTO_ESP = 50, /* encapsulating security payload. */ +#define IPPROTO_ESP IPPROTO_ESP + IPPROTO_AH = 51, /* authentication header. */ +#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 Transmission Protocol. */ +#define IPPROTO_SCTP IPPROTO_SCTP + IPPROTO_UDPLITE = 136, /* UDP-Lite protocol. */ +#define IPPROTO_UDPLITE IPPROTO_UDPLITE + IPPROTO_RAW = 255, /* Raw IP packets. */ +#define IPPROTO_RAW IPPROTO_RAW + IPPROTO_MAX +}; + +#endif /* _UAPI_LINUX_IPPROTO_H */ --- Cheers, Carlos. ^ permalink raw reply related [flat|nested] 45+ messages in thread
* Re: Redefinition of struct in6_addr in <netinet/in.h> and <linux/in6.h> 2013-01-17 2:15 ` Carlos O'Donell @ 2013-01-17 3:10 ` YOSHIFUJI Hideaki 2013-01-17 3:15 ` David Miller 2013-01-17 3:22 ` YOSHIFUJI Hideaki 2 siblings, 0 replies; 45+ messages in thread From: YOSHIFUJI Hideaki @ 2013-01-17 3:10 UTC (permalink / raw) To: Carlos O'Donell Cc: David Miller, vapier, libc-alpha, bhutchings, amwang, tmb, eblake, netdev, linux-kernel, libvirt-list, tgraf, schwab, 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 ^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: Redefinition of struct in6_addr in <netinet/in.h> and <linux/in6.h> 2013-01-17 2:15 ` Carlos O'Donell 2013-01-17 3:10 ` YOSHIFUJI Hideaki @ 2013-01-17 3:15 ` David Miller 2013-01-18 4:20 ` Mike Frysinger 2013-01-17 3:22 ` YOSHIFUJI Hideaki 2 siblings, 1 reply; 45+ messages in thread From: David Miller @ 2013-01-17 3:15 UTC (permalink / raw) To: carlos Cc: vapier, libc-alpha, bhutchings, yoshfuji, amwang, tmb, eblake, netdev, linux-kernel, libvirt-list, tgraf, schwab 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. 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. ^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: Redefinition of struct in6_addr in <netinet/in.h> and <linux/in6.h> 2013-01-17 3:15 ` David Miller @ 2013-01-18 4:20 ` Mike Frysinger 2013-01-18 4:22 ` Carlos O'Donell 0 siblings, 1 reply; 45+ messages in thread From: Mike Frysinger @ 2013-01-18 4:20 UTC (permalink / raw) To: David Miller Cc: carlos, libc-alpha, bhutchings, yoshfuji, amwang, tmb, eblake, netdev, linux-kernel, libvirt-list, tgraf, schwab [-- Attachment #1: Type: Text/Plain, Size: 1012 bytes --] 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 [-- Attachment #2: This is a digitally signed message part. --] [-- Type: application/pgp-signature, Size: 836 bytes --] ^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: Redefinition of struct in6_addr in <netinet/in.h> and <linux/in6.h> 2013-01-18 4:20 ` Mike Frysinger @ 2013-01-18 4:22 ` Carlos O'Donell 2013-01-18 4:34 ` Mike Frysinger 2013-01-18 10:44 ` Pedro Alves 0 siblings, 2 replies; 45+ messages in thread From: Carlos O'Donell @ 2013-01-18 4:22 UTC (permalink / raw) To: Mike Frysinger Cc: David Miller, libc-alpha, bhutchings, yoshfuji, amwang, tmb, eblake, netdev, linux-kernel, libvirt-list, tgraf, schwab 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. ^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: Redefinition of struct in6_addr in <netinet/in.h> and <linux/in6.h> 2013-01-18 4:22 ` Carlos O'Donell @ 2013-01-18 4:34 ` Mike Frysinger 2013-01-18 10:44 ` Pedro Alves 1 sibling, 0 replies; 45+ messages in thread From: Mike Frysinger @ 2013-01-18 4:34 UTC (permalink / raw) To: Carlos O'Donell Cc: David Miller, libc-alpha, bhutchings, yoshfuji, amwang, tmb, eblake, netdev, linux-kernel, libvirt-list, tgraf, schwab [-- Attachment #1: Type: Text/Plain, Size: 1587 bytes --] On Thursday 17 January 2013 23:22:26 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. > > I also see coda.h has grown a __GLIBC__ usage. that file is just a pile of cruft :). it's something that'd be rejected by today's kernel standard as it's full of OS shim code. "#ifdef DOS" ? comeon! fortunately, coda is pretty uncommon, so this issue doesn't bite most people, and i've never bothered with it. the same cannot be said of stat.h. -mike [-- Attachment #2: This is a digitally signed message part. --] [-- Type: application/pgp-signature, Size: 836 bytes --] ^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: Redefinition of struct in6_addr in <netinet/in.h> and <linux/in6.h> 2013-01-18 4:22 ` Carlos O'Donell 2013-01-18 4:34 ` Mike Frysinger @ 2013-01-18 10:44 ` Pedro Alves 2013-01-18 13:35 ` Carlos O'Donell 1 sibling, 1 reply; 45+ messages in thread From: Pedro Alves @ 2013-01-18 10:44 UTC (permalink / raw) To: Carlos O'Donell Cc: Mike Frysinger, David Miller, libc-alpha, bhutchings, yoshfuji, amwang, tmb, eblake, netdev, linux-kernel, libvirt-list, tgraf, schwab 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 ^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: Redefinition of struct in6_addr in <netinet/in.h> and <linux/in6.h> 2013-01-18 10:44 ` Pedro Alves @ 2013-01-18 13:35 ` Carlos O'Donell 2013-01-18 14:24 ` YOSHIFUJI Hideaki 0 siblings, 1 reply; 45+ messages in thread From: Carlos O'Donell @ 2013-01-18 13:35 UTC (permalink / raw) To: Pedro Alves Cc: Mike Frysinger, David Miller, libc-alpha, bhutchings, yoshfuji, amwang, tmb, eblake, netdev, linux-kernel, libvirt-list, tgraf, schwab 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. ^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: Redefinition of struct in6_addr in <netinet/in.h> and <linux/in6.h> 2013-01-18 13:35 ` Carlos O'Donell @ 2013-01-18 14:24 ` YOSHIFUJI Hideaki 2013-01-18 14:36 ` Pedro Alves 0 siblings, 1 reply; 45+ messages in thread From: YOSHIFUJI Hideaki @ 2013-01-18 14:24 UTC (permalink / raw) To: Carlos O'Donell Cc: Pedro Alves, Mike Frysinger, David Miller, libc-alpha, bhutchings, amwang, tmb, eblake, netdev, linux-kernel, libvirt-list, tgraf, schwab, 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 <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 ^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: Redefinition of struct in6_addr in <netinet/in.h> and <linux/in6.h> 2013-01-18 14:24 ` YOSHIFUJI Hideaki @ 2013-01-18 14:36 ` Pedro Alves 2013-01-18 14:54 ` Carlos O'Donell 2013-01-21 0:54 ` Mike Frysinger 0 siblings, 2 replies; 45+ messages in thread From: Pedro Alves @ 2013-01-18 14:36 UTC (permalink / raw) To: YOSHIFUJI Hideaki Cc: Carlos O'Donell, Mike Frysinger, David Miller, libc-alpha, bhutchings, amwang, tmb, eblake, netdev, linux-kernel, libvirt-list, tgraf, schwab 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 ^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: Redefinition of struct in6_addr in <netinet/in.h> and <linux/in6.h> 2013-01-18 14:36 ` Pedro Alves @ 2013-01-18 14:54 ` Carlos O'Donell 2013-01-21 0:54 ` Mike Frysinger 1 sibling, 0 replies; 45+ messages in thread From: Carlos O'Donell @ 2013-01-18 14:54 UTC (permalink / raw) To: Pedro Alves Cc: YOSHIFUJI Hideaki, Mike Frysinger, David Miller, libc-alpha, bhutchings, amwang, tmb, eblake, netdev, linux-kernel, libvirt-list, tgraf, schwab 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. ^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: Redefinition of struct in6_addr in <netinet/in.h> and <linux/in6.h> 2013-01-18 14:36 ` Pedro Alves 2013-01-18 14:54 ` Carlos O'Donell @ 2013-01-21 0:54 ` Mike Frysinger 1 sibling, 0 replies; 45+ messages in thread From: Mike Frysinger @ 2013-01-21 0:54 UTC (permalink / raw) To: Pedro Alves Cc: YOSHIFUJI Hideaki, Carlos O'Donell, David Miller, libc-alpha, bhutchings, amwang, tmb, eblake, netdev, linux-kernel, libvirt-list, tgraf, schwab [-- Attachment #1: Type: Text/Plain, Size: 1580 bytes --] On Friday 18 January 2013 09:36:35 Pedro Alves 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. i shipped some kernel header versions in Gentoo where the linux network headers would include the C library's header (as defined by POSIX) so the structs that were the same would only come from glibc. the only reported breakage was due to defines that the kernel provided but glibc did not. -mike [-- Attachment #2: This is a digitally signed message part. --] [-- Type: application/pgp-signature, Size: 836 bytes --] ^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: Redefinition of struct in6_addr in <netinet/in.h> and <linux/in6.h> 2013-01-17 2:15 ` Carlos O'Donell 2013-01-17 3:10 ` YOSHIFUJI Hideaki 2013-01-17 3:15 ` David Miller @ 2013-01-17 3:22 ` YOSHIFUJI Hideaki 2013-01-18 4:13 ` Carlos O'Donell 2 siblings, 1 reply; 45+ messages in thread From: YOSHIFUJI Hideaki @ 2013-01-17 3:22 UTC (permalink / raw) To: Carlos O'Donell Cc: David Miller, vapier, libc-alpha, bhutchings, amwang, tmb, eblake, netdev, linux-kernel, libvirt-list, tgraf, schwab, 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 { > __u8 u6_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? --yoshfuji ^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: Redefinition of struct in6_addr in <netinet/in.h> and <linux/in6.h> 2013-01-17 3:22 ` YOSHIFUJI Hideaki @ 2013-01-18 4:13 ` Carlos O'Donell 0 siblings, 0 replies; 45+ messages in thread From: Carlos O'Donell @ 2013-01-18 4:13 UTC (permalink / raw) To: YOSHIFUJI Hideaki Cc: David Miller, vapier, libc-alpha, bhutchings, amwang, tmb, eblake, netdev, linux-kernel, libvirt-list, tgraf, schwab 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 { >> __u8 u6_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. ^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: Redefinition of struct in6_addr in <netinet/in.h> and <linux/in6.h> 2013-01-16 15:47 ` Ben Hutchings 2013-01-16 17:04 ` Mike Frysinger @ 2013-01-16 21:45 ` David Miller 2013-01-17 1:58 ` Carlos O'Donell 2013-01-18 4:14 ` Mike Frysinger 1 sibling, 2 replies; 45+ messages in thread From: David Miller @ 2013-01-16 21:45 UTC (permalink / raw) To: bhutchings Cc: yoshfuji, amwang, tmb, eblake, netdev, linux-kernel, libvirt-list, tgraf, libc-alpha, schwab, carlos From: Ben Hutchings <bhutchings@solarflare.com> Date: Wed, 16 Jan 2013 15:47:12 +0000 > 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. ^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: Redefinition of struct in6_addr in <netinet/in.h> and <linux/in6.h> 2013-01-16 21:45 ` David Miller @ 2013-01-17 1:58 ` Carlos O'Donell 2013-01-17 2:05 ` David Miller 2013-01-18 4:14 ` Mike Frysinger 1 sibling, 1 reply; 45+ messages in thread From: Carlos O'Donell @ 2013-01-17 1:58 UTC (permalink / raw) To: David Miller Cc: bhutchings, yoshfuji, amwang, tmb, eblake, netdev, linux-kernel, libvirt-list, tgraf, libc-alpha, schwab On 01/16/2013 04:45 PM, David Miller wrote: > From: Ben Hutchings <bhutchings@solarflare.com> > Date: Wed, 16 Jan 2013 15:47:12 +0000 > >> 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. ^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: Redefinition of struct in6_addr in <netinet/in.h> and <linux/in6.h> 2013-01-17 1:58 ` Carlos O'Donell @ 2013-01-17 2:05 ` David Miller 2013-01-17 10:57 ` Jan Engelhardt 0 siblings, 1 reply; 45+ messages in thread From: David Miller @ 2013-01-17 2:05 UTC (permalink / raw) To: carlos Cc: bhutchings, yoshfuji, amwang, tmb, eblake, netdev, linux-kernel, libvirt-list, tgraf, libc-alpha, schwab From: Carlos O'Donell <carlos@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 :-) ^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: Redefinition of struct in6_addr in <netinet/in.h> and <linux/in6.h> 2013-01-17 2:05 ` David Miller @ 2013-01-17 10:57 ` Jan Engelhardt 0 siblings, 0 replies; 45+ messages in thread From: Jan Engelhardt @ 2013-01-17 10:57 UTC (permalink / raw) To: David Miller Cc: carlos, bhutchings, yoshfuji, amwang, tmb, eblake, netdev, linux-kernel, libvirt-list, tgraf, libc-alpha, schwab On Thursday 2013-01-17 03:05, David Miller wrote: >From: Carlos O'Donell <carlos@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> ^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: Redefinition of struct in6_addr in <netinet/in.h> and <linux/in6.h> 2013-01-16 21:45 ` David Miller 2013-01-17 1:58 ` Carlos O'Donell @ 2013-01-18 4:14 ` Mike Frysinger 2013-01-18 4:55 ` David Miller 1 sibling, 1 reply; 45+ messages in thread From: Mike Frysinger @ 2013-01-18 4:14 UTC (permalink / raw) To: libc-alpha Cc: David Miller, bhutchings, yoshfuji, amwang, tmb, eblake, netdev, linux-kernel, libvirt-list, tgraf, schwab, carlos [-- Attachment #1: Type: Text/Plain, Size: 1034 bytes --] On Wednesday 16 January 2013 16:45:11 David Miller wrote: > 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. 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. -mike [-- Attachment #2: This is a digitally signed message part. --] [-- Type: application/pgp-signature, Size: 836 bytes --] ^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: Redefinition of struct in6_addr in <netinet/in.h> and <linux/in6.h> 2013-01-18 4:14 ` Mike Frysinger @ 2013-01-18 4:55 ` David Miller 2013-01-18 5:27 ` Mike Frysinger 0 siblings, 1 reply; 45+ messages in thread From: David Miller @ 2013-01-18 4:55 UTC (permalink / raw) To: vapier Cc: libc-alpha, bhutchings, yoshfuji, amwang, tmb, eblake, netdev, linux-kernel, libvirt-list, tgraf, schwab, carlos From: Mike Frysinger <vapier@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. ^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: Redefinition of struct in6_addr in <netinet/in.h> and <linux/in6.h> 2013-01-18 4:55 ` David Miller @ 2013-01-18 5:27 ` Mike Frysinger 0 siblings, 0 replies; 45+ messages in thread From: Mike Frysinger @ 2013-01-18 5:27 UTC (permalink / raw) To: David Miller Cc: libc-alpha, bhutchings, yoshfuji, amwang, tmb, eblake, netdev, linux-kernel, libvirt-list, tgraf, schwab, carlos [-- Attachment #1: Type: Text/Plain, Size: 1683 bytes --] On Thursday 17 January 2013 23:55:24 David Miller wrote: > From: Mike Frysinger <vapier@gentoo.org> > > 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. i doubt there are code bases that have compiled successfully for that long w/out being changed. in fact, the kernel user headers (until recently) have been a volatile mess requiring constant care & feeding of user programs to keep compiling. if anything, this would simply be maintaining the status quo for a bit longer. > 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. i think you're making wrong assumptions about my position on the topic. i never said i was against cleaning up the kernel user headers (and in fact, you can find many commits from me doing exactly that over the years in the kernel). -mike [-- Attachment #2: This is a digitally signed message part. --] [-- Type: application/pgp-signature, Size: 836 bytes --] ^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: [libvirt] if_bridge.h: include in6.h for struct in6_addr use 2013-01-14 23:57 ` [libvirt] " Eric Blake 2013-01-15 10:03 ` [libvirt] the patch "bridge: export multicast database via netlink" broke kernel 3.8 uapi (was: Re: if_bridge.h: include in6.h for struct in6_addr use) Thomas Backlund @ 2013-03-13 15:17 ` Kumar Gala 2013-03-13 16:24 ` Eric Blake 1 sibling, 1 reply; 45+ messages in thread From: Kumar Gala @ 2013-03-13 15:17 UTC (permalink / raw) To: Eric Blake Cc: Thomas Backlund, netdev, linux-kernel@vger.kernel.org list, libvirt-list, stephen, Zhenhua Luo On Jan 14, 2013, at 5:57 PM, Eric Blake wrote: > On 01/13/2013 01:05 PM, Thomas Backlund wrote: >> Thomas Backlund skrev 13.1.2013 20:38: >>> patch both inline and attached as thunderbird tends to mess up ... >> >>> ----- >>> >>> if_bridge.h uses struct in6_addr ip6; but does not include the in6.h >>> header. >>> >>> Found by trying to build libvirt and connman against 3.8-rc3 headers. >>> >> >> Ok, >> ignore this patch, it's not the correct fix as it introduces >> redefinitions... >> >> Btw, the error that I hit that made me suggest this fix was libvirt >> config check bailing out: >> >> config.log:/usr/include/linux/if_bridge.h:173:20: error: field 'ip6' has >> incomplete type > > Hmm, just now noticing this thread, after independently hitting and and > having to work around the same problem in libvirt: > https://www.redhat.com/archives/libvir-list/2013-January/msg00930.html > https://bugzilla.redhat.com/show_bug.cgi?id=895141 > >>> --- linux-3.8-rc3/include/uapi/linux/if_bridge.h 2013-01-13 >>> 20:09:54.257271755 +0200 >>> +++ linux-3.8-rc3.fix/include/uapi/linux/if_bridge.h 2013-01-13 >>> 20:15:04.153676151 +0200 >>> @@ -14,6 +14,7 @@ >>> #define _UAPI_LINUX_IF_BRIDGE_H >>> >>> #include <linux/types.h> >>> +#include <linux/in6.h> >>> Did this ever get resolved ? - k ^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: [libvirt] if_bridge.h: include in6.h for struct in6_addr use 2013-03-13 15:17 ` [libvirt] if_bridge.h: include in6.h for struct in6_addr use Kumar Gala @ 2013-03-13 16:24 ` Eric Blake 0 siblings, 0 replies; 45+ messages in thread From: Eric Blake @ 2013-03-13 16:24 UTC (permalink / raw) To: Kumar Gala Cc: Thomas Backlund, netdev, linux-kernel@vger.kernel.org list, libvirt-list, stephen, Zhenhua Luo [-- Attachment #1: Type: text/plain, Size: 723 bytes --] On 03/13/2013 09:17 AM, Kumar Gala wrote: >>>> >>>> if_bridge.h uses struct in6_addr ip6; but does not include the in6.h >>>> header. >>>> >>>> Found by trying to build libvirt and connman against 3.8-rc3 headers. >>>> >>> > Did this ever get resolved ? Libvirt has managed to work around the kernel issue in the meantime. But just today, I hit the same issue with the latest kernel-headers-3.8.2-206.fc18.x86_64 on Fedora 18 when backporting to an older libvirt branch that did not have the workaround. A kernel person would have to speak up for certain, but I think it is still a problem. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org [-- Attachment #2: OpenPGP digital signature --] [-- Type: application/pgp-signature, Size: 621 bytes --] ^ permalink raw reply [flat|nested] 45+ messages in thread
end of thread, other threads:[~2013-03-13 16:24 UTC | newest] Thread overview: 45+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2013-01-13 18:38 if_bridge.h: include in6.h for struct in6_addr use Thomas Backlund 2013-01-13 20:05 ` Thomas Backlund 2013-01-14 23:57 ` [libvirt] " Eric Blake 2013-01-15 10:03 ` [libvirt] the patch "bridge: export multicast database via netlink" broke kernel 3.8 uapi (was: Re: if_bridge.h: include in6.h for struct in6_addr use) Thomas Backlund 2013-01-15 10:11 ` the patch "bridge: export multicast database via netlink" broke kernel 3.8 uapi (was: Re: [libvirt] " Cong Wang 2013-01-15 10:55 ` the patch "bridge: export multicast database via netlink" broke kernel 3.8 uapi Thomas Backlund 2013-01-16 5:51 ` Cong Wang 2013-01-16 6:06 ` Redefinition of struct in6_addr in <netinet/in.h> and <linux/in6.h> Cong Wang 2013-01-16 14:21 ` YOSHIFUJI Hideaki 2013-01-16 15:47 ` Ben Hutchings 2013-01-16 17:04 ` Mike Frysinger 2013-01-16 17:10 ` Ben Hutchings 2013-01-16 17:28 ` Mike Frysinger 2013-01-16 18:59 ` David Miller 2013-01-16 19:22 ` Mike Frysinger 2013-01-16 19:25 ` David Miller 2013-01-17 3:40 ` Cong Wang 2013-01-17 3:55 ` [libvirt] " Jike Song 2013-01-17 6:59 ` Cong Wang 2013-01-17 7:02 ` Cong Wang 2013-01-16 18:57 ` David Miller 2013-01-16 19:29 ` Mike Frysinger 2013-01-17 2:15 ` Carlos O'Donell 2013-01-17 3:10 ` YOSHIFUJI Hideaki 2013-01-17 3:15 ` David Miller 2013-01-18 4:20 ` Mike Frysinger 2013-01-18 4:22 ` Carlos O'Donell 2013-01-18 4:34 ` Mike Frysinger 2013-01-18 10:44 ` Pedro Alves 2013-01-18 13:35 ` Carlos O'Donell 2013-01-18 14:24 ` YOSHIFUJI Hideaki 2013-01-18 14:36 ` Pedro Alves 2013-01-18 14:54 ` Carlos O'Donell 2013-01-21 0:54 ` Mike Frysinger 2013-01-17 3:22 ` YOSHIFUJI Hideaki 2013-01-18 4:13 ` Carlos O'Donell 2013-01-16 21:45 ` David Miller 2013-01-17 1:58 ` Carlos O'Donell 2013-01-17 2:05 ` David Miller 2013-01-17 10:57 ` Jan Engelhardt 2013-01-18 4:14 ` Mike Frysinger 2013-01-18 4:55 ` David Miller 2013-01-18 5:27 ` Mike Frysinger 2013-03-13 15:17 ` [libvirt] if_bridge.h: include in6.h for struct in6_addr use Kumar Gala 2013-03-13 16:24 ` Eric Blake
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).