After rte_eth_dev_set_link_down() the port can come back up without a
corresponding rte_eth_dev_set_link_up(). Three paths reprogram the PHY
while the port is down:

- txgbe_dev_e56_check_bp_event() (the AN73 watchdog): keeps driving
  negotiation while the link is down and auto-negotiation is enabled.
- txgbe_dev_setup_link_alarm_handler_aml(): runs setup_link() from the
  2 s retry armed by txgbe_setup_mac_link_aml() and from the link
  thread.
- txgbe_dev_detect_sfp(): calls txgbe_dev_setup_link_alarm_handler(),
  which ignores the administrative state, after identifying a module.

The first two are gated on hw->dev_start before acquire_swfw_sync() and
setup_link(); the third only around the setup call, so identification
keeps running. It has to keep running: no setup path re-identifies the
module by itself, since txgbe_setup_phy_link_aml() never identifies and
txgbe_setup_phy_link_aml40() identifies only when the cached type is
not present. A module swapped while the port is down would otherwise be
configured as the module that was there before, out of hw->phy.sfp_type.

set_link_down() then stops what is already running:

- clears hw->dev_start first, so a callback that starts later stops at
  its gate;
- calls rte_eal_alarm_cancel(), which returns only after a matching
  callback that is already running has finished;
- waits for the link thread with txgbe_dev_wait_setup_link_complete(),
  which with timeout 0 has no bound and logs an error every 9 s;
- cancels the handler once more: the link thread runs the same handler
  and can arm a 2 s retry that the earlier cancel never saw;
- calls hw->mac.disable_tx_laser() last; on a 40G DAC that clears the
  transmit and receive enables in PMD_CFG0.

set_link_up() restarts the flow for both Amber-Lite MACs. The watchdog
is cancelled before it is armed because rte_eal_alarm_set() adds an
entry on every call without looking for an existing one, and each entry
arms the next when it fires: arming twice leaves two chains running
side by side. Link setup now runs for txgbe_mac_aml too and returns
-EIO on failure; txgbe_setup_phy_link_aml() resets the PHY and restarts
AN73 (when auto-negotiation is enabled).

Fixes: 234ce0d1fa9d ("net/txgbe: fix link stability for Amber-Lite backplane 
mode")
Fixes: 6104fd11086e ("net/txgbe: fix link stability for 25G NIC")
Cc: [email protected]

Signed-off-by: Zaiyu Wang <[email protected]>
---
 drivers/net/txgbe/txgbe_ethdev.c | 40 ++++++++++++++++++++++++++++----
 1 file changed, 35 insertions(+), 5 deletions(-)

diff --git a/drivers/net/txgbe/txgbe_ethdev.c b/drivers/net/txgbe/txgbe_ethdev.c
index 043c96b747..9dbdbfa309 100644
--- a/drivers/net/txgbe/txgbe_ethdev.c
+++ b/drivers/net/txgbe/txgbe_ethdev.c
@@ -2213,7 +2213,8 @@ txgbe_dev_set_link_up(struct rte_eth_dev *dev)
                /* Turn on the laser */
                hw->mac.enable_tx_laser(hw);
 
-               if (hw->mac.type == txgbe_mac_aml40) {
+               if (hw->mac.type == txgbe_mac_aml ||
+                   hw->mac.type == txgbe_mac_aml40) {
                        speed = hw->phy.autoneg_advertised;
                        if (speed == 0) {
                                err = hw->mac.get_link_capabilities(hw, &speed, 
&autoneg);
@@ -2227,12 +2228,33 @@ txgbe_dev_set_link_up(struct rte_eth_dev *dev)
                }
 
                hw->dev_start = true;
+               if (hw->mac.type == txgbe_mac_aml ||
+                   hw->mac.type == txgbe_mac_aml40) {
+                       rte_eal_alarm_cancel(txgbe_dev_e56_check_bp_event, dev);
+                       rte_eal_alarm_set(hw->bp_event_interval,
+                                         txgbe_dev_e56_check_bp_event, dev);
+               }
                txgbe_dev_link_update(dev, 0);
        }
 
        return 0;
 }
 
+static void
+txgbe_stop_link_handlers(struct rte_eth_dev *dev)
+{
+       struct txgbe_hw *hw = TXGBE_DEV_HW(dev);
+
+       if (hw->mac.type != txgbe_mac_aml &&
+           hw->mac.type != txgbe_mac_aml40)
+               return;
+
+       rte_eal_alarm_cancel(txgbe_dev_e56_check_bp_event, dev);
+       rte_eal_alarm_cancel(txgbe_dev_setup_link_alarm_handler_aml, hw);
+       txgbe_dev_wait_setup_link_complete(dev, 0);
+       rte_eal_alarm_cancel(txgbe_dev_setup_link_alarm_handler_aml, hw);
+}
+
 /*
  * Set device link down: disable tx.
  */
@@ -2245,9 +2267,11 @@ txgbe_dev_set_link_down(struct rte_eth_dev *dev)
                /* Turn off the copper */
                hw->phy.set_phy_power(hw, false);
        } else {
+               hw->dev_start = false;
+               txgbe_stop_link_handlers(dev);
+
                /* Turn off the laser */
                hw->mac.disable_tx_laser(hw);
-               hw->dev_start = false;
                txgbe_dev_link_update(dev, 0);
        }
 
@@ -3073,8 +3097,8 @@ void txgbe_dev_e56_check_bp_event(void *param)
        if (!hw->devarg.auto_neg)
                return;
 
-       /* only continue if link is down */
-       if (dev->data->dev_link.link_status)
+       /* Continue only for an administratively up port with link down. */
+       if (!hw->dev_start || dev->data->dev_link.link_status)
                goto out;
 
        value = rd32_epcs(hw, VR_AN_INTR);
@@ -3232,7 +3256,8 @@ txgbe_dev_detect_sfp(void *param)
        } else if (err == 0) {
                hw->mac.setup_sfp(hw);
                PMD_DRV_LOG(INFO, "detected SFP+: %d", hw->phy.sfp_type);
-               txgbe_dev_setup_link_alarm_handler(dev);
+               if (hw->dev_start)
+                       txgbe_dev_setup_link_alarm_handler(dev);
                /* Re-arm the AN73 watchdog for the newly inserted module, so
                 * that only one instance of it is running at a time.
                 */
@@ -3412,6 +3437,11 @@ txgbe_dev_setup_link_alarm_handler_aml(void *param)
        bool autoneg = false;
        u32 gssr = hw->phy.phy_semaphore_mask;
 
+       if (!hw->dev_start) {
+               intr->flags &= ~TXGBE_FLAG_NEED_LINK_CONFIG;
+               return;
+       }
+
        speed = hw->phy.autoneg_advertised;
        if (!speed)
                hw->mac.get_link_capabilities(hw, &speed, &autoneg);
-- 
2.55.0.windows.2

Reply via email to