Skip to content

Commit

Permalink
Merge branch 'bridge_vlan_cleanups_fixes'
Browse files Browse the repository at this point in the history
Nikolay Aleksandrov says:

====================
bridge: vlan: cleanups & fixes

This is the first follow-up set, patch 01 reduces the default rhashtable
size and the number of locks that can be allocated. Patch 02 and 04 fix
possible null pointer dereferences due to the new ordering and
initialization on port add/del, and patch 03 moves the "pvid" member in
the net_bridge_vlan_group struct in order to simplify code (similar to how
it was with the older struct). Patch 05 fixes adding a vlan on a port which
is pvid and doesn't have a global context yet.
Please review carefully, I think this is the first use of rhashtable's
"locks_mul" member in the tree and I'd like to make sure it's correct.
Another thing that needs special attention is the nbp_vlan_flush() move
after the rx_handler unregister.
====================

Signed-off-by: David S. Miller <davem@davemloft.net>
  • Loading branch information
David S. Miller committed Oct 2, 2015
2 parents 4bf1b54 + 248234c commit 2dc6a03
Show file tree
Hide file tree
Showing 6 changed files with 93 additions and 127 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, skb, &vid))
if (!br_allowed_ingress(br, br_vlan_group(br), skb, &vid))
goto out;

if (is_broadcast_ether_addr(dest))
Expand Down
3 changes: 2 additions & 1 deletion net/bridge/br_if.c
Original file line number Diff line number Diff line change
Expand Up @@ -248,7 +248,6 @@ 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 @@ -257,6 +256,8 @@ 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
2 changes: 1 addition & 1 deletion net/bridge/br_input.c
Original file line number Diff line number Diff line change
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 (!nbp_allowed_ingress(p, skb, &vid))
if (!br_allowed_ingress(p->br, nbp_vlan_group(p), skb, &vid))
goto out;

/* insert into forwarding database after filtering to avoid spoofing */
Expand Down
42 changes: 18 additions & 24 deletions net/bridge/br_netlink.c
Original file line number Diff line number Diff line change
Expand Up @@ -22,17 +22,17 @@
#include "br_private_stp.h"

static int __get_num_vlan_infos(struct net_bridge_vlan_group *vg,
u32 filter_mask,
u16 pvid)
u32 filter_mask)
{
struct net_bridge_vlan *v;
u16 vid_range_start = 0, vid_range_end = 0, vid_range_flags = 0;
u16 flags;
u16 flags, pvid;
int num_vlans = 0;

if (!(filter_mask & RTEXT_FILTER_BRVLAN_COMPRESSED))
return 0;

pvid = br_get_pvid(vg);
/* Count number of vlan infos */
list_for_each_entry(v, &vg->vlan_list, vlist) {
flags = 0;
Expand Down Expand Up @@ -74,15 +74,15 @@ static int __get_num_vlan_infos(struct net_bridge_vlan_group *vg,
}

static int br_get_num_vlan_infos(struct net_bridge_vlan_group *vg,
u32 filter_mask, u16 pvid)
u32 filter_mask)
{
if (!vg)
return 0;

if (filter_mask & RTEXT_FILTER_BRVLAN)
return vg->num_vlans;

return __get_num_vlan_infos(vg, filter_mask, pvid);
return __get_num_vlan_infos(vg, filter_mask);
}

static size_t br_get_link_af_size_filtered(const struct net_device *dev,
Expand All @@ -92,19 +92,16 @@ static size_t br_get_link_af_size_filtered(const struct net_device *dev,
struct net_bridge_port *p;
struct net_bridge *br;
int num_vlan_infos;
u16 pvid = 0;

rcu_read_lock();
if (br_port_exists(dev)) {
p = br_port_get_rcu(dev);
vg = nbp_vlan_group(p);
pvid = nbp_get_pvid(p);
} else if (dev->priv_flags & IFF_EBRIDGE) {
br = netdev_priv(dev);
vg = br_vlan_group(br);
pvid = br_get_pvid(br);
}
num_vlan_infos = br_get_num_vlan_infos(vg, filter_mask, pvid);
num_vlan_infos = br_get_num_vlan_infos(vg, filter_mask);
rcu_read_unlock();

/* Each VLAN is returned in bridge_vlan_info along with flags */
Expand Down Expand Up @@ -196,18 +193,18 @@ static int br_fill_ifvlaninfo_range(struct sk_buff *skb, u16 vid_start,
}

static int br_fill_ifvlaninfo_compressed(struct sk_buff *skb,
struct net_bridge_vlan_group *vg,
u16 pvid)
struct net_bridge_vlan_group *vg)
{
struct net_bridge_vlan *v;
u16 vid_range_start = 0, vid_range_end = 0, vid_range_flags = 0;
u16 flags;
u16 flags, pvid;
int err = 0;

/* Pack IFLA_BRIDGE_VLAN_INFO's for every vlan
* and mark vlan info with begin and end flags
* if vlaninfo represents a range
*/
pvid = br_get_pvid(vg);
list_for_each_entry(v, &vg->vlan_list, vlist) {
flags = 0;
if (!br_vlan_should_use(v))
Expand Down Expand Up @@ -251,12 +248,13 @@ static int br_fill_ifvlaninfo_compressed(struct sk_buff *skb,
}

static int br_fill_ifvlaninfo(struct sk_buff *skb,
struct net_bridge_vlan_group *vg,
u16 pvid)
struct net_bridge_vlan_group *vg)
{
struct bridge_vlan_info vinfo;
struct net_bridge_vlan *v;
u16 pvid;

pvid = br_get_pvid(vg);
list_for_each_entry(v, &vg->vlan_list, vlist) {
if (!br_vlan_should_use(v))
continue;
Expand Down Expand Up @@ -338,16 +336,12 @@ static int br_fill_ifinfo(struct sk_buff *skb,
(filter_mask & RTEXT_FILTER_BRVLAN_COMPRESSED)) {
struct net_bridge_vlan_group *vg;
struct nlattr *af;
u16 pvid;
int err;

if (port) {
if (port)
vg = nbp_vlan_group(port);
pvid = nbp_get_pvid(port);
} else {
else
vg = br_vlan_group(br);
pvid = br_get_pvid(br);
}

if (!vg || !vg->num_vlans)
goto done;
Expand All @@ -357,9 +351,9 @@ static int br_fill_ifinfo(struct sk_buff *skb,
goto nla_put_failure;

if (filter_mask & RTEXT_FILTER_BRVLAN_COMPRESSED)
err = br_fill_ifvlaninfo_compressed(skb, vg, pvid);
err = br_fill_ifvlaninfo_compressed(skb, vg);
else
err = br_fill_ifvlaninfo(skb, vg, pvid);
err = br_fill_ifvlaninfo(skb, vg);
if (err)
goto nla_put_failure;
nla_nest_end(skb, af);
Expand Down Expand Up @@ -884,11 +878,11 @@ static size_t br_get_link_af_size(const struct net_device *dev)
if (br_port_exists(dev)) {
p = br_port_get_rtnl(dev);
num_vlans = br_get_num_vlan_infos(nbp_vlan_group(p),
RTEXT_FILTER_BRVLAN, 0);
RTEXT_FILTER_BRVLAN);
} else if (dev->priv_flags & IFF_EBRIDGE) {
br = netdev_priv(dev);
num_vlans = br_get_num_vlan_infos(br_vlan_group(br),
RTEXT_FILTER_BRVLAN, 0);
RTEXT_FILTER_BRVLAN);
}

/* Each VLAN is returned in bridge_vlan_info along with flags */
Expand Down
44 changes: 12 additions & 32 deletions net/bridge/br_private.h
Original file line number Diff line number Diff line change
Expand Up @@ -119,6 +119,7 @@ struct net_bridge_vlan {
* @vlan_hash: VLAN entry rhashtable
* @vlan_list: sorted VLAN entry list
* @num_vlans: number of total VLAN entries
* @pvid: PVID VLAN id
*
* IMPORTANT: Be careful when checking if there're VLAN entries using list
* primitives because the bridge can have entries in its list which
Expand All @@ -130,6 +131,7 @@ struct net_bridge_vlan_group {
struct rhashtable vlan_hash;
struct list_head vlan_list;
u16 num_vlans;
u16 pvid;
};

struct net_bridge_fdb_entry
Expand Down Expand Up @@ -228,7 +230,6 @@ struct net_bridge_port
#endif
#ifdef CONFIG_BRIDGE_VLAN_FILTERING
struct net_bridge_vlan_group *vlgrp;
u16 pvid;
#endif
};

Expand Down Expand Up @@ -340,7 +341,6 @@ struct net_bridge
u8 vlan_enabled;
__be16 vlan_proto;
u16 default_pvid;
u16 pvid;
#endif
};

Expand Down Expand Up @@ -670,10 +670,10 @@ static inline void br_mdb_uninit(void)

/* br_vlan.c */
#ifdef CONFIG_BRIDGE_VLAN_FILTERING
bool br_allowed_ingress(struct net_bridge *br, struct sk_buff *skb, u16 *vid);
bool nbp_allowed_ingress(struct net_bridge_port *p, struct sk_buff *skb,
u16 *vid);
bool br_allowed_egress(struct net_bridge_vlan_group *br,
bool br_allowed_ingress(const struct net_bridge *br,
struct net_bridge_vlan_group *vg, struct sk_buff *skb,
u16 *vid);
bool br_allowed_egress(struct net_bridge_vlan_group *vg,
const struct sk_buff *skb);
bool br_should_learn(struct net_bridge_port *p, struct sk_buff *skb, u16 *vid);
struct sk_buff *br_handle_vlan(struct net_bridge *br,
Expand Down Expand Up @@ -725,43 +725,28 @@ static inline int br_vlan_get_tag(const struct sk_buff *skb, u16 *vid)
return err;
}

static inline u16 br_get_pvid(const struct net_bridge *br)
{
if (!br)
return 0;

smp_rmb();
return br->pvid;
}

static inline u16 nbp_get_pvid(const struct net_bridge_port *p)
static inline u16 br_get_pvid(const struct net_bridge_vlan_group *vg)
{
if (!p)
if (!vg)
return 0;

smp_rmb();
return p->pvid;
return vg->pvid;
}

static inline int br_vlan_enabled(struct net_bridge *br)
{
return br->vlan_enabled;
}
#else
static inline bool br_allowed_ingress(struct net_bridge *br,
static inline bool br_allowed_ingress(const struct net_bridge *br,
struct net_bridge_vlan_group *vg,
struct sk_buff *skb,
u16 *vid)
{
return true;
}

static inline bool nbp_allowed_ingress(struct net_bridge_port *p,
struct sk_buff *skb,
u16 *vid)
{
return true;
}

static inline bool br_allowed_egress(struct net_bridge_vlan_group *vg,
const struct sk_buff *skb)
{
Expand Down Expand Up @@ -834,12 +819,7 @@ static inline u16 br_vlan_get_tag(const struct sk_buff *skb, u16 *tag)
return 0;
}

static inline u16 br_get_pvid(const struct net_bridge *br)
{
return 0;
}

static inline u16 nbp_get_pvid(const struct net_bridge_port *p)
static inline u16 br_get_pvid(const struct net_bridge_vlan_group *vg)
{
return 0;
}
Expand Down
Loading

0 comments on commit 2dc6a03

Please sign in to comment.