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

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.

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.

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>

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