* [PATCH 0/5] bridge RCU patches (rev2)
@ 2010-11-15 16:38 Stephen Hemminger
2010-11-15 16:38 ` [PATCH 1/5] bridge: add RCU annotation to bridge multicast table Stephen Hemminger
` (5 more replies)
0 siblings, 6 replies; 8+ messages in thread
From: Stephen Hemminger @ 2010-11-15 16:38 UTC (permalink / raw)
To: David Miller; +Cc: netdev
^ permalink raw reply [flat|nested] 8+ messages in thread* [PATCH 1/5] bridge: add RCU annotation to bridge multicast table 2010-11-15 16:38 [PATCH 0/5] bridge RCU patches (rev2) Stephen Hemminger @ 2010-11-15 16:38 ` Stephen Hemminger 2010-11-15 16:38 ` [PATCH 2/5] bridge: add proper RCU annotation to should_route_hook Stephen Hemminger ` (4 subsequent siblings) 5 siblings, 0 replies; 8+ messages in thread From: Stephen Hemminger @ 2010-11-15 16:38 UTC (permalink / raw) To: David Miller; +Cc: netdev, Eric Dumazet [-- Attachment #1: bridge-mlock-rcu.patch --] [-- Type: text/plain, Size: 10232 bytes --] From: Eric Dumazet <eric.dumazet@gmail•com> Add modern __rcu annotatations to bridge multicast table. Use newer hlist macros to avoid direct access to hlist internals. Signed-off-by: Eric Dumazet <eric.dumazet@gmail•com> Signed-off-by: Stephen Hemminger <shemminger@vyatta•com> --- v2. Fix hlist_next_rcu call net/bridge/br_forward.c | 4 +- net/bridge/br_multicast.c | 78 ++++++++++++++++++++++++++++++---------------- net/bridge/br_private.h | 6 +-- 3 files changed, 56 insertions(+), 32 deletions(-) --- a/net/bridge/br_multicast.c 2010-11-14 12:36:30.383348571 -0800 +++ b/net/bridge/br_multicast.c 2010-11-14 12:36:37.084167303 -0800 @@ -33,6 +33,9 @@ #include "br_private.h" +#define mlock_dereference(X, br) \ + rcu_dereference_protected(X, lockdep_is_held(&br->multicast_lock)) + #if defined(CONFIG_IPV6) || defined(CONFIG_IPV6_MODULE) static inline int ipv6_is_local_multicast(const struct in6_addr *addr) { @@ -135,7 +138,7 @@ static struct net_bridge_mdb_entry *br_m struct net_bridge_mdb_entry *br_mdb_get(struct net_bridge *br, struct sk_buff *skb) { - struct net_bridge_mdb_htable *mdb = br->mdb; + struct net_bridge_mdb_htable *mdb = rcu_dereference(br->mdb); struct br_ip ip; if (br->multicast_disabled) @@ -235,7 +238,8 @@ static void br_multicast_group_expired(u if (mp->ports) goto out; - mdb = br->mdb; + mdb = mlock_dereference(br->mdb, br); + hlist_del_rcu(&mp->hlist[mdb->ver]); mdb->size--; @@ -249,16 +253,20 @@ out: static void br_multicast_del_pg(struct net_bridge *br, struct net_bridge_port_group *pg) { - struct net_bridge_mdb_htable *mdb = br->mdb; + struct net_bridge_mdb_htable *mdb; struct net_bridge_mdb_entry *mp; struct net_bridge_port_group *p; - struct net_bridge_port_group **pp; + struct net_bridge_port_group __rcu **pp; + + mdb = mlock_dereference(br->mdb, br); mp = br_mdb_ip_get(mdb, &pg->addr); if (WARN_ON(!mp)) return; - for (pp = &mp->ports; (p = *pp); pp = &p->next) { + for (pp = &mp->ports; + (p = mlock_dereference(*pp, br)) != NULL; + pp = &p->next) { if (p != pg) continue; @@ -294,10 +302,10 @@ out: spin_unlock(&br->multicast_lock); } -static int br_mdb_rehash(struct net_bridge_mdb_htable **mdbp, int max, +static int br_mdb_rehash(struct net_bridge_mdb_htable __rcu **mdbp, int max, int elasticity) { - struct net_bridge_mdb_htable *old = *mdbp; + struct net_bridge_mdb_htable *old = rcu_dereference_protected(*mdbp, 1); struct net_bridge_mdb_htable *mdb; int err; @@ -569,7 +577,7 @@ static struct net_bridge_mdb_entry *br_m struct net_bridge *br, struct net_bridge_port *port, struct br_ip *group, int hash) { - struct net_bridge_mdb_htable *mdb = br->mdb; + struct net_bridge_mdb_htable *mdb; struct net_bridge_mdb_entry *mp; struct hlist_node *p; unsigned count = 0; @@ -577,6 +585,7 @@ static struct net_bridge_mdb_entry *br_m int elasticity; int err; + mdb = rcu_dereference_protected(br->mdb, 1); hlist_for_each_entry(mp, p, &mdb->mhash[hash], hlist[mdb->ver]) { count++; if (unlikely(br_ip_equal(group, &mp->addr))) @@ -642,10 +651,11 @@ static struct net_bridge_mdb_entry *br_m struct net_bridge *br, struct net_bridge_port *port, struct br_ip *group) { - struct net_bridge_mdb_htable *mdb = br->mdb; + struct net_bridge_mdb_htable *mdb; struct net_bridge_mdb_entry *mp; int hash; + mdb = rcu_dereference_protected(br->mdb, 1); if (!mdb) { if (br_mdb_rehash(&br->mdb, BR_HASH_SIZE, 0)) return NULL; @@ -660,7 +670,7 @@ static struct net_bridge_mdb_entry *br_m case -EAGAIN: rehash: - mdb = br->mdb; + mdb = rcu_dereference_protected(br->mdb, 1); hash = br_ip_hash(mdb, group); break; @@ -692,7 +702,7 @@ static int br_multicast_add_group(struct { struct net_bridge_mdb_entry *mp; struct net_bridge_port_group *p; - struct net_bridge_port_group **pp; + struct net_bridge_port_group __rcu **pp; unsigned long now = jiffies; int err; @@ -712,7 +722,9 @@ static int br_multicast_add_group(struct goto out; } - for (pp = &mp->ports; (p = *pp); pp = &p->next) { + for (pp = &mp->ports; + (p = mlock_dereference(*pp, br)) != NULL; + pp = &p->next) { if (p->port == port) goto found; if ((unsigned long)p->port < (unsigned long)port) @@ -1106,7 +1118,7 @@ static int br_ip4_multicast_query(struct struct net_bridge_mdb_entry *mp; struct igmpv3_query *ih3; struct net_bridge_port_group *p; - struct net_bridge_port_group **pp; + struct net_bridge_port_group __rcu **pp; unsigned long max_delay; unsigned long now = jiffies; __be32 group; @@ -1145,7 +1157,7 @@ static int br_ip4_multicast_query(struct if (!group) goto out; - mp = br_mdb_ip4_get(br->mdb, group); + mp = br_mdb_ip4_get(mlock_dereference(br->mdb, br), group); if (!mp) goto out; @@ -1157,7 +1169,9 @@ static int br_ip4_multicast_query(struct try_to_del_timer_sync(&mp->timer) >= 0)) mod_timer(&mp->timer, now + max_delay); - for (pp = &mp->ports; (p = *pp); pp = &p->next) { + for (pp = &mp->ports; + (p = mlock_dereference(*pp, br)) != NULL; + pp = &p->next) { if (timer_pending(&p->timer) ? time_after(p->timer.expires, now + max_delay) : try_to_del_timer_sync(&p->timer) >= 0) @@ -1178,7 +1192,8 @@ static int br_ip6_multicast_query(struct struct mld_msg *mld = (struct mld_msg *) icmp6_hdr(skb); struct net_bridge_mdb_entry *mp; struct mld2_query *mld2q; - struct net_bridge_port_group *p, **pp; + struct net_bridge_port_group *p; + struct net_bridge_port_group __rcu **pp; unsigned long max_delay; unsigned long now = jiffies; struct in6_addr *group = NULL; @@ -1214,7 +1229,7 @@ static int br_ip6_multicast_query(struct if (!group) goto out; - mp = br_mdb_ip6_get(br->mdb, group); + mp = br_mdb_ip6_get(mlock_dereference(br->mdb, br), group); if (!mp) goto out; @@ -1225,7 +1240,9 @@ static int br_ip6_multicast_query(struct try_to_del_timer_sync(&mp->timer) >= 0)) mod_timer(&mp->timer, now + max_delay); - for (pp = &mp->ports; (p = *pp); pp = &p->next) { + for (pp = &mp->ports; + (p = mlock_dereference(*pp, br)) != NULL; + pp = &p->next) { if (timer_pending(&p->timer) ? time_after(p->timer.expires, now + max_delay) : try_to_del_timer_sync(&p->timer) >= 0) @@ -1254,7 +1271,7 @@ static void br_multicast_leave_group(str timer_pending(&br->multicast_querier_timer)) goto out; - mdb = br->mdb; + mdb = mlock_dereference(br->mdb, br); mp = br_mdb_ip_get(mdb, group); if (!mp) goto out; @@ -1277,7 +1294,9 @@ static void br_multicast_leave_group(str goto out; } - for (p = mp->ports; p; p = p->next) { + for (p = mlock_dereference(mp->ports, br); + p != NULL; + p = mlock_dereference(p->next, br)) { if (p->port != port) continue; @@ -1625,7 +1644,7 @@ void br_multicast_stop(struct net_bridge del_timer_sync(&br->multicast_query_timer); spin_lock_bh(&br->multicast_lock); - mdb = br->mdb; + mdb = mlock_dereference(br->mdb, br); if (!mdb) goto out; @@ -1729,6 +1748,7 @@ int br_multicast_toggle(struct net_bridg { struct net_bridge_port *port; int err = 0; + struct net_bridge_mdb_htable *mdb; spin_lock(&br->multicast_lock); if (br->multicast_disabled == !val) @@ -1741,15 +1761,16 @@ int br_multicast_toggle(struct net_bridg if (!netif_running(br->dev)) goto unlock; - if (br->mdb) { - if (br->mdb->old) { + mdb = mlock_dereference(br->mdb, br); + if (mdb) { + if (mdb->old) { err = -EEXIST; rollback: br->multicast_disabled = !!val; goto unlock; } - err = br_mdb_rehash(&br->mdb, br->mdb->max, + err = br_mdb_rehash(&br->mdb, mdb->max, br->hash_elasticity); if (err) goto rollback; @@ -1774,6 +1795,7 @@ int br_multicast_set_hash_max(struct net { int err = -ENOENT; u32 old; + struct net_bridge_mdb_htable *mdb; spin_lock(&br->multicast_lock); if (!netif_running(br->dev)) @@ -1782,7 +1804,9 @@ int br_multicast_set_hash_max(struct net err = -EINVAL; if (!is_power_of_2(val)) goto unlock; - if (br->mdb && val < br->mdb->size) + + mdb = mlock_dereference(br->mdb, br); + if (mdb && val < mdb->size) goto unlock; err = 0; @@ -1790,8 +1814,8 @@ int br_multicast_set_hash_max(struct net old = br->hash_max; br->hash_max = val; - if (br->mdb) { - if (br->mdb->old) { + if (mdb) { + if (mdb->old) { err = -EEXIST; rollback: br->hash_max = old; --- a/net/bridge/br_private.h 2010-11-14 12:36:30.399350527 -0800 +++ b/net/bridge/br_private.h 2010-11-14 12:44:07.257410977 -0800 @@ -72,7 +72,7 @@ struct net_bridge_fdb_entry struct net_bridge_port_group { struct net_bridge_port *port; - struct net_bridge_port_group *next; + struct net_bridge_port_group __rcu *next; struct hlist_node mglist; struct rcu_head rcu; struct timer_list timer; @@ -86,7 +86,7 @@ struct net_bridge_mdb_entry struct hlist_node hlist[2]; struct hlist_node mglist; struct net_bridge *br; - struct net_bridge_port_group *ports; + struct net_bridge_port_group __rcu *ports; struct rcu_head rcu; struct timer_list timer; struct timer_list query_timer; @@ -227,7 +227,7 @@ struct net_bridge unsigned long multicast_startup_query_interval; spinlock_t multicast_lock; - struct net_bridge_mdb_htable *mdb; + struct net_bridge_mdb_htable __rcu *mdb; struct hlist_head router_list; struct hlist_head mglist; --- a/net/bridge/br_forward.c 2010-11-14 12:36:47.833478598 -0800 +++ b/net/bridge/br_forward.c 2010-11-14 12:42:22.001208297 -0800 @@ -223,7 +223,7 @@ static void br_multicast_flood(struct ne struct net_bridge_port_group *p; struct hlist_node *rp; - rp = rcu_dereference(br->router_list.first); + rp = rcu_dereference(hlist_first_rcu(&br->router_list)); p = mdst ? rcu_dereference(mdst->ports) : NULL; while (p || rp) { struct net_bridge_port *port, *lport, *rport; @@ -242,7 +242,7 @@ static void br_multicast_flood(struct ne if ((unsigned long)lport >= (unsigned long)port) p = rcu_dereference(p->next); if ((unsigned long)rport >= (unsigned long)port) - rp = rcu_dereference(rp->next); + rp = rcu_dereference(hlist_next_rcu(rp)); } if (!prev) ^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH 2/5] bridge: add proper RCU annotation to should_route_hook 2010-11-15 16:38 [PATCH 0/5] bridge RCU patches (rev2) Stephen Hemminger 2010-11-15 16:38 ` [PATCH 1/5] bridge: add RCU annotation to bridge multicast table Stephen Hemminger @ 2010-11-15 16:38 ` Stephen Hemminger 2010-11-15 16:38 ` [PATCH 3/5] netdev: add rcu annotations to receive handler hook Stephen Hemminger ` (3 subsequent siblings) 5 siblings, 0 replies; 8+ messages in thread From: Stephen Hemminger @ 2010-11-15 16:38 UTC (permalink / raw) To: David Miller; +Cc: netdev, Eric Dumazet [-- Attachment #1: bridge-hook-typedef.patch --] [-- Type: text/plain, Size: 3069 bytes --] From: Eric Dumazet <eric.dumazet@gmail•com> Add br_should_route_hook_t typedef, this is the only way we can get a clean RCU implementation for function pointer. Move route_hook to location where it is used. Signed-off-by: Eric Dumazet <eric.dumazet@gmail•com> Signed-off-by: Stephen Hemminger <shemminger@vyatta•com> --- include/linux/if_bridge.h | 4 +++- net/bridge/br.c | 4 ---- net/bridge/br_input.c | 10 +++++++--- net/bridge/netfilter/ebtable_broute.c | 3 ++- 4 files changed, 12 insertions(+), 9 deletions(-) --- a/net/bridge/br.c 2010-11-14 11:18:54.048692005 -0800 +++ b/net/bridge/br.c 2010-11-14 11:19:47.347027185 -0800 @@ -22,8 +22,6 @@ #include "br_private.h" -int (*br_should_route_hook)(struct sk_buff *skb); - static const struct stp_proto br_stp_proto = { .rcv = br_stp_rcv, }; @@ -102,8 +100,6 @@ static void __exit br_deinit(void) br_fdb_fini(); } -EXPORT_SYMBOL(br_should_route_hook); - module_init(br_init) module_exit(br_deinit) MODULE_LICENSE("GPL"); --- a/net/bridge/br_input.c 2010-11-14 11:18:54.048692005 -0800 +++ b/net/bridge/br_input.c 2010-11-14 11:41:40.558700481 -0800 @@ -21,6 +21,10 @@ /* Bridge group multicast address 802.1d (pg 51). */ const u8 br_group_address[ETH_ALEN] = { 0x01, 0x80, 0xc2, 0x00, 0x00, 0x00 }; +/* Hook for brouter */ +br_should_route_hook_t __rcu *br_should_route_hook __read_mostly; +EXPORT_SYMBOL(br_should_route_hook); + static int br_pass_frame_up(struct sk_buff *skb) { struct net_device *indev, *brdev = BR_INPUT_SKB_CB(skb)->brdev; @@ -139,7 +143,7 @@ struct sk_buff *br_handle_frame(struct s { struct net_bridge_port *p; const unsigned char *dest = eth_hdr(skb)->h_dest; - int (*rhook)(struct sk_buff *skb); + br_should_route_hook_t *rhook; if (unlikely(skb->pkt_type == PACKET_LOOPBACK)) return skb; @@ -173,8 +177,8 @@ forward: switch (p->state) { case BR_STATE_FORWARDING: rhook = rcu_dereference(br_should_route_hook); - if (rhook != NULL) { - if (rhook(skb)) + if (rhook) { + if ((*rhook)(skb)) return skb; dest = eth_hdr(skb)->h_dest; } --- a/include/linux/if_bridge.h 2010-11-14 11:18:54.048692005 -0800 +++ b/include/linux/if_bridge.h 2010-11-14 11:19:47.351028008 -0800 @@ -102,7 +102,9 @@ struct __fdb_entry { #include <linux/netdevice.h> extern void brioctl_set(int (*ioctl_hook)(struct net *, unsigned int, void __user *)); -extern int (*br_should_route_hook)(struct sk_buff *skb); + +typedef int (*br_should_route_hook_t)(struct sk_buff *skb); +extern br_should_route_hook_t __rcu *br_should_route_hook; #endif --- a/net/bridge/netfilter/ebtable_broute.c 2010-11-14 11:20:39.745149494 -0800 +++ b/net/bridge/netfilter/ebtable_broute.c 2010-11-14 11:21:01.020917528 -0800 @@ -87,7 +87,8 @@ static int __init ebtable_broute_init(vo if (ret < 0) return ret; /* see br_input.c */ - rcu_assign_pointer(br_should_route_hook, ebt_broute); + rcu_assign_pointer(br_should_route_hook, + (br_should_route_hook_t *)ebt_broute); return 0; } ^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH 3/5] netdev: add rcu annotations to receive handler hook 2010-11-15 16:38 [PATCH 0/5] bridge RCU patches (rev2) Stephen Hemminger 2010-11-15 16:38 ` [PATCH 1/5] bridge: add RCU annotation to bridge multicast table Stephen Hemminger 2010-11-15 16:38 ` [PATCH 2/5] bridge: add proper RCU annotation to should_route_hook Stephen Hemminger @ 2010-11-15 16:38 ` Stephen Hemminger 2010-11-15 16:38 ` [PATCH 4/5] bridge: fix RCU races with bridge port Stephen Hemminger ` (2 subsequent siblings) 5 siblings, 0 replies; 8+ messages in thread From: Stephen Hemminger @ 2010-11-15 16:38 UTC (permalink / raw) To: David Miller; +Cc: netdev [-- Attachment #1: rx_handler_rcu.patch --] [-- Type: text/plain, Size: 595 bytes --] Suggested by Eric's bridge RCU changes. Signed-off-by: Stephen Hemminger <shemminger@vyatta•com> --- include/linux/netdevice.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) --- a/include/linux/netdevice.h 2010-11-14 11:41:53.224298362 -0800 +++ b/include/linux/netdevice.h 2010-11-14 11:42:42.546359900 -0800 @@ -995,8 +995,8 @@ struct net_device { unsigned int real_num_rx_queues; #endif - rx_handler_func_t *rx_handler; - void *rx_handler_data; + rx_handler_func_t __rcu *rx_handler; + void __rcu *rx_handler_data; struct netdev_queue __rcu *ingress_queue; ^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH 4/5] bridge: fix RCU races with bridge port 2010-11-15 16:38 [PATCH 0/5] bridge RCU patches (rev2) Stephen Hemminger ` (2 preceding siblings ...) 2010-11-15 16:38 ` [PATCH 3/5] netdev: add rcu annotations to receive handler hook Stephen Hemminger @ 2010-11-15 16:38 ` Stephen Hemminger 2010-11-15 16:38 ` [PATCH 5/5] bridge: add RCU annotations to bridge port lookup Stephen Hemminger 2010-11-15 19:13 ` [PATCH 0/5] bridge RCU patches (rev2) David Miller 5 siblings, 0 replies; 8+ messages in thread From: Stephen Hemminger @ 2010-11-15 16:38 UTC (permalink / raw) To: David Miller; +Cc: netdev [-- Attachment #1: br-port-rcu-race.patch --] [-- Type: text/plain, Size: 6605 bytes --] The macro br_port_exists() is not enough protection when only RCU is being used. There is a tiny race where other CPU has cleared port handler hook, but is bridge port flag might still be set. Signed-off-by: Stephen Hemminger <shemminger@vyatta•com> --- net/bridge/br_fdb.c | 15 +++++++++------ net/bridge/br_if.c | 5 +---- net/bridge/br_netfilter.c | 13 +++++++------ net/bridge/br_netlink.c | 10 ++++++---- net/bridge/br_notify.c | 2 +- net/bridge/br_private.h | 16 +++++++++++++--- net/bridge/br_stp_bpdu.c | 8 ++++---- net/bridge/netfilter/ebtables.c | 11 +++++------ 8 files changed, 46 insertions(+), 34 deletions(-) --- a/net/bridge/br_netfilter.c 2010-11-15 08:24:41.696606136 -0800 +++ b/net/bridge/br_netfilter.c 2010-11-15 08:25:50.037644994 -0800 @@ -131,17 +131,18 @@ void br_netfilter_rtable_init(struct net static inline struct rtable *bridge_parent_rtable(const struct net_device *dev) { - if (!br_port_exists(dev)) - return NULL; - return &br_port_get_rcu(dev)->br->fake_rtable; + struct net_bridge_port *port; + + port = br_port_get_rcu(dev); + return port ? &port->br->fake_rtable : NULL; } static inline struct net_device *bridge_parent(const struct net_device *dev) { - if (!br_port_exists(dev)) - return NULL; + struct net_bridge_port *port; - return br_port_get_rcu(dev)->br->dev; + port = br_port_get_rcu(dev); + return port ? port->br->dev : NULL; } static inline struct nf_bridge_info *nf_bridge_alloc(struct sk_buff *skb) --- a/net/bridge/br_stp_bpdu.c 2010-11-15 08:24:41.696606136 -0800 +++ b/net/bridge/br_stp_bpdu.c 2010-11-15 08:25:50.037644994 -0800 @@ -141,10 +141,6 @@ void br_stp_rcv(const struct stp_proto * struct net_bridge *br; const unsigned char *buf; - if (!br_port_exists(dev)) - goto err; - p = br_port_get_rcu(dev); - if (!pskb_may_pull(skb, 4)) goto err; @@ -153,6 +149,10 @@ void br_stp_rcv(const struct stp_proto * if (buf[0] != 0 || buf[1] != 0 || buf[2] != 0) goto err; + p = br_port_get_rcu(dev); + if (!p) + goto err; + br = p->br; spin_lock(&br->lock); --- a/net/bridge/netfilter/ebtables.c 2010-11-15 08:24:41.696606136 -0800 +++ b/net/bridge/netfilter/ebtables.c 2010-11-15 08:25:50.041645485 -0800 @@ -128,6 +128,7 @@ ebt_basic_match(const struct ebt_entry * const struct net_device *in, const struct net_device *out) { const struct ethhdr *h = eth_hdr(skb); + const struct net_bridge_port *p; __be16 ethproto; int verdict, i; @@ -148,13 +149,11 @@ ebt_basic_match(const struct ebt_entry * if (FWINV2(ebt_dev_check(e->out, out), EBT_IOUT)) return 1; /* rcu_read_lock()ed by nf_hook_slow */ - if (in && br_port_exists(in) && - FWINV2(ebt_dev_check(e->logical_in, br_port_get_rcu(in)->br->dev), - EBT_ILOGICALIN)) + if (in && (p = br_port_get_rcu(in)) != NULL && + FWINV2(ebt_dev_check(e->logical_in, p->br->dev), EBT_ILOGICALIN)) return 1; - if (out && br_port_exists(out) && - FWINV2(ebt_dev_check(e->logical_out, br_port_get_rcu(out)->br->dev), - EBT_ILOGICALOUT)) + if (out && (p = br_port_get_rcu(out)) != NULL && + FWINV2(ebt_dev_check(e->logical_out, p->br->dev), EBT_ILOGICALOUT)) return 1; if (e->bitmask & EBT_SOURCEMAC) { --- a/net/bridge/br_fdb.c 2010-11-15 08:20:20.020385965 -0800 +++ b/net/bridge/br_fdb.c 2010-11-15 08:25:50.041645485 -0800 @@ -238,15 +238,18 @@ struct net_bridge_fdb_entry *__br_fdb_ge int br_fdb_test_addr(struct net_device *dev, unsigned char *addr) { struct net_bridge_fdb_entry *fdb; + struct net_bridge_port *port; int ret; - if (!br_port_exists(dev)) - return 0; - rcu_read_lock(); - fdb = __br_fdb_get(br_port_get_rcu(dev)->br, addr); - ret = fdb && fdb->dst->dev != dev && - fdb->dst->state == BR_STATE_FORWARDING; + port = br_port_get_rcu(dev); + if (!port) + ret = 0; + else { + fdb = __br_fdb_get(port->br, addr); + ret = fdb && fdb->dst->dev != dev && + fdb->dst->state == BR_STATE_FORWARDING; + } rcu_read_unlock(); return ret; --- a/net/bridge/br_notify.c 2010-11-15 08:24:41.696606136 -0800 +++ b/net/bridge/br_notify.c 2010-11-15 08:25:50.045645976 -0800 @@ -32,7 +32,7 @@ struct notifier_block br_device_notifier static int br_device_event(struct notifier_block *unused, unsigned long event, void *ptr) { struct net_device *dev = ptr; - struct net_bridge_port *p = br_port_get(dev); + struct net_bridge_port *p; struct net_bridge *br; int err; --- a/net/bridge/br_private.h 2010-11-15 08:24:47.829474102 -0800 +++ b/net/bridge/br_private.h 2010-11-15 08:27:46.747612478 -0800 @@ -151,11 +151,19 @@ struct net_bridge_port #endif }; -#define br_port_get_rcu(dev) \ - ((struct net_bridge_port *) rcu_dereference(dev->rx_handler_data)) -#define br_port_get(dev) ((struct net_bridge_port *) dev->rx_handler_data) #define br_port_exists(dev) (dev->priv_flags & IFF_BRIDGE_PORT) +static inline struct net_bridge_port *br_port_get_rcu(const struct net_device *dev) +{ + struct net_bridge_port *port = rcu_dereference(dev->rx_handler_data); + return br_port_exists(dev) ? port : NULL; +} + +static inline struct net_bridge_port *br_port_get(struct net_device *dev) +{ + return br_port_exists(dev) ? dev->rx_handler_data : NULL; +} + struct br_cpu_netstats { u64 rx_packets; u64 rx_bytes; --- a/net/bridge/br_if.c 2010-11-15 08:20:20.084389475 -0800 +++ b/net/bridge/br_if.c 2010-11-15 08:25:50.049646467 -0800 @@ -475,11 +475,8 @@ int br_del_if(struct net_bridge *br, str { struct net_bridge_port *p; - if (!br_port_exists(dev)) - return -EINVAL; - p = br_port_get(dev); - if (p->br != br) + if (!p || p->br != br) return -EINVAL; del_nbp(p); --- a/net/bridge/br_netlink.c 2010-11-15 08:24:41.696606136 -0800 +++ b/net/bridge/br_netlink.c 2010-11-15 08:25:50.049646467 -0800 @@ -119,11 +119,13 @@ static int br_dump_ifinfo(struct sk_buff idx = 0; for_each_netdev(net, dev) { + struct net_bridge_port *port = br_port_get(dev); + /* not a bridge port */ - if (!br_port_exists(dev) || idx < cb->args[0]) + if (!port || idx < cb->args[0]) goto skip; - if (br_fill_ifinfo(skb, br_port_get(dev), + if (br_fill_ifinfo(skb, port, NETLINK_CB(cb->skb).pid, cb->nlh->nlmsg_seq, RTM_NEWLINK, NLM_F_MULTI) < 0) @@ -169,9 +171,9 @@ static int br_rtm_setlink(struct sk_buff if (!dev) return -ENODEV; - if (!br_port_exists(dev)) - return -EINVAL; p = br_port_get(dev); + if (!p) + return -EINVAL; /* if kernel STP is running, don't allow changes */ if (p->br->stp_enabled == BR_KERNEL_STP) ^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH 5/5] bridge: add RCU annotations to bridge port lookup 2010-11-15 16:38 [PATCH 0/5] bridge RCU patches (rev2) Stephen Hemminger ` (3 preceding siblings ...) 2010-11-15 16:38 ` [PATCH 4/5] bridge: fix RCU races with bridge port Stephen Hemminger @ 2010-11-15 16:38 ` Stephen Hemminger 2010-11-15 19:13 ` [PATCH 0/5] bridge RCU patches (rev2) David Miller 5 siblings, 0 replies; 8+ messages in thread From: Stephen Hemminger @ 2010-11-15 16:38 UTC (permalink / raw) To: David Miller; +Cc: netdev, Eric Dumazet [-- Attachment #1: bridge-port_get_rtnl.patch --] [-- Type: text/plain, Size: 2300 bytes --] From: Eric Dumazet <eric.dumazet@gmail•com> br_port_get() renamed to br_port_get_rtnl() to make clear RTNL is held. Signed-off-by: Eric Dumazet <eric.dumazet@gmail•com> Signed-off-by: Stephen Hemminger <shemminger@vyatta•com> --- net/bridge/br_if.c | 2 +- net/bridge/br_netlink.c | 4 ++-- net/bridge/br_notify.c | 4 ++-- net/bridge/br_private.h | 9 +++++---- 4 files changed, 10 insertions(+), 9 deletions(-) --- a/net/bridge/br_netlink.c 2010-11-15 08:25:50.049646467 -0800 +++ b/net/bridge/br_netlink.c 2010-11-15 08:27:57.772866001 -0800 @@ -119,7 +119,7 @@ static int br_dump_ifinfo(struct sk_buff idx = 0; for_each_netdev(net, dev) { - struct net_bridge_port *port = br_port_get(dev); + struct net_bridge_port *port = br_port_get_rtnl(dev); /* not a bridge port */ if (!port || idx < cb->args[0]) @@ -171,7 +171,7 @@ static int br_rtm_setlink(struct sk_buff if (!dev) return -ENODEV; - p = br_port_get(dev); + p = br_port_get_rtnl(dev); if (!p) return -EINVAL; --- a/net/bridge/br_private.h 2010-11-15 08:27:46.747612478 -0800 +++ b/net/bridge/br_private.h 2010-11-15 08:27:57.776866451 -0800 @@ -159,9 +159,10 @@ static inline struct net_bridge_port *br return br_port_exists(dev) ? port : NULL; } -static inline struct net_bridge_port *br_port_get(struct net_device *dev) +static inline struct net_bridge_port *br_port_get_rtnl(struct net_device *dev) { - return br_port_exists(dev) ? dev->rx_handler_data : NULL; + return br_port_exists(dev) ? + rtnl_dereference(dev->rx_handler_data) : NULL; } struct br_cpu_netstats { --- a/net/bridge/br_if.c 2010-11-15 08:25:50.049646467 -0800 +++ b/net/bridge/br_if.c 2010-11-15 08:27:57.776866451 -0800 @@ -475,7 +475,7 @@ int br_del_if(struct net_bridge *br, str { struct net_bridge_port *p; - p = br_port_get(dev); + p = br_port_get_rtnl(dev); if (!p || p->br != br) return -EINVAL; --- a/net/bridge/br_notify.c 2010-11-15 08:25:50.045645976 -0800 +++ b/net/bridge/br_notify.c 2010-11-15 08:27:57.780866901 -0800 @@ -37,10 +37,10 @@ static int br_device_event(struct notifi int err; /* not a port of a bridge */ - if (!br_port_exists(dev)) + p = br_port_get_rtnl(dev); + if (!p) return NOTIFY_DONE; - p = br_port_get(dev); br = p->br; switch (event) { ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 0/5] bridge RCU patches (rev2) 2010-11-15 16:38 [PATCH 0/5] bridge RCU patches (rev2) Stephen Hemminger ` (4 preceding siblings ...) 2010-11-15 16:38 ` [PATCH 5/5] bridge: add RCU annotations to bridge port lookup Stephen Hemminger @ 2010-11-15 19:13 ` David Miller 5 siblings, 0 replies; 8+ messages in thread From: David Miller @ 2010-11-15 19:13 UTC (permalink / raw) To: shemminger; +Cc: netdev All applied to net-next-2.6 ^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH 0/5] bridge: RCU annotation and cleanup @ 2010-11-14 21:12 Stephen Hemminger 2010-11-14 21:12 ` [PATCH 1/5] bridge: add RCU annotation to bridge multicast table Stephen Hemminger 0 siblings, 1 reply; 8+ messages in thread From: Stephen Hemminger @ 2010-11-14 21:12 UTC (permalink / raw) To: David Miller, Eric Dumazet; +Cc: netdev, bridge This is a split up of what Eric did with a couple of small changes and additions. ^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH 1/5] bridge: add RCU annotation to bridge multicast table 2010-11-14 21:12 [PATCH 0/5] bridge: RCU annotation and cleanup Stephen Hemminger @ 2010-11-14 21:12 ` Stephen Hemminger 0 siblings, 0 replies; 8+ messages in thread From: Stephen Hemminger @ 2010-11-14 21:12 UTC (permalink / raw) To: David Miller, Eric Dumazet; +Cc: netdev, bridge [-- Attachment #1: bridge-mlock-rcu.patch --] [-- Type: text/plain, Size: 10144 bytes --] From: Eric Dumazet <eric.dumazet@gmail•com> Add modern __rcu annotatations to bridge multicast table. Signed-off-by: Eric Dumazet <eric.dumazet@gmail•com> Signed-off-by: Stephen Hemminger <shemminger@vyatta•com> --- net/bridge/br_forward.c | 4 +- net/bridge/br_multicast.c | 78 ++++++++++++++++++++++++++++++---------------- net/bridge/br_private.h | 6 +-- 3 files changed, 56 insertions(+), 32 deletions(-) --- a/net/bridge/br_multicast.c 2010-11-14 12:36:30.383348571 -0800 +++ b/net/bridge/br_multicast.c 2010-11-14 12:36:37.084167303 -0800 @@ -33,6 +33,9 @@ #include "br_private.h" +#define mlock_dereference(X, br) \ + rcu_dereference_protected(X, lockdep_is_held(&br->multicast_lock)) + #if defined(CONFIG_IPV6) || defined(CONFIG_IPV6_MODULE) static inline int ipv6_is_local_multicast(const struct in6_addr *addr) { @@ -135,7 +138,7 @@ static struct net_bridge_mdb_entry *br_m struct net_bridge_mdb_entry *br_mdb_get(struct net_bridge *br, struct sk_buff *skb) { - struct net_bridge_mdb_htable *mdb = br->mdb; + struct net_bridge_mdb_htable *mdb = rcu_dereference(br->mdb); struct br_ip ip; if (br->multicast_disabled) @@ -235,7 +238,8 @@ static void br_multicast_group_expired(u if (mp->ports) goto out; - mdb = br->mdb; + mdb = mlock_dereference(br->mdb, br); + hlist_del_rcu(&mp->hlist[mdb->ver]); mdb->size--; @@ -249,16 +253,20 @@ out: static void br_multicast_del_pg(struct net_bridge *br, struct net_bridge_port_group *pg) { - struct net_bridge_mdb_htable *mdb = br->mdb; + struct net_bridge_mdb_htable *mdb; struct net_bridge_mdb_entry *mp; struct net_bridge_port_group *p; - struct net_bridge_port_group **pp; + struct net_bridge_port_group __rcu **pp; + + mdb = mlock_dereference(br->mdb, br); mp = br_mdb_ip_get(mdb, &pg->addr); if (WARN_ON(!mp)) return; - for (pp = &mp->ports; (p = *pp); pp = &p->next) { + for (pp = &mp->ports; + (p = mlock_dereference(*pp, br)) != NULL; + pp = &p->next) { if (p != pg) continue; @@ -294,10 +302,10 @@ out: spin_unlock(&br->multicast_lock); } -static int br_mdb_rehash(struct net_bridge_mdb_htable **mdbp, int max, +static int br_mdb_rehash(struct net_bridge_mdb_htable __rcu **mdbp, int max, int elasticity) { - struct net_bridge_mdb_htable *old = *mdbp; + struct net_bridge_mdb_htable *old = rcu_dereference_protected(*mdbp, 1); struct net_bridge_mdb_htable *mdb; int err; @@ -569,7 +577,7 @@ static struct net_bridge_mdb_entry *br_m struct net_bridge *br, struct net_bridge_port *port, struct br_ip *group, int hash) { - struct net_bridge_mdb_htable *mdb = br->mdb; + struct net_bridge_mdb_htable *mdb; struct net_bridge_mdb_entry *mp; struct hlist_node *p; unsigned count = 0; @@ -577,6 +585,7 @@ static struct net_bridge_mdb_entry *br_m int elasticity; int err; + mdb = rcu_dereference_protected(br->mdb, 1); hlist_for_each_entry(mp, p, &mdb->mhash[hash], hlist[mdb->ver]) { count++; if (unlikely(br_ip_equal(group, &mp->addr))) @@ -642,10 +651,11 @@ static struct net_bridge_mdb_entry *br_m struct net_bridge *br, struct net_bridge_port *port, struct br_ip *group) { - struct net_bridge_mdb_htable *mdb = br->mdb; + struct net_bridge_mdb_htable *mdb; struct net_bridge_mdb_entry *mp; int hash; + mdb = rcu_dereference_protected(br->mdb, 1); if (!mdb) { if (br_mdb_rehash(&br->mdb, BR_HASH_SIZE, 0)) return NULL; @@ -660,7 +670,7 @@ static struct net_bridge_mdb_entry *br_m case -EAGAIN: rehash: - mdb = br->mdb; + mdb = rcu_dereference_protected(br->mdb, 1); hash = br_ip_hash(mdb, group); break; @@ -692,7 +702,7 @@ static int br_multicast_add_group(struct { struct net_bridge_mdb_entry *mp; struct net_bridge_port_group *p; - struct net_bridge_port_group **pp; + struct net_bridge_port_group __rcu **pp; unsigned long now = jiffies; int err; @@ -712,7 +722,9 @@ static int br_multicast_add_group(struct goto out; } - for (pp = &mp->ports; (p = *pp); pp = &p->next) { + for (pp = &mp->ports; + (p = mlock_dereference(*pp, br)) != NULL; + pp = &p->next) { if (p->port == port) goto found; if ((unsigned long)p->port < (unsigned long)port) @@ -1106,7 +1118,7 @@ static int br_ip4_multicast_query(struct struct net_bridge_mdb_entry *mp; struct igmpv3_query *ih3; struct net_bridge_port_group *p; - struct net_bridge_port_group **pp; + struct net_bridge_port_group __rcu **pp; unsigned long max_delay; unsigned long now = jiffies; __be32 group; @@ -1145,7 +1157,7 @@ static int br_ip4_multicast_query(struct if (!group) goto out; - mp = br_mdb_ip4_get(br->mdb, group); + mp = br_mdb_ip4_get(mlock_dereference(br->mdb, br), group); if (!mp) goto out; @@ -1157,7 +1169,9 @@ static int br_ip4_multicast_query(struct try_to_del_timer_sync(&mp->timer) >= 0)) mod_timer(&mp->timer, now + max_delay); - for (pp = &mp->ports; (p = *pp); pp = &p->next) { + for (pp = &mp->ports; + (p = mlock_dereference(*pp, br)) != NULL; + pp = &p->next) { if (timer_pending(&p->timer) ? time_after(p->timer.expires, now + max_delay) : try_to_del_timer_sync(&p->timer) >= 0) @@ -1178,7 +1192,8 @@ static int br_ip6_multicast_query(struct struct mld_msg *mld = (struct mld_msg *) icmp6_hdr(skb); struct net_bridge_mdb_entry *mp; struct mld2_query *mld2q; - struct net_bridge_port_group *p, **pp; + struct net_bridge_port_group *p; + struct net_bridge_port_group __rcu **pp; unsigned long max_delay; unsigned long now = jiffies; struct in6_addr *group = NULL; @@ -1214,7 +1229,7 @@ static int br_ip6_multicast_query(struct if (!group) goto out; - mp = br_mdb_ip6_get(br->mdb, group); + mp = br_mdb_ip6_get(mlock_dereference(br->mdb, br), group); if (!mp) goto out; @@ -1225,7 +1240,9 @@ static int br_ip6_multicast_query(struct try_to_del_timer_sync(&mp->timer) >= 0)) mod_timer(&mp->timer, now + max_delay); - for (pp = &mp->ports; (p = *pp); pp = &p->next) { + for (pp = &mp->ports; + (p = mlock_dereference(*pp, br)) != NULL; + pp = &p->next) { if (timer_pending(&p->timer) ? time_after(p->timer.expires, now + max_delay) : try_to_del_timer_sync(&p->timer) >= 0) @@ -1254,7 +1271,7 @@ static void br_multicast_leave_group(str timer_pending(&br->multicast_querier_timer)) goto out; - mdb = br->mdb; + mdb = mlock_dereference(br->mdb, br); mp = br_mdb_ip_get(mdb, group); if (!mp) goto out; @@ -1277,7 +1294,9 @@ static void br_multicast_leave_group(str goto out; } - for (p = mp->ports; p; p = p->next) { + for (p = mlock_dereference(mp->ports, br); + p != NULL; + p = mlock_dereference(p->next, br)) { if (p->port != port) continue; @@ -1625,7 +1644,7 @@ void br_multicast_stop(struct net_bridge del_timer_sync(&br->multicast_query_timer); spin_lock_bh(&br->multicast_lock); - mdb = br->mdb; + mdb = mlock_dereference(br->mdb, br); if (!mdb) goto out; @@ -1729,6 +1748,7 @@ int br_multicast_toggle(struct net_bridg { struct net_bridge_port *port; int err = 0; + struct net_bridge_mdb_htable *mdb; spin_lock(&br->multicast_lock); if (br->multicast_disabled == !val) @@ -1741,15 +1761,16 @@ int br_multicast_toggle(struct net_bridg if (!netif_running(br->dev)) goto unlock; - if (br->mdb) { - if (br->mdb->old) { + mdb = mlock_dereference(br->mdb, br); + if (mdb) { + if (mdb->old) { err = -EEXIST; rollback: br->multicast_disabled = !!val; goto unlock; } - err = br_mdb_rehash(&br->mdb, br->mdb->max, + err = br_mdb_rehash(&br->mdb, mdb->max, br->hash_elasticity); if (err) goto rollback; @@ -1774,6 +1795,7 @@ int br_multicast_set_hash_max(struct net { int err = -ENOENT; u32 old; + struct net_bridge_mdb_htable *mdb; spin_lock(&br->multicast_lock); if (!netif_running(br->dev)) @@ -1782,7 +1804,9 @@ int br_multicast_set_hash_max(struct net err = -EINVAL; if (!is_power_of_2(val)) goto unlock; - if (br->mdb && val < br->mdb->size) + + mdb = mlock_dereference(br->mdb, br); + if (mdb && val < mdb->size) goto unlock; err = 0; @@ -1790,8 +1814,8 @@ int br_multicast_set_hash_max(struct net old = br->hash_max; br->hash_max = val; - if (br->mdb) { - if (br->mdb->old) { + if (mdb) { + if (mdb->old) { err = -EEXIST; rollback: br->hash_max = old; --- a/net/bridge/br_private.h 2010-11-14 12:36:30.399350527 -0800 +++ b/net/bridge/br_private.h 2010-11-14 12:44:07.257410977 -0800 @@ -72,7 +72,7 @@ struct net_bridge_fdb_entry struct net_bridge_port_group { struct net_bridge_port *port; - struct net_bridge_port_group *next; + struct net_bridge_port_group __rcu *next; struct hlist_node mglist; struct rcu_head rcu; struct timer_list timer; @@ -86,7 +86,7 @@ struct net_bridge_mdb_entry struct hlist_node hlist[2]; struct hlist_node mglist; struct net_bridge *br; - struct net_bridge_port_group *ports; + struct net_bridge_port_group __rcu *ports; struct rcu_head rcu; struct timer_list timer; struct timer_list query_timer; @@ -227,7 +227,7 @@ struct net_bridge unsigned long multicast_startup_query_interval; spinlock_t multicast_lock; - struct net_bridge_mdb_htable *mdb; + struct net_bridge_mdb_htable __rcu *mdb; struct hlist_head router_list; struct hlist_head mglist; --- a/net/bridge/br_forward.c 2010-11-14 12:36:47.833478598 -0800 +++ b/net/bridge/br_forward.c 2010-11-14 12:42:22.001208297 -0800 @@ -223,7 +223,7 @@ static void br_multicast_flood(struct ne struct net_bridge_port_group *p; struct hlist_node *rp; - rp = rcu_dereference(br->router_list.first); + rp = rcu_dereference(hlist_first_rcu(&br->router_list)); p = mdst ? rcu_dereference(mdst->ports) : NULL; while (p || rp) { struct net_bridge_port *port, *lport, *rport; @@ -242,7 +242,7 @@ static void br_multicast_flood(struct ne if ((unsigned long)lport >= (unsigned long)port) p = rcu_dereference(p->next); if ((unsigned long)rport >= (unsigned long)port) - rp = rcu_dereference(rp->next); + rp = rcu_dereference(hlist_next_rcu(rp->next)); } if (!prev) ^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2010-11-15 19:13 UTC | newest] Thread overview: 8+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2010-11-15 16:38 [PATCH 0/5] bridge RCU patches (rev2) Stephen Hemminger 2010-11-15 16:38 ` [PATCH 1/5] bridge: add RCU annotation to bridge multicast table Stephen Hemminger 2010-11-15 16:38 ` [PATCH 2/5] bridge: add proper RCU annotation to should_route_hook Stephen Hemminger 2010-11-15 16:38 ` [PATCH 3/5] netdev: add rcu annotations to receive handler hook Stephen Hemminger 2010-11-15 16:38 ` [PATCH 4/5] bridge: fix RCU races with bridge port Stephen Hemminger 2010-11-15 16:38 ` [PATCH 5/5] bridge: add RCU annotations to bridge port lookup Stephen Hemminger 2010-11-15 19:13 ` [PATCH 0/5] bridge RCU patches (rev2) David Miller -- strict thread matches above, loose matches on Subject: below -- 2010-11-14 21:12 [PATCH 0/5] bridge: RCU annotation and cleanup Stephen Hemminger 2010-11-14 21:12 ` [PATCH 1/5] bridge: add RCU annotation to bridge multicast table Stephen Hemminger
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox