[net-next 0/6][pull request] Intel Wired LAN Driver Updates 2015-03-06

[net-next 0/6][pull request] Intel Wired LAN Driver Updates 2015-03-06

[Jeff Kirsher]

This series contains updates to e1000, e1000e and igb.

Yanir provides updates to e1000e based on the patches provided by John
Linville.  First updates the code comment to better describe the changes
and the impact on the driver.  Second removed calls to ioremap/unmap for
i219 since this is only relevant to older hardware only.  Starting with
i219, the NVM will not be mapped to its one BAR but to a address region
in another bar.

Alex Duyck provides two fixes for igb, first fixes a compile warning
where a variable may be used uninitialized, so Alex initializes it.
Second fixes an issue where all of the pin register values were having
to be pushed onto the stack each time the function was called, so to
avoid this, Alex made them static const so that they should only need
to be allocated once and we can avoid all the instructions to get them
onto the stack.

Eliezer found an issue in e1000 where we needed to be calling
netif_carrier_off earlier in the down() to prevent the stack from 
queuing more packets to the interface.

Sabrina Dubroca resolved a potential race condition by adding a
dummy allocator.  There was a race condition between e1000_change_mtu()
cleanups and netpoll, when changing the MTU across jumbo sizes.

The following are changes since commit 23375a0fd549aa0a8c96b9f56a0b8120ae1389dd:
  ipv4: Fix unused variable warnings in fib_table_flush_external.
and are available in the git repository at:
  git://git.kernel.org/pub/scm/linux/kernel/git/jkirsher/net-next master

Alexander Duyck (2):
  igb: Fix warning pin may be used uninitialized
  igb: Make arrays on stack static const to avoid reallocation

Eliezer Tamir (1):
  e1000: call netif_carrier_off early on down

Sabrina Dubroca (1):
  e1000: add dummy allocator to fix race condition between mtu change
    and netpoll

Yanir Lubetkin (2):
  e1000e: fix obscure comments
  e1000e: remove calls to ioremap/unmap for NVM addr

 drivers/net/ethernet/intel/e1000/e1000_main.c | 12 ++++++++++--
 drivers/net/ethernet/intel/e1000e/ich8lan.c   | 12 ++++++++----
 drivers/net/ethernet/intel/e1000e/netdev.c    |  8 +++++---
 drivers/net/ethernet/intel/igb/igb_ptp.c      | 26 +++++++++++++-------------
 4 files changed, 36 insertions(+), 22 deletions(-)

-- 
1.9.3

[net-next 1/6] e1000e: fix obscure comments

[Jeff Kirsher]

From: Yanir Lubetkin <yanirx.lubetkin@intel.com>

The interface to the device flash was modified in i219 and later HW.
This patch better describes the change and the impact on the driver.

CC: John W Linville <linville@tuxdriver.com>
Reported-by: John W Linville <linville@tuxdriver.com>
Signed-off-by: Yanir Lubetkin <yanirx.lubetkin@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/e1000e/ich8lan.c | 12 ++++++++----
 1 file changed, 8 insertions(+), 4 deletions(-)

diff --git a/drivers/net/ethernet/intel/e1000e/ich8lan.c b/drivers/net/ethernet/intel/e1000e/ich8lan.c
index 7523f51..9d81c03 100644
--- a/drivers/net/ethernet/intel/e1000e/ich8lan.c
+++ b/drivers/net/ethernet/intel/e1000e/ich8lan.c
@@ -603,12 +603,15 @@ static s32 e1000_init_nvm_params_ich8lan(struct e1000_hw *hw)
 	u16 i;
 	u32 nvm_size;
 
-	/* Can't read flash registers if the register set isn't mapped. */
 	nvm->type = e1000_nvm_flash_sw;
-	/* in SPT, gfpreg doesn't exist. NVM size is taken from the
-	 * STRAP register
-	 */
+
 	if (hw->mac.type == e1000_pch_spt) {
+		/* in SPT, gfpreg doesn't exist. NVM size is taken from the
+		 * STRAP register. This is because in SPT the GbE Flash region
+		 * is no longer accessed through the flash registers. Instead,
+		 * the mechanism has changed, and the Flash region access
+		 * registers are now implemented in GbE memory space.
+		 */
 		nvm->flash_base_addr = 0;
 		nvm_size = (((er32(STRAP) >> 1) & 0x1F) + 1)
 		    * NVM_SIZE_MULTIPLIER;
@@ -618,6 +621,7 @@ static s32 e1000_init_nvm_params_ich8lan(struct e1000_hw *hw)
 		/* Set the base address for flash register access */
 		hw->flash_address = hw->hw_addr + E1000_FLASH_BASE_ADDR;
 	} else {
+		/* Can't read flash registers if register set isn't mapped. */
 		if (!hw->flash_address) {
 			e_dbg("ERROR: Flash registers not mapped\n");
 			return -E1000_ERR_CONFIG;
-- 
1.9.3

[net-next 2/6] e1000e: remove calls to ioremap/unmap for NVM addr

[Jeff Kirsher]

From: Yanir Lubetkin <yanirx.lubetkin@intel.com>

Starting I219, the NVM will not be mapped to its own BAR, but to an
address region in another bar.  The mapping/unmapping is relevant
to older HW only.

CC: John W Linville <linville@tuxdriver.com>
Reported-by: John W Linville <linville@tuxdriver.com>
Signed-off-by: Yanir Lubetkin <yanirx.lubetkin@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/e1000e/netdev.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/drivers/net/ethernet/intel/e1000e/netdev.c b/drivers/net/ethernet/intel/e1000e/netdev.c
index 6fa4fc0..4be4576 100644
--- a/drivers/net/ethernet/intel/e1000e/netdev.c
+++ b/drivers/net/ethernet/intel/e1000e/netdev.c
@@ -6833,7 +6833,8 @@ static int e1000_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
 		goto err_ioremap;
 
 	if ((adapter->flags & FLAG_HAS_FLASH) &&
-	    (pci_resource_flags(pdev, 1) & IORESOURCE_MEM)) {
+	    (pci_resource_flags(pdev, 1) & IORESOURCE_MEM) &&
+	    (hw->mac.type < e1000_pch_spt)) {
 		flash_start = pci_resource_start(pdev, 1);
 		flash_len = pci_resource_len(pdev, 1);
 		adapter->hw.flash_address = ioremap(flash_start, flash_len);
@@ -7069,7 +7070,7 @@ err_hw_init:
 	kfree(adapter->tx_ring);
 	kfree(adapter->rx_ring);
 err_sw_init:
-	if (adapter->hw.flash_address)
+	if ((adapter->hw.flash_address) && (hw->mac.type < e1000_pch_spt))
 		iounmap(adapter->hw.flash_address);
 	e1000e_reset_interrupt_capability(adapter);
 err_flashmap:
@@ -7142,7 +7143,8 @@ static void e1000_remove(struct pci_dev *pdev)
 	kfree(adapter->rx_ring);
 
 	iounmap(adapter->hw.hw_addr);
-	if (adapter->hw.flash_address)
+	if ((adapter->hw.flash_address) &&
+	    (adapter->hw.mac.type < e1000_pch_spt))
 		iounmap(adapter->hw.flash_address);
 	pci_release_selected_regions(pdev,
 				     pci_select_bars(pdev, IORESOURCE_MEM));
-- 
1.9.3

[net-next 3/6] igb: Fix warning pin may be used uninitialized

[Jeff Kirsher]

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

When building the kernel using the gcc 4.8.3 compiler included in Fedora 20
I was repeatedly seeing the warning:

 drivers/net/ethernet/intel/igb/igb_ptp.c: In function ‘igb_ptp_feature_enable_i210’:
 drivers/net/ethernet/intel/igb/igb_ptp.c:395:21: warning: ‘pin’ may be used uninitialized in this function
 [-Wmaybe-uninitialized]
   tssdp &= ~ts_sdp_en[pin];
                     ^
 drivers/net/ethernet/intel/igb/igb_ptp.c:471:6: note: ‘pin’ was declared here
   int pin;
       ^

To resolve it I am assigning the pin a value of -1 when it is instantiated.

Signed-off-by: Alexander Duyck <alexander.h.duyck@redhat.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_ptp.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/intel/igb/igb_ptp.c b/drivers/net/ethernet/intel/igb/igb_ptp.c
index d20fc8e..525e5c4 100644
--- a/drivers/net/ethernet/intel/igb/igb_ptp.c
+++ b/drivers/net/ethernet/intel/igb/igb_ptp.c
@@ -468,7 +468,7 @@ static int igb_ptp_feature_enable_i210(struct ptp_clock_info *ptp,
 	u32 tsauxc, tsim, tsauxc_mask, tsim_mask, trgttiml, trgttimh;
 	unsigned long flags;
 	struct timespec ts;
-	int pin;
+	int pin = -1;
 	s64 ns;
 
 	switch (rq->type) {
-- 
1.9.3

[net-next 4/6] igb: Make arrays on stack static const to avoid reallocation

[Jeff Kirsher]

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

While addressing the pin problem I noticed that all of the pin register
values where having to be pushed onto the stack each time the function was
called.  To avoid that I am making them static const so that they should
only need to be allocated once and we can avoid all the instructions to get
them onto the stack..

size before:
   text	   data	    bss	    dec	    hex	filename
 161477	  10512	      8	 171997	  29fdd	drivers/net/ethernet/intel/igb/igb.ko

size after:
   text	   data	    bss	    dec	    hex	filename
 161205	  10512	      8	 171725	  29ecd	drivers/net/ethernet/intel/igb/igb.ko

Signed-off-by: Alexander Duyck <alexander.h.duyck@redhat.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_ptp.c | 24 ++++++++++++------------
 1 file changed, 12 insertions(+), 12 deletions(-)

diff --git a/drivers/net/ethernet/intel/igb/igb_ptp.c b/drivers/net/ethernet/intel/igb/igb_ptp.c
index 525e5c4..d6be4c6 100644
--- a/drivers/net/ethernet/intel/igb/igb_ptp.c
+++ b/drivers/net/ethernet/intel/igb/igb_ptp.c
@@ -358,7 +358,7 @@ static int igb_ptp_settime_i210(struct ptp_clock_info *ptp,
 static void igb_pin_direction(int pin, int input, u32 *ctrl, u32 *ctrl_ext)
 {
 	u32 *ptr = pin < 2 ? ctrl : ctrl_ext;
-	u32 mask[IGB_N_SDP] = {
+	static const u32 mask[IGB_N_SDP] = {
 		E1000_CTRL_SDP0_DIR,
 		E1000_CTRL_SDP1_DIR,
 		E1000_CTRL_EXT_SDP2_DIR,
@@ -373,16 +373,16 @@ static void igb_pin_direction(int pin, int input, u32 *ctrl, u32 *ctrl_ext)
 
 static void igb_pin_extts(struct igb_adapter *igb, int chan, int pin)
 {
-	struct e1000_hw *hw = &igb->hw;
-	u32 aux0_sel_sdp[IGB_N_SDP] = {
+	static const u32 aux0_sel_sdp[IGB_N_SDP] = {
 		AUX0_SEL_SDP0, AUX0_SEL_SDP1, AUX0_SEL_SDP2, AUX0_SEL_SDP3,
 	};
-	u32 aux1_sel_sdp[IGB_N_SDP] = {
+	static const u32 aux1_sel_sdp[IGB_N_SDP] = {
 		AUX1_SEL_SDP0, AUX1_SEL_SDP1, AUX1_SEL_SDP2, AUX1_SEL_SDP3,
 	};
-	u32 ts_sdp_en[IGB_N_SDP] = {
+	static const u32 ts_sdp_en[IGB_N_SDP] = {
 		TS_SDP0_EN, TS_SDP1_EN, TS_SDP2_EN, TS_SDP3_EN,
 	};
+	struct e1000_hw *hw = &igb->hw;
 	u32 ctrl, ctrl_ext, tssdp = 0;
 
 	ctrl = rd32(E1000_CTRL);
@@ -409,28 +409,28 @@ static void igb_pin_extts(struct igb_adapter *igb, int chan, int pin)
 
 static void igb_pin_perout(struct igb_adapter *igb, int chan, int pin)
 {
-	struct e1000_hw *hw = &igb->hw;
-	u32 aux0_sel_sdp[IGB_N_SDP] = {
+	static const u32 aux0_sel_sdp[IGB_N_SDP] = {
 		AUX0_SEL_SDP0, AUX0_SEL_SDP1, AUX0_SEL_SDP2, AUX0_SEL_SDP3,
 	};
-	u32 aux1_sel_sdp[IGB_N_SDP] = {
+	static const u32 aux1_sel_sdp[IGB_N_SDP] = {
 		AUX1_SEL_SDP0, AUX1_SEL_SDP1, AUX1_SEL_SDP2, AUX1_SEL_SDP3,
 	};
-	u32 ts_sdp_en[IGB_N_SDP] = {
+	static const u32 ts_sdp_en[IGB_N_SDP] = {
 		TS_SDP0_EN, TS_SDP1_EN, TS_SDP2_EN, TS_SDP3_EN,
 	};
-	u32 ts_sdp_sel_tt0[IGB_N_SDP] = {
+	static const u32 ts_sdp_sel_tt0[IGB_N_SDP] = {
 		TS_SDP0_SEL_TT0, TS_SDP1_SEL_TT0,
 		TS_SDP2_SEL_TT0, TS_SDP3_SEL_TT0,
 	};
-	u32 ts_sdp_sel_tt1[IGB_N_SDP] = {
+	static const u32 ts_sdp_sel_tt1[IGB_N_SDP] = {
 		TS_SDP0_SEL_TT1, TS_SDP1_SEL_TT1,
 		TS_SDP2_SEL_TT1, TS_SDP3_SEL_TT1,
 	};
-	u32 ts_sdp_sel_clr[IGB_N_SDP] = {
+	static const u32 ts_sdp_sel_clr[IGB_N_SDP] = {
 		TS_SDP0_SEL_FC1, TS_SDP1_SEL_FC1,
 		TS_SDP2_SEL_FC1, TS_SDP3_SEL_FC1,
 	};
+	struct e1000_hw *hw = &igb->hw;
 	u32 ctrl, ctrl_ext, tssdp = 0;
 
 	ctrl = rd32(E1000_CTRL);
-- 
1.9.3

[net-next 5/6] e1000: call netif_carrier_off early on down

[Jeff Kirsher]

From: Eliezer Tamir <eliezer.tamir@linux.intel.com>

When bringing down an interface netif_carrier_off() should be
one the first things we do, since this will prevent the stack
from queuing more packets to this interface.
This operation is very fast, and should make the device behave
much nicer when trying to bring down an interface under load.

Also, this would Do The Right Thing (TM) if this device has some
sort of fail-over teaming and redirect traffic to the other IF.

Move netif_carrier_off as early as possible.

Signed-off-by: Eliezer Tamir <eliezer.tamir@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/e1000/e1000_main.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/intel/e1000/e1000_main.c b/drivers/net/ethernet/intel/e1000/e1000_main.c
index 7f997d3..26dcb44 100644
--- a/drivers/net/ethernet/intel/e1000/e1000_main.c
+++ b/drivers/net/ethernet/intel/e1000/e1000_main.c
@@ -516,6 +516,7 @@ void e1000_down(struct e1000_adapter *adapter)
 	struct net_device *netdev = adapter->netdev;
 	u32 rctl, tctl;
 
+	netif_carrier_off(netdev);
 
 	/* disable receives in the hardware */
 	rctl = er32(RCTL);
@@ -544,7 +545,6 @@ void e1000_down(struct e1000_adapter *adapter)
 
 	adapter->link_speed = 0;
 	adapter->link_duplex = 0;
-	netif_carrier_off(netdev);
 
 	e1000_reset(adapter);
 	e1000_clean_all_tx_rings(adapter);
-- 
1.9.3

[net-next 6/6] e1000: add dummy allocator to fix race condition between mtu change and netpoll

[Jeff Kirsher]

From: Sabrina Dubroca <sd@queasysnail.net>

There is a race condition between e1000_change_mtu's cleanups and
netpoll, when we change the MTU across jumbo size:

Changing MTU frees all the rx buffers:
    e1000_change_mtu -> e1000_down -> e1000_clean_all_rx_rings ->
        e1000_clean_rx_ring

Then, close to the end of e1000_change_mtu:
    pr_info -> ... -> netpoll_poll_dev -> e1000_clean ->
        e1000_clean_rx_irq -> e1000_alloc_rx_buffers -> e1000_alloc_frag

And when we come back to do the rest of the MTU change:
    e1000_up -> e1000_configure -> e1000_configure_rx ->
        e1000_alloc_jumbo_rx_buffers

alloc_jumbo finds the buffers already != NULL, since data (shared with
page in e1000_rx_buffer->rxbuf) has been re-alloc'd, but it's garbage,
or at least not what is expected when in jumbo state.

This results in an unusable adapter (packets don't get through), and a
NULL pointer dereference on the next call to e1000_clean_rx_ring
(other mtu change, link down, shutdown):

BUG: unable to handle kernel NULL pointer dereference at           (null)
IP: [<ffffffff81194d6e>] put_compound_page+0x7e/0x330

    [...]

Call Trace:
 [<ffffffff81195445>] put_page+0x55/0x60
 [<ffffffff815d9f44>] e1000_clean_rx_ring+0x134/0x200
 [<ffffffff815da055>] e1000_clean_all_rx_rings+0x45/0x60
 [<ffffffff815df5e0>] e1000_down+0x1c0/0x1d0
 [<ffffffff811e2260>] ? deactivate_slab+0x7f0/0x840
 [<ffffffff815e21bc>] e1000_change_mtu+0xdc/0x170
 [<ffffffff81647050>] dev_set_mtu+0xa0/0x140
 [<ffffffff81664218>] do_setlink+0x218/0xac0
 [<ffffffff814459e9>] ? nla_parse+0xb9/0x120
 [<ffffffff816652d0>] rtnl_newlink+0x6d0/0x890
 [<ffffffff8104f000>] ? kvm_clock_read+0x20/0x40
 [<ffffffff810a2068>] ? sched_clock_cpu+0xa8/0x100
 [<ffffffff81663802>] rtnetlink_rcv_msg+0x92/0x260

By setting the allocator to a dummy version, netpoll can't mess up our
rx buffers.  The allocator is set back to a sane value in
e1000_configure_rx.

Fixes: edbbb3ca1077 ("e1000: implement jumbo receive with partial descriptors")
Signed-off-by: Sabrina Dubroca <sd@queasysnail.net>
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 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/intel/e1000/e1000_main.c b/drivers/net/ethernet/intel/e1000/e1000_main.c
index 26dcb44..73c98d3 100644
--- a/drivers/net/ethernet/intel/e1000/e1000_main.c
+++ b/drivers/net/ethernet/intel/e1000/e1000_main.c
@@ -144,6 +144,11 @@ static bool e1000_clean_rx_irq(struct e1000_adapter *adapter,
 static bool e1000_clean_jumbo_rx_irq(struct e1000_adapter *adapter,
 				     struct e1000_rx_ring *rx_ring,
 				     int *work_done, int work_to_do);
+static void e1000_alloc_dummy_rx_buffers(struct e1000_adapter *adapter,
+					 struct e1000_rx_ring *rx_ring,
+					 int cleaned_count)
+{
+}
 static void e1000_alloc_rx_buffers(struct e1000_adapter *adapter,
 				   struct e1000_rx_ring *rx_ring,
 				   int cleaned_count);
@@ -3552,8 +3557,11 @@ static int e1000_change_mtu(struct net_device *netdev, int new_mtu)
 		msleep(1);
 	/* e1000_down has a dependency on max_frame_size */
 	hw->max_frame_size = max_frame;
-	if (netif_running(netdev))
+	if (netif_running(netdev)) {
+		/* prevent buffers from being reallocated */
+		adapter->alloc_rx_buf = e1000_alloc_dummy_rx_buffers;
 		e1000_down(adapter);
+	}
 
 	/* NOTE: netdev_alloc_skb reserves 16 bytes, and typically NET_IP_ALIGN
 	 * means we reserve 2 more, this pushes us to allocate from the next
-- 
1.9.3

Re: [net-next 0/6][pull request] Intel Wired LAN Driver Updates 2015-03-06

[David Miller]

From: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
Date: Fri,  6 Mar 2015 03:06:00 -0800

> This series contains updates to e1000, e1000e and igb.

Looks good, pulled, thanks Jeff.