[net 0/8][pull request] Intel Wired LAN Driver Updates

[net 0/8][pull request] Intel Wired LAN Driver Updates

[Jeff Kirsher]

The following series contains fixes to e1000, igb and ixgbe.
The e1000 fix comes from Red Hat, where a previous commit removed
code which broke WoL and this patch rectifies that. The igb patch
fixes an issue with VF lookup.  The remaining patches in the series
are against ixgbe and fix the following:
 - VF lookup and Tx hang with 32 VFs
 - broken dependency on MAX_SKB_FRAGS
 - ethtool buffer overrun in stats
 - DCB fix for up2tc mapping being lost on disable/enable

The following are changes since commit a1728800bed3b93b231d99e97c756f622b9991c2:
  net: enable TC35815 for MIPS again
and are available in the git repository at:
  git://git.kernel.org/pub/scm/linux/kernel/git/jkirsher/net master

Alexander Duyck (1):
  ixgbe: Fix broken dependency on MAX_SKB_FRAGS being related to page
    size

Dean Nelson (1):
  e1000: add dropped DMA receive enable back in for WoL

Greg Rose (3):
  igb: fix vf lookup
  ixgbe: fix vf lookup
  ixgbe: Fix case of Tx Hang in PF with 32 VFs

John Fastabend (2):
  ixgbe: dcb: up2tc mapping lost on disable/enable CEE DCB state
  ixgbe: ethtool: stats user buffer overrun

Yi Zou (1):
  ixgbe: do not update real num queues when netdev is going away

 drivers/net/ethernet/intel/e1000/e1000_main.c    |   10 ++-
 drivers/net/ethernet/intel/igb/igb_main.c        |    3 +-
 drivers/net/ethernet/intel/ixgbe/ixgbe_dcb_nl.c  |   11 ++-
 drivers/net/ethernet/intel/ixgbe/ixgbe_ethtool.c |   88 +++++++++++++---------
 drivers/net/ethernet/intel/ixgbe/ixgbe_main.c    |   18 +++--
 drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.c   |    3 +-
 6 files changed, 81 insertions(+), 52 deletions(-)

-- 
1.7.7.6

[net 1/8] e1000: add dropped DMA receive enable back in for WoL

[Jeff Kirsher]

From: Dean Nelson <dnelson@redhat.com>

Commit d5bc77a223b0e9b9dfb002048d2b34a79e7d0b48 broke Wake-on-LAN by
inadvertently dropping the enabling of DMA receives.

Restore the enabling of DMA receives for WoL.

This is applicable to 3.1+ stable trees.

CC: stable@vger.stable.org
Reported-by: Tobias Klausmann <klausman@schwarzvogel.de>
Signed-off-by: Dean Nelson <dnelson@redhat.com>
Tested-by: Tobias Klausmann <klausman@schwarzvogel.de>
Tested-by: Aaron Brown <aaron.f.brown@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
 drivers/net/ethernet/intel/e1000/e1000_main.c |   10 ++++++----
 1 files changed, 6 insertions(+), 4 deletions(-)

diff --git a/drivers/net/ethernet/intel/e1000/e1000_main.c b/drivers/net/ethernet/intel/e1000/e1000_main.c
index 669ca38..d94d64b 100644
--- a/drivers/net/ethernet/intel/e1000/e1000_main.c
+++ b/drivers/net/ethernet/intel/e1000/e1000_main.c
@@ -4740,12 +4740,14 @@ static int __e1000_shutdown(struct pci_dev *pdev, bool *enable_wake)
 		e1000_setup_rctl(adapter);
 		e1000_set_rx_mode(netdev);
 
+		rctl = er32(RCTL);
+
 		/* turn on all-multi mode if wake on multicast is enabled */
-		if (wufc & E1000_WUFC_MC) {
-			rctl = er32(RCTL);
+		if (wufc & E1000_WUFC_MC)
 			rctl |= E1000_RCTL_MPE;
-			ew32(RCTL, rctl);
-		}
+
+		/* enable receives in the hardware */
+		ew32(RCTL, rctl | E1000_RCTL_EN);
 
 		if (hw->mac_type >= e1000_82540) {
 			ctrl = er32(CTRL);
-- 
1.7.7.6

[net 2/8] igb: fix vf lookup

[Jeff Kirsher]

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

Recent addition of code to find already allocated VFs failed to take
account that systems with 2 or more multi-port SR-IOV capable controllers
might have already enabled VFs.  Make sure that the VFs the function is
finding are actually subordinate to the particular instance of the adapter
that is looking for them and not subordinate to some device that has
previously enabled SR-IOV.

This is applicable to 3.2+ kernels.

CC: stable@vger.kernel.org
Reported-by: David Ahern <daahern@cisco.com>
Signed-off-by: Greg Rose <gregory.v.rose@intel.com>
Tested-by: Robert E Garrett <robertX.e.garrett@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
 drivers/net/ethernet/intel/igb/igb_main.c |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/drivers/net/ethernet/intel/igb/igb_main.c b/drivers/net/ethernet/intel/igb/igb_main.c
index e91d73c..94be6c3 100644
--- a/drivers/net/ethernet/intel/igb/igb_main.c
+++ b/drivers/net/ethernet/intel/igb/igb_main.c
@@ -5012,7 +5012,8 @@ static int igb_find_enabled_vfs(struct igb_adapter *adapter)
 	vf_devfn = pdev->devfn + 0x80;
 	pvfdev = pci_get_device(hw->vendor_id, device_id, NULL);
 	while (pvfdev) {
-		if (pvfdev->devfn == vf_devfn)
+		if (pvfdev->devfn == vf_devfn &&
+		    (pvfdev->bus->number >= pdev->bus->number))
 			vfs_found++;
 		vf_devfn += vf_stride;
 		pvfdev = pci_get_device(hw->vendor_id,
-- 
1.7.7.6

[net 3/8] ixgbe: fix vf lookup

[Jeff Kirsher]

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

Recent addition of code to find already allocated VFs failed to take
account that systems with 2 or more multi-port SR-IOV capable controllers
might have already enabled VFs.  Make sure that the VFs the function is
finding are actually subordinate to the particular instance of the adapter
that is looking for them and not subordinate to some device that has
previously enabled SR-IOV.

This bug exists in 3.2 stable as well as 3.3 release candidates.

CC: stable@vger.kernel.org
Reported-by: David Ahern <daahern@cisco.com>
Signed-off-by: Greg Rose <gregory.v.rose@intel.com>
Tested-by: Robert E Garrett <robertX.e.garrett@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
 drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.c |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.c
index 8d8cdbc..b982710 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.c
@@ -67,7 +67,8 @@ static int ixgbe_find_enabled_vfs(struct ixgbe_adapter *adapter)
 	vf_devfn = pdev->devfn + 0x80;
 	pvfdev = pci_get_device(IXGBE_INTEL_VENDOR_ID, device_id, NULL);
 	while (pvfdev) {
-		if (pvfdev->devfn == vf_devfn)
+		if (pvfdev->devfn == vf_devfn &&
+				(pvfdev->bus->number >= pdev->bus->number))
 			vfs_found++;
 		vf_devfn += 2;
 		pvfdev = pci_get_device(IXGBE_INTEL_VENDOR_ID,
-- 
1.7.7.6

[net 4/8] ixgbe: Fix case of Tx Hang in PF with 32 VFs

[Jeff Kirsher]

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

A check for the number of VFs allocated should have used a greater than
equal operator instead of just greater than.  This caused allocation of
exactly 32 VFs to not enable the PF transmit and receive enables.

Signed-off-by: Greg Rose <gregory.v.rose@intel.com>
Tested-by: Robert E Garrett <robertX.e.garrett@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
 drivers/net/ethernet/intel/ixgbe/ixgbe_main.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
index 92192c6..819b5c0 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
@@ -2830,7 +2830,7 @@ static void ixgbe_configure_virtualization(struct ixgbe_adapter *adapter)
 	IXGBE_WRITE_REG(hw, IXGBE_VT_CTL, vmdctl | vt_reg_bits);
 
 	vf_shift = adapter->num_vfs % 32;
-	reg_offset = (adapter->num_vfs > 32) ? 1 : 0;
+	reg_offset = (adapter->num_vfs >= 32) ? 1 : 0;
 
 	/* Enable only the PF's pool for Tx/Rx */
 	IXGBE_WRITE_REG(hw, IXGBE_VFRE(reg_offset), (1 << vf_shift));
-- 
1.7.7.6

[net 5/8] ixgbe: Fix broken dependency on MAX_SKB_FRAGS being related to page size

[Jeff Kirsher]

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

This patch fixes an issue in which RSC will generate corrupted frames when
PAGE_SIZE is larger than 8K.  Specifically it looks like that in 2.6.39 a
change was made so that GRO would always have at least 16 frags available
for coalescing, but the ixgbe RSC logic was not updated.  As such the RSC
feature would generate a frame larger than 64K and then overflow the value
in the IP length field.

To correct that I am now basing things on the PAGE_SIZE.

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_main.c |   12 ++++++------
 1 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
index 819b5c0..0688704 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
@@ -2633,22 +2633,22 @@ static void ixgbe_configure_rscctl(struct ixgbe_adapter *adapter,
 	/*
 	 * we must limit the number of descriptors so that the
 	 * total size of max desc * buf_len is not greater
-	 * than 65535
+	 * than 65536
 	 */
 	if (ring_is_ps_enabled(ring)) {
-#if (MAX_SKB_FRAGS > 16)
+#if (PAGE_SIZE < 8192)
 		rscctrl |= IXGBE_RSCCTL_MAXDESC_16;
-#elif (MAX_SKB_FRAGS > 8)
+#elif (PAGE_SIZE < 16384)
 		rscctrl |= IXGBE_RSCCTL_MAXDESC_8;
-#elif (MAX_SKB_FRAGS > 4)
+#elif (PAGE_SIZE < 32768)
 		rscctrl |= IXGBE_RSCCTL_MAXDESC_4;
 #else
 		rscctrl |= IXGBE_RSCCTL_MAXDESC_1;
 #endif
 	} else {
-		if (rx_buf_len < IXGBE_RXBUFFER_4K)
+		if (rx_buf_len <= IXGBE_RXBUFFER_4K)
 			rscctrl |= IXGBE_RSCCTL_MAXDESC_16;
-		else if (rx_buf_len < IXGBE_RXBUFFER_8K)
+		else if (rx_buf_len <= IXGBE_RXBUFFER_8K)
 			rscctrl |= IXGBE_RSCCTL_MAXDESC_8;
 		else
 			rscctrl |= IXGBE_RSCCTL_MAXDESC_4;
-- 
1.7.7.6

[net 6/8] ixgbe: do not update real num queues when netdev is going away

[Jeff Kirsher]

From: Yi Zou <yi.zou@intel.com>

If the netdev is already in NETREG_UNREGISTERING/_UNREGISTERED state, do not
update the real num tx queues. netdev_queue_update_kobjects() is already
called via remove_queue_kobjects() at NETREG_UNREGISTERING time. So, when
upper layer driver, e.g., FCoE protocol stack is monitoring the netdev
event of NETDEV_UNREGISTER and calls back to LLD ndo_fcoe_disable() to remove
extra queues allocated for FCoE, the associated txq sysfs kobjects are already
removed, and trying to update the real num queues would cause something like
below:

...
PID: 25138  TASK: ffff88021e64c440  CPU: 3   COMMAND: "kworker/3:3"
 #0 [ffff88021f007760] machine_kexec at ffffffff810226d9
 #1 [ffff88021f0077d0] crash_kexec at ffffffff81089d2d
 #2 [ffff88021f0078a0] oops_end at ffffffff813bca78
 #3 [ffff88021f0078d0] no_context at ffffffff81029e72
 #4 [ffff88021f007920] __bad_area_nosemaphore at ffffffff8102a155
 #5 [ffff88021f0079f0] bad_area_nosemaphore at ffffffff8102a23e
 #6 [ffff88021f007a00] do_page_fault at ffffffff813bf32e
 #7 [ffff88021f007b10] page_fault at ffffffff813bc045
    [exception RIP: sysfs_find_dirent+17]
    RIP: ffffffff81178611  RSP: ffff88021f007bc0  RFLAGS: 00010246
    RAX: ffff88021e64c440  RBX: ffffffff8156cc63  RCX: 0000000000000004
    RDX: ffffffff8156cc63  RSI: 0000000000000000  RDI: 0000000000000000
    RBP: ffff88021f007be0   R8: 0000000000000004   R9: 0000000000000008
    R10: ffffffff816fed00  R11: 0000000000000004  R12: 0000000000000000
    R13: ffffffff8156cc63  R14: 0000000000000000  R15: ffff8802222a0000
    ORIG_RAX: ffffffffffffffff  CS: 0010  SS: 0018
 #8 [ffff88021f007be8] sysfs_get_dirent at ffffffff81178c07
 #9 [ffff88021f007c18] sysfs_remove_group at ffffffff8117ac27
#10 [ffff88021f007c48] netdev_queue_update_kobjects at ffffffff813178f9
#11 [ffff88021f007c88] netif_set_real_num_tx_queues at ffffffff81303e38
#12 [ffff88021f007cc8] ixgbe_set_num_queues at ffffffffa0249763 [ixgbe]
#13 [ffff88021f007cf8] ixgbe_init_interrupt_scheme at ffffffffa024ea89 [ixgbe]
#14 [ffff88021f007d48] ixgbe_fcoe_disable at ffffffffa0267113 [ixgbe]
#15 [ffff88021f007d68] vlan_dev_fcoe_disable at ffffffffa014fef5 [8021q]
#16 [ffff88021f007d78] fcoe_interface_cleanup at ffffffffa02b7dfd [fcoe]
#17 [ffff88021f007df8] fcoe_destroy_work at ffffffffa02b7f08 [fcoe]
#18 [ffff88021f007e18] process_one_work at ffffffff8105d7ca
#19 [ffff88021f007e68] worker_thread at ffffffff81060513
#20 [ffff88021f007ee8] kthread at ffffffff810648b6
#21 [ffff88021f007f48] kernel_thread_helper at ffffffff813c40f4

Signed-off-by: Yi Zou <yi.zou@intel.com>
Tested-by: Ross Brattain <ross.b.brattain@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 |    4 ++++
 1 files changed, 4 insertions(+), 0 deletions(-)

diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
index 0688704..3dc6cef 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
@@ -4330,6 +4330,10 @@ static int ixgbe_set_num_queues(struct ixgbe_adapter *adapter)
 	adapter->num_tx_queues = 1;
 
 done:
+	if ((adapter->netdev->reg_state == NETREG_UNREGISTERED) ||
+	    (adapter->netdev->reg_state == NETREG_UNREGISTERING))
+		return 0;
+
 	/* Notify the stack of the (possibly) reduced queue counts. */
 	netif_set_real_num_tx_queues(adapter->netdev, adapter->num_tx_queues);
 	return netif_set_real_num_rx_queues(adapter->netdev,
-- 
1.7.7.6

[net 7/8] ixgbe: dcb: up2tc mapping lost on disable/enable CEE DCB state

[Jeff Kirsher]

From: John Fastabend <john.r.fastabend@intel.com>

Users expect the up2tc mapping to be maintained across a DCB
enable/disable/enable transition. And since we maintain all
the other DCB attributes we should do this for up2tc mappings
as well just to be consistent. Also without this we break
user space applications that expect this to occur that
previously worked.

Signed-off-by: John Fastabend <john.r.fastabend@intel.com>
Tested-by: Stephen Ko <stephen.s.ko@intel.com>
Tested-by: Ross Brattain <ross.b.brattain@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
 drivers/net/ethernet/intel/ixgbe/ixgbe_dcb_nl.c |   11 +++++++++--
 1 files changed, 9 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_dcb_nl.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_dcb_nl.c
index 005e5f2..79a92fe 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_dcb_nl.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_dcb_nl.c
@@ -112,6 +112,8 @@ static u8 ixgbe_dcbnl_get_state(struct net_device *netdev)
 static u8 ixgbe_dcbnl_set_state(struct net_device *netdev, u8 state)
 {
 	u8 err = 0;
+	u8 prio_tc[MAX_USER_PRIORITY] = {0};
+	int i;
 	struct ixgbe_adapter *adapter = netdev_priv(netdev);
 
 	/* Fail command if not in CEE mode */
@@ -122,10 +124,15 @@ static u8 ixgbe_dcbnl_set_state(struct net_device *netdev, u8 state)
 	if (!!state != !(adapter->flags & IXGBE_FLAG_DCB_ENABLED))
 		return err;
 
-	if (state > 0)
+	if (state > 0) {
 		err = ixgbe_setup_tc(netdev, adapter->dcb_cfg.num_tcs.pg_tcs);
-	else
+		ixgbe_dcb_unpack_map(&adapter->dcb_cfg, DCB_TX_CONFIG, prio_tc);
+	} else {
 		err = ixgbe_setup_tc(netdev, 0);
+	}
+
+	for (i = 0; i < IEEE_8021QAZ_MAX_TCS; i++)
+		netdev_set_prio_tc_map(netdev, i, prio_tc[i]);
 
 	return err;
 }
-- 
1.7.7.6

[net 8/8] ixgbe: ethtool: stats user buffer overrun

[Jeff Kirsher]

From: John Fastabend <john.r.fastabend@intel.com>

If the number of tx/rx queues changes the ethtool ioctl
ETHTOOL_GSTATS may overrun the userspace buffer. This
occurs because the general practice in user space to
query stats is to issue a ETHTOOL_GSSET cmd to learn the
buffer size needed, allocate the buffer, then call
ETHTOOL_GSTIRNGS and ETHTOOL_GSTATS. If the number of
real_num_queues is changed or flow control attributes
are changed after ETHTOOL_GSSET but before the
ETHTOOL_GSTRINGS/ETHTOOL_GSTATS a user space buffer
overrun occurs.

To fix the overrun always return the max buffer size
needed from get_sset_count() then return all strings
and stats from get_strings()/get_ethtool_stats().

This _will_ change the output from the ioctl() call
which could break applications and script parsing in
theory. I believe these changes should not break existing
tools because the only changes will be more {tx|rx}_queues
and the {tx|rx}_pb_* stats will always be returned.
Existing scripts already need to handle changing number
of queues because this occurs today depending on system
and current features. The {tx|rx}_pb_* stats are at the
end of the output and should be handled by scripts today
regardless.

Finally get_ethtool_stats and get_strings are free-form
outputs tools parsing these outputs should be defensive
anyways. In the end these updates are better then
having a tool segfault because of a buffer overrun.

Signed-off-by: John Fastabend <john.r.fastabend@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
 drivers/net/ethernet/intel/ixgbe/ixgbe_ethtool.c |   88 +++++++++++++---------
 1 files changed, 51 insertions(+), 37 deletions(-)

diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_ethtool.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_ethtool.c
index 1f31a04..a629754 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_ethtool.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_ethtool.c
@@ -120,19 +120,23 @@ static const struct ixgbe_stats ixgbe_gstrings_stats[] = {
 #endif /* IXGBE_FCOE */
 };
 
-#define IXGBE_QUEUE_STATS_LEN \
-	((((struct ixgbe_adapter *)netdev_priv(netdev))->num_tx_queues + \
-	((struct ixgbe_adapter *)netdev_priv(netdev))->num_rx_queues) * \
+/* ixgbe allocates num_tx_queues and num_rx_queues symmetrically so
+ * we set the num_rx_queues to evaluate to num_tx_queues. This is
+ * used because we do not have a good way to get the max number of
+ * rx queues with CONFIG_RPS disabled.
+ */
+#define IXGBE_NUM_RX_QUEUES netdev->num_tx_queues
+
+#define IXGBE_QUEUE_STATS_LEN ( \
+	(netdev->num_tx_queues + IXGBE_NUM_RX_QUEUES) * \
 	(sizeof(struct ixgbe_queue_stats) / sizeof(u64)))
 #define IXGBE_GLOBAL_STATS_LEN ARRAY_SIZE(ixgbe_gstrings_stats)
 #define IXGBE_PB_STATS_LEN ( \
-                 (((struct ixgbe_adapter *)netdev_priv(netdev))->flags & \
-                 IXGBE_FLAG_DCB_ENABLED) ? \
-                 (sizeof(((struct ixgbe_adapter *)0)->stats.pxonrxc) + \
-                  sizeof(((struct ixgbe_adapter *)0)->stats.pxontxc) + \
-                  sizeof(((struct ixgbe_adapter *)0)->stats.pxoffrxc) + \
-                  sizeof(((struct ixgbe_adapter *)0)->stats.pxofftxc)) \
-                  / sizeof(u64) : 0)
+			(sizeof(((struct ixgbe_adapter *)0)->stats.pxonrxc) + \
+			 sizeof(((struct ixgbe_adapter *)0)->stats.pxontxc) + \
+			 sizeof(((struct ixgbe_adapter *)0)->stats.pxoffrxc) + \
+			 sizeof(((struct ixgbe_adapter *)0)->stats.pxofftxc)) \
+			/ sizeof(u64))
 #define IXGBE_STATS_LEN (IXGBE_GLOBAL_STATS_LEN + \
                          IXGBE_PB_STATS_LEN + \
                          IXGBE_QUEUE_STATS_LEN)
@@ -1078,8 +1082,15 @@ static void ixgbe_get_ethtool_stats(struct net_device *netdev,
 		data[i] = (ixgbe_gstrings_stats[i].sizeof_stat ==
 		           sizeof(u64)) ? *(u64 *)p : *(u32 *)p;
 	}
-	for (j = 0; j < adapter->num_tx_queues; j++) {
+	for (j = 0; j < IXGBE_NUM_RX_QUEUES; j++) {
 		ring = adapter->tx_ring[j];
+		if (!ring) {
+			data[i] = 0;
+			data[i+1] = 0;
+			i += 2;
+			continue;
+		}
+
 		do {
 			start = u64_stats_fetch_begin_bh(&ring->syncp);
 			data[i]   = ring->stats.packets;
@@ -1087,8 +1098,15 @@ static void ixgbe_get_ethtool_stats(struct net_device *netdev,
 		} while (u64_stats_fetch_retry_bh(&ring->syncp, start));
 		i += 2;
 	}
-	for (j = 0; j < adapter->num_rx_queues; j++) {
+	for (j = 0; j < IXGBE_NUM_RX_QUEUES; j++) {
 		ring = adapter->rx_ring[j];
+		if (!ring) {
+			data[i] = 0;
+			data[i+1] = 0;
+			i += 2;
+			continue;
+		}
+
 		do {
 			start = u64_stats_fetch_begin_bh(&ring->syncp);
 			data[i]   = ring->stats.packets;
@@ -1096,22 +1114,20 @@ static void ixgbe_get_ethtool_stats(struct net_device *netdev,
 		} while (u64_stats_fetch_retry_bh(&ring->syncp, start));
 		i += 2;
 	}
-	if (adapter->flags & IXGBE_FLAG_DCB_ENABLED) {
-		for (j = 0; j < MAX_TX_PACKET_BUFFERS; j++) {
-			data[i++] = adapter->stats.pxontxc[j];
-			data[i++] = adapter->stats.pxofftxc[j];
-		}
-		for (j = 0; j < MAX_RX_PACKET_BUFFERS; j++) {
-			data[i++] = adapter->stats.pxonrxc[j];
-			data[i++] = adapter->stats.pxoffrxc[j];
-		}
+
+	for (j = 0; j < IXGBE_MAX_PACKET_BUFFERS; j++) {
+		data[i++] = adapter->stats.pxontxc[j];
+		data[i++] = adapter->stats.pxofftxc[j];
+	}
+	for (j = 0; j < IXGBE_MAX_PACKET_BUFFERS; j++) {
+		data[i++] = adapter->stats.pxonrxc[j];
+		data[i++] = adapter->stats.pxoffrxc[j];
 	}
 }
 
 static void ixgbe_get_strings(struct net_device *netdev, u32 stringset,
                               u8 *data)
 {
-	struct ixgbe_adapter *adapter = netdev_priv(netdev);
 	char *p = (char *)data;
 	int i;
 
@@ -1126,31 +1142,29 @@ static void ixgbe_get_strings(struct net_device *netdev, u32 stringset,
 			       ETH_GSTRING_LEN);
 			p += ETH_GSTRING_LEN;
 		}
-		for (i = 0; i < adapter->num_tx_queues; i++) {
+		for (i = 0; i < netdev->num_tx_queues; i++) {
 			sprintf(p, "tx_queue_%u_packets", i);
 			p += ETH_GSTRING_LEN;
 			sprintf(p, "tx_queue_%u_bytes", i);
 			p += ETH_GSTRING_LEN;
 		}
-		for (i = 0; i < adapter->num_rx_queues; i++) {
+		for (i = 0; i < IXGBE_NUM_RX_QUEUES; i++) {
 			sprintf(p, "rx_queue_%u_packets", i);
 			p += ETH_GSTRING_LEN;
 			sprintf(p, "rx_queue_%u_bytes", i);
 			p += ETH_GSTRING_LEN;
 		}
-		if (adapter->flags & IXGBE_FLAG_DCB_ENABLED) {
-			for (i = 0; i < MAX_TX_PACKET_BUFFERS; i++) {
-				sprintf(p, "tx_pb_%u_pxon", i);
-				p += ETH_GSTRING_LEN;
-				sprintf(p, "tx_pb_%u_pxoff", i);
-				p += ETH_GSTRING_LEN;
-			}
-			for (i = 0; i < MAX_RX_PACKET_BUFFERS; i++) {
-				sprintf(p, "rx_pb_%u_pxon", i);
-				p += ETH_GSTRING_LEN;
-				sprintf(p, "rx_pb_%u_pxoff", i);
-				p += ETH_GSTRING_LEN;
-			}
+		for (i = 0; i < IXGBE_MAX_PACKET_BUFFERS; i++) {
+			sprintf(p, "tx_pb_%u_pxon", i);
+			p += ETH_GSTRING_LEN;
+			sprintf(p, "tx_pb_%u_pxoff", i);
+			p += ETH_GSTRING_LEN;
+		}
+		for (i = 0; i < IXGBE_MAX_PACKET_BUFFERS; i++) {
+			sprintf(p, "rx_pb_%u_pxon", i);
+			p += ETH_GSTRING_LEN;
+			sprintf(p, "rx_pb_%u_pxoff", i);
+			p += ETH_GSTRING_LEN;
 		}
 		/* BUG_ON(p - data != IXGBE_STATS_LEN * ETH_GSTRING_LEN); */
 		break;
-- 
1.7.7.6

Re: [net 8/8] ixgbe: ethtool: stats user buffer overrun

[Ben Hutchings]

On Wed, 2012-02-08 at 01:36 -0800, Jeff Kirsher wrote:
> From: John Fastabend <john.r.fastabend@intel.com>
> 
> If the number of tx/rx queues changes the ethtool ioctl
> ETHTOOL_GSTATS may overrun the userspace buffer. This
> occurs because the general practice in user space to
> query stats is to issue a ETHTOOL_GSSET cmd to learn the
> buffer size needed, allocate the buffer, then call
> ETHTOOL_GSTIRNGS and ETHTOOL_GSTATS. If the number of
> real_num_queues is changed or flow control attributes
> are changed after ETHTOOL_GSSET but before the
> ETHTOOL_GSTRINGS/ETHTOOL_GSTATS a user space buffer
> overrun occurs.

This is a problem with several ethtool operations - the user buffer size
is implicit.

> To fix the overrun always return the max buffer size
> needed from get_sset_count() then return all strings
> and stats from get_strings()/get_ethtool_stats().
> 
> This _will_ change the output from the ioctl() call
> which could break applications and script parsing in
> theory. I believe these changes should not break existing
> tools because the only changes will be more {tx|rx}_queues
> and the {tx|rx}_pb_* stats will always be returned.
[...]

Yes, this sounds perfectly reasonable.  And this change is definitely
worth making.

However, even if the number of stats (or other attributes) for a device
never change at run-time, devices can be renamed concurrently so that
the second operation runs on a different device!

Perhaps we could change dev_ioctl so that if ifreq::ifr_name is an empty
string then the socket's bound device is used.  However, setting
SO_BINDTODEVICE currently requires CAP_NET_RAW.  Alternately, there
could be some sort of union between ifr_name and an ifindex.

Ben.

-- 
Ben Hutchings, Staff Engineer, Solarflare
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.

Re: [net 3/8] ixgbe: fix vf lookup

[David Miller]

From: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
Date: Wed,  8 Feb 2012 01:36:33 -0800

> -		if (pvfdev->devfn == vf_devfn)
> +		if (pvfdev->devfn == vf_devfn &&
> +				(pvfdev->bus->number >= pdev->bus->number))

Come on.... do you look at these patches at all?

I don't think you did, because this kind of code formatting stuff
sticks out like a sore thumb.

RE: [net 3/8] ixgbe: fix vf lookup

[Rose, Gregory V]

> -----Original Message-----
> From: David Miller [mailto:davem@davemloft.net]
> Sent: Wednesday, February 08, 2012 3:33 PM
> To: Kirsher, Jeffrey T
> Cc: Rose, Gregory V; netdev@vger.kernel.org; gospo@redhat.com;
> sassmann@redhat.com; stable@vger.kernel.org
> Subject: Re: [net 3/8] ixgbe: fix vf lookup
> 
> From: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
> Date: Wed,  8 Feb 2012 01:36:33 -0800
> 
> > -		if (pvfdev->devfn == vf_devfn)
> > +		if (pvfdev->devfn == vf_devfn &&
> > +				(pvfdev->bus->number >= pdev->bus->number))
> 
> Come on.... do you look at these patches at all?
> 
> I don't think you did, because this kind of code formatting stuff
> sticks out like a sore thumb.

I'll fix it.

- Greg

RE: [net 2/8] igb: fix vf lookup

[Rose, Gregory V]

> -----Original Message-----
> From: Kirsher, Jeffrey T
> Sent: Wednesday, February 08, 2012 1:37 AM
> To: davem@davemloft.net
> Cc: Rose, Gregory V; netdev@vger.kernel.org; gospo@redhat.com;
> sassmann@redhat.com; stable@vger.kernel.org; Kirsher, Jeffrey T
> Subject: [net 2/8] igb: fix vf lookup
> 
> From: Greg Rose <gregory.v.rose@intel.com>
> 
> Recent addition of code to find already allocated VFs failed to take
> account that systems with 2 or more multi-port SR-IOV capable controllers
> might have already enabled VFs.  Make sure that the VFs the function is
> finding are actually subordinate to the particular instance of the adapter
> that is looking for them and not subordinate to some device that has
> previously enabled SR-IOV.
> 
> This is applicable to 3.2+ kernels.
> 
> CC: stable@vger.kernel.org
> Reported-by: David Ahern <daahern@cisco.com>
> Signed-off-by: Greg Rose <gregory.v.rose@intel.com>
> Tested-by: Robert E Garrett <robertX.e.garrett@intel.com>
> Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
> ---
>  drivers/net/ethernet/intel/igb/igb_main.c |    3 ++-
>  1 files changed, 2 insertions(+), 1 deletions(-)
> 
> diff --git a/drivers/net/ethernet/intel/igb/igb_main.c
> b/drivers/net/ethernet/intel/igb/igb_main.c
> index e91d73c..94be6c3 100644
> --- a/drivers/net/ethernet/intel/igb/igb_main.c
> +++ b/drivers/net/ethernet/intel/igb/igb_main.c
> @@ -5012,7 +5012,8 @@ static int igb_find_enabled_vfs(struct igb_adapter
> *adapter)
>  	vf_devfn = pdev->devfn + 0x80;
>  	pvfdev = pci_get_device(hw->vendor_id, device_id, NULL);
>  	while (pvfdev) {
> -		if (pvfdev->devfn == vf_devfn)
> +		if (pvfdev->devfn == vf_devfn &&
> +		    (pvfdev->bus->number >= pdev->bus->number))
>  			vfs_found++;
>  		vf_devfn += vf_stride;
>  		pvfdev = pci_get_device(hw->vendor_id,
> --
> 1.7.7.6

I'll fix this one too.  You start leaning on checkpatch and you get lazy I guess.

- Greg

RE: [net 2/8] igb: fix vf lookup

[Joe Perches]

On Wed, 2012-02-08 at 23:42 +0000, Rose, Gregory V wrote:
> > diff --git a/drivers/net/ethernet/intel/igb/igb_main.c
[]
> > @@ -5012,7 +5012,8 @@ static int igb_find_enabled_vfs(struct igb_adapter
> > *adapter)
> >  	vf_devfn = pdev->devfn + 0x80;
> >  	pvfdev = pci_get_device(hw->vendor_id, device_id, NULL);
> >  	while (pvfdev) {
> > -		if (pvfdev->devfn == vf_devfn)
> > +		if (pvfdev->devfn == vf_devfn &&
> > +		    (pvfdev->bus->number >= pdev->bus->number))
> >  			vfs_found++;
[]
> I'll fix this one too.  You start leaning on checkpatch and you get lazy I guess.

I suppose an indentation rule could be created when
arguments on multiple lines don't align at the open
parenthesis, but I'm not going to rewrite emacs
indentation rules.

Presumably it should only be used with --strict.

Anyone think multiple line tests with inequivalent uses
of parentheses like this one should be noted as well?

Re: [net 2/8] igb: fix vf lookup

[David Miller]

From: Joe Perches <joe@perches.com>
Date: Wed, 08 Feb 2012 15:49:40 -0800

> On Wed, 2012-02-08 at 23:42 +0000, Rose, Gregory V wrote:
>> > diff --git a/drivers/net/ethernet/intel/igb/igb_main.c
> []
>> > @@ -5012,7 +5012,8 @@ static int igb_find_enabled_vfs(struct igb_adapter
>> > *adapter)
>> >  	vf_devfn = pdev->devfn + 0x80;
>> >  	pvfdev = pci_get_device(hw->vendor_id, device_id, NULL);
>> >  	while (pvfdev) {
>> > -		if (pvfdev->devfn == vf_devfn)
>> > +		if (pvfdev->devfn == vf_devfn &&
>> > +		    (pvfdev->bus->number >= pdev->bus->number))
>> >  			vfs_found++;
> []
>> I'll fix this one too.  You start leaning on checkpatch and you get lazy I guess.
> 
> I suppose an indentation rule could be created when
> arguments on multiple lines don't align at the open
> parenthesis, but I'm not going to rewrite emacs
> indentation rules.
> 
> Presumably it should only be used with --strict.
> 
> Anyone think multiple line tests with inequivalent uses
> of parentheses like this one should be noted as well?

Actually I thought this case was perfectly fine.

RE: [net 2/8] igb: fix vf lookup

[Rose, Gregory V]

> -----Original Message-----
> From: David Miller [mailto:davem@davemloft.net]
> Sent: Wednesday, February 08, 2012 3:53 PM
> To: joe@perches.com
> Cc: Rose, Gregory V; Kirsher, Jeffrey T; netdev@vger.kernel.org;
> gospo@redhat.com; sassmann@redhat.com; stable@vger.kernel.org
> Subject: Re: [net 2/8] igb: fix vf lookup
> 
> From: Joe Perches <joe@perches.com>
> Date: Wed, 08 Feb 2012 15:49:40 -0800
> 
> > On Wed, 2012-02-08 at 23:42 +0000, Rose, Gregory V wrote:
> >> > diff --git a/drivers/net/ethernet/intel/igb/igb_main.c
> > []
> >> > @@ -5012,7 +5012,8 @@ static int igb_find_enabled_vfs(struct
> igb_adapter
> >> > *adapter)
> >> >  	vf_devfn = pdev->devfn + 0x80;
> >> >  	pvfdev = pci_get_device(hw->vendor_id, device_id, NULL);
> >> >  	while (pvfdev) {
> >> > -		if (pvfdev->devfn == vf_devfn)
> >> > +		if (pvfdev->devfn == vf_devfn &&
> >> > +		    (pvfdev->bus->number >= pdev->bus->number))
> >> >  			vfs_found++;
> > []
> >> I'll fix this one too.  You start leaning on checkpatch and you get
> lazy I guess.
> >
> > I suppose an indentation rule could be created when
> > arguments on multiple lines don't align at the open
> > parenthesis, but I'm not going to rewrite emacs
> > indentation rules.
> >
> > Presumably it should only be used with --strict.
> >
> > Anyone think multiple line tests with inequivalent uses
> > of parentheses like this one should be noted as well?
> 
> Actually I thought this case was perfectly fine.

The imbalanced parenthesis usage bothers me.  And yes, if you're going to have a tool that checks patch formatting it'd be nice if it caught things like this.  But then I'm the lazy fool here...

- Greg

Re: [net 3/8] ixgbe: fix vf lookup

[Jeff Kirsher]

[-- Attachment #1: Type: text/plain, Size: 800 bytes --]

On Wed, 2012-02-08 at 18:33 -0500, David Miller wrote:
> From: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
> Date: Wed,  8 Feb 2012 01:36:33 -0800
> 
> > -             if (pvfdev->devfn == vf_devfn)
> > +             if (pvfdev->devfn == vf_devfn &&
> > +                             (pvfdev->bus->number >=
> pdev->bus->number))
> 
> Come on.... do you look at these patches at all?
> 
> I don't think you did, because this kind of code formatting stuff
> sticks out like a sore thumb. 

Your right, I over-looked this, and it is especially bad since I caught
the problem with the igb patch (which was right before this one).  I
remember thinking that I need to make sure the ixgbe patch did not have
the same formatting issue.  But completely forgot before I applied the
patch.


[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

RE: [net 2/8] igb: fix vf lookup

[Jeff Kirsher]

[-- Attachment #1: Type: text/plain, Size: 1246 bytes --]

On Wed, 2012-02-08 at 15:49 -0800, Joe Perches wrote:
> On Wed, 2012-02-08 at 23:42 +0000, Rose, Gregory V wrote:
> > > diff --git a/drivers/net/ethernet/intel/igb/igb_main.c
> []
> > > @@ -5012,7 +5012,8 @@ static int igb_find_enabled_vfs(struct igb_adapter
> > > *adapter)
> > >  	vf_devfn = pdev->devfn + 0x80;
> > >  	pvfdev = pci_get_device(hw->vendor_id, device_id, NULL);
> > >  	while (pvfdev) {
> > > -		if (pvfdev->devfn == vf_devfn)
> > > +		if (pvfdev->devfn == vf_devfn &&
> > > +		    (pvfdev->bus->number >= pdev->bus->number))
> > >  			vfs_found++;
> []
> > I'll fix this one too.  You start leaning on checkpatch and you get lazy I guess.
> 
> I suppose an indentation rule could be created when
> arguments on multiple lines don't align at the open
> parenthesis, but I'm not going to rewrite emacs
> indentation rules.
> 
> Presumably it should only be used with --strict.
> 
> Anyone think multiple line tests with inequivalent uses
> of parentheses like this one should be noted as well?
> 

I tried to create this very fix last year, but I must admit my
checkpatch.pl foo was not that strong... :)

While it would be a good enhancement to checkpatch.pl, I do not see it
as a "critical" fix.

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

[net 8/8] ixgbe: ethtool: stats user buffer overrun

[Jeff Kirsher]

From: John Fastabend <john.r.fastabend@intel.com>

If the number of tx/rx queues changes the ethtool ioctl
ETHTOOL_GSTATS may overrun the userspace buffer. This
occurs because the general practice in user space to
query stats is to issue a ETHTOOL_GSSET cmd to learn the
buffer size needed, allocate the buffer, then call
ETHTOOL_GSTIRNGS and ETHTOOL_GSTATS. If the number of
real_num_queues is changed or flow control attributes
are changed after ETHTOOL_GSSET but before the
ETHTOOL_GSTRINGS/ETHTOOL_GSTATS a user space buffer
overrun occurs.

To fix the overrun always return the max buffer size
needed from get_sset_count() then return all strings
and stats from get_strings()/get_ethtool_stats().

This _will_ change the output from the ioctl() call
which could break applications and script parsing in
theory. I believe these changes should not break existing
tools because the only changes will be more {tx|rx}_queues
and the {tx|rx}_pb_* stats will always be returned.
Existing scripts already need to handle changing number
of queues because this occurs today depending on system
and current features. The {tx|rx}_pb_* stats are at the
end of the output and should be handled by scripts today
regardless.

Finally get_ethtool_stats and get_strings are free-form
outputs tools parsing these outputs should be defensive
anyways. In the end these updates are better then
having a tool segfault because of a buffer overrun.

Signed-off-by: John Fastabend <john.r.fastabend@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
 drivers/net/ethernet/intel/ixgbe/ixgbe_ethtool.c |   88 +++++++++++++---------
 1 files changed, 51 insertions(+), 37 deletions(-)

diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_ethtool.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_ethtool.c
index 1f31a04..a629754 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_ethtool.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_ethtool.c
@@ -120,19 +120,23 @@ static const struct ixgbe_stats ixgbe_gstrings_stats[] = {
 #endif /* IXGBE_FCOE */
 };
 
-#define IXGBE_QUEUE_STATS_LEN \
-	((((struct ixgbe_adapter *)netdev_priv(netdev))->num_tx_queues + \
-	((struct ixgbe_adapter *)netdev_priv(netdev))->num_rx_queues) * \
+/* ixgbe allocates num_tx_queues and num_rx_queues symmetrically so
+ * we set the num_rx_queues to evaluate to num_tx_queues. This is
+ * used because we do not have a good way to get the max number of
+ * rx queues with CONFIG_RPS disabled.
+ */
+#define IXGBE_NUM_RX_QUEUES netdev->num_tx_queues
+
+#define IXGBE_QUEUE_STATS_LEN ( \
+	(netdev->num_tx_queues + IXGBE_NUM_RX_QUEUES) * \
 	(sizeof(struct ixgbe_queue_stats) / sizeof(u64)))
 #define IXGBE_GLOBAL_STATS_LEN ARRAY_SIZE(ixgbe_gstrings_stats)
 #define IXGBE_PB_STATS_LEN ( \
-                 (((struct ixgbe_adapter *)netdev_priv(netdev))->flags & \
-                 IXGBE_FLAG_DCB_ENABLED) ? \
-                 (sizeof(((struct ixgbe_adapter *)0)->stats.pxonrxc) + \
-                  sizeof(((struct ixgbe_adapter *)0)->stats.pxontxc) + \
-                  sizeof(((struct ixgbe_adapter *)0)->stats.pxoffrxc) + \
-                  sizeof(((struct ixgbe_adapter *)0)->stats.pxofftxc)) \
-                  / sizeof(u64) : 0)
+			(sizeof(((struct ixgbe_adapter *)0)->stats.pxonrxc) + \
+			 sizeof(((struct ixgbe_adapter *)0)->stats.pxontxc) + \
+			 sizeof(((struct ixgbe_adapter *)0)->stats.pxoffrxc) + \
+			 sizeof(((struct ixgbe_adapter *)0)->stats.pxofftxc)) \
+			/ sizeof(u64))
 #define IXGBE_STATS_LEN (IXGBE_GLOBAL_STATS_LEN + \
                          IXGBE_PB_STATS_LEN + \
                          IXGBE_QUEUE_STATS_LEN)
@@ -1078,8 +1082,15 @@ static void ixgbe_get_ethtool_stats(struct net_device *netdev,
 		data[i] = (ixgbe_gstrings_stats[i].sizeof_stat ==
 		           sizeof(u64)) ? *(u64 *)p : *(u32 *)p;
 	}
-	for (j = 0; j < adapter->num_tx_queues; j++) {
+	for (j = 0; j < IXGBE_NUM_RX_QUEUES; j++) {
 		ring = adapter->tx_ring[j];
+		if (!ring) {
+			data[i] = 0;
+			data[i+1] = 0;
+			i += 2;
+			continue;
+		}
+
 		do {
 			start = u64_stats_fetch_begin_bh(&ring->syncp);
 			data[i]   = ring->stats.packets;
@@ -1087,8 +1098,15 @@ static void ixgbe_get_ethtool_stats(struct net_device *netdev,
 		} while (u64_stats_fetch_retry_bh(&ring->syncp, start));
 		i += 2;
 	}
-	for (j = 0; j < adapter->num_rx_queues; j++) {
+	for (j = 0; j < IXGBE_NUM_RX_QUEUES; j++) {
 		ring = adapter->rx_ring[j];
+		if (!ring) {
+			data[i] = 0;
+			data[i+1] = 0;
+			i += 2;
+			continue;
+		}
+
 		do {
 			start = u64_stats_fetch_begin_bh(&ring->syncp);
 			data[i]   = ring->stats.packets;
@@ -1096,22 +1114,20 @@ static void ixgbe_get_ethtool_stats(struct net_device *netdev,
 		} while (u64_stats_fetch_retry_bh(&ring->syncp, start));
 		i += 2;
 	}
-	if (adapter->flags & IXGBE_FLAG_DCB_ENABLED) {
-		for (j = 0; j < MAX_TX_PACKET_BUFFERS; j++) {
-			data[i++] = adapter->stats.pxontxc[j];
-			data[i++] = adapter->stats.pxofftxc[j];
-		}
-		for (j = 0; j < MAX_RX_PACKET_BUFFERS; j++) {
-			data[i++] = adapter->stats.pxonrxc[j];
-			data[i++] = adapter->stats.pxoffrxc[j];
-		}
+
+	for (j = 0; j < IXGBE_MAX_PACKET_BUFFERS; j++) {
+		data[i++] = adapter->stats.pxontxc[j];
+		data[i++] = adapter->stats.pxofftxc[j];
+	}
+	for (j = 0; j < IXGBE_MAX_PACKET_BUFFERS; j++) {
+		data[i++] = adapter->stats.pxonrxc[j];
+		data[i++] = adapter->stats.pxoffrxc[j];
 	}
 }
 
 static void ixgbe_get_strings(struct net_device *netdev, u32 stringset,
                               u8 *data)
 {
-	struct ixgbe_adapter *adapter = netdev_priv(netdev);
 	char *p = (char *)data;
 	int i;
 
@@ -1126,31 +1142,29 @@ static void ixgbe_get_strings(struct net_device *netdev, u32 stringset,
 			       ETH_GSTRING_LEN);
 			p += ETH_GSTRING_LEN;
 		}
-		for (i = 0; i < adapter->num_tx_queues; i++) {
+		for (i = 0; i < netdev->num_tx_queues; i++) {
 			sprintf(p, "tx_queue_%u_packets", i);
 			p += ETH_GSTRING_LEN;
 			sprintf(p, "tx_queue_%u_bytes", i);
 			p += ETH_GSTRING_LEN;
 		}
-		for (i = 0; i < adapter->num_rx_queues; i++) {
+		for (i = 0; i < IXGBE_NUM_RX_QUEUES; i++) {
 			sprintf(p, "rx_queue_%u_packets", i);
 			p += ETH_GSTRING_LEN;
 			sprintf(p, "rx_queue_%u_bytes", i);
 			p += ETH_GSTRING_LEN;
 		}
-		if (adapter->flags & IXGBE_FLAG_DCB_ENABLED) {
-			for (i = 0; i < MAX_TX_PACKET_BUFFERS; i++) {
-				sprintf(p, "tx_pb_%u_pxon", i);
-				p += ETH_GSTRING_LEN;
-				sprintf(p, "tx_pb_%u_pxoff", i);
-				p += ETH_GSTRING_LEN;
-			}
-			for (i = 0; i < MAX_RX_PACKET_BUFFERS; i++) {
-				sprintf(p, "rx_pb_%u_pxon", i);
-				p += ETH_GSTRING_LEN;
-				sprintf(p, "rx_pb_%u_pxoff", i);
-				p += ETH_GSTRING_LEN;
-			}
+		for (i = 0; i < IXGBE_MAX_PACKET_BUFFERS; i++) {
+			sprintf(p, "tx_pb_%u_pxon", i);
+			p += ETH_GSTRING_LEN;
+			sprintf(p, "tx_pb_%u_pxoff", i);
+			p += ETH_GSTRING_LEN;
+		}
+		for (i = 0; i < IXGBE_MAX_PACKET_BUFFERS; i++) {
+			sprintf(p, "rx_pb_%u_pxon", i);
+			p += ETH_GSTRING_LEN;
+			sprintf(p, "rx_pb_%u_pxoff", i);
+			p += ETH_GSTRING_LEN;
 		}
 		/* BUG_ON(p - data != IXGBE_STATS_LEN * ETH_GSTRING_LEN); */
 		break;
-- 
1.7.7.6