Sun Sep 1 11:07:06 2019 UTC ()
Pull up following revision(s) (requested by msaitoh in ticket #133):

	sys/dev/pci/ixgbe/ixgbe.c: revision 1.200
	sys/dev/pci/ixgbe/ixgbe.c: revision 1.201
	sys/dev/pci/ixgbe/ixv.c: revision 1.126
	sys/dev/pci/ixgbe/ixv.c: revision 1.127
	sys/net/if_vlan.c: revision 1.142
	sys/net/if_vlan.c: revision 1.143
	sys/net/if_vlan.c: revision 1.144
	sys/net/if_vlan.c: revision 1.145
	sys/net/if_vlan.c: revision 1.146

 Check ec_capenable instead of ec_capabilities to control TX side of VLAN HW
tagging correctly.
XXX pullup-9

 Add missing IFNET_LOCK() and IFNET_UNLOCK() in vlan_config().
XXX pullup-9

 Fix a bug that VLAN HW "tagging" enable/disable may not reflect correctly.
  - Always call ec_vlan_cb() if it exists.
  - Some (or all?) ethernet drivers don't enable HW tagging if no any vlan is
    attached. ixgbe is one of them. Check the the transition and update
    VLAN HW tagging function.
XXX pullup-9

 Use ETHER_LOCK()/ETHER_UNLOCK() suggested by knakahara.
- kmem_alloc(,KM_SLEEP) never return NULL, so remove NULL check.
- VLAN ID is never duplicated, so break the loop when found. Also move
  kmen_free() outside of ETHER_LOCK(ec)/ETHER_UNLOCK(ec) to reduce the hold
  time. suggested by ozaki-r.
- Whitespace fix.


(martin)
diff -r1.199 -r1.199.2.1 src/sys/dev/pci/ixgbe/ixgbe.c
diff -r1.125 -r1.125.2.1 src/sys/dev/pci/ixgbe/ixv.c
diff -r1.141 -r1.141.2.1 src/sys/net/if_vlan.c

cvs diff -r1.199 -r1.199.2.1 src/sys/dev/pci/ixgbe/ixgbe.c (expand / switch to context diff)
--- src/sys/dev/pci/ixgbe/ixgbe.c 2019/07/30 08:44:28 1.199
+++ src/sys/dev/pci/ixgbe/ixgbe.c 2019/09/01 11:07:05 1.199.2.1
@@ -1,4 +1,4 @@
-/* $NetBSD: ixgbe.c,v 1.199 2019/07/30 08:44:28 msaitoh Exp $ */
+/* $NetBSD: ixgbe.c,v 1.199.2.1 2019/09/01 11:07:05 martin Exp $ */
 
 /******************************************************************************
 
@@ -219,6 +219,7 @@
 static u8 *	ixgbe_mc_array_itr(struct ixgbe_hw *, u8 **, u32 *);
 static void	ixgbe_eitr_write(struct adapter *, uint32_t, uint32_t);
 
+static void	ixgbe_setup_vlan_hw_tagging(struct adapter *);
 static void	ixgbe_setup_vlan_hw_support(struct adapter *);
 static int	ixgbe_vlan_cb(struct ethercom *, uint16_t, bool);
 static int	ixgbe_register_vlan(void *, struct ifnet *, u16);
@@ -2305,6 +2306,7 @@
 ixgbe_vlan_cb(struct ethercom *ec, uint16_t vid, bool set)
 {
 	struct ifnet *ifp = &ec->ec_if;
+	struct adapter *adapter = ifp->if_softc;
 	int rv;
 
 	if (set)
@@ -2312,6 +2314,16 @@
 	else
 		rv = ixgbe_unregister_vlan(ifp->if_softc, ifp, vid);
 
+	if (rv != 0)
+		return rv;
+
+	/*
+	 * Control VLAN HW tagging when ec_nvlan is changed from 1 to 0
+	 * or 0 to 1.
+	 */
+	if ((set && (ec->ec_nvlans == 1)) || (!set && (ec->ec_nvlans == 0)))
+		ixgbe_setup_vlan_hw_tagging(adapter);
+
 	return rv;
 }
 
@@ -2381,21 +2393,15 @@
 } /* ixgbe_unregister_vlan */
 
 static void
-ixgbe_setup_vlan_hw_support(struct adapter *adapter)
+ixgbe_setup_vlan_hw_tagging(struct adapter *adapter)
 {
 	struct ethercom *ec = &adapter->osdep.ec;
 	struct ixgbe_hw *hw = &adapter->hw;
 	struct rx_ring	*rxr;
-	int		i;
 	u32		ctrl;
-	struct vlanid_list *vlanidp;
+	int		i;
 	bool		hwtagging;
 
-	/*
-	 *  This function is called from both if_init and ifflags_cb()
-	 * on NetBSD.
-	 */
-
 	/* Enable HW tagging only if any vlan is attached */
 	hwtagging = (ec->ec_capenable & ETHERCAP_VLAN_HWTAGGING)
 	    && VLAN_ATTACHED(ec);
@@ -2417,11 +2423,46 @@
 		rxr->vtag_strip = hwtagging ? TRUE : FALSE;
 	}
 
+	/* VLAN hw tagging for 82598 */
+	if (hw->mac.type == ixgbe_mac_82598EB) {
+		ctrl = IXGBE_READ_REG(hw, IXGBE_VLNCTRL);
+		if (hwtagging)
+			ctrl |= IXGBE_VLNCTRL_VME;
+		else
+			ctrl &= ~IXGBE_VLNCTRL_VME;
+		IXGBE_WRITE_REG(hw, IXGBE_VLNCTRL, ctrl);
+	}
+} /* ixgbe_setup_vlan_hw_tagging */
+
+static void
+ixgbe_setup_vlan_hw_support(struct adapter *adapter)
+{
+	struct ethercom *ec = &adapter->osdep.ec;
+	struct ixgbe_hw *hw = &adapter->hw;
+	int		i;
+	u32		ctrl;
+	struct vlanid_list *vlanidp;
+
+	/*
+	 *  This function is called from both if_init and ifflags_cb()
+	 * on NetBSD.
+	 */
+
+	/*
+	 * Part 1:
+	 * Setup VLAN HW tagging
+	 */
+	ixgbe_setup_vlan_hw_tagging(adapter);
+
+	/*
+	 * Part 2:
+	 * Setup VLAN HW filter
+	 */
 	/* Cleanup shadow_vfta */
 	for (i = 0; i < IXGBE_VFTA_SIZE; i++)
 		adapter->shadow_vfta[i] = 0;
 	/* Generate shadow_vfta from ec_vids */
-	mutex_enter(ec->ec_lock);
+	ETHER_LOCK(ec);
 	SIMPLEQ_FOREACH(vlanidp, &ec->ec_vids, vid_list) {
 		uint32_t idx;
 
@@ -2429,7 +2470,7 @@
 		KASSERT(idx < IXGBE_VFTA_SIZE);
 		adapter->shadow_vfta[idx] |= (u32)1 << (vlanidp->vid % 32);
 	}
-	mutex_exit(ec->ec_lock);
+	ETHER_UNLOCK(ec);
 	for (i = 0; i < IXGBE_VFTA_SIZE; i++)
 		IXGBE_WRITE_REG(hw, IXGBE_VFTA(i), adapter->shadow_vfta[i]);
 
@@ -2439,13 +2480,6 @@
 		ctrl |= IXGBE_VLNCTRL_VFE;
 	else
 		ctrl &= ~IXGBE_VLNCTRL_VFE;
-	/* VLAN hw tagging for 82598 */
-	if (hw->mac.type == ixgbe_mac_82598EB) {
-		if (hwtagging)
-			ctrl |= IXGBE_VLNCTRL_VME;
-		else
-			ctrl &= ~IXGBE_VLNCTRL_VME;
-	}
 	IXGBE_WRITE_REG(hw, IXGBE_VLNCTRL, ctrl);
 } /* ixgbe_setup_vlan_hw_support */
 

cvs diff -r1.125 -r1.125.2.1 src/sys/dev/pci/ixgbe/ixv.c (expand / switch to context diff)
--- src/sys/dev/pci/ixgbe/ixv.c 2019/07/30 08:38:03 1.125
+++ src/sys/dev/pci/ixgbe/ixv.c 2019/09/01 11:07:05 1.125.2.1
@@ -1,4 +1,4 @@
-/*$NetBSD: ixv.c,v 1.125 2019/07/30 08:38:03 msaitoh Exp $*/
+/*$NetBSD: ixv.c,v 1.125.2.1 2019/09/01 11:07:05 martin Exp $*/
 
 /******************************************************************************
 
@@ -120,6 +120,7 @@
 static u8 *	ixv_mc_array_itr(struct ixgbe_hw *, u8 **, u32 *);
 static void	ixv_eitr_write(struct adapter *, uint32_t, uint32_t);
 
+static void	ixv_setup_vlan_tagging(struct adapter *);
 static int	ixv_setup_vlan_support(struct adapter *);
 static int	ixv_vlan_cb(struct ethercom *, uint16_t, bool);
 static int	ixv_register_vlan(void *, struct ifnet *, u16);
@@ -1935,33 +1936,22 @@
 	return sysctl_lookup(SYSCTLFN_CALL(&node));
 } /* ixv_sysctl_rdt_handler */
 
-/************************************************************************
- * ixv_setup_vlan_support
- ************************************************************************/
-static int
-ixv_setup_vlan_support(struct adapter *adapter)
+static void
+ixv_setup_vlan_tagging(struct adapter *adapter)
 {
 	struct ethercom *ec = &adapter->osdep.ec;
 	struct ixgbe_hw *hw = &adapter->hw;
 	struct rx_ring	*rxr;
-	u32		ctrl, vid, vfta, retry;
-	struct vlanid_list *vlanidp;
-	int rv, error = 0;
-	bool usevlan;
+	u32		ctrl;
+	int		i;
 	bool		hwtagging;
 
-	/*
-	 *  This function is called from both if_init and ifflags_cb()
-	 * on NetBSD.
-	 */
-	usevlan = VLAN_ATTACHED(ec);
-
 	/* Enable HW tagging only if any vlan is attached */
 	hwtagging = (ec->ec_capenable & ETHERCAP_VLAN_HWTAGGING)
 	    && VLAN_ATTACHED(ec);
 
 	/* Enable the queues */
-	for (int i = 0; i < adapter->num_queues; i++) {
+	for (i = 0; i < adapter->num_queues; i++) {
 		rxr = &adapter->rx_rings[i];
 		ctrl = IXGBE_READ_REG(hw, IXGBE_VFRXDCTL(rxr->me));
 		if (hwtagging)
@@ -1975,15 +1965,43 @@
 		 */
 		rxr->vtag_strip = hwtagging ? TRUE : FALSE;
 	}
+} /* ixv_setup_vlan_tagging */
 
-	if (!usevlan)
+/************************************************************************
+ * ixv_setup_vlan_support
+ ************************************************************************/
+static int
+ixv_setup_vlan_support(struct adapter *adapter)
+{
+	struct ethercom *ec = &adapter->osdep.ec;
+	struct ixgbe_hw *hw = &adapter->hw;
+	u32		vid, vfta, retry;
+	struct vlanid_list *vlanidp;
+	int rv, error = 0;
+
+	/*
+	 *  This function is called from both if_init and ifflags_cb()
+	 * on NetBSD.
+	 */
+
+	/*
+	 * Part 1:
+	 * Setup VLAN HW tagging
+	 */
+	ixv_setup_vlan_tagging(adapter);
+
+	if (!VLAN_ATTACHED(ec))
 		return 0;
 
+	/*
+	 * Part 2:
+	 * Setup VLAN HW filter
+	 */
 	/* Cleanup shadow_vfta */
 	for (int i = 0; i < IXGBE_VFTA_SIZE; i++)
 		adapter->shadow_vfta[i] = 0;
 	/* Generate shadow_vfta from ec_vids */
-	mutex_enter(ec->ec_lock);
+	ETHER_LOCK(ec);
 	SIMPLEQ_FOREACH(vlanidp, &ec->ec_vids, vid_list) {
 		uint32_t idx;
 
@@ -1991,7 +2009,7 @@
 		KASSERT(idx < IXGBE_VFTA_SIZE);
 		adapter->shadow_vfta[idx] |= (u32)1 << (vlanidp->vid % 32);
 	}
-	mutex_exit(ec->ec_lock);
+	ETHER_UNLOCK(ec);
 	
 	/*
 	 * A soft reset zero's out the VFTA, so
@@ -2036,12 +2054,23 @@
 ixv_vlan_cb(struct ethercom *ec, uint16_t vid, bool set)
 {
 	struct ifnet *ifp = &ec->ec_if;
+	struct adapter *adapter = ifp->if_softc;
 	int rv;
 
 	if (set)
 		rv = ixv_register_vlan(ifp->if_softc, ifp, vid);
 	else
 		rv = ixv_unregister_vlan(ifp->if_softc, ifp, vid);
+
+	if (rv != 0)
+		return rv;
+
+	/*
+	 * Control VLAN HW tagging when ec_nvlan is changed from 1 to 0
+	 * or 0 to 1.
+	 */
+	if ((set && (ec->ec_nvlans == 1)) || (!set && (ec->ec_nvlans == 0)))
+		ixv_setup_vlan_tagging(adapter);
 
 	return rv;
 }

cvs diff -r1.141 -r1.141.2.1 src/sys/net/if_vlan.c (expand / switch to context diff)
--- src/sys/net/if_vlan.c 2019/07/17 03:26:24 1.141
+++ src/sys/net/if_vlan.c 2019/09/01 11:07:06 1.141.2.1
@@ -1,4 +1,4 @@
-/*	$NetBSD: if_vlan.c,v 1.141 2019/07/17 03:26:24 msaitoh Exp $	*/
+/*	$NetBSD: if_vlan.c,v 1.141.2.1 2019/09/01 11:07:06 martin Exp $	*/
 
 /*
  * Copyright (c) 2000, 2001 The NetBSD Foundation, Inc.
@@ -78,7 +78,7 @@
  */
 
 #include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: if_vlan.c,v 1.141 2019/07/17 03:26:24 msaitoh Exp $");
+__KERNEL_RCSID(0, "$NetBSD: if_vlan.c,v 1.141.2.1 2019/09/01 11:07:06 martin Exp $");
 
 #ifdef _KERNEL_OPT
 #include "opt_inet.h"
@@ -485,36 +485,29 @@
 			}
 			error = 0;
 		}
-		/*
-		 * Add a vid to the list even if it's not enabled in case
-		 * it's enabled later.
-		 */
-		if (ec->ec_capabilities & ETHERCAP_VLAN_HWFILTER) {
-			vidmem = kmem_alloc(sizeof(struct vlanid_list),
-			    KM_SLEEP);
-			if (vidmem == NULL){
+		/* Add a vid to the list */
+		vidmem = kmem_alloc(sizeof(struct vlanid_list), KM_SLEEP);
+		vidmem->vid = vid;
+		ETHER_LOCK(ec);
+		SIMPLEQ_INSERT_TAIL(&ec->ec_vids, vidmem, vid_list);
+		ETHER_UNLOCK(ec);
+
+		if (ec->ec_vlan_cb != NULL) {
+			/*
+			 * Call ec_vlan_cb(). It will setup VLAN HW filter or
+			 * HW tagging function.
+			 */
+			error = (*ec->ec_vlan_cb)(ec, vid, true);
+			if (error) {
 				ec->ec_nvlans--;
-				if (ec->ec_nvlans == 0)
+				if (ec->ec_nvlans == 0) {
+					IFNET_LOCK(p);
 					(void)ether_disable_vlan_mtu(p);
-				error = ENOMEM;
+					IFNET_UNLOCK(p);
+				}
 				goto done;
 			}
-			vidmem->vid = vid;
-			mutex_enter(ec->ec_lock);
-			SIMPLEQ_INSERT_TAIL(&ec->ec_vids, vidmem, vid_list);
-			mutex_exit(ec->ec_lock);
 		}
-		if (ec->ec_capenable & ETHERCAP_VLAN_HWFILTER) {
-			if (ec->ec_vlan_cb != NULL) {
-				error = (*ec->ec_vlan_cb)(ec, vid, true);
-				if (error) {
-					ec->ec_nvlans--;
-					if (ec->ec_nvlans == 0)
-						(void)ether_disable_vlan_mtu(p);
-					goto done;
-				}
-			}
-		}
 		/*
 		 * If the parent interface can do hardware-assisted
 		 * VLAN encapsulation, then propagate its hardware-
@@ -640,20 +633,28 @@
 	case IFT_ETHER:
 	    {
 		struct ethercom *ec = (void *)p;
-		struct vlanid_list *vlanidp, *tmpp;
+		struct vlanid_list *vlanidp;
 		uint16_t vid = EVL_VLANOFTAG(nmib->ifvm_tag);
 
-		mutex_enter(ec->ec_lock);
-		SIMPLEQ_FOREACH_SAFE(vlanidp, &ec->ec_vids, vid_list, tmpp) {
+		ETHER_LOCK(ec);
+		SIMPLEQ_FOREACH(vlanidp, &ec->ec_vids, vid_list) {
 			if (vlanidp->vid == vid) {
 				SIMPLEQ_REMOVE(&ec->ec_vids, vlanidp,
 				    vlanid_list, vid_list);
-				kmem_free(vlanidp, sizeof(*vlanidp));
+				break;
 			}
 		}
-		mutex_exit(ec->ec_lock);
-		if (ec->ec_vlan_cb != NULL)
+		ETHER_UNLOCK(ec);
+		if (vlanidp != NULL)
+			kmem_free(vlanidp, sizeof(*vlanidp));
+
+		if (ec->ec_vlan_cb != NULL) {
+			/*
+			 * Call ec_vlan_cb(). It will setup VLAN HW filter or
+			 * HW tagging function.
+			 */
 			(void)(*ec->ec_vlan_cb)(ec, vid, false);
+		}
 		if (--ec->ec_nvlans == 0) {
 			IFNET_LOCK(p);
 			(void)ether_disable_vlan_mtu(p);
@@ -1035,7 +1036,7 @@
 			error = ENOENT;
 			break;
 		}
-		
+
 		error = vlan_config(ifv, pr, vlr.vlr_tag);
 		if (error != 0)
 			break;
@@ -1373,7 +1374,7 @@
 		 * If the parent can insert the tag itself, just mark
 		 * the tag in the mbuf header.
 		 */
-		if (ec->ec_capabilities & ETHERCAP_VLAN_HWTAGGING) {
+		if (ec->ec_capenable & ETHERCAP_VLAN_HWTAGGING) {
 			vlan_set_tag(m, mib->ifvm_tag);
 		} else {
 			/*
@@ -1491,7 +1492,7 @@
 	 * If the parent can insert the tag itself, just mark
 	 * the tag in the mbuf header.
 	 */
-	if (ec->ec_capabilities & ETHERCAP_VLAN_HWTAGGING) {
+	if (ec->ec_capenable & ETHERCAP_VLAN_HWTAGGING) {
 		vlan_set_tag(m, mib->ifvm_tag);
 	} else {
 		/*