netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [Patch net-next v2 1/3] bridge: select IP addr for source addr of querier
@ 2013-04-30  3:41 Cong Wang
  2013-04-30  3:41 ` [Patch net-next v2 2/3] bridge: only expire the mdb entry when query is received Cong Wang
  2013-04-30  3:41 ` [Patch net-next v2 3/3] bridge: send query as soon as leave " Cong Wang
  0 siblings, 2 replies; 7+ messages in thread
From: Cong Wang @ 2013-04-30  3:41 UTC (permalink / raw)
  To: netdev
  Cc: Herbert Xu, Stephen Hemminger, David S. Miller, Adam Baker, Cong Wang

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, by default we still use
0.0.0.0 as src IP.

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>
---
v2: introduce a new troggle

 net/bridge/br_multicast.c |    5 ++++-
 net/bridge/br_private.h   |    1 +
 net/bridge/br_sysfs_br.c  |   25 +++++++++++++++++++++++++
 3 files changed, 30 insertions(+), 1 deletions(-)

diff --git a/net/bridge/br_multicast.c b/net/bridge/br_multicast.c
index 81f2389..c52d4f3 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_zero_source ?
+		     0 : inet_select_addr(br->dev, 0, RT_SCOPE_LINK);
 	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_zero_source = 1;
 	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..695033f 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_zero_source: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..ebdfafb 100644
--- a/net/bridge/br_sysfs_br.c
+++ b/net/bridge/br_sysfs_br.c
@@ -375,6 +375,30 @@ 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_zero_source(struct device *d,
+				      struct device_attribute *attr,
+				      char *buf)
+{
+	struct net_bridge *br = to_bridge(d);
+	return sprintf(buf, "%d\n", br->multicast_query_zero_source);
+}
+
+static int set_query_zero_source(struct net_bridge *br, unsigned long val)
+{
+	br->multicast_query_zero_source = !!val;
+	return 0;
+}
+
+static ssize_t store_multicast_query_zero_source(struct device *d,
+				       struct device_attribute *attr,
+				       const char *buf, size_t len)
+{
+	return store_bridge_parm(d, buf, len, set_query_zero_source);
+}
+static DEVICE_ATTR(multicast_query_zero_source, S_IRUGO | S_IWUSR,
+		   show_multicast_query_zero_source,
+		   store_multicast_query_zero_source);
+
 static ssize_t show_multicast_querier(struct device *d,
 				      struct device_attribute *attr,
 				      char *buf)
@@ -734,6 +758,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_zero_source.attr,
 	&dev_attr_hash_elasticity.attr,
 	&dev_attr_hash_max.attr,
 	&dev_attr_multicast_last_member_count.attr,
-- 
1.7.7.6

^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [Patch net-next v2 2/3] bridge: only expire the mdb entry when query is received
  2013-04-30  3:41 [Patch net-next v2 1/3] bridge: select IP addr for source addr of querier Cong Wang
@ 2013-04-30  3:41 ` Cong Wang
  2013-04-30  4:09   ` Cong Wang
  2013-04-30  3:41 ` [Patch net-next v2 3/3] bridge: send query as soon as leave " Cong Wang
  1 sibling, 1 reply; 7+ messages in thread
From: Cong Wang @ 2013-04-30  3:41 UTC (permalink / raw)
  To: netdev
  Cc: Herbert Xu, Stephen Hemminger, David S. Miller, Adam Baker, Cong Wang

From: Cong Wang <amwang@redhat.com>

Currently we arm the expire timer when the mdb entry is added,
however, this causes problem when there is no querier sent
out after that.

So we should only arm the timer when a corresponding query is
received, as suggested by Herbert.

And "if there is no querier then group subscriptions shouldn't expire.
There has to be at least one querier in the network for this thing to
work.  Otherwise it just degenerates into a non-snooping switch,
which is OK."

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>
---
v2: add a flag to check if timer is fired or not

 net/bridge/br_multicast.c |   22 ++++++++++++++--------
 net/bridge/br_private.h   |    1 +
 2 files changed, 15 insertions(+), 8 deletions(-)

diff --git a/net/bridge/br_multicast.c b/net/bridge/br_multicast.c
index c52d4f3..bab3d5a 100644
--- a/net/bridge/br_multicast.c
+++ b/net/bridge/br_multicast.c
@@ -617,8 +617,6 @@ rehash:
 
 	mp->br = br;
 	mp->addr = *group;
-	setup_timer(&mp->timer, br_multicast_group_expired,
-		    (unsigned long)mp);
 
 	hlist_add_head_rcu(&mp->hlist[mdb->ver], &mdb->mhash[hash]);
 	mdb->size++;
@@ -656,7 +654,6 @@ static int br_multicast_add_group(struct net_bridge *br,
 	struct net_bridge_mdb_entry *mp;
 	struct net_bridge_port_group *p;
 	struct net_bridge_port_group __rcu **pp;
-	unsigned long now = jiffies;
 	int err;
 
 	spin_lock(&br->multicast_lock);
@@ -671,7 +668,6 @@ static int br_multicast_add_group(struct net_bridge *br,
 
 	if (!port) {
 		mp->mglist = true;
-		mod_timer(&mp->timer, now + br->multicast_membership_interval);
 		goto out;
 	}
 
@@ -679,7 +675,7 @@ static int br_multicast_add_group(struct net_bridge *br,
 	     (p = mlock_dereference(*pp, br)) != NULL;
 	     pp = &p->next) {
 		if (p->port == port)
-			goto found;
+			goto out;
 		if ((unsigned long)p->port < (unsigned long)port)
 			break;
 	}
@@ -690,8 +686,6 @@ static int br_multicast_add_group(struct net_bridge *br,
 	rcu_assign_pointer(*pp, p);
 	br_mdb_notify(br->dev, port, group, RTM_NEWMDB);
 
-found:
-	mod_timer(&p->timer, now + br->multicast_membership_interval);
 out:
 	err = 0;
 
@@ -1131,6 +1125,10 @@ static int br_ip4_multicast_query(struct net_bridge *br,
 	if (!mp)
 		goto out;
 
+	setup_timer(&mp->timer, br_multicast_group_expired, (unsigned long)mp);
+	mod_timer(&mp->timer, now + br->multicast_membership_interval);
+	mp->timer_armed = true;
+
 	max_delay *= br->multicast_last_member_count;
 
 	if (mp->mglist &&
@@ -1205,6 +1203,10 @@ static int br_ip6_multicast_query(struct net_bridge *br,
 	if (!mp)
 		goto out;
 
+	setup_timer(&mp->timer, br_multicast_group_expired, (unsigned long)mp);
+	mod_timer(&mp->timer, now + br->multicast_membership_interval);
+	mp->timer_armed = true;
+
 	max_delay *= br->multicast_last_member_count;
 	if (mp->mglist &&
 	    (timer_pending(&mp->timer) ?
@@ -1263,13 +1265,16 @@ static void br_multicast_leave_group(struct net_bridge *br,
 			call_rcu_bh(&p->rcu, br_multicast_free_pg);
 			br_mdb_notify(br->dev, port, group, RTM_DELMDB);
 
-			if (!mp->ports && !mp->mglist &&
+			if (!mp->ports && !mp->mglist && mp->timer_armed &&
 			    netif_running(br->dev))
 				mod_timer(&mp->timer, jiffies);
 		}
 		goto out;
 	}
 
+	if (!mp->timer_armed)
+		goto out;
+
 	now = jiffies;
 	time = now + br->multicast_last_member_count *
 		     br->multicast_last_member_interval;
@@ -1674,6 +1679,7 @@ void br_multicast_stop(struct net_bridge *br)
 		hlist_for_each_entry_safe(mp, n, &mdb->mhash[i],
 					  hlist[ver]) {
 			del_timer(&mp->timer);
+			mp->timer_armed = false;
 			call_rcu_bh(&mp->rcu, br_multicast_free_group);
 		}
 	}
diff --git a/net/bridge/br_private.h b/net/bridge/br_private.h
index 695033f..cd2552c 100644
--- a/net/bridge/br_private.h
+++ b/net/bridge/br_private.h
@@ -112,6 +112,7 @@ struct net_bridge_mdb_entry
 	struct timer_list		timer;
 	struct br_ip			addr;
 	bool				mglist;
+	bool				timer_armed;
 };
 
 struct net_bridge_mdb_htable
-- 
1.7.7.6

^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [Patch net-next v2 3/3] bridge: send query as soon as leave is received
  2013-04-30  3:41 [Patch net-next v2 1/3] bridge: select IP addr for source addr of querier Cong Wang
  2013-04-30  3:41 ` [Patch net-next v2 2/3] bridge: only expire the mdb entry when query is received Cong Wang
@ 2013-04-30  3:41 ` Cong Wang
  2013-04-30  3:45   ` Herbert Xu
  1 sibling, 1 reply; 7+ messages in thread
From: Cong Wang @ 2013-04-30  3:41 UTC (permalink / raw)
  To: netdev
  Cc: Herbert Xu, Stephen Hemminger, David S. Miller, Adam Baker, Cong Wang

From: Cong Wang <amwang@redhat.com>

Continue sending queries when leave is received if the user marks
it as a querier.

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>
---
v2: only send query to the port received leave

 net/bridge/br_multicast.c |    7 +++++++
 1 files changed, 7 insertions(+), 0 deletions(-)

diff --git a/net/bridge/br_multicast.c b/net/bridge/br_multicast.c
index bab3d5a..b1879c8 100644
--- a/net/bridge/br_multicast.c
+++ b/net/bridge/br_multicast.c
@@ -1250,6 +1250,13 @@ static void br_multicast_leave_group(struct net_bridge *br,
 	if (!mp)
 		goto out;
 
+	if (br->multicast_querier) {
+		if (port)
+			__br_multicast_enable_port(port);
+		else
+			mod_timer(&br->multicast_query_timer, jiffies);
+	}
+
 	if (port && (port->flags & BR_MULTICAST_FAST_LEAVE)) {
 		struct net_bridge_port_group __rcu **pp;
 
-- 
1.7.7.6

^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [Patch net-next v2 3/3] bridge: send query as soon as leave is received
  2013-04-30  3:41 ` [Patch net-next v2 3/3] bridge: send query as soon as leave " Cong Wang
@ 2013-04-30  3:45   ` Herbert Xu
  2013-04-30  4:05     ` Cong Wang
  0 siblings, 1 reply; 7+ messages in thread
From: Herbert Xu @ 2013-04-30  3:45 UTC (permalink / raw)
  To: Cong Wang; +Cc: netdev, Stephen Hemminger, David S. Miller, Adam Baker

On Tue, Apr 30, 2013 at 11:41:10AM +0800, Cong Wang wrote:
> From: Cong Wang <amwang@redhat.com>
> 
> Continue sending queries when leave is received if the user marks
> it as a querier.
> 
> 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>
> ---
> v2: only send query to the port received leave
> 
>  net/bridge/br_multicast.c |    7 +++++++
>  1 files changed, 7 insertions(+), 0 deletions(-)
> 
> diff --git a/net/bridge/br_multicast.c b/net/bridge/br_multicast.c
> index bab3d5a..b1879c8 100644
> --- a/net/bridge/br_multicast.c
> +++ b/net/bridge/br_multicast.c
> @@ -1250,6 +1250,13 @@ static void br_multicast_leave_group(struct net_bridge *br,
>  	if (!mp)
>  		goto out;
>  
> +	if (br->multicast_querier) {
> +		if (port)
> +			__br_multicast_enable_port(port);
> +		else
> +			mod_timer(&br->multicast_query_timer, jiffies);
> +	}

AFAICS this is still sending a general query as opposed to a
group-specific query, no?

Thanks,
-- 
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [Patch net-next v2 3/3] bridge: send query as soon as leave is received
  2013-04-30  3:45   ` Herbert Xu
@ 2013-04-30  4:05     ` Cong Wang
  2013-04-30  4:07       ` Herbert Xu
  0 siblings, 1 reply; 7+ messages in thread
From: Cong Wang @ 2013-04-30  4:05 UTC (permalink / raw)
  To: Herbert Xu; +Cc: netdev, Stephen Hemminger, David S. Miller, Adam Baker

On Tue, 2013-04-30 at 11:45 +0800, Herbert Xu wrote:
> On Tue, Apr 30, 2013 at 11:41:10AM +0800, Cong Wang wrote:
> > From: Cong Wang <amwang@redhat.com>
> > 
> > Continue sending queries when leave is received if the user marks
> > it as a querier.
> > 
> > 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>
> > ---
> > v2: only send query to the port received leave
> > 
> >  net/bridge/br_multicast.c |    7 +++++++
> >  1 files changed, 7 insertions(+), 0 deletions(-)
> > 
> > diff --git a/net/bridge/br_multicast.c b/net/bridge/br_multicast.c
> > index bab3d5a..b1879c8 100644
> > --- a/net/bridge/br_multicast.c
> > +++ b/net/bridge/br_multicast.c
> > @@ -1250,6 +1250,13 @@ static void br_multicast_leave_group(struct net_bridge *br,
> >  	if (!mp)
> >  		goto out;
> >  
> > +	if (br->multicast_querier) {
> > +		if (port)
> > +			__br_multicast_enable_port(port);
> > +		else
> > +			mod_timer(&br->multicast_query_timer, jiffies);
> > +	}
> 
> AFAICS this is still sending a general query as opposed to a
> group-specific query, no?
> 

When port is NULL, the query should only be sent to bridge itself, so I
think this is what we expect, right? Since we receive leave from bridge
itself.

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [Patch net-next v2 3/3] bridge: send query as soon as leave is received
  2013-04-30  4:05     ` Cong Wang
@ 2013-04-30  4:07       ` Herbert Xu
  0 siblings, 0 replies; 7+ messages in thread
From: Herbert Xu @ 2013-04-30  4:07 UTC (permalink / raw)
  To: Cong Wang; +Cc: netdev, Stephen Hemminger, David S. Miller, Adam Baker

On Tue, Apr 30, 2013 at 12:05:17PM +0800, Cong Wang wrote:
>
> When port is NULL, the query should only be sent to bridge itself, so I
> think this is what we expect, right? Since we receive leave from bridge
> itself.

No I'm not talking about which port it's being sent to (which
obviously should only be the port where the leave arrived on),
but what is in the query message.

A general query queries every single group out there, for a leave
we need to specifically query for the group that is being left.

Cheers,
-- 
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [Patch net-next v2 2/3] bridge: only expire the mdb entry when query is received
  2013-04-30  3:41 ` [Patch net-next v2 2/3] bridge: only expire the mdb entry when query is received Cong Wang
@ 2013-04-30  4:09   ` Cong Wang
  0 siblings, 0 replies; 7+ messages in thread
From: Cong Wang @ 2013-04-30  4:09 UTC (permalink / raw)
  To: netdev; +Cc: Herbert Xu, Stephen Hemminger, David S. Miller, Adam Baker

On Tue, 2013-04-30 at 11:41 +0800, Cong Wang wrote:
> @@ -1263,13 +1265,16 @@ static void br_multicast_leave_group(struct
> net_bridge *br,
...
>  
> +       if (!mp->timer_armed)
> +               goto out;
> + 

The above piece doesn't look right, will fix it in v3.

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2013-04-30  4:09 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-04-30  3:41 [Patch net-next v2 1/3] bridge: select IP addr for source addr of querier Cong Wang
2013-04-30  3:41 ` [Patch net-next v2 2/3] bridge: only expire the mdb entry when query is received Cong Wang
2013-04-30  4:09   ` Cong Wang
2013-04-30  3:41 ` [Patch net-next v2 3/3] bridge: send query as soon as leave " Cong Wang
2013-04-30  3:45   ` Herbert Xu
2013-04-30  4:05     ` Cong Wang
2013-04-30  4:07       ` Herbert Xu

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).