* [PATCH net-next 1/6] ionic: add timeout error checking for queue disable
2020-03-20 2:31 [PATCH net-next 0/6] ionic error recovery fixes Shannon Nelson
@ 2020-03-20 2:31 ` Shannon Nelson
2020-03-20 2:31 ` [PATCH net-next 2/6] ionic: leave dev cmd request contents alone on FW timeout Shannon Nelson
` (6 subsequent siblings)
7 siblings, 0 replies; 13+ messages in thread
From: Shannon Nelson @ 2020-03-20 2:31 UTC (permalink / raw)
To: netdev, davem; +Cc: Shannon Nelson
Short circuit the cleanup if we get a timeout error from
ionic_qcq_disable() so as to not have to wait too long
on shutdown when we already know the FW is not responding.
Fixes: 0f3154e6bcb3 ("ionic: Add Tx and Rx handling")
Signed-off-by: Shannon Nelson <snelson@pensando.io>
---
.../net/ethernet/pensando/ionic/ionic_lif.c | 20 ++++++++++++++-----
1 file changed, 15 insertions(+), 5 deletions(-)
diff --git a/drivers/net/ethernet/pensando/ionic/ionic_lif.c b/drivers/net/ethernet/pensando/ionic/ionic_lif.c
index 12e3823b0bc1..c62d3d01d5aa 100644
--- a/drivers/net/ethernet/pensando/ionic/ionic_lif.c
+++ b/drivers/net/ethernet/pensando/ionic/ionic_lif.c
@@ -1425,10 +1425,15 @@ static void ionic_lif_rss_deinit(struct ionic_lif *lif)
static void ionic_txrx_disable(struct ionic_lif *lif)
{
unsigned int i;
+ int err;
for (i = 0; i < lif->nxqs; i++) {
- ionic_qcq_disable(lif->txqcqs[i].qcq);
- ionic_qcq_disable(lif->rxqcqs[i].qcq);
+ err = ionic_qcq_disable(lif->txqcqs[i].qcq);
+ if (err == -ETIMEDOUT)
+ break;
+ err = ionic_qcq_disable(lif->rxqcqs[i].qcq);
+ if (err == -ETIMEDOUT)
+ break;
}
}
@@ -1552,7 +1557,8 @@ static int ionic_txrx_enable(struct ionic_lif *lif)
ionic_rx_fill(&lif->rxqcqs[i].qcq->q);
err = ionic_qcq_enable(lif->rxqcqs[i].qcq);
if (err) {
- ionic_qcq_disable(lif->txqcqs[i].qcq);
+ if (err != -ETIMEDOUT)
+ ionic_qcq_disable(lif->txqcqs[i].qcq);
goto err_out;
}
}
@@ -1561,8 +1567,12 @@ static int ionic_txrx_enable(struct ionic_lif *lif)
err_out:
while (i--) {
- ionic_qcq_disable(lif->rxqcqs[i].qcq);
- ionic_qcq_disable(lif->txqcqs[i].qcq);
+ err = ionic_qcq_disable(lif->rxqcqs[i].qcq);
+ if (err == -ETIMEDOUT)
+ break;
+ err = ionic_qcq_disable(lif->txqcqs[i].qcq);
+ if (err == -ETIMEDOUT)
+ break;
}
return err;
--
2.17.1
^ permalink raw reply related [flat|nested] 13+ messages in thread* [PATCH net-next 2/6] ionic: leave dev cmd request contents alone on FW timeout
2020-03-20 2:31 [PATCH net-next 0/6] ionic error recovery fixes Shannon Nelson
2020-03-20 2:31 ` [PATCH net-next 1/6] ionic: add timeout error checking for queue disable Shannon Nelson
@ 2020-03-20 2:31 ` Shannon Nelson
2020-03-20 2:31 ` [PATCH net-next 3/6] ionic: only save good lif dentry Shannon Nelson
` (5 subsequent siblings)
7 siblings, 0 replies; 13+ messages in thread
From: Shannon Nelson @ 2020-03-20 2:31 UTC (permalink / raw)
To: netdev, davem; +Cc: Shannon Nelson
It is possible (but unlikely) that FW was busy and missed a heartbeat
check but is still alive and will process the pending request, so don't
clean the dev_cmd in this case. This occasionally occurs when working
with a card that is supporting many devices and is trying to shut them
all down at once, but still wants to see that last LIF disable request.
Fixes: 97ca486592c0 ("ionic: add heartbeat check")
Signed-off-by: Shannon Nelson <snelson@pensando.io>
---
drivers/net/ethernet/pensando/ionic/ionic_main.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/pensando/ionic/ionic_main.c b/drivers/net/ethernet/pensando/ionic/ionic_main.c
index c5e3d7639f7e..a0dc100b12e6 100644
--- a/drivers/net/ethernet/pensando/ionic/ionic_main.c
+++ b/drivers/net/ethernet/pensando/ionic/ionic_main.c
@@ -360,7 +360,10 @@ int ionic_dev_cmd_wait(struct ionic *ionic, unsigned long max_seconds)
done, duration / HZ, duration);
if (!done && hb) {
- ionic_dev_cmd_clean(ionic);
+ /* It is possible (but unlikely) that FW was busy and missed a
+ * heartbeat check but is still alive and will process this
+ * request, so don't clean the dev_cmd in this case.
+ */
dev_warn(ionic->dev, "DEVCMD %s (%d) failed - FW halted\n",
ionic_opcode_to_str(opcode), opcode);
return -ENXIO;
--
2.17.1
^ permalink raw reply related [flat|nested] 13+ messages in thread* [PATCH net-next 3/6] ionic: only save good lif dentry
2020-03-20 2:31 [PATCH net-next 0/6] ionic error recovery fixes Shannon Nelson
2020-03-20 2:31 ` [PATCH net-next 1/6] ionic: add timeout error checking for queue disable Shannon Nelson
2020-03-20 2:31 ` [PATCH net-next 2/6] ionic: leave dev cmd request contents alone on FW timeout Shannon Nelson
@ 2020-03-20 2:31 ` Shannon Nelson
2020-03-20 2:31 ` [PATCH net-next 4/6] ionic: ignore eexist on rx filter add Shannon Nelson
` (4 subsequent siblings)
7 siblings, 0 replies; 13+ messages in thread
From: Shannon Nelson @ 2020-03-20 2:31 UTC (permalink / raw)
To: netdev, davem; +Cc: Shannon Nelson
Don't save the lif->dentry until we know we have
a good value.
Fixes: 1a58e196467f ("ionic: Add basic lif support")
Signed-off-by: Shannon Nelson <snelson@pensando.io>
---
drivers/net/ethernet/pensando/ionic/ionic_debugfs.c | 8 +++++++-
1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/pensando/ionic/ionic_debugfs.c b/drivers/net/ethernet/pensando/ionic/ionic_debugfs.c
index bc03cecf80cc..5f8fc58d42b3 100644
--- a/drivers/net/ethernet/pensando/ionic/ionic_debugfs.c
+++ b/drivers/net/ethernet/pensando/ionic/ionic_debugfs.c
@@ -228,7 +228,13 @@ DEFINE_SHOW_ATTRIBUTE(netdev);
void ionic_debugfs_add_lif(struct ionic_lif *lif)
{
- lif->dentry = debugfs_create_dir(lif->name, lif->ionic->dentry);
+ struct dentry *lif_dentry;
+
+ lif_dentry = debugfs_create_dir(lif->name, lif->ionic->dentry);
+ if (IS_ERR_OR_NULL(lif_dentry))
+ return;
+ lif->dentry = lif_dentry;
+
debugfs_create_file("netdev", 0400, lif->dentry,
lif->netdev, &netdev_fops);
}
--
2.17.1
^ permalink raw reply related [flat|nested] 13+ messages in thread* [PATCH net-next 4/6] ionic: ignore eexist on rx filter add
2020-03-20 2:31 [PATCH net-next 0/6] ionic error recovery fixes Shannon Nelson
` (2 preceding siblings ...)
2020-03-20 2:31 ` [PATCH net-next 3/6] ionic: only save good lif dentry Shannon Nelson
@ 2020-03-20 2:31 ` Shannon Nelson
2020-03-20 3:43 ` Jakub Kicinski
2020-03-20 2:31 ` [PATCH net-next 5/6] ionic: clean irq affinity on queue deinit Shannon Nelson
` (3 subsequent siblings)
7 siblings, 1 reply; 13+ messages in thread
From: Shannon Nelson @ 2020-03-20 2:31 UTC (permalink / raw)
To: netdev, davem; +Cc: Shannon Nelson
Don't worry if the rx filter add firmware request fails on
EEXIST, at least we know the filter is there. Same for
the delete request, at least we know it isn't there.
Fixes: 2a654540be10 ("ionic: Add Rx filter and rx_mode ndo support")
Signed-off-by: Shannon Nelson <snelson@pensando.io>
---
drivers/net/ethernet/pensando/ionic/ionic_lif.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/net/ethernet/pensando/ionic/ionic_lif.c b/drivers/net/ethernet/pensando/ionic/ionic_lif.c
index c62d3d01d5aa..ea44f510cb76 100644
--- a/drivers/net/ethernet/pensando/ionic/ionic_lif.c
+++ b/drivers/net/ethernet/pensando/ionic/ionic_lif.c
@@ -832,7 +832,7 @@ static int ionic_lif_addr_add(struct ionic_lif *lif, const u8 *addr)
memcpy(ctx.cmd.rx_filter_add.mac.addr, addr, ETH_ALEN);
err = ionic_adminq_post_wait(lif, &ctx);
- if (err)
+ if (err && err != -EEXIST)
return err;
return ionic_rx_filter_save(lif, 0, IONIC_RXQ_INDEX_ANY, 0, &ctx);
@@ -862,7 +862,7 @@ static int ionic_lif_addr_del(struct ionic_lif *lif, const u8 *addr)
spin_unlock_bh(&lif->rx_filters.lock);
err = ionic_adminq_post_wait(lif, &ctx);
- if (err)
+ if (err && err != -EEXIST)
return err;
netdev_dbg(lif->netdev, "rx_filter del ADDR %pM (id %d)\n", addr,
--
2.17.1
^ permalink raw reply related [flat|nested] 13+ messages in thread* Re: [PATCH net-next 4/6] ionic: ignore eexist on rx filter add
2020-03-20 2:31 ` [PATCH net-next 4/6] ionic: ignore eexist on rx filter add Shannon Nelson
@ 2020-03-20 3:43 ` Jakub Kicinski
2020-03-20 5:21 ` Shannon Nelson
0 siblings, 1 reply; 13+ messages in thread
From: Jakub Kicinski @ 2020-03-20 3:43 UTC (permalink / raw)
To: Shannon Nelson; +Cc: netdev, davem
On Thu, 19 Mar 2020 19:31:51 -0700 Shannon Nelson wrote:
> Don't worry if the rx filter add firmware request fails on
> EEXIST, at least we know the filter is there. Same for
> the delete request, at least we know it isn't there.
>
> Fixes: 2a654540be10 ("ionic: Add Rx filter and rx_mode ndo support")
> Signed-off-by: Shannon Nelson <snelson@pensando.io>
Why could the filter be there? Seems like the FW shouldn't have filters
the driver didn't add, could a flush/reset command help to start from
clean state?
Just curious.
^ permalink raw reply [flat|nested] 13+ messages in thread* Re: [PATCH net-next 4/6] ionic: ignore eexist on rx filter add
2020-03-20 3:43 ` Jakub Kicinski
@ 2020-03-20 5:21 ` Shannon Nelson
2020-03-20 17:16 ` Jakub Kicinski
0 siblings, 1 reply; 13+ messages in thread
From: Shannon Nelson @ 2020-03-20 5:21 UTC (permalink / raw)
To: Jakub Kicinski; +Cc: netdev, davem
On 3/19/20 8:43 PM, Jakub Kicinski wrote:
> On Thu, 19 Mar 2020 19:31:51 -0700 Shannon Nelson wrote:
>> Don't worry if the rx filter add firmware request fails on
>> EEXIST, at least we know the filter is there. Same for
>> the delete request, at least we know it isn't there.
>>
>> Fixes: 2a654540be10 ("ionic: Add Rx filter and rx_mode ndo support")
>> Signed-off-by: Shannon Nelson <snelson@pensando.io>
> Why could the filter be there? Seems like the FW shouldn't have filters
> the driver didn't add, could a flush/reset command help to start from
> clean state?
>
> Just curious.
Because there are use cases where the device is configured by an
external centralized agent and may have already stuck the appropriate
filters into the its list.
sln
^ permalink raw reply [flat|nested] 13+ messages in thread* Re: [PATCH net-next 4/6] ionic: ignore eexist on rx filter add
2020-03-20 5:21 ` Shannon Nelson
@ 2020-03-20 17:16 ` Jakub Kicinski
0 siblings, 0 replies; 13+ messages in thread
From: Jakub Kicinski @ 2020-03-20 17:16 UTC (permalink / raw)
To: Shannon Nelson; +Cc: netdev, davem
On Thu, 19 Mar 2020 22:21:35 -0700 Shannon Nelson wrote:
> On 3/19/20 8:43 PM, Jakub Kicinski wrote:
> > On Thu, 19 Mar 2020 19:31:51 -0700 Shannon Nelson wrote:
> >> Don't worry if the rx filter add firmware request fails on
> >> EEXIST, at least we know the filter is there. Same for
> >> the delete request, at least we know it isn't there.
> >>
> >> Fixes: 2a654540be10 ("ionic: Add Rx filter and rx_mode ndo support")
> >> Signed-off-by: Shannon Nelson <snelson@pensando.io>
> > Why could the filter be there? Seems like the FW shouldn't have filters
> > the driver didn't add, could a flush/reset command help to start from
> > clean state?
> >
> > Just curious.
> Because there are use cases where the device is configured by an
> external centralized agent and may have already stuck the appropriate
> filters into the its list.
Thanks, seems a little leaky for the host to be able to probe the state
installed by the agent, but it does explain the need.
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH net-next 5/6] ionic: clean irq affinity on queue deinit
2020-03-20 2:31 [PATCH net-next 0/6] ionic error recovery fixes Shannon Nelson
` (3 preceding siblings ...)
2020-03-20 2:31 ` [PATCH net-next 4/6] ionic: ignore eexist on rx filter add Shannon Nelson
@ 2020-03-20 2:31 ` Shannon Nelson
2020-03-20 2:31 ` [PATCH net-next 6/6] ionic: check for NULL structs on teardown Shannon Nelson
` (2 subsequent siblings)
7 siblings, 0 replies; 13+ messages in thread
From: Shannon Nelson @ 2020-03-20 2:31 UTC (permalink / raw)
To: netdev, davem; +Cc: Shannon Nelson
Add a little more cleanup when tearing down the queues.
Fixes: 1d062b7b6f64 ("ionic: Add basic adminq support")
Signed-off-by: Shannon Nelson <snelson@pensando.io>
---
drivers/net/ethernet/pensando/ionic/ionic_lif.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/net/ethernet/pensando/ionic/ionic_lif.c b/drivers/net/ethernet/pensando/ionic/ionic_lif.c
index ea44f510cb76..490f79c82bf1 100644
--- a/drivers/net/ethernet/pensando/ionic/ionic_lif.c
+++ b/drivers/net/ethernet/pensando/ionic/ionic_lif.c
@@ -275,8 +275,10 @@ static void ionic_lif_qcq_deinit(struct ionic_lif *lif, struct ionic_qcq *qcq)
if (qcq->flags & IONIC_QCQ_F_INTR) {
ionic_intr_mask(idev->intr_ctrl, qcq->intr.index,
IONIC_INTR_MASK_SET);
+ irq_set_affinity_hint(qcq->intr.vector, NULL);
devm_free_irq(dev, qcq->intr.vector, &qcq->napi);
netif_napi_del(&qcq->napi);
+ qcq->intr.vector = 0;
}
qcq->flags &= ~IONIC_QCQ_F_INITED;
--
2.17.1
^ permalink raw reply related [flat|nested] 13+ messages in thread* [PATCH net-next 6/6] ionic: check for NULL structs on teardown
2020-03-20 2:31 [PATCH net-next 0/6] ionic error recovery fixes Shannon Nelson
` (4 preceding siblings ...)
2020-03-20 2:31 ` [PATCH net-next 5/6] ionic: clean irq affinity on queue deinit Shannon Nelson
@ 2020-03-20 2:31 ` Shannon Nelson
2020-03-20 3:46 ` [PATCH net-next 0/6] ionic error recovery fixes Jakub Kicinski
2020-03-22 2:56 ` David Miller
7 siblings, 0 replies; 13+ messages in thread
From: Shannon Nelson @ 2020-03-20 2:31 UTC (permalink / raw)
To: netdev, davem; +Cc: Shannon Nelson
Make sure the queue structs exist before trying to tear
them down to make for safer error recovery.
Fixes: 0f3154e6bcb3 ("ionic: Add Tx and Rx handling")
Signed-off-by: Shannon Nelson <snelson@pensando.io>
---
.../net/ethernet/pensando/ionic/ionic_lif.c | 26 ++++++++++---------
.../net/ethernet/pensando/ionic/ionic_main.c | 7 ++++-
2 files changed, 20 insertions(+), 13 deletions(-)
diff --git a/drivers/net/ethernet/pensando/ionic/ionic_lif.c b/drivers/net/ethernet/pensando/ionic/ionic_lif.c
index 490f79c82bf1..8b442eb010a2 100644
--- a/drivers/net/ethernet/pensando/ionic/ionic_lif.c
+++ b/drivers/net/ethernet/pensando/ionic/ionic_lif.c
@@ -320,19 +320,21 @@ static void ionic_qcqs_free(struct ionic_lif *lif)
lif->adminqcq = NULL;
}
- for (i = 0; i < lif->nxqs; i++)
- if (lif->rxqcqs[i].stats)
- devm_kfree(dev, lif->rxqcqs[i].stats);
-
- devm_kfree(dev, lif->rxqcqs);
- lif->rxqcqs = NULL;
-
- for (i = 0; i < lif->nxqs; i++)
- if (lif->txqcqs[i].stats)
- devm_kfree(dev, lif->txqcqs[i].stats);
+ if (lif->rxqcqs) {
+ for (i = 0; i < lif->nxqs; i++)
+ if (lif->rxqcqs[i].stats)
+ devm_kfree(dev, lif->rxqcqs[i].stats);
+ devm_kfree(dev, lif->rxqcqs);
+ lif->rxqcqs = NULL;
+ }
- devm_kfree(dev, lif->txqcqs);
- lif->txqcqs = NULL;
+ if (lif->txqcqs) {
+ for (i = 0; i < lif->nxqs; i++)
+ if (lif->txqcqs[i].stats)
+ devm_kfree(dev, lif->txqcqs[i].stats);
+ devm_kfree(dev, lif->txqcqs);
+ lif->txqcqs = NULL;
+ }
}
static void ionic_link_qcq_interrupts(struct ionic_qcq *src_qcq,
diff --git a/drivers/net/ethernet/pensando/ionic/ionic_main.c b/drivers/net/ethernet/pensando/ionic/ionic_main.c
index a0dc100b12e6..c16dbbe54bf7 100644
--- a/drivers/net/ethernet/pensando/ionic/ionic_main.c
+++ b/drivers/net/ethernet/pensando/ionic/ionic_main.c
@@ -243,11 +243,16 @@ static void ionic_adminq_cb(struct ionic_queue *q,
static int ionic_adminq_post(struct ionic_lif *lif, struct ionic_admin_ctx *ctx)
{
- struct ionic_queue *adminq = &lif->adminqcq->q;
+ struct ionic_queue *adminq;
int err = 0;
WARN_ON(in_interrupt());
+ if (!lif->adminqcq)
+ return -EIO;
+
+ adminq = &lif->adminqcq->q;
+
spin_lock(&lif->adminq_lock);
if (!ionic_q_has_space(adminq, 1)) {
err = -ENOSPC;
--
2.17.1
^ permalink raw reply related [flat|nested] 13+ messages in thread* Re: [PATCH net-next 0/6] ionic error recovery fixes
2020-03-20 2:31 [PATCH net-next 0/6] ionic error recovery fixes Shannon Nelson
` (5 preceding siblings ...)
2020-03-20 2:31 ` [PATCH net-next 6/6] ionic: check for NULL structs on teardown Shannon Nelson
@ 2020-03-20 3:46 ` Jakub Kicinski
2020-03-20 5:21 ` Shannon Nelson
2020-03-22 2:56 ` David Miller
7 siblings, 1 reply; 13+ messages in thread
From: Jakub Kicinski @ 2020-03-20 3:46 UTC (permalink / raw)
To: Shannon Nelson; +Cc: netdev, davem
On Thu, 19 Mar 2020 19:31:47 -0700 Shannon Nelson wrote:
> These are a few little patches to make error recovery a little
> more safe and successful.
Patches looks good to me, FWIW. Thanks for dropping the controversial
one. I think this should have been v2 since we seen most if not all of
these.
I'm not sure why most of them have a Fixes tag, though. The
AUTOSEL bot is quite likely to pull them into the stable trees once
they land. Is this some intentional strategy on your part?
^ permalink raw reply [flat|nested] 13+ messages in thread* Re: [PATCH net-next 0/6] ionic error recovery fixes
2020-03-20 3:46 ` [PATCH net-next 0/6] ionic error recovery fixes Jakub Kicinski
@ 2020-03-20 5:21 ` Shannon Nelson
0 siblings, 0 replies; 13+ messages in thread
From: Shannon Nelson @ 2020-03-20 5:21 UTC (permalink / raw)
To: Jakub Kicinski; +Cc: netdev, davem
On 3/19/20 8:46 PM, Jakub Kicinski wrote:
> On Thu, 19 Mar 2020 19:31:47 -0700 Shannon Nelson wrote:
>> These are a few little patches to make error recovery a little
>> more safe and successful.
> Patches looks good to me, FWIW. Thanks for dropping the controversial
> one. I think this should have been v2 since we seen most if not all of
> these.
I suppose v2 would have been okay, but that patchset was specifically
labeled as for firmware upgrade, and these are "merely" supporting
fixes. It didn't seem right to keep the firmware upgrade in the name,
yet it didn't seem right to call something with a new name 'v2'. I
suppose it would have been good to call out in the cover letter that
these were refugee patches from a previous patchset.
> I'm not sure why most of them have a Fixes tag, though. The
> AUTOSEL bot is quite likely to pull them into the stable trees once
> they land. Is this some intentional strategy on your part?
Because in my last patchset I had similar types of fixes without the
Fixes tag and got reminded that I should have the tag. These do fix
"issues in a previous commit" as suggested in the
submitting-patches.rst, so it seemed appropriate. They aren't high
priority issues, else I would have targeted them at net rather than
net-next. If accepted, they'll get there soon enough.
Meanwhile, thanks for the review time :-)
Cheers,
sln
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH net-next 0/6] ionic error recovery fixes
2020-03-20 2:31 [PATCH net-next 0/6] ionic error recovery fixes Shannon Nelson
` (6 preceding siblings ...)
2020-03-20 3:46 ` [PATCH net-next 0/6] ionic error recovery fixes Jakub Kicinski
@ 2020-03-22 2:56 ` David Miller
7 siblings, 0 replies; 13+ messages in thread
From: David Miller @ 2020-03-22 2:56 UTC (permalink / raw)
To: snelson; +Cc: netdev
From: Shannon Nelson <snelson@pensando.io>
Date: Thu, 19 Mar 2020 19:31:47 -0700
> These are a few little patches to make error recovery a little
> more safe and successful.
Series applied, thank you.
^ permalink raw reply [flat|nested] 13+ messages in thread