| Message ID | 1369209176-6841-1-git-send-email-amwang@redhat.com |
|---|---|
| State | Accepted, archived |
| Delegated to: | David Miller |
| Headers | show |
On Wed, May 22, 2013 at 03:52:54PM +0800, Cong Wang wrote: > From: Cong Wang <amwang@redhat.com> > > Quote from Adam: > "If it is believed that the use of 0.0.0.0 > as the IP address is what is causing strange behaviour on other devices > then is there a good reason that a bridge rather than a router shouldn't > be the active querier? If not then using the bridge IP address and > having the querier enabled by default may be a reasonable solution > (provided that our querier obeys the election rules and shuts up if it > sees a query from a lower IP address that isn't 0.0.0.0). Just because a > device is the elected querier for IGMP doesn't appear to mean it is > required to perform any other routing functions." > > And introduce a new troggle for it, as suggested by Herbert. > > Suggested-by: Adam Baker <linux@baker-net.org.uk> > Cc: Herbert Xu <herbert@gondor.apana.org.au> > Cc: Stephen Hemminger <stephen@networkplumber.org> > Cc: "David S. Miller" <davem@davemloft.net> > Cc: Adam Baker <linux@baker-net.org.uk> > Signed-off-by: Cong Wang <amwang@redhat.com> Acked-by: Herbert Xu <herbert@gondor.apana.org.au> to all three patches. Thanks!
On 22/05/13 08:52, Cong Wang wrote: > From: Cong Wang<amwang@redhat.com> > > Quote from Adam: > "If it is believed that the use of 0.0.0.0 > as the IP address is what is causing strange behaviour on other devices > then is there a good reason that a bridge rather than a router shouldn't > be the active querier? If not then using the bridge IP address and > having the querier enabled by default may be a reasonable solution > (provided that our querier obeys the election rules and shuts up if it > sees a query from a lower IP address that isn't 0.0.0.0). Just because a > device is the elected querier for IGMP doesn't appear to mean it is > required to perform any other routing functions." > > And introduce a new troggle for it, as suggested by Herbert. I've now tested this series applied to a 3.9.4 kernel Using wireshark I can see that if the multicast_querier and multicast_query_use_ifaddr flags are set then queries do get the correct IP address in them and if multicast_querier is set and multicast_query_use_ifaddr isn't we get queries with the address set to 0.0.0.0 I next tested with 2 bridges configured on different nodes (this is my normal network configuration with the 2 bridge devices acting as wireless routers with different coverage areas with a wired network between them). If multicast_query_use_ifaddr is set whichever device starts querying first will act as the querier and the other will shut up. According to RFC 2236 it should be the device with the lower IP address that ends up as the querier in that scenario but I can't imagine a situation where that exact behaviour matters If multicast_query_use_ifaddr is not set but multicast_querier is then both bridges end up generating queries with a source address of 0.0.0.0. Whilst this results in a small amount of unnecessary network traffic it does provide a functional setup. In all of these cases I also verified that multicast UPnP AV applications on different network segments remain able to talk to each other. I would therefore suggest that making multicast_query_use_ifaddr the default and making the querier only shut up if it sees a query from a lower non zero address rather than any non zero address would constitute minor improvements to this patch but as it stands it is still an improvement on the current behaviour. Tested-By: Adam Baker <linux@baker-net.org.uk> -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/net/bridge/br_multicast.c b/net/bridge/br_multicast.c index 81f2389..2475147 100644 --- a/net/bridge/br_multicast.c +++ b/net/bridge/br_multicast.c @@ -23,6 +23,7 @@ #include <linux/skbuff.h> #include <linux/slab.h> #include <linux/timer.h> +#include <linux/inetdevice.h> #include <net/ip.h> #if IS_ENABLED(CONFIG_IPV6) #include <net/ipv6.h> @@ -381,7 +382,8 @@ static struct sk_buff *br_ip4_multicast_alloc_query(struct net_bridge *br, iph->frag_off = htons(IP_DF); iph->ttl = 1; iph->protocol = IPPROTO_IGMP; - iph->saddr = 0; + iph->saddr = br->multicast_query_use_ifaddr ? + inet_select_addr(br->dev, 0, RT_SCOPE_LINK) : 0; iph->daddr = htonl(INADDR_ALLHOSTS_GROUP); ((u8 *)&iph[1])[0] = IPOPT_RA; ((u8 *)&iph[1])[1] = 4; @@ -1618,6 +1620,7 @@ void br_multicast_init(struct net_bridge *br) br->multicast_router = 1; br->multicast_querier = 0; + br->multicast_query_use_ifaddr = 0; br->multicast_last_member_count = 2; br->multicast_startup_query_count = 2; diff --git a/net/bridge/br_private.h b/net/bridge/br_private.h index d2c043a..e260710 100644 --- a/net/bridge/br_private.h +++ b/net/bridge/br_private.h @@ -249,6 +249,7 @@ struct net_bridge u8 multicast_disabled:1; u8 multicast_querier:1; + u8 multicast_query_use_ifaddr:1; u32 hash_elasticity; u32 hash_max; diff --git a/net/bridge/br_sysfs_br.c b/net/bridge/br_sysfs_br.c index 8baa9c0..394bb96 100644 --- a/net/bridge/br_sysfs_br.c +++ b/net/bridge/br_sysfs_br.c @@ -375,6 +375,31 @@ static ssize_t store_multicast_snooping(struct device *d, static DEVICE_ATTR(multicast_snooping, S_IRUGO | S_IWUSR, show_multicast_snooping, store_multicast_snooping); +static ssize_t show_multicast_query_use_ifaddr(struct device *d, + struct device_attribute *attr, + char *buf) +{ + struct net_bridge *br = to_bridge(d); + return sprintf(buf, "%d\n", br->multicast_query_use_ifaddr); +} + +static int set_query_use_ifaddr(struct net_bridge *br, unsigned long val) +{ + br->multicast_query_use_ifaddr = !!val; + return 0; +} + +static ssize_t +store_multicast_query_use_ifaddr(struct device *d, + struct device_attribute *attr, + const char *buf, size_t len) +{ + return store_bridge_parm(d, buf, len, set_query_use_ifaddr); +} +static DEVICE_ATTR(multicast_query_use_ifaddr, S_IRUGO | S_IWUSR, + show_multicast_query_use_ifaddr, + store_multicast_query_use_ifaddr); + static ssize_t show_multicast_querier(struct device *d, struct device_attribute *attr, char *buf) @@ -734,6 +759,7 @@ static struct attribute *bridge_attrs[] = { &dev_attr_multicast_router.attr, &dev_attr_multicast_snooping.attr, &dev_attr_multicast_querier.attr, + &dev_attr_multicast_query_use_ifaddr.attr, &dev_attr_hash_elasticity.attr, &dev_attr_hash_max.attr, &dev_attr_multicast_last_member_count.attr,