Date: Sun, 02 Aug 2026 23:10:36 +0000 From: Adrian Chadd <adrian@FreeBSD.org> To: src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-main@FreeBSD.org Cc: Nick Price <nick@spun.io> Subject: git: c956cc0f033a - main - aq(4): interface lifecycle and link-state fixes Message-ID: <6a6fce6c.43579.744d2b5c@gitrepo.freebsd.org>
index | next in thread | raw e-mail
The branch main has been updated by adrian: URL: https://cgit.FreeBSD.org/src/commit/?id=c956cc0f033a4050fbca4e39fdace7276a588e15 commit c956cc0f033a4050fbca4e39fdace7276a588e15 Author: Nick Price <nick@spun.io> AuthorDate: 2026-08-02 22:45:03 +0000 Commit: Adrian Chadd <adrian@FreeBSD.org> CommitDate: 2026-08-02 22:45:03 +0000 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 --- sys/dev/aq/aq_device.h | 1 + sys/dev/aq/aq_irq.c | 29 +++++++++++++++++++++++------ sys/dev/aq/aq_main.c | 12 ++++++++++-- 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); }home | help
Want to link to this message? Use this
URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?6a6fce6c.43579.744d2b5c>