Re: [PATCH iwl-net] e1000e: fix flow control stuck off after link partner reset

[Matt Vollrath <tactii@gmail.com>]

On 9/26/26 09:27, Craig I. Hagan wrote:
> On 2026-09-24, Matt Vollrath wrote:
>> I didn't try your patch, but there's a four line fix. Where the watchdog
>> resets just the PHY when the MAC exits DMoff state, set FLAG_RESTART_NOW
>> instead of resetting the PHY, then goto link_up. This works for me at
>> both 1500 and 9000 MTU.
> 
> Thanks Matt,
> I've tested this approach on two I219-LM boxes, each attached to a different switch:
> one a TP-Link TL-SG108PE V3, and the other a Cisco Catalyst 9200.
> 
> On the Cisco with jumbo frames, I'm seeing, via a diagnostic message, that
> the fix appears to fire and work. I have also noticed that I no longer see
> it on every boot, but this may be correct as I've not seen traffic stalls.
> 
> With 1500 mtu, I'm seeing it engage fairly regularly with the TP-Link but
> not the Cisco. I'm guessing it is related to how the Cisco negotiates. In both cases
> traffic doesn't seem to stall.
> 
> I think you are correct about flow control being a red herring since
> both switches advertise no pause (Flow Control: None) before and after
> recovery.
> 
> I'd like to drop my patch in favour of your fix.
> 
> How should I proceed, should I propose a patch with suggested-by you, or with your authorship and
> tested by me?

Feel free to add:

Suggested-by: Matt Vollrath <tactii@gmail.com>

> 
> For completeness, here is the diff I tested (I added an e_info so I could see it fire on the
> test rig; happy to drop that line - it isn't needed for the fix):

I would drop the e_info.

> 
> 
> 
> --- a/drivers/net/ethernet/intel/e1000e/netdev.c
> +++ b/drivers/net/ethernet/intel/e1000e/netdev.c
> @@ -5254,9 +5254,16 @@
>   					usleep_range(10000, 20000);
>   					pcim_state = er32(STATUS);
>   
> -					/* Checking if MAC exited DMoff state */
> -					if (!(pcim_state & E1000_STATUS_PCIM_STATE))
> -						e1000_phy_hw_reset(&adapter->hw);
> +					/* Checking if MAC exited DMoff state.  A bare
> +					 * PHY reset here does not re-run the jumbo
> +					 * workaround (e1000_lv_jumbo_workaround_ich8lan),
> +					 * so request a full reset instead, which does.
> +					 */

My two cents is that this comment is over-specific. In most places a bare PHY reset
is not used, so the transition is toward less exceptional.

> +					if (!(pcim_state & E1000_STATUS_PCIM_STATE)) {
> +						e_info("full reset on DMoff exit to restore jumbo-frame configuration\n");
> +						adapter->flags |= FLAG_RESTART_NOW;
> +						goto link_up;
> +					}
>   				}
>   			}
>