aq(4): interface lifecycle and link-state fixes

aq_if_init() programmed the address captured at attach, so an address set
with "ifconfig ether" or by lagg(4) enslavement was never written to
unicast filter slot 0: the interface transmitted with the new address but
the MAC still filtered on the old one, so it received nothing.  Copy the
current if_getlladdr() the way the other iflib drivers do.

The link state could latch UP forever.  aq_if_stop() cleared linkup
before calling aq_if_update_admin_status(), which suppressed the
LINK_STATE_DOWN transition the "link was UP" branch would have made.
Announce the down transition directly from aq_if_stop() instead, and do
not poll the admin status there at all: the MAC has just been reset, so a
stale link reading would re-announce the link as up.

The admin task itself had to stop reporting a link on a stopped
interface.  iflib runs it while either IFF_DRV_RUNNING or IFF_DRV_OACTIVE
is set, and iflib_stop() sets OACTIVE, so the task kept polling after the
stop and re-announced LINK_STATE_UP behind the driver's back.  Treat a
non-running interface as having no link.  A lagg(4) parent otherwise
keeps hashing flows onto a port whose carrier is gone, because
LAGG_PORTACTIVE tests if_link_state together with IFF_UP.  Stop the rest
of the task there as well: the PHY thermal poll and the initialization
retry both end in iflib_request_reset(), and _task_fn_admin() acts on
that with no test of its own, so either could re-initialize an interface
the operator had just taken down.

aq_if_update_admin_status() also only reacted to transitions in and out
of zero speed, so an autoneg downshift that kept the link up left
if_baudrate, ifmedia, RX pause and interrupt moderation programmed for
the old speed.  Track the announced speed and re-run that work when it
changes.

aq_if_suspend() resets the MAC and stops the rings, but
iflib_device_suspend() only calls IFDI_SUSPEND and never stops the
interface, leaving IFF_DRV_RUNNING set over a suspended device.  Clear it.

Signed-off-by: Nick Price <nick@spun.io>

Reviewed by:	adrian
Differential Revision:	https://reviews.freebsd.org/D58473

3 files changed, 34 insertions, 8 deletions

diff --git a/sys/dev/aq/aq_device.h b/sys/dev/aq/aq_device.h
index c8942e19cf13..31c225bf2ddf 100644
--- a/sys/dev/aq/aq_device.h
+++ b/sys/dev/aq/aq_device.h
@@ -121,6 +121,7 @@ struct aq_dev {
 	uint32_t          tx_rings_count;
 	uint32_t          rx_rings_count;
 	bool              linkup;
+	uint32_t          link_speed;	/* Mbit/s last announced to the stack */
 	uint16_t          phy_fault_last;	/* last fault code reported */
 	enum aq_thermal_state {
 		AQ_THERMAL_NORMAL = 0,	/* no thermal shutdown pending */
diff --git a/sys/dev/aq/aq_irq.c b/sys/dev/aq/aq_irq.c
index a15424cbeeba..054492bfe3df 100644
--- a/sys/dev/aq/aq_irq.c
+++ b/sys/dev/aq/aq_irq.c
@@ -217,20 +217,31 @@ aq_if_update_admin_status(if_ctx_t ctx)
 	struct aq_dev *aq_dev = iflib_get_softc(ctx);
 	struct aq_hw *hw = &aq_dev->hw;
 	uint32_t link_speed;
+	bool running;
 
 
 	struct aq_hw_fc_info fc_neg;
 	aq_hw_get_link_state(hw, &link_speed, &fc_neg);
 
-	/* An interface whose initialization did not complete has no link. */
-	if (aq_dev->init_failed)
+	/* A stopped or half-initialized interface has no link. */
+	running = (if_getdrvflags(iflib_get_ifp(ctx)) & IFF_DRV_RUNNING) != 0;
+	if (!running || aq_dev->init_failed)
 		link_speed = 0;
 
-	if (link_speed && !aq_dev->linkup) { /* link was DOWN */
-		device_printf(aq_dev->dev, "link UP: speed=%d\n", link_speed);
+	/* A retrain can change the speed without ever dropping the link. */
+	if (link_speed != 0U &&
+	    (!aq_dev->linkup || link_speed != aq_dev->link_speed)) {
+		if (!aq_dev->linkup) {
+			if (bootverbose)
+				device_printf(aq_dev->dev,
+				    "link UP: speed=%d\n", link_speed);
+			aq_dev->phy_fault_last = 0;
+		} else
+			device_printf(aq_dev->dev, "link speed=%d\n",
+			    link_speed);
 
 		aq_dev->linkup = 1;
-		aq_dev->phy_fault_last = 0;
+		aq_dev->link_speed = link_speed;
 
 		/* turn on/off RX Pause in RPB */
 		rpb_rx_xoff_en_per_tc_set(hw, fc_neg.fc_rx, 0);
@@ -242,9 +253,11 @@ aq_if_update_admin_status(if_ctx_t ctx)
 		/* update ITR settings according new link speed */
 		aq_hw_interrupt_moderation_set(hw);
 	} else if (link_speed == 0U && aq_dev->linkup) { /* link was UP */
-		device_printf(aq_dev->dev, "link DOWN\n");
+		if (bootverbose)
+			device_printf(aq_dev->dev, "link DOWN\n");
 
 		aq_dev->linkup = 0;
+		aq_dev->link_speed = 0;
 
 		/* turn off RX Pause in RPB */
 		rpb_rx_xoff_en_per_tc_set(hw, 0, 0);
@@ -253,6 +266,10 @@ aq_if_update_admin_status(if_ctx_t ctx)
 		aq_mediastatus_update(aq_dev, link_speed, &fc_neg);
 	}
 
+	/* A stopped interface must not be re-initialized behind the operator. */
+	if (!running)
+		return;
+
 	/* Re-arming while a reset is queued would re-init once too often. */
 	if (aq_dev->init_failed) {
 		if (aq_dev->reset_pending)
diff --git a/sys/dev/aq/aq_main.c b/sys/dev/aq/aq_main.c
index cb04ccf5d158..d958a2aa62c5 100644
--- a/sys/dev/aq/aq_main.c
+++ b/sys/dev/aq/aq_main.c
@@ -559,6 +559,8 @@ aq_if_suspend(if_ctx_t ctx)
 
 	aq_if_stop(ctx);
 	aq_hw_deinit(&softc->hw);
+	/* iflib_device_suspend() does not stop the interface for us. */
+	if_setdrvflagbits(iflib_get_ifp(ctx), IFF_DRV_OACTIVE, IFF_DRV_RUNNING);
 
 	AQ_DBG_EXIT(0);
 	return (0);
@@ -755,6 +757,9 @@ aq_if_init(if_ctx_t ctx)
 	softc->reset_pending = false;
 	hw->tx_rings_count = softc->tx_rings_count;
 
+	/* Pick up a locally administered address set since the last init. */
+	bcopy(if_getlladdr(iflib_get_ifp(ctx)), hw->mac_addr, ETHER_ADDR_LEN);
+
 	err = aq_hw_init(&softc->hw, softc->hw.mac_addr, softc->msix,
 	    softc->scctx->isc_intr == IFLIB_INTR_MSIX);
 	if (err != 0) {
@@ -855,8 +860,11 @@ aq_if_stop(if_ctx_t ctx)
 	memset(&softc->last_stats, 0, sizeof(softc->last_stats));
 	/* Each bring-up gets its own budget of re-init attempts. */
 	softc->init_retries = 0;
-	softc->linkup = false;
-	aq_if_update_admin_status(ctx);
+	if (softc->linkup) {
+		softc->linkup = false;
+		softc->link_speed = 0;
+		iflib_link_state_change(ctx, LINK_STATE_DOWN, 0);
+	}
 	AQ_DBG_EXIT(0);
 }