[net-next 00/10][pull request] Intel Wired LAN Driver Updates

[net-next 00/10][pull request] Intel Wired LAN Driver Updates

[Jeff Kirsher]

This series contains updates to ixgbe, e100, igb, and e1000e.

Alex provides a ixgbe patch to use the generic helper pci_vfs_assigned
instead of the ixgbe specific function ixgbe_vfs_are_assigned.

Greg provides 2 ixgbe patches, the first patch retains VLAN filtering
in promiscuous mode because using the new bridge FDB interface to allow
SR-IOV virtual function network devices to communicate with SW bridged
network devices the physical function is placed into promiscuous mode
and hardware VLAN filtering is disabled.  The second patch fixes a typo
where the code should use a bitwise "and" rather than a logical "and".

Andy Shevchenko provides an e100 patch to dump small buffers via %*ph.

Akeem provides three patches against igb.  The first resets the link when
a user enables/disables EEE when link is up.  The second changes register
read to "just-read" without returning a value for hardware to accurately
latch the register value.  His third patch adds rcu_lock to avoid possible
race conditions with igb_update_stats.

Mitch provides a igb patch to fix an issue where MSI-X interrupts are required
for SR-IOV operation/functionality.

Dean (from Redhat) provides a patch to resolve minor merger conflicts.

Wei Yang provides a e1000e patch to remove duplicate assignment of default
Rx/Tx ring size.

The following are changes since commit 8599b52e14a1611dcb563289421bee76751f1d53:
  bonding: add an option to fail when any of arp_ip_target is inaccessible
and are available in the git repository at:
  git://git.kernel.org/pub/scm/linux/kernel/git/jkirsher/net-next master

Akeem G. Abodunrin (3):
  igb: Reset the link when EEE setting changed
  igb: Read register for latch_on without return value
  igb: Added rcu_lock to avoid race

Alexander Duyck (1):
  ixgbe: Use pci_vfs_assigned instead of ixgbe_vfs_are_assigned

Andy Shevchenko (1):
  e100: dump small buffers via %*ph

Dean Nelson (1):
  e1000e: restore call to pci_clear_master()

Greg Rose (2):
  ixgbe: Retain VLAN filtering in promiscuous + VT mode
  ixgbe: Fix typo

Mitch A Williams (1):
  igb: don't allow SR-IOV without MSI-X

Wei Yang (1):
  e1000e: Remove duplicate assignment of default rx/tx ring size

 drivers/net/ethernet/intel/e100.c              |  15 ++--
 drivers/net/ethernet/intel/e1000e/netdev.c     |   6 +-
 drivers/net/ethernet/intel/igb/e1000_82575.c   |   9 +--
 drivers/net/ethernet/intel/igb/igb_ethtool.c   |   4 +-
 drivers/net/ethernet/intel/igb/igb_main.c      |  26 ++++---
 drivers/net/ethernet/intel/igb/igb_ptp.c       |   8 +-
 drivers/net/ethernet/intel/ixgbe/ixgbe_main.c  |  11 ++-
 drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.c | 101 ++++++++++++++++---------
 8 files changed, 110 insertions(+), 70 deletions(-)

-- 
1.7.11.7

[net-next 01/10] ixgbe: Retain VLAN filtering in promiscuous + VT mode

[Jeff Kirsher]

From: Greg Rose <gregory.v.rose@intel.com>

When using the new bridge FDB interface to allow SR-IOV virtual function
network devices to communicate with SW bridged network devices the
physical function is placed into promiscuous mode and hardware VLAN
filtering is disabled.  This defeats the ability to use VLAN tagging
to isolate user networks.  When the device is in promiscuous mode and
VT mode simultaneously ensure that VLAN hardware filtering remains
enabled.

Signed-off-by: Greg Rose <gregory.v.rose@intel.com>
Tested-by: Stephen Ko <stephen.s.ko@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
 drivers/net/ethernet/intel/ixgbe/ixgbe_main.c  | 11 ++++-
 drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.c | 66 ++++++++++++++++++++++++++
 2 files changed, 75 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
index 047ebaa..30b4f5b 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
@@ -3724,8 +3724,15 @@ void ixgbe_set_rx_mode(struct net_device *netdev)
 		hw->addr_ctrl.user_set_promisc = true;
 		fctrl |= (IXGBE_FCTRL_UPE | IXGBE_FCTRL_MPE);
 		vmolr |= (IXGBE_VMOLR_ROPE | IXGBE_VMOLR_MPE);
-		/* don't hardware filter vlans in promisc mode */
-		ixgbe_vlan_filter_disable(adapter);
+		/* Only disable hardware filter vlans in promiscuous mode
+		 * if SR-IOV and VMDQ are disabled - otherwise ensure
+		 * that hardware VLAN filters remain enabled.
+		 */
+		if (!(adapter->flags & (IXGBE_FLAG_VMDQ_ENABLED |
+					IXGBE_FLAG_SRIOV_ENABLED)))
+			ixgbe_vlan_filter_disable(adapter);
+		else
+			ixgbe_vlan_filter_enable(adapter);
 	} else {
 		if (netdev->flags & IFF_ALLMULTI) {
 			fctrl |= IXGBE_FCTRL_MPE;
diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.c
index 1e7d587..a85d5f4 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.c
@@ -768,6 +768,29 @@ static int ixgbe_set_vf_mac_addr(struct ixgbe_adapter *adapter,
 	return ixgbe_set_vf_mac(adapter, vf, new_mac) < 0;
 }
 
+static int ixgbe_find_vlvf_entry(struct ixgbe_hw *hw, u32 vlan)
+{
+	u32 vlvf;
+	s32 regindex;
+
+	/* short cut the special case */
+	if (vlan == 0)
+		return 0;
+
+	/* Search for the vlan id in the VLVF entries */
+	for (regindex = 1; regindex < IXGBE_VLVF_ENTRIES; regindex++) {
+		vlvf = IXGBE_READ_REG(hw, IXGBE_VLVF(regindex));
+		if ((vlvf & VLAN_VID_MASK) == vlan)
+			break;
+	}
+
+	/* Return a negative value if not found */
+	if (regindex >= IXGBE_VLVF_ENTRIES)
+		regindex = -1;
+
+	return regindex;
+}
+
 static int ixgbe_set_vf_vlan_msg(struct ixgbe_adapter *adapter,
 				 u32 *msgbuf, u32 vf)
 {
@@ -775,6 +798,9 @@ static int ixgbe_set_vf_vlan_msg(struct ixgbe_adapter *adapter,
 	int add = (msgbuf[0] & IXGBE_VT_MSGINFO_MASK) >> IXGBE_VT_MSGINFO_SHIFT;
 	int vid = (msgbuf[1] & IXGBE_VLVF_VLANID_MASK);
 	int err;
+	s32 reg_ndx;
+	u32 vlvf;
+	u32 bits;
 	u8 tcs = netdev_get_num_tc(adapter->netdev);
 
 	if (adapter->vfinfo[vf].pf_vlan || tcs) {
@@ -790,10 +816,50 @@ static int ixgbe_set_vf_vlan_msg(struct ixgbe_adapter *adapter,
 	else if (adapter->vfinfo[vf].vlan_count)
 		adapter->vfinfo[vf].vlan_count--;
 
+	/* in case of promiscuous mode any VLAN filter set for a VF must
+	 * also have the PF pool added to it.
+	 */
+	if (add && adapter->netdev->flags & IFF_PROMISC)
+		err = ixgbe_set_vf_vlan(adapter, add, vid, VMDQ_P(0));
+
 	err = ixgbe_set_vf_vlan(adapter, add, vid, vf);
 	if (!err && adapter->vfinfo[vf].spoofchk_enabled)
 		hw->mac.ops.set_vlan_anti_spoofing(hw, true, vf);
 
+	/* Go through all the checks to see if the VLAN filter should
+	 * be wiped completely.
+	 */
+	if (!add && adapter->netdev->flags & IFF_PROMISC) {
+		reg_ndx = ixgbe_find_vlvf_entry(hw, vid);
+		if (reg_ndx < 0)
+			goto out;
+		vlvf = IXGBE_READ_REG(hw, IXGBE_VLVF(reg_ndx));
+		/* See if any other pools are set for this VLAN filter
+		 * entry other than the PF.
+		 */
+		if (VMDQ_P(0) < 32) {
+			bits = IXGBE_READ_REG(hw, IXGBE_VLVFB(reg_ndx * 2));
+			bits &= ~(1 << VMDQ_P(0));
+			bits |= IXGBE_READ_REG(hw,
+					       IXGBE_VLVFB(reg_ndx * 2) + 1);
+		} else {
+			bits = IXGBE_READ_REG(hw,
+					      IXGBE_VLVFB(reg_ndx * 2) + 1);
+			bits &= ~(1 << (VMDQ_P(0) - 32));
+			bits |= IXGBE_READ_REG(hw, IXGBE_VLVFB(reg_ndx * 2));
+		}
+
+		/* If the filter was removed then ensure PF pool bit
+		 * is cleared if the PF only added itself to the pool
+		 * because the PF is in promiscuous mode.
+		 */
+		if ((vlvf && VLAN_VID_MASK) == vid &&
+		    !test_bit(vid, adapter->active_vlans) && !bits)
+			ixgbe_set_vf_vlan(adapter, add, vid, VMDQ_P(0));
+	}
+
+out:
+
 	return err;
 }
 
-- 
1.7.11.7

[net-next 02/10] ixgbe: Use pci_vfs_assigned instead of ixgbe_vfs_are_assigned

[Jeff Kirsher]

From: Alexander Duyck <alexander.h.duyck@intel.com>

This change makes it so that the ixgbe driver uses the generic helper
pci_vfs_assigned instead of the ixgbe specific function
ixgbe_vfs_are_assigned.

Signed-off-by: Alexander Duyck <alexander.h.duyck@intel.com>
Tested-by: Stephen Ko <stephen.s.ko@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
 drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.c | 35 +-------------------------
 1 file changed, 1 insertion(+), 34 deletions(-)

diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.c
index a85d5f4..6369fb9 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.c
@@ -173,39 +173,6 @@ void ixgbe_enable_sriov(struct ixgbe_adapter *adapter)
 	ixgbe_disable_sriov(adapter);
 }
 
-static bool ixgbe_vfs_are_assigned(struct ixgbe_adapter *adapter)
-{
-	struct pci_dev *pdev = adapter->pdev;
-	struct pci_dev *vfdev;
-	int dev_id;
-
-	switch (adapter->hw.mac.type) {
-	case ixgbe_mac_82599EB:
-		dev_id = IXGBE_DEV_ID_82599_VF;
-		break;
-	case ixgbe_mac_X540:
-		dev_id = IXGBE_DEV_ID_X540_VF;
-		break;
-	default:
-		return false;
-	}
-
-	/* loop through all the VFs to see if we own any that are assigned */
-	vfdev = pci_get_device(PCI_VENDOR_ID_INTEL, dev_id, NULL);
-	while (vfdev) {
-		/* if we don't own it we don't care */
-		if (vfdev->is_virtfn && vfdev->physfn == pdev) {
-			/* if it is assigned we cannot release it */
-			if (vfdev->dev_flags & PCI_DEV_FLAGS_ASSIGNED)
-				return true;
-		}
-
-		vfdev = pci_get_device(PCI_VENDOR_ID_INTEL, dev_id, vfdev);
-	}
-
-	return false;
-}
-
 #endif /* #ifdef CONFIG_PCI_IOV */
 int ixgbe_disable_sriov(struct ixgbe_adapter *adapter)
 {
@@ -235,7 +202,7 @@ int ixgbe_disable_sriov(struct ixgbe_adapter *adapter)
 	 * without causing issues, so just leave the hardware
 	 * available but disabled
 	 */
-	if (ixgbe_vfs_are_assigned(adapter)) {
+	if (pci_vfs_assigned(adapter->pdev)) {
 		e_dev_warn("Unloading driver while VFs are assigned - VFs will not be deallocated\n");
 		return -EPERM;
 	}
-- 
1.7.11.7

[net-next 03/10] ixgbe: Fix typo

[Jeff Kirsher]

From: Greg Rose <gregory.v.rose@intel.com>

Fix a typo where the code should use a bitwise "and" rather than a logical
"and".

Signed-off-by: Greg Rose <gregory.v.rose@intel.com>
Tested-by: Stephen Ko <stephen.s.ko@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
 drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.c
index 6369fb9..73c8e73 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.c
@@ -820,7 +820,7 @@ static int ixgbe_set_vf_vlan_msg(struct ixgbe_adapter *adapter,
 		 * is cleared if the PF only added itself to the pool
 		 * because the PF is in promiscuous mode.
 		 */
-		if ((vlvf && VLAN_VID_MASK) == vid &&
+		if ((vlvf & VLAN_VID_MASK) == vid &&
 		    !test_bit(vid, adapter->active_vlans) && !bits)
 			ixgbe_set_vf_vlan(adapter, add, vid, VMDQ_P(0));
 	}
-- 
1.7.11.7

[net-next 04/10] e100: dump small buffers via %*ph

[Jeff Kirsher]

From: Andy Shevchenko <andriy.shevchenko@linux.intel.com>

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Tested-by: Aaron Brown <aaron.f.brown@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
 drivers/net/ethernet/intel/e100.c | 15 ++++++---------
 1 file changed, 6 insertions(+), 9 deletions(-)

diff --git a/drivers/net/ethernet/intel/e100.c b/drivers/net/ethernet/intel/e100.c
index d2bea3f..68849cc 100644
--- a/drivers/net/ethernet/intel/e100.c
+++ b/drivers/net/ethernet/intel/e100.c
@@ -1175,15 +1175,12 @@ static int e100_configure(struct nic *nic, struct cb *cb, struct sk_buff *skb)
 		config->rx_discard_short_frames = 0x0;  /* 1=discard, 0=save */
 	}
 
-	netif_printk(nic, hw, KERN_DEBUG, nic->netdev,
-		     "[00-07]=%02X:%02X:%02X:%02X:%02X:%02X:%02X:%02X\n",
-		     c[0], c[1], c[2], c[3], c[4], c[5], c[6], c[7]);
-	netif_printk(nic, hw, KERN_DEBUG, nic->netdev,
-		     "[08-15]=%02X:%02X:%02X:%02X:%02X:%02X:%02X:%02X\n",
-		     c[8], c[9], c[10], c[11], c[12], c[13], c[14], c[15]);
-	netif_printk(nic, hw, KERN_DEBUG, nic->netdev,
-		     "[16-23]=%02X:%02X:%02X:%02X:%02X:%02X:%02X:%02X\n",
-		     c[16], c[17], c[18], c[19], c[20], c[21], c[22], c[23]);
+	netif_printk(nic, hw, KERN_DEBUG, nic->netdev, "[00-07]=%8ph\n",
+		     c + 0);
+	netif_printk(nic, hw, KERN_DEBUG, nic->netdev, "[08-15]=%8ph\n",
+		     c + 8);
+	netif_printk(nic, hw, KERN_DEBUG, nic->netdev, "[16-23]=%8ph\n",
+		     c + 16);
 	return 0;
 }
 
-- 
1.7.11.7

[net-next 05/10] igb: Reset the link when EEE setting changed

[Jeff Kirsher]

From: "Akeem G. Abodunrin" <akeem.g.abodunrin@intel.com>

This patch resets the link, if link is up - whenever users enable or disable EEE

Signed-off-by: Akeem G. Abodunrin <akeem.g.abodunrin@intel.com>
Tested-by: Aaron Brown <aaron.f.brown@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
 drivers/net/ethernet/intel/igb/igb_ethtool.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/intel/igb/igb_ethtool.c b/drivers/net/ethernet/intel/igb/igb_ethtool.c
index 85fe7b5..6d861a5 100644
--- a/drivers/net/ethernet/intel/igb/igb_ethtool.c
+++ b/drivers/net/ethernet/intel/igb/igb_ethtool.c
@@ -2672,7 +2672,9 @@ static int igb_set_eee(struct net_device *netdev,
 		igb_set_eee_i350(hw);
 
 		/* reset link */
-		if (!netif_running(netdev))
+		if (netif_running(netdev))
+			igb_reinit_locked(adapter);
+		else
 			igb_reset(adapter);
 	}
 
-- 
1.7.11.7

[net-next 06/10] igb: Read register for latch_on without return value

[Jeff Kirsher]

From: "Akeem G. Abodunrin" <akeem.g.abodunrin@intel.com>

This patch changes register read to "just-read" without returning a value
for hardware to accurately latch the register value.

Signed-off-by: Akeem G Abodunrin <akeem.g.abodunrin@intel.com>
Tested-by: Aaron Brown <aaron.f.brown@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
 drivers/net/ethernet/intel/igb/e1000_82575.c |  9 ++++-----
 drivers/net/ethernet/intel/igb/igb_main.c    | 14 ++++----------
 drivers/net/ethernet/intel/igb/igb_ptp.c     |  8 ++++----
 3 files changed, 12 insertions(+), 19 deletions(-)

diff --git a/drivers/net/ethernet/intel/igb/e1000_82575.c b/drivers/net/ethernet/intel/igb/e1000_82575.c
index f21a91a..9057d10 100644
--- a/drivers/net/ethernet/intel/igb/e1000_82575.c
+++ b/drivers/net/ethernet/intel/igb/e1000_82575.c
@@ -1320,7 +1320,7 @@ void igb_shutdown_serdes_link_82575(struct e1000_hw *hw)
  **/
 static s32 igb_reset_hw_82575(struct e1000_hw *hw)
 {
-	u32 ctrl, icr;
+	u32 ctrl;
 	s32 ret_val;
 
 	/* Prevent the PCI-E bus from sticking if there is no TLP connection
@@ -1365,7 +1365,7 @@ static s32 igb_reset_hw_82575(struct e1000_hw *hw)
 
 	/* Clear any pending interrupt events. */
 	wr32(E1000_IMC, 0xffffffff);
-	icr = rd32(E1000_ICR);
+	rd32(E1000_ICR);
 
 	/* Install any alternate MAC address into RAR0 */
 	ret_val = igb_check_alt_mac_addr(hw);
@@ -2103,10 +2103,9 @@ static s32 igb_reset_hw_82580(struct e1000_hw *hw)
 	s32 ret_val = 0;
 	/* BH SW mailbox bit in SW_FW_SYNC */
 	u16 swmbsw_mask = E1000_SW_SYNCH_MB;
-	u32 ctrl, icr;
+	u32 ctrl;
 	bool global_device_reset = hw->dev_spec._82575.global_device_reset;
 
-
 	hw->dev_spec._82575.global_device_reset = false;
 
 	/* due to hw errata, global device reset doesn't always
@@ -2165,7 +2164,7 @@ static s32 igb_reset_hw_82580(struct e1000_hw *hw)
 
 	/* Clear any pending interrupt events. */
 	wr32(E1000_IMC, 0xffffffff);
-	icr = rd32(E1000_ICR);
+	rd32(E1000_ICR);
 
 	ret_val = igb_reset_mdicnfg_82580(hw);
 	if (ret_val)
diff --git a/drivers/net/ethernet/intel/igb/igb_main.c b/drivers/net/ethernet/intel/igb/igb_main.c
index 6a0c1b6..7112e52 100644
--- a/drivers/net/ethernet/intel/igb/igb_main.c
+++ b/drivers/net/ethernet/intel/igb/igb_main.c
@@ -3845,7 +3845,6 @@ bool igb_has_link(struct igb_adapter *adapter)
 {
 	struct e1000_hw *hw = &adapter->hw;
 	bool link_active = false;
-	s32 ret_val = 0;
 
 	/* get_link_status is set on LSC (link status) interrupt or
 	 * rx sequence error interrupt.  get_link_status will stay
@@ -3854,16 +3853,11 @@ bool igb_has_link(struct igb_adapter *adapter)
 	 */
 	switch (hw->phy.media_type) {
 	case e1000_media_type_copper:
-		if (hw->mac.get_link_status) {
-			ret_val = hw->mac.ops.check_for_link(hw);
-			link_active = !hw->mac.get_link_status;
-		} else {
-			link_active = true;
-		}
-		break;
+		if (!hw->mac.get_link_status)
+			return true;
 	case e1000_media_type_internal_serdes:
-		ret_val = hw->mac.ops.check_for_link(hw);
-		link_active = hw->mac.serdes_has_link;
+		hw->mac.ops.check_for_link(hw);
+		link_active = !hw->mac.get_link_status;
 		break;
 	default:
 	case e1000_media_type_unknown:
diff --git a/drivers/net/ethernet/intel/igb/igb_ptp.c b/drivers/net/ethernet/intel/igb/igb_ptp.c
index 7e8c477..5a54e3d 100644
--- a/drivers/net/ethernet/intel/igb/igb_ptp.c
+++ b/drivers/net/ethernet/intel/igb/igb_ptp.c
@@ -97,14 +97,14 @@ static cycle_t igb_ptp_read_82580(const struct cyclecounter *cc)
 {
 	struct igb_adapter *igb = container_of(cc, struct igb_adapter, cc);
 	struct e1000_hw *hw = &igb->hw;
+	u32 lo, hi;
 	u64 val;
-	u32 lo, hi, jk;
 
 	/* The timestamp latches on lowest register read. For the 82580
 	 * the lowest register is SYSTIMR instead of SYSTIML.  However we only
 	 * need to provide nanosecond resolution, so we just ignore it.
 	 */
-	jk = rd32(E1000_SYSTIMR);
+	rd32(E1000_SYSTIMR);
 	lo = rd32(E1000_SYSTIML);
 	hi = rd32(E1000_SYSTIMH);
 
@@ -118,13 +118,13 @@ static cycle_t igb_ptp_read_82580(const struct cyclecounter *cc)
 static void igb_ptp_read_i210(struct igb_adapter *adapter, struct timespec *ts)
 {
 	struct e1000_hw *hw = &adapter->hw;
-	u32 sec, nsec, jk;
+	u32 sec, nsec;
 
 	/* The timestamp latches on lowest register read. For I210/I211, the
 	 * lowest register is SYSTIMR. Since we only need to provide nanosecond
 	 * resolution, we can ignore it.
 	 */
-	jk = rd32(E1000_SYSTIMR);
+	rd32(E1000_SYSTIMR);
 	nsec = rd32(E1000_SYSTIML);
 	sec = rd32(E1000_SYSTIMH);
 
-- 
1.7.11.7

[net-next 07/10] igb: Added rcu_lock to avoid race

[Jeff Kirsher]

From: "Akeem G. Abodunrin" <akeem.g.abodunrin@intel.com>

This patch adds rcu_lock to avoid possible race condition with igb_update_stats
function accessing the rings in free_ q_vector.

Signed-off-by: Akeem G Abodunrin <akeem.g.abodunrin@intel.com>
Tested-by: Aaron Brown <aaron.f.brown@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
 drivers/net/ethernet/intel/igb/igb_main.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/intel/igb/igb_main.c b/drivers/net/ethernet/intel/igb/igb_main.c
index 7112e52..c74ad78 100644
--- a/drivers/net/ethernet/intel/igb/igb_main.c
+++ b/drivers/net/ethernet/intel/igb/igb_main.c
@@ -705,6 +705,8 @@ static void __exit igb_exit_module(void)
 	dca_unregister_notify(&dca_notifier);
 #endif
 	pci_unregister_driver(&igb_driver);
+
+	rcu_barrier(); /* Wait for completion of call_rcu()'s */
 }
 
 module_exit(igb_exit_module);
@@ -1013,7 +1015,7 @@ static void igb_free_q_vector(struct igb_adapter *adapter, int v_idx)
 	adapter->q_vector[v_idx] = NULL;
 	netif_napi_del(&q_vector->napi);
 
-	/* ixgbe_get_stats64() might access the rings on this vector,
+	/* igb_get_stats64() might access the rings on this vector,
 	 * we must wait a grace period before freeing it.
 	 */
 	kfree_rcu(q_vector, rcu);
@@ -4860,6 +4862,8 @@ void igb_update_stats(struct igb_adapter *adapter,
 
 	bytes = 0;
 	packets = 0;
+
+	rcu_read_lock();
 	for (i = 0; i < adapter->num_rx_queues; i++) {
 		u32 rqdpc = rd32(E1000_RQDPC(i));
 		struct igb_ring *ring = adapter->rx_ring[i];
@@ -4895,6 +4899,7 @@ void igb_update_stats(struct igb_adapter *adapter,
 	}
 	net_stats->tx_bytes = bytes;
 	net_stats->tx_packets = packets;
+	rcu_read_unlock();
 
 	/* read stats registers */
 	adapter->stats.crcerrs += rd32(E1000_CRCERRS);
-- 
1.7.11.7

[net-next 08/10] igb: don't allow SR-IOV without MSI-X

[Jeff Kirsher]

From: Mitch A Williams <mitch.a.williams@intel.com>

MSI-X interrupts are required for SR-IOV operation. Check to make sure
they're enabled before allowing the user to turn on VFs.

Signed-off-by: Mitch Williams <mitch.a.williams@intel.com>
Tested-by: Aaron Brown <aaron.f.brown@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
 drivers/net/ethernet/intel/igb/igb_main.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/drivers/net/ethernet/intel/igb/igb_main.c b/drivers/net/ethernet/intel/igb/igb_main.c
index c74ad78..35d31c6 100644
--- a/drivers/net/ethernet/intel/igb/igb_main.c
+++ b/drivers/net/ethernet/intel/igb/igb_main.c
@@ -2438,6 +2438,11 @@ static int igb_enable_sriov(struct pci_dev *pdev, int num_vfs)
 	int err = 0;
 	int i;
 
+	if (!adapter->msix_entries) {
+		err = -EPERM;
+		goto out;
+	}
+
 	if (!num_vfs)
 		goto out;
 	else if (old_vfs && old_vfs == num_vfs)
-- 
1.7.11.7

[net-next 09/10] e1000e: restore call to pci_clear_master()

[Jeff Kirsher]

From: Dean Nelson <dnelson@redhat.com>

In attempting to resolve a minor merge conflict, commit e5f2ef7ab4690d2e8faa
(Merge git://git.kernel.org/pub/scm/linux/kernel/git/davem/net) accidently
dropped a call to pci_clear_master() that was intended to remain in place.

Commit 4e0855dff094b0d56d6b (e1000e: fix pci-device enable-counter balance)
replaced a call to pci_disable_device() by one to pci_clear_master(). And then
commit 66148babe728f3e00e13 (e1000e: fix runtime power management transitions)
deleted a number of lines starting two lines following that call.

This patch restores the call to pci_clear_master() in __e1000_shutdown().

v2: added summary lines (enclosed in parens) following commit IDs

Signed-off-by: Dean Nelson <dnelson@redhat.com>
Tested-by: Aaron Brown <aaron.f.brown@intel.com>
Acked-by: Bruce Allan <bruce.w.allan@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
 drivers/net/ethernet/intel/e1000e/netdev.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/net/ethernet/intel/e1000e/netdev.c b/drivers/net/ethernet/intel/e1000e/netdev.c
index 77f81cb..5475cf4 100644
--- a/drivers/net/ethernet/intel/e1000e/netdev.c
+++ b/drivers/net/ethernet/intel/e1000e/netdev.c
@@ -5995,6 +5995,8 @@ static int __e1000_shutdown(struct pci_dev *pdev, bool runtime)
 	 */
 	e1000e_release_hw_control(adapter);
 
+	pci_clear_master(pdev);
+
 	/* The pci-e switch on some quad port adapters will report a
 	 * correctable error when the MAC transitions from D0 to D3.  To
 	 * prevent this we need to mask off the correctable errors on the
-- 
1.7.11.7

[net-next 10/10] e1000e: Remove duplicate assignment of default rx/tx ring size

[Jeff Kirsher]

From: Wei Yang <weiyang@linux.vnet.ibm.com>

tx_ring/rx_ring size is assigned in function e1000_alloc_queues(), which is
called by e1000_sw_init() in the early stage of e1000_probe().

This patch just remove the duplicate assignment of this default ring size
value.

Signed-off-by: Wei Yang <weiyang@linux.vnet.ibm.com>
Reviewed-by: Gavin Shan <shangw@linux.vnet.ibm.com>
Reviewed-by: Da Yu Qiu <qiudayu@cn.ibm.com>
Tested-by: Aaron Brown <aaron.f.brown@intel.com>
Acked-by: Bruce Allan <bruce.w.allan@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
 drivers/net/ethernet/intel/e1000e/netdev.c | 4 ----
 1 file changed, 4 deletions(-)

diff --git a/drivers/net/ethernet/intel/e1000e/netdev.c b/drivers/net/ethernet/intel/e1000e/netdev.c
index 5475cf4..40e6232 100644
--- a/drivers/net/ethernet/intel/e1000e/netdev.c
+++ b/drivers/net/ethernet/intel/e1000e/netdev.c
@@ -6725,10 +6725,6 @@ static int e1000_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
 	adapter->hw.fc.current_mode = e1000_fc_default;
 	adapter->hw.phy.autoneg_advertised = 0x2f;
 
-	/* ring size defaults */
-	adapter->rx_ring->count = E1000_DEFAULT_RXD;
-	adapter->tx_ring->count = E1000_DEFAULT_TXD;
-
 	/* Initial Wake on LAN setting - If APM wake is enabled in
 	 * the EEPROM, enable the ACPI Magic Packet filter
 	 */
-- 
1.7.11.7

Re: [net-next 07/10] igb: Added rcu_lock to avoid race

[Eric Dumazet]

On Wed, 2013-06-26 at 03:55 -0700, Jeff Kirsher wrote:
> From: "Akeem G. Abodunrin" <akeem.g.abodunrin@intel.com>
> 
> This patch adds rcu_lock to avoid possible race condition with igb_update_stats
> function accessing the rings in free_ q_vector.
> 
> Signed-off-by: Akeem G Abodunrin <akeem.g.abodunrin@intel.com>
> Tested-by: Aaron Brown <aaron.f.brown@intel.com>
> Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
> ---
>  drivers/net/ethernet/intel/igb/igb_main.c | 7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/net/ethernet/intel/igb/igb_main.c b/drivers/net/ethernet/intel/igb/igb_main.c
> index 7112e52..c74ad78 100644
> --- a/drivers/net/ethernet/intel/igb/igb_main.c
> +++ b/drivers/net/ethernet/intel/igb/igb_main.c
> @@ -705,6 +705,8 @@ static void __exit igb_exit_module(void)
>  	dca_unregister_notify(&dca_notifier);
>  #endif
>  	pci_unregister_driver(&igb_driver);
> +
> +	rcu_barrier(); /* Wait for completion of call_rcu()'s */
>  }
>  

This is not needed.

If this is really needed, a better comment would be welcomed ;)


>  module_exit(igb_exit_module);
> @@ -1013,7 +1015,7 @@ static void igb_free_q_vector(struct igb_adapter *adapter, int v_idx)
>  	adapter->q_vector[v_idx] = NULL;
>  	netif_napi_del(&q_vector->napi);
>  
> -	/* ixgbe_get_stats64() might access the rings on this vector,
> +	/* igb_get_stats64() might access the rings on this vector,
>  	 * we must wait a grace period before freeing it.
>  	 */
>  	kfree_rcu(q_vector, rcu);
> @@ -4860,6 +4862,8 @@ void igb_update_stats(struct igb_adapter *adapter,
>  
>  	bytes = 0;
>  	packets = 0;
> +
> +	rcu_read_lock();
>  	for (i = 0; i < adapter->num_rx_queues; i++) {
>  		u32 rqdpc = rd32(E1000_RQDPC(i));
>  		struct igb_ring *ring = adapter->rx_ring[i];
> @@ -4895,6 +4899,7 @@ void igb_update_stats(struct igb_adapter *adapter,
>  	}
>  	net_stats->tx_bytes = bytes;
>  	net_stats->tx_packets = packets;
> +	rcu_read_unlock();
>  
>  	/* read stats registers */
>  	adapter->stats.crcerrs += rd32(E1000_CRCERRS);

Re: [net-next 03/10] ixgbe: Fix typo

[Joe Perches]

On Wed, 2013-06-26 at 03:55 -0700, Jeff Kirsher wrote:
> From: Greg Rose <gregory.v.rose@intel.com>
> 
> Fix a typo where the code should use a bitwise "and" rather than a logical
> "and".

This patch is bad because it should be rolled into 1/10
instead of sent as a separate patch.

I also think the subject line is poor.

It sounds like it's just a misspelling in a comment
or a dmesg instead of a logical error.

RE: [net-next 07/10] igb: Added rcu_lock to avoid race

[Abodunrin, Akeem G]

> -----Original Message-----
> From: Eric Dumazet [mailto:eric.dumazet@gmail.com]
> Sent: Wednesday, June 26, 2013 4:20 AM
> To: Kirsher, Jeffrey T
> Cc: davem@davemloft.net; Abodunrin, Akeem G; netdev@vger.kernel.org;
> gospo@redhat.com; sassmann@redhat.com
> Subject: Re: [net-next 07/10] igb: Added rcu_lock to avoid race
> 
> On Wed, 2013-06-26 at 03:55 -0700, Jeff Kirsher wrote:
> > From: "Akeem G. Abodunrin" <akeem.g.abodunrin@intel.com>
> >
> > This patch adds rcu_lock to avoid possible race condition with
> > igb_update_stats function accessing the rings in free_ q_vector.
> >
> > Signed-off-by: Akeem G Abodunrin <akeem.g.abodunrin@intel.com>
> > Tested-by: Aaron Brown <aaron.f.brown@intel.com>
> > Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
> > ---
> >  drivers/net/ethernet/intel/igb/igb_main.c | 7 ++++++-
> >  1 file changed, 6 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/net/ethernet/intel/igb/igb_main.c
> > b/drivers/net/ethernet/intel/igb/igb_main.c
> > index 7112e52..c74ad78 100644
> > --- a/drivers/net/ethernet/intel/igb/igb_main.c
> > +++ b/drivers/net/ethernet/intel/igb/igb_main.c
> > @@ -705,6 +705,8 @@ static void __exit igb_exit_module(void)
> >  	dca_unregister_notify(&dca_notifier);
> >  #endif
> >  	pci_unregister_driver(&igb_driver);
> > +
> > +	rcu_barrier(); /* Wait for completion of call_rcu()'s */
> >  }
> >
> 
> This is not needed.
> 
> If this is really needed, a better comment would be welcomed ;)

Thanks Eric!

I believe we need this to make sure memory is released before removing driver code from the kernel, especially with each call to rcu - that is the reason for the comment "Wait for completion of call_rcu()'s".

Regards,
~Akeem

> 
> >  module_exit(igb_exit_module);
> > @@ -1013,7 +1015,7 @@ static void igb_free_q_vector(struct igb_adapter
> *adapter, int v_idx)
> >  	adapter->q_vector[v_idx] = NULL;
> >  	netif_napi_del(&q_vector->napi);
> >
> > -	/* ixgbe_get_stats64() might access the rings on this vector,
> > +	/* igb_get_stats64() might access the rings on this vector,
> >  	 * we must wait a grace period before freeing it.
> >  	 */
> >  	kfree_rcu(q_vector, rcu);
> > @@ -4860,6 +4862,8 @@ void igb_update_stats(struct igb_adapter
> > *adapter,
> >
> >  	bytes = 0;
> >  	packets = 0;
> > +
> > +	rcu_read_lock();
> >  	for (i = 0; i < adapter->num_rx_queues; i++) {
> >  		u32 rqdpc = rd32(E1000_RQDPC(i));
> >  		struct igb_ring *ring = adapter->rx_ring[i]; @@ -4895,6 +4899,7
> @@
> > void igb_update_stats(struct igb_adapter *adapter,
> >  	}
> >  	net_stats->tx_bytes = bytes;
> >  	net_stats->tx_packets = packets;
> > +	rcu_read_unlock();
> >
> >  	/* read stats registers */
> >  	adapter->stats.crcerrs += rd32(E1000_CRCERRS);
> 

RE: [net-next 07/10] igb: Added rcu_lock to avoid race

[Eric Dumazet]

On Wed, 2013-06-26 at 16:42 +0000, Abodunrin, Akeem G wrote:
> 
> > -----Original Message-----
> > From: Eric Dumazet [mailto:eric.dumazet@gmail.com]
> > Sent: Wednesday, June 26, 2013 4:20 AM
> > To: Kirsher, Jeffrey T
> > Cc: davem@davemloft.net; Abodunrin, Akeem G; netdev@vger.kernel.org;
> > gospo@redhat.com; sassmann@redhat.com
> > Subject: Re: [net-next 07/10] igb: Added rcu_lock to avoid race
> > 
> > On Wed, 2013-06-26 at 03:55 -0700, Jeff Kirsher wrote:
> > > From: "Akeem G. Abodunrin" <akeem.g.abodunrin@intel.com>
> > >
> > > This patch adds rcu_lock to avoid possible race condition with
> > > igb_update_stats function accessing the rings in free_ q_vector.
> > >
> > > Signed-off-by: Akeem G Abodunrin <akeem.g.abodunrin@intel.com>
> > > Tested-by: Aaron Brown <aaron.f.brown@intel.com>
> > > Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
> > > ---
> > >  drivers/net/ethernet/intel/igb/igb_main.c | 7 ++++++-
> > >  1 file changed, 6 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/net/ethernet/intel/igb/igb_main.c
> > > b/drivers/net/ethernet/intel/igb/igb_main.c
> > > index 7112e52..c74ad78 100644
> > > --- a/drivers/net/ethernet/intel/igb/igb_main.c
> > > +++ b/drivers/net/ethernet/intel/igb/igb_main.c
> > > @@ -705,6 +705,8 @@ static void __exit igb_exit_module(void)
> > >  	dca_unregister_notify(&dca_notifier);
> > >  #endif
> > >  	pci_unregister_driver(&igb_driver);
> > > +
> > > +	rcu_barrier(); /* Wait for completion of call_rcu()'s */
> > >  }
> > >
> > 
> > This is not needed.
> > 
> > If this is really needed, a better comment would be welcomed ;)
> 
> Thanks Eric!
> 
> I believe we need this to make sure memory is released before removing
> driver code from the kernel, especially with each call to rcu - that
> is the reason for the comment "Wait for completion of call_rcu()'s".

You did not add call_rcu() in this patch, therefore you do not need
this.

I am extra careful of people adding useless rcu synchronization just
because they feel better :)

RE: [net-next 07/10] igb: Added rcu_lock to avoid race

[Eric Dumazet]

On Wed, 2013-06-26 at 12:52 -0700, Eric Dumazet wrote:

> 
> You did not add call_rcu() in this patch, therefore you do not need
> this.
> 
> I am extra careful of people adding useless rcu synchronization just
> because they feel better :)

BTW the rcu_barrier() in ixgbe is no longer needed as well, now it uses
kfree_rcu()