Message ID | 1367305819-14013-2-git-send-email-amwang@redhat.com |
---|---|
State | Changes Requested, archived |
Delegated to: | David Miller |
Headers | show |
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,
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! -- 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
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,
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