Skip to content

Commit

Permalink
bridge: vlan: use proper rcu for the vlgrp member
Browse files Browse the repository at this point in the history
The bridge and port's vlgrp member is already used in RCU way, currently
we rely on the fact that it cannot disappear while the port exists but
that is error-prone and we might miss places with improper locking
(either RCU or RTNL must be held to walk the vlan_list). So make it
official and use RCU for vlgrp to catch offenders. Introduce proper vlgrp
accessors and use them consistently throughout the code.

Signed-off-by: Nikolay Aleksandrov <nikolay@cumulusnetworks.com>
Reviewed-by: Ido Schimmel <idosch@mellanox.com>
Signed-off-by: David S. Miller <davem@davemloft.net>
  • Loading branch information
Nikolay Aleksandrov authored and David S. Miller committed Oct 13, 2015
1 parent 4b91816 commit 907b1e6
Show file tree
Hide file tree
Showing 6 changed files with 104 additions and 53 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
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
4 changes: 2 additions & 2 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
34 changes: 30 additions & 4 deletions net/bridge/br_private.h
Original file line number Diff line number Diff line change
Expand Up @@ -132,6 +132,7 @@ struct net_bridge_vlan_group {
struct list_head vlan_list;
u16 num_vlans;
u16 pvid;
struct rcu_head rcu;
};

struct net_bridge_fdb_entry
Expand Down Expand Up @@ -229,7 +230,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 +338,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 +701,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 +866,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 907b1e6

Please sign in to comment.