diff options
author | David S. Miller <davem@davemloft.net> | 2015-10-13 04:58:04 -0700 |
---|---|---|
committer | David S. Miller <davem@davemloft.net> | 2015-10-13 04:58:04 -0700 |
commit | bbb300eb976b613a8e4e666d3af39f5ab1031d22 (patch) | |
tree | ebe13617bfc4b22390d6a73f73f9a1db9b20deb6 /net | |
parent | 4b918163aecdec1c5424dcc317907282c58838df (diff) | |
parent | f409d0ed87d2721e1099ce36266e98c5aea2d486 (diff) |
Merge branch 'bridge-vlan'
Nikolay Aleksandrov says:
====================
bridge: vlan: cleanups & fixes (part 3)
Patch 01 converts the vlgrp member to use rcu as it was already used in a
similar way so better to make it official and use all the available RCU
instrumentation. Patch 02 fixes a bug where the vlan_list can be traversed
without rtnl or rcu held which could lead to using freed entries.
Patch 03 removes some redundant code that isn't needed anymore.
Patch 04 fixes a bug reported by Ido Schimmel about the vlan_flush order
and switchdevs, it moves it back.
v2: patch 03 and 04 are new, couldn't escape the second synchronize_rcu()
since the rhtable destruction can sleep
====================
Signed-off-by: David S. Miller <davem@davemloft.net>
Diffstat (limited to 'net')
-rw-r--r-- | net/bridge/br_device.c | 2 | ||||
-rw-r--r-- | net/bridge/br_forward.c | 6 | ||||
-rw-r--r-- | net/bridge/br_if.c | 3 | ||||
-rw-r--r-- | net/bridge/br_input.c | 4 | ||||
-rw-r--r-- | net/bridge/br_netlink.c | 25 | ||||
-rw-r--r-- | net/bridge/br_private.h | 33 | ||||
-rw-r--r-- | net/bridge/br_vlan.c | 133 |
7 files changed, 135 insertions, 71 deletions
diff --git a/net/bridge/br_device.c b/net/bridge/br_device.c index bdfb9544ca03..5e88d3e17546 100644 --- a/net/bridge/br_device.c +++ b/net/bridge/br_device.c @@ -56,7 +56,7 @@ netdev_tx_t br_dev_xmit(struct sk_buff *skb, struct net_device *dev) skb_reset_mac_header(skb); skb_pull(skb, ETH_HLEN); - if (!br_allowed_ingress(br, br_vlan_group(br), skb, &vid)) + if (!br_allowed_ingress(br, br_vlan_group_rcu(br), skb, &vid)) goto out; if (is_broadcast_ether_addr(dest)) diff --git a/net/bridge/br_forward.c b/net/bridge/br_forward.c index 6d5ed795c3e2..a9d424e20229 100644 --- a/net/bridge/br_forward.c +++ b/net/bridge/br_forward.c @@ -32,7 +32,7 @@ static inline int should_deliver(const struct net_bridge_port *p, { struct net_bridge_vlan_group *vg; - vg = nbp_vlan_group(p); + vg = nbp_vlan_group_rcu(p); return ((p->flags & BR_HAIRPIN_MODE) || skb->dev != p->dev) && br_allowed_egress(vg, skb) && p->state == BR_STATE_FORWARDING; } @@ -80,7 +80,7 @@ static void __br_deliver(const struct net_bridge_port *to, struct sk_buff *skb) { struct net_bridge_vlan_group *vg; - vg = nbp_vlan_group(to); + vg = nbp_vlan_group_rcu(to); skb = br_handle_vlan(to->br, vg, skb); if (!skb) return; @@ -112,7 +112,7 @@ static void __br_forward(const struct net_bridge_port *to, struct sk_buff *skb) return; } - vg = nbp_vlan_group(to); + vg = nbp_vlan_group_rcu(to); skb = br_handle_vlan(to->br, vg, skb); if (!skb) return; diff --git a/net/bridge/br_if.c b/net/bridge/br_if.c index 934cae9fa317..45e4757c6fd2 100644 --- a/net/bridge/br_if.c +++ b/net/bridge/br_if.c @@ -248,6 +248,7 @@ static void del_nbp(struct net_bridge_port *p) list_del_rcu(&p->list); + nbp_vlan_flush(p); br_fdb_delete_by_port(br, p, 0, 1); nbp_update_port_count(br); @@ -256,8 +257,6 @@ static void del_nbp(struct net_bridge_port *p) dev->priv_flags &= ~IFF_BRIDGE_PORT; netdev_rx_handler_unregister(dev); - /* use the synchronize_rcu done by netdev_rx_handler_unregister */ - nbp_vlan_flush(p); br_multicast_del_port(p); diff --git a/net/bridge/br_input.c b/net/bridge/br_input.c index f5c5a4500e2f..f7fba74108a9 100644 --- a/net/bridge/br_input.c +++ b/net/bridge/br_input.c @@ -44,7 +44,7 @@ static int br_pass_frame_up(struct sk_buff *skb) brstats->rx_bytes += skb->len; u64_stats_update_end(&brstats->syncp); - vg = br_vlan_group(br); + vg = br_vlan_group_rcu(br); /* Bridge is just like any other port. Make sure the * packet is allowed except in promisc modue when someone * may be running packet capture. @@ -140,7 +140,7 @@ int br_handle_frame_finish(struct net *net, struct sock *sk, struct sk_buff *skb if (!p || p->state == BR_STATE_DISABLED) goto drop; - if (!br_allowed_ingress(p->br, nbp_vlan_group(p), skb, &vid)) + if (!br_allowed_ingress(p->br, nbp_vlan_group_rcu(p), skb, &vid)) goto out; /* insert into forwarding database after filtering to avoid spoofing */ diff --git a/net/bridge/br_netlink.c b/net/bridge/br_netlink.c index d792d1a848ad..94b4de8c4646 100644 --- a/net/bridge/br_netlink.c +++ b/net/bridge/br_netlink.c @@ -102,10 +102,10 @@ static size_t br_get_link_af_size_filtered(const struct net_device *dev, rcu_read_lock(); if (br_port_exists(dev)) { p = br_port_get_rcu(dev); - vg = nbp_vlan_group(p); + vg = nbp_vlan_group_rcu(p); } else if (dev->priv_flags & IFF_EBRIDGE) { br = netdev_priv(dev); - vg = br_vlan_group(br); + vg = br_vlan_group_rcu(br); } num_vlan_infos = br_get_num_vlan_infos(vg, filter_mask); rcu_read_unlock(); @@ -253,7 +253,7 @@ static int br_fill_ifvlaninfo_compressed(struct sk_buff *skb, * if vlaninfo represents a range */ pvid = br_get_pvid(vg); - list_for_each_entry(v, &vg->vlan_list, vlist) { + list_for_each_entry_rcu(v, &vg->vlan_list, vlist) { flags = 0; if (!br_vlan_should_use(v)) continue; @@ -303,7 +303,7 @@ static int br_fill_ifvlaninfo(struct sk_buff *skb, u16 pvid; pvid = br_get_pvid(vg); - list_for_each_entry(v, &vg->vlan_list, vlist) { + list_for_each_entry_rcu(v, &vg->vlan_list, vlist) { if (!br_vlan_should_use(v)) continue; @@ -386,22 +386,27 @@ static int br_fill_ifinfo(struct sk_buff *skb, struct nlattr *af; int err; + /* RCU needed because of the VLAN locking rules (rcu || rtnl) */ + rcu_read_lock(); if (port) - vg = nbp_vlan_group(port); + vg = nbp_vlan_group_rcu(port); else - vg = br_vlan_group(br); + vg = br_vlan_group_rcu(br); - if (!vg || !vg->num_vlans) + if (!vg || !vg->num_vlans) { + rcu_read_unlock(); goto done; - + } af = nla_nest_start(skb, IFLA_AF_SPEC); - if (!af) + if (!af) { + rcu_read_unlock(); goto nla_put_failure; - + } if (filter_mask & RTEXT_FILTER_BRVLAN_COMPRESSED) err = br_fill_ifvlaninfo_compressed(skb, vg); else err = br_fill_ifvlaninfo(skb, vg); + rcu_read_unlock(); if (err) goto nla_put_failure; nla_nest_end(skb, af); diff --git a/net/bridge/br_private.h b/net/bridge/br_private.h index ba0c67b2159a..216018c76018 100644 --- a/net/bridge/br_private.h +++ b/net/bridge/br_private.h @@ -229,7 +229,7 @@ struct net_bridge_port struct netpoll *np; #endif #ifdef CONFIG_BRIDGE_VLAN_FILTERING - struct net_bridge_vlan_group *vlgrp; + struct net_bridge_vlan_group __rcu *vlgrp; #endif }; @@ -337,7 +337,7 @@ struct net_bridge struct kobject *ifobj; u32 auto_cnt; #ifdef CONFIG_BRIDGE_VLAN_FILTERING - struct net_bridge_vlan_group *vlgrp; + struct net_bridge_vlan_group __rcu *vlgrp; u8 vlan_enabled; __be16 vlan_proto; u16 default_pvid; @@ -700,13 +700,25 @@ int nbp_get_num_vlan_infos(struct net_bridge_port *p, u32 filter_mask); static inline struct net_bridge_vlan_group *br_vlan_group( const struct net_bridge *br) { - return br->vlgrp; + return rtnl_dereference(br->vlgrp); } static inline struct net_bridge_vlan_group *nbp_vlan_group( const struct net_bridge_port *p) { - return p->vlgrp; + return rtnl_dereference(p->vlgrp); +} + +static inline struct net_bridge_vlan_group *br_vlan_group_rcu( + const struct net_bridge *br) +{ + return rcu_dereference(br->vlgrp); +} + +static inline struct net_bridge_vlan_group *nbp_vlan_group_rcu( + const struct net_bridge_port *p) +{ + return rcu_dereference(p->vlgrp); } /* Since bridge now depends on 8021Q module, but the time bridge sees the @@ -853,6 +865,19 @@ static inline struct net_bridge_vlan_group *nbp_vlan_group( { return NULL; } + +static inline struct net_bridge_vlan_group *br_vlan_group_rcu( + const struct net_bridge *br) +{ + return NULL; +} + +static inline struct net_bridge_vlan_group *nbp_vlan_group_rcu( + const struct net_bridge_port *p) +{ + return NULL; +} + #endif struct nf_br_ops { diff --git a/net/bridge/br_vlan.c b/net/bridge/br_vlan.c index ad7e4f6b6d6b..5f0d0cc4744f 100644 --- a/net/bridge/br_vlan.c +++ b/net/bridge/br_vlan.c @@ -54,9 +54,9 @@ static void __vlan_add_flags(struct net_bridge_vlan *v, u16 flags) struct net_bridge_vlan_group *vg; if (br_vlan_is_master(v)) - vg = v->br->vlgrp; + vg = br_vlan_group(v->br); else - vg = v->port->vlgrp; + vg = nbp_vlan_group(v->port); if (flags & BRIDGE_VLAN_INFO_PVID) __vlan_add_pvid(vg, v->vid); @@ -91,11 +91,16 @@ static int __vlan_vid_add(struct net_device *dev, struct net_bridge *br, static void __vlan_add_list(struct net_bridge_vlan *v) { + struct net_bridge_vlan_group *vg; struct list_head *headp, *hpos; struct net_bridge_vlan *vent; - headp = br_vlan_is_master(v) ? &v->br->vlgrp->vlan_list : - &v->port->vlgrp->vlan_list; + if (br_vlan_is_master(v)) + vg = br_vlan_group(v->br); + else + vg = nbp_vlan_group(v->port); + + headp = &vg->vlan_list; list_for_each_prev(hpos, headp) { vent = list_entry(hpos, struct net_bridge_vlan, vlist); if (v->vid < vent->vid) @@ -137,14 +142,16 @@ static int __vlan_vid_del(struct net_device *dev, struct net_bridge *br, */ static struct net_bridge_vlan *br_vlan_get_master(struct net_bridge *br, u16 vid) { + struct net_bridge_vlan_group *vg; struct net_bridge_vlan *masterv; - masterv = br_vlan_find(br->vlgrp, vid); + vg = br_vlan_group(br); + masterv = br_vlan_find(vg, vid); if (!masterv) { /* missing global ctx, create it now */ if (br_vlan_add(br, vid, 0)) return NULL; - masterv = br_vlan_find(br->vlgrp, vid); + masterv = br_vlan_find(vg, vid); if (WARN_ON(!masterv)) return NULL; } @@ -155,11 +162,14 @@ static struct net_bridge_vlan *br_vlan_get_master(struct net_bridge *br, u16 vid static void br_vlan_put_master(struct net_bridge_vlan *masterv) { + struct net_bridge_vlan_group *vg; + if (!br_vlan_is_master(masterv)) return; + vg = br_vlan_group(masterv->br); if (atomic_dec_and_test(&masterv->refcnt)) { - rhashtable_remove_fast(&masterv->br->vlgrp->vlan_hash, + rhashtable_remove_fast(&vg->vlan_hash, &masterv->vnode, br_vlan_rht_params); __vlan_del_list(masterv); kfree_rcu(masterv, rcu); @@ -189,12 +199,12 @@ static int __vlan_add(struct net_bridge_vlan *v, u16 flags) if (br_vlan_is_master(v)) { br = v->br; dev = br->dev; - vg = br->vlgrp; + vg = br_vlan_group(br); } else { p = v->port; br = p->br; dev = p->dev; - vg = p->vlgrp; + vg = nbp_vlan_group(p); } if (p) { @@ -266,10 +276,10 @@ static int __vlan_del(struct net_bridge_vlan *v) int err = 0; if (br_vlan_is_master(v)) { - vg = v->br->vlgrp; + vg = br_vlan_group(v->br); } else { p = v->port; - vg = v->port->vlgrp; + vg = nbp_vlan_group(v->port); masterv = v->brvlan; } @@ -297,15 +307,20 @@ out: return err; } -static void __vlan_flush(struct net_bridge_vlan_group *vlgrp) +static void __vlan_group_free(struct net_bridge_vlan_group *vg) +{ + WARN_ON(!list_empty(&vg->vlan_list)); + rhashtable_destroy(&vg->vlan_hash); + kfree(vg); +} + +static void __vlan_flush(struct net_bridge_vlan_group *vg) { struct net_bridge_vlan *vlan, *tmp; - __vlan_delete_pvid(vlgrp, vlgrp->pvid); - list_for_each_entry_safe(vlan, tmp, &vlgrp->vlan_list, vlist) + __vlan_delete_pvid(vg, vg->pvid); + list_for_each_entry_safe(vlan, tmp, &vg->vlan_list, vlist) __vlan_del(vlan); - rhashtable_destroy(&vlgrp->vlan_hash); - kfree(vlgrp); } struct sk_buff *br_handle_vlan(struct net_bridge *br, @@ -467,7 +482,7 @@ bool br_should_learn(struct net_bridge_port *p, struct sk_buff *skb, u16 *vid) if (!br->vlan_enabled) return true; - vg = p->vlgrp; + vg = nbp_vlan_group(p); if (!vg || !vg->num_vlans) return false; @@ -493,12 +508,14 @@ bool br_should_learn(struct net_bridge_port *p, struct sk_buff *skb, u16 *vid) */ int br_vlan_add(struct net_bridge *br, u16 vid, u16 flags) { + struct net_bridge_vlan_group *vg; struct net_bridge_vlan *vlan; int ret; ASSERT_RTNL(); - vlan = br_vlan_find(br->vlgrp, vid); + vg = br_vlan_group(br); + vlan = br_vlan_find(vg, vid); if (vlan) { if (!br_vlan_is_brentry(vlan)) { /* Trying to change flags of non-existent bridge vlan */ @@ -513,7 +530,7 @@ int br_vlan_add(struct net_bridge *br, u16 vid, u16 flags) } atomic_inc(&vlan->refcnt); vlan->flags |= BRIDGE_VLAN_INFO_BRENTRY; - br->vlgrp->num_vlans++; + vg->num_vlans++; } __vlan_add_flags(vlan, flags); return 0; @@ -541,11 +558,13 @@ int br_vlan_add(struct net_bridge *br, u16 vid, u16 flags) */ int br_vlan_delete(struct net_bridge *br, u16 vid) { + struct net_bridge_vlan_group *vg; struct net_bridge_vlan *v; ASSERT_RTNL(); - v = br_vlan_find(br->vlgrp, vid); + vg = br_vlan_group(br); + v = br_vlan_find(vg, vid); if (!v || !br_vlan_is_brentry(v)) return -ENOENT; @@ -557,9 +576,15 @@ int br_vlan_delete(struct net_bridge *br, u16 vid) void br_vlan_flush(struct net_bridge *br) { + struct net_bridge_vlan_group *vg; + ASSERT_RTNL(); - __vlan_flush(br_vlan_group(br)); + vg = br_vlan_group(br); + __vlan_flush(vg); + RCU_INIT_POINTER(br->vlgrp, NULL); + synchronize_rcu(); + __vlan_group_free(vg); } struct net_bridge_vlan *br_vlan_find(struct net_bridge_vlan_group *vg, u16 vid) @@ -626,6 +651,7 @@ int __br_vlan_set_proto(struct net_bridge *br, __be16 proto) int err = 0; struct net_bridge_port *p; struct net_bridge_vlan *vlan; + struct net_bridge_vlan_group *vg; __be16 oldproto; if (br->vlan_proto == proto) @@ -633,7 +659,8 @@ int __br_vlan_set_proto(struct net_bridge *br, __be16 proto) /* Add VLANs for the new proto to the device filter. */ list_for_each_entry(p, &br->port_list, list) { - list_for_each_entry(vlan, &p->vlgrp->vlan_list, vlist) { + vg = nbp_vlan_group(p); + list_for_each_entry(vlan, &vg->vlan_list, vlist) { err = vlan_vid_add(p->dev, proto, vlan->vid); if (err) goto err_filt; @@ -647,19 +674,23 @@ int __br_vlan_set_proto(struct net_bridge *br, __be16 proto) br_recalculate_fwd_mask(br); /* Delete VLANs for the old proto from the device filter. */ - list_for_each_entry(p, &br->port_list, list) - list_for_each_entry(vlan, &p->vlgrp->vlan_list, vlist) + list_for_each_entry(p, &br->port_list, list) { + vg = nbp_vlan_group(p); + list_for_each_entry(vlan, &vg->vlan_list, vlist) vlan_vid_del(p->dev, oldproto, vlan->vid); + } return 0; err_filt: - list_for_each_entry_continue_reverse(vlan, &p->vlgrp->vlan_list, vlist) + list_for_each_entry_continue_reverse(vlan, &vg->vlan_list, vlist) vlan_vid_del(p->dev, proto, vlan->vid); - list_for_each_entry_continue_reverse(p, &br->port_list, list) - list_for_each_entry(vlan, &p->vlgrp->vlan_list, vlist) + list_for_each_entry_continue_reverse(p, &br->port_list, list) { + vg = nbp_vlan_group(p); + list_for_each_entry(vlan, &vg->vlan_list, vlist) vlan_vid_del(p->dev, proto, vlan->vid); + } return err; } @@ -703,11 +734,11 @@ static void br_vlan_disable_default_pvid(struct net_bridge *br) /* Disable default_pvid on all ports where it is still * configured. */ - if (vlan_default_pvid(br->vlgrp, pvid)) + if (vlan_default_pvid(br_vlan_group(br), pvid)) br_vlan_delete(br, pvid); list_for_each_entry(p, &br->port_list, list) { - if (vlan_default_pvid(p->vlgrp, pvid)) + if (vlan_default_pvid(nbp_vlan_group(p), pvid)) nbp_vlan_delete(p, pvid); } @@ -717,6 +748,7 @@ static void br_vlan_disable_default_pvid(struct net_bridge *br) int __br_vlan_set_default_pvid(struct net_bridge *br, u16 pvid) { const struct net_bridge_vlan *pvent; + struct net_bridge_vlan_group *vg; struct net_bridge_port *p; u16 old_pvid; int err = 0; @@ -737,8 +769,9 @@ int __br_vlan_set_default_pvid(struct net_bridge *br, u16 pvid) /* Update default_pvid config only if we do not conflict with * user configuration. */ - pvent = br_vlan_find(br->vlgrp, pvid); - if ((!old_pvid || vlan_default_pvid(br->vlgrp, old_pvid)) && + vg = br_vlan_group(br); + pvent = br_vlan_find(vg, pvid); + if ((!old_pvid || vlan_default_pvid(vg, old_pvid)) && (!pvent || !br_vlan_should_use(pvent))) { err = br_vlan_add(br, pvid, BRIDGE_VLAN_INFO_PVID | @@ -754,9 +787,10 @@ int __br_vlan_set_default_pvid(struct net_bridge *br, u16 pvid) /* Update default_pvid config only if we do not conflict with * user configuration. */ + vg = nbp_vlan_group(p); if ((old_pvid && - !vlan_default_pvid(p->vlgrp, old_pvid)) || - br_vlan_find(p->vlgrp, pvid)) + !vlan_default_pvid(vg, old_pvid)) || + br_vlan_find(vg, pvid)) continue; err = nbp_vlan_add(p, pvid, @@ -825,17 +859,19 @@ unlock: int br_vlan_init(struct net_bridge *br) { + struct net_bridge_vlan_group *vg; int ret = -ENOMEM; - br->vlgrp = kzalloc(sizeof(struct net_bridge_vlan_group), GFP_KERNEL); - if (!br->vlgrp) + vg = kzalloc(sizeof(*vg), GFP_KERNEL); + if (!vg) goto out; - ret = rhashtable_init(&br->vlgrp->vlan_hash, &br_vlan_rht_params); + ret = rhashtable_init(&vg->vlan_hash, &br_vlan_rht_params); if (ret) goto err_rhtbl; - INIT_LIST_HEAD(&br->vlgrp->vlan_list); + INIT_LIST_HEAD(&vg->vlan_list); br->vlan_proto = htons(ETH_P_8021Q); br->default_pvid = 1; + rcu_assign_pointer(br->vlgrp, vg); ret = br_vlan_add(br, 1, BRIDGE_VLAN_INFO_PVID | BRIDGE_VLAN_INFO_UNTAGGED | BRIDGE_VLAN_INFO_BRENTRY); @@ -846,9 +882,9 @@ out: return ret; err_vlan_add: - rhashtable_destroy(&br->vlgrp->vlan_hash); + rhashtable_destroy(&vg->vlan_hash); err_rhtbl: - kfree(br->vlgrp); + kfree(vg); goto out; } @@ -866,9 +902,7 @@ int nbp_vlan_init(struct net_bridge_port *p) if (ret) goto err_rhtbl; INIT_LIST_HEAD(&vg->vlan_list); - /* Make sure everything's committed before publishing vg */ - smp_wmb(); - p->vlgrp = vg; + rcu_assign_pointer(p->vlgrp, vg); if (p->br->default_pvid) { ret = nbp_vlan_add(p, p->br->default_pvid, BRIDGE_VLAN_INFO_PVID | @@ -897,7 +931,7 @@ int nbp_vlan_add(struct net_bridge_port *port, u16 vid, u16 flags) ASSERT_RTNL(); - vlan = br_vlan_find(port->vlgrp, vid); + vlan = br_vlan_find(nbp_vlan_group(port), vid); if (vlan) { __vlan_add_flags(vlan, flags); return 0; @@ -925,7 +959,7 @@ int nbp_vlan_delete(struct net_bridge_port *port, u16 vid) ASSERT_RTNL(); - v = br_vlan_find(port->vlgrp, vid); + v = br_vlan_find(nbp_vlan_group(port), vid); if (!v) return -ENOENT; br_fdb_find_delete_local(port->br, port, port->dev->dev_addr, vid); @@ -936,12 +970,13 @@ int nbp_vlan_delete(struct net_bridge_port *port, u16 vid) void nbp_vlan_flush(struct net_bridge_port *port) { - struct net_bridge_vlan *vlan; + struct net_bridge_vlan_group *vg; ASSERT_RTNL(); - list_for_each_entry(vlan, &port->vlgrp->vlan_list, vlist) - vlan_vid_del(port->dev, port->br->vlan_proto, vlan->vid); - - __vlan_flush(nbp_vlan_group(port)); + vg = nbp_vlan_group(port); + __vlan_flush(vg); + RCU_INIT_POINTER(port->vlgrp, NULL); + synchronize_rcu(); + __vlan_group_free(vg); } |