[net-next 00/12][pull request] Intel Wired LAN Driver Updates 2018-11-07

[net-next 00/12][pull request] Intel Wired LAN Driver Updates 2018-11-07

[Jeff Kirsher]

This series contains updates to almost all of the Intel wired LAN
drivers.

Lance Roy replaces a spin lock with lockdep_assert_held() for igbvf
driver in move toward trying to remove spin_is_locked().

Colin Ian King fixes a potential null pointer dereference by adding a
check in ixgbe.  Also fixed the igc driver by properly assigning the
return error code of a function call, so that we can properly check it.

Shannon Nelson updates the ixgbe driver to not block IPsec offload when
in VEPA mode, in VEB mode, IPsec offload is still blocked because the
device drops packets into a black hole.

Jake adds support for software timestamping for packets sent over
ixgbevf.  Also modifies i40e, iavf, igb, igc, and ixgbe to delay calling
skb_tx_timestamp() to the latest point possible, which is just prior to
notifying the hardware of the new Tx packet.

Todd adds the new WoL filter flag so that we properly report that we do
not support this new feature.

YueHaibing from Huawei fixes the igc driver by cleaning up variables
that are not "really" used.

Dan Carpenter cleans up igc whitespace issues.

Miroslav Lichvar fixes e1000e for potential underflow issue in the
timecounter, so modify the driver to use timecounter_cyc2time() to allow
non-monotonic SYSTIM readings.

Sasha provides additional igc cleanups based on community feedback.

The following are changes since commit 7c588c7468ea3f9b2fc8fa6840bed6262b5d1b00:
  Merge branch 'net-systemport-Unmap-queues-upon-DSA-unregister-event'
and are available in the git repository at:
  git://git.kernel.org/pub/scm/linux/kernel/git/jkirsher/next-queue 1GbE

Colin Ian King (2):
  ixgbe: don't clear_bit on xdp_ring->state if xdp_ring is null
  igc: fix error return handling from call to
    netif_set_real_num_tx_queues

Dan Carpenter (1):
  igc: Tidy up some white space

Jacob Keller (2):
  ixgbevf: add support for software timestamps
  intel-ethernet: software timestamp skbs as late as possible

Lance Roy (1):
  igbvf: Replace spin_is_locked() with lockdep

Miroslav Lichvar (1):
  e1000e: allow non-monotonic SYSTIM readings

Sasha Neftin (1):
  igc: Clean up code

Shannon Nelson (1):
  ixgbe: allow IPsec Tx offload in VEPA mode

Todd Fujinaka (1):
  i40e/ixgbe/igb: fail on new WoL flag setting WAKE_MAGICSECURE

YueHaibing (2):
  igc: Remove set but not used variables 'ctrl_ext, link_mode'
  igc: Remove set but not used variable 'pci_using_dac'

 drivers/net/ethernet/intel/e1000e/ptp.c       | 13 +++++--
 .../net/ethernet/intel/i40e/i40e_ethtool.c    |  3 +-
 drivers/net/ethernet/intel/i40e/i40e_txrx.c   |  4 +--
 drivers/net/ethernet/intel/iavf/iavf_txrx.c   |  4 +--
 drivers/net/ethernet/intel/igb/igb_ethtool.c  |  2 +-
 drivers/net/ethernet/intel/igb/igb_main.c     |  4 +--
 drivers/net/ethernet/intel/igbvf/mbx.c        |  4 +--
 drivers/net/ethernet/intel/igc/igc.h          |  9 -----
 drivers/net/ethernet/intel/igc/igc_base.c     |  8 -----
 drivers/net/ethernet/intel/igc/igc_main.c     | 36 +++++--------------
 .../net/ethernet/intel/ixgbe/ixgbe_ethtool.c  |  3 +-
 .../net/ethernet/intel/ixgbe/ixgbe_ipsec.c    |  4 ++-
 drivers/net/ethernet/intel/ixgbe/ixgbe_main.c |  7 ++--
 .../net/ethernet/intel/ixgbevf/ixgbevf_main.c |  2 ++
 14 files changed, 41 insertions(+), 62 deletions(-)

-- 
2.19.1

[net-next 01/12] igbvf: Replace spin_is_locked() with lockdep

[Jeff Kirsher]

From: Lance Roy <ldr709@gmail.com>

lockdep_assert_held() is better suited to checking locking requirements,
since it won't get confused when someone else holds the lock. This is
also a step towards possibly removing spin_is_locked().

Signed-off-by: Lance Roy <ldr709@gmail.com>
Tested-by: Aaron Brown <aaron.f.brown@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
 drivers/net/ethernet/intel/igbvf/mbx.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/intel/igbvf/mbx.c b/drivers/net/ethernet/intel/igbvf/mbx.c
index 163e5838f7c2..a3cd7ac48d4b 100644
--- a/drivers/net/ethernet/intel/igbvf/mbx.c
+++ b/drivers/net/ethernet/intel/igbvf/mbx.c
@@ -241,7 +241,7 @@ static s32 e1000_write_mbx_vf(struct e1000_hw *hw, u32 *msg, u16 size)
 	s32 err;
 	u16 i;
 
-	WARN_ON_ONCE(!spin_is_locked(&hw->mbx_lock));
+	lockdep_assert_held(&hw->mbx_lock);
 
 	/* lock the mailbox to prevent pf/vf race condition */
 	err = e1000_obtain_mbx_lock_vf(hw);
@@ -279,7 +279,7 @@ static s32 e1000_read_mbx_vf(struct e1000_hw *hw, u32 *msg, u16 size)
 	s32 err;
 	u16 i;
 
-	WARN_ON_ONCE(!spin_is_locked(&hw->mbx_lock));
+	lockdep_assert_held(&hw->mbx_lock);
 
 	/* lock the mailbox to prevent pf/vf race condition */
 	err = e1000_obtain_mbx_lock_vf(hw);
-- 
2.19.1

[net-next 02/12] ixgbe: don't clear_bit on xdp_ring->state if xdp_ring is null

[Jeff Kirsher]

From: Colin Ian King <colin.king@canonical.com>

There is an earlier check to see if xdp_ring is null when configuring
the tx ring, so assuming that it can still be null, the clearing of
the xdp_ring->state currently could end up with a null pointer
dereference.  Fix this by only clearing the bit if xdp_ring is not null.

Detected by CoverityScan, CID#1473795 ("Dereference after null check")

Fixes: 024aa5800f32 ("ixgbe: added Rx/Tx ring disable/enable functions")
Signed-off-by: Colin Ian King <colin.king@canonical.com>
Tested-by: Andrew Bowers <andrewx.bowers@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
 drivers/net/ethernet/intel/ixgbe/ixgbe_main.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
index 113b38e0defb..aeda1834e66a 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
@@ -10517,7 +10517,8 @@ void ixgbe_txrx_ring_enable(struct ixgbe_adapter *adapter, int ring)
 	ixgbe_configure_rx_ring(adapter, rx_ring);
 
 	clear_bit(__IXGBE_TX_DISABLED, &tx_ring->state);
-	clear_bit(__IXGBE_TX_DISABLED, &xdp_ring->state);
+	if (xdp_ring)
+		clear_bit(__IXGBE_TX_DISABLED, &xdp_ring->state);
 }
 
 /**
-- 
2.19.1

[net-next 03/12] ixgbe: allow IPsec Tx offload in VEPA mode

[Jeff Kirsher]

From: Shannon Nelson <shannon.nelson@oracle.com>

When it's possible that the PF might end up trying to send a
packet to one of its own VFs, we have to forbid IPsec offload
because the device drops the packets into a black hole.
See commit 47b6f50077e6 ("ixgbe: disallow IPsec Tx offload
when in SR-IOV mode") for more info.

This really is only necessary when the device is in the default
VEB mode.  If instead the device is running in VEPA mode,
the packets will go through the encryption engine and out the
MAC/PHY as normal, and get "hairpinned" as needed by the switch.

So let's not block IPsec offload when in VEPA mode.  To get
there with the ixgbe device, use the handy 'bridge' command:
	bridge link set dev eth1 hwmode vepa

Signed-off-by: Shannon Nelson <shannon.nelson@oracle.com>
Tested-by: Andrew Bowers <andrewx.bowers@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
 drivers/net/ethernet/intel/ixgbe/ixgbe_ipsec.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_ipsec.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_ipsec.c
index fd1b0546fd67..4d77f42e035c 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_ipsec.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_ipsec.c
@@ -4,6 +4,7 @@
 #include "ixgbe.h"
 #include <net/xfrm.h>
 #include <crypto/aead.h>
+#include <linux/if_bridge.h>
 
 #define IXGBE_IPSEC_KEY_BITS  160
 static const char aes_gcm_name[] = "rfc4106(gcm(aes))";
@@ -693,7 +694,8 @@ static int ixgbe_ipsec_add_sa(struct xfrm_state *xs)
 	} else {
 		struct tx_sa tsa;
 
-		if (adapter->num_vfs)
+		if (adapter->num_vfs &&
+		    adapter->bridge_mode != BRIDGE_MODE_VEPA)
 			return -EOPNOTSUPP;
 
 		/* find the first unused index */
-- 
2.19.1

[net-next 04/12] ixgbevf: add support for software timestamps

[Jeff Kirsher]

From: Jacob Keller <jacob.e.keller@intel.com>

Add a call to skb_tx_timestamp in the ixgbevf_tx_map function. This
enables software timestamping for packets sent over this device driver.
The call is placed just prior to when we notify hardware of the new
packet, in order to software timestamp as close as possible to when the
hardware will transmit.

Signed-off-by: Jacob Keller <jacob.e.keller@intel.com>
Tested-by: Andrew Bowers <andrewx.bowers@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
 drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c b/drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c
index 5e47ede7e832..196b890467b2 100644
--- a/drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c
+++ b/drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c
@@ -4016,6 +4016,8 @@ static void ixgbevf_tx_map(struct ixgbevf_ring *tx_ring,
 	/* set the timestamp */
 	first->time_stamp = jiffies;
 
+	skb_tx_timestamp(skb);
+
 	/* Force memory writes to complete before letting h/w know there
 	 * are new descriptors to fetch.  (Only applicable for weak-ordered
 	 * memory model archs, such as IA-64).
-- 
2.19.1

[net-next 05/12] intel-ethernet: software timestamp skbs as late as possible

[Jeff Kirsher]

From: Jacob Keller <jacob.e.keller@intel.com>

Many of the Intel Ethernet drivers call skb_tx_timestamp() earlier than
necessary. Move the calls to this function to the latest point possible,
just prior to notifying hardware of the new Tx packet when we bump the
tail register.

This affects i40e, iavf, igb, igc, and ixgbe.

The e100, e1000, e1000e, fm10k, and ice drivers already call the
skb_tx_timestamp() function just prior to indicating the Tx packet to
hardware, so they do not need to be changed.

Signed-off-by: Jacob Keller <jacob.e.keller@intel.com>
Tested-by: Andrew Bowers <andrewx.bowers@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
 drivers/net/ethernet/intel/i40e/i40e_txrx.c   | 4 ++--
 drivers/net/ethernet/intel/iavf/iavf_txrx.c   | 4 ++--
 drivers/net/ethernet/intel/igb/igb_main.c     | 4 ++--
 drivers/net/ethernet/intel/igc/igc_main.c     | 4 ++--
 drivers/net/ethernet/intel/ixgbe/ixgbe_main.c | 4 ++--
 5 files changed, 10 insertions(+), 10 deletions(-)

diff --git a/drivers/net/ethernet/intel/i40e/i40e_txrx.c b/drivers/net/ethernet/intel/i40e/i40e_txrx.c
index aef3c89ee79c..1384a5a006a4 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_txrx.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_txrx.c
@@ -3473,6 +3473,8 @@ static inline int i40e_tx_map(struct i40e_ring *tx_ring, struct sk_buff *skb,
 	tx_desc->cmd_type_offset_bsz =
 			build_ctob(td_cmd, td_offset, size, td_tag);
 
+	skb_tx_timestamp(skb);
+
 	/* Force memory writes to complete before letting h/w know there
 	 * are new descriptors to fetch.
 	 *
@@ -3652,8 +3654,6 @@ static netdev_tx_t i40e_xmit_frame_ring(struct sk_buff *skb,
 	if (tsyn)
 		tx_flags |= I40E_TX_FLAGS_TSYN;
 
-	skb_tx_timestamp(skb);
-
 	/* always enable CRC insertion offload */
 	td_cmd |= I40E_TX_DESC_CMD_ICRC;
 
diff --git a/drivers/net/ethernet/intel/iavf/iavf_txrx.c b/drivers/net/ethernet/intel/iavf/iavf_txrx.c
index fb9bfad96daf..3b1dc77ae368 100644
--- a/drivers/net/ethernet/intel/iavf/iavf_txrx.c
+++ b/drivers/net/ethernet/intel/iavf/iavf_txrx.c
@@ -2343,6 +2343,8 @@ static inline void iavf_tx_map(struct iavf_ring *tx_ring, struct sk_buff *skb,
 	tx_desc->cmd_type_offset_bsz =
 			build_ctob(td_cmd, td_offset, size, td_tag);
 
+	skb_tx_timestamp(skb);
+
 	/* Force memory writes to complete before letting h/w know there
 	 * are new descriptors to fetch.
 	 *
@@ -2461,8 +2463,6 @@ static netdev_tx_t iavf_xmit_frame_ring(struct sk_buff *skb,
 	if (tso < 0)
 		goto out_drop;
 
-	skb_tx_timestamp(skb);
-
 	/* always enable CRC insertion offload */
 	td_cmd |= IAVF_TX_DESC_CMD_ICRC;
 
diff --git a/drivers/net/ethernet/intel/igb/igb_main.c b/drivers/net/ethernet/intel/igb/igb_main.c
index 5df88ad8ac81..4584ebc9e8fe 100644
--- a/drivers/net/ethernet/intel/igb/igb_main.c
+++ b/drivers/net/ethernet/intel/igb/igb_main.c
@@ -6019,6 +6019,8 @@ static int igb_tx_map(struct igb_ring *tx_ring,
 	/* set the timestamp */
 	first->time_stamp = jiffies;
 
+	skb_tx_timestamp(skb);
+
 	/* Force memory writes to complete before letting h/w know there
 	 * are new descriptors to fetch.  (Only applicable for weak-ordered
 	 * memory model archs, such as IA-64).
@@ -6147,8 +6149,6 @@ netdev_tx_t igb_xmit_frame_ring(struct sk_buff *skb,
 	else if (!tso)
 		igb_tx_csum(tx_ring, first);
 
-	skb_tx_timestamp(skb);
-
 	if (igb_tx_map(tx_ring, first, hdr_len))
 		goto cleanup_tx_tstamp;
 
diff --git a/drivers/net/ethernet/intel/igc/igc_main.c b/drivers/net/ethernet/intel/igc/igc_main.c
index 9d85707e8a81..615a5fcd5a00 100644
--- a/drivers/net/ethernet/intel/igc/igc_main.c
+++ b/drivers/net/ethernet/intel/igc/igc_main.c
@@ -865,6 +865,8 @@ static int igc_tx_map(struct igc_ring *tx_ring,
 	/* set the timestamp */
 	first->time_stamp = jiffies;
 
+	skb_tx_timestamp(skb);
+
 	/* Force memory writes to complete before letting h/w know there
 	 * are new descriptors to fetch.  (Only applicable for weak-ordered
 	 * memory model archs, such as IA-64).
@@ -959,8 +961,6 @@ static netdev_tx_t igc_xmit_frame_ring(struct sk_buff *skb,
 	first->bytecount = skb->len;
 	first->gso_segs = 1;
 
-	skb_tx_timestamp(skb);
-
 	/* record initial flags and protocol */
 	first->tx_flags = tx_flags;
 	first->protocol = protocol;
diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
index aeda1834e66a..cfb83687c3d8 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
@@ -8269,6 +8269,8 @@ static int ixgbe_tx_map(struct ixgbe_ring *tx_ring,
 	/* set the timestamp */
 	first->time_stamp = jiffies;
 
+	skb_tx_timestamp(skb);
+
 	/*
 	 * Force memory writes to complete before letting h/w know there
 	 * are new descriptors to fetch.  (Only applicable for weak-ordered
@@ -8646,8 +8648,6 @@ netdev_tx_t ixgbe_xmit_frame_ring(struct sk_buff *skb,
 		}
 	}
 
-	skb_tx_timestamp(skb);
-
 #ifdef CONFIG_PCI_IOV
 	/*
 	 * Use the l2switch_enable flag - would be false if the DMA
-- 
2.19.1

[net-next 06/12] i40e/ixgbe/igb: fail on new WoL flag setting WAKE_MAGICSECURE

[Jeff Kirsher]

From: Todd Fujinaka <todd.fujinaka@intel.com>

There's a new flag for setting WoL filters that is only
enabled on one manufacturer's NICs, and it's not ours. Fail
with EOPNOTSUPP.

Signed-off-by: Todd Fujinaka <todd.fujinaka@intel.com>
Tested-by: Andrew Bowers <andrewx.bowers@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
 drivers/net/ethernet/intel/i40e/i40e_ethtool.c   | 3 ++-
 drivers/net/ethernet/intel/igb/igb_ethtool.c     | 2 +-
 drivers/net/ethernet/intel/ixgbe/ixgbe_ethtool.c | 3 ++-
 3 files changed, 5 insertions(+), 3 deletions(-)

diff --git a/drivers/net/ethernet/intel/i40e/i40e_ethtool.c b/drivers/net/ethernet/intel/i40e/i40e_ethtool.c
index 9f8464f80783..9c1211ad2c6b 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_ethtool.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_ethtool.c
@@ -2377,7 +2377,8 @@ static int i40e_set_wol(struct net_device *netdev, struct ethtool_wolinfo *wol)
 		return -EOPNOTSUPP;
 
 	/* only magic packet is supported */
-	if (wol->wolopts && (wol->wolopts != WAKE_MAGIC))
+	if (wol->wolopts && (wol->wolopts != WAKE_MAGIC)
+			  | (wol->wolopts != WAKE_FILTER))
 		return -EOPNOTSUPP;
 
 	/* is this a new value? */
diff --git a/drivers/net/ethernet/intel/igb/igb_ethtool.c b/drivers/net/ethernet/intel/igb/igb_ethtool.c
index 5acf3b743876..c57671068245 100644
--- a/drivers/net/ethernet/intel/igb/igb_ethtool.c
+++ b/drivers/net/ethernet/intel/igb/igb_ethtool.c
@@ -2113,7 +2113,7 @@ static int igb_set_wol(struct net_device *netdev, struct ethtool_wolinfo *wol)
 {
 	struct igb_adapter *adapter = netdev_priv(netdev);
 
-	if (wol->wolopts & (WAKE_ARP | WAKE_MAGICSECURE))
+	if (wol->wolopts & (WAKE_ARP | WAKE_MAGICSECURE | WAKE_FILTER))
 		return -EOPNOTSUPP;
 
 	if (!(adapter->flags & IGB_FLAG_WOL_SUPPORTED))
diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_ethtool.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_ethtool.c
index 732b1e6ecc43..acba067cc15a 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_ethtool.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_ethtool.c
@@ -2206,7 +2206,8 @@ static int ixgbe_set_wol(struct net_device *netdev, struct ethtool_wolinfo *wol)
 {
 	struct ixgbe_adapter *adapter = netdev_priv(netdev);
 
-	if (wol->wolopts & (WAKE_PHY | WAKE_ARP | WAKE_MAGICSECURE))
+	if (wol->wolopts & (WAKE_PHY | WAKE_ARP | WAKE_MAGICSECURE |
+			    WAKE_FILTER))
 		return -EOPNOTSUPP;
 
 	if (ixgbe_wol_exclusion(adapter, wol))
-- 
2.19.1

[net-next 07/12] igc: Remove set but not used variables 'ctrl_ext, link_mode'

[Jeff Kirsher]

From: YueHaibing <yuehaibing@huawei.com>

Fixes gcc '-Wunused-but-set-variable' warning:

drivers/net/ethernet/intel/igc/igc_base.c: In function 'igc_init_phy_params_base':
drivers/net/ethernet/intel/igc/igc_base.c:240:6: warning:
 variable 'ctrl_ext' set but not used [-Wunused-but-set-variable]
  u32 ctrl_ext;

drivers/net/ethernet/intel/igc/igc_base.c: In function 'igc_get_invariants_base':
drivers/net/ethernet/intel/igc/igc_base.c:290:6: warning:
 variable 'link_mode' set but not used [-Wunused-but-set-variable]
  u32 link_mode = 0;

It never used since introduction in
commit c0071c7aa5fe ("igc: Add HW initialization code")

Signed-off-by: YueHaibing <yuehaibing@huawei.com>
Acked-by: Sasha Neftin <sasha.neftin@intel.com>
Tested-by: Aaron Brown <aaron.f.brown@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
 drivers/net/ethernet/intel/igc/igc_base.c | 8 --------
 1 file changed, 8 deletions(-)

diff --git a/drivers/net/ethernet/intel/igc/igc_base.c b/drivers/net/ethernet/intel/igc/igc_base.c
index 832da609d9a7..df40af759542 100644
--- a/drivers/net/ethernet/intel/igc/igc_base.c
+++ b/drivers/net/ethernet/intel/igc/igc_base.c
@@ -237,7 +237,6 @@ static s32 igc_init_phy_params_base(struct igc_hw *hw)
 {
 	struct igc_phy_info *phy = &hw->phy;
 	s32 ret_val = 0;
-	u32 ctrl_ext;
 
 	if (hw->phy.media_type != igc_media_type_copper) {
 		phy->type = igc_phy_none;
@@ -247,8 +246,6 @@ static s32 igc_init_phy_params_base(struct igc_hw *hw)
 	phy->autoneg_mask	= AUTONEG_ADVERTISE_SPEED_DEFAULT_2500;
 	phy->reset_delay_us	= 100;
 
-	ctrl_ext = rd32(IGC_CTRL_EXT);
-
 	/* set lan id */
 	hw->bus.func = (rd32(IGC_STATUS) & IGC_STATUS_FUNC_MASK) >>
 			IGC_STATUS_FUNC_SHIFT;
@@ -287,8 +284,6 @@ static s32 igc_init_phy_params_base(struct igc_hw *hw)
 static s32 igc_get_invariants_base(struct igc_hw *hw)
 {
 	struct igc_mac_info *mac = &hw->mac;
-	u32 link_mode = 0;
-	u32 ctrl_ext = 0;
 	s32 ret_val = 0;
 
 	switch (hw->device_id) {
@@ -302,9 +297,6 @@ static s32 igc_get_invariants_base(struct igc_hw *hw)
 
 	hw->phy.media_type = igc_media_type_copper;
 
-	ctrl_ext = rd32(IGC_CTRL_EXT);
-	link_mode = ctrl_ext & IGC_CTRL_EXT_LINK_MODE_MASK;
-
 	/* mac initialization and operations */
 	ret_val = igc_init_mac_params_base(hw);
 	if (ret_val)
-- 
2.19.1

[net-next 08/12] igc: Remove set but not used variable 'pci_using_dac'

[Jeff Kirsher]

From: YueHaibing <yuehaibing@huawei.com>

Fixes gcc '-Wunused-but-set-variable' warning:

drivers/net/ethernet/intel/igc/igc_main.c: In function 'igc_probe':
drivers/net/ethernet/intel/igc/igc_main.c:3535:11: warning:
 variable 'pci_using_dac' set but not used [-Wunused-but-set-variable]

It never used since introduction in commit
d89f88419f99 ("igc: Add skeletal frame for Intel(R) 2.5G Ethernet Controller support")

Signed-off-by: YueHaibing <yuehaibing@huawei.com>
Acked-by: Sasha Neftin <sasha.neftin@intel.com>
Tested-by: Aaron Brown <aaron.f.brown@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
 drivers/net/ethernet/intel/igc/igc_main.c | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/drivers/net/ethernet/intel/igc/igc_main.c b/drivers/net/ethernet/intel/igc/igc_main.c
index 615a5fcd5a00..529c3e952621 100644
--- a/drivers/net/ethernet/intel/igc/igc_main.c
+++ b/drivers/net/ethernet/intel/igc/igc_main.c
@@ -3532,19 +3532,16 @@ static int igc_probe(struct pci_dev *pdev,
 	struct net_device *netdev;
 	struct igc_hw *hw;
 	const struct igc_info *ei = igc_info_tbl[ent->driver_data];
-	int err, pci_using_dac;
+	int err;
 
 	err = pci_enable_device_mem(pdev);
 	if (err)
 		return err;
 
-	pci_using_dac = 0;
 	err = dma_set_mask(&pdev->dev, DMA_BIT_MASK(64));
 	if (!err) {
 		err = dma_set_coherent_mask(&pdev->dev,
 					    DMA_BIT_MASK(64));
-		if (!err)
-			pci_using_dac = 1;
 	} else {
 		err = dma_set_mask(&pdev->dev, DMA_BIT_MASK(32));
 		if (err) {
-- 
2.19.1

[net-next 09/12] igc: fix error return handling from call to netif_set_real_num_tx_queues

[Jeff Kirsher]

From: Colin Ian King <colin.king@canonical.com>

The call to netif_set_real_num_tx_queues is not assigning the error
return to variable err even though the next line checks err for an
error.  Fix this by adding the missing err assignment.

Detected by CoverityScan, CID#1474551 ("Logically dead code")

Fixes: 3df25e4c1e66 ("igc: Add interrupt support")
Signed-off-by: Colin Ian King <colin.king@canonical.com>
Acked-by: Sasha Neftin <sasha.neftin@intel.com>
Tested-by: Aaron Brown <aaron.f.brown@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
 drivers/net/ethernet/intel/igc/igc_main.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/intel/igc/igc_main.c b/drivers/net/ethernet/intel/igc/igc_main.c
index 529c3e952621..f176540f5ed7 100644
--- a/drivers/net/ethernet/intel/igc/igc_main.c
+++ b/drivers/net/ethernet/intel/igc/igc_main.c
@@ -3358,7 +3358,7 @@ static int __igc_open(struct net_device *netdev, bool resuming)
 		goto err_req_irq;
 
 	/* Notify the stack of the actual queue counts. */
-	netif_set_real_num_tx_queues(netdev, adapter->num_tx_queues);
+	err = netif_set_real_num_tx_queues(netdev, adapter->num_tx_queues);
 	if (err)
 		goto err_set_queues;
 
-- 
2.19.1

[net-next 10/12] igc: Tidy up some white space

[Jeff Kirsher]

From: Dan Carpenter <dan.carpenter@oracle.com>

I just cleaned up a couple small white space issues.

Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
Acked-by: Sasha Neftin <sasha.neftin@intel.com>
Tested-by: Aaron Brown <aaron.f.brown@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
 drivers/net/ethernet/intel/igc/igc_main.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/net/ethernet/intel/igc/igc_main.c b/drivers/net/ethernet/intel/igc/igc_main.c
index f176540f5ed7..827eda044d97 100644
--- a/drivers/net/ethernet/intel/igc/igc_main.c
+++ b/drivers/net/ethernet/intel/igc/igc_main.c
@@ -1108,7 +1108,7 @@ static struct sk_buff *igc_build_skb(struct igc_ring *rx_ring,
 
 	/* update pointers within the skb to store the data */
 	skb_reserve(skb, IGC_SKB_PAD);
-	 __skb_put(skb, size);
+	__skb_put(skb, size);
 
 	/* update buffer offset */
 #if (PAGE_SIZE < 8192)
@@ -1160,9 +1160,9 @@ static struct sk_buff *igc_construct_skb(struct igc_ring *rx_ring,
 				(va + headlen) - page_address(rx_buffer->page),
 				size, truesize);
 #if (PAGE_SIZE < 8192)
-	rx_buffer->page_offset ^= truesize;
+		rx_buffer->page_offset ^= truesize;
 #else
-	rx_buffer->page_offset += truesize;
+		rx_buffer->page_offset += truesize;
 #endif
 	} else {
 		rx_buffer->pagecnt_bias++;
@@ -1668,8 +1668,8 @@ static bool igc_clean_tx_irq(struct igc_q_vector *q_vector, int napi_budget)
 				tx_buffer->next_to_watch,
 				jiffies,
 				tx_buffer->next_to_watch->wb.status);
-				netif_stop_subqueue(tx_ring->netdev,
-						    tx_ring->queue_index);
+			netif_stop_subqueue(tx_ring->netdev,
+					    tx_ring->queue_index);
 
 			/* we are about to reset, no point in enabling stuff */
 			return true;
-- 
2.19.1

[net-next 11/12] e1000e: allow non-monotonic SYSTIM readings

[Jeff Kirsher]

From: Miroslav Lichvar <mlichvar@redhat.com>

It seems with some NICs supported by the e1000e driver a SYSTIM reading
may occasionally be few microseconds before the previous reading and if
enabled also pass e1000e_sanitize_systim() without reaching the maximum
number of rereads, even if the function is modified to check three
consecutive readings (i.e. it doesn't look like a double read error).
This causes an underflow in the timecounter and the PHC time jumps hours
ahead.

This was observed on 82574, I217 and I219. The fastest way to reproduce
it is to run a program that continuously calls the PTP_SYS_OFFSET ioctl
on the PHC.

Modify e1000e_phc_gettime() to use timecounter_cyc2time() instead of
timecounter_read() in order to allow non-monotonic SYSTIM readings and
prevent the PHC from jumping.

Cc: Richard Cochran <richardcochran@gmail.com>
Signed-off-by: Miroslav Lichvar <mlichvar@redhat.com>
Acked-by: Jacob Keller <jacob.e.keller@intel.com>
Tested-by: Aaron Brown <aaron.f.brown@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
 drivers/net/ethernet/intel/e1000e/ptp.c | 13 ++++++++++---
 1 file changed, 10 insertions(+), 3 deletions(-)

diff --git a/drivers/net/ethernet/intel/e1000e/ptp.c b/drivers/net/ethernet/intel/e1000e/ptp.c
index 37c76945ad9b..e1f821edbc21 100644
--- a/drivers/net/ethernet/intel/e1000e/ptp.c
+++ b/drivers/net/ethernet/intel/e1000e/ptp.c
@@ -173,10 +173,14 @@ static int e1000e_phc_gettime(struct ptp_clock_info *ptp, struct timespec64 *ts)
 	struct e1000_adapter *adapter = container_of(ptp, struct e1000_adapter,
 						     ptp_clock_info);
 	unsigned long flags;
-	u64 ns;
+	u64 cycles, ns;
 
 	spin_lock_irqsave(&adapter->systim_lock, flags);
-	ns = timecounter_read(&adapter->tc);
+
+	/* Use timecounter_cyc2time() to allow non-monotonic SYSTIM readings */
+	cycles = adapter->cc.read(&adapter->cc);
+	ns = timecounter_cyc2time(&adapter->tc, cycles);
+
 	spin_unlock_irqrestore(&adapter->systim_lock, flags);
 
 	*ts = ns_to_timespec64(ns);
@@ -232,9 +236,12 @@ static void e1000e_systim_overflow_work(struct work_struct *work)
 						     systim_overflow_work.work);
 	struct e1000_hw *hw = &adapter->hw;
 	struct timespec64 ts;
+	u64 ns;
 
-	adapter->ptp_clock_info.gettime64(&adapter->ptp_clock_info, &ts);
+	/* Update the timecounter */
+	ns = timecounter_read(&adapter->tc);
 
+	ts = ns_to_timespec64(ns);
 	e_dbg("SYSTIM overflow check at %lld.%09lu\n",
 	      (long long) ts.tv_sec, ts.tv_nsec);
 
-- 
2.19.1

[net-next 12/12] igc: Clean up code

[Jeff Kirsher]

From: Sasha Neftin <sasha.neftin@intel.com>

Address few community comments.
Remove unused code, will be added per demand.
Remove blank lines and unneeded includes.

Signed-off-by: Sasha Neftin <sasha.neftin@intel.com>
Tested-by: Aaron Brown <aaron.f.brown@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
 drivers/net/ethernet/intel/igc/igc.h      |  9 ---------
 drivers/net/ethernet/intel/igc/igc_main.c | 15 ---------------
 2 files changed, 24 deletions(-)

diff --git a/drivers/net/ethernet/intel/igc/igc.h b/drivers/net/ethernet/intel/igc/igc.h
index cdf18a5d9e08..3b00b109b34a 100644
--- a/drivers/net/ethernet/intel/igc/igc.h
+++ b/drivers/net/ethernet/intel/igc/igc.h
@@ -5,23 +5,14 @@
 #define _IGC_H_
 
 #include <linux/kobject.h>
-
 #include <linux/pci.h>
 #include <linux/netdevice.h>
 #include <linux/vmalloc.h>
-
 #include <linux/ethtool.h>
-
 #include <linux/sctp.h>
 
 #define IGC_ERR(args...) pr_err("igc: " args)
 
-#define PFX "igc: "
-
-#include <linux/timecounter.h>
-#include <linux/net_tstamp.h>
-#include <linux/ptp_clock_kernel.h>
-
 #include "igc_hw.h"
 
 /* main */
diff --git a/drivers/net/ethernet/intel/igc/igc_main.c b/drivers/net/ethernet/intel/igc/igc_main.c
index 827eda044d97..d002055c0623 100644
--- a/drivers/net/ethernet/intel/igc/igc_main.c
+++ b/drivers/net/ethernet/intel/igc/igc_main.c
@@ -1699,20 +1699,6 @@ static bool igc_clean_tx_irq(struct igc_q_vector *q_vector, int napi_budget)
 	return !!budget;
 }
 
-/**
- * igc_ioctl - I/O control method
- * @netdev: network interface device structure
- * @ifreq: frequency
- * @cmd: command
- */
-static int igc_ioctl(struct net_device *netdev, struct ifreq *ifr, int cmd)
-{
-	switch (cmd) {
-	default:
-		return -EOPNOTSUPP;
-	}
-}
-
 /**
  * igc_up - Open the interface and prepare it to handle traffic
  * @adapter: board private structure
@@ -3445,7 +3431,6 @@ static const struct net_device_ops igc_netdev_ops = {
 	.ndo_set_mac_address	= igc_set_mac,
 	.ndo_change_mtu		= igc_change_mtu,
 	.ndo_get_stats		= igc_get_stats,
-	.ndo_do_ioctl		= igc_ioctl,
 };
 
 /* PCIe configuration access */
-- 
2.19.1

Re: [net-next 06/12] i40e/ixgbe/igb: fail on new WoL flag setting WAKE_MAGICSECURE

[Kevin Easton]

On Wed, Nov 07, 2018 at 02:48:24PM -0800, Jeff Kirsher wrote:
> From: Todd Fujinaka <todd.fujinaka@intel.com>
> 
> There's a new flag for setting WoL filters that is only
> enabled on one manufacturer's NICs, and it's not ours. Fail
> with EOPNOTSUPP.
> 
> Signed-off-by: Todd Fujinaka <todd.fujinaka@intel.com>
> Tested-by: Andrew Bowers <andrewx.bowers@intel.com>
> Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
> ---
>  drivers/net/ethernet/intel/i40e/i40e_ethtool.c   | 3 ++-
>  drivers/net/ethernet/intel/igb/igb_ethtool.c     | 2 +-
>  drivers/net/ethernet/intel/ixgbe/ixgbe_ethtool.c | 3 ++-
>  3 files changed, 5 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/net/ethernet/intel/i40e/i40e_ethtool.c b/drivers/net/ethernet/intel/i40e/i40e_ethtool.c
> index 9f8464f80783..9c1211ad2c6b 100644
> --- a/drivers/net/ethernet/intel/i40e/i40e_ethtool.c
> +++ b/drivers/net/ethernet/intel/i40e/i40e_ethtool.c
> @@ -2377,7 +2377,8 @@ static int i40e_set_wol(struct net_device *netdev, struct ethtool_wolinfo *wol)
>  		return -EOPNOTSUPP;
>  
>  	/* only magic packet is supported */
> -	if (wol->wolopts && (wol->wolopts != WAKE_MAGIC))
> +	if (wol->wolopts && (wol->wolopts != WAKE_MAGIC)
> +			  | (wol->wolopts != WAKE_FILTER))
>  		return -EOPNOTSUPP;

This doesn't look right.  WAKE_MAGIC and WAKE_FILTER are distinct, so

(wol->wolopts != WAKE_MAGIC) | (wol->wolopts != WAKE_FILTER)

will always be 1.

It looks like the existing test in this driver was fine - it *only*
accepted wol->wolopts of either 0 or WAKE_MAGIC, it was already
rejecting everything else including WAKE_FILTER.

Suggest you drop that hunk.

    - Kevin

>
>  
>  	/* is this a new value? */
> diff --git a/drivers/net/ethernet/intel/igb/igb_ethtool.c b/drivers/net/ethernet/intel/igb/igb_ethtool.c
> index 5acf3b743876..c57671068245 100644
> --- a/drivers/net/ethernet/intel/igb/igb_ethtool.c
> +++ b/drivers/net/ethernet/intel/igb/igb_ethtool.c
> @@ -2113,7 +2113,7 @@ static int igb_set_wol(struct net_device *netdev, struct ethtool_wolinfo *wol)
>  {
>  	struct igb_adapter *adapter = netdev_priv(netdev);
>  
> -	if (wol->wolopts & (WAKE_ARP | WAKE_MAGICSECURE))
> +	if (wol->wolopts & (WAKE_ARP | WAKE_MAGICSECURE | WAKE_FILTER))
>  		return -EOPNOTSUPP;
>  
>  	if (!(adapter->flags & IGB_FLAG_WOL_SUPPORTED))
> diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_ethtool.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_ethtool.c
> index 732b1e6ecc43..acba067cc15a 100644
> --- a/drivers/net/ethernet/intel/ixgbe/ixgbe_ethtool.c
> +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_ethtool.c
> @@ -2206,7 +2206,8 @@ static int ixgbe_set_wol(struct net_device *netdev, struct ethtool_wolinfo *wol)
>  {
>  	struct ixgbe_adapter *adapter = netdev_priv(netdev);
>  
> -	if (wol->wolopts & (WAKE_PHY | WAKE_ARP | WAKE_MAGICSECURE))
> +	if (wol->wolopts & (WAKE_PHY | WAKE_ARP | WAKE_MAGICSECURE |
> +			    WAKE_FILTER))
>  		return -EOPNOTSUPP;
>  
>  	if (ixgbe_wol_exclusion(adapter, wol))
> -- 
> 2.19.1
> 
> 
> 

Re: [net-next 06/12] i40e/ixgbe/igb: fail on new WoL flag setting WAKE_MAGICSECURE

[Michal Kubecek]

On Thu, Nov 08, 2018 at 06:05:26AM +0000, Kevin Easton wrote:
> On Wed, Nov 07, 2018 at 02:48:24PM -0800, Jeff Kirsher wrote:
> > From: Todd Fujinaka <todd.fujinaka@intel.com>
> > 
> > There's a new flag for setting WoL filters that is only
> > enabled on one manufacturer's NICs, and it's not ours. Fail
> > with EOPNOTSUPP.
> > 
> > Signed-off-by: Todd Fujinaka <todd.fujinaka@intel.com>
> > Tested-by: Andrew Bowers <andrewx.bowers@intel.com>
> > Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
> > ---
> >  drivers/net/ethernet/intel/i40e/i40e_ethtool.c   | 3 ++-
> >  drivers/net/ethernet/intel/igb/igb_ethtool.c     | 2 +-
> >  drivers/net/ethernet/intel/ixgbe/ixgbe_ethtool.c | 3 ++-
> >  3 files changed, 5 insertions(+), 3 deletions(-)
> > 
> > diff --git a/drivers/net/ethernet/intel/i40e/i40e_ethtool.c b/drivers/net/ethernet/intel/i40e/i40e_ethtool.c
> > index 9f8464f80783..9c1211ad2c6b 100644
> > --- a/drivers/net/ethernet/intel/i40e/i40e_ethtool.c
> > +++ b/drivers/net/ethernet/intel/i40e/i40e_ethtool.c
> > @@ -2377,7 +2377,8 @@ static int i40e_set_wol(struct net_device *netdev, struct ethtool_wolinfo *wol)
> >  		return -EOPNOTSUPP;
> >  
> >  	/* only magic packet is supported */
> > -	if (wol->wolopts && (wol->wolopts != WAKE_MAGIC))
> > +	if (wol->wolopts && (wol->wolopts != WAKE_MAGIC)
> > +			  | (wol->wolopts != WAKE_FILTER))
> >  		return -EOPNOTSUPP;
> 
> This doesn't look right.  WAKE_MAGIC and WAKE_FILTER are distinct, so
> 
> (wol->wolopts != WAKE_MAGIC) | (wol->wolopts != WAKE_FILTER)
> 
> will always be 1.

Right. Also, using "|" with logical values is rather confusing. While
the result works as expected, its priority is higher than priority of
&& (which would not be true for ||), making the code counterintuitive.

BtW, the patch subject is also wrong, the newly added flag it is dealing
with is WAKE_FILTER, not WAKE_MAGICSECURE.

> It looks like the existing test in this driver was fine - it *only*
> accepted wol->wolopts of either 0 or WAKE_MAGIC, it was already
> rejecting everything else including WAKE_FILTER.

Another way to write the check would be

	if (wol->wolopts & ~WAKE_MAGIC)

> > diff --git a/drivers/net/ethernet/intel/igb/igb_ethtool.c b/drivers/net/ethernet/intel/igb/igb_ethtool.c
> > index 5acf3b743876..c57671068245 100644
> > --- a/drivers/net/ethernet/intel/igb/igb_ethtool.c
> > +++ b/drivers/net/ethernet/intel/igb/igb_ethtool.c
> > @@ -2113,7 +2113,7 @@ static int igb_set_wol(struct net_device *netdev, struct ethtool_wolinfo *wol)
> >  {
> >  	struct igb_adapter *adapter = netdev_priv(netdev);
> >  
> > -	if (wol->wolopts & (WAKE_ARP | WAKE_MAGICSECURE))
> > +	if (wol->wolopts & (WAKE_ARP | WAKE_MAGICSECURE | WAKE_FILTER))
> >  		return -EOPNOTSUPP;
> >  
> >  	if (!(adapter->flags & IGB_FLAG_WOL_SUPPORTED))

I would also suggest taking the opposite approach here, i.e. listing the
flags which _are_ supported so that we don't have to update the code if
another wol flag is added (or userspace sends an invalid one):

#define SUPPORTED_WOL_MODES \
	(WAKE_PHY | WAKE_UCAST | WAKE_MCAST | WAKE_BCAST | WAKE_MAGIC)
...
	if (wol->wolopts & ~SUPPORTED_WOL_MODES)
		return -EOPNOTSUPP;


> > diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_ethtool.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_ethtool.c
> > index 732b1e6ecc43..acba067cc15a 100644
> > --- a/drivers/net/ethernet/intel/ixgbe/ixgbe_ethtool.c
> > +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_ethtool.c
> > @@ -2206,7 +2206,8 @@ static int ixgbe_set_wol(struct net_device *netdev, struct ethtool_wolinfo *wol)
> >  {
> >  	struct ixgbe_adapter *adapter = netdev_priv(netdev);
> >  
> > -	if (wol->wolopts & (WAKE_PHY | WAKE_ARP | WAKE_MAGICSECURE))
> > +	if (wol->wolopts & (WAKE_PHY | WAKE_ARP | WAKE_MAGICSECURE |
> > +			    WAKE_FILTER))
> >  		return -EOPNOTSUPP;
> >  
> >  	if (ixgbe_wol_exclusion(adapter, wol))

...and here.

Michal Kubecek

Re: [net-next 00/12][pull request] Intel Wired LAN Driver Updates 2018-11-07

[David Miller]

From: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
Date: Wed,  7 Nov 2018 14:48:18 -0800

> This series contains updates to almost all of the Intel wired LAN
> drivers.
 ...
> The following are changes since commit 7c588c7468ea3f9b2fc8fa6840bed6262b5d1b00:
>   Merge branch 'net-systemport-Unmap-queues-upon-DSA-unregister-event'
> and are available in the git repository at:
>   git://git.kernel.org/pub/scm/linux/kernel/git/jkirsher/next-queue 1GbE

Pulled, thanks Jeff.

Re: [net-next 12/12] igc: Clean up code

[Joe Perches]

On Wed, 2018-11-07 at 14:48 -0800, Jeff Kirsher wrote:
> From: Sasha Neftin <sasha.neftin@intel.com>
> 
> Address few community comments.
> Remove unused code, will be added per demand.
> Remove blank lines and unneeded includes.
> 
> Signed-off-by: Sasha Neftin <sasha.neftin@intel.com>
> Tested-by: Aaron Brown <aaron.f.brown@intel.com>
> Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
> ---
>  drivers/net/ethernet/intel/igc/igc.h      |  9 ---------
>  drivers/net/ethernet/intel/igc/igc_main.c | 15 ---------------
>  2 files changed, 24 deletions(-)
> 
> diff --git a/drivers/net/ethernet/intel/igc/igc.h b/drivers/net/ethernet/intel/igc/igc.h
[]
>  #define IGC_ERR(args...) pr_err("igc: " args)

This is used just once and should probably be removed.

maybe:

---
 drivers/net/ethernet/intel/igc/igc_main.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/intel/igc/igc_main.c b/drivers/net/ethernet/intel/igc/igc_main.c
index 9d85707e8a81..b58542b20623 100644
--- a/drivers/net/ethernet/intel/igc/igc_main.c
+++ b/drivers/net/ethernet/intel/igc/igc_main.c
@@ -3551,7 +3551,7 @@ static int igc_probe(struct pci_dev *pdev,
 			err = dma_set_coherent_mask(&pdev->dev,
 						    DMA_BIT_MASK(32));
 			if (err) {
-				IGC_ERR("Wrong DMA configuration, aborting\n");
+				dev_err(&pdev->dev, "igc: Wrong DMA configuration, aborting\n");
 				goto err_dma;
 			}
 		}

Re: [net-next 12/12] igc: Clean up code

[Neftin, Sasha]

On 11/8/2018 13:00, Joe Perches wrote:
> On Wed, 2018-11-07 at 14:48 -0800, Jeff Kirsher wrote:
>> From: Sasha Neftin <sasha.neftin@intel.com>
>>
>> Address few community comments.
>> Remove unused code, will be added per demand.
>> Remove blank lines and unneeded includes.
>>
>> Signed-off-by: Sasha Neftin <sasha.neftin@intel.com>
>> Tested-by: Aaron Brown <aaron.f.brown@intel.com>
>> Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
>> ---
>>   drivers/net/ethernet/intel/igc/igc.h      |  9 ---------
>>   drivers/net/ethernet/intel/igc/igc_main.c | 15 ---------------
>>   2 files changed, 24 deletions(-)
>>
>> diff --git a/drivers/net/ethernet/intel/igc/igc.h b/drivers/net/ethernet/intel/igc/igc.h
> []
>>   #define IGC_ERR(args...) pr_err("igc: " args)
> 
> This is used just once and should probably be removed.
> 
> maybe:
> 
> ---
>   drivers/net/ethernet/intel/igc/igc_main.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/net/ethernet/intel/igc/igc_main.c b/drivers/net/ethernet/intel/igc/igc_main.c
> index 9d85707e8a81..b58542b20623 100644
> --- a/drivers/net/ethernet/intel/igc/igc_main.c
> +++ b/drivers/net/ethernet/intel/igc/igc_main.c
> @@ -3551,7 +3551,7 @@ static int igc_probe(struct pci_dev *pdev,
>   			err = dma_set_coherent_mask(&pdev->dev,
>   						    DMA_BIT_MASK(32));
>   			if (err) {
> -				IGC_ERR("Wrong DMA configuration, aborting\n");
> +				dev_err(&pdev->dev, "igc: Wrong DMA configuration, aborting\n");
>   				goto err_dma;
>   			}
>   		}
> 
> 
Good note, thanks. I will process another patch address this suggestion.

Re: [net-next 06/12] i40e/ixgbe/igb: fail on new WoL flag setting WAKE_MAGICSECURE

[Jeff Kirsher]

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

On Thu, 2018-11-08 at 07:42 +0100, Michal Kubecek wrote:
> On Thu, Nov 08, 2018 at 06:05:26AM +0000, Kevin Easton wrote:
> > On Wed, Nov 07, 2018 at 02:48:24PM -0800, Jeff Kirsher wrote:
> > > From: Todd Fujinaka <todd.fujinaka@intel.com>
> > > 
> > > There's a new flag for setting WoL filters that is only
> > > enabled on one manufacturer's NICs, and it's not ours. Fail
> > > with EOPNOTSUPP.
> > > 
> > > Signed-off-by: Todd Fujinaka <todd.fujinaka@intel.com>
> > > Tested-by: Andrew Bowers <andrewx.bowers@intel.com>
> > > Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
> > > ---
> > >  drivers/net/ethernet/intel/i40e/i40e_ethtool.c   | 3 ++-
> > >  drivers/net/ethernet/intel/igb/igb_ethtool.c     | 2 +-
> > >  drivers/net/ethernet/intel/ixgbe/ixgbe_ethtool.c | 3 ++-
> > >  3 files changed, 5 insertions(+), 3 deletions(-)
> > > 
> > > diff --git a/drivers/net/ethernet/intel/i40e/i40e_ethtool.c
> > > b/drivers/net/ethernet/intel/i40e/i40e_ethtool.c
> > > index 9f8464f80783..9c1211ad2c6b 100644
> > > --- a/drivers/net/ethernet/intel/i40e/i40e_ethtool.c
> > > +++ b/drivers/net/ethernet/intel/i40e/i40e_ethtool.c
> > > @@ -2377,7 +2377,8 @@ static int i40e_set_wol(struct net_device
> > > *netdev, struct ethtool_wolinfo *wol)
> > >  		return -EOPNOTSUPP;
> > >  
> > >  	/* only magic packet is supported */
> > > -	if (wol->wolopts && (wol->wolopts != WAKE_MAGIC))
> > > +	if (wol->wolopts && (wol->wolopts != WAKE_MAGIC)
> > > +			  | (wol->wolopts != WAKE_FILTER))
> > >  		return -EOPNOTSUPP;
> > 
> > This doesn't look right.  WAKE_MAGIC and WAKE_FILTER are distinct, so
> > 
> > (wol->wolopts != WAKE_MAGIC) | (wol->wolopts != WAKE_FILTER)
> > 
> > will always be 1.
> 
> Right. Also, using "|" with logical values is rather confusing. While
> the result works as expected, its priority is higher than priority of
> && (which would not be true for ||), making the code counterintuitive.
> 
> BtW, the patch subject is also wrong, the newly added flag it is dealing
> with is WAKE_FILTER, not WAKE_MAGICSECURE.

After looking into this this more, this does appear to be incorrect.  The
author of this change is currently out on vacation, so once he gets back, I
will address both Kevin's and your concerns.

> 
> > It looks like the existing test in this driver was fine - it *only*
> > accepted wol->wolopts of either 0 or WAKE_MAGIC, it was already
> > rejecting everything else including WAKE_FILTER.
> 
> Another way to write the check would be
> 
> 	if (wol->wolopts & ~WAKE_MAGIC)
> 
> > > diff --git a/drivers/net/ethernet/intel/igb/igb_ethtool.c
> > > b/drivers/net/ethernet/intel/igb/igb_ethtool.c
> > > index 5acf3b743876..c57671068245 100644
> > > --- a/drivers/net/ethernet/intel/igb/igb_ethtool.c
> > > +++ b/drivers/net/ethernet/intel/igb/igb_ethtool.c
> > > @@ -2113,7 +2113,7 @@ static int igb_set_wol(struct net_device
> > > *netdev, struct ethtool_wolinfo *wol)
> > >  {
> > >  	struct igb_adapter *adapter = netdev_priv(netdev);
> > >  
> > > -	if (wol->wolopts & (WAKE_ARP | WAKE_MAGICSECURE))
> > > +	if (wol->wolopts & (WAKE_ARP | WAKE_MAGICSECURE | WAKE_FILTER))
> > >  		return -EOPNOTSUPP;
> > >  
> > >  	if (!(adapter->flags & IGB_FLAG_WOL_SUPPORTED))
> 
> I would also suggest taking the opposite approach here, i.e. listing the
> flags which _are_ supported so that we don't have to update the code if
> another wol flag is added (or userspace sends an invalid one):
> 
> #define SUPPORTED_WOL_MODES \
> 	(WAKE_PHY | WAKE_UCAST | WAKE_MCAST | WAKE_BCAST | WAKE_MAGIC)
> ...
> 	if (wol->wolopts & ~SUPPORTED_WOL_MODES)
> 		return -EOPNOTSUPP;
> 
> 
> > > diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_ethtool.c
> > > b/drivers/net/ethernet/intel/ixgbe/ixgbe_ethtool.c
> > > index 732b1e6ecc43..acba067cc15a 100644
> > > --- a/drivers/net/ethernet/intel/ixgbe/ixgbe_ethtool.c
> > > +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_ethtool.c
> > > @@ -2206,7 +2206,8 @@ static int ixgbe_set_wol(struct net_device
> > > *netdev, struct ethtool_wolinfo *wol)
> > >  {
> > >  	struct ixgbe_adapter *adapter = netdev_priv(netdev);
> > >  
> > > -	if (wol->wolopts & (WAKE_PHY | WAKE_ARP | WAKE_MAGICSECURE))
> > > +	if (wol->wolopts & (WAKE_PHY | WAKE_ARP | WAKE_MAGICSECURE |
> > > +			    WAKE_FILTER))
> > >  		return -EOPNOTSUPP;
> > >  
> > >  	if (ixgbe_wol_exclusion(adapter, wol))
> 
> ...and here.
> 
> Michal Kubecek


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