Merge branch 'bridge_vlan_cleanups_fixes'

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: NDavid S. Miller <davem@davemloft.net>

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, skb, &vid))
+	if (!br_allowed_ingress(br, br_vlan_group(br), skb, &vid))
 		goto out;
 
 	if (is_broadcast_ether_addr(dest))

net/bridge/br_if.c

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

net/bridge/br_input.c

@@ -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 */

net/bridge/br_netlink.c

@@ -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;
@@ -74,7 +74,7 @@ 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;
@@ -82,7 +82,7 @@ static int br_get_num_vlan_infos(struct net_bridge_vlan_group *vg,
 	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,
@@ -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 */
@@ -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))
@@ -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;
@@ -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;
@@ -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);
@@ -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 */

net/bridge/br_private.h

@@ -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
@@ -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
@@ -228,7 +230,6 @@ struct net_bridge_port
 #endif
 #ifdef CONFIG_BRIDGE_VLAN_FILTERING
 	struct net_bridge_vlan_group	*vlgrp;
-	u16				pvid;
 #endif
 };
 
@@ -340,7 +341,6 @@ struct net_bridge
 	u8				vlan_enabled;
 	__be16				vlan_proto;
 	u16				default_pvid;
-	u16				pvid;
 #endif
 };
 
@@ -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,
@@ -725,22 +725,13 @@ 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)
@@ -748,20 +739,14 @@ 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)
 {
@@ -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;
 }

net/bridge/br_vlan.c

@@ -19,6 +19,8 @@ static const struct rhashtable_params br_vlan_rht_params = {
 	.head_offset = offsetof(struct net_bridge_vlan, vnode),
 	.key_offset = offsetof(struct net_bridge_vlan, vid),
 	.key_len = sizeof(u16),
+	.nelem_hint = 3,
+	.locks_mul = 1,
 	.max_size = VLAN_N_VID,
 	.obj_cmpfn = br_vlan_cmp,
 	.automatic_shrinking = true,
@@ -29,37 +31,37 @@ static struct net_bridge_vlan *br_vlan_lookup(struct rhashtable *tbl, u16 vid)
 	return rhashtable_lookup_fast(tbl, &vid, br_vlan_rht_params);
 }
 
-static void __vlan_add_pvid(u16 *pvid, u16 vid)
+static void __vlan_add_pvid(struct net_bridge_vlan_group *vg, u16 vid)
 {
-	if (*pvid == vid)
+	if (vg->pvid == vid)
 		return;
 
 	smp_wmb();
-	*pvid = vid;
+	vg->pvid = vid;
 }
 
-static void __vlan_delete_pvid(u16 *pvid, u16 vid)
+static void __vlan_delete_pvid(struct net_bridge_vlan_group *vg, u16 vid)
 {
-	if (*pvid != vid)
+	if (vg->pvid != vid)
 		return;
 
 	smp_wmb();
-	*pvid = 0;
+	vg->pvid = 0;
 }
 
 static void __vlan_add_flags(struct net_bridge_vlan *v, u16 flags)
 {
-	if (flags & BRIDGE_VLAN_INFO_PVID) {
-		if (br_vlan_is_master(v))
-			__vlan_add_pvid(&v->br->pvid, v->vid);
-		else
-			__vlan_add_pvid(&v->port->pvid, v->vid);
-	} else {
-		if (br_vlan_is_master(v))
-			__vlan_delete_pvid(&v->br->pvid, v->vid);
-		else
-			__vlan_delete_pvid(&v->port->pvid, v->vid);
-	}
+	struct net_bridge_vlan_group *vg;
+
+	if (br_vlan_is_master(v))
+		vg = v->br->vlgrp;
+	else
+		vg = v->port->vlgrp;
+
+	if (flags & BRIDGE_VLAN_INFO_PVID)
+		__vlan_add_pvid(vg, v->vid);
+	else
+		__vlan_delete_pvid(vg, v->vid);
 
 	if (flags & BRIDGE_VLAN_INFO_UNTAGGED)
 		v->flags |= BRIDGE_VLAN_INFO_UNTAGGED;
@@ -195,7 +197,7 @@ static int __vlan_add(struct net_bridge_vlan *v, u16 flags)
 		masterv = br_vlan_find(br->vlgrp, v->vid);
 		if (!masterv) {
 			/* missing global ctx, create it now */
-			err = br_vlan_add(br, v->vid, master_flags);
+			err = br_vlan_add(br, v->vid, 0);
 			if (err)
 				goto out_filt;
 			masterv = br_vlan_find(br->vlgrp, v->vid);
@@ -247,25 +249,22 @@ static int __vlan_add(struct net_bridge_vlan *v, u16 flags)
 static int __vlan_del(struct net_bridge_vlan *v)
 {
 	struct net_bridge_vlan *masterv = v;
+	struct net_bridge_vlan_group *vg;
 	struct net_bridge_port *p = NULL;
 	struct net_bridge *br;
 	int err = 0;
-	struct rhashtable *tbl;
-	u16 *pvid;
 
 	if (br_vlan_is_master(v)) {
 		br = v->br;
-		tbl = &v->br->vlgrp->vlan_hash;
-		pvid = &v->br->pvid;
+		vg = v->br->vlgrp;
 	} else {
 		p = v->port;
 		br = p->br;
-		tbl = &p->vlgrp->vlan_hash;
+		vg = v->port->vlgrp;
 		masterv = v->brvlan;
-		pvid = &p->pvid;
 	}
 
-	__vlan_delete_pvid(pvid, v->vid);
+	__vlan_delete_pvid(vg, v->vid);
 	if (p) {
 		err = __vlan_vid_del(p->dev, p->br, v->vid);
 		if (err)
@@ -282,7 +281,8 @@ static int __vlan_del(struct net_bridge_vlan *v)
 	}
 
 	if (masterv != v) {
-		rhashtable_remove_fast(tbl, &v->vnode, br_vlan_rht_params);
+		rhashtable_remove_fast(&vg->vlan_hash, &v->vnode,
+				       br_vlan_rht_params);
 		__vlan_del_list(v);
 		kfree_rcu(v, rcu);
 	}
@@ -297,11 +297,11 @@ static int __vlan_del(struct net_bridge_vlan *v)
 	return err;
 }
 
-static void __vlan_flush(struct net_bridge_vlan_group *vlgrp, u16 *pvid)
+static void __vlan_flush(struct net_bridge_vlan_group *vlgrp)
 {
 	struct net_bridge_vlan *vlan, *tmp;
 
-	__vlan_delete_pvid(pvid, *pvid);
+	__vlan_delete_pvid(vlgrp, vlgrp->pvid);
 	list_for_each_entry_safe(vlan, tmp, &vlgrp->vlan_list, vlist)
 		__vlan_del(vlan);
 	rhashtable_destroy(&vlgrp->vlan_hash);
@@ -346,7 +346,7 @@ struct sk_buff *br_handle_vlan(struct net_bridge *br,
 }
 
 /* Called under RCU */
-static bool __allowed_ingress(struct rhashtable *tbl, u16 pvid, __be16 proto,
+static bool __allowed_ingress(struct net_bridge_vlan_group *vg, __be16 proto,
 			      struct sk_buff *skb, u16 *vid)
 {
 	const struct net_bridge_vlan *v;
@@ -387,6 +387,8 @@ static bool __allowed_ingress(struct rhashtable *tbl, u16 pvid, __be16 proto,
 	}
 
 	if (!*vid) {
+		u16 pvid = br_get_pvid(vg);
+
 		/* Frame had a tag with VID 0 or did not have a tag.
 		 * See if pvid is set on this port.  That tells us which
 		 * vlan untagged or priority-tagged traffic belongs to.
@@ -413,7 +415,7 @@ static bool __allowed_ingress(struct rhashtable *tbl, u16 pvid, __be16 proto,
 	}
 
 	/* Frame had a valid vlan tag.  See if vlan is allowed */
-	v = br_vlan_lookup(tbl, *vid);
+	v = br_vlan_find(vg, *vid);
 	if (v && br_vlan_should_use(v))
 		return true;
 drop:
@@ -421,7 +423,9 @@ static bool __allowed_ingress(struct rhashtable *tbl, u16 pvid, __be16 proto,
 	return false;
 }
 
-bool br_allowed_ingress(struct net_bridge *br, struct sk_buff *skb, u16 *vid)
+bool br_allowed_ingress(const struct net_bridge *br,
+			struct net_bridge_vlan_group *vg, struct sk_buff *skb,
+			u16 *vid)
 {
 	/* If VLAN filtering is disabled on the bridge, all packets are
 	 * permitted.
@@ -431,25 +435,7 @@ bool br_allowed_ingress(struct net_bridge *br, struct sk_buff *skb, u16 *vid)
 		return true;
 	}
 
-	return __allowed_ingress(&br->vlgrp->vlan_hash, br->pvid,
-				 br->vlan_proto, skb, vid);
-}
-
-bool nbp_allowed_ingress(struct net_bridge_port *p, struct sk_buff *skb,
-			 u16 *vid)
-{
-	struct net_bridge *br = p->br;
-
-	/* If VLAN filtering is disabled on the bridge, all packets are
-	 * permitted.
-	 */
-	if (!br->vlan_enabled) {
-		BR_INPUT_SKB_CB(skb)->vlan_filtered = false;
-		return true;
-	}
-
-	return __allowed_ingress(&p->vlgrp->vlan_hash, p->pvid, br->vlan_proto,
-				 skb, vid);
+	return __allowed_ingress(vg, br->vlan_proto, skb, vid);
 }
 
 /* Called under RCU. */
@@ -474,27 +460,29 @@ bool br_allowed_egress(struct net_bridge_vlan_group *vg,
 /* Called under RCU */
 bool br_should_learn(struct net_bridge_port *p, struct sk_buff *skb, u16 *vid)
 {
+	struct net_bridge_vlan_group *vg;
 	struct net_bridge *br = p->br;
 
 	/* If filtering was disabled at input, let it pass. */
 	if (!br->vlan_enabled)
 		return true;
 
-	if (!p->vlgrp->num_vlans)
+	vg = p->vlgrp;
+	if (!vg || !vg->num_vlans)
 		return false;
 
 	if (!br_vlan_get_tag(skb, vid) && skb->vlan_proto != br->vlan_proto)
 		*vid = 0;
 
 	if (!*vid) {
-		*vid = nbp_get_pvid(p);
+		*vid = br_get_pvid(vg);
 		if (!*vid)
 			return false;
 
 		return true;
 	}
 
-	if (br_vlan_find(p->vlgrp, *vid))
+	if (br_vlan_find(vg, *vid))
 		return true;
 
 	return false;
@@ -570,7 +558,7 @@ void br_vlan_flush(struct net_bridge *br)
 {
 	ASSERT_RTNL();
 
-	__vlan_flush(br_vlan_group(br), &br->pvid);
+	__vlan_flush(br_vlan_group(br));
 }
 
 struct net_bridge_vlan *br_vlan_find(struct net_bridge_vlan_group *vg, u16 vid)
@@ -691,12 +679,11 @@ int br_vlan_set_proto(struct net_bridge *br, unsigned long val)
 	return err;
 }
 
-static bool vlan_default_pvid(struct net_bridge_vlan_group *vg, u16 pvid,
-			      u16 vid)
+static bool vlan_default_pvid(struct net_bridge_vlan_group *vg, u16 vid)
 {
 	struct net_bridge_vlan *v;
 
-	if (vid != pvid)
+	if (vid != vg->pvid)
 		return false;
 
 	v = br_vlan_lookup(&vg->vlan_hash, vid);
@@ -715,11 +702,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, br->pvid, pvid))
+	if (vlan_default_pvid(br->vlgrp, pvid))
 		br_vlan_delete(br, pvid);
 
 	list_for_each_entry(p, &br->port_list, list) {
-		if (vlan_default_pvid(p->vlgrp, p->pvid, pvid))
+		if (vlan_default_pvid(p->vlgrp, pvid))
 			nbp_vlan_delete(p, pvid);
 	}
 
@@ -745,7 +732,7 @@ static int __br_vlan_set_default_pvid(struct net_bridge *br, u16 pvid)
 	 * user configuration.
 	 */
 	pvent = br_vlan_find(br->vlgrp, pvid);
-	if ((!old_pvid || vlan_default_pvid(br->vlgrp, br->pvid, old_pvid)) &&
+	if ((!old_pvid || vlan_default_pvid(br->vlgrp, old_pvid)) &&
 	    (!pvent || !br_vlan_should_use(pvent))) {
 		err = br_vlan_add(br, pvid,
 				  BRIDGE_VLAN_INFO_PVID |
@@ -762,7 +749,7 @@ static int __br_vlan_set_default_pvid(struct net_bridge *br, u16 pvid)
 		 * user configuration.
 		 */
 		if ((old_pvid &&
-		     !vlan_default_pvid(p->vlgrp, p->pvid, old_pvid)) ||
+		     !vlan_default_pvid(p->vlgrp, old_pvid)) ||
 		    br_vlan_find(p->vlgrp, pvid))
 			continue;
 
@@ -867,16 +854,20 @@ int br_vlan_init(struct net_bridge *br)
 
 int nbp_vlan_init(struct net_bridge_port *p)
 {
+	struct net_bridge_vlan_group *vg;
 	int ret = -ENOMEM;
 
-	p->vlgrp = kzalloc(sizeof(struct net_bridge_vlan_group), GFP_KERNEL);
-	if (!p->vlgrp)
+	vg = kzalloc(sizeof(struct net_bridge_vlan_group), GFP_KERNEL);
+	if (!vg)
 		goto out;
 
-	ret = rhashtable_init(&p->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(&p->vlgrp->vlan_list);
+	INIT_LIST_HEAD(&vg->vlan_list);
+	/* Make sure everything's committed before publishing vg */
+	smp_wmb();
+	p->vlgrp = vg;
 	if (p->br->default_pvid) {
 		ret = nbp_vlan_add(p, p->br->default_pvid,
 				   BRIDGE_VLAN_INFO_PVID |
@@ -888,9 +879,9 @@ int nbp_vlan_init(struct net_bridge_port *p)
 	return ret;
 
 err_vlan_add:
-	rhashtable_destroy(&p->vlgrp->vlan_hash);
+	rhashtable_destroy(&vg->vlan_hash);
 err_rhtbl:
-	kfree(p->vlgrp);
+	kfree(vg);
 
 	goto out;
 }
@@ -951,5 +942,5 @@ void nbp_vlan_flush(struct net_bridge_port *port)
 	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), &port->pvid);
+	__vlan_flush(nbp_vlan_group(port));
 }