RE: [Intel-wired-lan] [PATCH] e1000e: Do not allow CRC stripping to be disabled on 82579 w/ jumbo frames

2015-04-20 Thread Brown, Aaron F
 From: Intel-wired-lan [mailto:intel-wired-lan-boun...@lists.osuosl.org] On
 Behalf Of Jeff Kirsher
 Sent: Wednesday, April 08, 2015 7:58 PM
 To: Alexander Duyck
 Cc: netdev@vger.kernel.org; intel-wired-...@lists.osuosl.org
 Subject: Re: [Intel-wired-lan] [PATCH] e1000e: Do not allow CRC stripping
 to be disabled on 82579 w/ jumbo frames
 
 On Wed, 2015-04-08 at 18:37 -0700, Alexander Duyck wrote:
  The driver wasn't allowing jumbo frames to be enabled when CRC
  stripping
  was disabled, however it was allowing CRC stripping to be disabled
  while
  jumbo frames were enabled.  This fixes that by making it so that the
  NETIF_F_RXFCS flag cannot be set when jumbo frames are enabled on
  82579 and
  newer parts.
 
  Signed-off-by: Alexander Duyck alexander.h.du...@redhat.com
  ---
   drivers/net/ethernet/intel/e1000e/netdev.c |   14 ++
   1 file changed, 14 insertions(+)
 
 Thanks Alex, I will add your patch to my queue.

Tested-by: Aaron Brown aaron.f.br...@intel.com

 --
 git://git.kernel.org/pub/scm/linux/kernel/git/jkirsher/next-queue.git
 dev-queue
N�r��yb�X��ǧv�^�)޺{.n�+���z�^�)w*jg����ݢj/���z�ޖ��2�ޙ�)ߡ�a�����G���h��j:+v���w��٥

RE: [Intel-wired-lan] [PATCH] e1000e: Cleanup handling of VLAN_HLEN as a part of max frame size

2015-04-20 Thread Brown, Aaron F
 From: Intel-wired-lan [mailto:intel-wired-lan-boun...@lists.osuosl.org] On
 Behalf Of Alexander Duyck
 Sent: Wednesday, April 08, 2015 2:03 PM
 To: intel-wired-...@lists.osuosl.org; Kirsher, Jeffrey T
 Cc: netdev@vger.kernel.org; m...@cchtml.com; ht...@twofifty.com
 Subject: [Intel-wired-lan] [PATCH] e1000e: Cleanup handling of VLAN_HLEN
 as a part of max frame size
 
 When the VLAN_HLEN was added to the calculation for the maximum frame size
 there seems to have been a number of issues added to the driver.
 
 The first issue is that in some cases the maximum frame size for a device
 never really reached the actual maximum frame size as the VLAN header
 length was not included the calculation for that value.  As a result some
 parts only supported a maximum frame size of either 1496 in the case of
 parts that didn't support jumbo frames, and 8996 in the case of the parts
 that do.
 
 The second issue is the fact that there were several checks that weren't
 updated so as a result setting an MTU of 1500 was treated as enabling
 jumbo
 frames as the calculated value was 1522 instead of 1518.  I have addressed
 those by replacing ETH_FRAME_LEN with VLAN_ETH_FRAME_LEN where
 appropriate.
 
 The final issue was the fact that lowering the MTU below 1500 would cause
 the driver to allocate 2K buffers for the rings.  This is an old issue
 that
 was fixed several years ago in igb/ixgbe and I am addressing now by just
 replacing == with a = so that we always just round up to 1522 for
 anything
 that isn't a jumbo frame.
 
 Fixes: c751a3d58cf2d (e1000e: Correctly include VLAN_HLEN when changing
 interface MTU)
 Signed-off-by: Alexander Duyck alexander.h.du...@redhat.com
 ---
 
 I have only build tested this though I am 99% sure the fixes here are
 correct.  This patch should fix issues on 82573 and ich8 w/ setting an MTU
 of 1500, and for the PCH series w/ setting an MTU of 9000.
 
 I assume I can get away with bumping the max_hw_frame_size for the PCH
 parts from 9018 to 9022 based on the fact that the Windows INF for the
 parts
 lists supporting either 1514, 4088, and 9014 all of which exclude the 8
 bytes for CRC and VLAN header.
 
  drivers/net/ethernet/intel/e1000e/82571.c   |2 +-
  drivers/net/ethernet/intel/e1000e/ich8lan.c |   10 +-
  drivers/net/ethernet/intel/e1000e/netdev.c  |   18 --
  3 files changed, 14 insertions(+), 16 deletions(-)

Tested-by: Aaron Brown aaron.f.br...@intel.com

--
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


RE: [PATCH v1 net-next 1/3] igb: set driver data for device entries that were missing

2015-04-27 Thread Brown, Aaron F
 From: netdev-ow...@vger.kernel.org [mailto:netdev-ow...@vger.kernel.org]
 On Behalf Of Jonathan Toppins
 Sent: Monday, April 27, 2015 8:42 AM
 To: Kirsher, Jeffrey T
 Cc: Brandeburg, Jesse; Nelson, Shannon; Wyborny, Carolyn; Skidmore, Donald
 C; Vick, Matthew; Ronciak, John; Williams, Mitch A; intel-wired-
 l...@lists.osuosl.org; netdev@vger.kernel.org; go...@cumulusnetworks.com;
 s...@cumulusnetworks.com
 Subject: Re: [PATCH v1 net-next 1/3] igb: set driver data for device
 entries that were missing
 
 On 4/10/15 7:03 PM, Jonathan Toppins wrote:
  Three of the supported PCI device entries neglected to set driver data
  to board_82575. This is not a problem until a new board type is
  defined, so avoid the potential problem all together.
 
  Signed-off-by: Jonathan Toppins jtopp...@cumulusnetworks.com
  ---
drivers/net/ethernet/intel/igb/igb_main.c |6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)

Tested-by: Aaron Brown aaron.f.br...@intel.com

Well, regression tested that is as I don't have any of the potential new boards.
--
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


RE: [PATCH v1 net-next 2/3] igb: move initialization of link properties before igb_sw_init

2015-04-27 Thread Brown, Aaron F
 From: netdev-ow...@vger.kernel.org [mailto:netdev-ow...@vger.kernel.org]
 On Behalf Of Jonathan Toppins
 Sent: Friday, April 10, 2015 4:04 PM
 To: Kirsher, Jeffrey T
 Cc: Brandeburg, Jesse; Nelson, Shannon; Wyborny, Carolyn; Skidmore, Donald
 C; Vick, Matthew; Ronciak, John; Williams, Mitch A; intel-wired-
 l...@lists.osuosl.org; netdev@vger.kernel.org; go...@cumulusnetworks.com;
 s...@cumulusnetworks.com
 Subject: [PATCH v1 net-next 2/3] igb: move initialization of link
 properties before igb_sw_init
 
 This is required otherwise the driver may experience a NULL ptr
 dereference if CONFIG_PCI_IOV is set to yes.
 
 Since the code can follow the flow on init (driver insmod):
   hw-mac.autoneg = false; (this is not set it is its default)
   igb_probe()
   +- igb_sw_init()
   +- igb_probe_vfs()
   +- igb_pci_enable_sriov()
   +- igb_sriov_reinit()
  +- igb_reset()
 trimmed path is the same see call path for
 igb_reset below
   +- hw-mac.autoneg = true;
   +- igb_reset()
 
   /* igb_reset() call chain */
   igb_reset()
   +- hw-mac.ops.init_hw() == igb_init_hw_82575
  +- igb_setup_link()
 +- hw-mac.ops.setup_physical_interface() ==
  igb_setup_copper_link_82575()
+- igb_setup_copper_link()
   +- possible NULL dereference
 
 Pseudo code from igb_setup_copper_link():
   if (hw-mac.autoneg) {
   /* setup link */
   } else {
   hw-phy.ops.force_speed_duplex(hw);  // -- NULL deref here
   }
 
 Since the way the current code is designed the driver will call
 igb_setup_copper_link twice if SRIOV is configured to be enabled. The
 call will occur once with autoneg == false and the second
 time autoneg == true. Since the decision to call the function pointer
 (hw-phy.ops.force_speed_duplex) is predicated on autoneg being set
 correctly move the setting of these parameters to as early as possible
 in the probe function.
 
 Signed-off-by: Jonathan Toppins jtopp...@cumulusnetworks.com
 ---
  drivers/net/ethernet/intel/igb/igb_main.c |   16 
  1 file changed, 8 insertions(+), 8 deletions(-)

Tested-by: Aaron Brown aaron.f.br...@intel.com
--
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


RE: [PATCH v1 net-next 3/3] igb: add PHY support for Broadcom 54616

2015-04-27 Thread Brown, Aaron F
 From: netdev-ow...@vger.kernel.org [mailto:netdev-ow...@vger.kernel.org]
 On Behalf Of Jonathan Toppins
 Sent: Friday, April 10, 2015 4:04 PM
 To: Kirsher, Jeffrey T
 Cc: Brandeburg, Jesse; Nelson, Shannon; Wyborny, Carolyn; Skidmore, Donald
 C; Vick, Matthew; Ronciak, John; Williams, Mitch A; intel-wired-
 l...@lists.osuosl.org; netdev@vger.kernel.org; go...@cumulusnetworks.com;
 s...@cumulusnetworks.com; Alan Liebthal
 Subject: [PATCH v1 net-next 3/3] igb: add PHY support for Broadcom 54616
 
 From: Alan Liebthal al...@cumulusnetworks.com
 
 The Celestica Redstone-XP Ethernet management port uses a Broadcom 54616
 chip for the PHY layer. This adds support for this PHY to the Intel igb
 e1000 driver.
 
 Signed-off-by: Alan Liebthal al...@cumulusnetworks.com
 Signed-off-by: Jonathan Toppins jtopp...@cumulusnetworks.com
 ---
  drivers/net/ethernet/intel/igb/e1000_82575.c   |5 +
  drivers/net/ethernet/intel/igb/e1000_defines.h |1 +
  drivers/net/ethernet/intel/igb/e1000_hw.h  |1 +
  3 files changed, 7 insertions(+)

Tested-by: Aaron Brown aaron.f.br...@intel.com

Again, just regression, I do not have this phy.
--
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


RE: RE: [bisected regression] e1000e: Detected Hardware Unit Hang

2015-05-29 Thread Brown, Aaron F
 From: Thomas Jarosch [mailto:thomas.jaro...@intra2net.com]
 Sent: Wednesday, May 27, 2015 9:01 AM
 To: Brown, Aaron F
 Cc: Kirsher, Jeffrey T; 'Linux Netdev List'; Eric Dumazet; e1000-devel
 Subject: Re: RE: [bisected regression] e1000e: Detected Hardware Unit
 Hang
 
 Hi Aaron,
 
 On Monday, 23. March 2015 22:37:08 Brown, Aaron F wrote:
   
And with an internal reproduction of the issue I have created an
  
   internal
  
bug report, described my set of reproductions, referenced the
 similar
external ones and assigned it to our current e1000e developer.
  
  
   just wanted to quickly check if there has been any progress
   since the internal bug report has been filed?
 
 
  No, no updates beyond a bit of investigation.
 
 any news on this from the Intel labs?

Nothing significant.  Another one of our testers (whom works more closely with 
the current e1000e driver owner than I) has managed to replicate it on several 
systems and I know the developer spent some time poking around the setup, but I 
don't think he's found the root cause yet and has been busy chasing a number of 
other issues.

 
 Another two months passed ;) It would be nice to get rid
 of the workaround that limits the max fragment size to 4096.
 
 Thanks,
 Thomas

--
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


RE: [Intel-wired-lan] [PATCH] igbvf: clear buffer_info-dma after dma_unmap_single()

2015-08-12 Thread Brown, Aaron F
 From: Intel-wired-lan [mailto:intel-wired-lan-boun...@lists.osuosl.org] On
 Behalf Of Stefan Assmann
 Sent: Thursday, August 06, 2015 12:32 AM
 To: intel-wired-...@lists.osuosl.org
 Cc: netdev@vger.kernel.org; sassm...@kpanic.de
 Subject: [Intel-wired-lan] [PATCH] igbvf: clear buffer_info-dma after
 dma_unmap_single()
 
 The driver doesn't clear buffer_info-dma after calling
 dma_unmap_single() in all cases. This has been discovered by changing
 the mtu twice, which caused the following backtrace.
 
 [   68.569280] WARNING: CPU: 2 PID: 1860 at drivers/iommu/intel-
 iommu.c:3517 intel_unmap+0x20c/0x220()
 [   68.579392] Driver unmaps unmatched page at PFN fffc2a40
 [   68.585322] Modules linked in: igbvf ipt_MASQUERADE
 nf_nat_masquerade_ipv4 iptable_nat nf_nat_ipv4 nf_nat kvm_intel kvm igb
 megs
 [   68.599163] CPU: 2 PID: 1860 Comm: ifconfig Not tainted 4.2.0-rc4+ #147
 [   68.606543] Hardware name: IBM  -[546025Z]-/00Y7630, BIOS -[VVE134TUS-
 1.51]- 10/17/2013
 [   68.615473]  0dbd 88046441bb08 81a5ad0b
 81e2f9ea
 [   68.623775]  88046441bb58 88046441bb48 81056b55
 88047fc583c0
 [   68.632075]   880469a8e600 fffc2a40
 880465b32098
 [   68.640375] Call Trace:
 [   68.643109]  [81a5ad0b] dump_stack+0x48/0x5d
 [   68.648844]  [81056b55] warn_slowpath_common+0x95/0xe0
 [   68.655549]  [81056c56] warn_slowpath_fmt+0x46/0x70
 [   68.661960]  [8158a614] ? find_iova+0x54/0x90
 [   68.667791]  [815988dc] intel_unmap+0x20c/0x220
 [   68.673815]  [8159891e] intel_unmap_page+0xe/0x10
 [   68.680038]  [a0067536] igbvf_clean_rx_ring+0x96/0x370
 [igbvf]
 [   68.687516]  [a0067915] igbvf_down+0x105/0x110 [igbvf]
 [   68.694219]  [a0067beb] igbvf_change_mtu+0x16b/0x180 [igbvf]
 [...]
 
 Signed-off-by: Stefan Assmann sassm...@kpanic.de
 ---
  drivers/net/ethernet/intel/igbvf/netdev.c | 1 +
  1 file changed, 1 insertion(+)

Tested-by: Aaron Brown aaron.f.br...@intel.com
--
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


RE: [Intel-wired-lan] [PATCH] e100: Add a check after pci_pool_create to avoid null pointer dereference

2015-08-17 Thread Brown, Aaron F
 From: Intel-wired-lan [mailto:intel-wired-lan-boun...@lists.osuosl.org] On
 Behalf Of Jia-Ju Bai
 Sent: Sunday, August 02, 2015 7:17 PM
 To: Kirsher, Jeffrey T; Brandeburg, Jesse
 Cc: netdev@vger.kernel.org; Jia-Ju Bai; intel-wired-...@lists.osuosl.org;
 linux-ker...@vger.kernel.org
 Subject: [Intel-wired-lan] [PATCH] e100: Add a check after pci_pool_create
 to avoid null pointer dereference
 
 The driver lacks the check of nic-cbs_pool after pci_pool_create
 in e100_probe. When this function is failed, a null pointer dereference
 occurs when pci_pool_alloc uses nic-cbs_pool in e100_alloc_cbs.
 This patch adds a check and related error handling code to fix it.
 
 Signed-off-by: Jia-Ju Bai baijiaju1...@163.com
 ---
  drivers/net/ethernet/intel/e100.c |7 +++
  1 file changed, 7 insertions(+)

Tested-by: Aaron Brown aaron.f.br...@intel.com
--
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


RE: [Intel-wired-lan] [PATCH] e100: Release skb when DMA mapping is failed in e100_xmit_prepare

2015-08-17 Thread Brown, Aaron F
 From: Intel-wired-lan [mailto:intel-wired-lan-boun...@lists.osuosl.org] On
 Behalf Of Jia-Ju Bai
 Sent: Sunday, August 02, 2015 7:41 PM
 To: Kirsher, Jeffrey T; Brandeburg, Jesse
 Cc: netdev@vger.kernel.org; Jia-Ju Bai; intel-wired-...@lists.osuosl.org;
 linux-ker...@vger.kernel.org
 Subject: [Intel-wired-lan] [PATCH] e100: Release skb when DMA mapping is
 failed in e100_xmit_prepare
 
 When pci_dma_mapping_error in e100_xmit_prepare is failed, the skb buffer
 allocated by netdev_alloc_skb_ip_align in e100_rx_alloc_skb is not
 released, which causes a possible resource leak.
 This patch adds error handling code to fix it.
 
 Signed-off-by: Jia-Ju Bai baijiaju1...@163.com
 ---
  drivers/net/ethernet/intel/e100.c |5 -
  1 file changed, 4 insertions(+), 1 deletion(-)

Tested-by: Aaron Brown aaron.f.br...@intel.com
--
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


RE: [Intel-wired-lan] [PATCH v2 1/2] igb: Teardown SR-IOV before unregister_netdev()

2015-08-11 Thread Brown, Aaron F
 From: Intel-wired-lan [mailto:intel-wired-lan-boun...@lists.osuosl.org] On
 Behalf Of Alex Williamson
 Sent: Wednesday, July 29, 2015 1:38 PM
 To: intel-wired-...@lists.osuosl.org; da...@davemloft.net; Kirsher,
 Jeffrey T
 Cc: netdev@vger.kernel.org; linux-ker...@vger.kernel.org
 Subject: [Intel-wired-lan] [PATCH v2 1/2] igb: Teardown SR-IOV before
 unregister_netdev()
 
 When the .remove() callback for a PF is called, SR-IOV support for the
 device is disabled, which requires unbinding and removing the VFs.
 The VFs may be in-use either by the host kernel or userspace, such as
 assigned to a VM through vfio-pci.  In this latter case, the VFs may
 be removed either by shutting down the VM or hot-unplugging the
 devices from the VM.  Unfortunately in the case of a Windows 2012 R2
 guest, hot-unplug is broken due to the ordering of the PF driver
 teardown.  Disabling SR-IOV prior to unregister_netdev() avoids this
 issue.
 
 Signed-off-by: Alex Williamson alex.william...@redhat.com
 Acked-by: Mitch Williams mitch.a.willi...@intel.com
 ---
  drivers/net/ethernet/intel/igb/igb_main.c |8 
  1 file changed, 4 insertions(+), 4 deletions(-)

Tested-by: Aaron Brown aaron.f.br...@intel.com
--
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


RE: [PATCH 2/3] missing rtnl_unlock in igb_sriov_reinit()

2015-08-04 Thread Brown, Aaron F
 From: netdev-ow...@vger.kernel.org [mailto:netdev-ow...@vger.kernel.org]
 On Behalf Of Vasily Averin
 Sent: Tuesday, July 07, 2015 8:54 AM
 To: David S. Miller; netdev@vger.kernel.org
 Cc: intel-wired-...@lists.osuosl.org; Kirsher, Jeffrey T; Fujinaka, Todd
 Subject: [PATCH 2/3] missing rtnl_unlock in igb_sriov_reinit()
 
 Signed-off-by: Vasily Averin v...@virtuozzo.com
 ---
  drivers/net/ethernet/intel/igb/igb_main.c | 1 +
  1 file changed, 1 insertion(+)

Tested-by: Aaron Brown aaron.f.br...@intel.com
N�r��yb�X��ǧv�^�)޺{.n�+���z�^�)w*jg����ݢj/���z�ޖ��2�ޙ�)ߡ�a�����G���h��j:+v���w��٥

RE: [PATCH] igb: do not re-init SR-IOV during probe

2015-08-04 Thread Brown, Aaron F
 From: netdev-ow...@vger.kernel.org [mailto:netdev-ow...@vger.kernel.org]
 On Behalf Of Stefan Assmann
 Sent: Friday, July 10, 2015 6:01 AM
 To: intel-wired-...@lists.osuosl.org
 Cc: netdev@vger.kernel.org; Kirsher, Jeffrey T; sassm...@kpanic.de
 Subject: [PATCH] igb: do not re-init SR-IOV during probe
 
 During driver probing the following code path is triggered.
 igb_probe
 -igb_sw_init
   -igb_probe_vfs
 -igb_pci_enable_sriov
   -igb_sriov_reinit
 
 Doing the SR-IOV re-init is not necessary during probing since we're
 starting from scratch. Here we can call igb_enable_sriov() right away.
 
 Running igb_sriov_reinit() during igb_probe() also seems to cause
 occasional packet loss on some onboard 82576 NICs. Reproduced on
 Dell and HP servers with onboard 82576 NICs.
 Example:
 Intel Corporation 82576 Gigabit Network Connection [8086:10c9] (rev 01)
 Subsystem: Dell Device [1028:0481]
 
 Signed-off-by: Stefan Assmann sassm...@kpanic.de
 ---
  drivers/net/ethernet/intel/igb/igb_main.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)
 

Tested-by: Aaron Brown aaron.f.br...@intel.com
--
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


RE: [PATCH] igb: Fix a deadlock in igb_sriov_reinit

2015-08-10 Thread Brown, Aaron F
 From: netdev-ow...@vger.kernel.org [mailto:netdev-ow...@vger.kernel.org]
 On Behalf Of Jia-Ju Bai
 Sent: Sunday, August 02, 2015 8:36 PM
 To: Kirsher, Jeffrey T; Brandeburg, Jesse
 Cc: intel-wired-...@lists.osuosl.org; netdev@vger.kernel.org; linux-
 ker...@vger.kernel.org; Jia-Ju Bai
 Subject: [PATCH] igb: Fix a deadlock in igb_sriov_reinit
 
 When igb_init_interrupt_scheme in igb_sriov_reinit is failed, the lock
 acquired by rtnl_lock() is not released, which causes a deadlock.
 This patch adds rtnl_unlock() in error handling to fix it.
 
 Signed-off-by: Jia-Ju Bai baijiaju1...@163.com
 ---
  drivers/net/ethernet/intel/igb/igb_main.c |1 +
  1 file changed, 1 insertion(+)

Tested-by: Aaron Brown aaron.f.br...@intel.com
--
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


RE: [Intel-wired-lan] [PATCH 1/2] igb: Teardown SR-IOV before unregister_netdev()

2015-08-10 Thread Brown, Aaron F
 From: Intel-wired-lan [mailto:intel-wired-lan-boun...@lists.osuosl.org] On
 Behalf Of Alex Williamson
 Sent: Monday, July 27, 2015 4:19 PM
 To: intel-wired-...@lists.osuosl.org; Kirsher, Jeffrey T
 Cc: netdev@vger.kernel.org; linux-ker...@vger.kernel.org
 Subject: [Intel-wired-lan] [PATCH 1/2] igb: Teardown SR-IOV before
 unregister_netdev()
 
 When the .remove() callback for a PF is called, SR-IOV support for the
 device is disabled, which requires unbinding and removing the VFs.
 The VFs may be in-use either by the host kernel or userspace, such as
 assigned to a VM through vfio-pci.  In this latter case, the VFs may
 be removed either by shutting down the VM or hot-unplugging the
 devices from the VM.  Unfortunately in the case of a Windows 2012 R2
 guest, hot-unplug is broken due to the ordering of the PF driver
 teardown.  Disabling SR-IOV prior to unregister_netdev() avoids this
 issue.
 
 Signed-off-by: Alex Williamson alex.william...@redhat.com
 ---
  drivers/net/ethernet/intel/igb/igb_main.c |8 
  1 file changed, 4 insertions(+), 4 deletions(-)

Tested-by: Aaron Brown aaron.f.br...@intel.com
--
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


RE: [PATCH] igb: Fix a memory leak in igb_probe

2015-08-10 Thread Brown, Aaron F
 From: netdev-ow...@vger.kernel.org [mailto:netdev-ow...@vger.kernel.org]
 On Behalf Of Jia-Ju Bai
 Sent: Wednesday, August 05, 2015 7:05 AM
 To: Kirsher, Jeffrey T; Brandeburg, Jesse
 Cc: intel-wired-...@lists.osuosl.org; netdev@vger.kernel.org; linux-
 ker...@vger.kernel.org; Jia-Ju Bai
 Subject: [PATCH] igb: Fix a memory leak in igb_probe
 
 In error handling code of igb_probe, the memory adapter-shadow_vfta
 allocated by kcalloc in igb_sw_init is not freed. So when register_netdev
 or igb_init_i2c is failed, a memory leak will occur.
 This patch adds kfree to fix it.
 
 Signed-off-by: Jia-Ju Bai baijiaju1...@163.com
 ---
  drivers/net/ethernet/intel/igb/igb_main.c |1 +
  1 file changed, 1 insertion(+)

Tested-by: Aaron Brown aaron.f.br...@intel.com
--
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


RE: [Intel-wired-lan] [PATCH v2] e1000e: Modify tx/rx configurations to avoid null pointer dereferences in e1000_open

2015-08-14 Thread Brown, Aaron F
 From: Intel-wired-lan [mailto:intel-wired-lan-boun...@lists.osuosl.org] On
 Behalf Of Jia-Ju Bai
 Sent: Wednesday, August 05, 2015 3:16 AM
 To: Kirsher, Jeffrey T; Brandeburg, Jesse
 Cc: netdev@vger.kernel.org; Jia-Ju Bai; intel-wired-...@lists.osuosl.org;
 linux-ker...@vger.kernel.org
 Subject: [Intel-wired-lan] [PATCH v2] e1000e: Modify tx/rx configurations
 to avoid null pointer dereferences in e1000_open
 
 When e1000e_setup_rx_resources is failed in e1000_open,
 e1000e_free_tx_resources in err_setup_rx segment is executed.
 writel(0, tx_ring-head) statement in e1000_clean_tx_ring
 in e1000e_free_tx_resources will cause a null poonter dereference(crash),
 because tx_ring-head is only assigned in e1000_configure_tx
 in e1000_configure, but it is after e1000e_setup_rx_resources.
 
 This patch moves head/tail register writing to e1000_configure_tx/rx,
 which can fix this problem. It is inspired by igb_configure_tx_ring
 in the igb driver.
 
 Specially, thank Alexander Duyck for his valuable suggestion.
 
 Signed-off-by: Jia-Ju Bai baijiaju1...@163.com
 ---
  drivers/net/ethernet/intel/e1000e/netdev.c |   24 ---
 -
  1 file changed, 12 insertions(+), 12 deletions(-)

Tested-by: Aaron Brown aaron.f.br...@intel.com
--
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


RE: [PATCH net] igb: Fix oops caused by missing queue pairing

2015-08-14 Thread Brown, Aaron F
 From: netdev-ow...@vger.kernel.org [mailto:netdev-ow...@vger.kernel.org]
 On Behalf Of Shota Suzuki
 Sent: Tuesday, June 30, 2015 5:26 PM
 To: Kirsher, Jeffrey T; Brandeburg, Jesse; Nelson, Shannon; Wyborny,
 Carolyn; Skidmore, Donald C; Vick, Matthew; Ronciak, John; Williams, Mitch
 A; intel-wired-...@lists.osuosl.org; netdev@vger.kernel.org; linux-
 ker...@vger.kernel.org
 Cc: Shota Suzuki
 Subject: [PATCH net] igb: Fix oops caused by missing queue pairing
 
 When initializing igb driver (e.g. 82576, I350), IGB_FLAG_QUEUE_PAIRS is
 set if adapter-rss_queues exceeds half of max_rss_queues in
 igb_init_queue_configuration().
 On the other hand, IGB_FLAG_QUEUE_PAIRS is not set even if the number of
 queues exceeds half of max_combined in igb_set_channels() when changing
 the number of queues by ethtool -L.
 In this case, if numvecs is larger than MAX_MSIX_ENTRIES (10), the size
 of adapter-msix_entries[], an overflow can occur in
 igb_set_interrupt_capability(), which in turn leads to an oops.
 
 Fix this problem as follows:
  - When changing the number of queues by ethtool -L, set
IGB_FLAG_QUEUE_PAIRS in the same way as initializing igb driver.
  - When increasing the size of q_vector, reallocate it appropriately.
(With IGB_FLAG_QUEUE_PAIRS set, the size of q_vector gets larger.)
 
 Another possible way to fix this problem is to cap the queues at its
 initial number, which is the number of the initial online cpus. But this
 is not the optimal way because we cannnot increase queues when another
 cpu becomes online.
 
 Note that before commit cd14ef54d25b (igb: Change to use statically
 allocated array for MSIx entries), this problem did not cause oops
 but just made the number of queues become 1 because of entering msi_only
 mode in igb_set_interrupt_capability().
 
 Fixes: 907b7835799f (igb: Add ethtool support to configure number of
 channels)
 Signed-off-by: Shota Suzuki suzuki_shota...@lab.ntt.co.jp
 ---
 Although we might be able to additionally unset IGB_FLAG_QUEUE_PAIRS
 when it is not needed, this patch doesn't change existing behaviour
 because such a change is not a bug fix.
 
  drivers/net/ethernet/intel/igb/igb.h |  1 +
  drivers/net/ethernet/intel/igb/igb_ethtool.c |  5 -
  drivers/net/ethernet/intel/igb/igb_main.c| 16 ++--
  3 files changed, 19 insertions(+), 3 deletions(-)

Tested-by: Aaron Brown aaron.f.br...@intel.com
--
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


RE: [PATCH net-next 1/2] igb: Remove unnecessary flag setting in igb_set_flag_queue_pairs()

2015-12-18 Thread Brown, Aaron F
> From: netdev-ow...@vger.kernel.org [netdev-ow...@vger.kernel.org] on behalf 
> of Shota Suzuki 
> [suzuki_shota...@lab.ntt.co.jp]
> Sent: Friday, December 11, 2015 1:43 AM
> To: Kirsher, Jeffrey T; Brandeburg, Jesse; Nelson, Shannon; Wyborny, Carolyn; 
> Skidmore, Donald C; 
> Ronciak, John; Williams, Mitch A; intel-wired-...@lists.osuosl.org; 
> netdev@vger.kernel.org
> Cc: suzuki_shota...@lab.ntt.co.jp
> Subject: [PATCH net-next 1/2] igb: Remove unnecessary flag setting in 
> igb_set_flag_queue_pairs()
> 
> If VFs are enabled (max_vfs >= 1), both max_rss_queues and
> adapter->rss_queues are set to 2 in the case of e1000_82576.
> In this case, IGB_FLAG_QUEUE_PAIRS is always set in the default block as a
> result of fall-through, thus setting it in the e1000_82576 block is not
> necessary.

> Signed-off-by: Shota Suzuki 
> ---
>  drivers/net/ethernet/intel/igb/igb_main.c | 8 
>  1 file changed, 8 deletions(-)

Tested-by: Aaron Brown --
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


RE: [Intel-wired-lan] [PATCH net-next 2/2] igb: Unpair the queues when changing the number of queues

2015-12-18 Thread Brown, Aaron F
> From: Intel-wired-lan [intel-wired-lan-boun...@lists.osuosl.org] on behalf of 
> Shota Suzuki 
> [suzuki_shota...@lab.ntt.co.jp]
> Sent: Friday, December 11, 2015 1:44 AM
> To: Kirsher, Jeffrey T; Brandeburg, Jesse; Nelson, Shannon; Wyborny, Carolyn; 
> Skidmore, Donald C; 
> Ronciak, John; Williams, Mitch A; intel-wired-...@lists.osuosl.org; > 
> netdev@vger.kernel.org
> Subject: [Intel-wired-lan] [PATCH net-next 2/2] igb: Unpair the queues when   
>   changing the number of queues
> 
> By the commit 72ddef0506da ("igb: Fix oops caused by missing queue
> pairing"), the IGB_FLAG_QUEUE_PAIRS flag can now be set when changing the
> number of queues by "ethtool -L", but it is never cleared unless the igb
> driver is reloaded.
> This patch clears it if queue pairing becomes unnecessary as a result of
> "ethtool -L".
> 
> Signed-off-by: Shota Suzuki 
> ---
>  drivers/net/ethernet/intel/igb/igb_main.c | 2 ++
>  1 file changed, 2 insertions(+)

Tested-by: Aaron Brown --
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


RE: [PATCH] e1000e: fix division by zero on jumbo MTUs

2015-11-19 Thread Brown, Aaron F
> From: netdev-ow...@vger.kernel.org [mailto:netdev-
> ow...@vger.kernel.org] On Behalf Of Leonid Bloch
> Sent: Tuesday, October 13, 2015 2:48 AM
> To: linux-ker...@vger.kernel.org
> Cc: Kirsher, Jeffrey T ; Brandeburg, Jesse
> ; Nelson, Shannon
> ; Wyborny, Carolyn
> ; Skidmore, Donald C
> ; Vick, Matthew ;
> Ronciak, John ; Williams, Mitch A
> ; intel-wired-...@lists.osuosl.org;
> netdev@vger.kernel.org; Dmitry Fleytman 
> Subject: [PATCH] e1000e: fix division by zero on jumbo MTUs
> 
> From: Dmitry Fleytman 
> 
> This patch fixes possible division by zero in receive
> interrupt handler when working without adaptive interrupt
> moderation.
> 
> The adaptive interrupt moderation mechanism is typically
> disabled on jumbo MTUs.
> 
> Signed-off-by: Dmitry Fleytman 
> Signed-off-by: Leonid Bloch 
> ---
>  drivers/net/ethernet/intel/e1000e/netdev.c | 6 --
>  1 file changed, 4 insertions(+), 2 deletions(-)

Tested-by: Aaron Brown 
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


RE: [Intel-wired-lan] [PATCH v2] igb: improve handling of disconnected adapters

2015-11-19 Thread Brown, Aaron F
> From: Intel-wired-lan [mailto:intel-wired-lan-boun...@lists.osuosl.org] On
> Behalf Of Jarod Wilson
> Sent: Monday, October 19, 2015 8:52 AM
> To: linux-ker...@vger.kernel.org
> Cc: netdev@vger.kernel.org; Jarod Wilson ; intel-wired-
> l...@lists.osuosl.org
> Subject: [Intel-wired-lan] [PATCH v2] igb: improve handling of disconnected
> adapters
> 
> Clean up array_rd32 so that it uses igb_rd32 the same as rd32, per the
> suggestion of Alexander Duyck, and use io_addr in more places, so that
> we don't have the need to call E1000_REMOVED (which simply looks for a
> null hw_addr) nearly as much.
> 
> CC: Mark Rustad 
> CC: Jeff Kirsher 
> CC: Alexander Duyck 
> CC: intel-wired-...@lists.osuosl.org
> CC: netdev@vger.kernel.org
> Acked-by: Alexander Duyck 
> Signed-off-by: Jarod Wilson 
> ---
> Note: this patch is rebased on Jeff's next-queue/dev-queue branch, which
> already had an earlier revision of this applied, so I've essentially
> reverted a2675ab and applied the revised version of this, squashed them
> together, and here is the end result, which matches the version Alex acked.
> 
>  drivers/net/ethernet/intel/igb/e1000_regs.h |  3 +--
>  drivers/net/ethernet/intel/igb/igb_main.c   | 15 ++-
>  2 files changed, 3 insertions(+), 15 deletions(-)
> 

Tested-by: Aaron Brown 
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


RE: [Intel-wired-lan] [PATCHv2 net-next] net: igb: Only dma sync frame length

2016-06-14 Thread Brown, Aaron F
> From: Intel-wired-lan [mailto:intel-wired-lan-boun...@lists.osuosl.org] On
> Behalf Of Andrew Lunn
> Sent: Friday, June 3, 2016 2:03 PM
> To: Kirsher, Jeffrey T ; David Miller
> 
> Cc: netdev ; intel-wired-...@lists.osuosl.org;
> Andrew Lunn 
> Subject: [Intel-wired-lan] [PATCHv2 net-next] net: igb: Only dma sync frame
> length
> 
> On some platforms, syncing a buffer for DMA is expensive. Rather than
> sync the whole 2K receive buffer, only synchronise the length of the
> frame, which will typically be the MTU, or a much smaller TCP ACK.
> 
> For an IMX6Q, this gives around 6% increased TCP receive performance,
> which is cache operations bound and reduces CPU load for TCP transmit.
> 
> Signed-off-by: Andrew Lunn 
> ---
> v2:
> Christmas tree the local variables
> Pass size into igb_add_rx_frag() rather than repeating the endiness swap.
> ---
>  drivers/net/ethernet/intel/igb/igb_main.c | 7 ---
>  1 file changed, 4 insertions(+), 3 deletions(-)

Tested-by: Aaron Brown 


RE: [Intel-wired-lan] [RFC PATCH net] e1000e: keep vlan interfaces functional after rxvlan off

2016-05-26 Thread Brown, Aaron F
> From: Intel-wired-lan [mailto:intel-wired-lan-boun...@lists.osuosl.org] On
> Behalf Of Jeff Kirsher
> Sent: Wednesday, May 18, 2016 2:40 PM
> To: Jarod Wilson ; linux-ker...@vger.kernel.org;
> Avargil, Raanan 
> Cc: netdev@vger.kernel.org; intel-wired-...@lists.osuosl.org
> Subject: Re: [Intel-wired-lan] [RFC PATCH net] e1000e: keep vlan interfaces
> functional after rxvlan off
> 
> On Tue, 2016-05-17 at 15:03 -0400, Jarod Wilson wrote:
> > I've got a bug report about an e1000e interface, where a vlan interface
> > is
> > set up on top of it:
> >
> > $ ip link add link ens1f0 name ens1f0.99 type vlan id 99
> > $ ip link set ens1f0 up
> > $ ip link set ens1f0.99 up
> > $ ip addr add 192.168.99.92 dev ens1f0.99
> >
> > At this point, I can ping another host on vlan 99, ip 192.168.99.91.
> > However, if I do the following:
> >
> > $ ethtool -K ens1f0 rxvlan off
> >
> > Then no traffic passes on ens1f0.99. It comes back if I toggle rxvlan on
> > again. I'm not sure if this is actually intended behavior, or if there's
> > a
> > lack of software vlan stripping fallback, or what, but things continue to
> > work if I simply don't call e1000e_vlan_strip_disable() if there are
> > active vlans (plagiarizing a function from the e1000 driver here) on the
> > interface.
> >
> > Also slipped a related-ish fix to the kerneldoc text for
> > e1000e_vlan_strip_disable here...
> >
> > CC: Jeff Kirsher 
> > CC: intel-wired-...@lists.osuosl.org
> > CC: netdev@vger.kernel.org
> > Signed-off-by: Jarod Wilson 
> > ---
> >  drivers/net/ethernet/intel/e1000e/netdev.c | 15 +--
> >  1 file changed, 13 insertions(+), 2 deletions(-)
> 
> Raanan, please review this patch.  Even though it is an RFC I will be
> adding it to my queue for testing.
> http://patchwork.ozlabs.org/patch/623238/

Yup, without this patch disabling rxvlan offload does indeed break vlan 
connectivity and with the patch I can disable and re-enable rxvlan offloads as 
much as I care to.  It also makes it through my regression tests without 
problems.

Tested-by: Aaron Brown 

This is from functional - does it work - testing perspective so review is 
probably still in order.


RE: [Intel-wired-lan] [PATCH net v2] e1000e: keep vlan interfaces functional after rxvlan off

2016-06-15 Thread Brown, Aaron F
> From: Intel-wired-lan [mailto:intel-wired-lan-boun...@lists.osuosl.org] On
> Behalf Of Jarod Wilson
> Sent: Thursday, June 9, 2016 4:50 PM
> To: linux-ker...@vger.kernel.org
> Cc: netdev@vger.kernel.org; intel-wired-...@lists.osuosl.org
> Subject: [Intel-wired-lan] [PATCH net v2] e1000e: keep vlan interfaces
> functional after rxvlan off
> 
> I've got a bug report about an e1000e interface, where a vlan interface is
> set up on top of it:
> 
> $ ip link add link ens1f0 name ens1f0.99 type vlan id 99
> $ ip link set ens1f0 up
> $ ip link set ens1f0.99 up
> $ ip addr add 192.168.99.92 dev ens1f0.99
> 
> At this point, I can ping another host on vlan 99, ip 192.168.99.91.
> However, if I do the following:
> 
> $ ethtool -K ens1f0 rxvlan off
> 
> Then no traffic passes on ens1f0.99. It comes back if I toggle rxvlan on
> again. Upon discussion with folks here, and closer inspection, it appears
> that the software *receive* fallback is working correctly, but outbound
> traffic is broken. Upon glancing at the e1000 driver, I saw a note about
> having to keep RX and TX accel flags in sync, because there's no
> (hardware?) support for separate vlan accel toggle. Swipe the same hack
> from the e1000 driver, and things start to behave again with rxvlan off.
> 
> After patch:
> 
> $ ping 192.168.99.91
> PING 192.168.99.91 (192.168.99.91) 56(84) bytes of data.
> 64 bytes from 192.168.99.91: icmp_seq=1 ttl=64 time=0.591 ms
> 64 bytes from 192.168.99.91: icmp_seq=2 ttl=64 time=0.335 ms
> 64 bytes from 192.168.99.91: icmp_seq=3 ttl=64 time=0.417 ms
> ^C
> --- 192.168.99.91 ping statistics ---
> 3 packets transmitted, 3 received, 0% packet loss, time 2000ms
> rtt min/avg/max/mdev = 0.335/0.447/0.591/0.109 ms
> 
> $ sudo ethtool -K ens1f0 rxvlan off
> Actual changes:
> rx-vlan-offload: off
> tx-vlan-offload: off [requested on]
> 
> $ ping 192.168.99.91
> PING 192.168.99.91 (192.168.99.91) 56(84) bytes of data.
> 64 bytes from 192.168.99.91: icmp_seq=1 ttl=64 time=0.327 ms
> 64 bytes from 192.168.99.91: icmp_seq=2 ttl=64 time=0.393 ms
> 64 bytes from 192.168.99.91: icmp_seq=3 ttl=64 time=0.424 ms
> ^C
> --- 192.168.99.91 ping statistics ---
> 3 packets transmitted, 3 received, 0% packet loss, time 1999ms
> rtt min/avg/max/mdev = 0.327/0.381/0.424/0.043 ms
> 
> Also slipped a related-ish fix to the kerneldoc text for
> e1000e_vlan_strip_disable here...
> 
> CC: Jeff Kirsher 
> CC: intel-wired-...@lists.osuosl.org
> CC: netdev@vger.kernel.org
> Signed-off-by: Jarod Wilson 
> ---
>  drivers/net/ethernet/intel/e1000e/netdev.c | 10 +-
>  1 file changed, 9 insertions(+), 1 deletion(-)

Tested-by: Aaron Brown 


RE: [net] igbvf: remove "link is Up" message when registering mcast address

2016-02-04 Thread Brown, Aaron F
> From: netdev-ow...@vger.kernel.org [netdev-ow...@vger.kernel.org] on behalf 
> of Jon Maxwell [jmaxwel...@gmail.com]
> Sent: Sunday, January 24, 2016 3:22 PM
> To: Kirsher, Jeffrey T
> Cc: da...@davemloft.net; jmaxw...@redhat.com; vinsc...@redhat.com; 
> intel-wired-...@lists.osuosl.org; netdev@vger.kernel.org; 
> linux-ker...@vger.kernel.org; Jon Maxwell
> Subject: [net] igbvf: remove "link is Up" message when registering mcast 
> address
> 
> A similar issue was addressed a few years ago in the following thread:
> 
> http://www.spinics.net/lists/netdev/msg245877.html
> 
> At that time there were concerns that removing this statement may cause other
> side effects. However the submitter addressed those concerns. But the dialogue
> went cold. We have a new case where a customers application is registering and
> un-registering multicast addresses every few seconds. This is leading to many
> "Link is Up" messages in the logs as a result of the
> "netif_carrier_off(netdev)" statement called by igbvf_msix_other(). Also on
> some kernels it is interfering with the bonding driver causing it to failover
> and subsequently affecting connectivity.
> 
> The Sourgeforge driver does not make this call and is therefore not affected.
> If there were any side effects I would expect that driver to also be affected.
> I have tested re-loading the igbvf driver and downing the adapter with the PF
> entity on the host where the VM has this patch. When I bring it back up again
> connectivity is restored as expected. Therefore I request that this patch gets
> submitted.
> 
> Signed-off-by: Jon Maxwell 
> ---
>  drivers/net/ethernet/intel/igbvf/netdev.c | 1 -
>  1 file changed, 1 deletion(-)
> 

Tested-by: Aaron Brown 


RE: [Intel-wired-lan] [PATCH] igb: Fix VLAN tag stripping on Intel i350

2016-02-10 Thread Brown, Aaron F
> From: Intel-wired-lan [intel-wired-lan-boun...@lists.osuosl.org] on behalf of 
> Corinna Vinschen [vinsc...@redhat.com]
> Sent: Thursday, January 28, 2016 4:53 AM
> To: Kirsher, Jeffrey T
> Cc: netdev@vger.kernel.org; intel-wired-...@lists.osuosl.org
> Subject: Re: [Intel-wired-lan] [PATCH] igb: Fix VLAN tag stripping on Intel   
>   i350
> 
> Hi Jeff,

On Jan 27 11:25, Jeff Kirsher wrote:
> On Wed, 2016-01-27 at 14:28 +0100, Corinna Vinschen wrote:
> > Problem: When switching off VLAN offloading on an i350, the VLAN
> > interface gets unusable.  For testing, set up a VLAN on an i350
> > and some remote machine, e.g.:
> > [...]
> I tried applying your patch to my tree for review and validation, but
> due to patches already applied against the igb driver in my tree, your
> patch does not apply cleanly.
>
> Can you please update your patch to apply cleanly to my tree?
> git://git.kernel.org/pub/scm/linux/kernel/git/jkirsher/next-queue.git
> dev-queue
>
> (next-queue tree and dev-queue branch)
> 
> The attached patch is against the dev-queue branch of your next-queue tree.

Tested-by: Aaron Brown 


RE: [Intel-wired-lan] [PATCH net-next V2 5/6] e1000: call ndo_stop() instead of dev_close() when running offline selftest

2016-02-16 Thread Brown, Aaron F
> From: Rustad, Mark D
> Sent: Tuesday, February 16, 2016 9:23 AM
> To: Stefan Assmann
> Cc: intel-wired-...@lists.osuosl.org; netdev@vger.kernel.org; 
> da...@davemloft.net; Brown, Aaron F
> Subject: Re: [Intel-wired-lan] [PATCH net-next V2 5/6] e1000: call ndo_stop() 
>   instead of dev_close() when running offline selftest
> 
> > Checkpatch warns that externs should be avoided in .c files, but they
> > pre-existed and are just being flagged due to the name changing, so...
> >
> > Tested-by: Aaron Brown <aaron.f.br...@intel.com>
> 
> Actually, it is the forward declarations in the .c that should be deleted.
> The prototypes should only exist in the .h file.

Thanks Mark,  I completely missed that the patch added it to both files, too 
busy reviving older hardware to extend to run the diags against a larger set of 
parts I guess :")


RE: [Intel-wired-lan] [PATCH net-next V2 6/6] e1000e: call ndo_stop() instead of dev_close() when running offline selftest

2016-02-12 Thread Brown, Aaron F
> From: Intel-wired-lan [intel-wired-lan-boun...@lists.osuosl.org] on behalf of 
> Stefan Assmann [sassm...@kpanic.de]
> Sent: Wednesday, February 03, 2016 12:20 AM
> To: intel-wired-...@lists.osuosl.org
> Cc: netdev@vger.kernel.org; da...@davemloft.net; sassm...@kpanic.de
> Subject: [Intel-wired-lan] [PATCH net-next V2 6/6] e1000e: call ndo_stop()
>   instead of dev_close() when running offline selftest
> 
> Calling dev_close() causes IFF_UP to be cleared which will remove the
> interfaces routes and some addresses. That's probably not what the user
> intended when running the offline selftest. Besides this does not happen
> if the interface is brought down before the test, so the current
> behaviour is inconsistent.
> Instead call the net_device_ops ndo_stop function directly and avoid
> touching IFF_UP at all.
> 
> V2: rename e1000_open(), e1000_close() to e1000e_open(), e1000e_close()
> to avoid name clash with e1000.
> 
> Signed-off-by: Stefan Assmann 
> ---
>  drivers/net/ethernet/intel/e1000e/e1000.h   |  2 ++
>  drivers/net/ethernet/intel/e1000e/ethtool.c |  4 ++--
>  drivers/net/ethernet/intel/e1000e/netdev.c  | 12 ++--
>  3 files changed, 10 insertions(+), 8 deletions(-)

Tested-by: Aaron Brown 


RE: [Intel-wired-lan] [PATCH net-next V2 4/6] igb: call ndo_stop() instead of dev_close() when running offline selftest

2016-02-12 Thread Brown, Aaron F

> From: Intel-wired-lan [intel-wired-lan-boun...@lists.osuosl.org] on behalf of 
> Stefan Assmann [sassm...@kpanic.de]
> Sent: Wednesday, February 03, 2016 12:20 AM
> To: intel-wired-...@lists.osuosl.org
> Cc: netdev@vger.kernel.org; da...@davemloft.net; sassm...@kpanic.de
> Subject: [Intel-wired-lan] [PATCH net-next V2 4/6] igb: call ndo_stop() 
> instead of dev_close() when running offline selftest
> 
> Calling dev_close() causes IFF_UP to be cleared which will remove the
> interfaces routes and some addresses. That's probably not what the user
> intended when running the offline selftest. Besides this does not happen
> if the interface is brought down before the test, so the current
> behaviour is inconsistent.
> Instead call the net_device_ops ndo_stop function directly and avoid
> touching IFF_UP at all.
> 
> Signed-off-by: Stefan Assmann 
> ---
>  drivers/net/ethernet/intel/igb/igb.h | 2 ++
>  drivers/net/ethernet/intel/igb/igb_ethtool.c | 4 ++--
>  drivers/net/ethernet/intel/igb/igb_main.c| 8 
>  3 files changed, 8 insertions(+), 6 deletions(-)

Checkpatch warns that externs should be avoided in .c files, but they 
pre-existed and are just being flagged due to the name changing, so...

Tested-by: Aaron Brown  


RE: [Intel-wired-lan] [PATCH net-next V2 5/6] e1000: call ndo_stop() instead of dev_close() when running offline selftest

2016-02-12 Thread Brown, Aaron F
> From: Intel-wired-lan [intel-wired-lan-boun...@lists.osuosl.org] on behalf of 
> Stefan Assmann [sassm...@kpanic.de]
> Sent: Wednesday, February 03, 2016 12:20 AM
> To: intel-wired-...@lists.osuosl.org
> Cc: netdev@vger.kernel.org; da...@davemloft.net; sassm...@kpanic.de
> Subject: [Intel-wired-lan] [PATCH net-next V2 5/6] e1000: call ndo_stop() 
>   instead of dev_close() when running offline selftest
> 
> Calling dev_close() causes IFF_UP to be cleared which will remove the
> interfaces routes and some addresses. That's probably not what the user
> intended when running the offline selftest. Besides this does not happen
> if the interface is brought down before the test, so the current
> behaviour is inconsistent.
> Instead call the net_device_ops ndo_stop function directly and avoid
> touching IFF_UP at all.
> 
> Signed-off-by: Stefan Assmann 
> ---
>  drivers/net/ethernet/intel/e1000/e1000.h | 2 ++
>  drivers/net/ethernet/intel/e1000/e1000_ethtool.c | 4 ++--
>  drivers/net/ethernet/intel/e1000/e1000_main.c| 8 
>  3 files changed, 8 insertions(+), 6 deletions(-)

Checkpatch warns that externs should be avoided in .c files, but they 
pre-existed and are just being flagged due to the name changing, so...

Tested-by: Aaron Brown  



RE: [Intel-wired-lan] [PATCH] igb: Garbled output for "ethtool -m"

2016-02-29 Thread Brown, Aaron F
> From: Intel-wired-lan [intel-wired-lan-boun...@lists.osuosl.org] on behalf of 
> Doron Shikmoni [doron.shikm...@gmail.com]
> Sent: Tuesday, February 16, 2016 11:34 PM
> To: intel-wired-...@lists.osuosl.org
> Cc: netdev@vger.kernel.org
> Subject: [Intel-wired-lan] [PATCH] igb: Garbled output for "ethtool -m"
> 
> Hello,
> 
> Garbled output for "ethtool -m ethX", in igb-driven NICs with module /
> plugin EEPROM (i.e. SFP information). Each output data byte appears
> duplicated.
> 
> In igb_ethtool.c, igb_get_module_eeprom() is reading the EEPROM via i2c;
> the eeprom offset for each word that's read via igb_read_phy_reg_i2c()
> was passed in #words, whereas it needs to be a byte offset.
> This patches fixes the bug.
>
> Signed-off-by: Doron Shikmoni 
> ---
>  drivers/net/ethernet/intel/igb/igb_ethtool.c |2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

Checkpatch complains that you pushed the line over 80 characters:

u1463:[0]/usr/src/kernels/next-queue> git format-patch $item -1 
--stdout|./scripts/checkpatch.pl -
WARNING: line over 80 characters
#29: FILE: drivers/net/ethernet/intel/igb/igb_ethtool.c:3010:
+   status = igb_read_phy_reg_i2c(hw, (first_word + i) * 2, 
[i]);

total: 0 errors, 1 warnings, 0 checks, 8 lines checked

Your patch has style problems, please review.

NOTE: If any of the errors are false positives, please report
  them to the maintainer, see CHECKPATCH in MAINTAINERS.
u1463:[0]/usr/src/kernels/next-queue> 


But functionally seems good.  I'll let Jeff choose whether to be a stickler for 
the warning or not, so...

Tested-by: Aaron Brown 


RE: [Intel-wired-lan] [next] igb: allow setting MAC address on i211 using a device tree blob V5

2016-02-29 Thread Brown, Aaron F
> From: netdev-ow...@vger.kernel.org [mailto:netdev-
> ow...@vger.kernel.org] On Behalf Of John Holland
> Sent: Thursday, February 18, 2016 3:11 AM
> To: intel-wired-...@lists.osuosl.org; netdev@vger.kernel.org
> Subject: [Intel-wired-lan] [next] igb: allow setting MAC address on i211 using
> a device tree blob V5
> 
> Hello,
> 
> The Intel i211 LOM PCIe Ethernet controllers' iNVM operates as an OTP and
> has no external EEPROM interface [1]. The following allows the driver to
> pickup the MAC address from a device tree blob when CONFIG_OF has been
> enabled.
> 
> [1]
> http://www.intel.com/content/www/us/en/embedded/products/networkin
> g/i211-ethernet-controller-datasheet.html
> 
> Changes V2
> - Restrict searching for compatible devices to current pci device.
> 
> Changes V3
> - Add device tree binding documentation.
> 
> Changes V4
> - Rebase patch.
> 
> Changes V5
> - Use eth_platform_get_mac_address() to resolve MAC specified in a dtb.
> - Remove now invalid device tree binding documentation specified in V3
>und V4.
> 
> Signed-off-by: John Holland
> ---
>   drivers/net/ethernet/intel/igb/igb_main.c | 9 ++---
>   1 file changed, 6 insertions(+), 3 deletions(-)

This throws a few checkpatch warnings, but I won't withhold my tested by for 
these:
-
u1463:[0]/usr/src/kernels/next-queue> git format-patch $item -1 
--stdout|./scripts/checkpatch.pl -
WARNING: Possible unwrapped commit description (prefer a maximum 75 chars per 
line)
#14:
http://www.intel.com/content/www/us/en/embedded/products/networking/i211-ethernet-controller-datasheet.html

WARNING: email address 'John Holland' might be better as 
'John Holland '
#30:
Signed-off-by: John Holland

total: 0 errors, 2 warnings, 0 checks, 21 lines checked

Your patch has style problems, please review.

NOTE: If any of the errors are false positives, please report
  them to the maintainer, see CHECKPATCH in MAINTAINERS.
u1463:[0]/usr/src/kernels/next-queue>
-

I do not seem to have hardware that uses device tree, so my testing is 
relegated to regression tests with my existing set of chipsets.

Tested-by: Aaron Brown 


RE: [Intel-wired-lan] [next] igb: allow setting MAC address on i211 using a device tree blob V5

2016-03-01 Thread Brown, Aaron F
> From: John Holland [mailto:jotih...@gmail.com]
> Sent: Monday, February 29, 2016 10:56 PM
> To: Brown, Aaron F <aaron.f.br...@intel.com>
> Cc: intel-wired-...@lists.osuosl.org; netdev@vger.kernel.org
> Subject: Re: [Intel-wired-lan] [next] igb: allow setting MAC address on i211
> using a device tree blob V5
> 
> On Mar 1, 2016, at 03:52, Brown, Aaron F <aaron.f.br...@intel.com>
> wrote:
> 
> > This throws a few checkpatch warnings, but I won't withhold my tested by
> for these:
> >
> > total: 0 errors, 2 warnings, 0 checks, 21 lines checked
> >
> > Your patch has style problems, please review.
> >
> > NOTE: If any of the errors are false positives, please report
> >  them to the maintainer, see CHECKPATCH in MAINTAINERS.
> > u1463:[0]/usr/src/kernels/next-queue>
> 
> Thanks for testing...
> 
> Do you require me to reformat the patch text? And won't that break the link?

Unless Jeff Kirsher wants you to resubmit it for the space between your name 
and email address I don't think resubmitting should be necessary.  Yes, 
breaking the URL up would break the URL and for that reason I think it is 
appropriate to ignore that (Possible unwrapped commit description) warning.

Thanks,
Aaron
> 
> John


RE: [Intel-wired-lan] [net PATCH 2/2] e1000: Double Tx descriptors needed check for 82544

2016-03-09 Thread Brown, Aaron F
> From: Intel-wired-lan [mailto:intel-wired-lan-boun...@lists.osuosl.org] On
> Behalf Of Alexander Duyck
> Sent: Wednesday, March 2, 2016 1:16 PM
> To: netdev@vger.kernel.org; jogre...@redhat.com; intel-wired-
> l...@lists.osuosl.org; Kirsher, Jeffrey T ;
> sassm...@redhat.com
> Subject: [Intel-wired-lan] [net PATCH 2/2] e1000: Double Tx descriptors
> needed check for 82544
> 
> The 82544 has code that adds one additional descriptor per data buffer.
> However we weren't taking that into acount when determining the
> descriptors
> needed for the next transmit at the end of the xmit_frame path.
> 
> This change takes that into account by doubling the number of descriptors
> needed for the 82544 so that we can avoid a potential issue where we could
> hang the Tx ring by loading frames with xmit_more enabled and then
> stopping
> the ring without writing the tail.
> 
> In addition it adds a few more descriptors to account for some additional
> workarounds that have been added over time.
> 
> Signed-off-by: Alexander Duyck 
> ---
>  drivers/net/ethernet/intel/e1000/e1000_main.c |   19
> ++-
>  1 file changed, 18 insertions(+), 1 deletion(-)

Tested-by: Aaron Brown 


RE: [net PATCH 1/2] e1000: Do not overestimate descriptor counts in Tx pre-check

2016-03-09 Thread Brown, Aaron F
> From: netdev-ow...@vger.kernel.org [mailto:netdev-
> ow...@vger.kernel.org] On Behalf Of Alexander Duyck
> Sent: Wednesday, March 2, 2016 1:16 PM
> To: netdev@vger.kernel.org; jogre...@redhat.com; intel-wired-
> l...@lists.osuosl.org; Kirsher, Jeffrey T ;
> sassm...@redhat.com
> Subject: [net PATCH 1/2] e1000: Do not overestimate descriptor counts in Tx
> pre-check
> 
> The current code path is capable of grossly overestimating the number of
> descriptors needed to transmit a new frame.  This specifically occurs if
> the skb contains a number of 4K pages.  The issue is that the logic for
> determining the descriptors needed is ((S) >> (X)) + 1.  When X is 12 it
> means that we were indicating that we required 2 descriptors for each 4K
> page when we only needed one.
> 
> This change corrects this by instead adding (1 << (X)) - 1 to the S value
> instead of adding 1 after the fact.  This way we get an accurate descriptor
> needed count as we are essentially doing a DIV_ROUNDUP().
> 
> Reported-by: Ivan Suzdal 
> Signed-off-by: Alexander Duyck 
> ---
>  drivers/net/ethernet/intel/e1000/e1000_main.c |2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

Tested-by: Aaron Brown 


RE: [PATCH net-next] net: intel: remove dead links

2016-04-06 Thread Brown, Aaron F
> From: netdev-ow...@vger.kernel.org [mailto:netdev-
> ow...@vger.kernel.org] On Behalf Of Jiri Benc
> Sent: Tuesday, April 5, 2016 7:25 AM
> To: netdev@vger.kernel.org
> Cc: Kirsher, Jeffrey T ; Brandeburg, Jesse
> ; Nelson, Shannon
> ; Wyborny, Carolyn
> ; Skidmore, Donald C
> ; Allan, Bruce W ;
> Ronciak, John ; Williams, Mitch A
> ; intel-wired-...@lists.osuosl.org
> Subject: [PATCH net-next] net: intel: remove dead links
> 
> The Kconfig for Intel NICs references two different URLs for the "Adapter
> & Driver ID Guide". Neither of those two links works. The current URL seems
> to be
> http://www.intel.com/content/www/us/en/support/network-and-i-
> o/ethernet-products/05584.html
> but given it's apparently constantly changing, there's no point in having it
> in the help text.
> 
> Just keep a generic pointer to http://support.intel.com. Hopefully, this one
> will have a longer live. It still works, at least.
> 
> Futhermore, remove a link to "the latest Intel PRO/100 network driver for
> Linux", this has no place in the mainline kernel and the latest Linux driver
> it offers is from 2006, anyway.
> 
> Signed-off-by: Jiri Benc 
> ---
>  drivers/net/ethernet/intel/Kconfig | 80 
> +++---
>  1 file changed, 14 insertions(+), 66 deletions(-)

If reading through the file qualifies as tested...

Tested-by: Aaron Brown 


RE: [PATCH -v2] drivers: net: ethernet: intel: e1000e: fix ethtool autoneg off for non-copper

2016-04-08 Thread Brown, Aaron F
> From: netdev-ow...@vger.kernel.org [mailto:netdev-
> ow...@vger.kernel.org] On Behalf Of Daniel Walker
> Sent: Tuesday, April 5, 2016 11:30 AM
> To: Ruinskiy, Dima ; Kirsher, Jeffrey T
> ; Brandeburg, Jesse
> ; Nelson, Shannon
> ; Wyborny, Carolyn
> ; Skidmore, Donald C
> ; Allan, Bruce W ;
> Ronciak, John ; Williams, Mitch A
> 
> Cc: Steve Shih ; xe-ker...@external.cisco.com; Daniel
> Walker ; intel-wired-...@lists.osuosl.org;
> netdev@vger.kernel.org; linux-ker...@vger.kernel.org
> Subject: [PATCH -v2] drivers: net: ethernet: intel: e1000e: fix ethtool 
> autoneg
> off for non-copper
> 
> From: Steve Shih 
> 
> This patch fixes the issues for disabling auto-negotiation and forcing
> speed and duplex settings for the non-copper media.
> 
> For non-copper media, e1000_get_settings should return
> ETH_TP_MDI_INVALID for
> eth_tp_mdix_ctrl instead of ETH_TP_MDI_AUTO so subsequent
> e1000_set_settings
> call would not fail with -EOPNOTSUPP.
> 
> e1000_set_spd_dplx should not automatically turn autoneg back on for
> forced
> 1000 Mbps full duplex settings for non-copper media.
> 
> Cc: xe-ker...@external.cisco.com
> Cc: Daniel Walker 
> Signed-off-by: Steve Shih 
> ---
>  drivers/net/ethernet/intel/e1000e/ethtool.c | 11 +--
>  1 file changed, 9 insertions(+), 2 deletions(-)

Tested-by: Aaron Brown 



RE: [Intel-wired-lan] [PATCH] e1000e: prevent division by zero if TIMINCA is zero

2016-05-16 Thread Brown, Aaron F
> From: Intel-wired-lan [mailto:intel-wired-lan-boun...@lists.osuosl.org] On
> Behalf Of Denys Vlasenko
> Sent: Friday, May 6, 2016 12:42 PM
> To: Kirsher, Jeffrey T 
> Cc: intel-wired-...@lists.osuosl.org; Denys Vlasenko
> ; LKML ;
> netdev@vger.kernel.org
> Subject: [Intel-wired-lan] [PATCH] e1000e: prevent division by zero if
> TIMINCA is zero
> 
> Users report that under VMWare, er32(TIMINCA) returns zero.
> This causes division by zero at init time as follows:
> 
>  ==>incvalue = er32(TIMINCA) & E1000_TIMINCA_INCVALUE_MASK;
> for (i = 0; i < E1000_MAX_82574_SYSTIM_REREADS; i++) {
> /* latch SYSTIMH on read of SYSTIML */
> systim_next = (cycle_t)er32(SYSTIML);
> systim_next |= (cycle_t)er32(SYSTIMH) << 32;
> 
> time_delta = systim_next - systim;
> temp = time_delta;
>  >  rem = do_div(temp, incvalue);
> 
> This change makes kernel survive this, and users report that
> NIC does work after this change.
> 
> Since on real hardware incvalue is never zero, this should not affect
> real hardware use case.
> 
> Signed-off-by: Denys Vlasenko 
> CC: Jeff Kirsher 
> CC: "Ruinskiy, Dima" 
> CC: intel-wired-...@lists.osuosl.org
> CC: netdev@vger.kernel.org
> CC: LKML 
> ---
>  drivers/net/ethernet/intel/e1000e/netdev.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)

As Mark Rustad pointed out I recall this was earlier rejected as something that 
is a VMWare error and it should be fixed there so that existing VMs will start 
working without installing a new driver.  Having said that, it does not seem to 
be causing any harm in my testing, so...

Tested-by: Aaron Brown 


RE: [PATCH] igb: adjust ptp timestamps for tx/rx latency

2016-05-09 Thread Brown, Aaron F
> From: netdev-ow...@vger.kernel.org [mailto:netdev-
> ow...@vger.kernel.org] On Behalf Of Nathan Sullivan
> Sent: Tuesday, May 3, 2016 4:11 PM
> To: Kirsher, Jeffrey T ; Brandeburg, Jesse
> ; Nelson, Shannon
> ; Wyborny, Carolyn
> ; Skidmore, Donald C
> ; Allan, Bruce W ;
> Ronciak, John ; Williams, Mitch A
> 
> Cc: intel-wired-...@lists.osuosl.org; netdev@vger.kernel.org; linux-
> ker...@vger.kernel.org; Nathan Sullivan 
> Subject: [PATCH] igb: adjust ptp timestamps for tx/rx latency
> 
> Table 7-62 on page 338 of the i210 datasheet lists TX and RX latencies
> for the various speeds the chip supports.  To give better ptp timestamp
> accuracy, adjust the timestamps by the amounts Intel gives based on
> current link speed.
> 
> Signed-off-by: Nathan Sullivan 
> ---
>  drivers/net/ethernet/intel/igb/igb.h |8 +++
>  drivers/net/ethernet/intel/igb/igb_ptp.c |   36
> ++
>  2 files changed, 44 insertions(+)

Tested-by: Aaron Brown 



RE: [Intel-wired-lan] [PATCH net-next v3 1/2] e1000e: factor out systim sanitization

2016-08-02 Thread Brown, Aaron F
> From: Intel-wired-lan [mailto:intel-wired-lan-boun...@lists.osuosl.org] On
> Behalf Of Jarod Wilson
> Sent: Monday, August 1, 2016 6:32 PM
> To: Avargil, Raanan 
> Cc: Hall, Christopher S ;
> netdev@vger.kernel.org; intel-wired-...@lists.osuosl.org; linux-
> ker...@vger.kernel.org
> Subject: Re: [Intel-wired-lan] [PATCH net-next v3 1/2] e1000e: factor out
> systim sanitization
> 
> On Wed, Jul 27, 2016 at 11:01:55AM -0400, Jarod Wilson wrote:
> > On Wed, Jul 27, 2016 at 02:09:13PM +, Avargil, Raanan wrote:
> > >> This is prepatory work for an expanding list of adapter families that 
> > >> have
> occasional ~10 hour clock jumps when being used for PTP. Factor out the
> sanitization function and convert to using a feature (bug) flag, per 
> suggestion
> from Jesse Brandeburg.
> > >>
> > >> Littering functional code with device-specific checks is much messier
> than simply checking a flag, and having device-specific init set flags as
> needed.
> > >> There are probably a number of other cases in the e1000e code that
> could/should be converted similarly.
> > >
> > > Looks ok to me.
> > > Adding Chris who asked what happens if we reach the max retry counter
> (E1000_MAX_82574_SYSTIM_REREAD)?
> > > This counter is set to 50.
> > > Can you, for testing purposes, decreased this value (or even set it to 0)
> and see what happens?
> >
> > Unfortunately, I don't have direct access to the affected hardware myself,
> > so I'd have to prep a test build, hand it off to someone and play relay. I
> > could do that, but it'd have some lag and possible multiple round-trips...
> > Anyone inside Intel have hardware handy to test on? :p
> 
> Was tied up with other work the middle of last week, then on vacation for
> a bit. There was some testing feedback provided from someone at neither
> Red Hat or Intel, but I'm not sure where it leaves us right now. What
> needs to happen next?

Probably nothing else needs to be done on your end.  I was out for the last 
week and a half and am now running the patches through a series of regression 
test covering a fair number of the different e1000e parts.  I will also try to 
duplicate Tim Woodford' success on a NUC with an i218 in my lab.  Assuming 
nothing jumps out at me I'll probably give it a tested-by later this week so 
that Jeff can push it on up.

> 
> --
> Jarod Wilson
> ja...@redhat.com
> 
> ___
> Intel-wired-lan mailing list
> intel-wired-...@lists.osuosl.org
> http://lists.osuosl.org/mailman/listinfo/intel-wired-lan


RE: [PATCH] igb: fix adjusting ptp timestamps for tx/rx latency

2016-07-20 Thread Brown, Aaron F
> From: netdev-ow...@vger.kernel.org [mailto:netdev-
> ow...@vger.kernel.org] On Behalf Of Kshitiz Gupta
> Sent: Saturday, July 16, 2016 12:24 AM
> To: Kirsher, Jeffrey T <jeffrey.t.kirs...@intel.com>; Nathan Sullivan
> <nathan.sulli...@ni.com>; Brown, Aaron F <aaron.f.br...@intel.com>
> Cc: intel-wired-...@lists.osuosl.org; netdev@vger.kernel.org; Kshitiz Gupta
> <kshitiz.gu...@ni.com>
> Subject: [PATCH] igb: fix adjusting ptp timestamps for tx/rx latency
> 
> Fix PHY delay compensation math in igb_ptp_tx_hwtstamp() and
> igb_ptp_rx_rgtstamp. Add PHY delay compensation in
> igb_ptp_rx_pktstamp().
> 
> In the IGB driver, there are two functions that retrieve timestamps
> received by the PHY - igb_ptp_rx_rgtstamp() and igb_ptp_rx_pktstamp().
> The previous commit only changed igb_ptp_rx_rgtstamp(), and the change
> was incorrect.
> 
> There are two instances in which PHY delay compensations should be
> made:
> 
> - Before the packet transmission over the PHY, the latency between
>   when the packet is timestamped and transmission of the packets,
>   should be an add operation, but it is currently a subtract.
> 
> - After the packets are received from the PHY, the latency between
>   the receiving and timestamping of the packets should be a subtract
>   operation, but it is currently an add.
> 
> Signed-off-by: Kshitiz Gupta <kshitiz.gu...@ni.com>
> Fixes: 3f544d2 (igb: adjust ptp timestamps for tx/rx latency)
> ---
>  drivers/net/ethernet/intel/igb/igb_ptp.c | 26 +++---
>  1 file changed, 23 insertions(+), 3 deletions(-)

I think I was so engrossed in checking the values for 10 / 100 / 1000 delays 
that I completely missed the reversed add / subtract when looking at 3f544d2.  
Thanks for catching that.
Tested-by: Aaron Brown <aaron.f.br...@intel.com>


RE: [Intel-wired-lan] [PATCH net-next v3 1/2] e1000e: factor out systim sanitization

2016-08-04 Thread Brown, Aaron F
> From: Intel-wired-lan [mailto:intel-wired-lan-boun...@lists.osuosl.org] On
> Behalf Of Woodford, Timothy W.
> Sent: Friday, July 29, 2016 7:41 AM
> To: Woodford, Timothy W. ; Avargil,
> Raanan ; Jarod Wilson ;
> linux-ker...@vger.kernel.org; Hall, Christopher S
> 
> Cc: netdev@vger.kernel.org; intel-wired-...@lists.osuosl.org
> Subject: Re: [Intel-wired-lan] [PATCH net-next v3 1/2] e1000e: factor out
> systim sanitization
> 
> >>> This is prepatory work for an expanding list of adapter families that have
> occasional ~10 hour clock jumps when being used for PTP. Factor out the
> sanitization function and convert to using a feature (bug) flag, per 
> suggestion
> from Jesse Brandeburg.
> >>>
> >>> Littering functional code with device-specific checks is much messier than
> simply checking a flag, and having device-specific init set flags as needed.
> >>> There are probably a number of other cases in the e1000e code that
> could/should be converted similarly.
> >>
> >> Looks ok to me.
> >> Adding Chris who asked what happens if we reach the max retry counter
> (E1000_MAX_82574_SYSTIM_REREAD)?
> >> This counter is set to 50.
> >> Can you, for testing purposes, decreased this value (or even set it to 0)
> and see what happens?
> >  I'll set the max retry counter to 1 and run an overnight test to see what
> happens.
> 
> After running with this configuration for about 36 hours, I haven't seen any
> timing jumps.  Either this configuration eliminates the error, or it makes it
> significantly less likely to occur.
> 
> Tim Woodford

Feel free to throw a Tested-by: on it if you like.  Not a big deal either way, 
I managed to get enough cycles in on it I'm pretty happy with it as well.

> ___
> Intel-wired-lan mailing list
> intel-wired-...@lists.osuosl.org
> http://lists.osuosl.org/mailman/listinfo/intel-wired-lan


RE: [Intel-wired-lan] [PATCH net-next v3 1/2] e1000e: factor out systim sanitization

2016-08-04 Thread Brown, Aaron F
> From: Intel-wired-lan [mailto:intel-wired-lan-boun...@lists.osuosl.org] On
> Behalf Of Jarod Wilson
> Sent: Tuesday, July 26, 2016 11:26 AM
> To: linux-ker...@vger.kernel.org
> Cc: netdev@vger.kernel.org; intel-wired-...@lists.osuosl.org
> Subject: [Intel-wired-lan] [PATCH net-next v3 1/2] e1000e: factor out systim
> sanitization
> 
> This is prepatory work for an expanding list of adapter families that have
> occasional ~10 hour clock jumps when being used for PTP. Factor out the
> sanitization function and convert to using a feature (bug) flag, per
> suggestion from Jesse Brandeburg.
> 
> Littering functional code with device-specific checks is much messier than
> simply checking a flag, and having device-specific init set flags as needed.
> There are probably a number of other cases in the e1000e code that
> could/should be converted similarly.
> 
> Suggested-by: Jesse Brandeburg 
> CC: Jesse Brandeburg 
> CC: Jeff Kirsher 
> CC: intel-wired-...@lists.osuosl.org
> CC: netdev@vger.kernel.org
> Signed-off-by: Jarod Wilson 
> ---
>  drivers/net/ethernet/intel/e1000e/82571.c  |  6 ++-
>  drivers/net/ethernet/intel/e1000e/e1000.h  |  1 +
>  drivers/net/ethernet/intel/e1000e/netdev.c | 66 ++---
> -
>  3 files changed, 44 insertions(+), 29 deletions(-)

Tested-by: Aaron Brown 


RE: [Intel-wired-lan] [PATCH net-next v3 2/2] e1000e: fix PTP on e1000_pch_lpt variants

2016-08-04 Thread Brown, Aaron F
> From: Intel-wired-lan [mailto:intel-wired-lan-boun...@lists.osuosl.org] On
> Behalf Of Jarod Wilson
> Sent: Tuesday, July 26, 2016 11:26 AM
> To: linux-ker...@vger.kernel.org
> Cc: netdev@vger.kernel.org; intel-wired-...@lists.osuosl.org
> Subject: [Intel-wired-lan] [PATCH net-next v3 2/2] e1000e: fix PTP on
> e1000_pch_lpt variants
> 
> I've got reports that the Intel I-218V NIC in Intel NUC5i5RYH systems used
> as a PTP slave experiences random ~10 hour clock jumps, which are resolved
> if the same workaround for the 82574 and 82583 is employed, so set the
> appropriate flag2 in e1000_pch_lpt_info too.
> 
> Reported-by: Rupesh Patel 
> CC: Jesse Brandeburg 
> CC: Jeff Kirsher 
> CC: intel-wired-...@lists.osuosl.org
> CC: netdev@vger.kernel.org
> Signed-off-by: Jarod Wilson 
> ---
>  drivers/net/ethernet/intel/e1000e/ich8lan.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)

Tested-by: Aaron Brown 


RE: [Intel-wired-lan] e1000e: PHY cann't be initialized correctly on some I218 controllers

2016-08-09 Thread Brown, Aaron F


> -Original Message-
> From: Avargil, Raanan
> Sent: Tuesday, August 9, 2016 8:11 AM
> To: Denis Turischev <denis.turisc...@compulab.co.il>; intel-wired-
> l...@lists.osuosl.org; Brown, Aaron F <aaron.f.br...@intel.com>; Kirsher,
> Jeffrey T <jeffrey.t.kirs...@intel.com>
> Cc: netdev@vger.kernel.org; linux-ker...@vger.kernel.org
> Subject: Re: [Intel-wired-lan] e1000e: PHY cann't be initialized correctly on
> some I218 controllers
> 
> 
> 
> On 8/7/2016 11:56, Denis Turischev wrote:
> > Hi Raanan,
> >
> > On 08/04/2016 07:18 PM, Raanan Avargil wrote:
> >> It is hard to determine what is exactly the problem here and what causes
> the -3 error, but this is not the place to perform hw reset and retry to init 
> the
> phy.
> > I agree that may be this is not the correct place for hw reset, can you 
> > advice
> about another solution?
> >
> > It works, not only for me, for example it helps here:
> > http://www.linux.org.ru/forum/linux-hardware/10268216
> >
> > Another fact that Windows drivers work fine with the same hw.
> >
> > Denis
> >
> 
> Hi Denis,
> 
> More suitable place to retry init the device would be in probe function.
> However, we haven't seen initialization issues like this on i218.
> 
> Adding Aaron.
> 
> @Aaron - have you seen initialization errors on i218 devices?

I have not.  But I don't have many systems with i218 so all that means is my 
systems do not exhibit the problem.

> 
> Raanan


RE: [PATCH] net: intel: e1000e: use new api ethtool_{get|set}_link_ksettings

2017-02-21 Thread Brown, Aaron F
> From: netdev-ow...@vger.kernel.org [mailto:netdev-
> ow...@vger.kernel.org] On Behalf Of Philippe Reynes
> Sent: Thursday, January 26, 2017 1:20 PM
> To: Kirsher, Jeffrey T ; da...@davemloft.net
> Cc: intel-wired-...@lists.osuosl.org; netdev@vger.kernel.org; linux-
> ker...@vger.kernel.org; Philippe Reynes 
> Subject: [PATCH] net: intel: e1000e: use new api
> ethtool_{get|set}_link_ksettings
> 
> The ethtool api {get|set}_settings is deprecated.
> We move this driver to new api {get|set}_link_ksettings.
> 
> As I don't have the hardware, I'd be very pleased if
> someone may test this patch.
> 
> Signed-off-by: Philippe Reynes 
> ---
>  drivers/net/ethernet/intel/e1000e/ethtool.c |   91 ++-
> ---
>  1 files changed, 49 insertions(+), 42 deletions(-)

Tested-by: Aaron Brown 



RE: [PATCH] net: intel: e1000: use new api ethtool_{get|set}_link_ksettings

2017-02-24 Thread Brown, Aaron F
> From: netdev-ow...@vger.kernel.org [mailto:netdev-
> ow...@vger.kernel.org] On Behalf Of Philippe Reynes
> Sent: Saturday, January 21, 2017 7:06 AM
> To: Kirsher, Jeffrey T ; da...@davemloft.net
> Cc: intel-wired-...@lists.osuosl.org; netdev@vger.kernel.org; linux-
> ker...@vger.kernel.org; Philippe Reynes 
> Subject: [PATCH] net: intel: e1000: use new api
> ethtool_{get|set}_link_ksettings
> 
> The ethtool api {get|set}_settings is deprecated.
> We move this driver to new api {get|set}_link_ksettings.
> 
> As I don't have the hardware, I'd be very pleased if
> someone may test this patch.
> 
> Signed-off-by: Philippe Reynes 
> ---
>  drivers/net/ethernet/intel/e1000/e1000_ethtool.c |   93 +++-
> --
>  1 files changed, 46 insertions(+), 47 deletions(-)

Tested-by: Aaron Brown 


RE: [Intel-wired-lan] [net-next PATCH v3 2/3] e1000: add initial XDP support

2016-09-14 Thread Brown, Aaron F
> From: Intel-wired-lan [mailto:intel-wired-lan-boun...@lists.osuosl.org] On
> Behalf Of Rustad, Mark D
> Sent: Tuesday, September 13, 2016 3:41 PM
> To: Alexei Starovoitov 
> Cc: Eric Dumazet ; Tom Herbert
> ; Brenden Blanco ; Linux
> Kernel Network Developers ; intel-wired-lan  wired-...@lists.osuosl.org>; William Tu ; Jesper
> Dangaard Brouer ; Cong Wang
> ; David S. Miller 
> Subject: Re: [Intel-wired-lan] [net-next PATCH v3 2/3] e1000: add initial XDP
> support
> 
> Alexei Starovoitov  wrote:
> 
> > On Tue, Sep 13, 2016 at 07:14:27PM +, Rustad, Mark D wrote:
> >> Alexei Starovoitov  wrote:
> >>
> >>> On Tue, Sep 13, 2016 at 06:28:03PM +, Rustad, Mark D wrote:
>  Alexei Starovoitov  wrote:
> 
> > I've looked through qemu and it appears only emulate e1k and tg3.
> > The latter is still used in the field, so the risk of touching
> > it is higher.
> 
>  I have no idea what makes you think that e1k is *not* "used in the
>  field".

I personally use a number of physical e1000 parts in my lab.  Primarily for an 
out of band control adapter, as it almost never changes so tends to be very 
stable, and it is rare that I have to test it so I'm not fighting with a 
control adapter that I am also testing the driver on.

>  I grant you it probably is used more virtualized than not these days,
>  but it
>  certainly exists and is used. You can still buy them new at Newegg for
>  goodness sakes!
> >>>
> >>> the point that it's only used virtualized, since PCI (not PCIE) have
> >>> been long dead.

Maybe for new server installations.  But modern motherboards often still have 
PCI, university / lab environments are slow to upgrade specialty test equipment 
that uses PCI and home users often don't want to get rid of their expensive 
high quality sound card, video capture card (or whatever.)  Plain old PCI is 
still in use.

> >>
> >> My point is precisely the opposite. It is a real device, it exists in real
> >> systems and it is used in those systems. I worked on embedded systems
> that
> >> ran Linux and used e1000 devices. I am sure they are still out there
> >> because
> >> customers are still paying for support of those systems.
> >>
> >> Yes, PCI(-X) is absent from any current hardware and has been for some
> >> years
> >> now, but there is an installed base that continues. What part of that
> >> installed base updates software? I don't know, but I would not just
> assume
> >> that it is 0. I know that I updated the kernel on those embedded systems
> >> that I worked on when I was supporting them. Never at the bleeding edge,
> >> but
> >> generally hopping from one LTS kernel to another as needed.
> >
> > I suspect modern linux won't boot on such old pci only systems for other
> > reasons not related to networking, since no one really cares to test
> > kernels there.
> 
> Actually it does boot, because although the motherboard was PCIe, the slots
> and the adapters in them were PCI-X. So the core architecture was not so
> stale.
> 
> > So I think we mostly agree. There is chance that this xdp e1k code will
> > find a way to that old system. What are the chances those users will
> > be using xdp there? I think pretty close to zero.
> 
> For sure they wouldn't be using XDP, but they could suffer regressions in a
> changed driver that might find its way there. That is the risk.
> 
> > The pci-e nics integrated into motherboards that pretend to be tg3
> > (though they're not at all build by broadcom) are significantly more
> > common.
> > That's why I picked e1k instead of tg3.
> 
> That may be true (I really don't know anything about tg3 so I certainly
> can't dispute it), so the risk could be smaller with e1k, but there is
> still a regression risk for real existing hardware. That is my concern.

And one of the patches in supporting this seems to have done just that.  I'll 
reply to the patch itself with details.
> 
> > Also note how this patch is not touching anything in the main e1k path
> > (because I don't have a hw to test and I suspect Intel's driver team
> > doesn't have it either)

Your suspicion is incorrect.  My lab has multiple boxes out on benches and at 
least one cabinet 3/4 full of ancient hardware, including a decent variety of 
e1000 cards (and even a number of e100 ones.)  I also keep a handful of test 
systems in my regression rack devoted to e1000, though those have dwindled (old 
hardware eventually dies) and is currently in need of a bit of work, which this 
patch set has encouraged me to get on to.

> >  to make sure there is no breakage on those
> > old systems. I created separate e1000_xmit_raw_frame() routine
> > instead of adding flags into 

RE: [Intel-wired-lan] [net-next PATCH v3 1/3] e1000: track BQL bytes regardless of skb or not

2016-09-14 Thread Brown, Aaron F
> From: Intel-wired-lan [mailto:intel-wired-lan-boun...@lists.osuosl.org] On
> Behalf Of John Fastabend
> Sent: Monday, September 12, 2016 3:13 PM
> To: bbla...@plumgrid.com; john.fastab...@gmail.com;
> alexei.starovoi...@gmail.com; Kirsher, Jeffrey T
> ; bro...@redhat.com; da...@davemloft.net
> Cc: xiyou.wangc...@gmail.com; intel-wired-...@lists.osuosl.org;
> u9012...@gmail.com; netdev@vger.kernel.org
> Subject: [Intel-wired-lan] [net-next PATCH v3 1/3] e1000: track BQL bytes
> regardless of skb or not
> 
> The BQL API does not reference the sk_buff nor does the driver need to
> reference the sk_buff to calculate the length of a transmitted frame.
> This patch removes an sk_buff reference from the xmit irq path and
> also allows packets sent from XDP to use BQL.
> 
> Signed-off-by: John Fastabend 
> ---
>  drivers/net/ethernet/intel/e1000/e1000_main.c |7 ++-
>  1 file changed, 2 insertions(+), 5 deletions(-)

This patch is causing all my e1000 adapters to fail a simple ftp session with 
really slow response (hashing on) followed by adapter resets.  I have seen this 
on 4 different e1000 nics now, an 82543GC, 82544GC, 82546EB and an 82545GM.  On 
a few occasions I get a splat captured to dmesg.  Here is an example:

[ cut here ]
WARNING: CPU: 1 PID: 0 at net/sched/sch_generic.c:316 dev_watchdog+0x1c2/0x1d0
NETDEV WATCHDOG: eth1 (e1000): transmit queue 0 timed out
Modules linked in: e1000 e100 nfsd lockd grace nfs_acl auth_rpcgss sunrpc autofs
4 ipv6 crc_ccitt p4_clockmod dm_mirror dm_region_hash dm_log uinput sg serio_raw
 dcdbas mii i2c_piix4 i2c_core cfi_probe gen_probe cfi_util scb2_flash dm_mod(E)
 ext4(E) mbcache(E) jbd2(E) sd_mod(E) sr_mod(E) cdrom(E) aacraid(E) pata_acpi(E)
 ata_generic(E) pata_serverworks(E) [last unloaded: e1000]
CPU: 1 PID: 0 Comm: swapper/1 Tainted: GE   4.8.0-rc6_next-queue_dev
-queue_b0b5ade #8
Hardware name: Dell Computer Corporation PowerEdge 2650 /0D5995, BIO
S A21 10/05/2006
  c06724fb c0b6038a c0b6038a 013c c04596d5 c0b647ac f3d2fed0
  c0b6038a 013c c08bff52 c08bff52 0009 f2922000 
 f318cf00 0001e788 c045979b 0009  f3d2feb8 c0b647ac f3d2fed0
Call Trace:
 [] ? dump_stack+0x47/0x6c
 [] ? __warn+0x105/0x120
 [] ? dev_watchdog+0x1c2/0x1d0
 [] ? dev_watchdog+0x1c2/0x1d0
 [] ? warn_slowpath_fmt+0x3b/0x40
 [] ? dev_watchdog+0x1c2/0x1d0
 [] ? call_timer_fn+0x3b/0x140
 [] ? dev_trans_start+0x60/0x60
 [] ? expire_timers+0x9b/0x110
 [] ? run_timer_softirq+0xd1/0x260
 [] ? net_tx_action+0xe0/0x1a0
 [] ? rcu_process_callbacks+0x47/0xe0
 [] ? __do_softirq+0xc8/0x280
 [] ? irq_exit+0x90/0x90
 [] ? do_softirq_own_stack+0x22/0x30
   [] ? irq_exit+0x85/0x90
 [] ? smp_apic_timer_interrupt+0x30/0x40
 [] ? apic_timer_interrupt+0x2d/0x34
 [] ? default_idle+0x1c/0xd0
 [] ? __tick_nohz_idle_enter+0x92/0x140
 [] ? arch_cpu_idle+0x6/0x10
 [] ? cpuidle_idle_call+0x7d/0xe0
 [] ? cpu_idle_loop+0x155/0x210
 [] ? start_secondary+0xb5/0xe0
---[ end trace fa448b49f7848a42 ]---
e1000 :02:06.0 eth1: Reset adapter
e1000: eth1 NIC Link is Up 1000 Mbps Full Duplex, Flow Control: RX/TX
e1000 :02:06.0 eth1: Reset adapter
e1000: eth1 NIC Link is Up 1000 Mbps Full Duplex, Flow Control: RX/TX
e1000 :02:06.0 eth1: Reset adapter
e1000: eth1 NIC Link is Up 1000 Mbps Full Duplex, Flow Control: RX/TX
e1000 :02:06.0 eth1: Reset adapter
e1000: eth1 NIC Link is Up 1000 Mbps Full Duplex, Flow Control: RX/TX
e1000 :02:06.0 eth1: Reset adapter
e1000: eth1 NIC Link is Up 1000 Mbps Full Duplex, Flow Control: RX/TX
e1000 :02:06.0 eth1: Reset adapter
e1000: eth1 NIC Link is Up 1000 Mbps Full Duplex, Flow Control: RX/TX
e1000 :02:06.0 eth1: Reset adapter
e1000: eth1 NIC Link is Up 1000 Mbps Full Duplex, Flow Control: RX/TX


RE: [Intel-wired-lan] [net-next PATCH v3 1/3] e1000: track BQL bytes regardless of skb or not

2016-09-15 Thread Brown, Aaron F
> > [ cut here ]
> > WARNING: CPU: 1 PID: 0 at net/sched/sch_generic.c:316
> dev_watchdog+0x1c2/0x1d0
> > NETDEV WATCHDOG: eth1 (e1000): transmit queue 0 timed out
> 
> Thanks a lot for the tests! Really appreciate it.

np, I needed to get my old compatibility systems back in running order anyway.


RE: [PATCH] igb: Realign bad indentation

2016-10-04 Thread Brown, Aaron F
> From: netdev-ow...@vger.kernel.org [mailto:netdev-
> ow...@vger.kernel.org] On Behalf Of Joe Perches
> Sent: Monday, September 26, 2016 8:46 PM
> To: Kirsher, Jeffrey T 
> Cc: intel-wired-...@lists.osuosl.org; netdev@vger.kernel.org; linux-
> ker...@vger.kernel.org
> Subject: [PATCH] igb: Realign bad indentation
> 
> Statements should start on tabstops.
> 
> Use a single statement and test instead of multiple tests.
> 
> Signed-off-by: Joe Perches 
> ---
>  drivers/net/ethernet/intel/igb/e1000_mac.c | 15 ++-
>  1 file changed, 6 insertions(+), 9 deletions(-)

Tested-by: Aaron Brown 


RE: [PATCH -next] igb: fix non static symbol warning

2016-08-25 Thread Brown, Aaron F
> From: netdev-ow...@vger.kernel.org [mailto:netdev-
> ow...@vger.kernel.org] On Behalf Of Wei Yongjun
> Sent: Tuesday, August 23, 2016 8:08 AM
> To: Kirsher, Jeffrey T 
> Cc: Wei Yongjun ; intel-wired-
> l...@lists.osuosl.org; netdev@vger.kernel.org
> Subject: [PATCH -next] igb: fix non static symbol warning
> 
> From: Wei Yongjun 
> 
> Fixes the following sparse warning:
> 
> drivers/net/ethernet/intel/igb/igb_ethtool.c:2707:5: warning:
>  symbol 'igb_rxnfc_write_vlan_prio_filter' was not declared. Should it be
> static?
> 
> Signed-off-by: Wei Yongjun 
> ---
>  drivers/net/ethernet/intel/igb/igb_ethtool.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)

Tested-by: Aaron Brown 


RE: [PATCH] igb/e1000: correct register comments

2016-11-07 Thread Brown, Aaron F
> From: netdev-ow...@vger.kernel.org [mailto:netdev-
> ow...@vger.kernel.org] On Behalf Of Cao jin
> Sent: Wednesday, November 2, 2016 12:20 AM
> To: linux-ker...@vger.kernel.org; netdev@vger.kernel.org
> Cc: intel-wired-...@lists.osuosl.org; Kirsher, Jeffrey T
> 
> Subject: [PATCH] igb/e1000: correct register comments
> 
> Signed-off-by: Cao jin 
> ---
>  drivers/net/ethernet/intel/igb/e1000_regs.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

Tested-by: Aaron Brown 


RE: [PATCH] igb: Workaround for igb i210 firmware issue.

2016-11-07 Thread Brown, Aaron F
> From: netdev-ow...@vger.kernel.org [mailto:netdev-
> ow...@vger.kernel.org] On Behalf Of Chris J Arges
> Sent: Wednesday, November 2, 2016 7:14 AM
> To: j...@henneberg-systemdesign.com
> Cc: intel-wired-...@lists.osuosl.org; Chris J Arges
> ; Kirsher, Jeffrey T
> ; netdev@vger.kernel.org; linux-
> ker...@vger.kernel.org
> Subject: [PATCH] igb: Workaround for igb i210 firmware issue.
> 
> Sometimes firmware may not properly initialize I347AT4_PAGE_SELECT
> causing
> the probe of an igb i210 NIC to fail. This patch adds an addition zeroing of
> this register during igb_get_phy_id to workaround this issue.
> 
> Thanks for Jochen Henneberg for the idea and original patch.
> 
> Signed-off-by: Chris J Arges 
> ---
>  drivers/net/ethernet/intel/igb/e1000_phy.c | 4 
>  1 file changed, 4 insertions(+)

Tested-by: Aaron Brown 


RE: [Intel-wired-lan] [PATCH] igb: use igb_adapter->io_addr instead of e1000_hw->hw_addr

2016-11-23 Thread Brown, Aaron F
> From: Intel-wired-lan [intel-wired-lan-boun...@lists.osuosl.org] on behalf of 
> Cao jin [caoj.f...@cn.fujitsu.com]
> Sent: Monday, November 07, 2016 11:06 PM
To> : linux-ker...@vger.kernel.org; netdev@vger.kernel.org
> Cc: izumi.t...@jp.fujitsu.com; intel-wired-...@lists.osuosl.org
> Subject: [Intel-wired-lan] [PATCH] igb: use igb_adapter->io_addr instead of   
>   e1000_hw->hw_addr
> 
> When running as guest, under certain condition, it will oops as following.
> writel() in igb_configure_tx_ring() results in oops, because hw->hw_addr
> is NULL. While other register access won't oops kernel because they use
> wr32/rd32 which have a defense against NULL pointer.
> 
> [  141.225449] pcieport :00:1c.0: AER: Multiple Uncorrected (Fatal)
> error received: id=0101
> [  141.225523] igb :01:00.1: PCIe Bus Error:
> severity=Uncorrected (Fatal), type=Unaccessible,
> id=0101(Unregistered Agent ID)
> [  141.299442] igb :01:00.1: broadcast error_detected message
> [  141.300539] igb :01:00.0 enp1s0f0: PCIe link lost, device now
> detached
> [  141.351019] igb :01:00.1 enp1s0f1: PCIe link lost, device now
> detached
> [  143.465904] pcieport :00:1c.0: Root Port link has been reset
> [  143.465994] igb :01:00.1: broadcast slot_reset message
> [  143.466039] igb :01:00.0: enabling device ( -> 0002)
> [  144.389078] igb :01:00.1: enabling device ( -> 0002)
> [  145.312078] igb :01:00.1: broadcast resume message
> [  145.322211] BUG: unable to handle kernel paging request at
> 3818
> [  145.361275] IP: []
> igb_configure_tx_ring+0x14d/0x280 [igb]
> [  145.400048] PGD 0
> [  145.438007] Oops: 0002 [#1] SMP
> 
> A similiar issue & solution could be found at:
> http://patchwork.ozlabs.org/patch/689592/
> 
> Signed-off-by: Cao jin 
> ---
>  drivers/net/ethernet/intel/igb/igb_main.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)

Tested-by: Aaron Brown 


RE: [PATCH] e1000e: x86: e1000 driver trying to free already-free irq.

2016-10-27 Thread Brown, Aaron F
> From: netdev-ow...@vger.kernel.org [mailto:netdev-
> ow...@vger.kernel.org] On Behalf Of David Singleton
> Sent: Monday, October 17, 2016 9:51 AM
> To: Kirsher, Jeffrey T 
> Cc: khalidm ; intel-wired-...@lists.osuosl.org;
> netdev@vger.kernel.org; linux-ker...@vger.kernel.org
> Subject: [PATCH] e1000e: x86: e1000 driver trying to free already-free irq.
> 
> From: khalidm 
> 
> During systemd reboot sequence network driver interface is shutdown
> by e1000_close. The PCI driver interface is shut by e1000_shutdown.
> The e1000_shutdown checks for netif_running status, if still up it
> brings down driver. But it disables msi outside of this if statement,
> regardless of netif status. All this is OK when e1000_close happens
> after shutdown. However, by default, everything in systemd is done
> in parallel. This creates a conditions where e1000_shutdown is called
> after e1000_close, therefore hitting BUG_ON assert in free_msi_irqs.
> 
> Cc-Id: xe-ker...@external.cisco.com
> Signed-off-by: khalidm 
> Signed-off-by: David Singleton 
> ---
>  drivers/net/ethernet/intel/e1000e/netdev.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

Tested-by: Aaron Brown 


RE: [Intel-wired-lan] [PATCH net v2] igb: re-assign hw address pointer on reset after PCI error

2016-12-13 Thread Brown, Aaron F
> From: Intel-wired-lan [mailto:intel-wired-lan-boun...@lists.osuosl.org] On
> Behalf Of Guilherme G. Piccoli
> Sent: Thursday, November 10, 2016 10:47 AM
> To: intel-wired-...@lists.osuosl.org
> Cc: netdev@vger.kernel.org; gpicc...@linux.vnet.ibm.com
> Subject: [Intel-wired-lan] [PATCH net v2] igb: re-assign hw address pointer on
> reset after PCI error
> 
> Whenever the igb driver detects the result of a read operation returns
> a value composed only by F's (like 0x), it will detach the
> net_device, clear the hw_addr pointer and warn to the user that adapter's
> link is lost - those steps happen on igb_rd32().
> 
> In case a PCI error happens on Power architecture, there's a recovery
> mechanism called EEH, that will reset the PCI slot and call driver's
> handlers to reset the adapter and network functionality as well.
> 
> We observed that once hw_addr is NULL after the error is detected on
> igb_rd32(), it's never assigned back, so in the process of resetting
> the network functionality we got a NULL pointer dereference in both
> igb_configure_tx_ring() and igb_configure_rx_ring(). In order to avoid
> such bug, this patch re-assigns the hw_addr value in the slot_reset
> handler.
> 
> Reported-by: Anthony H. Thai 
> Reported-by: Harsha Thyagaraja 
> Signed-off-by: Guilherme G. Piccoli 
> ---
>  drivers/net/ethernet/intel/igb/igb_main.c | 5 +
>  1 file changed, 5 insertions(+)

Tested-by: Aaron Brown 


RE: [PATCH 1/1] igb: Fix hw_dbg logging in igb_update_flash_i210

2017-01-05 Thread Brown, Aaron F
> From: netdev-ow...@vger.kernel.org [mailto:netdev-
> ow...@vger.kernel.org] On Behalf Of Peter Senna Tschudin
> Sent: Monday, January 2, 2017 9:26 AM
> Cc: Hannu Lounento ; Peter Senna Tschudin
> ; Kirsher, Jeffrey T
> ; moderated list:INTEL ETHERNET DRIVERS
> ; open list:NETWORKING DRIVERS
> ; open list 
> Subject: [PATCH 1/1] igb: Fix hw_dbg logging in igb_update_flash_i210
> 
> From: Hannu Lounento 
> 
> Fix an if statement with hw_dbg lines where the logic was inverted with
> regards to the corresponding return value used in the if statement.
> 
> Signed-off-by: Hannu Lounento 
> Signed-off-by: Peter Senna Tschudin 
> ---
>  drivers/net/ethernet/intel/igb/e1000_i210.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)

Tested-by: Aaron Brown 


RE: [PATCH] e1000e: fix timing for 82579 Gigabit Ethernet controller

2017-03-23 Thread Brown, Aaron F
> From: netdev-ow...@vger.kernel.org [mailto:netdev-
> ow...@vger.kernel.org] On Behalf Of Bernd Faust
> Sent: Thursday, February 16, 2017 10:42 AM
> To: Kirsher, Jeffrey T ; Lubetkin, YanirX
> ; intel-wired-...@lists.osuosl.org;
> netdev@vger.kernel.org; linux-ker...@vger.kernel.org
> Cc: Bernd Faust 
> Subject: [PATCH] e1000e: fix timing for 82579 Gigabit Ethernet controller
> 
> After an upgrade to Linux kernel v4.x the hardware timestamps of the
> 82579 Gigabit Ethernet Controller are different than expected.
> The values that are being read are almost four times as big as before
> the kernel upgrade.
> 
> The difference is that after the upgrade the driver sets the clock
> frequency to 25MHz, where before the upgrade it was set to 96MHz. Intel
> confirmed that the correct frequency for this network adapter is 96MHz.
> 
> Signed-off-by: Bernd Faust 
> ---
>  drivers/net/ethernet/intel/e1000e/netdev.c | 6 ++
>  1 file changed, 6 insertions(+)

Tested-by: Aaron Brown 


RE: [PATCH net-next] ixgb: Omit private ndo_get_stats function

2017-03-24 Thread Brown, Aaron F
> From: netdev-ow...@vger.kernel.org [mailto:netdev-
> ow...@vger.kernel.org] On Behalf Of Tobias Klauser
> Sent: Wednesday, February 15, 2017 3:08 AM
> To: netdev@vger.kernel.org
> Cc: Kirsher, Jeffrey T ; intel-wired-
> l...@lists.osuosl.org; Joe Perches 
> Subject: [PATCH net-next] ixgb: Omit private ndo_get_stats function
> 
> ixgb_get_stats() just returns dev->stats so we can leave it
> out altogether and let dev_get_stats() do the job.
> 
> Suggested-by: Joe Perches 
> Signed-off-by: Tobias Klauser 
> ---
>  drivers/net/ethernet/intel/ixgb/ixgb_main.c | 16 
>  1 file changed, 16 deletions(-)

Well, well.  I managed to locate one of my ixgb based adapters as well as a 
system with a working PCI-X bus to host it.

Tested-by: Aaron Brown 


RE: [PATCH net-next] e1000: Omit private ndo_get_stats function

2017-03-22 Thread Brown, Aaron F
> From: netdev-ow...@vger.kernel.org [mailto:netdev-
> ow...@vger.kernel.org] On Behalf Of Tobias Klauser
> Sent: Wednesday, February 15, 2017 3:08 AM
> To: netdev@vger.kernel.org
> Cc: Kirsher, Jeffrey T ; intel-wired-
> l...@lists.osuosl.org; Joe Perches 
> Subject: [PATCH net-next] e1000: Omit private ndo_get_stats function
> 
> e1000_get_stats() just returns dev->stats so we can leave it
> out altogether and let dev_get_stats() do the job.
> 
> Suggested-by: Joe Perches 
> Signed-off-by: Tobias Klauser 
> ---
>  drivers/net/ethernet/intel/e1000/e1000_main.c | 15 ---
>  1 file changed, 15 deletions(-)

Tested-by: Aaron Brown 


RE: [Intel-wired-lan] [PATCH] net: intel: igbvf: use new api ethtool_{get|set}_link_ksettings

2017-03-13 Thread Brown, Aaron F
> From: Intel-wired-lan [mailto:intel-wired-lan-boun...@lists.osuosl.org] On
> Behalf Of Philippe Reynes
> Sent: Sunday, February 5, 2017 2:55 PM
> To: Kirsher, Jeffrey T ; da...@davemloft.net
> Cc: netdev@vger.kernel.org; intel-wired-...@lists.osuosl.org; linux-
> ker...@vger.kernel.org; Philippe Reynes 
> Subject: [Intel-wired-lan] [PATCH] net: intel: igbvf: use new api
> ethtool_{get|set}_link_ksettings
> 
> The ethtool api {get|set}_settings is deprecated.
> We move this driver to new api {get|set}_link_ksettings.
> 
> As I don't have the hardware, I'd be very pleased if
> someone may test this patch.
> 
> Signed-off-by: Philippe Reynes 
> ---
>  drivers/net/ethernet/intel/igbvf/ethtool.c |   38 
> ++--
>  1 files changed, 19 insertions(+), 19 deletions(-)

Tested-by: Aaron Brown 


RE: [Intel-wired-lan] [PATCH] net: intel: igb: use new api ethtool_{get|set}_link_ksettings

2017-03-13 Thread Brown, Aaron F
> From: Intel-wired-lan [mailto:intel-wired-lan-boun...@lists.osuosl.org] On
> Behalf Of Philippe Reynes
> Sent: Sunday, February 5, 2017 9:56 AM
> To: Kirsher, Jeffrey T ; da...@davemloft.net
> Cc: netdev@vger.kernel.org; intel-wired-...@lists.osuosl.org; linux-
> ker...@vger.kernel.org; Philippe Reynes 
> Subject: [Intel-wired-lan] [PATCH] net: intel: igb: use new api
> ethtool_{get|set}_link_ksettings
> 
> The ethtool api {get|set}_settings is deprecated.
> We move this driver to new api {get|set}_link_ksettings.
> 
> As I don't have the hardware, I'd be very pleased if
> someone may test this patch.
> 
> Signed-off-by: Philippe Reynes 
> ---
>  drivers/net/ethernet/intel/igb/igb_ethtool.c |  108 ++---
> -
>  1 files changed, 59 insertions(+), 49 deletions(-)
> 

Tested-by: Aaron Brown 


RE: [Intel-wired-lan] [PATCH] net: intel: ixgb: use new api ethtool_{get|set}_link_ksettings

2017-03-13 Thread Brown, Aaron F
> From: Intel-wired-lan [mailto:intel-wired-lan-boun...@lists.osuosl.org] On
> Behalf Of Philippe Reynes
> Sent: Sunday, February 5, 2017 3:11 PM
> To: Kirsher, Jeffrey T ; da...@davemloft.net
> Cc: netdev@vger.kernel.org; intel-wired-...@lists.osuosl.org; linux-
> ker...@vger.kernel.org; Philippe Reynes 
> Subject: [Intel-wired-lan] [PATCH] net: intel: ixgb: use new api
> ethtool_{get|set}_link_ksettings
> 
> The ethtool api {get|set}_settings is deprecated.
> We move this driver to new api {get|set}_link_ksettings.
> 
> As I don't have the hardware, I'd be very pleased if
> someone may test this patch.
> 
> Signed-off-by: Philippe Reynes 
> ---
>  drivers/net/ethernet/intel/ixgb/ixgb_ethtool.c |   39 ++-
> -
>  1 files changed, 23 insertions(+), 16 deletions(-)
> 

Tested-by: Aaron Brown 
Well, compile / static tested by:  I probably have an ixgb adapter somewhere as 
well as a PCI-X slot that might work for it, but am not sure I can find them or 
that they still function.  If I stumble upon one I'll fire it up and run it 
through some paces, but the variant of this patch for other drivers seem good 
and I don't want to hold this patch up any longer.


RE: [Intel-wired-lan] [PATCH] net: igbvf: Use net_device_stats from struct net_device

2017-04-17 Thread Brown, Aaron F
> From: Intel-wired-lan [mailto:intel-wired-lan-boun...@lists.osuosl.org] On
> Behalf Of Tobias Klauser
> Sent: Wednesday, April 5, 2017 11:45 PM
> To: Kirsher, Jeffrey T 
> Cc: netdev@vger.kernel.org; intel-wired-...@lists.osuosl.org
> Subject: [Intel-wired-lan] [PATCH] net: igbvf: Use net_device_stats from
> struct net_device
> 
> Instead of using a private copy of struct net_device_stats in
> struct igbvf_adapter, use stats from struct net_device. Also remove the
> now unnecessary .ndo_get_stats function.
> 
> Signed-off-by: Tobias Klauser 
> ---
>  drivers/net/ethernet/intel/igbvf/igbvf.h  |  1 -
>  drivers/net/ethernet/intel/igbvf/netdev.c | 26 +-
>  2 files changed, 5 insertions(+), 22 deletions(-)

Tested-by: Aaron Brown 


RE: [Intel-wired-lan] [PATCH] igb: support BCM54616 PHY

2017-07-26 Thread Brown, Aaron F
> From: Intel-wired-lan [mailto:intel-wired-lan-boun...@osuosl.org] On Behalf
> Of John W. Linville
> Sent: Friday, July 21, 2017 11:12 AM
> To: netdev@vger.kernel.org
> Cc: intel-wired-...@lists.osuosl.org; John W. Linville
> 
> Subject: [Intel-wired-lan] [PATCH] igb: support BCM54616 PHY
> 
> The management port on an Edgecore AS7712-32 switch uses an igb MAC,
> but
> it uses a BCM54616 PHY. Without a patch like this, loading the igb
> module produces dmesg output like this:
> 
> [3.439125] igb: Copyright (c) 2007-2014 Intel Corporation.
> [3.439866] igb: probe of :00:14.0 failed with error -2
> 
> Signed-off-by: John W. Linville 
> Cc: Jeff Kirsher 
> ---
>  drivers/net/ethernet/intel/igb/e1000_82575.c   | 6 ++
>  drivers/net/ethernet/intel/igb/e1000_defines.h | 1 +
>  drivers/net/ethernet/intel/igb/e1000_hw.h  | 1 +
>  3 files changed, 8 insertions(+)

I do not have the specific hardware (Edgecore switch) but as far as regression 
tests go this works fine.
Tested-by: Aaron Brown 


RE: [Intel-wired-lan] [PATCH 1/2] e1000e: Don't return uninitialized stats

2017-04-25 Thread Brown, Aaron F
> From: Intel-wired-lan [mailto:intel-wired-lan-boun...@lists.osuosl.org] On
> Behalf Of Benjamin Poirier
> Sent: Monday, April 24, 2017 12:10 PM
> To: Neftin, Sasha 
> Cc: kirs...@f1.synalogic.ca; Stefan Priebe ;
> netdev@vger.kernel.org; intel-wired-...@lists.osuosl.org
> Subject: Re: [Intel-wired-lan] [PATCH 1/2] e1000e: Don't return uninitialized
> stats
> 
> Sasha, please use reply-all to keep everyone in cc (including me...).
> 
> On 2017/04/24 11:17, Neftin, Sasha wrote:
> > On 4/23/2017 15:53, Neftin, Sasha wrote:
> > > -Original Message-
> > > From: Intel-wired-lan [mailto:intel-wired-lan-boun...@lists.osuosl.org]
> On Behalf Of Benjamin Poirier
> > > Sent: Saturday, April 22, 2017 00:20
> > > To: Kirsher, Jeffrey T 
> > > Cc: netdev@vger.kernel.org; intel-wired-...@lists.osuosl.org; Stefan
> Priebe 
> > > Subject: [Intel-wired-lan] [PATCH 1/2] e1000e: Don't return uninitialized
> stats
> > >
> > > Some statistics passed to ethtool are garbage because
> e1000e_get_stats64() doesn't write them, for example: tx_heartbeat_errors.
> This leaks kernel memory to userspace and confuses users.
> > >
> > > Do like ixgbe and use dev_get_stats() which first zeroes out
> rtnl_link_stats64.
> > >
> > > Reported-by: Stefan Priebe 
> > > Signed-off-by: Benjamin Poirier 
> > > ---
> > >   drivers/net/ethernet/intel/e1000e/ethtool.c | 2 +-
> > >   1 file changed, 1 insertion(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/net/ethernet/intel/e1000e/ethtool.c
> b/drivers/net/ethernet/intel/e1000e/ethtool.c
> > > index 7aff68a4a4df..f117b90cdc2f 100644
> > > --- a/drivers/net/ethernet/intel/e1000e/ethtool.c
> > > +++ b/drivers/net/ethernet/intel/e1000e/ethtool.c
> > > @@ -2063,7 +2063,7 @@ static void e1000_get_ethtool_stats(struct
> net_device *netdev,
> > >   pm_runtime_get_sync(netdev->dev.parent);
> > > - e1000e_get_stats64(netdev, _stats);
> > > + dev_get_stats(netdev, _stats);
> > >   pm_runtime_put_sync(netdev->dev.parent);
> > > --
> > > 2.12.2
> > >
> > > ___
> > > Intel-wired-lan mailing list
> > > intel-wired-...@lists.osuosl.org
> > > http://lists.osuosl.org/mailman/listinfo/intel-wired-lan
> >
> > Hello,
> >
> > We would like to not accept this patch. Suggested generic method
> > '*dev_get_stats' (net/core/dev.c) calls 'ops->ndo_get_stats64' method
> which
> > eventually calls e1000e_get_stats64 (netdev.c) - so there is same
> > functionality. Also, see that 'e1000e_get_stats64' method in netdev.c (line
> 
> No, it's not the same functionality because dev_get_stats() does a
> memset on the rtnl_link_stats64 struct.
> 
> > 5928) calls 'memset' with 0's before update statistics.  Local sanity check
> 
> I don't see any memset in e1000e_get_stats64(). What kernel version are
> you looking at?

The call to memset was removed from the upstream kernel with:

commit 5944701df90d9577658e2354cc27c4ceaeca30fe
Author: stephen hemminger 
Date:   Fri Jan 6 19:12:53 2017 -0800

net: remove useless memset's in drivers get_stats64

In dev_get_stats() the statistic structure storage has already been
zeroed. Therefore network drivers do not need to call memset() again.
...
< changes to other drivers snipped out >
...
diff --git a/drivers/net/ethernet/intel/e1000e/netdev.c 
b/drivers/net/ethernet/int
index 723025b..79651eb 100644
--- a/drivers/net/ethernet/intel/e1000e/netdev.c
+++ b/drivers/net/ethernet/intel/e1000e/netdev.c
@@ -5925,7 +5925,6 @@ void e1000e_get_stats64(struct net_device *netdev,
 {
struct e1000_adapter *adapter = netdev_priv(netdev);

-   memset(stats, 0, sizeof(struct rtnl_link_stats64));
spin_lock(>stats64_lock);
e1000e_update_stats(adapter);
/* Fill out the OS statistics structure */


This also is where the bad counters start to show up for e1000e for my test 
systems.  From this driver on I see (very) large values for tx_dropped, 
rx_over_errors and tx_fifo_errors on driver load (even before bringing the 
interface up.  It seems the memset is not so useless for this driver after all. 
 Would simply reverting the e1000e portion of this patch resolve the issue?

> 
> > in our lab shows 'tx_heartbeat_errors' counter reported as 0.
> >
> 
> Please see the mail I just sent to Paul Menzel 
> for more information about the issue and how to reproduce it.
> ___
> Intel-wired-lan mailing list
> intel-wired-...@lists.osuosl.org
> http://lists.osuosl.org/mailman/listinfo/intel-wired-lan


RE: [Intel-wired-lan] [PATCH] net: intel: e1000e: add check on e1e_wphy() return value

2017-06-26 Thread Brown, Aaron F
> From: Intel-wired-lan [mailto:intel-wired-lan-boun...@osuosl.org] On Behalf
> Of Gustavo A. R. Silva
> Sent: Tuesday, June 20, 2017 2:23 PM
> To: Kirsher, Jeffrey T 
> Cc: netdev@vger.kernel.org; intel-wired-...@lists.osuosl.org; linux-
> ker...@vger.kernel.org; Gustavo A. R. Silva 
> Subject: [Intel-wired-lan] [PATCH] net: intel: e1000e: add check on
> e1e_wphy() return value
> 
> Check return value from call to e1e_wphy(). This value is being
> checked during previous calls to function e1e_wphy() and it seems
> a check was missing here.
> 
> Addresses-Coverity-ID: 1226905
> Signed-off-by: Gustavo A. R. Silva 
> ---
>  drivers/net/ethernet/intel/e1000e/ich8lan.c | 2 ++
>  1 file changed, 2 insertions(+)

Tested-by: Aaron Brown 


RE: [PATCH] e1000e: use disable_hardirq() also for MSIX vectors in e1000_netpoll()

2017-05-26 Thread Brown, Aaron F
> From: netdev-ow...@vger.kernel.org [mailto:netdev-
> ow...@vger.kernel.org] On Behalf Of Konstantin Khlebnikov
> Sent: Friday, May 19, 2017 12:19 AM
> To: netdev@vger.kernel.org; intel-wired-...@lists.osuosl.org; Kirsher, Jeffrey
> T 
> Cc: Dave Jones ; WANG Cong
> ; David S. Miller ;
> Sabrina Dubroca 
> Subject: [PATCH] e1000e: use disable_hardirq() also for MSIX vectors in
> e1000_netpoll()
> 
> Replace disable_irq() which waits for threaded irq handlers with
> disable_hardirq() which waits only for hardirq part.
> 
> Signed-off-by: Konstantin Khlebnikov 
> Fixes: 311191297125 ("e1000: use disable_hardirq() for e1000_netpoll()")
> ---
>  drivers/net/ethernet/intel/e1000e/netdev.c |   12 ++--
>  1 file changed, 6 insertions(+), 6 deletions(-)

Tested-by: Aaron Brown 



RE: [PATCH v2] e1000e: Don't return uninitialized stats

2017-05-19 Thread Brown, Aaron F
> From: Kirsher, Jeffrey T
> Sent: Friday, May 19, 2017 1:17 AM
> To: David Miller <da...@davemloft.net>; bpoir...@suse.com
> Cc: s.pri...@profihost.ag; intel-wired-...@lists.osuosl.org;
> netdev@vger.kernel.org; pmen...@molgen.mpg.de; Neftin, Sasha
> <sasha.nef...@intel.com>; Brown, Aaron F <aaron.f.br...@intel.com>;
> step...@networkplumber.org
> Subject: Re: [PATCH v2] e1000e: Don't return uninitialized stats
> 
> On Thu, 2017-05-18 at 10:46 -0400, David Miller wrote:
> > From: Benjamin Poirier <bpoir...@suse.com>
> > Date: Wed, 17 May 2017 16:24:13 -0400
> >
> > > Some statistics passed to ethtool are garbage because
> > > e1000e_get_stats64()
> > > doesn't write them, for example: tx_heartbeat_errors. This leaks kernel
> > > memory to userspace and confuses users.
> > >
> > > Do like ixgbe and use dev_get_stats() which first zeroes out
> > > rtnl_link_stats64.
> > >
> > > Fixes: 5944701df90d ("net: remove useless memset's in drivers
> > > get_stats64")
> > > Reported-by: Stefan Priebe <s.pri...@profihost.ag>
> > > Signed-off-by: Benjamin Poirier <bpoir...@suse.com>

Tested-by: Aaron Brown <aaron.f.br...@intel.com>


RE: [Intel-wired-lan] [PATCH v2 1/1] e1000e: Undo e1000e_pm_freeze if __e1000_shutdown fails

2017-06-06 Thread Brown, Aaron F
> From: Intel-wired-lan [mailto:intel-wired-lan-boun...@osuosl.org] On Behalf
> Of Jeff Kirsher
> Sent: Tuesday, June 6, 2017 1:46 PM
> To: David Miller ; Nikula, Jani
> 
> Cc: Ursulin, Tvrtko ; daniel.vet...@ffwll.ch; intel-
> g...@lists.freedesktop.org; linux-ker...@vger.kernel.org;
> jani.nik...@linux.intel.com; ch...@chris-wilson.co.uk; Ertman, David M
> ; intel-wired-...@lists.osuosl.org; dri-
> de...@lists.freedesktop.org; netdev@vger.kernel.org; airl...@gmail.com
> Subject: Re: [Intel-wired-lan] [PATCH v2 1/1] e1000e: Undo
> e1000e_pm_freeze if __e1000_shutdown fails
> 
> On Fri, 2017-06-02 at 14:14 -0400, David Miller wrote:
> > From: Jani Nikula 
> > Date: Wed, 31 May 2017 18:50:43 +0300
> >
> > > From: Chris Wilson 
> > >
> > > An error during suspend (e100e_pm_suspend),
> >
> >  ...
> > > lead to complete failure:
> >
> >  ...
> > > The unwind failures stems from commit 2800209994f8 ("e1000e:
> > > Refactor PM
> > > flows"), but it may be a later patch that introduced the non-
> > > recoverable
> > > behaviour.
> > >
> > > Fixes: 2800209994f8 ("e1000e: Refactor PM flows")
> > > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=99847
> > > Cc: Tvrtko Ursulin 
> > > Cc: Jeff Kirsher 
> > > Cc: Dave Ertman 
> > > Cc: Bruce Allan 
> > > Cc: intel-wired-...@lists.osuosl.org
> > > Cc: netdev@vger.kernel.org
> > > Signed-off-by: Chris Wilson 
> > > [Jani: bikeshed repainted]
> > > Signed-off-by: Jani Nikula 
> >
> > Jeff, please make sure this gets submitted to me soon.
> 
> Expect it later tonight, just finishing up testing.

Tested-by: Aaron Brown 


RE: [Intel-wired-lan] [PATCH net-next] igb: mark PM functions as __maybe_unused

2017-05-02 Thread Brown, Aaron F
> From: Intel-wired-lan [mailto:intel-wired-lan-boun...@lists.osuosl.org] On
> Behalf Of Arnd Bergmann
> Sent: Thursday, April 27, 2017 12:10 PM
> To: Kirsher, Jeffrey T 
> Cc: Arnd Bergmann ; netdev@vger.kernel.org; linux-
> ker...@vger.kernel.org; intel-wired-...@lists.osuosl.org; David S. Miller
> 
> Subject: [Intel-wired-lan] [PATCH net-next] igb: mark PM functions as
> __maybe_unused
> 
> The new wake function is only used by the suspend/resume handlers that
> are defined in inside of an #ifdef, which can cause this harmless
> warning:
> 
> drivers/net/ethernet/intel/igb/igb_main.c:7988:13: warning:
> 'igb_deliver_wake_packet' defined but not used [-Wunused-function]
> 
> Removing the #ifdef, instead using a __maybe_unused annotation
> simplifies the code and avoids the warning.
> 
> Fixes: b90fa8763560 ("igb: Enable reading of wake up packet")
> Signed-off-by: Arnd Bergmann 
> ---
>  drivers/net/ethernet/intel/igb/igb_main.c | 18 +-
>  1 file changed, 5 insertions(+), 13 deletions(-)

Tested-by: Aaron Brown 


RE: [Intel-wired-lan] [PATCH] igb: make a few local functions static

2017-05-02 Thread Brown, Aaron F
> From: Intel-wired-lan [mailto:intel-wired-lan-boun...@lists.osuosl.org] On
> Behalf Of Colin King
> Sent: Thursday, April 27, 2017 10:59 AM
> To: Kirsher, Jeffrey T ; intel-wired-
> l...@lists.osuosl.org; netdev@vger.kernel.org
> Cc: kernel-janit...@vger.kernel.org; linux-ker...@vger.kernel.org
> Subject: [Intel-wired-lan] [PATCH] igb: make a few local functions static
> 
> From: Colin Ian King 
> 
> clean up a few sparse warnings, these following
> functions can be made static:
> 
> drivers/net/ethernet/intel/igb/igb_main.c: warning: symbol
>   'igb_add_mac_filter' was not declared. Should it be static?
> drivers/net/ethernet/intel/igb/igb_main.c: warning: symbol
>   'igb_del_mac_filter' was not declared. Should it be static?
> drivers/net/ethernet/intel/igb/igb_main.c: warning: symbol
>   'igb_set_vf_mac_filter' was not declared. Should it be static?
> 
> Signed-off-by: Colin Ian King 
> ---
>  drivers/net/ethernet/intel/igb/igb_main.c | 12 ++--
>  1 file changed, 6 insertions(+), 6 deletions(-)

Tested-by: Aaron Brown 


RE: [PATCH] e1000e: changed some expensive calls of udelay to usleep_range

2017-09-11 Thread Brown, Aaron F
> From: netdev-ow...@vger.kernel.org [mailto:netdev-
> ow...@vger.kernel.org] On Behalf Of Kardonik Michael
> Sent: Tuesday, September 5, 2017 1:27 PM
> To: leoyang...@nxp.com; michael.kardo...@nxp.com; Kirsher, Jeffrey T
> ; Brandeburg, Jesse
> ; Shannon Nelson
> ; Wyborny, Carolyn
> ; Skidmore, Donald C
> ; Vick, Matthew ;
> John Ronciak ; Williams, Mitch A
> ; intel-wired-...@lists.osuosl.org;
> netdev@vger.kernel.org; linux-ker...@vger.kernel.org
> Cc: Matthew Tan 
> Subject: [PATCH] e1000e: changed some expensive calls of udelay to
> usleep_range
> 
> Calls to udelay are not preemtable by userspace so userspace
> applications experience a large (~200us) latency when running on core0.
> Instead usleep_range can be used to be more friendly to userspace
> since it is preemtable. This is due to udelay using busy-wait loops
> while usleep_rang uses hrtimers instead. It is recommended to use
> udelay when the delay is <10us since at that precision overhead of
> usleep_range hrtimer setup causes issues. However, the replaced calls
> are for 50us and 100us so this should not be not an issue.
> There is no open bug that this patch is fixing, but we see a good
> boost in zero loss performance of specific user space application
> (dpdk l3fwd) when this patch is applied: we get from 32% of 10Gb line
> to 49%.
> 
> Signed-off-by: Matthew Tan 
> Signed-off-by: Michael Kardonik 
> 
> ---
>  drivers/net/ethernet/intel/e1000e/phy.c | 8 
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 

This version continues to crash the same systems as the last (an 82577 and an 
i218.)  As before, when link partner is a lower speed (autoneg set for 100 half 
for this instance) and I simply bring up the interface I start getting stuck 
CPU messages:

Message from syslogd@u1459 at Sep 11 17:16:25 ...
 kernel:watchdog: BUG: soft lockup - CPU#2 stuck for 22s! [kworker/2:0:4204]

Message from syslogd@u1459 at Sep 11 17:16:53 ...
 kernel:watchdog: BUG: soft lockup - CPU#2 stuck for 22s! [kworker/2:0:4204]


System becomes unresponsive and on a reboot I am able to pull out  a trace file:

Sep 11 17:16:00 u1459 kernel: INFO: rcu_sched self-detected stall on CPU
Sep 11 17:16:00 u1459 kernel:   2-...: (20999 ticks this GP) 
idle=a5a/141/0 softirq=8232/8232 fqs=5250
Sep 11 17:16:00 u1459 kernel:(t=21000 jiffies g=4013 c=4012 q=616)
Sep 11 17:16:00 u1459 kernel: NMI backtrace for cpu 2
Sep 11 17:16:00 u1459 kernel: CPU: 2 PID: 4204 Comm: kworker/2:0 Tainted: G 
   E   4.13.0_next-queue_dev-queue_41b1c97 #39
Sep 11 17:16:00 u1459 kernel: Hardware name:  /NUC5i5MYBE, BIOS 
MYBDWi5v.86A.0030.2016.0527.1657 05/27/2016
Sep 11 17:16:00 u1459 kernel: Workqueue: events linkwatch_event
Sep 11 17:16:00 u1459 kernel: Call Trace:
Sep 11 17:16:00 u1459 kernel: 
Sep 11 17:16:00 u1459 kernel: dump_stack+0x51/0x78
Sep 11 17:16:00 u1459 kernel: nmi_cpu_backtrace+0xd3/0xe0
Sep 11 17:16:00 u1459 kernel: ? hw_nmi_get_sample_period+0x20/0x20
Sep 11 17:16:00 u1459 kernel: nmi_trigger_cpumask_backtrace+0xf5/0x130
Sep 11 17:16:00 u1459 kernel: arch_trigger_cpumask_backtrace+0x19/0x20
Sep 11 17:16:00 u1459 kernel: rcu_dump_cpu_stacks+0xb7/0xe0
Sep 11 17:16:00 u1459 kernel: print_cpu_stall+0xfa/0x170
Sep 11 17:16:00 u1459 kernel: ? __update_load_avg_cfs_rq+0x183/0x190
Sep 11 17:16:00 u1459 kernel: ? sched_slice+0x55/0xa0
Sep 11 17:16:00 u1459 kernel: check_cpu_stall+0x110/0x120
Sep 11 17:16:00 u1459 kernel: ? check_preempt_curr+0x73/0x90
Sep 11 17:16:00 u1459 kernel: ? ttwu_do_wakeup+0x2b/0x170
Sep 11 17:16:00 u1459 kernel: ? cpu_needs_another_gp+0x7b/0x80
Sep 11 17:16:00 u1459 kernel: ? notifier_call_chain+0x56/0x80
Sep 11 17:16:00 u1459 kernel: rcu_pending+0x5f/0x180
Sep 11 17:16:00 u1459 kernel: ? raw_notifier_call_chain+0x16/0x20
Sep 11 17:16:00 u1459 kernel: ? timekeeping_update+0xd9/0x130
Sep 11 17:16:00 u1459 kernel: rcu_check_callbacks+0x8a/0x1b0
Sep 11 17:16:00 u1459 kernel: ? account_system_index_time+0x6b/0x80
Sep 11 17:16:00 u1459 kernel: update_process_times+0x39/0x70
Sep 11 17:16:00 u1459 kernel: tick_sched_handle+0x37/0x70
Sep 11 17:16:00 u1459 kernel: tick_sched_timer+0x52/0xa0
Sep 11 17:16:00 u1459 kernel: __run_hrtimer+0x77/0x1b0
Sep 11 17:16:00 u1459 kernel: ? tick_nohz_handler+0xc0/0xc0
Sep 11 17:16:00 u1459 kernel: ? ktime_get+0x5a/0xd0
Sep 11 17:16:00 u1459 kernel: __hrtimer_run_queues+0x67/0x90
Sep 11 17:16:00 u1459 kernel: hrtimer_interrupt+0xa4/0x1d0
Sep 11 17:16:00 u1459 kernel: 

RE: [Intel-wired-lan] [PATCH 5/5] e1000e: Avoid receiver overrun interrupt bursts

2017-09-14 Thread Brown, Aaron F
> From: Intel-wired-lan [mailto:intel-wired-lan-boun...@osuosl.org] On Behalf
> Of Benjamin Poirier
> Sent: Friday, July 21, 2017 11:36 AM
> To: Kirsher, Jeffrey T 
> Cc: netdev@vger.kernel.org; intel-wired-...@lists.osuosl.org; linux-
> ker...@vger.kernel.org; Lennart Sorensen 
> Subject: [Intel-wired-lan] [PATCH 5/5] e1000e: Avoid receiver overrun
> interrupt bursts
> 
> When e1000e_poll() is not fast enough to keep up with incoming traffic, the
> adapter (when operating in msix mode) raises the Other interrupt to signal
> Receiver Overrun.
> 
> This is a double problem because 1) at the moment e1000_msix_other()
> assumes that it is only called in case of Link Status Change and 2) if the
> condition persists, the interrupt is repeatedly raised again in quick
> succession.
> 
> Ideally we would configure the Other interrupt to not be raised in case of
> receiver overrun but this doesn't seem possible on this adapter. Instead,
> we handle the first part of the problem by reverting to the practice of
> reading ICR in the other interrupt handler, like before commit 16ecba59bc33
> ("e1000e: Do not read ICR in Other interrupt"). Thanks to commit
> 0a8047ac68e5 ("e1000e: Fix msi-x interrupt automask") which cleared IAME
> from CTRL_EXT, reading ICR doesn't interfere with RxQ0, TxQ0 interrupts
> anymore. We handle the second part of the problem by not re-enabling the
> Other interrupt right away when there is overrun. Instead, we wait until
> traffic subsides, napi polling mode is exited and interrupts are
> re-enabled.
> 
> Reported-by: Lennart Sorensen 
> Fixes: 16ecba59bc33 ("e1000e: Do not read ICR in Other interrupt")
> Signed-off-by: Benjamin Poirier 
> ---
>  drivers/net/ethernet/intel/e1000e/defines.h |  1 +
>  drivers/net/ethernet/intel/e1000e/netdev.c  | 33
> +++--
>  2 files changed, 27 insertions(+), 7 deletions(-)
> 

I get an error and a few warnings out of checkpatch from this, but I think the 
error is false (thinking the reference to a commit in the description is this 
commit, a fixes commit or something like that) and I'm more concerned with the 
fix than the warnings, so...

Tested-by: Aaron Brown 

Here is the checkpatch output in case anyone has a different opinion on the 
severity:
-
u1484:[0]/usr/src/kernels/next-queue> git format-patch d81d1e6 -1 
--stdout|./scripts/checkpatch.pl -
ERROR: Please use git commit description style 'commit <12+ chars of sha1> 
("")' - ie: 'commit 0a8047ac68e5 ("e1000e: Fix msi-x interrupt 
automask")'
#20:
0a8047ac68e5 ("e1000e: Fix msi-x interrupt automask") which cleared IAME

WARNING: braces {} are not necessary for single statement blocks
#73: FILE: drivers/net/ethernet/intel/e1000e/netdev.c:1931:
+   if (!test_bit(__E1000_DOWN, >state)) {
+   mod_timer(>watchdog_timer, jiffies + 1);
+   }

WARNING: braces {} are not necessary for single statement blocks
#83: FILE: drivers/net/ethernet/intel/e1000e/netdev.c:1936:
+   if (enable && !test_bit(__E1000_DOWN, >state)) {
ew32(IMS, E1000_IMS_OTHER);
}

total: 1 errors, 2 warnings, 0 checks, 59 lines checked

NOTE: For some of the reported defects, checkpatch may be able to
  mechanically convert to the typical style using --fix or --fix-inplace.

Your patch has style problems, please review.

NOTE: If any of the errors are false positives, please report
  them to the maintainer, see CHECKPATCH in MAINTAINERS.
u1484:[0]/usr/src/kernels/next-queue>


RE: [Intel-wired-lan] [PATCH] igb: check memory allocation failure

2017-09-13 Thread Brown, Aaron F
> From: Intel-wired-lan [mailto:intel-wired-lan-boun...@osuosl.org] On Behalf
> Of Christophe JAILLET
> Sent: Monday, August 28, 2017 10:13 AM
> To: Waskiewicz Jr, Peter ; Kirsher, Jeffrey T
> 
> Cc: netdev@vger.kernel.org; kernel-janit...@vger.kernel.org; intel-wired-
> l...@lists.osuosl.org; linux-ker...@vger.kernel.org
> Subject: Re: [Intel-wired-lan] [PATCH] igb: check memory allocation failure
> 
> Le 28/08/2017 à 01:09, Waskiewicz Jr, Peter a écrit :
> > On 8/27/17 2:42 AM, Christophe JAILLET wrote:
> >> Check memory allocation failures and return -ENOMEM in such cases, as
> >> already done for other memory allocations in this function.
> >>
> >> This avoids NULL pointers dereference.
> >>
> >> Signed-off-by: Christophe JAILLET 
> >> ---
> >>drivers/net/ethernet/intel/igb/igb_main.c | 2 ++
> >>1 file changed, 2 insertions(+)
> >>

This seems to be fine from a "it does not break in testing" perspective, so...

Tested-by: Aaron Brown  > -PJ
> >
> Hi,
> 
> in fact, there is no leak because the only caller of 'igb_sw_init()'
> (i.e. 'igb_probe()'), already frees these resources in case of error,
> see [1]
> 
> These resources are also freed  in 'igb_remove()'.
> 
> Best reagrds,
> CJ
> 
> [1]:
> https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-
> next.git/tree/drivers/net/ethernet/intel/igb/igb_main.c#n2775

But is PJ's comment saying that it is not really necessary?  If so I tend to 
lean towards the don't touch it if it's not broken perspective.
 



RE: [PATCH] e1000e: apply burst mode settings only on default

2017-09-14 Thread Brown, Aaron F
> From: netdev-ow...@vger.kernel.org [mailto:netdev-
> ow...@vger.kernel.org] On Behalf Of Willem de Bruijn
> Sent: Friday, August 25, 2017 8:06 AM
> To: Kirsher, Jeffrey T 
> Cc: intel-wired-...@lists.osuosl.org; netdev@vger.kernel.org; Brandeburg,
> Jesse ; Willem de Bruijn
> 
> Subject: [PATCH] e1000e: apply burst mode settings only on default
> 
> From: Willem de Bruijn 
> 
> Devices that support FLAG2_DMA_BURST have different default values
> for RDTR and RADV. Apply burst mode default settings only when no
> explicit value was passed at module load.
> 
> The RDTR default is zero. If the module is loaded for low latency
> operation with RxIntDelay=0, do not override this value with a burst
> default of 32.
> 
> Move the decision to apply burst values earlier, where explicitly
> initialized module variables can be distinguished from defaults.
> 
> Signed-off-by: Willem de Bruijn 
> ---
>  drivers/net/ethernet/intel/e1000e/e1000.h  |  4 
>  drivers/net/ethernet/intel/e1000e/netdev.c |  8 
>  drivers/net/ethernet/intel/e1000e/param.c  | 16 +++-
>  3 files changed, 15 insertions(+), 13 deletions(-)

Tested-by: Aaron Brown 


RE: [Intel-wired-lan] [PATCH net-next v3] e1000e: Be drop monitor friendly

2017-09-14 Thread Brown, Aaron F
> From: Intel-wired-lan [mailto:intel-wired-lan-boun...@osuosl.org] On Behalf
> Of Florian Fainelli
> Sent: Friday, August 25, 2017 6:14 PM
> To: netdev@vger.kernel.org
> Cc: eduma...@gmail.com; Florian Fainelli ; open list
> ; moderated list:INTEL ETHERNET DRIVERS
> ; da...@davemloft.net
> Subject: [Intel-wired-lan] [PATCH net-next v3] e1000e: Be drop monitor
> friendly
> 
> e1000e_put_txbuf() can be called from normal reclamation path as well as
> when a DMA mapping failure, so we need to differentiate these two cases
> when freeing SKBs to be drop monitor friendly. e1000e_tx_hwtstamp_work()
> and e1000_remove() are processing TX timestamped SKBs and those should
> not be accounted as drops either.
> 
> Signed-off-by: Florian Fainelli 
> ---
> Changes in v3:
> 
> - differentiate normal reclamation from TX DMA fragment mapping errors
> - removed a few invalid dev_kfree_skb() replacements (those are already
>   drop monitor friendly)
> 
> Changes in v2:
> 
> - make it compile
> 
>  drivers/net/ethernet/intel/e1000e/netdev.c | 18 +++---
>  1 file changed, 11 insertions(+), 7 deletions(-)

Tested-by: Aaron Brown 



RE: [Intel-wired-lan] [PATCH 3/5] e1000e: Fix return value test

2017-09-14 Thread Brown, Aaron F
> From: Intel-wired-lan [mailto:intel-wired-lan-boun...@osuosl.org] On Behalf
> Of Benjamin Poirier
> Sent: Friday, July 21, 2017 11:36 AM
> To: Kirsher, Jeffrey T 
> Cc: netdev@vger.kernel.org; intel-wired-...@lists.osuosl.org; linux-
> ker...@vger.kernel.org; Lennart Sorensen 
> Subject: [Intel-wired-lan] [PATCH 3/5] e1000e: Fix return value test
> 
> All the helpers return -E1000_ERR_PHY.
> 
> Signed-off-by: Benjamin Poirier 
> ---
>  drivers/net/ethernet/intel/e1000e/netdev.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

Tested-by: Aaron Brown 



RE: [Intel-wired-lan] [PATCH 4/5] e1000e: Separate signaling for link check/link up

2017-09-14 Thread Brown, Aaron F

On 7/21/2017 21:36, Benjamin Poirier wrote:
> Lennart reported the following race condition:
>
> \ e1000_watchdog_task
>  \ e1000e_has_link
>  \ hw->mac.ops.check_for_link() === e1000e_check_for_copper_link
>  /* link is up */
>  mac->get_link_status = false;
>
>  /* interrupt */
>  \ e1000_msix_other
>  hw->mac.get_link_status = true;
>
>  link_active = !hw->mac.get_link_status
>  /* link_active is false, wrongly */
>
> This problem arises because the single flag get_link_status is used to
> signal two different states: link status needs checking and link status is
> down.
>
> Avoid the problem by using the return value of .check_for_link to signal
> the link status to e1000e_has_link().
>
> Reported-by: Lennart Sorensen 
> Signed-off-by: Benjamin Poirier 
> ---
>   drivers/net/ethernet/intel/e1000e/mac.c| 11 ---
>   drivers/net/ethernet/intel/e1000e/netdev.c |  2 +-
>   2 files changed, 9 insertions(+), 4 deletions(-)

Tested-by: Aaron Brown 


RE: [Intel-wired-lan] [PATCH 1/5] e1000e: Fix error path in link detection

2017-09-14 Thread Brown, Aaron F
> From: Intel-wired-lan [mailto:intel-wired-lan-boun...@osuosl.org] On Behalf
> Of Benjamin Poirier
> Sent: Friday, July 21, 2017 11:36 AM
> To: Kirsher, Jeffrey T 
> Cc: netdev@vger.kernel.org; intel-wired-...@lists.osuosl.org; linux-
> ker...@vger.kernel.org; Lennart Sorensen 
> Subject: [Intel-wired-lan] [PATCH 1/5] e1000e: Fix error path in link 
> detection
> 
> In case of error from e1e_rphy(), the loop will exit early and "success"
> will be set to true erroneously.
> 
> Signed-off-by: Benjamin Poirier 
> ---
>  drivers/net/ethernet/intel/e1000e/phy.c | 7 ---
>  1 file changed, 4 insertions(+), 3 deletions(-)
> 

Tested-by: Aaron Brown 


RE: [Intel-wired-lan] [PATCH 2/5] e1000e: Fix wrong comment related to link detection

2017-09-18 Thread Brown, Aaron F
> From: Intel-wired-lan [mailto:intel-wired-lan-boun...@osuosl.org] On Behalf
> Of Benjamin Poirier
> Sent: Friday, July 21, 2017 11:36 AM
> To: Kirsher, Jeffrey T 
> Cc: netdev@vger.kernel.org; intel-wired-...@lists.osuosl.org; linux-
> ker...@vger.kernel.org; Lennart Sorensen 
> Subject: [Intel-wired-lan] [PATCH 2/5] e1000e: Fix wrong comment related to
> link detection
> 
> Reading e1000e_check_for_copper_link() shows that get_link_status is set to
> false after link has been detected. Therefore, it stays TRUE until then.
> 
> Signed-off-by: Benjamin Poirier 
> ---
>  drivers/net/ethernet/intel/e1000e/netdev.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 

Tested-by: Aaron Brown 


RE: [Intel-wired-lan] [PATCH][V3] e1000: avoid null pointer dereference on invalid stat type

2017-10-13 Thread Brown, Aaron F
> From: Intel-wired-lan [mailto:intel-wired-lan-boun...@osuosl.org] On Behalf
> Of Colin King
> Sent: Friday, September 22, 2017 10:14 AM
> To: Kirsher, Jeffrey T ; intel-wired-
> l...@lists.osuosl.org; netdev@vger.kernel.org
> Cc: kernel-janit...@vger.kernel.org; linux-ker...@vger.kernel.org
> Subject: [Intel-wired-lan] [PATCH][V3] e1000: avoid null pointer dereference
> on invalid stat type
> 
> From: Colin Ian King 
> 
> Currently if the stat type is invalid then data[i] is being set
> either by dereferencing a null pointer p, or it is reading from
> an incorrect previous location if we had a valid stat type
> previously.  Fix this by skipping over the read of p on an invalid
> stat type.
> 
> Detected by CoverityScan, CID#113385 ("Explicit null dereferenced")
> 
> Signed-off-by: Colin Ian King 
> ---
>  drivers/net/ethernet/intel/e1000/e1000_ethtool.c | 9 -
>  1 file changed, 4 insertions(+), 5 deletions(-)

Tested-by: Aaron Brown 


RE: [Intel-wired-lan] [PATCH] e1000e: changed some expensive calls of udelay to usleep_range

2017-09-07 Thread Brown, Aaron F
> From: Intel-wired-lan [mailto:intel-wired-lan-boun...@osuosl.org] On
> Behalf Of Pavel Machek
> Sent: Monday, September 4, 2017 9:26 AM
> To: Matthew Tan 
> Cc: michael.kardo...@nxp.com; Williams, Mitch A
> ; linux-ker...@vger.kernel.org;
> john.ronc...@intel.com; intel-wired-...@lists.osuosl.org;
> netdev@vger.kernel.org
> Subject: Re: [Intel-wired-lan] [PATCH] e1000e: changed some expensive calls
> of udelay to usleep_range
> 
> Hi!
> 
> > @@ -183,7 +183,7 @@ s32 e1000e_read_phy_reg_mdic(struct e1000_hw
> *hw, u32 offset, u16 *data)
> >  * reading duplicate data in the next MDIC transaction.
> >  */
> > if (hw->mac.type == e1000_pch2lan)
> > -   udelay(100);
> > +   usleep_range(90, 100);
> >
> > return 0;
> >  }
> 
> Can you explain why shortening the delay is acceptable here?

Maybe it's not.

This patch is causing speed / duplex tests to fail on several of my test 
systems.  Specifically a Lenova laptop with an 82577 and a NUC with an i218 
(though that does not mean it is limited to those or that it's not related to 
the individual link partner.)

In both cases they "blow up" when attempting to link at 10 Mb with a Call Trace 
on the console / log and a watchdog: BUG: soft lockup - CPU#X stuck appearing 
in the current login session.  The simplest way to produce the crash is simply 
load the interface and attempt to bring it up with the link partner configured 
to 10 Mb half (force or autoneg) or forced to 10 Mb full (autoneg to 10 Mb full 
did not blow up.)  The problem is also appearing at some, but not all 100 Mb 
speeds and duplexes.  100 Mb also crashes, apparently with the same pattern.  
With the link partner set to 100 Mb half Autoneg on, 100 Mb half forced and 100 
Mb full forced the system crashes, 100 Full autoneg seems to work fine.

The call trace repeats every 30 seconds, is captured in the log and looks a lot 
like this:

Sep  7 14:17:10 u1459 kernel: watchdog: BUG: soft lockup - CPU#2 stuck for 22s! 
[kworker/2:0:23]
Sep  7 14:17:10 u1459 kernel: Modules linked in: e1000e ptp pps_core 
ip6table_filter ip6_tables ebtable_nat ebtables nf_conntrack_ipv4 
nf_defrag_ipv4 xt_state nf_conntrack libcrc32c ipt_REJECT nf_reject_ipv4 
xt_CHECKSUM iptable_mangle iptable_filter ip_tables bridge stp llc nfsd lockd 
grace nfs_acl auth_rpcgss sunrpc autofs4 ipv6 crc_ccitt dm_mirror 
dm_region_hash dm_log vhost_net vhost tap tun kvm_intel kvm irqbypass uinput 
joydev ax88179_178a usbnet mii iTCO_wdt iTCO_vendor_support sg i2c_i801 lpc_ich 
mfd_core xhci_pci snd_hda_codec_realtek xhci_hcd snd_hda_codec_hdmi 
snd_hda_codec_generic snd_hda_intel snd_hda_codec snd_hda_core snd_hwdep 
snd_seq snd_seq_device snd_pcm snd_timer snd soundcore dm_mod(E) dax(E) ext4(E) 
jbd2(E) mbcache(E) sd_mod(E) ahci(E) libahci(E) i915(E) drm_kms_helper(E) 
drm(E) fb_sys_fops(E)
Sep  7 14:17:10 u1459 kernel: sysimgblt(E) sysfillrect(E) syscopyarea(E) 
i2c_algo_bit(E) i2c_core(E) iosf_mbi(E) video(E)
Sep  7 14:17:10 u1459 kernel: CPU: 2 PID: 23 Comm: kworker/2:0 Tainted: G   
 EL  4.13.0-rc6_next-queue_dev-queue_regress #38
Sep  7 14:17:10 u1459 kernel: Hardware name:  /NUC5i5MYBE, BIOS 
MYBDWi5v.86A.0030.2016.0527.1657 05/27/2016
Sep  7 14:17:10 u1459 kernel: Workqueue: events linkwatch_event
Sep  7 14:17:10 u1459 kernel: task: 88024cf84680 task.stack: 
c9d44000
Sep  7 14:17:10 u1459 kernel: RIP: 0010:queued_spin_lock_slowpath+0x56/0x1d0
Sep  7 14:17:10 u1459 kernel: RSP: 0018:c9d478c8 EFLAGS: 0202 
ORIG_RAX: ff10
Sep  7 14:17:10 u1459 kernel: RAX: 0101 RBX: 880239fe8000 RCX: 
0101
Sep  7 14:17:10 u1459 kernel: RDX: c9d47948 RSI: 0001 RDI: 
880239feb1a0
Sep  7 14:17:10 u1459 kernel: RBP: c9d47968 R08: 0001 R09: 
88024ce181a4
Sep  7 14:17:10 u1459 kernel: R10:  R11:  R12: 
880239fe8840
Sep  7 14:17:10 u1459 kernel: R13: 88024ce180e4 R14: 88024ce180e4 R15: 
c9d47a48
Sep  7 14:17:10 u1459 kernel: FS:  () 
GS:880255d0() knlGS:
Sep  7 14:17:10 u1459 kernel: CS:  0010 DS:  ES:  CR0: 80050033
Sep  7 14:17:10 u1459 kernel: CR2: 7ffd4c3579b8 CR3: 01c09000 CR4: 
003406e0
Sep  7 14:17:10 u1459 kernel: DR0:  DR1:  DR2: 

Sep  7 14:17:10 u1459 kernel: DR3:  DR6: fffe0ff0 DR7: 
0400
Sep  7 14:17:10 u1459 kernel: Call Trace:
Sep  7 14:17:10 u1459 kernel: ? netlink_broadcast_filtered+0x15f/0x1a0
Sep  7 14:17:10 u1459 kernel: _raw_spin_lock+0x21/0x30
Sep  7 14:17:10 u1459 kernel: e1000e_get_stats64+0x2b/0x140 [e1000e]
Sep  7 14:17:10 u1459 kernel: dev_get_stats+0x3d/0xc0
Sep  7 14:17:10 u1459 kernel: ? __nla_reserve+0x53/0x70
Sep  7 14:17:10 u1459 kernel: 

RE: [PATCH net] driver: e1000: fix race condition between e1000_down() and e1000_watchdog

2017-10-10 Thread Brown, Aaron F
> From: netdev-ow...@vger.kernel.org [mailto:netdev-
> ow...@vger.kernel.org] On Behalf Of Vincenzo Maffione
> Sent: Saturday, September 16, 2017 9:00 AM
> To: Kirsher, Jeffrey T 
> Cc: intel-wired-...@lists.osuosl.org; netdev@vger.kernel.org; linux-
> ker...@vger.kernel.org; Vincenzo Maffione 
> Subject: [PATCH net] driver: e1000: fix race condition between e1000_down()
> and e1000_watchdog
> 
> This patch fixes a race condition that can result into the interface being
> up and carrier on, but with transmits disabled in the hardware.
> The bug may show up by repeatedly IFF_DOWN+IFF_UP the interface, which
> allows e1000_watchdog() interleave with e1000_down().
> 
> CPU x   CPU y
> 
> e1000_down():
> netif_carrier_off()
> e1000_watchdog():
> if (carrier == off) {
> netif_carrier_on();
> enable_hw_transmit();
> }
> disable_hw_transmit();
> e1000_watchdog():
> /* carrier on, do nothing */
> 
> Signed-off-by: Vincenzo Maffione 
> ---
>  drivers/net/ethernet/intel/e1000/e1000_main.c | 11 +--
>  1 file changed, 9 insertions(+), 2 deletions(-)

Tested-by: Aaron Brown 


RE: [net-next,RESEND] igb: add function to get maximum RSS queues

2017-10-10 Thread Brown, Aaron F
> From: netdev-ow...@vger.kernel.org [mailto:netdev-
> ow...@vger.kernel.org] On Behalf Of Zhang Shengju
> Sent: Tuesday, September 19, 2017 6:41 AM
> To: Kirsher, Jeffrey T ; intel-wired-
> l...@lists.osuosl.org; netdev@vger.kernel.org
> Subject: [net-next,RESEND] igb: add function to get maximum RSS queues
> 
> This patch adds a new function igb_get_max_rss_queues() to get maximum
> RSS queues, this will reduce duplicate code and facilitate future
> maintenance.
> 
> Signed-off-by: Zhang Shengju 
> ---
>  drivers/net/ethernet/intel/igb/igb.h |  1 +
>  drivers/net/ethernet/intel/igb/igb_ethtool.c | 32 
> +---
>  drivers/net/ethernet/intel/igb/igb_main.c| 12 +--
>  3 files changed, 12 insertions(+), 33 deletions(-)
> 

Tested-by: Aaron Brown 


RE: [PATCH] e1000: Fix off-by-one in debug message

2017-11-27 Thread Brown, Aaron F
> From: netdev-ow...@vger.kernel.org [mailto:netdev-
> ow...@vger.kernel.org] On Behalf Of Ahmad Fatoum
> Sent: Saturday, November 18, 2017 12:54 PM
> To: Kirsher, Jeffrey T 
> Cc: intel-wired-...@lists.osuosl.org; netdev@vger.kernel.org; Ahmad
> Fatoum 
> Subject: [PATCH] e1000: Fix off-by-one in debug message
> 
> Signed-off-by: Ahmad Fatoum 
> ---
>  drivers/net/ethernet/intel/e1000/e1000_hw.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)

Tested-by: Aaron Brown 


RE: [Intel-wired-lan] [PATCH] i40e: remove redundant initialization of read_size

2017-11-09 Thread Brown, Aaron F
> From: Intel-wired-lan [mailto:intel-wired-lan-boun...@osuosl.org] On Behalf
> Of Colin King
> Sent: Sunday, November 5, 2017 5:04 AM
> To: Kirsher, Jeffrey T ; intel-wired-
> l...@lists.osuosl.org; netdev@vger.kernel.org
> Cc: kernel-janit...@vger.kernel.org; linux-ker...@vger.kernel.org
> Subject: [Intel-wired-lan] [PATCH] i40e: remove redundant initialization of
> read_size
> 
> From: Colin Ian King 
> 
> Variable read_size is initialized and this value is never read, it is
> instead set inside the do-loop, hence the intialization is redundant
> and can be removed. Cleans up clang warning:
> 
> drivers/net/ethernet/intel/i40e/i40e_nvm.c:390:6: warning: Value stored
> to 'read_size' during its initialization is never read
> 
> Signed-off-by: Colin Ian King 

s/intialization/initialization/g


RE: [Intel-wired-lan] [PATCH] i40e: remove redundant initialization of read_size

2017-11-09 Thread Brown, Aaron F
> From: Brown, Aaron F
> Sent: Thursday, November 9, 2017 7:16 PM
> To: 'Colin King' <colin.k...@canonical.com>; Kirsher, Jeffrey T
> <jeffrey.t.kirs...@intel.com>; intel-wired-...@lists.osuosl.org;
> netdev@vger.kernel.org
> Cc: kernel-janit...@vger.kernel.org; linux-ker...@vger.kernel.org
> Subject: RE: [Intel-wired-lan] [PATCH] i40e: remove redundant initialization
> of read_size
> 
> > From: Intel-wired-lan [mailto:intel-wired-lan-boun...@osuosl.org] On
> Behalf
> > Of Colin King
> > Sent: Sunday, November 5, 2017 5:04 AM
> > To: Kirsher, Jeffrey T <jeffrey.t.kirs...@intel.com>; intel-wired-
> > l...@lists.osuosl.org; netdev@vger.kernel.org
> > Cc: kernel-janit...@vger.kernel.org; linux-ker...@vger.kernel.org
> > Subject: [Intel-wired-lan] [PATCH] i40e: remove redundant initialization of
> > read_size
> >
> > From: Colin Ian King <colin.k...@canonical.com>
> >
> > Variable read_size is initialized and this value is never read, it is
> > instead set inside the do-loop, hence the intialization is redundant
> > and can be removed. Cleans up clang warning:
> >
> > drivers/net/ethernet/intel/i40e/i40e_nvm.c:390:6: warning: Value stored
> > to 'read_size' during its initialization is never read
> >
> > Signed-off-by: Colin Ian King <colin.k...@canonical.com>
> 
> s/intialization/initialization/g

Other than that typo, looks fine:
Tested-by: Aaron Brown <aaron.f.br...@intel.com>


RE: [Intel-wired-lan] [PATCH] e1000e: Ignore TSYNCRXCTL when getting I219 clock attributes

2018-05-22 Thread Brown, Aaron F
> From: Intel-wired-lan [mailto:intel-wired-lan-boun...@osuosl.org] On
> Behalf Of Benjamin Poirier
> Sent: Thursday, May 10, 2018 12:29 AM
> To: Kirsher, Jeffrey T 
> Cc: ehabk...@redhat.com; netdev@vger.kernel.org; jaya...@goubiq.com;
> linux-ker...@vger.kernel.org; bart.vanass...@wdc.com;
> postmodern.m...@gmail.com; Achim Mildenberger
> ; intel-wired-...@lists.osuosl.org;
> olouvig...@gmail.com
> Subject: [Intel-wired-lan] [PATCH] e1000e: Ignore TSYNCRXCTL when getting
> I219 clock attributes
> 
> There have been multiple reports of crashes that look like
> kernel: RIP: 0010:[] timecounter_read+0xf/0x50
> [...]
> kernel: Call Trace:
> kernel:  [] e1000e_phc_gettime+0x2f/0x60 [e1000e]
> kernel:  [] e1000e_systim_overflow_work+0x1d/0x80
> [e1000e]
> kernel:  [] process_one_work+0x155/0x440
> kernel:  [] worker_thread+0x116/0x4b0
> kernel:  [] kthread+0xd2/0xf0
> kernel:  [] ret_from_fork+0x3f/0x70
> 
> These can be traced back to the fact that e1000e_systim_reset() skips the
> timecounter_init() call if e1000e_get_base_timinca() returns -EINVAL, which
> leads to a null deref in timecounter_read().
> 
> Commit 83129b37ef35 ("e1000e: fix systim issues", v4.2-rc1) reworked
> e1000e_get_base_timinca() in such a way that it can return -EINVAL for
> e1000_pch_spt if the SYSCFI bit is not set in TSYNCRXCTL.
> 
> Some experimentation has shown that on I219 (e1000_pch_spt, "MAC: 12")
> adapters, the E1000_TSYNCRXCTL_SYSCFI flag is unstable; TSYNCRXCTL reads
> sometimes don't have the SYSCFI bit set. Retrying the read shortly after
> finds the bit to be set. This was observed at boot (probe) but also link up
> and link down.
> 
> Moreover, the phc (PTP Hardware Clock) seems to operate normally even
> after
> reads where SYSCFI=0. Therefore, remove this register read and
> unconditionally set the clock parameters.
> 
> Reported-by: Achim Mildenberger 
> Message-Id: <20180425065243.g5mqewg5irkwgwgv@f2>
> Bugzilla: https://bugzilla.suse.com/show_bug.cgi?id=1075876
> Fixes: 83129b37ef35 ("e1000e: fix systim issues")
> Signed-off-by: Benjamin Poirier 
> ---
>  drivers/net/ethernet/intel/e1000e/netdev.c | 15 ++-
>  1 file changed, 6 insertions(+), 9 deletions(-)

Tested-by: Aaron Brown 


RE: [Intel-wired-lan] [next-queue PATCH v9 6/6] igb: Add support for CBS offload

2017-10-19 Thread Brown, Aaron F
> From: Intel-wired-lan [mailto:intel-wired-lan-boun...@osuosl.org] On Behalf
> Of Vinicius Costa Gomes
> Sent: Monday, October 16, 2017 6:01 PM
> To: netdev@vger.kernel.org; intel-wired-...@lists.osuosl.org
> Cc: rodney.cummi...@ni.com; Guedes, Andre ;
> j...@resnulli.us; Briano, Ivan ;
> richardcoch...@gmail.com; hen...@austad.us; j...@mojatatu.com;
> levipear...@gmail.com; Ong, Boon Leong ;
> xiyou.wangc...@gmail.com; Sanchez-Palencia, Jesus  palen...@intel.com>
> Subject: [Intel-wired-lan] [next-queue PATCH v9 6/6] igb: Add support for
> CBS offload
> 
> From: Andre Guedes 
> 
> This patch adds support for Credit-Based Shaper (CBS) qdisc offload
> from Traffic Control system. This support enable us to leverage the
> Forwarding and Queuing for Time-Sensitive Streams (FQTSS) features
> from Intel i210 Ethernet Controller. FQTSS is the former 802.1Qav
> standard which was merged into 802.1Q in 2014. It enables traffic
> prioritization and bandwidth reservation via the Credit-Based Shaper
> which is implemented in hardware by i210 controller.
> 
> The patch introduces the igb_setup_tc() function which implements the
> support for CBS qdisc hardware offload in the IGB driver. CBS offload
> is the only traffic control offload supported by the driver at the
> moment.
> 
> FQTSS transmission mode from i210 controller is automatically enabled
> by the IGB driver when the CBS is enabled for the first hardware
> queue. Likewise, FQTSS mode is automatically disabled when CBS is
> disabled for the last hardware queue. Changing FQTSS mode requires NIC
> reset.
> 
> FQTSS feature is supported by i210 controller only.
> 
> Signed-off-by: Andre Guedes 
> ---
>  drivers/net/ethernet/intel/igb/e1000_defines.h |  23 ++
>  drivers/net/ethernet/intel/igb/e1000_regs.h|   8 +
>  drivers/net/ethernet/intel/igb/igb.h   |   6 +
>  drivers/net/ethernet/intel/igb/igb_main.c  | 347
> +
>  4 files changed, 384 insertions(+)

>From a regression standpoint:
Tested-by: Aaron Brown 


RE: [PATCH v3] igb: Free IRQs when device is hotplugged

2017-12-29 Thread Brown, Aaron F
> From: netdev-ow...@vger.kernel.org [mailto:netdev-
> ow...@vger.kernel.org] On Behalf Of Lyude Paul
> Sent: Tuesday, December 12, 2017 11:32 AM
> To: intel-wired-...@lists.osuosl.org
> Cc: Fujinaka, Todd ; Stephen Hemminger
> ; sta...@vger.kernel.org; Kirsher, Jeffrey T
> ; netdev@vger.kernel.org; linux-
> ker...@vger.kernel.org
> Subject: [PATCH v3] igb: Free IRQs when device is hotplugged
> 
> Recently I got a Caldigit TS3 Thunderbolt 3 dock, and noticed that upon
> hotplugging my kernel would immediately crash due to igb:
> 
> [  680.825801] kernel BUG at drivers/pci/msi.c:352!
> [  680.828388] invalid opcode:  [#1] SMP
> [  680.829194] Modules linked in: igb(O) thunderbolt i2c_algo_bit joydev vfat
> fat btusb btrtl btbcm btintel bluetooth ecdh_generic hp_wmi
> sparse_keymap rfkill wmi_bmof iTCO_wdt intel_rapl
> x86_pkg_temp_thermal coretemp crc32_pclmul snd_pcm rtsx_pci_ms
> mei_me snd_timer memstick snd pcspkr mei soundcore i2c_i801 tpm_tis
> psmouse shpchp wmi tpm_tis_core tpm video hp_wireless acpi_pad
> rtsx_pci_sdmmc mmc_core crc32c_intel serio_raw rtsx_pci mfd_core
> xhci_pci xhci_hcd i2c_hid i2c_core [last unloaded: igb]
> [  680.831085] CPU: 1 PID: 78 Comm: kworker/u16:1 Tainted: G   O
> 4.15.0-rc3Lyude-Test+ #6
> [  680.831596] Hardware name: HP HP ZBook Studio G4/826B, BIOS P71 Ver.
> 01.03 06/09/2017
> [  680.832168] Workqueue: kacpi_hotplug acpi_hotplug_work_fn
> [  680.832687] RIP: 0010:free_msi_irqs+0x180/0x1b0
> [  680.833271] RSP: 0018:c930fbf0 EFLAGS: 00010286
> [  680.833761] RAX: 8803405f9c00 RBX: 88033e3d2e40 RCX:
> 002c
> [  680.834278] RDX:  RSI: 00ac RDI:
> 880340be2178
> [  680.834832] RBP:  R08: 880340be1ff0 R09:
> 8803405f9c00
> [  680.835342] R10:  R11: 0040 R12:
> 88033d63a298
> [  680.835822] R13: 88033d63a000 R14: 0060 R15:
> 880341959000
> [  680.836332] FS:  () GS:88034f44()
> knlGS:
> [  680.836817] CS:  0010 DS:  ES:  CR0: 80050033
> [  680.837360] CR2: 55e64044afdf CR3: 01c09002 CR4:
> 003606e0
> [  680.837954] Call Trace:
> [  680.838853]  pci_disable_msix+0xce/0xf0
> [  680.839616]  igb_reset_interrupt_capability+0x5d/0x60 [igb]
> [  680.840278]  igb_remove+0x9d/0x110 [igb]
> [  680.840764]  pci_device_remove+0x36/0xb0
> [  680.841279]  device_release_driver_internal+0x157/0x220
> [  680.841739]  pci_stop_bus_device+0x7d/0xa0
> [  680.842255]  pci_stop_bus_device+0x2b/0xa0
> [  680.842722]  pci_stop_bus_device+0x3d/0xa0
> [  680.843189]  pci_stop_and_remove_bus_device+0xe/0x20
> [  680.843627]  trim_stale_devices+0xf3/0x140
> [  680.844086]  trim_stale_devices+0x94/0x140
> [  680.844532]  trim_stale_devices+0xa6/0x140
> [  680.845031]  ? get_slot_status+0x90/0xc0
> [  680.845536]  acpiphp_check_bridge.part.5+0xfe/0x140
> [  680.846021]  acpiphp_hotplug_notify+0x175/0x200
> [  680.846581]  ? free_bridge+0x100/0x100
> [  680.847113]  acpi_device_hotplug+0x8a/0x490
> [  680.847535]  acpi_hotplug_work_fn+0x1a/0x30
> [  680.848076]  process_one_work+0x182/0x3a0
> [  680.848543]  worker_thread+0x2e/0x380
> [  680.848963]  ? process_one_work+0x3a0/0x3a0
> [  680.849373]  kthread+0x111/0x130
> [  680.849776]  ? kthread_create_worker_on_cpu+0x50/0x50
> [  680.850188]  ret_from_fork+0x1f/0x30
> [  680.850601] Code: 43 14 85 c0 0f 84 d5 fe ff ff 31 ed eb 0f 83 c5 01 39 6b 
> 14 0f
> 86 c5 fe ff ff 8b 7b 10 01 ef e8 b7 e4 d2 ff 48 83 78 70 00 74 e3 <0f> 0b 49 
> 8d b5
> a0 00 00 00 e8 62 6f d3 ff e9 c7 fe ff ff 48 8b
> [  680.851497] RIP: free_msi_irqs+0x180/0x1b0 RSP: c930fbf0
> 
> As it turns out, normally the freeing of IRQs that would fix this is called
> inside of the scope of __igb_close(). However, since the device is
> already gone by the point we try to unregister the netdevice from the
> driver due to a hotplug we end up seeing that the netif isn't present
> and thus, forget to free any of the device IRQs.
> 
> So: make sure that if we're in the process of dismantling the netdev, we
> always allow __igb_close() to be called so that IRQs may be freed
> normally. Additionally, only allow igb_close() to be called from
> __igb_close() if it hasn't already been called for the given adapter.
> 
> Signed-off-by: Lyude Paul 
> Fixes: 9474933caf21 ("igb: close/suspend race in netif_device_detach")
> Cc: Todd Fujinaka 
> Cc: Stephen Hemminger 
> Cc: sta...@vger.kernel.org
> ---
> Changes since v2:
>   - Remove hunk in __igb_close() that was left over by accident, it's
> not needed
> 
>  drivers/net/ethernet/intel/igb/igb_main.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

Tested-by: Aaron Brown 


RE: [Intel-wired-lan] [PATCH 08/27] e1000e: Use timecounter_initialize interface

2018-01-05 Thread Brown, Aaron F
> From: Intel-wired-lan [mailto:intel-wired-lan-boun...@osuosl.org] On
> Behalf Of Sagar Arun Kamble
> Sent: Thursday, December 14, 2017 11:38 PM
> To: linux-ker...@vger.kernel.org
> Cc: intel-wired-...@lists.osuosl.org; Richard Cochran
> ; Kamble, Sagar A
> ; netdev@vger.kernel.org
> Subject: [Intel-wired-lan] [PATCH 08/27] e1000e: Use timecounter_initialize
> interface
> 
> With new interface timecounter_initialize we can initialize timecounter
> fields and underlying cyclecounter together. Update e1000e timecounter
> init with this new function.
> 
> Signed-off-by: Sagar Arun Kamble 
> Cc: Richard Cochran 
> Cc: Jeff Kirsher 
> Cc: intel-wired-...@lists.osuosl.org
> Cc: netdev@vger.kernel.org
> Cc: linux-ker...@vger.kernel.org
> ---
>  drivers/net/ethernet/intel/e1000e/e1000.h  |  4 
>  drivers/net/ethernet/intel/e1000e/netdev.c | 31 +-
> 
>  2 files changed, 22 insertions(+), 13 deletions(-)
> 

Tested-by: Aaron Brown 


RE: [Intel-wired-lan] [PATCH 22/27] ixgbe: Use timecounter_reset interface

2018-01-05 Thread Brown, Aaron F
> From: Intel-wired-lan [mailto:intel-wired-lan-boun...@osuosl.org] On
> Behalf Of Sagar Arun Kamble
> Sent: Thursday, December 14, 2017 11:39 PM
> To: linux-ker...@vger.kernel.org
> Cc: intel-wired-...@lists.osuosl.org; Richard Cochran
> ; Kamble, Sagar A
> ; netdev@vger.kernel.org
> Subject: [Intel-wired-lan] [PATCH 22/27] ixgbe: Use timecounter_reset
> interface
> 
> With new interface timecounter_reset we can update the start time for
> timecounter. Update ixgbe_ptp_settime with this new function.
> 
> Signed-off-by: Sagar Arun Kamble 
> Cc: Richard Cochran 
> Cc: Jeff Kirsher 
> Cc: intel-wired-...@lists.osuosl.org
> Cc: netdev@vger.kernel.org
> Cc: linux-ker...@vger.kernel.org
> ---
>  drivers/net/ethernet/intel/ixgbe/ixgbe_ptp.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 

Tested-by: Aaron Brown 


RE: [Intel-wired-lan] [PATCH 21/27] igb: Use timecounter_reset interface

2018-01-05 Thread Brown, Aaron F
> From: Intel-wired-lan [mailto:intel-wired-lan-boun...@osuosl.org] On
> Behalf Of Sagar Arun Kamble
> Sent: Thursday, December 14, 2017 11:39 PM
> To: linux-ker...@vger.kernel.org
> Cc: intel-wired-...@lists.osuosl.org; Richard Cochran
> ; Kamble, Sagar A
> ; netdev@vger.kernel.org
> Subject: [Intel-wired-lan] [PATCH 21/27] igb: Use timecounter_reset
> interface
> 
> With new interface timecounter_reset we can update the start time for
> timecounter. Update igb_ptp_settime_82576 with this new function.
> 
> Signed-off-by: Sagar Arun Kamble 
> Cc: Richard Cochran 
> Cc: Jeff Kirsher 
> Cc: intel-wired-...@lists.osuosl.org
> Cc: netdev@vger.kernel.org
> Cc: linux-ker...@vger.kernel.org
> ---
>  drivers/net/ethernet/intel/igb/igb_ptp.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 

Tested-by: Aaron Brown 


RE: [Intel-wired-lan] [PATCH 22/27] ixgbe: Use timecounter_reset interface

2018-01-05 Thread Brown, Aaron F


> -Original Message-
> From: Brown, Aaron F
> Sent: Friday, January 5, 2018 8:34 PM
> To: 'Sagar Arun Kamble' <sagar.a.kam...@intel.com>; linux-
> ker...@vger.kernel.org
> Cc: intel-wired-...@lists.osuosl.org; Richard Cochran
> <richardcoch...@gmail.com>; Kamble, Sagar A
> <sagar.a.kam...@intel.com>; netdev@vger.kernel.org
> Subject: RE: [Intel-wired-lan] [PATCH 22/27] ixgbe: Use timecounter_reset
> interface
> 
> > From: Intel-wired-lan [mailto:intel-wired-lan-boun...@osuosl.org] On
> > Behalf Of Sagar Arun Kamble
> > Sent: Thursday, December 14, 2017 11:39 PM
> > To: linux-ker...@vger.kernel.org
> > Cc: intel-wired-...@lists.osuosl.org; Richard Cochran
> > <richardcoch...@gmail.com>; Kamble, Sagar A
> > <sagar.a.kam...@intel.com>; netdev@vger.kernel.org
> > Subject: [Intel-wired-lan] [PATCH 22/27] ixgbe: Use timecounter_reset
> > interface
> >
> > With new interface timecounter_reset we can update the start time for
> > timecounter. Update ixgbe_ptp_settime with this new function.
> >
> > Signed-off-by: Sagar Arun Kamble <sagar.a.kam...@intel.com>
> > Cc: Richard Cochran <richardcoch...@gmail.com>
> > Cc: Jeff Kirsher <jeffrey.t.kirs...@intel.com>
> > Cc: intel-wired-...@lists.osuosl.org
> > Cc: netdev@vger.kernel.org
> > Cc: linux-ker...@vger.kernel.org
> > ---
> >  drivers/net/ethernet/intel/ixgbe/ixgbe_ptp.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> 
> Tested-by: Aaron Brown <aaron.f.br...@intel.com>
Strike my Tested-by: for this (ixgbe) instance.  It was meant for igb.


RE: [Intel-wired-lan] [PATCH 09/27] igb: Use timecounter_initialize interface

2018-01-05 Thread Brown, Aaron F
> From: Intel-wired-lan [mailto:intel-wired-lan-boun...@osuosl.org] On
> Behalf Of Sagar Arun Kamble
> Sent: Thursday, December 14, 2017 11:38 PM
> To: linux-ker...@vger.kernel.org
> Cc: intel-wired-...@lists.osuosl.org; Richard Cochran
> ; Kamble, Sagar A
> ; netdev@vger.kernel.org
> Subject: [Intel-wired-lan] [PATCH 09/27] igb: Use timecounter_initialize
> interface
> 
> With new interface timecounter_initialize we can initialize timecounter
> fields and underlying cyclecounter together. Update igb ptp timecounter
> init with this new function.
> 
> Signed-off-by: Sagar Arun Kamble 
> Cc: Richard Cochran 
> Cc: Jeff Kirsher 
> Cc: intel-wired-...@lists.osuosl.org
> Cc: netdev@vger.kernel.org
> Cc: linux-ker...@vger.kernel.org
> ---
>  drivers/net/ethernet/intel/igb/igb.h |  4 
>  drivers/net/ethernet/intel/igb/igb_ptp.c | 23 ++-
>  2 files changed, 18 insertions(+), 9 deletions(-)
> 

Tested-by: Aaron Brown 


RE: [Intel-wired-lan] [PATCH 1/1] timecounter: Make cyclecounter struct part of timecounter struct

2018-01-05 Thread Brown, Aaron F
> From: Intel-wired-lan [mailto:intel-wired-lan-boun...@osuosl.org] On
> Behalf Of Jeff Kirsher
> Sent: Wednesday, December 6, 2017 8:25 AM
> To: Kamble, Sagar A ; linux-
> ker...@vger.kernel.org
> Cc: alsa-de...@alsa-project.org; linux-r...@vger.kernel.org;
> netdev@vger.kernel.org; Richard Cochran ;
> Stephen Boyd ; Chris Wilson  wilson.co.uk>; John Stultz ; intel-wired-
> l...@lists.osuosl.org; Thomas Gleixner ;
> kvm...@lists.cs.columbia.edu; linux-arm-ker...@lists.infradead.org
> Subject: Re: [Intel-wired-lan] [PATCH 1/1] timecounter: Make cyclecounter
> struct part of timecounter struct
> 
> On Sat, 2017-12-02 at 10:01 +0530, Sagar Arun Kamble wrote:
> > There is no real need for the users of timecounters to define
> > cyclecounter
> > and timecounter variables separately. Since timecounter will always
> > be
> > based on cyclecounter, have cyclecounter struct as member of
> > timecounter
> > struct.
> >
> > Suggested-by: Chris Wilson 
> > Signed-off-by: Sagar Arun Kamble 
> > Cc: Chris Wilson 
> > Cc: Richard Cochran 
> > Cc: John Stultz 
> > Cc: Thomas Gleixner 
> > Cc: Stephen Boyd 
> > Cc: linux-ker...@vger.kernel.org
> > Cc: linux-arm-ker...@lists.infradead.org
> > Cc: netdev@vger.kernel.org
> > Cc: intel-wired-...@lists.osuosl.org
> > Cc: linux-r...@vger.kernel.org
> > Cc: alsa-de...@alsa-project.org
> > Cc: kvm...@lists.cs.columbia.edu
> 
> Acked-by: Jeff Kirsher 
> 

Tested-by: Aaron Brown 

> For the changes to the Intel drivers.
> 
> > ---
> >  arch/microblaze/kernel/timer.c | 20 ++--
> >  drivers/clocksource/arm_arch_timer.c   | 19 ++--
> >  drivers/net/ethernet/amd/xgbe/xgbe-dev.c   |  3 +-
> >  drivers/net/ethernet/amd/xgbe/xgbe-ptp.c   |  9 +++---
> >  drivers/net/ethernet/amd/xgbe/xgbe.h   |  1 -
> >  drivers/net/ethernet/broadcom/bnx2x/bnx2x.h|  1 -
> >  drivers/net/ethernet/broadcom/bnx2x/bnx2x_main.c   | 20 ++--
> >  drivers/net/ethernet/freescale/fec.h   |  1 -
> >  drivers/net/ethernet/freescale/fec_ptp.c   | 30 +---
> > --
> >  drivers/net/ethernet/intel/e1000e/e1000.h  |  1 -
> >  drivers/net/ethernet/intel/e1000e/netdev.c | 27 --
> > --
> >  drivers/net/ethernet/intel/e1000e/ptp.c|  2 +-
> >  drivers/net/ethernet/intel/igb/igb.h   |  1 -
> >  drivers/net/ethernet/intel/igb/igb_ptp.c   | 25 --
> > -
> >  drivers/net/ethernet/intel/ixgbe/ixgbe.h   |  1 -
> >  drivers/net/ethernet/intel/ixgbe/ixgbe_ptp.c   | 17 +-
> >  drivers/net/ethernet/mellanox/mlx4/en_clock.c  | 28 --
> > ---
> >  drivers/net/ethernet/mellanox/mlx4/mlx4_en.h   |  1 -
> >  .../net/ethernet/mellanox/mlx5/core/lib/clock.c| 34 ++
> > --
> >  drivers/net/ethernet/qlogic/qede/qede_ptp.c| 20 ++--
> >  drivers/net/ethernet/ti/cpts.c | 36
> > --
> >  drivers/net/ethernet/ti/cpts.h |  1 -
> >  include/linux/mlx5/driver.h|  1 -
> >  include/linux/timecounter.h|  4 +--
> >  include/sound/hdaudio.h|  1 -
> >  kernel/time/timecounter.c  | 28 --
> > ---
> >  sound/hda/hdac_stream.c|  7 +++--
> >  virt/kvm/arm/arch_timer.c  |  6 ++--
> >  28 files changed, 163 insertions(+), 182 deletions(-)


RE: [Intel-wired-lan] [PATCH 20/27] e1000e: Use timecounter_reset interface

2018-01-05 Thread Brown, Aaron F
> From: Intel-wired-lan [mailto:intel-wired-lan-boun...@osuosl.org] On
> Behalf Of Sagar Arun Kamble
> Sent: Thursday, December 14, 2017 11:39 PM
> To: linux-ker...@vger.kernel.org
> Cc: intel-wired-...@lists.osuosl.org; Richard Cochran
> ; Kamble, Sagar A
> ; netdev@vger.kernel.org
> Subject: [Intel-wired-lan] [PATCH 20/27] e1000e: Use timecounter_reset
> interface
> 
> With new interface timecounter_reset we can update the start time for
> timecounter. Update e1000e_phc_settime with this new function.
> 
> Signed-off-by: Sagar Arun Kamble 
> Cc: Richard Cochran 
> Cc: Jeff Kirsher 
> Cc: intel-wired-...@lists.osuosl.org
> Cc: netdev@vger.kernel.org
> Cc: linux-ker...@vger.kernel.org
> ---
>  drivers/net/ethernet/intel/e1000e/ptp.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
Tested-by: Aaron Brown 


RE: [Intel-wired-lan] [PATCH 01/27] timecounter: Make cyclecounter struct part of timecounter struct

2018-01-08 Thread Brown, Aaron F
> From: Intel-wired-lan [mailto:intel-wired-lan-boun...@osuosl.org] On
> Behalf Of Sagar Arun Kamble
> Sent: Thursday, December 14, 2017 11:38 PM
> To: linux-ker...@vger.kernel.org
> Cc: alsa-de...@alsa-project.org; linux-r...@vger.kernel.org;
> netdev@vger.kernel.org; Richard Cochran ;
> Stephen Boyd ; Chris Wilson  wilson.co.uk>; John Stultz ; intel-wired-
> l...@lists.osuosl.org; Thomas Gleixner ; Kamble, Sagar A
> ; kvm...@lists.cs.columbia.edu; linux-arm-
> ker...@lists.infradead.org
> Subject: [Intel-wired-lan] [PATCH 01/27] timecounter: Make cyclecounter
> struct part of timecounter struct
> 
> There is no real need for the users of timecounters to define cyclecounter
> and timecounter variables separately. Since timecounter will always be
> based on cyclecounter, have cyclecounter struct as member of timecounter
> struct.
> 
> v2: Rebase.
> 
> Suggested-by: Chris Wilson 
> Signed-off-by: Sagar Arun Kamble 
> Cc: Chris Wilson 
> Cc: Richard Cochran 
> Cc: John Stultz 
> Cc: Thomas Gleixner 
> Cc: Stephen Boyd 
> Cc: linux-ker...@vger.kernel.org
> Cc: linux-arm-ker...@lists.infradead.org
> Cc: netdev@vger.kernel.org
> Cc: intel-wired-...@lists.osuosl.org
> Cc: linux-r...@vger.kernel.org
> Cc: alsa-de...@alsa-project.org
> Cc: kvm...@lists.cs.columbia.edu
> Acked-by: Jeff Kirsher  (Intel drivers)
> ---
>  arch/microblaze/kernel/timer.c | 20 ++--
>  drivers/clocksource/arm_arch_timer.c   | 19 ++--
>  drivers/net/ethernet/amd/xgbe/xgbe-dev.c   |  3 +-
>  drivers/net/ethernet/amd/xgbe/xgbe-ptp.c   |  9 +++---
>  drivers/net/ethernet/amd/xgbe/xgbe.h   |  1 -
>  drivers/net/ethernet/broadcom/bnx2x/bnx2x.h|  1 -
>  drivers/net/ethernet/broadcom/bnx2x/bnx2x_main.c   | 20 ++--
>  drivers/net/ethernet/freescale/fec.h   |  1 -
>  drivers/net/ethernet/freescale/fec_ptp.c   | 30 +-
>  drivers/net/ethernet/intel/e1000e/e1000.h  |  1 -
>  drivers/net/ethernet/intel/e1000e/netdev.c | 27 
>  drivers/net/ethernet/intel/e1000e/ptp.c|  2 +-
>  drivers/net/ethernet/intel/igb/igb.h   |  1 -
>  drivers/net/ethernet/intel/igb/igb_ptp.c   | 25 ---
>  drivers/net/ethernet/intel/ixgbe/ixgbe.h   |  1 -
>  drivers/net/ethernet/intel/ixgbe/ixgbe_ptp.c   | 17 +-
>  drivers/net/ethernet/mellanox/mlx4/en_clock.c  | 28 -
>  drivers/net/ethernet/mellanox/mlx4/mlx4_en.h   |  1 -
>  .../net/ethernet/mellanox/mlx5/core/lib/clock.c| 34 ++--
>  drivers/net/ethernet/qlogic/qede/qede_ptp.c| 20 ++--
>  drivers/net/ethernet/ti/cpts.c | 36 
> --
>  drivers/net/ethernet/ti/cpts.h |  1 -
>  include/linux/mlx5/driver.h|  1 -
>  include/linux/timecounter.h|  4 +--
>  include/sound/hdaudio.h|  1 -
>  kernel/time/timecounter.c  | 28 -
>  sound/hda/hdac_stream.c|  7 +++--
>  virt/kvm/arm/arch_timer.c  |  6 ++--
>  28 files changed, 163 insertions(+), 182 deletions(-)
> 

For Intel e1000e and igb drivers:
Tested-by: Aaron Brown 


  1   2   >