[PATCH net-next 0/6] ionic error recovery fixes

[PATCH net-next 0/6] ionic error recovery fixes

[Shannon Nelson]

These are a few little patches to make error recovery a little
more safe and successful.

Shannon Nelson (6):
  ionic: add timeout error checking for queue disable
  ionic: leave dev cmd request contents alone on FW timeout
  ionic: only save good lif dentry
  ionic: ignore eexist on rx filter add
  ionic: clean irq affinity on queue deinit
  ionic: check for NULL structs on teardown

 .../ethernet/pensando/ionic/ionic_debugfs.c   |  8 ++-
 .../net/ethernet/pensando/ionic/ionic_lif.c   | 52 ++++++++++++-------
 .../net/ethernet/pensando/ionic/ionic_main.c  | 12 ++++-
 3 files changed, 50 insertions(+), 22 deletions(-)

-- 
2.17.1

[PATCH net-next 1/6] ionic: add timeout error checking for queue disable

[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

[PATCH net-next 2/6] ionic: leave dev cmd request contents alone on FW timeout

[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

[PATCH net-next 3/6] ionic: only save good lif dentry

[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

[PATCH net-next 4/6] ionic: ignore eexist on rx filter add

[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

[PATCH net-next 5/6] ionic: clean irq affinity on queue deinit

[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

[PATCH net-next 6/6] ionic: check for NULL structs on teardown

[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

Re: [PATCH net-next 4/6] ionic: ignore eexist on rx filter add

[Jakub Kicinski]

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.

Re: [PATCH net-next 0/6] ionic error recovery fixes

[Jakub Kicinski]

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?

Re: [PATCH net-next 4/6] ionic: ignore eexist on rx filter add

[Shannon Nelson]

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

Re: [PATCH net-next 0/6] ionic error recovery fixes

[Shannon Nelson]

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

Re: [PATCH net-next 4/6] ionic: ignore eexist on rx filter add

[Jakub Kicinski]

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.

Re: [PATCH net-next 0/6] ionic error recovery fixes

[David Miller]

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.