[PATCH net-next 0/6] ionic: queue mgmt updates

STALE1803d

10 messages, 3 authors, 2021-08-28 · open the first message on its own page

[PATCH net-next 0/6] ionic: queue mgmt updates

From: Shannon Nelson <hidden>
Date: 2021-08-27 18:55:23

The first pair of patches help smooth the driver's response when
the firmware has gone through a recovery/reboot cycle.

The next four patches take care of a couple things seen when
changing the interface status.

Shannon Nelson (6):
  ionic: fire watchdog again after fw_down
  ionic: squelch unnecessary fw halted message
  ionic: fill mac addr earlier in add_addr
  ionic: add queue lock around open and stop
  ionic: pull hwstamp queue_lock up a level
  ionic: recreate hwstamp queues on ifup

 .../net/ethernet/pensando/ionic/ionic_lif.c   | 45 +++++++++++--------
 .../net/ethernet/pensando/ionic/ionic_lif.h   |  2 +
 .../net/ethernet/pensando/ionic/ionic_main.c  |  4 +-
 .../net/ethernet/pensando/ionic/ionic_phc.c   | 28 ++++++++++++
 4 files changed, 59 insertions(+), 20 deletions(-)

-- 
2.17.1

[PATCH net-next 1/6] ionic: fire watchdog again after fw_down

From: Shannon Nelson <hidden>
Date: 2021-08-27 18:55:25

In some cases of fw_down it was called because there was a
fw_generation change, and the firmware is already back up.
In order to keep the down time to a minimum, don't wait for
the next watchdog polling cycle, fire another watchdog off
as soon as we can - an out-of-cycle check won't hurt, and
may well speed up the recovery.

Signed-off-by: Shannon Nelson <redacted>
---
 drivers/net/ethernet/pensando/ionic/ionic_lif.c | 11 +++++++++--
 1 file changed, 9 insertions(+), 2 deletions(-)
diff --git a/drivers/net/ethernet/pensando/ionic/ionic_lif.c b/drivers/net/ethernet/pensando/ionic/ionic_lif.c
index e494d6b909c7..df0137044c03 100644
--- a/drivers/net/ethernet/pensando/ionic/ionic_lif.c
+++ b/drivers/net/ethernet/pensando/ionic/ionic_lif.c
@@ -93,10 +93,17 @@ static void ionic_lif_deferred_work(struct work_struct *work)
 			ionic_link_status_check(lif);
 			break;
 		case IONIC_DW_TYPE_LIF_RESET:
-			if (w->fw_status)
+			if (w->fw_status) {
 				ionic_lif_handle_fw_up(lif);
-			else
+			} else {
 				ionic_lif_handle_fw_down(lif);
+
+				/* Fire off another watchdog to see
+				 * if the FW is already back rather than
+				 * waiting another whole cycle
+				 */
+				mod_timer(&lif->ionic->watchdog_timer, jiffies + 1);
+			}
 			break;
 		default:
 			break;
-- 
2.17.1

[PATCH net-next 2/6] ionic: squelch unnecessary fw halted message

From: Shannon Nelson <hidden>
Date: 2021-08-27 18:55:29

Since the heartbeat check will already have complained about
the firmware status, don't bother complaining about the
DEVCMD failing.  We'll keep the print message but demote it
to a debug messages so that we normally no longer see it.

Signed-off-by: Shannon Nelson <redacted>
---
 drivers/net/ethernet/pensando/ionic/ionic_main.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/net/ethernet/pensando/ionic/ionic_main.c b/drivers/net/ethernet/pensando/ionic/ionic_main.c
index 5f1e5b6e85c3..6f07bf509efe 100644
--- a/drivers/net/ethernet/pensando/ionic/ionic_main.c
+++ b/drivers/net/ethernet/pensando/ionic/ionic_main.c
@@ -375,8 +375,8 @@ int ionic_dev_cmd_wait(struct ionic *ionic, unsigned long max_seconds)
 		 * 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);
+		dev_dbg(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: fill mac addr earlier in add_addr

From: Shannon Nelson <hidden>
Date: 2021-08-27 18:55:32

Make sure the ctx struct has the new mac address before
any save operations happen.

Signed-off-by: Shannon Nelson <redacted>
---
 drivers/net/ethernet/pensando/ionic/ionic_lif.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/pensando/ionic/ionic_lif.c b/drivers/net/ethernet/pensando/ionic/ionic_lif.c
index df0137044c03..d69c80c3eaa2 100644
--- a/drivers/net/ethernet/pensando/ionic/ionic_lif.c
+++ b/drivers/net/ethernet/pensando/ionic/ionic_lif.c
@@ -1268,6 +1268,8 @@ int ionic_lif_addr_add(struct ionic_lif *lif, const u8 *addr)
 	struct ionic_rx_filter *f;
 	int err = 0;
 
+	memcpy(ctx.cmd.rx_filter_add.mac.addr, addr, ETH_ALEN);
+
 	spin_lock_bh(&lif->rx_filters.lock);
 	f = ionic_rx_filter_by_addr(lif, addr);
 	if (f) {
@@ -1281,7 +1283,6 @@ int ionic_lif_addr_add(struct ionic_lif *lif, const u8 *addr)
 		f->state = IONIC_FILTER_STATE_SYNCED;
 	} else {
 		/* save as SYNCED to catch any DEL requests while processing */
-		memcpy(ctx.cmd.rx_filter_add.mac.addr, addr, ETH_ALEN);
 		err = ionic_rx_filter_save(lif, 0, IONIC_RXQ_INDEX_ANY, 0, &ctx,
 					   IONIC_FILTER_STATE_SYNCED);
 	}
-- 
2.17.1

[PATCH net-next 4/6] ionic: add queue lock around open and stop

From: Shannon Nelson <hidden>
Date: 2021-08-27 18:55:37

Add the queue configuration lock to ionic_open() and
ionic_stop() so that they don't collide with other in parallel
queue configuration actions such as MTU changes as can be
demonstrated with a tight loop of ifup/change-mtu/ifdown.

Signed-off-by: Shannon Nelson <redacted>
---
 drivers/net/ethernet/pensando/ionic/ionic_lif.c | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/pensando/ionic/ionic_lif.c b/drivers/net/ethernet/pensando/ionic/ionic_lif.c
index d69c80c3eaa2..1d31b9385849 100644
--- a/drivers/net/ethernet/pensando/ionic/ionic_lif.c
+++ b/drivers/net/ethernet/pensando/ionic/ionic_lif.c
@@ -2233,9 +2233,11 @@ static int ionic_open(struct net_device *netdev)
 	if (test_and_clear_bit(IONIC_LIF_F_BROKEN, lif->state))
 		netdev_info(netdev, "clearing broken state\n");
 
+	mutex_lock(&lif->queue_lock);
+
 	err = ionic_txrx_alloc(lif);
 	if (err)
-		return err;
+		goto err_unlock;
 
 	err = ionic_txrx_init(lif);
 	if (err)
@@ -2256,12 +2258,15 @@ static int ionic_open(struct net_device *netdev)
 			goto err_txrx_deinit;
 	}
 
+	mutex_unlock(&lif->queue_lock);
 	return 0;
 
 err_txrx_deinit:
 	ionic_txrx_deinit(lif);
 err_txrx_free:
 	ionic_txrx_free(lif);
+err_unlock:
+	mutex_unlock(&lif->queue_lock);
 	return err;
 }
 
@@ -2281,9 +2286,11 @@ static int ionic_stop(struct net_device *netdev)
 	if (test_bit(IONIC_LIF_F_FW_RESET, lif->state))
 		return 0;
 
+	mutex_lock(&lif->queue_lock);
 	ionic_stop_queues(lif);
 	ionic_txrx_deinit(lif);
 	ionic_txrx_free(lif);
+	mutex_unlock(&lif->queue_lock);
 
 	return 0;
 }
-- 
2.17.1

[PATCH net-next 5/6] ionic: pull hwstamp queue_lock up a level

From: Shannon Nelson <hidden>
Date: 2021-08-27 18:55:38

Move the hwstamp configuration use of queue_lock up
a level to simplify use and error handling.

Signed-off-by: Shannon Nelson <redacted>
---
 drivers/net/ethernet/pensando/ionic/ionic_lif.c | 16 ++--------------
 drivers/net/ethernet/pensando/ionic/ionic_phc.c |  4 ++++
 2 files changed, 6 insertions(+), 14 deletions(-)
diff --git a/drivers/net/ethernet/pensando/ionic/ionic_lif.c b/drivers/net/ethernet/pensando/ionic/ionic_lif.c
index 1d31b9385849..96e7e289b7d3 100644
--- a/drivers/net/ethernet/pensando/ionic/ionic_lif.c
+++ b/drivers/net/ethernet/pensando/ionic/ionic_lif.c
@@ -849,10 +849,8 @@ int ionic_lif_create_hwstamp_txq(struct ionic_lif *lif)
 	u64 features;
 	int err;
 
-	mutex_lock(&lif->queue_lock);
-
 	if (lif->hwstamp_txq)
-		goto out;
+		return 0;
 
 	features = IONIC_Q_F_2X_CQ_DESC | IONIC_TXQ_F_HWSTAMP;
 
@@ -894,9 +892,6 @@ int ionic_lif_create_hwstamp_txq(struct ionic_lif *lif)
 		}
 	}
 
-out:
-	mutex_unlock(&lif->queue_lock);
-
 	return 0;
 
 err_qcq_enable:
@@ -907,7 +902,6 @@ int ionic_lif_create_hwstamp_txq(struct ionic_lif *lif)
 	ionic_qcq_free(lif, txq);
 	devm_kfree(lif->ionic->dev, txq);
 err_qcq_alloc:
-	mutex_unlock(&lif->queue_lock);
 	return err;
 }
 
@@ -919,10 +913,8 @@ int ionic_lif_create_hwstamp_rxq(struct ionic_lif *lif)
 	u64 features;
 	int err;
 
-	mutex_lock(&lif->queue_lock);
-
 	if (lif->hwstamp_rxq)
-		goto out;
+		return 0;
 
 	features = IONIC_Q_F_2X_CQ_DESC | IONIC_RXQ_F_HWSTAMP;
 
@@ -960,9 +952,6 @@ int ionic_lif_create_hwstamp_rxq(struct ionic_lif *lif)
 		}
 	}
 
-out:
-	mutex_unlock(&lif->queue_lock);
-
 	return 0;
 
 err_qcq_enable:
@@ -973,7 +962,6 @@ int ionic_lif_create_hwstamp_rxq(struct ionic_lif *lif)
 	ionic_qcq_free(lif, rxq);
 	devm_kfree(lif->ionic->dev, rxq);
 err_qcq_alloc:
-	mutex_unlock(&lif->queue_lock);
 	return err;
 }
 
diff --git a/drivers/net/ethernet/pensando/ionic/ionic_phc.c b/drivers/net/ethernet/pensando/ionic/ionic_phc.c
index afc45da399d4..c39790a6c436 100644
--- a/drivers/net/ethernet/pensando/ionic/ionic_phc.c
+++ b/drivers/net/ethernet/pensando/ionic/ionic_phc.c
@@ -194,7 +194,9 @@ int ionic_lif_hwstamp_set(struct ionic_lif *lif, struct ifreq *ifr)
 	if (copy_from_user(&config, ifr->ifr_data, sizeof(config)))
 		return -EFAULT;
 
+	mutex_lock(&lif->queue_lock);
 	err = ionic_lif_hwstamp_set_ts_config(lif, &config);
+	mutex_unlock(&lif->queue_lock);
 	if (err) {
 		netdev_info(lif->netdev, "hwstamp set failed: %d\n", err);
 		return err;
@@ -213,7 +215,9 @@ void ionic_lif_hwstamp_replay(struct ionic_lif *lif)
 	if (!lif->phc || !lif->phc->ptp)
 		return;
 
+	mutex_lock(&lif->queue_lock);
 	err = ionic_lif_hwstamp_set_ts_config(lif, NULL);
+	mutex_unlock(&lif->queue_lock);
 	if (err)
 		netdev_info(lif->netdev, "hwstamp replay failed: %d\n", err);
 }
-- 
2.17.1

[PATCH net-next 6/6] ionic: recreate hwstamp queues on ifup

From: Shannon Nelson <hidden>
Date: 2021-08-27 18:55:39

The queues can be freed in ionic_close().  They need to be recreated
after ionic_open().  It doesn't need to replay the whole config.  It
only needs to create the timestamping queues again.

Signed-off-by: Allen Hubbe <redacted>
Signed-off-by: Shannon Nelson <redacted>
---
 .../net/ethernet/pensando/ionic/ionic_lif.c   |  6 +++++
 .../net/ethernet/pensando/ionic/ionic_lif.h   |  2 ++
 .../net/ethernet/pensando/ionic/ionic_phc.c   | 24 +++++++++++++++++++
 3 files changed, 32 insertions(+)
diff --git a/drivers/net/ethernet/pensando/ionic/ionic_lif.c b/drivers/net/ethernet/pensando/ionic/ionic_lif.c
index 96e7e289b7d3..23c9e196a784 100644
--- a/drivers/net/ethernet/pensando/ionic/ionic_lif.c
+++ b/drivers/net/ethernet/pensando/ionic/ionic_lif.c
@@ -2246,7 +2246,13 @@ static int ionic_open(struct net_device *netdev)
 			goto err_txrx_deinit;
 	}
 
+	/* If hardware timestamping is enabled, but the queues were freed by
+	 * ionic_stop, those need to be reallocated and initialized, too.
+	 */
+	ionic_lif_hwstamp_recreate_queues(lif);
+
 	mutex_unlock(&lif->queue_lock);
+
 	return 0;
 
 err_txrx_deinit:
diff --git a/drivers/net/ethernet/pensando/ionic/ionic_lif.h b/drivers/net/ethernet/pensando/ionic/ionic_lif.h
index cad193d358e8..4915184f3efb 100644
--- a/drivers/net/ethernet/pensando/ionic/ionic_lif.h
+++ b/drivers/net/ethernet/pensando/ionic/ionic_lif.h
@@ -306,6 +306,7 @@ int ionic_lif_size(struct ionic *ionic);
 
 #if IS_ENABLED(CONFIG_PTP_1588_CLOCK)
 void ionic_lif_hwstamp_replay(struct ionic_lif *lif);
+void ionic_lif_hwstamp_recreate_queues(struct ionic_lif *lif);
 int ionic_lif_hwstamp_set(struct ionic_lif *lif, struct ifreq *ifr);
 int ionic_lif_hwstamp_get(struct ionic_lif *lif, struct ifreq *ifr);
 ktime_t ionic_lif_phc_ktime(struct ionic_lif *lif, u64 counter);
@@ -315,6 +316,7 @@ void ionic_lif_alloc_phc(struct ionic_lif *lif);
 void ionic_lif_free_phc(struct ionic_lif *lif);
 #else
 static inline void ionic_lif_hwstamp_replay(struct ionic_lif *lif) {}
+static inline void ionic_lif_hwstamp_recreate_queues(struct ionic_lif *lif) {}
 
 static inline int ionic_lif_hwstamp_set(struct ionic_lif *lif, struct ifreq *ifr)
 {
diff --git a/drivers/net/ethernet/pensando/ionic/ionic_phc.c b/drivers/net/ethernet/pensando/ionic/ionic_phc.c
index c39790a6c436..eed2db69d708 100644
--- a/drivers/net/ethernet/pensando/ionic/ionic_phc.c
+++ b/drivers/net/ethernet/pensando/ionic/ionic_phc.c
@@ -222,6 +222,30 @@ void ionic_lif_hwstamp_replay(struct ionic_lif *lif)
 		netdev_info(lif->netdev, "hwstamp replay failed: %d\n", err);
 }
 
+void ionic_lif_hwstamp_recreate_queues(struct ionic_lif *lif)
+{
+	int err;
+
+	if (!lif->phc || !lif->phc->ptp)
+		return;
+
+	mutex_lock(&lif->phc->config_lock);
+
+	if (lif->phc->ts_config_tx_mode) {
+		err = ionic_lif_create_hwstamp_txq(lif);
+		if (err)
+			netdev_info(lif->netdev, "hwstamp recreate txq failed: %d\n", err);
+	}
+
+	if (lif->phc->ts_config_rx_filt) {
+		err = ionic_lif_create_hwstamp_rxq(lif);
+		if (err)
+			netdev_info(lif->netdev, "hwstamp recreate rxq failed: %d\n", err);
+	}
+
+	mutex_unlock(&lif->phc->config_lock);
+}
+
 int ionic_lif_hwstamp_get(struct ionic_lif *lif, struct ifreq *ifr)
 {
 	struct hwtstamp_config config;
-- 
2.17.1

Re: [PATCH net-next 4/6] ionic: add queue lock around open and stop

From: Jakub Kicinski <kuba@kernel.org>
Date: 2021-08-28 00:39:09

On Fri, 27 Aug 2021 11:55:10 -0700 Shannon Nelson wrote:
Add the queue configuration lock to ionic_open() and
ionic_stop() so that they don't collide with other in parallel
queue configuration actions such as MTU changes as can be
demonstrated with a tight loop of ifup/change-mtu/ifdown.
Say more? how are up/down/change mtu not under rtnl_lock?

Re: [PATCH net-next 4/6] ionic: add queue lock around open and stop

From: Shannon Nelson <hidden>
Date: 2021-08-28 05:17:31

On 8/27/21 5:39 PM, Jakub Kicinski wrote:
On Fri, 27 Aug 2021 11:55:10 -0700 Shannon Nelson wrote:
quoted
Add the queue configuration lock to ionic_open() and
ionic_stop() so that they don't collide with other in parallel
queue configuration actions such as MTU changes as can be
demonstrated with a tight loop of ifup/change-mtu/ifdown.
Say more? how are up/down/change mtu not under rtnl_lock?
Sorry, that commit message didn't get updated as it should have. The MTU 
change played with the timing of actions just right, but wasn't the 
culprit.  The actual issue was that the watchdog and the ionic_stop 
collided: ionic_stop had started taking the queues down but without 
grabbing the mutex, and the watchdog timer went off and ran the 
link_check which grabbed the mutex and tried to bring them back up 
again.  This didn't break anything in the driver, but confused the NIC 
firmware and left the interface non-operational. This was cleared with 
another ifdown/ifup cycle.

I can repost with a better commit description.

sln

Re: [PATCH net-next 0/6] ionic: queue mgmt updates

From: patchwork-bot+netdevbpf@kernel.org
Date: 2021-08-28 11:30:13

Hello:

This series was applied to netdev/net-next.git (refs/heads/master):

On Fri, 27 Aug 2021 11:55:06 -0700 you wrote:
The first pair of patches help smooth the driver's response when
the firmware has gone through a recovery/reboot cycle.

The next four patches take care of a couple things seen when
changing the interface status.

Shannon Nelson (6):
  ionic: fire watchdog again after fw_down
  ionic: squelch unnecessary fw halted message
  ionic: fill mac addr earlier in add_addr
  ionic: add queue lock around open and stop
  ionic: pull hwstamp queue_lock up a level
  ionic: recreate hwstamp queues on ifup

[...]
Here is the summary with links:
  - [net-next,1/6] ionic: fire watchdog again after fw_down
    https://git.kernel.org/netdev/net-next/c/d3e2dcdb6853
  - [net-next,2/6] ionic: squelch unnecessary fw halted message
    https://git.kernel.org/netdev/net-next/c/970dfbf428c4
  - [net-next,3/6] ionic: fill mac addr earlier in add_addr
    https://git.kernel.org/netdev/net-next/c/92c90dff687f
  - [net-next,4/6] ionic: add queue lock around open and stop
    https://git.kernel.org/netdev/net-next/c/af3d2ae11443
  - [net-next,5/6] ionic: pull hwstamp queue_lock up a level
    https://git.kernel.org/netdev/net-next/c/7ee99fc5ed2e
  - [net-next,6/6] ionic: recreate hwstamp queues on ifup
    https://git.kernel.org/netdev/net-next/c/ccbbd002a419

You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html

Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help