Skip to content

Commit

Permalink
Merge branch 'bridge-vlan'
Browse files Browse the repository at this point in the history
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>
  • Loading branch information
davem330 committed Oct 13, 2015
2 parents 4b91816 + f409d0e commit bbb300e
Show file tree
Hide file tree
Showing 7 changed files with 135 additions and 71 deletions.
2 changes: 1 addition & 1 deletion net/bridge/br_device.c
Original file line number Diff line number Diff line change
Expand Up @@ -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))
Expand Down
6 changes: 3 additions & 3 deletions net/bridge/br_forward.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand Down
3 changes: 1 addition & 2 deletions net/bridge/br_if.c
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand All @@ -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);

Expand Down
4 changes: 2 additions & 2 deletions net/bridge/br_input.c
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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 */
Expand Down
25 changes: 15 additions & 10 deletions net/bridge/br_netlink.c
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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;

Expand Down Expand Up @@ -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);
Expand Down
33 changes: 29 additions & 4 deletions net/bridge/br_private.h
Original file line number Diff line number Diff line change
Expand Up @@ -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
};

Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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 {
Expand Down
Loading

0 comments on commit bbb300e

Please sign in to comment.