* [net-next 1/7] fm10k: correctly check if interface is removed
2017-04-06 7:54 [net-next 0/7][pull request] 100GbE Intel Wired LAN Driver Updates 2017-04-05 Jeff Kirsher
@ 2017-04-06 7:54 ` Jeff Kirsher
2017-04-06 7:54 ` [net-next 2/7] fm10k: use a BITMAP for flags to avoid race conditions Jeff Kirsher
` (6 subsequent siblings)
7 siblings, 0 replies; 9+ messages in thread
From: Jeff Kirsher @ 2017-04-06 7:54 UTC (permalink / raw)
To: davem; +Cc: Phil Turnbull, netdev, nhorman, sassmann, jogreene, Jeff Kirsher
From: Phil Turnbull <phil.turnbull@oracle.com>
FM10K_REMOVED expects a hardware address, not a 'struct fm10k_hw'.
Fixes: 5cb8db4a4cbc ("fm10k: Add support for VF")
Signed-off-by: Phil Turnbull <phil.turnbull@oracle.com>
Tested-by: Krishneil Singh <krishneil.k.singh@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
drivers/net/ethernet/intel/fm10k/fm10k_ethtool.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/intel/fm10k/fm10k_ethtool.c b/drivers/net/ethernet/intel/fm10k/fm10k_ethtool.c
index 0c84fef750f4..078e8d93db5a 100644
--- a/drivers/net/ethernet/intel/fm10k/fm10k_ethtool.c
+++ b/drivers/net/ethernet/intel/fm10k/fm10k_ethtool.c
@@ -939,7 +939,7 @@ static void fm10k_self_test(struct net_device *dev,
memset(data, 0, sizeof(*data) * FM10K_TEST_LEN);
- if (FM10K_REMOVED(hw)) {
+ if (FM10K_REMOVED(hw->hw_addr)) {
netif_err(interface, drv, dev,
"Interface removed - test blocked\n");
eth_test->flags |= ETH_TEST_FL_FAILED;
--
2.12.2
^ permalink raw reply related [flat|nested] 9+ messages in thread* [net-next 2/7] fm10k: use a BITMAP for flags to avoid race conditions
2017-04-06 7:54 [net-next 0/7][pull request] 100GbE Intel Wired LAN Driver Updates 2017-04-05 Jeff Kirsher
2017-04-06 7:54 ` [net-next 1/7] fm10k: correctly check if interface is removed Jeff Kirsher
@ 2017-04-06 7:54 ` Jeff Kirsher
2017-04-06 7:54 ` [net-next 3/7] fm10k: future-proof state bitmaps using DECLARE_BITMAP Jeff Kirsher
` (5 subsequent siblings)
7 siblings, 0 replies; 9+ messages in thread
From: Jeff Kirsher @ 2017-04-06 7:54 UTC (permalink / raw)
To: davem; +Cc: Jacob Keller, netdev, nhorman, sassmann, jogreene, Jeff Kirsher
From: Jacob Keller <jacob.e.keller@intel.com>
Replace bitwise operators and #defines with a BITMAP and enumeration
values. This is similar to how we handle the "state" values as well.
This has two distinct advantages over the old method. First, we ensure
correctness of operations which are currently problematic due to race
conditions. Suppose that two kernel threads are running, such as the
watchdog and an ethtool ioctl, and both modify flags. We'll say that the
watchdog is CPU A, and the ethtool ioctl is CPU B.
CPU A sets FLAG_1, which can be seen as
CPU A read FLAGS
CPU A write FLAGS | FLAG_1
CPU B sets FLAG_2, which can be seen as
CPU B read FLAGS
CPU A write FLAGS | FLAG_2
However, "|=" and "&=" operators are not actually atomic. So this could
be ordered like the following:
CPU A read FLAGS -> variable
CPU B read FLAGS -> variable
CPU A write FLAGS (variable | FLAG_1)
CPU B write FLAGS (variable | FLAG_2)
Notice how the 2nd write from CPU B could actually undo the write from
CPU A because it isn't guaranteed that the |= operation is atomic.
In practice the race windows for most flag writes is incredibly narrow
so it is not easy to isolate issues. However, the more flags we have,
the more likely they will cause problems. Additionally, if such
a problem were to arise, it would be incredibly difficult to track down.
Second, there is an additional advantage beyond code correctness. We can
now automatically size the BITMAP if more flags were added, so that we
do not need to remember that flags is u32 and thus if we added too many
flags we would over-run the variable. This is not a likely occurrence
for fm10k driver, but this patch can serve as an example for other
drivers which have many more flags.
This particular change does have a bit of trouble converting some of the
idioms previously used with the #defines for flags. Specifically, when
converting FM10K_FLAG_RSS_FIELD_IPV[46]_UDP flags. This whole operation
was actually quite problematic, because we actually stored flags
separately. This could more easily show the problem of the above
re-ordering issue.
This is really difficult to test whether atomics make a difference in
practical scenarios, but you can ensure that basic functionality remains
the same. This patch has a lot of code coverage, but most of it is
relatively simple.
While we are modifying these files, update their copyright year.
Signed-off-by: Jacob Keller <jacob.e.keller@intel.com>
Tested-by: Krishneil Singh <krishneil.k.singh@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
drivers/net/ethernet/intel/fm10k/fm10k.h | 27 ++++++++---
drivers/net/ethernet/intel/fm10k/fm10k_ethtool.c | 62 ++++++++++++++++--------
drivers/net/ethernet/intel/fm10k/fm10k_main.c | 6 +--
drivers/net/ethernet/intel/fm10k/fm10k_netdev.c | 4 +-
drivers/net/ethernet/intel/fm10k/fm10k_pci.c | 31 ++++++------
5 files changed, 82 insertions(+), 48 deletions(-)
diff --git a/drivers/net/ethernet/intel/fm10k/fm10k.h b/drivers/net/ethernet/intel/fm10k/fm10k.h
index 52b979443cde..d6db1c48d4dc 100644
--- a/drivers/net/ethernet/intel/fm10k/fm10k.h
+++ b/drivers/net/ethernet/intel/fm10k/fm10k.h
@@ -1,5 +1,5 @@
/* Intel(R) Ethernet Switch Host Interface Driver
- * Copyright(c) 2013 - 2016 Intel Corporation.
+ * Copyright(c) 2013 - 2017 Intel Corporation.
*
* This program is free software; you can redistribute it and/or modify it
* under the terms and conditions of the GNU General Public License,
@@ -249,6 +249,23 @@ struct fm10k_udp_port {
/* one work queue for entire driver */
extern struct workqueue_struct *fm10k_workqueue;
+/* The following enumeration contains flags which indicate or enable modified
+ * driver behaviors. To avoid race conditions, the flags are stored in
+ * a BITMAP in the fm10k_intfc structure. The BITMAP should be accessed using
+ * atomic *_bit() operations.
+ */
+enum fm10k_flags_t {
+ FM10K_FLAG_RESET_REQUESTED,
+ FM10K_FLAG_RSS_FIELD_IPV4_UDP,
+ FM10K_FLAG_RSS_FIELD_IPV6_UDP,
+ FM10K_FLAG_SWPRI_CONFIG,
+ /* __FM10K_FLAGS_SIZE__ is used to calculate the size of
+ * interface->flags and must be the last value in this
+ * enumeration.
+ */
+ __FM10K_FLAGS_SIZE__
+};
+
struct fm10k_intfc {
unsigned long active_vlans[BITS_TO_LONGS(VLAN_N_VID)];
struct net_device *netdev;
@@ -256,11 +273,9 @@ struct fm10k_intfc {
struct pci_dev *pdev;
unsigned long state;
- u32 flags;
-#define FM10K_FLAG_RESET_REQUESTED (u32)(BIT(0))
-#define FM10K_FLAG_RSS_FIELD_IPV4_UDP (u32)(BIT(1))
-#define FM10K_FLAG_RSS_FIELD_IPV6_UDP (u32)(BIT(2))
-#define FM10K_FLAG_SWPRI_CONFIG (u32)(BIT(3))
+ /* Access flag values using atomic *_bit() operations */
+ DECLARE_BITMAP(flags, __FM10K_FLAGS_SIZE__);
+
int xcast_mode;
/* Tx fast path data */
diff --git a/drivers/net/ethernet/intel/fm10k/fm10k_ethtool.c b/drivers/net/ethernet/intel/fm10k/fm10k_ethtool.c
index 078e8d93db5a..5269f3ed4682 100644
--- a/drivers/net/ethernet/intel/fm10k/fm10k_ethtool.c
+++ b/drivers/net/ethernet/intel/fm10k/fm10k_ethtool.c
@@ -1,5 +1,5 @@
/* Intel(R) Ethernet Switch Host Interface Driver
- * Copyright(c) 2013 - 2016 Intel Corporation.
+ * Copyright(c) 2013 - 2017 Intel Corporation.
*
* This program is free software; you can redistribute it and/or modify it
* under the terms and conditions of the GNU General Public License,
@@ -716,7 +716,8 @@ static int fm10k_get_rss_hash_opts(struct fm10k_intfc *interface,
cmd->data |= RXH_L4_B_0_1 | RXH_L4_B_2_3;
/* fall through */
case UDP_V4_FLOW:
- if (interface->flags & FM10K_FLAG_RSS_FIELD_IPV4_UDP)
+ if (test_bit(FM10K_FLAG_RSS_FIELD_IPV4_UDP,
+ interface->flags))
cmd->data |= RXH_L4_B_0_1 | RXH_L4_B_2_3;
/* fall through */
case SCTP_V4_FLOW:
@@ -732,7 +733,8 @@ static int fm10k_get_rss_hash_opts(struct fm10k_intfc *interface,
cmd->data |= RXH_IP_SRC | RXH_IP_DST;
break;
case UDP_V6_FLOW:
- if (interface->flags & FM10K_FLAG_RSS_FIELD_IPV6_UDP)
+ if (test_bit(FM10K_FLAG_RSS_FIELD_IPV6_UDP,
+ interface->flags))
cmd->data |= RXH_L4_B_0_1 | RXH_L4_B_2_3;
cmd->data |= RXH_IP_SRC | RXH_IP_DST;
break;
@@ -764,12 +766,13 @@ static int fm10k_get_rxnfc(struct net_device *dev, struct ethtool_rxnfc *cmd,
return ret;
}
-#define UDP_RSS_FLAGS (FM10K_FLAG_RSS_FIELD_IPV4_UDP | \
- FM10K_FLAG_RSS_FIELD_IPV6_UDP)
static int fm10k_set_rss_hash_opt(struct fm10k_intfc *interface,
struct ethtool_rxnfc *nfc)
{
- u32 flags = interface->flags;
+ int rss_ipv4_udp = test_bit(FM10K_FLAG_RSS_FIELD_IPV4_UDP,
+ interface->flags);
+ int rss_ipv6_udp = test_bit(FM10K_FLAG_RSS_FIELD_IPV6_UDP,
+ interface->flags);
/* RSS does not support anything other than hashing
* to queues on src and dst IPs and ports
@@ -793,10 +796,12 @@ static int fm10k_set_rss_hash_opt(struct fm10k_intfc *interface,
return -EINVAL;
switch (nfc->data & (RXH_L4_B_0_1 | RXH_L4_B_2_3)) {
case 0:
- flags &= ~FM10K_FLAG_RSS_FIELD_IPV4_UDP;
+ clear_bit(FM10K_FLAG_RSS_FIELD_IPV4_UDP,
+ interface->flags);
break;
case (RXH_L4_B_0_1 | RXH_L4_B_2_3):
- flags |= FM10K_FLAG_RSS_FIELD_IPV4_UDP;
+ set_bit(FM10K_FLAG_RSS_FIELD_IPV4_UDP,
+ interface->flags);
break;
default:
return -EINVAL;
@@ -808,10 +813,12 @@ static int fm10k_set_rss_hash_opt(struct fm10k_intfc *interface,
return -EINVAL;
switch (nfc->data & (RXH_L4_B_0_1 | RXH_L4_B_2_3)) {
case 0:
- flags &= ~FM10K_FLAG_RSS_FIELD_IPV6_UDP;
+ clear_bit(FM10K_FLAG_RSS_FIELD_IPV6_UDP,
+ interface->flags);
break;
case (RXH_L4_B_0_1 | RXH_L4_B_2_3):
- flags |= FM10K_FLAG_RSS_FIELD_IPV6_UDP;
+ set_bit(FM10K_FLAG_RSS_FIELD_IPV6_UDP,
+ interface->flags);
break;
default:
return -EINVAL;
@@ -835,28 +842,41 @@ static int fm10k_set_rss_hash_opt(struct fm10k_intfc *interface,
return -EINVAL;
}
- /* if we changed something we need to update flags */
- if (flags != interface->flags) {
+ /* If something changed we need to update the MRQC register. Note that
+ * test_bit() is guaranteed to return strictly 0 or 1, so testing for
+ * equality is safe.
+ */
+ if ((rss_ipv4_udp != test_bit(FM10K_FLAG_RSS_FIELD_IPV4_UDP,
+ interface->flags)) ||
+ (rss_ipv6_udp != test_bit(FM10K_FLAG_RSS_FIELD_IPV6_UDP,
+ interface->flags))) {
struct fm10k_hw *hw = &interface->hw;
+ bool warn = false;
u32 mrqc;
- if ((flags & UDP_RSS_FLAGS) &&
- !(interface->flags & UDP_RSS_FLAGS))
- netif_warn(interface, drv, interface->netdev,
- "enabling UDP RSS: fragmented packets may arrive out of order to the stack above\n");
-
- interface->flags = flags;
-
/* Perform hash on these packet types */
mrqc = FM10K_MRQC_IPV4 |
FM10K_MRQC_TCP_IPV4 |
FM10K_MRQC_IPV6 |
FM10K_MRQC_TCP_IPV6;
- if (flags & FM10K_FLAG_RSS_FIELD_IPV4_UDP)
+ if (test_bit(FM10K_FLAG_RSS_FIELD_IPV4_UDP,
+ interface->flags)) {
mrqc |= FM10K_MRQC_UDP_IPV4;
- if (flags & FM10K_FLAG_RSS_FIELD_IPV6_UDP)
+ warn = true;
+ }
+ if (test_bit(FM10K_FLAG_RSS_FIELD_IPV6_UDP,
+ interface->flags)) {
mrqc |= FM10K_MRQC_UDP_IPV6;
+ warn = true;
+ }
+
+ /* If we enable UDP RSS display a warning that this may cause
+ * fragmented UDP packets to arrive out of order.
+ */
+ if (warn)
+ netif_warn(interface, drv, interface->netdev,
+ "enabling UDP RSS: fragmented packets may arrive out of order to the stack above\n");
fm10k_write_reg(hw, FM10K_MRQC(0), mrqc);
}
diff --git a/drivers/net/ethernet/intel/fm10k/fm10k_main.c b/drivers/net/ethernet/intel/fm10k/fm10k_main.c
index 5bb233a9614c..f9612a6b8524 100644
--- a/drivers/net/ethernet/intel/fm10k/fm10k_main.c
+++ b/drivers/net/ethernet/intel/fm10k/fm10k_main.c
@@ -1,5 +1,5 @@
/* Intel(R) Ethernet Switch Host Interface Driver
- * Copyright(c) 2013 - 2016 Intel Corporation.
+ * Copyright(c) 2013 - 2017 Intel Corporation.
*
* This program is free software; you can redistribute it and/or modify it
* under the terms and conditions of the GNU General Public License,
@@ -34,7 +34,7 @@ const char fm10k_driver_version[] = DRV_VERSION;
char fm10k_driver_name[] = "fm10k";
static const char fm10k_driver_string[] = DRV_SUMMARY;
static const char fm10k_copyright[] =
- "Copyright (c) 2013 - 2016 Intel Corporation.";
+ "Copyright(c) 2013 - 2017 Intel Corporation.";
MODULE_AUTHOR("Intel Corporation, <linux.nics@intel.com>");
MODULE_DESCRIPTION(DRV_SUMMARY);
@@ -1193,7 +1193,7 @@ void fm10k_tx_timeout_reset(struct fm10k_intfc *interface)
/* Do the reset outside of interrupt context */
if (!test_bit(__FM10K_DOWN, &interface->state)) {
interface->tx_timeout_count++;
- interface->flags |= FM10K_FLAG_RESET_REQUESTED;
+ set_bit(FM10K_FLAG_RESET_REQUESTED, interface->flags);
fm10k_service_event_schedule(interface);
}
}
diff --git a/drivers/net/ethernet/intel/fm10k/fm10k_netdev.c b/drivers/net/ethernet/intel/fm10k/fm10k_netdev.c
index 72481670478c..008408dd6a56 100644
--- a/drivers/net/ethernet/intel/fm10k/fm10k_netdev.c
+++ b/drivers/net/ethernet/intel/fm10k/fm10k_netdev.c
@@ -1,5 +1,5 @@
/* Intel(R) Ethernet Switch Host Interface Driver
- * Copyright(c) 2013 - 2016 Intel Corporation.
+ * Copyright(c) 2013 - 2017 Intel Corporation.
*
* This program is free software; you can redistribute it and/or modify it
* under the terms and conditions of the GNU General Public License,
@@ -1207,7 +1207,7 @@ int fm10k_setup_tc(struct net_device *dev, u8 tc)
goto err_open;
/* flag to indicate SWPRI has yet to be updated */
- interface->flags |= FM10K_FLAG_SWPRI_CONFIG;
+ set_bit(FM10K_FLAG_SWPRI_CONFIG, interface->flags);
return 0;
err_open:
diff --git a/drivers/net/ethernet/intel/fm10k/fm10k_pci.c b/drivers/net/ethernet/intel/fm10k/fm10k_pci.c
index 60d9b6aaf63a..7ae22dd5dd7c 100644
--- a/drivers/net/ethernet/intel/fm10k/fm10k_pci.c
+++ b/drivers/net/ethernet/intel/fm10k/fm10k_pci.c
@@ -1,5 +1,5 @@
/* Intel(R) Ethernet Switch Host Interface Driver
- * Copyright(c) 2013 - 2016 Intel Corporation.
+ * Copyright(c) 2013 - 2017 Intel Corporation.
*
* This program is free software; you can redistribute it and/or modify it
* under the terms and conditions of the GNU General Public License,
@@ -137,7 +137,7 @@ static void fm10k_detach_subtask(struct fm10k_intfc *interface)
if (~value) {
interface->hw.hw_addr = interface->uc_addr;
netif_device_attach(netdev);
- interface->flags |= FM10K_FLAG_RESET_REQUESTED;
+ set_bit(FM10K_FLAG_RESET_REQUESTED, interface->flags);
netdev_warn(netdev, "PCIe link restored, device now attached\n");
return;
}
@@ -273,11 +273,10 @@ static void fm10k_reinit(struct fm10k_intfc *interface)
static void fm10k_reset_subtask(struct fm10k_intfc *interface)
{
- if (!(interface->flags & FM10K_FLAG_RESET_REQUESTED))
+ if (!test_and_clear_bit(FM10K_FLAG_RESET_REQUESTED,
+ interface->flags))
return;
- interface->flags &= ~FM10K_FLAG_RESET_REQUESTED;
-
netdev_err(interface->netdev, "Reset interface\n");
fm10k_reinit(interface);
@@ -296,7 +295,7 @@ static void fm10k_configure_swpri_map(struct fm10k_intfc *interface)
int i;
/* clear flag indicating update is needed */
- interface->flags &= ~FM10K_FLAG_SWPRI_CONFIG;
+ clear_bit(FM10K_FLAG_SWPRI_CONFIG, interface->flags);
/* these registers are only available on the PF */
if (hw->mac.type != fm10k_mac_pf)
@@ -324,7 +323,7 @@ static void fm10k_watchdog_update_host_state(struct fm10k_intfc *interface)
clear_bit(__FM10K_LINK_DOWN, &interface->state);
}
- if (interface->flags & FM10K_FLAG_SWPRI_CONFIG) {
+ if (test_bit(FM10K_FLAG_SWPRI_CONFIG, interface->flags)) {
if (rtnl_trylock()) {
fm10k_configure_swpri_map(interface);
rtnl_unlock();
@@ -336,7 +335,7 @@ static void fm10k_watchdog_update_host_state(struct fm10k_intfc *interface)
err = hw->mac.ops.get_host_state(hw, &interface->host_ready);
if (err && time_is_before_jiffies(interface->last_reset))
- interface->flags |= FM10K_FLAG_RESET_REQUESTED;
+ set_bit(FM10K_FLAG_RESET_REQUESTED, interface->flags);
/* free the lock */
fm10k_mbx_unlock(interface);
@@ -523,7 +522,7 @@ static void fm10k_watchdog_flush_tx(struct fm10k_intfc *interface)
* controller to flush Tx.
*/
if (some_tx_pending)
- interface->flags |= FM10K_FLAG_RESET_REQUESTED;
+ set_bit(FM10K_FLAG_RESET_REQUESTED, interface->flags);
}
/**
@@ -864,9 +863,9 @@ static void fm10k_configure_dglort(struct fm10k_intfc *interface)
FM10K_MRQC_IPV6 |
FM10K_MRQC_TCP_IPV6;
- if (interface->flags & FM10K_FLAG_RSS_FIELD_IPV4_UDP)
+ if (test_bit(FM10K_FLAG_RSS_FIELD_IPV4_UDP, interface->flags))
mrqc |= FM10K_MRQC_UDP_IPV4;
- if (interface->flags & FM10K_FLAG_RSS_FIELD_IPV6_UDP)
+ if (test_bit(FM10K_FLAG_RSS_FIELD_IPV6_UDP, interface->flags))
mrqc |= FM10K_MRQC_UDP_IPV6;
fm10k_write_reg(hw, FM10K_MRQC(0), mrqc);
@@ -1168,7 +1167,7 @@ static irqreturn_t fm10k_msix_mbx_pf(int __always_unused irq, void *data)
}
if (err == FM10K_ERR_RESET_REQUESTED)
- interface->flags |= FM10K_FLAG_RESET_REQUESTED;
+ set_bit(FM10K_FLAG_RESET_REQUESTED, interface->flags);
/* if switch toggled state we should reset GLORTs */
if (eicr & FM10K_EICR_SWITCHNOTREADY) {
@@ -1247,12 +1246,12 @@ static s32 fm10k_mbx_mac_addr(struct fm10k_hw *hw, u32 **results,
/* MAC was changed so we need reset */
if (is_valid_ether_addr(hw->mac.perm_addr) &&
!ether_addr_equal(hw->mac.perm_addr, hw->mac.addr))
- interface->flags |= FM10K_FLAG_RESET_REQUESTED;
+ set_bit(FM10K_FLAG_RESET_REQUESTED, interface->flags);
/* VLAN override was changed, or default VLAN changed */
if ((vlan_override != hw->mac.vlan_override) ||
(default_vid != hw->mac.default_vid))
- interface->flags |= FM10K_FLAG_RESET_REQUESTED;
+ set_bit(FM10K_FLAG_RESET_REQUESTED, interface->flags);
return 0;
}
@@ -1357,7 +1356,7 @@ static s32 fm10k_lport_map(struct fm10k_hw *hw, u32 **results,
/* we need to reset if port count was just updated */
if (dglort_map != hw->mac.dglort_map)
- interface->flags |= FM10K_FLAG_RESET_REQUESTED;
+ set_bit(FM10K_FLAG_RESET_REQUESTED, interface->flags);
return 0;
}
@@ -1396,7 +1395,7 @@ static s32 fm10k_update_pvid(struct fm10k_hw *hw, u32 **results,
/* we need to reset if default VLAN was just updated */
if (pvid != hw->mac.default_vid)
- interface->flags |= FM10K_FLAG_RESET_REQUESTED;
+ set_bit(FM10K_FLAG_RESET_REQUESTED, interface->flags);
hw->mac.default_vid = pvid;
--
2.12.2
^ permalink raw reply related [flat|nested] 9+ messages in thread* [net-next 3/7] fm10k: future-proof state bitmaps using DECLARE_BITMAP
2017-04-06 7:54 [net-next 0/7][pull request] 100GbE Intel Wired LAN Driver Updates 2017-04-05 Jeff Kirsher
2017-04-06 7:54 ` [net-next 1/7] fm10k: correctly check if interface is removed Jeff Kirsher
2017-04-06 7:54 ` [net-next 2/7] fm10k: use a BITMAP for flags to avoid race conditions Jeff Kirsher
@ 2017-04-06 7:54 ` Jeff Kirsher
2017-04-06 7:54 ` [net-next 4/7] fm10k: allow service task to reschedule itself Jeff Kirsher
` (4 subsequent siblings)
7 siblings, 0 replies; 9+ messages in thread
From: Jeff Kirsher @ 2017-04-06 7:54 UTC (permalink / raw)
To: davem; +Cc: Jacob Keller, netdev, nhorman, sassmann, jogreene, Jeff Kirsher
From: Jacob Keller <jacob.e.keller@intel.com>
This ensures that future programmers do not have to remember to re-size
the bitmaps due to adding new values. Although this is unlikely for this
driver, it may happen and it's best to prevent it from ever being an
issue.
Signed-off-by: Jacob Keller <jacob.e.keller@intel.com>
Tested-by: Krishneil Singh <krishneil.k.singh@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
drivers/net/ethernet/intel/fm10k/fm10k.h | 40 ++++++++-------
drivers/net/ethernet/intel/fm10k/fm10k_ethtool.c | 4 +-
drivers/net/ethernet/intel/fm10k/fm10k_main.c | 10 ++--
drivers/net/ethernet/intel/fm10k/fm10k_netdev.c | 2 +-
drivers/net/ethernet/intel/fm10k/fm10k_pci.c | 62 ++++++++++++------------
5 files changed, 61 insertions(+), 57 deletions(-)
diff --git a/drivers/net/ethernet/intel/fm10k/fm10k.h b/drivers/net/ethernet/intel/fm10k/fm10k.h
index d6db1c48d4dc..b496300d0268 100644
--- a/drivers/net/ethernet/intel/fm10k/fm10k.h
+++ b/drivers/net/ethernet/intel/fm10k/fm10k.h
@@ -65,14 +65,16 @@ enum fm10k_ring_state_t {
__FM10K_TX_DETECT_HANG,
__FM10K_HANG_CHECK_ARMED,
__FM10K_TX_XPS_INIT_DONE,
+ /* This must be last and is used to calculate BITMAP size */
+ __FM10K_TX_STATE_SIZE__,
};
#define check_for_tx_hang(ring) \
- test_bit(__FM10K_TX_DETECT_HANG, &(ring)->state)
+ test_bit(__FM10K_TX_DETECT_HANG, (ring)->state)
#define set_check_for_tx_hang(ring) \
- set_bit(__FM10K_TX_DETECT_HANG, &(ring)->state)
+ set_bit(__FM10K_TX_DETECT_HANG, (ring)->state)
#define clear_check_for_tx_hang(ring) \
- clear_bit(__FM10K_TX_DETECT_HANG, &(ring)->state)
+ clear_bit(__FM10K_TX_DETECT_HANG, (ring)->state)
struct fm10k_tx_buffer {
struct fm10k_tx_desc *next_to_watch;
@@ -126,7 +128,7 @@ struct fm10k_ring {
struct fm10k_rx_buffer *rx_buffer;
};
u32 __iomem *tail;
- unsigned long state;
+ DECLARE_BITMAP(state, __FM10K_TX_STATE_SIZE__);
dma_addr_t dma; /* phys. address of descriptor ring */
unsigned int size; /* length in bytes */
@@ -266,12 +268,24 @@ enum fm10k_flags_t {
__FM10K_FLAGS_SIZE__
};
+enum fm10k_state_t {
+ __FM10K_RESETTING,
+ __FM10K_DOWN,
+ __FM10K_SERVICE_SCHED,
+ __FM10K_SERVICE_DISABLE,
+ __FM10K_MBX_LOCK,
+ __FM10K_LINK_DOWN,
+ __FM10K_UPDATING_STATS,
+ /* This value must be last and determines the BITMAP size */
+ __FM10K_STATE_SIZE__,
+};
+
struct fm10k_intfc {
unsigned long active_vlans[BITS_TO_LONGS(VLAN_N_VID)];
struct net_device *netdev;
struct fm10k_l2_accel *l2_accel; /* pointer to L2 acceleration list */
struct pci_dev *pdev;
- unsigned long state;
+ DECLARE_BITMAP(state, __FM10K_STATE_SIZE__);
/* Access flag values using atomic *_bit() operations */
DECLARE_BITMAP(flags, __FM10K_FLAGS_SIZE__);
@@ -367,22 +381,12 @@ struct fm10k_intfc {
u16 vid;
};
-enum fm10k_state_t {
- __FM10K_RESETTING,
- __FM10K_DOWN,
- __FM10K_SERVICE_SCHED,
- __FM10K_SERVICE_DISABLE,
- __FM10K_MBX_LOCK,
- __FM10K_LINK_DOWN,
- __FM10K_UPDATING_STATS,
-};
-
static inline void fm10k_mbx_lock(struct fm10k_intfc *interface)
{
/* busy loop if we cannot obtain the lock as some calls
* such as ndo_set_rx_mode may be made in atomic context
*/
- while (test_and_set_bit(__FM10K_MBX_LOCK, &interface->state))
+ while (test_and_set_bit(__FM10K_MBX_LOCK, interface->state))
udelay(20);
}
@@ -390,12 +394,12 @@ static inline void fm10k_mbx_unlock(struct fm10k_intfc *interface)
{
/* flush memory to make sure state is correct */
smp_mb__before_atomic();
- clear_bit(__FM10K_MBX_LOCK, &interface->state);
+ clear_bit(__FM10K_MBX_LOCK, interface->state);
}
static inline int fm10k_mbx_trylock(struct fm10k_intfc *interface)
{
- return !test_and_set_bit(__FM10K_MBX_LOCK, &interface->state);
+ return !test_and_set_bit(__FM10K_MBX_LOCK, interface->state);
}
/* fm10k_test_staterr - test bits in Rx descriptor status and error fields */
diff --git a/drivers/net/ethernet/intel/fm10k/fm10k_ethtool.c b/drivers/net/ethernet/intel/fm10k/fm10k_ethtool.c
index 5269f3ed4682..c7234f35f8ff 100644
--- a/drivers/net/ethernet/intel/fm10k/fm10k_ethtool.c
+++ b/drivers/net/ethernet/intel/fm10k/fm10k_ethtool.c
@@ -562,7 +562,7 @@ static int fm10k_set_ringparam(struct net_device *netdev,
return 0;
}
- while (test_and_set_bit(__FM10K_RESETTING, &interface->state))
+ while (test_and_set_bit(__FM10K_RESETTING, interface->state))
usleep_range(1000, 2000);
if (!netif_running(interface->netdev)) {
@@ -648,7 +648,7 @@ static int fm10k_set_ringparam(struct net_device *netdev,
fm10k_up(interface);
vfree(temp_ring);
clear_reset:
- clear_bit(__FM10K_RESETTING, &interface->state);
+ clear_bit(__FM10K_RESETTING, interface->state);
return err;
}
diff --git a/drivers/net/ethernet/intel/fm10k/fm10k_main.c b/drivers/net/ethernet/intel/fm10k/fm10k_main.c
index f9612a6b8524..9dffaba85ae6 100644
--- a/drivers/net/ethernet/intel/fm10k/fm10k_main.c
+++ b/drivers/net/ethernet/intel/fm10k/fm10k_main.c
@@ -1175,13 +1175,13 @@ bool fm10k_check_tx_hang(struct fm10k_ring *tx_ring)
/* update completed stats and continue */
tx_ring->tx_stats.tx_done_old = tx_done;
/* reset the countdown */
- clear_bit(__FM10K_HANG_CHECK_ARMED, &tx_ring->state);
+ clear_bit(__FM10K_HANG_CHECK_ARMED, tx_ring->state);
return false;
}
/* make sure it is true for two checks in a row */
- return test_and_set_bit(__FM10K_HANG_CHECK_ARMED, &tx_ring->state);
+ return test_and_set_bit(__FM10K_HANG_CHECK_ARMED, tx_ring->state);
}
/**
@@ -1191,7 +1191,7 @@ bool fm10k_check_tx_hang(struct fm10k_ring *tx_ring)
void fm10k_tx_timeout_reset(struct fm10k_intfc *interface)
{
/* Do the reset outside of interrupt context */
- if (!test_bit(__FM10K_DOWN, &interface->state)) {
+ if (!test_bit(__FM10K_DOWN, interface->state)) {
interface->tx_timeout_count++;
set_bit(FM10K_FLAG_RESET_REQUESTED, interface->flags);
fm10k_service_event_schedule(interface);
@@ -1214,7 +1214,7 @@ static bool fm10k_clean_tx_irq(struct fm10k_q_vector *q_vector,
unsigned int budget = q_vector->tx.work_limit;
unsigned int i = tx_ring->next_to_clean;
- if (test_bit(__FM10K_DOWN, &interface->state))
+ if (test_bit(__FM10K_DOWN, interface->state))
return true;
tx_buffer = &tx_ring->tx_buffer[i];
@@ -1344,7 +1344,7 @@ static bool fm10k_clean_tx_irq(struct fm10k_q_vector *q_vector,
smp_mb();
if (__netif_subqueue_stopped(tx_ring->netdev,
tx_ring->queue_index) &&
- !test_bit(__FM10K_DOWN, &interface->state)) {
+ !test_bit(__FM10K_DOWN, interface->state)) {
netif_wake_subqueue(tx_ring->netdev,
tx_ring->queue_index);
++tx_ring->tx_stats.restart_queue;
diff --git a/drivers/net/ethernet/intel/fm10k/fm10k_netdev.c b/drivers/net/ethernet/intel/fm10k/fm10k_netdev.c
index 008408dd6a56..344a392f7a76 100644
--- a/drivers/net/ethernet/intel/fm10k/fm10k_netdev.c
+++ b/drivers/net/ethernet/intel/fm10k/fm10k_netdev.c
@@ -822,7 +822,7 @@ static int fm10k_update_vid(struct net_device *netdev, u16 vid, bool set)
/* Do not throw an error if the interface is down. We will sync once
* we come up
*/
- if (test_bit(__FM10K_DOWN, &interface->state))
+ if (test_bit(__FM10K_DOWN, interface->state))
return 0;
fm10k_mbx_lock(interface);
diff --git a/drivers/net/ethernet/intel/fm10k/fm10k_pci.c b/drivers/net/ethernet/intel/fm10k/fm10k_pci.c
index 7ae22dd5dd7c..2a54372975f6 100644
--- a/drivers/net/ethernet/intel/fm10k/fm10k_pci.c
+++ b/drivers/net/ethernet/intel/fm10k/fm10k_pci.c
@@ -93,18 +93,18 @@ static int fm10k_hw_ready(struct fm10k_intfc *interface)
void fm10k_service_event_schedule(struct fm10k_intfc *interface)
{
- if (!test_bit(__FM10K_SERVICE_DISABLE, &interface->state) &&
- !test_and_set_bit(__FM10K_SERVICE_SCHED, &interface->state))
+ if (!test_bit(__FM10K_SERVICE_DISABLE, interface->state) &&
+ !test_and_set_bit(__FM10K_SERVICE_SCHED, interface->state))
queue_work(fm10k_workqueue, &interface->service_task);
}
static void fm10k_service_event_complete(struct fm10k_intfc *interface)
{
- WARN_ON(!test_bit(__FM10K_SERVICE_SCHED, &interface->state));
+ WARN_ON(!test_bit(__FM10K_SERVICE_SCHED, interface->state));
/* flush memory to make sure state is correct before next watchog */
smp_mb__before_atomic();
- clear_bit(__FM10K_SERVICE_SCHED, &interface->state);
+ clear_bit(__FM10K_SERVICE_SCHED, interface->state);
}
/**
@@ -159,7 +159,7 @@ static void fm10k_prepare_for_reset(struct fm10k_intfc *interface)
/* put off any impending NetWatchDogTimeout */
netif_trans_update(netdev);
- while (test_and_set_bit(__FM10K_RESETTING, &interface->state))
+ while (test_and_set_bit(__FM10K_RESETTING, interface->state))
usleep_range(1000, 2000);
rtnl_lock();
@@ -242,7 +242,7 @@ static int fm10k_handle_reset(struct fm10k_intfc *interface)
rtnl_unlock();
- clear_bit(__FM10K_RESETTING, &interface->state);
+ clear_bit(__FM10K_RESETTING, interface->state);
return err;
err_open:
@@ -254,7 +254,7 @@ static int fm10k_handle_reset(struct fm10k_intfc *interface)
rtnl_unlock();
- clear_bit(__FM10K_RESETTING, &interface->state);
+ clear_bit(__FM10K_RESETTING, interface->state);
return err;
}
@@ -316,11 +316,11 @@ static void fm10k_watchdog_update_host_state(struct fm10k_intfc *interface)
struct fm10k_hw *hw = &interface->hw;
s32 err;
- if (test_bit(__FM10K_LINK_DOWN, &interface->state)) {
+ if (test_bit(__FM10K_LINK_DOWN, interface->state)) {
interface->host_ready = false;
if (time_is_after_jiffies(interface->link_down_event))
return;
- clear_bit(__FM10K_LINK_DOWN, &interface->state);
+ clear_bit(__FM10K_LINK_DOWN, interface->state);
}
if (test_bit(FM10K_FLAG_SWPRI_CONFIG, interface->flags)) {
@@ -411,7 +411,7 @@ void fm10k_update_stats(struct fm10k_intfc *interface)
int i;
/* ensure only one thread updates stats at a time */
- if (test_and_set_bit(__FM10K_UPDATING_STATS, &interface->state))
+ if (test_and_set_bit(__FM10K_UPDATING_STATS, interface->state))
return;
/* do not allow stats update via service task for next second */
@@ -492,7 +492,7 @@ void fm10k_update_stats(struct fm10k_intfc *interface)
net_stats->rx_errors = rx_errors;
net_stats->rx_dropped = interface->stats.nodesc_drop.count;
- clear_bit(__FM10K_UPDATING_STATS, &interface->state);
+ clear_bit(__FM10K_UPDATING_STATS, interface->state);
}
/**
@@ -532,8 +532,8 @@ static void fm10k_watchdog_flush_tx(struct fm10k_intfc *interface)
static void fm10k_watchdog_subtask(struct fm10k_intfc *interface)
{
/* if interface is down do nothing */
- if (test_bit(__FM10K_DOWN, &interface->state) ||
- test_bit(__FM10K_RESETTING, &interface->state))
+ if (test_bit(__FM10K_DOWN, interface->state) ||
+ test_bit(__FM10K_RESETTING, interface->state))
return;
if (interface->host_ready)
@@ -563,8 +563,8 @@ static void fm10k_check_hang_subtask(struct fm10k_intfc *interface)
int i;
/* If we're down or resetting, just bail */
- if (test_bit(__FM10K_DOWN, &interface->state) ||
- test_bit(__FM10K_RESETTING, &interface->state))
+ if (test_bit(__FM10K_DOWN, interface->state) ||
+ test_bit(__FM10K_RESETTING, interface->state))
return;
/* rate limit tx hang checks to only once every 2 seconds */
@@ -663,7 +663,7 @@ static void fm10k_configure_tx_ring(struct fm10k_intfc *interface,
FM10K_PFVTCTL_FTAG_DESC_ENABLE);
/* Initialize XPS */
- if (!test_and_set_bit(__FM10K_TX_XPS_INIT_DONE, &ring->state) &&
+ if (!test_and_set_bit(__FM10K_TX_XPS_INIT_DONE, ring->state) &&
ring->q_vector)
netif_set_xps_queue(ring->netdev,
&ring->q_vector->affinity_mask,
@@ -980,7 +980,7 @@ void fm10k_netpoll(struct net_device *netdev)
int i;
/* if interface is down do nothing */
- if (test_bit(__FM10K_DOWN, &interface->state))
+ if (test_bit(__FM10K_DOWN, interface->state))
return;
for (i = 0; i < interface->num_q_vectors; i++)
@@ -1173,7 +1173,7 @@ static irqreturn_t fm10k_msix_mbx_pf(int __always_unused irq, void *data)
if (eicr & FM10K_EICR_SWITCHNOTREADY) {
/* force link down for at least 4 seconds */
interface->link_down_event = jiffies + (4 * HZ);
- set_bit(__FM10K_LINK_DOWN, &interface->state);
+ set_bit(__FM10K_LINK_DOWN, interface->state);
/* reset dglort_map back to no config */
hw->mac.dglort_map = FM10K_DGLORTMAP_NONE;
@@ -1325,7 +1325,7 @@ static s32 fm10k_lport_map(struct fm10k_hw *hw, u32 **results,
if (!err && hw->swapi.status) {
/* force link down for a reasonable delay */
interface->link_down_event = jiffies + (2 * HZ);
- set_bit(__FM10K_LINK_DOWN, &interface->state);
+ set_bit(__FM10K_LINK_DOWN, interface->state);
/* reset dglort_map back to no config */
hw->mac.dglort_map = FM10K_DGLORTMAP_NONE;
@@ -1623,10 +1623,10 @@ void fm10k_up(struct fm10k_intfc *interface)
hw->mac.ops.update_int_moderator(hw);
/* enable statistics capture again */
- clear_bit(__FM10K_UPDATING_STATS, &interface->state);
+ clear_bit(__FM10K_UPDATING_STATS, interface->state);
/* clear down bit to indicate we are ready to go */
- clear_bit(__FM10K_DOWN, &interface->state);
+ clear_bit(__FM10K_DOWN, interface->state);
/* enable polling cleanups */
fm10k_napi_enable_all(interface);
@@ -1660,7 +1660,7 @@ void fm10k_down(struct fm10k_intfc *interface)
int err, i = 0, count = 0;
/* signal that we are down to the interrupt handler and service task */
- if (test_and_set_bit(__FM10K_DOWN, &interface->state))
+ if (test_and_set_bit(__FM10K_DOWN, interface->state))
return;
/* call carrier off first to avoid false dev_watchdog timeouts */
@@ -1680,7 +1680,7 @@ void fm10k_down(struct fm10k_intfc *interface)
fm10k_update_stats(interface);
/* prevent updating statistics while we're down */
- while (test_and_set_bit(__FM10K_UPDATING_STATS, &interface->state))
+ while (test_and_set_bit(__FM10K_UPDATING_STATS, interface->state))
usleep_range(1000, 2000);
/* skip waiting for TX DMA if we lost PCIe link */
@@ -1849,8 +1849,8 @@ static int fm10k_sw_init(struct fm10k_intfc *interface,
memcpy(interface->rssrk, rss_key, sizeof(rss_key));
/* Start off interface as being down */
- set_bit(__FM10K_DOWN, &interface->state);
- set_bit(__FM10K_UPDATING_STATS, &interface->state);
+ set_bit(__FM10K_DOWN, interface->state);
+ set_bit(__FM10K_UPDATING_STATS, interface->state);
return 0;
}
@@ -2027,7 +2027,7 @@ static int fm10k_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
* must ensure it is disabled since we haven't yet requested the timer
* or work item.
*/
- set_bit(__FM10K_SERVICE_DISABLE, &interface->state);
+ set_bit(__FM10K_SERVICE_DISABLE, interface->state);
err = fm10k_mbx_request_irq(interface);
if (err)
@@ -2068,7 +2068,7 @@ static int fm10k_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
fm10k_iov_configure(pdev, 0);
/* clear the service task disable bit to allow service task to start */
- clear_bit(__FM10K_SERVICE_DISABLE, &interface->state);
+ clear_bit(__FM10K_SERVICE_DISABLE, interface->state);
return 0;
@@ -2106,7 +2106,7 @@ static void fm10k_remove(struct pci_dev *pdev)
del_timer_sync(&interface->service_timer);
- set_bit(__FM10K_SERVICE_DISABLE, &interface->state);
+ set_bit(__FM10K_SERVICE_DISABLE, interface->state);
cancel_work_sync(&interface->service_task);
/* free netdev, this may bounce the interrupts due to setup_tc */
@@ -2145,7 +2145,7 @@ static void fm10k_prepare_suspend(struct fm10k_intfc *interface)
* stopped. We stop the watchdog task until after we resume software
* activity.
*/
- set_bit(__FM10K_SERVICE_DISABLE, &interface->state);
+ set_bit(__FM10K_SERVICE_DISABLE, interface->state);
cancel_work_sync(&interface->service_task);
fm10k_prepare_for_reset(interface);
@@ -2171,10 +2171,10 @@ static int fm10k_handle_resume(struct fm10k_intfc *interface)
/* force link to stay down for a second to prevent link flutter */
interface->link_down_event = jiffies + (HZ);
- set_bit(__FM10K_LINK_DOWN, &interface->state);
+ set_bit(__FM10K_LINK_DOWN, interface->state);
/* clear the service task disable bit to allow service task to start */
- clear_bit(__FM10K_SERVICE_DISABLE, &interface->state);
+ clear_bit(__FM10K_SERVICE_DISABLE, interface->state);
fm10k_service_event_schedule(interface);
return err;
--
2.12.2
^ permalink raw reply related [flat|nested] 9+ messages in thread* [net-next 4/7] fm10k: allow service task to reschedule itself
2017-04-06 7:54 [net-next 0/7][pull request] 100GbE Intel Wired LAN Driver Updates 2017-04-05 Jeff Kirsher
` (2 preceding siblings ...)
2017-04-06 7:54 ` [net-next 3/7] fm10k: future-proof state bitmaps using DECLARE_BITMAP Jeff Kirsher
@ 2017-04-06 7:54 ` Jeff Kirsher
2017-04-06 7:54 ` [net-next 5/7] fm10k: update function header comment for fm10k_get_stats64 Jeff Kirsher
` (3 subsequent siblings)
7 siblings, 0 replies; 9+ messages in thread
From: Jeff Kirsher @ 2017-04-06 7:54 UTC (permalink / raw)
To: davem; +Cc: Jacob Keller, netdev, nhorman, sassmann, jogreene, Jeff Kirsher
From: Jacob Keller <jacob.e.keller@intel.com>
If some code path executes fm10k_service_event_schedule(), it is
guaranteed that we only queue the service task once, since we use
__FM10K_SERVICE_SCHED flag. Unfortunately this has a side effect that if
a service request occurs while we are currently running the watchdog, it
is possible that we will fail to notice the request and ignore it until
the next time the request occurs.
This can cause problems with pf/vf mailbox communication and other
service event tasks. To avoid this, introduce a FM10K_SERVICE_REQUEST
bit. When we successfully schedule (and set the _SCHED bit) the service
task, we will clear this bit. However, if we are unable to currently
schedule the service event, we just set the new SERVICE_REQUEST bit.
Finally, after the service event completes, we will re-schedule if the
request bit has been set.
This should ensure that we do not miss any service event schedules,
since we will re-schedule it once the currently running task finishes.
This means that for each request, we will always schedule the service
task to run at least once in full after the request came in.
This will avoid timing issues that can occur with the service event
scheduling. We do pay a cost in re-running many tasks, but all the
service event tasks use either flags to avoid duplicate work, or are
tolerant of being run multiple times.
Signed-off-by: Jacob Keller <jacob.e.keller@intel.com>
Tested-by: Krishneil Singh <krishneil.k.singh@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
drivers/net/ethernet/intel/fm10k/fm10k.h | 1 +
drivers/net/ethernet/intel/fm10k/fm10k_pci.c | 13 ++++++++++++-
2 files changed, 13 insertions(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/intel/fm10k/fm10k.h b/drivers/net/ethernet/intel/fm10k/fm10k.h
index b496300d0268..689c413b7782 100644
--- a/drivers/net/ethernet/intel/fm10k/fm10k.h
+++ b/drivers/net/ethernet/intel/fm10k/fm10k.h
@@ -272,6 +272,7 @@ enum fm10k_state_t {
__FM10K_RESETTING,
__FM10K_DOWN,
__FM10K_SERVICE_SCHED,
+ __FM10K_SERVICE_REQUEST,
__FM10K_SERVICE_DISABLE,
__FM10K_MBX_LOCK,
__FM10K_LINK_DOWN,
diff --git a/drivers/net/ethernet/intel/fm10k/fm10k_pci.c b/drivers/net/ethernet/intel/fm10k/fm10k_pci.c
index 2a54372975f6..c62d1884f75b 100644
--- a/drivers/net/ethernet/intel/fm10k/fm10k_pci.c
+++ b/drivers/net/ethernet/intel/fm10k/fm10k_pci.c
@@ -94,8 +94,12 @@ static int fm10k_hw_ready(struct fm10k_intfc *interface)
void fm10k_service_event_schedule(struct fm10k_intfc *interface)
{
if (!test_bit(__FM10K_SERVICE_DISABLE, interface->state) &&
- !test_and_set_bit(__FM10K_SERVICE_SCHED, interface->state))
+ !test_and_set_bit(__FM10K_SERVICE_SCHED, interface->state)) {
+ clear_bit(__FM10K_SERVICE_REQUEST, interface->state);
queue_work(fm10k_workqueue, &interface->service_task);
+ } else {
+ set_bit(__FM10K_SERVICE_REQUEST, interface->state);
+ }
}
static void fm10k_service_event_complete(struct fm10k_intfc *interface)
@@ -105,6 +109,13 @@ static void fm10k_service_event_complete(struct fm10k_intfc *interface)
/* flush memory to make sure state is correct before next watchog */
smp_mb__before_atomic();
clear_bit(__FM10K_SERVICE_SCHED, interface->state);
+
+ /* If a service event was requested since we started, immediately
+ * re-schedule now. This ensures we don't drop a request until the
+ * next timer event.
+ */
+ if (test_bit(__FM10K_SERVICE_REQUEST, interface->state))
+ fm10k_service_event_schedule(interface);
}
/**
--
2.12.2
^ permalink raw reply related [flat|nested] 9+ messages in thread* [net-next 5/7] fm10k: update function header comment for fm10k_get_stats64
2017-04-06 7:54 [net-next 0/7][pull request] 100GbE Intel Wired LAN Driver Updates 2017-04-05 Jeff Kirsher
` (3 preceding siblings ...)
2017-04-06 7:54 ` [net-next 4/7] fm10k: allow service task to reschedule itself Jeff Kirsher
@ 2017-04-06 7:54 ` Jeff Kirsher
2017-04-06 7:54 ` [net-next 6/7] fm10k: disable receive queue when configuring ring Jeff Kirsher
` (2 subsequent siblings)
7 siblings, 0 replies; 9+ messages in thread
From: Jeff Kirsher @ 2017-04-06 7:54 UTC (permalink / raw)
To: davem; +Cc: Jacob Keller, netdev, nhorman, sassmann, jogreene, Jeff Kirsher
From: Jacob Keller <jacob.e.keller@intel.com>
Re-word the comment to avoid stating that we return a value for this
void function. Additionally, there is no need to mention older kernels,
since this is the upstream kernel.
Signed-off-by: Jacob Keller <jacob.e.keller@intel.com>
Tested-by: Krishneil Singh <krishneil.k.singh@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
drivers/net/ethernet/intel/fm10k/fm10k_netdev.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/net/ethernet/intel/fm10k/fm10k_netdev.c b/drivers/net/ethernet/intel/fm10k/fm10k_netdev.c
index 344a392f7a76..70cd121d465f 100644
--- a/drivers/net/ethernet/intel/fm10k/fm10k_netdev.c
+++ b/drivers/net/ethernet/intel/fm10k/fm10k_netdev.c
@@ -1115,8 +1115,8 @@ void fm10k_reset_rx_state(struct fm10k_intfc *interface)
* @netdev: network interface device structure
* @stats: storage space for 64bit statistics
*
- * Returns 64bit statistics, for use in the ndo_get_stats64 callback. This
- * function replaces fm10k_get_stats for kernels which support it.
+ * Obtain 64bit statistics in a way that is safe for both 32bit and 64bit
+ * architectures.
*/
static void fm10k_get_stats64(struct net_device *netdev,
struct rtnl_link_stats64 *stats)
--
2.12.2
^ permalink raw reply related [flat|nested] 9+ messages in thread* [net-next 6/7] fm10k: disable receive queue when configuring ring
2017-04-06 7:54 [net-next 0/7][pull request] 100GbE Intel Wired LAN Driver Updates 2017-04-05 Jeff Kirsher
` (4 preceding siblings ...)
2017-04-06 7:54 ` [net-next 5/7] fm10k: update function header comment for fm10k_get_stats64 Jeff Kirsher
@ 2017-04-06 7:54 ` Jeff Kirsher
2017-04-06 7:54 ` [net-next 7/7] fm10k: do not enqueue mailbox when host not ready Jeff Kirsher
2017-04-06 20:53 ` [net-next 0/7][pull request] 100GbE Intel Wired LAN Driver Updates 2017-04-05 David Miller
7 siblings, 0 replies; 9+ messages in thread
From: Jeff Kirsher @ 2017-04-06 7:54 UTC (permalink / raw)
To: davem; +Cc: Ngai-Mint Kwan, netdev, nhorman, sassmann, jogreene, Jeff Kirsher
From: Ngai-Mint Kwan <ngai-mint.kwan@intel.com>
Write to RXQCTL register to disable the receive queue when configuring
the RX ring.
Signed-off-by: Ngai-Mint Kwan <ngai-mint.kwan@intel.com>
Tested-by: Krishneil Singh <krishneil.k.singh@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
drivers/net/ethernet/intel/fm10k/fm10k_pci.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/net/ethernet/intel/fm10k/fm10k_pci.c b/drivers/net/ethernet/intel/fm10k/fm10k_pci.c
index c62d1884f75b..3e26d27ad213 100644
--- a/drivers/net/ethernet/intel/fm10k/fm10k_pci.c
+++ b/drivers/net/ethernet/intel/fm10k/fm10k_pci.c
@@ -754,6 +754,7 @@ static void fm10k_configure_rx_ring(struct fm10k_intfc *interface,
/* disable queue to avoid issues while updating state */
rxqctl = fm10k_read_reg(hw, FM10K_RXQCTL(reg_idx));
rxqctl &= ~FM10K_RXQCTL_ENABLE;
+ fm10k_write_reg(hw, FM10K_RXQCTL(reg_idx), rxqctl);
fm10k_write_flush(hw);
/* possible poll here to verify ring resources have been cleaned */
--
2.12.2
^ permalink raw reply related [flat|nested] 9+ messages in thread* [net-next 7/7] fm10k: do not enqueue mailbox when host not ready
2017-04-06 7:54 [net-next 0/7][pull request] 100GbE Intel Wired LAN Driver Updates 2017-04-05 Jeff Kirsher
` (5 preceding siblings ...)
2017-04-06 7:54 ` [net-next 6/7] fm10k: disable receive queue when configuring ring Jeff Kirsher
@ 2017-04-06 7:54 ` Jeff Kirsher
2017-04-06 20:53 ` [net-next 0/7][pull request] 100GbE Intel Wired LAN Driver Updates 2017-04-05 David Miller
7 siblings, 0 replies; 9+ messages in thread
From: Jeff Kirsher @ 2017-04-06 7:54 UTC (permalink / raw)
To: davem
Cc: Ngai-Mint Kwan, netdev, nhorman, sassmann, jogreene,
Jacob Keller, Jeff Kirsher
From: Ngai-Mint Kwan <ngai-mint.kwan@intel.com>
Interfaces will reset whenever the TX mailbox FIFO has become full. This
occurs more frequently whenever the IES API application is not running
to process and clear the messages in the FIFO. Thus, this could lead to
situations where the interface would enter an infinite reset loop. That
is: if the interface is trying to synchronize a huge number of unicast
and multicast entries with the IES API application, the TX mailbox FIFO
will become full and the interface resets. Once the interface exits
reset, it'll try to synchronize the unicast and multicast entries again.
Ergo, this creates an infinite loop. Other actions such as multiple
mulitcast mode or up/down transitions will fill the TX mailbox FIFO and
induce the interface to reset. To correct these situations, check if the
interface's "host_ready" flag is enabled before enqueuing any messages
to the TX mailbox FIFO. This check will be conducted by a function call.
Lastly, this issue mainly affects the PF and, thus, the VF is exempt.
Signed-off-by: Ngai-Mint Kwan <ngai-mint.kwan@intel.com>
Signed-off-by: Jacob Keller <jacob.e.keller@intel.com>
Tested-by: Krishneil Singh <krishneil.k.singh@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
drivers/net/ethernet/intel/fm10k/fm10k_netdev.c | 105 ++++++++++++++++++------
1 file changed, 80 insertions(+), 25 deletions(-)
diff --git a/drivers/net/ethernet/intel/fm10k/fm10k_netdev.c b/drivers/net/ethernet/intel/fm10k/fm10k_netdev.c
index 70cd121d465f..24f2f6f86f5a 100644
--- a/drivers/net/ethernet/intel/fm10k/fm10k_netdev.c
+++ b/drivers/net/ethernet/intel/fm10k/fm10k_netdev.c
@@ -737,6 +737,23 @@ static void fm10k_tx_timeout(struct net_device *netdev)
}
}
+/**
+ * fm10k_host_mbx_ready - Check PF interface's mailbox readiness
+ * @interface: board private structure
+ *
+ * This function checks if the PF interface's mailbox is ready before queueing
+ * mailbox messages for transmission. This will prevent filling the TX mailbox
+ * queue when the receiver is not ready. VF interfaces are exempt from this
+ * check since it will block all PF-VF mailbox messages from being sent from
+ * the VF to the PF at initialization.
+ **/
+static bool fm10k_host_mbx_ready(struct fm10k_intfc *interface)
+{
+ struct fm10k_hw *hw = &interface->hw;
+
+ return (hw->mac.type == fm10k_mac_vf || interface->host_ready);
+}
+
static int fm10k_uc_vlan_unsync(struct net_device *netdev,
const unsigned char *uc_addr)
{
@@ -745,12 +762,15 @@ static int fm10k_uc_vlan_unsync(struct net_device *netdev,
u16 glort = interface->glort;
u16 vid = interface->vid;
bool set = !!(vid / VLAN_N_VID);
- int err;
+ int err = -EHOSTDOWN;
/* drop any leading bits on the VLAN ID */
vid &= VLAN_N_VID - 1;
- err = hw->mac.ops.update_uc_addr(hw, glort, uc_addr, vid, set, 0);
+ if (fm10k_host_mbx_ready(interface))
+ err = hw->mac.ops.update_uc_addr(hw, glort, uc_addr,
+ vid, set, 0);
+
if (err)
return err;
@@ -766,12 +786,14 @@ static int fm10k_mc_vlan_unsync(struct net_device *netdev,
u16 glort = interface->glort;
u16 vid = interface->vid;
bool set = !!(vid / VLAN_N_VID);
- int err;
+ int err = -EHOSTDOWN;
/* drop any leading bits on the VLAN ID */
vid &= VLAN_N_VID - 1;
- err = hw->mac.ops.update_mc_addr(hw, glort, mc_addr, vid, set);
+ if (fm10k_host_mbx_ready(interface))
+ err = hw->mac.ops.update_mc_addr(hw, glort, mc_addr, vid, set);
+
if (err)
return err;
@@ -834,9 +856,13 @@ static int fm10k_update_vid(struct net_device *netdev, u16 vid, bool set)
goto err_out;
}
- /* update our base MAC address */
- err = hw->mac.ops.update_uc_addr(hw, interface->glort, hw->mac.addr,
- vid, set, 0);
+ /* update our base MAC address if host's mailbox is ready */
+ if (fm10k_host_mbx_ready(interface))
+ err = hw->mac.ops.update_uc_addr(hw, interface->glort,
+ hw->mac.addr, vid, set, 0);
+ else
+ err = -EHOSTDOWN;
+
if (err)
goto err_out;
@@ -907,12 +933,15 @@ static int __fm10k_uc_sync(struct net_device *dev,
if (!is_valid_ether_addr(addr))
return -EADDRNOTAVAIL;
- /* update table with current entries */
+ /* update table with current entries if host's mailbox is ready */
+ if (!fm10k_host_mbx_ready(interface))
+ return -EHOSTDOWN;
+
for (vid = hw->mac.default_vid ? fm10k_find_next_vlan(interface, 0) : 1;
vid < VLAN_N_VID;
vid = fm10k_find_next_vlan(interface, vid)) {
err = hw->mac.ops.update_uc_addr(hw, glort, addr,
- vid, sync, 0);
+ vid, sync, 0);
if (err)
return err;
}
@@ -970,7 +999,10 @@ static int __fm10k_mc_sync(struct net_device *dev,
struct fm10k_hw *hw = &interface->hw;
u16 vid, glort = interface->glort;
- /* update table with current entries */
+ /* update table with current entries if host's mailbox is ready */
+ if (!fm10k_host_mbx_ready(interface))
+ return 0;
+
for (vid = hw->mac.default_vid ? fm10k_find_next_vlan(interface, 0) : 1;
vid < VLAN_N_VID;
vid = fm10k_find_next_vlan(interface, vid)) {
@@ -1018,8 +1050,10 @@ static void fm10k_set_rx_mode(struct net_device *dev)
if (interface->xcast_mode == FM10K_XCAST_MODE_PROMISC)
fm10k_clear_unused_vlans(interface);
- /* update xcast mode */
- hw->mac.ops.update_xcast_mode(hw, interface->glort, xcast_mode);
+ /* update xcast mode if host's mailbox is ready */
+ if (fm10k_host_mbx_ready(interface))
+ hw->mac.ops.update_xcast_mode(hw, interface->glort,
+ xcast_mode);
/* record updated xcast mode state */
interface->xcast_mode = xcast_mode;
@@ -1054,8 +1088,10 @@ void fm10k_restore_rx_state(struct fm10k_intfc *interface)
fm10k_mbx_lock(interface);
- /* Enable logical port */
- hw->mac.ops.update_lport_state(hw, glort, interface->glort_count, true);
+ /* Enable logical port if host's mailbox is ready */
+ if (fm10k_host_mbx_ready(interface))
+ hw->mac.ops.update_lport_state(hw, glort,
+ interface->glort_count, true);
/* update VLAN table */
hw->mac.ops.update_vlan(hw, FM10K_VLAN_ALL, 0,
@@ -1069,12 +1105,18 @@ void fm10k_restore_rx_state(struct fm10k_intfc *interface)
vid < VLAN_N_VID;
vid = fm10k_find_next_vlan(interface, vid)) {
hw->mac.ops.update_vlan(hw, vid, 0, true);
- hw->mac.ops.update_uc_addr(hw, glort, hw->mac.addr,
- vid, true, 0);
+
+ /* Update unicast entries if host's mailbox is ready */
+ if (fm10k_host_mbx_ready(interface))
+ hw->mac.ops.update_uc_addr(hw, glort, hw->mac.addr,
+ vid, true, 0);
}
- /* update xcast mode before synchronizing addresses */
- hw->mac.ops.update_xcast_mode(hw, glort, xcast_mode);
+ /* update xcast mode before synchronizing addresses if host's mailbox
+ * is ready
+ */
+ if (fm10k_host_mbx_ready(interface))
+ hw->mac.ops.update_xcast_mode(hw, glort, xcast_mode);
/* synchronize all of the addresses */
__dev_uc_sync(netdev, fm10k_uc_sync, fm10k_uc_unsync);
@@ -1096,9 +1138,12 @@ void fm10k_reset_rx_state(struct fm10k_intfc *interface)
fm10k_mbx_lock(interface);
- /* clear the logical port state on lower device */
- hw->mac.ops.update_lport_state(hw, interface->glort,
- interface->glort_count, false);
+ /* clear the logical port state on lower device if host's mailbox is
+ * ready
+ */
+ if (fm10k_host_mbx_ready(interface))
+ hw->mac.ops.update_lport_state(hw, interface->glort,
+ interface->glort_count, false);
fm10k_mbx_unlock(interface);
@@ -1319,8 +1364,13 @@ static void *fm10k_dfwd_add_station(struct net_device *dev,
fm10k_mbx_lock(interface);
glort = l2_accel->dglort + 1 + i;
- hw->mac.ops.update_xcast_mode(hw, glort, FM10K_XCAST_MODE_MULTI);
- hw->mac.ops.update_uc_addr(hw, glort, sdev->dev_addr, 0, true, 0);
+
+ if (fm10k_host_mbx_ready(interface)) {
+ hw->mac.ops.update_xcast_mode(hw, glort,
+ FM10K_XCAST_MODE_MULTI);
+ hw->mac.ops.update_uc_addr(hw, glort, sdev->dev_addr,
+ 0, true, 0);
+ }
fm10k_mbx_unlock(interface);
@@ -1354,8 +1404,13 @@ static void fm10k_dfwd_del_station(struct net_device *dev, void *priv)
fm10k_mbx_lock(interface);
glort = l2_accel->dglort + 1 + i;
- hw->mac.ops.update_xcast_mode(hw, glort, FM10K_XCAST_MODE_NONE);
- hw->mac.ops.update_uc_addr(hw, glort, sdev->dev_addr, 0, false, 0);
+
+ if (fm10k_host_mbx_ready(interface)) {
+ hw->mac.ops.update_xcast_mode(hw, glort,
+ FM10K_XCAST_MODE_NONE);
+ hw->mac.ops.update_uc_addr(hw, glort, sdev->dev_addr,
+ 0, false, 0);
+ }
fm10k_mbx_unlock(interface);
--
2.12.2
^ permalink raw reply related [flat|nested] 9+ messages in thread* Re: [net-next 0/7][pull request] 100GbE Intel Wired LAN Driver Updates 2017-04-05
2017-04-06 7:54 [net-next 0/7][pull request] 100GbE Intel Wired LAN Driver Updates 2017-04-05 Jeff Kirsher
` (6 preceding siblings ...)
2017-04-06 7:54 ` [net-next 7/7] fm10k: do not enqueue mailbox when host not ready Jeff Kirsher
@ 2017-04-06 20:53 ` David Miller
7 siblings, 0 replies; 9+ messages in thread
From: David Miller @ 2017-04-06 20:53 UTC (permalink / raw)
To: jeffrey.t.kirsher; +Cc: netdev, nhorman, sassmann, jogreene
From: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
Date: Thu, 6 Apr 2017 00:54:49 -0700
> This series contains updates to fm10k only.
>
> Phil Turnbull from Oracle fixes an issue where the argument provided to
> FM10K_REMOVED macro was not what was expecting.
>
> Jake modifies the driver to replace the bitwise operators and defines with
> a BITMAP and enumeration values to avoid race conditions. Also future
> proof the driver so that developers do not have to remember to re-size the
> bitmaps when adding new values. Fixed the wording of a code comment to
> avoid stating that we return a value for a void function.
>
> Ngai-Mint makes sure that when configuring the receive ring, we make sure
> the receive queue is disabled. Fixed an issue where interfaces were
> resetting because the transmit mailbox FIFO was becoming full since the
> host was not ready, so ensure the host is ready before queueing up
> mailbox messages.
Pulled, thanks Jeff.
^ permalink raw reply [flat|nested] 9+ messages in thread