Re: [Intel-wired-lan] [PATCH iwl-next] ixgbe: call ixgbe_setup_fc() before fc_enable() after NVM update

[Simon Horman <horms@kernel.org>]

This is an AI-generated review of your patch. The human sending this
email has considered the AI review valid, or at least plausible.

For local reproduction steps see https://netdev-ai.bots.linux.dev/ai-local.html
---
ixgbe: call ixgbe_setup_fc() before fc_enable() after NVM update

During an NVM update the PHY reset clears the Technology Ability Field
(IEEE 802.3 clause 37 register 7.10) back to hardware defaults. When
the driver subsequently calls only hw->mac.ops.fc_enable() the SRRCTL
register is recalculated from the stale autonegotiated capability bits,
which the MDD (Malicious Driver Detect) logic treats as an invalid
change and halts traffic on the PF.

Fix by calling ixgbe_setup_fc() immediately before fc_enable() in
ixgbe_watchdog_update_link() so that flow-control autoneg and the
PHY registers are re-programmed in the correct order after any reset.

> diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
> index 0bc806aaed90..96e912371d9a 100644
> --- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
> +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
> @@ -8029,6 +8029,12 @@ static void ixgbe_watchdog_update_link(struct ixgbe_adapter *adapter)
>  	pfc_en |= !!(adapter->ixgbe_ieee_pfc->pfc_en);
>
>  	if (link_up && !((adapter->flags & IXGBE_FLAG_DCB_ENABLED) && pfc_en)) {
> +		if (hw->mac.ops.setup_fc) {
> +			int err = hw->mac.ops.setup_fc(hw);
> +
> +			if (err)
> +				e_warn(drv, "setup_fc failed: %d\n", err);
> +		}

What happens if setup_fc() fails here? The patch adds this call to fix
MDD issues caused by stale PHY autoneg state after NVM updates.

However, if setup_fc() returns an error, the code only logs a warning
and continues:

	if (hw->mac.ops.setup_fc) {
		int err = hw->mac.ops.setup_fc(hw);

		if (err)
			e_warn(drv, "setup_fc failed: %d\n", err);
	}
	hw->mac.ops.fc_enable(hw);

Since fc_enable() relies on hw->fc.current_mode being properly set by
the autoneg process initiated by setup_fc(), calling it after a
setup_fc failure means it operates on the same stale state the patch
was meant to fix.

Could this leave a residual bug where MDD can still trigger and halt
traffic if setup_fc fails?

>  		hw->mac.ops.fc_enable(hw);
>  		ixgbe_set_rx_drop_en(adapter);
>  	}