* [net-next 01/13] fm10k: ensure we process SM mbx when processing VF mbx
2017-10-02 15:42 [net-next 00/13][pull request] 100GbE Intel Wired LAN Driver Updates 2017-10-02 Jeff Kirsher
@ 2017-10-02 15:42 ` Jeff Kirsher
2017-10-02 15:42 ` [net-next 02/13] fm10k: reschedule service event if we stall the PF<->SM mailbox Jeff Kirsher
` (12 subsequent siblings)
13 siblings, 0 replies; 15+ messages in thread
From: Jeff Kirsher @ 2017-10-02 15:42 UTC (permalink / raw)
To: davem; +Cc: Jacob Keller, netdev, nhorman, sassmann, jogreene, Jeff Kirsher
From: Jacob Keller <jacob.e.keller@intel.com>
When we process VF mailboxes, the driver is likely going to also queue
up messages to the switch manager. This process merely queues up the
FIFO, but doesn't actually begin the transmission process. Because we
hold the mailbox lock during this VF processing, the PF<->SM mailbox is
not getting processed at this time. Ensure that we actually process the
PF<->SM mailbox in between each PF<->VF mailbox.
This should ensure prompt transmission of the messages queued up after
each VF message is received and handled.
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_iov.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/drivers/net/ethernet/intel/fm10k/fm10k_iov.c b/drivers/net/ethernet/intel/fm10k/fm10k_iov.c
index 5f4dac0d36ef..2ec49116fe91 100644
--- a/drivers/net/ethernet/intel/fm10k/fm10k_iov.c
+++ b/drivers/net/ethernet/intel/fm10k/fm10k_iov.c
@@ -126,6 +126,9 @@ s32 fm10k_iov_mbx(struct fm10k_intfc *interface)
struct fm10k_mbx_info *mbx = &vf_info->mbx;
u16 glort = vf_info->glort;
+ /* process the SM mailbox first to drain outgoing messages */
+ hw->mbx.ops.process(hw, &hw->mbx);
+
/* verify port mapping is valid, if not reset port */
if (vf_info->vf_flags && !fm10k_glort_valid_pf(hw, glort))
hw->iov.ops.reset_lport(hw, vf_info);
--
2.14.2
^ permalink raw reply related [flat|nested] 15+ messages in thread* [net-next 02/13] fm10k: reschedule service event if we stall the PF<->SM mailbox
2017-10-02 15:42 [net-next 00/13][pull request] 100GbE Intel Wired LAN Driver Updates 2017-10-02 Jeff Kirsher
2017-10-02 15:42 ` [net-next 01/13] fm10k: ensure we process SM mbx when processing VF mbx Jeff Kirsher
@ 2017-10-02 15:42 ` Jeff Kirsher
2017-10-02 15:42 ` [net-next 03/13] fm10k: Use seq_putc() in fm10k_dbg_desc_break() Jeff Kirsher
` (11 subsequent siblings)
13 siblings, 0 replies; 15+ messages in thread
From: Jeff Kirsher @ 2017-10-02 15:42 UTC (permalink / raw)
To: davem; +Cc: Jacob Keller, netdev, nhorman, sassmann, jogreene, Jeff Kirsher
From: Jacob Keller <jacob.e.keller@intel.com>
When we are handling PF<->VF mailbox messages, it is possible that the
VF will send us so many messages that the PF<->SM FIFO will fill up. In
this case, we stop the loop and wait until the service event is
rescheduled.
Normally this should happen due to an interrupt. But it is possible that
we don't get another interrupt for a while and it isn't until the
service timer actually reschedules us. Instead, simply reschedule
immediately which will cause the service event to be run again as soon
as we exit.
This ensures that we promptly handle all of the PF<->VF messages with
minimal delay, while still giving time for the SM mailbox to drain.
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_iov.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/drivers/net/ethernet/intel/fm10k/fm10k_iov.c b/drivers/net/ethernet/intel/fm10k/fm10k_iov.c
index 2ec49116fe91..d8356c494f06 100644
--- a/drivers/net/ethernet/intel/fm10k/fm10k_iov.c
+++ b/drivers/net/ethernet/intel/fm10k/fm10k_iov.c
@@ -143,6 +143,10 @@ s32 fm10k_iov_mbx(struct fm10k_intfc *interface)
if (!hw->mbx.ops.tx_ready(&hw->mbx, FM10K_VFMBX_MSG_MTU)) {
/* keep track of how many times this occurs */
interface->hw_sm_mbx_full++;
+
+ /* make sure we try again momentarily */
+ fm10k_service_event_schedule(interface);
+
break;
}
--
2.14.2
^ permalink raw reply related [flat|nested] 15+ messages in thread* [net-next 03/13] fm10k: Use seq_putc() in fm10k_dbg_desc_break()
2017-10-02 15:42 [net-next 00/13][pull request] 100GbE Intel Wired LAN Driver Updates 2017-10-02 Jeff Kirsher
2017-10-02 15:42 ` [net-next 01/13] fm10k: ensure we process SM mbx when processing VF mbx Jeff Kirsher
2017-10-02 15:42 ` [net-next 02/13] fm10k: reschedule service event if we stall the PF<->SM mailbox Jeff Kirsher
@ 2017-10-02 15:42 ` Jeff Kirsher
2017-10-02 15:42 ` [net-next 04/13] fm10k: stop spurious link down messages when Tx FIFO is full Jeff Kirsher
` (10 subsequent siblings)
13 siblings, 0 replies; 15+ messages in thread
From: Jeff Kirsher @ 2017-10-02 15:42 UTC (permalink / raw)
To: davem; +Cc: Markus Elfring, netdev, nhorman, sassmann, jogreene, Jeff Kirsher
From: Markus Elfring <elfring@users.sourceforge.net>
Two single characters should be put into a sequence.
Thus use the corresponding function "seq_putc".
This issue was detected by using the Coccinelle software.
Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
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_debugfs.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/net/ethernet/intel/fm10k/fm10k_debugfs.c b/drivers/net/ethernet/intel/fm10k/fm10k_debugfs.c
index 5116fd043630..14df09e2d964 100644
--- a/drivers/net/ethernet/intel/fm10k/fm10k_debugfs.c
+++ b/drivers/net/ethernet/intel/fm10k/fm10k_debugfs.c
@@ -52,9 +52,9 @@ static void fm10k_dbg_desc_seq_stop(struct seq_file __always_unused *s,
static void fm10k_dbg_desc_break(struct seq_file *s, int i)
{
while (i--)
- seq_puts(s, "-");
+ seq_putc(s, '-');
- seq_puts(s, "\n");
+ seq_putc(s, '\n');
}
static int fm10k_dbg_tx_desc_seq_show(struct seq_file *s, void *v)
--
2.14.2
^ permalink raw reply related [flat|nested] 15+ messages in thread* [net-next 04/13] fm10k: stop spurious link down messages when Tx FIFO is full
2017-10-02 15:42 [net-next 00/13][pull request] 100GbE Intel Wired LAN Driver Updates 2017-10-02 Jeff Kirsher
` (2 preceding siblings ...)
2017-10-02 15:42 ` [net-next 03/13] fm10k: Use seq_putc() in fm10k_dbg_desc_break() Jeff Kirsher
@ 2017-10-02 15:42 ` Jeff Kirsher
2017-10-02 15:42 ` [net-next 05/13] fm10k: fix typos on fall through comments Jeff Kirsher
` (9 subsequent siblings)
13 siblings, 0 replies; 15+ messages in thread
From: Jeff Kirsher @ 2017-10-02 15:42 UTC (permalink / raw)
To: davem; +Cc: Jacob Keller, netdev, nhorman, sassmann, jogreene, Jeff Kirsher
From: Jacob Keller <jacob.e.keller@intel.com>
In fm10k_get_host_state_generic, we check the mailbox tx_read() function
to ensure that the mailbox is still open. This function also checks to
make sure we have space to transmit another message. Unfortunately, if
we just recently sent a bunch of messages (such as enabling hundreds of
VLANs on a VF) this can result in a race where the watchdog task thinks
the link went down just because we haven't had time to process all these
messages yet.
Instead, lets just check whether the mailbox is still open. This ensures
that we don't race with the Tx FIFO, and we only link down once the
mailbox is not open.
This is safe, because if the FIFO fills up and we're unable to send
a message for too long, we'll end up triggering the timeout detection
which results in a reset. Additionally, since we still check to ensure
the mailbox state is OPEN, we'll transition to link down whenever the
mailbox closes as well.
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_common.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/net/ethernet/intel/fm10k/fm10k_common.c b/drivers/net/ethernet/intel/fm10k/fm10k_common.c
index 62a6ad9b3eed..736a9f087bc9 100644
--- a/drivers/net/ethernet/intel/fm10k/fm10k_common.c
+++ b/drivers/net/ethernet/intel/fm10k/fm10k_common.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,
@@ -517,8 +517,8 @@ s32 fm10k_get_host_state_generic(struct fm10k_hw *hw, bool *host_ready)
goto out;
}
- /* verify Mailbox is still valid */
- if (!mbx->ops.tx_ready(mbx, FM10K_VFMBX_MSG_MTU))
+ /* verify Mailbox is still open */
+ if (mbx->state != FM10K_STATE_OPEN)
goto out;
/* interface cannot receive traffic without logical ports */
--
2.14.2
^ permalink raw reply related [flat|nested] 15+ messages in thread* [net-next 05/13] fm10k: fix typos on fall through comments
2017-10-02 15:42 [net-next 00/13][pull request] 100GbE Intel Wired LAN Driver Updates 2017-10-02 Jeff Kirsher
` (3 preceding siblings ...)
2017-10-02 15:42 ` [net-next 04/13] fm10k: stop spurious link down messages when Tx FIFO is full Jeff Kirsher
@ 2017-10-02 15:42 ` Jeff Kirsher
2017-10-02 15:42 ` [net-next 06/13] fm10k: avoid possible truncation of q_vector->name Jeff Kirsher
` (8 subsequent siblings)
13 siblings, 0 replies; 15+ messages in thread
From: Jeff Kirsher @ 2017-10-02 15:42 UTC (permalink / raw)
To: davem; +Cc: Jacob Keller, netdev, nhorman, sassmann, jogreene, Jeff Kirsher
From: Jacob Keller <jacob.e.keller@intel.com>
Newer versions of GCC since version 7 now warn when a case statement may
fall through without an explicit comment. "Fallthough" does not count as
it is misspelled. Fix the typos for these comments to appease the new
warnings.
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_mbx.c | 4 ++--
drivers/net/ethernet/intel/fm10k/fm10k_pf.c | 10 +++++-----
2 files changed, 7 insertions(+), 7 deletions(-)
diff --git a/drivers/net/ethernet/intel/fm10k/fm10k_mbx.c b/drivers/net/ethernet/intel/fm10k/fm10k_mbx.c
index 334088a101c3..244d3ad58ca7 100644
--- a/drivers/net/ethernet/intel/fm10k/fm10k_mbx.c
+++ b/drivers/net/ethernet/intel/fm10k/fm10k_mbx.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,
@@ -1586,7 +1586,7 @@ s32 fm10k_pfvf_mbx_init(struct fm10k_hw *hw, struct fm10k_mbx_info *mbx,
mbx->mbmem_reg = FM10K_MBMEM_VF(id, 0);
break;
}
- /* fallthough */
+ /* fall through */
default:
return FM10K_MBX_ERR_NO_MBX;
}
diff --git a/drivers/net/ethernet/intel/fm10k/fm10k_pf.c b/drivers/net/ethernet/intel/fm10k/fm10k_pf.c
index 40ee0242a80a..9e4fb3a44376 100644
--- a/drivers/net/ethernet/intel/fm10k/fm10k_pf.c
+++ b/drivers/net/ethernet/intel/fm10k/fm10k_pf.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,
@@ -1334,19 +1334,19 @@ static u8 fm10k_iov_supported_xcast_mode_pf(struct fm10k_vf_info *vf_info,
case FM10K_XCAST_MODE_PROMISC:
if (vf_flags & FM10K_VF_FLAG_PROMISC_CAPABLE)
return FM10K_XCAST_MODE_PROMISC;
- /* fallthough */
+ /* fall through */
case FM10K_XCAST_MODE_ALLMULTI:
if (vf_flags & FM10K_VF_FLAG_ALLMULTI_CAPABLE)
return FM10K_XCAST_MODE_ALLMULTI;
- /* fallthough */
+ /* fall through */
case FM10K_XCAST_MODE_MULTI:
if (vf_flags & FM10K_VF_FLAG_MULTI_CAPABLE)
return FM10K_XCAST_MODE_MULTI;
- /* fallthough */
+ /* fall through */
case FM10K_XCAST_MODE_NONE:
if (vf_flags & FM10K_VF_FLAG_NONE_CAPABLE)
return FM10K_XCAST_MODE_NONE;
- /* fallthough */
+ /* fall through */
default:
break;
}
--
2.14.2
^ permalink raw reply related [flat|nested] 15+ messages in thread* [net-next 06/13] fm10k: avoid possible truncation of q_vector->name
2017-10-02 15:42 [net-next 00/13][pull request] 100GbE Intel Wired LAN Driver Updates 2017-10-02 Jeff Kirsher
` (4 preceding siblings ...)
2017-10-02 15:42 ` [net-next 05/13] fm10k: fix typos on fall through comments Jeff Kirsher
@ 2017-10-02 15:42 ` Jeff Kirsher
2017-10-02 15:42 ` [net-next 07/13] fm10k: add missing fall through comment Jeff Kirsher
` (7 subsequent siblings)
13 siblings, 0 replies; 15+ messages in thread
From: Jeff Kirsher @ 2017-10-02 15:42 UTC (permalink / raw)
To: davem; +Cc: Jacob Keller, netdev, nhorman, sassmann, jogreene, Jeff Kirsher
From: Jacob Keller <jacob.e.keller@intel.com>
New versions of GCC since version 7 began warning about possible
truncation of calls to snprintf. We can fix this and avoid false
positives. First, we should pass the full buffer size to snprintf,
because it guarantees a NULL character as part of its passed length, so
passing len-1 is simply wasting a byte of possible storage.
Second, if we make the ri and ti variables unsigned, the compiler is
able to correctly reason that the value never gets larger than 256, so
it doesn't need to warn about the full space required to print a signed
integer.
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_pci.c | 14 +++++++-------
1 file changed, 7 insertions(+), 7 deletions(-)
diff --git a/drivers/net/ethernet/intel/fm10k/fm10k_pci.c b/drivers/net/ethernet/intel/fm10k/fm10k_pci.c
index 63784576ae8b..9212b3fa3b62 100644
--- a/drivers/net/ethernet/intel/fm10k/fm10k_pci.c
+++ b/drivers/net/ethernet/intel/fm10k/fm10k_pci.c
@@ -1544,7 +1544,7 @@ int fm10k_qv_request_irq(struct fm10k_intfc *interface)
struct net_device *dev = interface->netdev;
struct fm10k_hw *hw = &interface->hw;
struct msix_entry *entry;
- int ri = 0, ti = 0;
+ unsigned int ri = 0, ti = 0;
int vector, err;
entry = &interface->msix_entries[NON_Q_VECTORS(hw)];
@@ -1554,15 +1554,15 @@ int fm10k_qv_request_irq(struct fm10k_intfc *interface)
/* name the vector */
if (q_vector->tx.count && q_vector->rx.count) {
- snprintf(q_vector->name, sizeof(q_vector->name) - 1,
- "%s-TxRx-%d", dev->name, ri++);
+ snprintf(q_vector->name, sizeof(q_vector->name),
+ "%s-TxRx-%u", dev->name, ri++);
ti++;
} else if (q_vector->rx.count) {
- snprintf(q_vector->name, sizeof(q_vector->name) - 1,
- "%s-rx-%d", dev->name, ri++);
+ snprintf(q_vector->name, sizeof(q_vector->name),
+ "%s-rx-%u", dev->name, ri++);
} else if (q_vector->tx.count) {
- snprintf(q_vector->name, sizeof(q_vector->name) - 1,
- "%s-tx-%d", dev->name, ti++);
+ snprintf(q_vector->name, sizeof(q_vector->name),
+ "%s-tx-%u", dev->name, ti++);
} else {
/* skip this unused q_vector */
continue;
--
2.14.2
^ permalink raw reply related [flat|nested] 15+ messages in thread* [net-next 07/13] fm10k: add missing fall through comment
2017-10-02 15:42 [net-next 00/13][pull request] 100GbE Intel Wired LAN Driver Updates 2017-10-02 Jeff Kirsher
` (5 preceding siblings ...)
2017-10-02 15:42 ` [net-next 06/13] fm10k: avoid possible truncation of q_vector->name Jeff Kirsher
@ 2017-10-02 15:42 ` Jeff Kirsher
2017-10-02 15:42 ` [net-next 08/13] fm10k: avoid needless delay when loading driver Jeff Kirsher
` (6 subsequent siblings)
13 siblings, 0 replies; 15+ messages in thread
From: Jeff Kirsher @ 2017-10-02 15:42 UTC (permalink / raw)
To: davem; +Cc: Jacob Keller, netdev, nhorman, sassmann, jogreene, Jeff Kirsher
From: Jacob Keller <jacob.e.keller@intel.com>
Newer versions of GCC starting with 7 now additionally warn when a case
statement may fall through without an explicit comment mentioning it.
Add such a comment to silence the warning, as this is expected.
Unfortunately the comment must come directly before the next case
statement, so we put it outside the #ifdef. Otherwise, the compiler
cannot properly detect it and thus the warning is displayed regardless.
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_main.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/net/ethernet/intel/fm10k/fm10k_main.c b/drivers/net/ethernet/intel/fm10k/fm10k_main.c
index 9dffaba85ae6..189d52a8a605 100644
--- a/drivers/net/ethernet/intel/fm10k/fm10k_main.c
+++ b/drivers/net/ethernet/intel/fm10k/fm10k_main.c
@@ -876,6 +876,7 @@ static void fm10k_tx_csum(struct fm10k_ring *tx_ring,
case IPPROTO_GRE:
if (skb->encapsulation)
break;
+ /* fall through */
default:
if (unlikely(net_ratelimit())) {
dev_warn(tx_ring->dev,
--
2.14.2
^ permalink raw reply related [flat|nested] 15+ messages in thread* [net-next 08/13] fm10k: avoid needless delay when loading driver
2017-10-02 15:42 [net-next 00/13][pull request] 100GbE Intel Wired LAN Driver Updates 2017-10-02 Jeff Kirsher
` (6 preceding siblings ...)
2017-10-02 15:42 ` [net-next 07/13] fm10k: add missing fall through comment Jeff Kirsher
@ 2017-10-02 15:42 ` Jeff Kirsher
2017-10-02 15:42 ` [net-next 09/13] fm10k: simplify reading PFVFLRE register Jeff Kirsher
` (5 subsequent siblings)
13 siblings, 0 replies; 15+ messages in thread
From: Jeff Kirsher @ 2017-10-02 15:42 UTC (permalink / raw)
To: davem; +Cc: Jacob Keller, netdev, nhorman, sassmann, jogreene, Jeff Kirsher
From: Jacob Keller <jacob.e.keller@intel.com>
When we load the driver, we set the last_reset to be in the future,
which delays the initial driver reset. Additionally, the service task
isn't scheduled to run automatically until the timer runs out. This
causes a needless delay of the first reset to begin talking to the
switch manager.
We can avoid this by simply not setting last_reset and immediately
scheduling the service task while in probe. This allows the device to
wake up faster, and avoids this delay.
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_pci.c | 6 ++----
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/drivers/net/ethernet/intel/fm10k/fm10k_pci.c b/drivers/net/ethernet/intel/fm10k/fm10k_pci.c
index 9212b3fa3b62..6c2c4bffaedf 100644
--- a/drivers/net/ethernet/intel/fm10k/fm10k_pci.c
+++ b/drivers/net/ethernet/intel/fm10k/fm10k_pci.c
@@ -1800,9 +1800,6 @@ static int fm10k_sw_init(struct fm10k_intfc *interface,
netdev->vlan_features |= NETIF_F_HIGHDMA;
}
- /* delay any future reset requests */
- interface->last_reset = jiffies + (10 * HZ);
-
/* reset and initialize the hardware so it is in a known state */
err = hw->mac.ops.reset_hw(hw);
if (err) {
@@ -2079,8 +2076,9 @@ static int fm10k_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
/* enable SR-IOV after registering netdev to enforce PF/VF ordering */
fm10k_iov_configure(pdev, 0);
- /* clear the service task disable bit to allow service task to start */
+ /* clear the service task disable bit and kick off service task */
clear_bit(__FM10K_SERVICE_DISABLE, interface->state);
+ fm10k_service_event_schedule(interface);
return 0;
--
2.14.2
^ permalink raw reply related [flat|nested] 15+ messages in thread* [net-next 09/13] fm10k: simplify reading PFVFLRE register
2017-10-02 15:42 [net-next 00/13][pull request] 100GbE Intel Wired LAN Driver Updates 2017-10-02 Jeff Kirsher
` (7 preceding siblings ...)
2017-10-02 15:42 ` [net-next 08/13] fm10k: avoid needless delay when loading driver Jeff Kirsher
@ 2017-10-02 15:42 ` Jeff Kirsher
2017-10-02 15:42 ` [net-next 10/13] fm10k: don't loop while resetting VFs due to VFLR event Jeff Kirsher
` (4 subsequent siblings)
13 siblings, 0 replies; 15+ messages in thread
From: Jeff Kirsher @ 2017-10-02 15:42 UTC (permalink / raw)
To: davem; +Cc: Jacob Keller, netdev, nhorman, sassmann, jogreene, Jeff Kirsher
From: Jacob Keller <jacob.e.keller@intel.com>
We're doing a really convoluted bitshift and read for the PFVFLRE
register. Just reading the PFVFLRE(1), shifting it by 32, then reading
PFVFLRE(0) should be sufficient.
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_iov.c | 6 ++----
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/drivers/net/ethernet/intel/fm10k/fm10k_iov.c b/drivers/net/ethernet/intel/fm10k/fm10k_iov.c
index d8356c494f06..dfc88a463735 100644
--- a/drivers/net/ethernet/intel/fm10k/fm10k_iov.c
+++ b/drivers/net/ethernet/intel/fm10k/fm10k_iov.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,
@@ -67,10 +67,8 @@ s32 fm10k_iov_event(struct fm10k_intfc *interface)
/* read VFLRE to determine if any VFs have been reset */
do {
- vflre = fm10k_read_reg(hw, FM10K_PFVFLRE(0));
+ vflre = fm10k_read_reg(hw, FM10K_PFVFLRE(1));
vflre <<= 32;
- vflre |= fm10k_read_reg(hw, FM10K_PFVFLRE(1));
- vflre = (vflre << 32) | (vflre >> 32);
vflre |= fm10k_read_reg(hw, FM10K_PFVFLRE(0));
i = iov_data->num_vfs;
--
2.14.2
^ permalink raw reply related [flat|nested] 15+ messages in thread* [net-next 10/13] fm10k: don't loop while resetting VFs due to VFLR event
2017-10-02 15:42 [net-next 00/13][pull request] 100GbE Intel Wired LAN Driver Updates 2017-10-02 Jeff Kirsher
` (8 preceding siblings ...)
2017-10-02 15:42 ` [net-next 09/13] fm10k: simplify reading PFVFLRE register Jeff Kirsher
@ 2017-10-02 15:42 ` Jeff Kirsher
2017-10-02 15:42 ` [net-next 11/13] fm10k: avoid divide by zero in rare cases when device is resetting Jeff Kirsher
` (3 subsequent siblings)
13 siblings, 0 replies; 15+ messages in thread
From: Jeff Kirsher @ 2017-10-02 15:42 UTC (permalink / raw)
To: davem; +Cc: Jacob Keller, netdev, nhorman, sassmann, jogreene, Jeff Kirsher
From: Jacob Keller <jacob.e.keller@intel.com>
We've always had a really weird looping construction for resetting VFs.
We read the VFLRE register and reset the VF if the corresponding bit is
set, which makes sense. However we loop continuously until we no longer
have any bits left unset. At first this makes sense, as a sort of "keep
trying until we succeed" concept.
Unfortunately this causes a problem if we happen to surprise remove
while this code is executing, because in this case we'll always read all
1s for the VFLRE register. This results in a hard lockup on the CPU
because the loop will never terminate.
Because our own reset function will clear the VFLR event register
always, (except when we've lost PCIe link obviously) there is no real
reason to loop. In practice, we'll loop over once and find that no VFs
are pending anymore.
Lets just check once. Since we're clear the notification when we reset
there's no benefit to the loop. Additionally, there shouldn't be a race
as future VLFRE events should trigger an interrupt. Additionally, we
didn't warn or do anything in the looped case anyways.
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_iov.c | 24 +++++++++++-------------
1 file changed, 11 insertions(+), 13 deletions(-)
diff --git a/drivers/net/ethernet/intel/fm10k/fm10k_iov.c b/drivers/net/ethernet/intel/fm10k/fm10k_iov.c
index dfc88a463735..03897720bf0b 100644
--- a/drivers/net/ethernet/intel/fm10k/fm10k_iov.c
+++ b/drivers/net/ethernet/intel/fm10k/fm10k_iov.c
@@ -66,23 +66,21 @@ s32 fm10k_iov_event(struct fm10k_intfc *interface)
goto read_unlock;
/* read VFLRE to determine if any VFs have been reset */
- do {
- vflre = fm10k_read_reg(hw, FM10K_PFVFLRE(1));
- vflre <<= 32;
- vflre |= fm10k_read_reg(hw, FM10K_PFVFLRE(0));
+ vflre = fm10k_read_reg(hw, FM10K_PFVFLRE(1));
+ vflre <<= 32;
+ vflre |= fm10k_read_reg(hw, FM10K_PFVFLRE(0));
- i = iov_data->num_vfs;
+ i = iov_data->num_vfs;
- for (vflre <<= 64 - i; vflre && i--; vflre += vflre) {
- struct fm10k_vf_info *vf_info = &iov_data->vf_info[i];
+ for (vflre <<= 64 - i; vflre && i--; vflre += vflre) {
+ struct fm10k_vf_info *vf_info = &iov_data->vf_info[i];
- if (vflre >= 0)
- continue;
+ if (vflre >= 0)
+ continue;
- hw->iov.ops.reset_resources(hw, vf_info);
- vf_info->mbx.ops.connect(hw, &vf_info->mbx);
- }
- } while (i != iov_data->num_vfs);
+ hw->iov.ops.reset_resources(hw, vf_info);
+ vf_info->mbx.ops.connect(hw, &vf_info->mbx);
+ }
read_unlock:
rcu_read_unlock();
--
2.14.2
^ permalink raw reply related [flat|nested] 15+ messages in thread* [net-next 11/13] fm10k: avoid divide by zero in rare cases when device is resetting
2017-10-02 15:42 [net-next 00/13][pull request] 100GbE Intel Wired LAN Driver Updates 2017-10-02 Jeff Kirsher
` (9 preceding siblings ...)
2017-10-02 15:42 ` [net-next 10/13] fm10k: don't loop while resetting VFs due to VFLR event Jeff Kirsher
@ 2017-10-02 15:42 ` Jeff Kirsher
2017-10-02 15:42 ` [net-next 12/13] fm10k: move fm10k_prepare_for_reset and fm10k_handle_reset Jeff Kirsher
` (2 subsequent siblings)
13 siblings, 0 replies; 15+ messages in thread
From: Jeff Kirsher @ 2017-10-02 15:42 UTC (permalink / raw)
To: davem; +Cc: Jacob Keller, netdev, nhorman, sassmann, jogreene, Jeff Kirsher
From: Jacob Keller <jacob.e.keller@intel.com>
It is possible that under rare circumstances the device is undergoing
a reset, such as when a PFLR occurs, and the device may be transmitting
simultaneously. In this case, we might attempt to divide by zero when
finding the proper r_idx. Instead, lets read the num_tx_queues once,
and make sure it's non-zero.
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 | 8 ++++++--
1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/drivers/net/ethernet/intel/fm10k/fm10k_netdev.c b/drivers/net/ethernet/intel/fm10k/fm10k_netdev.c
index e69d49d91d67..77d495fedced 100644
--- a/drivers/net/ethernet/intel/fm10k/fm10k_netdev.c
+++ b/drivers/net/ethernet/intel/fm10k/fm10k_netdev.c
@@ -643,9 +643,13 @@ int fm10k_close(struct net_device *netdev)
static netdev_tx_t fm10k_xmit_frame(struct sk_buff *skb, struct net_device *dev)
{
struct fm10k_intfc *interface = netdev_priv(dev);
+ int num_tx_queues = READ_ONCE(interface->num_tx_queues);
unsigned int r_idx = skb->queue_mapping;
int err;
+ if (!num_tx_queues)
+ return NETDEV_TX_BUSY;
+
if ((skb->protocol == htons(ETH_P_8021Q)) &&
!skb_vlan_tag_present(skb)) {
/* FM10K only supports hardware tagging, any tags in frame
@@ -698,8 +702,8 @@ static netdev_tx_t fm10k_xmit_frame(struct sk_buff *skb, struct net_device *dev)
__skb_put(skb, pad_len);
}
- if (r_idx >= interface->num_tx_queues)
- r_idx %= interface->num_tx_queues;
+ if (r_idx >= num_tx_queues)
+ r_idx %= num_tx_queues;
err = fm10k_xmit_frame_ring(skb, interface->tx_ring[r_idx]);
--
2.14.2
^ permalink raw reply related [flat|nested] 15+ messages in thread* [net-next 12/13] fm10k: move fm10k_prepare_for_reset and fm10k_handle_reset
2017-10-02 15:42 [net-next 00/13][pull request] 100GbE Intel Wired LAN Driver Updates 2017-10-02 Jeff Kirsher
` (10 preceding siblings ...)
2017-10-02 15:42 ` [net-next 11/13] fm10k: avoid divide by zero in rare cases when device is resetting Jeff Kirsher
@ 2017-10-02 15:42 ` Jeff Kirsher
2017-10-02 15:42 ` [net-next 13/13] fm10k: prevent race condition of __FM10K_SERVICE_SCHED Jeff Kirsher
2017-10-02 18:59 ` [net-next 00/13][pull request] 100GbE Intel Wired LAN Driver Updates 2017-10-02 David Miller
13 siblings, 0 replies; 15+ messages in thread
From: Jeff Kirsher @ 2017-10-02 15:42 UTC (permalink / raw)
To: davem; +Cc: Jacob Keller, netdev, nhorman, sassmann, jogreene, Jeff Kirsher
From: Jacob Keller <jacob.e.keller@intel.com>
A future patch needs these functions defined earlier in the file. Move
them closer to above where they will be called.
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_pci.c | 58 ++++++++++++++--------------
1 file changed, 29 insertions(+), 29 deletions(-)
diff --git a/drivers/net/ethernet/intel/fm10k/fm10k_pci.c b/drivers/net/ethernet/intel/fm10k/fm10k_pci.c
index 6c2c4bffaedf..41335154d6b1 100644
--- a/drivers/net/ethernet/intel/fm10k/fm10k_pci.c
+++ b/drivers/net/ethernet/intel/fm10k/fm10k_pci.c
@@ -132,35 +132,6 @@ static void fm10k_service_timer(unsigned long data)
fm10k_service_event_schedule(interface);
}
-static void fm10k_detach_subtask(struct fm10k_intfc *interface)
-{
- struct net_device *netdev = interface->netdev;
- u32 __iomem *hw_addr;
- u32 value;
-
- /* do nothing if device is still present or hw_addr is set */
- if (netif_device_present(netdev) || interface->hw.hw_addr)
- return;
-
- /* check the real address space to see if we've recovered */
- hw_addr = READ_ONCE(interface->uc_addr);
- value = readl(hw_addr);
- if (~value) {
- interface->hw.hw_addr = interface->uc_addr;
- netif_device_attach(netdev);
- set_bit(FM10K_FLAG_RESET_REQUESTED, interface->flags);
- netdev_warn(netdev, "PCIe link restored, device now attached\n");
- return;
- }
-
- rtnl_lock();
-
- if (netif_running(netdev))
- dev_close(netdev);
-
- rtnl_unlock();
-}
-
static void fm10k_prepare_for_reset(struct fm10k_intfc *interface)
{
struct net_device *netdev = interface->netdev;
@@ -270,6 +241,35 @@ static int fm10k_handle_reset(struct fm10k_intfc *interface)
return err;
}
+static void fm10k_detach_subtask(struct fm10k_intfc *interface)
+{
+ struct net_device *netdev = interface->netdev;
+ u32 __iomem *hw_addr;
+ u32 value;
+
+ /* do nothing if device is still present or hw_addr is set */
+ if (netif_device_present(netdev) || interface->hw.hw_addr)
+ return;
+
+ /* check the real address space to see if we've recovered */
+ hw_addr = READ_ONCE(interface->uc_addr);
+ value = readl(hw_addr);
+ if (~value) {
+ interface->hw.hw_addr = interface->uc_addr;
+ netif_device_attach(netdev);
+ set_bit(FM10K_FLAG_RESET_REQUESTED, interface->flags);
+ netdev_warn(netdev, "PCIe link restored, device now attached\n");
+ return;
+ }
+
+ rtnl_lock();
+
+ if (netif_running(netdev))
+ dev_close(netdev);
+
+ rtnl_unlock();
+}
+
static void fm10k_reinit(struct fm10k_intfc *interface)
{
int err;
--
2.14.2
^ permalink raw reply related [flat|nested] 15+ messages in thread* [net-next 13/13] fm10k: prevent race condition of __FM10K_SERVICE_SCHED
2017-10-02 15:42 [net-next 00/13][pull request] 100GbE Intel Wired LAN Driver Updates 2017-10-02 Jeff Kirsher
` (11 preceding siblings ...)
2017-10-02 15:42 ` [net-next 12/13] fm10k: move fm10k_prepare_for_reset and fm10k_handle_reset Jeff Kirsher
@ 2017-10-02 15:42 ` Jeff Kirsher
2017-10-02 18:59 ` [net-next 00/13][pull request] 100GbE Intel Wired LAN Driver Updates 2017-10-02 David Miller
13 siblings, 0 replies; 15+ messages in thread
From: Jeff Kirsher @ 2017-10-02 15:42 UTC (permalink / raw)
To: davem; +Cc: Jacob Keller, netdev, nhorman, sassmann, jogreene, Jeff Kirsher
From: Jacob Keller <jacob.e.keller@intel.com>
Although very unlikely, it is possible that cancel_work_sync() may stop
the service_task before it actually started. In this case, the
__FM10K_SERVICE_SCHED bit will never be cleared. This results in the
service task being unable to reschedule in the future. Add a helper
function which sets the service disable bit, waits for the service task
to stop and clears the schedule bit, thus avoiding the race condition.
We know the schedule bit is safe to clear because the cancel_work_sync()
guarantees the service task is not running.
Add a helper function also to restart the service task, for symmetry.
This is not strictly needed but helps the mental model of how to stop
and start the service task.
This race could only happen in fm10k_suspend/fm10k_resume as this is the
only place where the service task is actually restarted. Thus,
suspend/resume testing would be ideal. However, note that the chance of
this happening is very slim as the service event is scheduled for
immediate execution, and you would have to trigger a suspend at almost
the exact same time as the service task was scheduled.
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_pci.c | 32 ++++++++++++++++++++++------
1 file changed, 25 insertions(+), 7 deletions(-)
diff --git a/drivers/net/ethernet/intel/fm10k/fm10k_pci.c b/drivers/net/ethernet/intel/fm10k/fm10k_pci.c
index 41335154d6b1..9575f7c1862d 100644
--- a/drivers/net/ethernet/intel/fm10k/fm10k_pci.c
+++ b/drivers/net/ethernet/intel/fm10k/fm10k_pci.c
@@ -118,6 +118,27 @@ static void fm10k_service_event_complete(struct fm10k_intfc *interface)
fm10k_service_event_schedule(interface);
}
+static void fm10k_stop_service_event(struct fm10k_intfc *interface)
+{
+ set_bit(__FM10K_SERVICE_DISABLE, interface->state);
+ cancel_work_sync(&interface->service_task);
+
+ /* It's possible that cancel_work_sync stopped the service task from
+ * running before it could actually start. In this case the
+ * __FM10K_SERVICE_SCHED bit will never be cleared. Since we know that
+ * the service task cannot be running at this point, we need to clear
+ * the scheduled bit, as otherwise the service task may never be
+ * restarted.
+ */
+ clear_bit(__FM10K_SERVICE_SCHED, interface->state);
+}
+
+static void fm10k_start_service_event(struct fm10k_intfc *interface)
+{
+ clear_bit(__FM10K_SERVICE_DISABLE, interface->state);
+ fm10k_service_event_schedule(interface);
+}
+
/**
* fm10k_service_timer - Timer Call-back
* @data: pointer to interface cast into an unsigned long
@@ -2116,8 +2137,7 @@ static void fm10k_remove(struct pci_dev *pdev)
del_timer_sync(&interface->service_timer);
- set_bit(__FM10K_SERVICE_DISABLE, interface->state);
- cancel_work_sync(&interface->service_task);
+ fm10k_stop_service_event(interface);
/* free netdev, this may bounce the interrupts due to setup_tc */
if (netdev->reg_state == NETREG_REGISTERED)
@@ -2155,8 +2175,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);
- cancel_work_sync(&interface->service_task);
+ fm10k_stop_service_event(interface);
fm10k_prepare_for_reset(interface);
}
@@ -2183,9 +2202,8 @@ static int fm10k_handle_resume(struct fm10k_intfc *interface)
interface->link_down_event = jiffies + (HZ);
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);
- fm10k_service_event_schedule(interface);
+ /* restart the service task */
+ fm10k_start_service_event(interface);
return err;
}
--
2.14.2
^ permalink raw reply related [flat|nested] 15+ messages in thread* Re: [net-next 00/13][pull request] 100GbE Intel Wired LAN Driver Updates 2017-10-02
2017-10-02 15:42 [net-next 00/13][pull request] 100GbE Intel Wired LAN Driver Updates 2017-10-02 Jeff Kirsher
` (12 preceding siblings ...)
2017-10-02 15:42 ` [net-next 13/13] fm10k: prevent race condition of __FM10K_SERVICE_SCHED Jeff Kirsher
@ 2017-10-02 18:59 ` David Miller
13 siblings, 0 replies; 15+ messages in thread
From: David Miller @ 2017-10-02 18:59 UTC (permalink / raw)
To: jeffrey.t.kirsher; +Cc: netdev, nhorman, sassmann, jogreene
From: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
Date: Mon, 2 Oct 2017 08:42:23 -0700
> This series contains updates to fm10k only.
>
> Jake provides all but one of the changes in this series. Most are small
> fixes, starting with ensuring prompt transmission of messages queued up
> after each VF message is received and handled. Fix a possible race
> condition between the watchdog task and the processing of mailbox
> messages by just checking whether the mailbox is still open. Fix a
> couple of GCC v7 warnings, including misspelled "fall through" comments
> and warnings about possible truncation of calls to snprintf(). Cleaned
> up a convoluted bitshift and read for the PFVFLRE register. Fixed a
> potential divide by zero when finding the proper r_idx.
>
> Markus Elfring fixes an issue which was found using Coccinelle, where
> we should have been using seq_putc() instead of seq_puts().
>
> The following are changes since commit 0929567a7a2dab8455a7313956973ff0d339709a:
> samples/bpf: fix warnings in xdp_monitor_user
> and are available in the git repository at:
> git://git.kernel.org/pub/scm/linux/kernel/git/jkirsher/next-queue 100GbE
Pulled, thanks Jeff.
^ permalink raw reply [flat|nested] 15+ messages in thread