Re: [PATCH v12 16/17] net/txgbe: respect manual link-down on Amber-Lite

[Stephen Hemminger <stephen@networkplumber.org>]

On Tue, 27 Oct 2026 19:38:32 +0800
Zaiyu Wang <zaiyuwang@trustnetic.com> wrote:

> txgbe_dev_set_link_down() clears hw->dev_start, but the Amber-Lite
> AN73 watchdog treats a down link as a recovery condition and can start
> negotiation again. The setup-link alarm handler can likewise configure
> the PHY after a link update or a previous setup retry.
> 
> As a result, a port brought down through rte_eth_dev_set_link_down()
> can link up again without a matching rte_eth_dev_set_link_up() call.
> 
> Use hw->dev_start as the administrative link gate, and skip link
> recovery while the port is administratively down.
> 
> The gate alone is not enough. A setup-link run that started before
> link-down keeps going and rewrites the PMD transmit and receive
> enables, so the port reports down while the peer still sees a signal.
> Stop the AN73 watchdog and the setup-link alarm, wait for a running
> link thread to finish, and only then drop the transmitter.
> 
> Arm the watchdog again on link-up so that monitoring resumes, and
> cancel it first. The alarm list keeps every entry registered for the
> same callback, and each entry re-arms itself, so arming without
> cancelling would leave two watchdog chains running.
> 
> This keeps the link down until rte_eth_dev_set_link_up() explicitly
> re-enables it. The link-up path then restores the required PHY
> configuration.
> 
> Fixes: 234ce0d1fa9d ("net/txgbe: fix link stability for Amber-Lite backplane mode")
> Fixes: 6104fd11086e ("net/txgbe: fix link stability for 25G NIC")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Zaiyu Wang <zaiyuwang@trustnetic.com>
> ---

The only valid warning left on AI review of this series is:

Subject: Re: [PATCH v12 00/17] Wangxun fixes and new features

Series applies to main and every commit builds with -Dwerror=true.
v12 addresses the v11 comments on 05, 12, 13, 15 and 16.

Warnings
--------

Patch 16/17 (net/txgbe: respect manual link-down on Amber-Lite)

set_link_down() now stops the link alarm, the AN73 watchdog and the
link thread, but module insertion still brings the link back.
txgbe_dev_detect_sfp() is not cancelled, and on a successful
identify it calls

		txgbe_dev_setup_link_alarm_handler(dev);

which, unlike the _aml variant, does not check hw->dev_start and
calls setup_link() directly. On AML40 the 2 s module poll added in
10/17 keeps running while the port is administratively down, so
plugging in a DAC or QSFP module reprograms the PHY and lights the
link the user turned off. On the 25G MAC the same happens through
the module interrupt, and txgbe_set_link_to_amlite() also re-enables
the GPIO laser there.

Gate it on dev_start, and since set_link_up() then has to configure a
module that arrived while the port was down, run the link setup there
for both MACs:

	} else if (err == 0) {
		hw->mac.setup_sfp(hw);
		PMD_DRV_LOG(INFO, "detected SFP+: %d", hw->phy.sfp_type);
		if (hw->dev_start)
			txgbe_dev_setup_link_alarm_handler(dev);

and in txgbe_dev_set_link_up():

-		if (hw->mac.type == txgbe_mac_aml40) {
+		if (hw->mac.type == txgbe_mac_aml ||
+		    hw->mac.type == txgbe_mac_aml40) {

Review-Result: WARNING