netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [Patch net-next v3 1/3] bridge: select IP addr as source addr for querier
@ 2013-04-30  7:10 Cong Wang
  2013-04-30  7:10 ` [Patch net-next v3 2/3] bridge: only expire the mdb entry when query is received Cong Wang
  2013-04-30  7:10 ` [Patch net-next v3 3/3] bridge: send query as soon as leave " Cong Wang
  0 siblings, 2 replies; 12+ messages in thread
From: Cong Wang @ 2013-04-30  7:10 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] 12+ messages in thread

* [Patch net-next v3 2/3] bridge: only expire the mdb entry when query is received
  2013-04-30  7:10 [Patch net-next v3 1/3] bridge: select IP addr as source addr for querier Cong Wang
@ 2013-04-30  7:10 ` Cong Wang
  2013-05-02  1:59   ` Herbert Xu
  2013-04-30  7:10 ` [Patch net-next v3 3/3] bridge: send query as soon as leave " Cong Wang
  1 sibling, 1 reply; 12+ messages in thread
From: Cong Wang @ 2013-04-30  7:10 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>
---
v3: fix the code in br_multicast_leave_group()
v2: add a flag to check if timer is fired or not

 net/bridge/br_multicast.c |   21 ++++++++++++---------
 net/bridge/br_private.h   |    1 +
 2 files changed, 13 insertions(+), 9 deletions(-)

diff --git a/net/bridge/br_multicast.c b/net/bridge/br_multicast.c
index c52d4f3..6a05522 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,7 +1265,7 @@ 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);
 		}
@@ -1275,7 +1277,7 @@ static void br_multicast_leave_group(struct net_bridge *br,
 		     br->multicast_last_member_interval;
 
 	if (!port) {
-		if (mp->mglist &&
+		if (mp->mglist && mp->timer_armed &&
 		    (timer_pending(&mp->timer) ?
 		     time_after(mp->timer.expires, time) :
 		     try_to_del_timer_sync(&mp->timer) >= 0)) {
@@ -1674,6 +1676,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] 12+ messages in thread

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

From: Cong Wang <amwang@redhat.com>

Send a non-general query 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>
---
v3: use non-general query
v2: only send query to the port received leave

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

diff --git a/net/bridge/br_multicast.c b/net/bridge/br_multicast.c
index 6a05522..d73276b 100644
--- a/net/bridge/br_multicast.c
+++ b/net/bridge/br_multicast.c
@@ -1250,6 +1250,21 @@ static void br_multicast_leave_group(struct net_bridge *br,
 	if (!mp)
 		goto out;
 
+	if (br->multicast_querier &&
+	    !timer_pending(&br->multicast_querier_timer)) {
+		u32 sent;
+
+		__br_multicast_send_query(br, port, &mp->addr);
+		sent = port ? port->multicast_startup_queries_sent :
+			      br->multicast_startup_queries_sent;
+		time = jiffies;
+		time += sent < br->multicast_startup_query_count ?
+			br->multicast_startup_query_interval :
+			br->multicast_query_interval;
+		mod_timer(port ? &port->multicast_query_timer :
+				 &br->multicast_query_timer, time);
+	}
+
 	if (port && (port->flags & BR_MULTICAST_FAST_LEAVE)) {
 		struct net_bridge_port_group __rcu **pp;
 

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

* Re: [Patch net-next v3 2/3] bridge: only expire the mdb entry when query is received
  2013-04-30  7:10 ` [Patch net-next v3 2/3] bridge: only expire the mdb entry when query is received Cong Wang
@ 2013-05-02  1:59   ` Herbert Xu
  2013-05-03  3:50     ` Cong Wang
  0 siblings, 1 reply; 12+ messages in thread
From: Herbert Xu @ 2013-05-02  1:59 UTC (permalink / raw)
  To: Cong Wang; +Cc: netdev, Stephen Hemminger, David S. Miller, Adam Baker

On Tue, Apr 30, 2013 at 03:10:18PM +0800, Cong Wang wrote:
>
> @@ -1275,7 +1277,7 @@ static void br_multicast_leave_group(struct net_bridge *br,
>  		     br->multicast_last_member_interval;
>  
>  	if (!port) {
> -		if (mp->mglist &&
> +		if (mp->mglist && mp->timer_armed &&
>  		    (timer_pending(&mp->timer) ?
>  		     time_after(mp->timer.expires, time) :
>  		     try_to_del_timer_sync(&mp->timer) >= 0)) {

This whole section of code should now be removed since we're now
only arming the timer in response to a query to that group.  Of
course if we were the querier and sent out a query here then you'd
need to simulate the reception of a query.

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] 12+ messages in thread

* Re: [Patch net-next v3 2/3] bridge: only expire the mdb entry when query is received
  2013-05-02  1:59   ` Herbert Xu
@ 2013-05-03  3:50     ` Cong Wang
  2013-05-03  4:01       ` Herbert Xu
  0 siblings, 1 reply; 12+ messages in thread
From: Cong Wang @ 2013-05-03  3:50 UTC (permalink / raw)
  To: Herbert Xu; +Cc: netdev, Stephen Hemminger, David S. Miller, Adam Baker

On Thu, 2013-05-02 at 09:59 +0800, Herbert Xu wrote:
> On Tue, Apr 30, 2013 at 03:10:18PM +0800, Cong Wang wrote:
> >
> > @@ -1275,7 +1277,7 @@ static void br_multicast_leave_group(struct net_bridge *br,
> >  		     br->multicast_last_member_interval;
> >  
> >  	if (!port) {
> > -		if (mp->mglist &&
> > +		if (mp->mglist && mp->timer_armed &&
> >  		    (timer_pending(&mp->timer) ?
> >  		     time_after(mp->timer.expires, time) :
> >  		     try_to_del_timer_sync(&mp->timer) >= 0)) {
> 
> This whole section of code should now be removed since we're now
> only arming the timer in response to a query to that group.  Of
> course if we were the querier and sent out a query here then you'd
> need to simulate the reception of a query.

Hmm, this piece of code is checking if bridge itself is in the mdb entry
and if it just receives a leave, which seems still necessary... What am
I missing?

Thanks!

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

* Re: [Patch net-next v3 2/3] bridge: only expire the mdb entry when query is received
  2013-05-03  3:50     ` Cong Wang
@ 2013-05-03  4:01       ` Herbert Xu
  2013-05-03  7:08         ` Cong Wang
  0 siblings, 1 reply; 12+ messages in thread
From: Herbert Xu @ 2013-05-03  4:01 UTC (permalink / raw)
  To: Cong Wang; +Cc: netdev, Stephen Hemminger, David S. Miller, Adam Baker

On Fri, May 03, 2013 at 11:50:22AM +0800, Cong Wang wrote:
> On Thu, 2013-05-02 at 09:59 +0800, Herbert Xu wrote:
> > On Tue, Apr 30, 2013 at 03:10:18PM +0800, Cong Wang wrote:
> > >
> > > @@ -1275,7 +1277,7 @@ static void br_multicast_leave_group(struct net_bridge *br,
> > >  		     br->multicast_last_member_interval;
> > >  
> > >  	if (!port) {
> > > -		if (mp->mglist &&
> > > +		if (mp->mglist && mp->timer_armed &&
> > >  		    (timer_pending(&mp->timer) ?
> > >  		     time_after(mp->timer.expires, time) :
> > >  		     try_to_del_timer_sync(&mp->timer) >= 0)) {
> > 
> > This whole section of code should now be removed since we're now
> > only arming the timer in response to a query to that group.  Of
> > course if we were the querier and sent out a query here then you'd
> > need to simulate the reception of a query.
> 
> Hmm, this piece of code is checking if bridge itself is in the mdb entry
> and if it just receives a leave, which seems still necessary... What am
> I missing?

I think I quoted the wrong hunk in the patch, I meant the code
that arms the timer should no longer be in the leave_group function
unless we just sent a query ourselves (and in that case the expiration
should also be adjusted accordingly).

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] 12+ messages in thread

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

On Fri, 2013-05-03 at 12:01 +0800, Herbert Xu wrote:
> 
> I think I quoted the wrong hunk in the patch, I meant the code
> that arms the timer should no longer be in the leave_group function
> unless we just sent a query ourselves (and in that case the expiration
> should also be adjusted accordingly).

Is the following patch what you meant?

diff --git a/net/bridge/br_multicast.c b/net/bridge/br_multicast.c
index d73276b..21fcc42 100644
--- a/net/bridge/br_multicast.c
+++ b/net/bridge/br_multicast.c
@@ -1302,22 +1302,6 @@ static void br_multicast_leave_group(struct
net_bridge *br,
                goto out;
        }
 
-       for (p = mlock_dereference(mp->ports, br);
-            p != NULL;
-            p = mlock_dereference(p->next, br)) {
-               if (p->port != port)
-                       continue;
-
-               if (!hlist_unhashed(&p->mglist) &&
-                   (timer_pending(&p->timer) ?
-                    time_after(p->timer.expires, time) :
-                    try_to_del_timer_sync(&p->timer) >= 0)) {
-                       mod_timer(&p->timer, time);
-               }
-
-               break;
-       }
-
 out:
        spin_unlock(&br->multicast_lock);
 }

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

* Re: [Patch net-next v3 2/3] bridge: only expire the mdb entry when query is received
  2013-05-03  7:08         ` Cong Wang
@ 2013-05-03  9:30           ` Herbert Xu
  2013-05-06  3:24             ` Cong Wang
  0 siblings, 1 reply; 12+ messages in thread
From: Herbert Xu @ 2013-05-03  9:30 UTC (permalink / raw)
  To: Cong Wang; +Cc: netdev, Stephen Hemminger, David S. Miller, Adam Baker

On Fri, May 03, 2013 at 03:08:46PM +0800, Cong Wang wrote:
> On Fri, 2013-05-03 at 12:01 +0800, Herbert Xu wrote:
> > 
> > I think I quoted the wrong hunk in the patch, I meant the code
> > that arms the timer should no longer be in the leave_group function
> > unless we just sent a query ourselves (and in that case the expiration
> > should also be adjusted accordingly).
> 
> Is the following patch what you meant?

Right, but also add the corresponding timers in case we send a
group-specific query since that won't loop back to ourselves.

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] 12+ messages in thread

* Re: [Patch net-next v3 2/3] bridge: only expire the mdb entry when query is received
  2013-05-03  9:30           ` Herbert Xu
@ 2013-05-06  3:24             ` Cong Wang
  2013-05-09  5:15               ` Herbert Xu
  0 siblings, 1 reply; 12+ messages in thread
From: Cong Wang @ 2013-05-06  3:24 UTC (permalink / raw)
  To: Herbert Xu; +Cc: netdev, Stephen Hemminger, David S. Miller, Adam Baker

On Fri, 2013-05-03 at 17:30 +0800, Herbert Xu wrote:
> On Fri, May 03, 2013 at 03:08:46PM +0800, Cong Wang wrote:
> > On Fri, 2013-05-03 at 12:01 +0800, Herbert Xu wrote:
> > > 
> > > I think I quoted the wrong hunk in the patch, I meant the code
> > > that arms the timer should no longer be in the leave_group function
> > > unless we just sent a query ourselves (and in that case the expiration
> > > should also be adjusted accordingly).
> > 
> > Is the following patch what you meant?
> 
> Right, but also add the corresponding timers in case we send a
> group-specific query since that won't loop back to ourselves.

Ok, then something like below , it is a delta patch, the first piece
should go to patch 3/3.

diff --git a/net/bridge/br_multicast.c b/net/bridge/br_multicast.c
index d73276b..11946a4 100644
--- a/net/bridge/br_multicast.c
+++ b/net/bridge/br_multicast.c
@@ -1263,6 +1263,24 @@ static void br_multicast_leave_group(struct
net_bridge *br,
 			br->multicast_query_interval;
 		mod_timer(port ? &port->multicast_query_timer :
 				 &br->multicast_query_timer, time);
+
+		time = jiffies + br->multicast_last_member_count *
+				 br->multicast_last_member_interval;
+		for (p = mlock_dereference(mp->ports, br);
+		     p != NULL;
+		     p = mlock_dereference(p->next, br)) {
+			if (p->port != port)
+				continue;
+
+			if (!hlist_unhashed(&p->mglist) &&
+			    (timer_pending(&p->timer) ?
+			     time_after(p->timer.expires, time) :
+			     try_to_del_timer_sync(&p->timer) >= 0)) {
+				mod_timer(&p->timer, time);
+			}
+
+			break;
+		}
 	}
 
 	if (port && (port->flags & BR_MULTICAST_FAST_LEAVE)) {
@@ -1298,24 +1316,6 @@ static void br_multicast_leave_group(struct
net_bridge *br,
 		     try_to_del_timer_sync(&mp->timer) >= 0)) {
 			mod_timer(&mp->timer, time);
 		}
-
-		goto out;
-	}
-
-	for (p = mlock_dereference(mp->ports, br);
-	     p != NULL;
-	     p = mlock_dereference(p->next, br)) {
-		if (p->port != port)
-			continue;
-
-		if (!hlist_unhashed(&p->mglist) &&
-		    (timer_pending(&p->timer) ?
-		     time_after(p->timer.expires, time) :
-		     try_to_del_timer_sync(&p->timer) >= 0)) {
-			mod_timer(&p->timer, time);
-		}
-
-		break;
 	}
 
 out:

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

* Re: [Patch net-next v3 2/3] bridge: only expire the mdb entry when query is received
  2013-05-06  3:24             ` Cong Wang
@ 2013-05-09  5:15               ` Herbert Xu
  2013-05-09 12:53                 ` Cong Wang
  0 siblings, 1 reply; 12+ messages in thread
From: Herbert Xu @ 2013-05-09  5:15 UTC (permalink / raw)
  To: Cong Wang; +Cc: netdev, Stephen Hemminger, David S. Miller, Adam Baker

On Mon, May 06, 2013 at 11:24:13AM +0800, Cong Wang wrote:
> On Fri, 2013-05-03 at 17:30 +0800, Herbert Xu wrote:
> > On Fri, May 03, 2013 at 03:08:46PM +0800, Cong Wang wrote:
> > > On Fri, 2013-05-03 at 12:01 +0800, Herbert Xu wrote:
> > > > 
> > > > I think I quoted the wrong hunk in the patch, I meant the code
> > > > that arms the timer should no longer be in the leave_group function
> > > > unless we just sent a query ourselves (and in that case the expiration
> > > > should also be adjusted accordingly).
> > > 
> > > Is the following patch what you meant?
> > 
> > Right, but also add the corresponding timers in case we send a
> > group-specific query since that won't loop back to ourselves.
> 
> Ok, then something like below , it is a delta patch, the first piece
> should go to patch 3/3.

Yes this looks about right.  BTW, for the groups-epcific queries
you should use the leave timeout values and not the startup timeout
values.

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] 12+ messages in thread

* Re: [Patch net-next v3 2/3] bridge: only expire the mdb entry when query is received
  2013-05-09  5:15               ` Herbert Xu
@ 2013-05-09 12:53                 ` Cong Wang
  2013-05-10  3:21                   ` Herbert Xu
  0 siblings, 1 reply; 12+ messages in thread
From: Cong Wang @ 2013-05-09 12:53 UTC (permalink / raw)
  To: Herbert Xu; +Cc: netdev, Stephen Hemminger, David S. Miller, Adam Baker

On Thu, 2013-05-09 at 13:15 +0800, Herbert Xu wrote:
> 
> Yes this looks about right.  BTW, for the groups-epcific queries
> you should use the leave timeout values and not the startup timeout
> values.
> 

Right, something like below:

diff --git a/net/bridge/br_multicast.c b/net/bridge/br_multicast.c
index 11946a4..1e414ce 100644
--- a/net/bridge/br_multicast.c
+++ b/net/bridge/br_multicast.c
@@ -1252,20 +1252,13 @@ static void br_multicast_leave_group(struct
net_bridge *br,
 
        if (br->multicast_querier &&
            !timer_pending(&br->multicast_querier_timer)) {
-               u32 sent;
-
                __br_multicast_send_query(br, port, &mp->addr);
-               sent = port ? port->multicast_startup_queries_sent :
-                             br->multicast_startup_queries_sent;
-               time = jiffies;
-               time += sent < br->multicast_startup_query_count ?
-                       br->multicast_startup_query_interval :
-                       br->multicast_query_interval;
-               mod_timer(port ? &port->multicast_query_timer :
-                                &br->multicast_query_timer, time);
 
                time = jiffies + br->multicast_last_member_count *
                                 br->multicast_last_member_interval;
+               mod_timer(port ? &port->multicast_query_timer :
+                                &br->multicast_query_timer, time);
+
                for (p = mlock_dereference(mp->ports, br);
                     p != NULL;
                     p = mlock_dereference(p->next, br)) {

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

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

On Thu, May 09, 2013 at 08:53:48PM +0800, Cong Wang wrote:
> On Thu, 2013-05-09 at 13:15 +0800, Herbert Xu wrote:
> > 
> > Yes this looks about right.  BTW, for the groups-epcific queries
> > you should use the leave timeout values and not the startup timeout
> > values.
> > 
> 
> Right, something like below:

Yes 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] 12+ messages in thread

end of thread, other threads:[~2013-05-10  3:21 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-04-30  7:10 [Patch net-next v3 1/3] bridge: select IP addr as source addr for querier Cong Wang
2013-04-30  7:10 ` [Patch net-next v3 2/3] bridge: only expire the mdb entry when query is received Cong Wang
2013-05-02  1:59   ` Herbert Xu
2013-05-03  3:50     ` Cong Wang
2013-05-03  4:01       ` Herbert Xu
2013-05-03  7:08         ` Cong Wang
2013-05-03  9:30           ` Herbert Xu
2013-05-06  3:24             ` Cong Wang
2013-05-09  5:15               ` Herbert Xu
2013-05-09 12:53                 ` Cong Wang
2013-05-10  3:21                   ` Herbert Xu
2013-04-30  7:10 ` [Patch net-next v3 3/3] bridge: send query as soon as leave " Cong Wang

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