Re: [PATCH net-next v7 2/7] net: phy: phylink: add helper to modify pause

[Andrew Lunn]

On Thu, Jul 23, 2026 at 02:50:46AM +0000, Javen wrote:
> 
> >
> >> +/**
> >> + * phylink_update_mac_pause_capabilities() - Dynamically update MAC
> >> +pause
> >> + * @pl: a pointer to a &struct phylink returned from phylink_create()
> >> + * @mac_pause: the new MAC pause capabilities mask
> >> + *
> >> + * This function allows a MAC driver to dynamically change its pause
> >> +state,
> >> + * such as losing/gaining Pause frame support based on MTU size.
> >> + * It recalculates supported link modes and triggers renegotiation if needed.
> >> + */
> >> +void phylink_update_mac_pause_capabilities(struct phylink *pl,
> >> +unsigned long mac_pause) {
> >> +     struct phylink_link_state *config = &pl->link_config;
> >> +     unsigned long old_pause;
> >> +     int pause_state;
> >> +
> >> +     ASSERT_RTNL();
> >> +
> >> +     if (mac_pause & ~(MAC_SYM_PAUSE | MAC_ASYM_PAUSE)) {
> >> +             phylink_err(pl, "Attempted to dynamically change non-pause MAC
> >capabilities\n");
> >> +             return;
> >> +     }
> >> +
> >> +     old_pause = pl->config->mac_capabilities & (MAC_SYM_PAUSE |
> >MAC_ASYM_PAUSE);
> >> +     if (old_pause == mac_pause)
> >> +             return;
> >> +
> >> +     mutex_lock(&pl->state_mutex);
> >> +
> >> +     pl->config->mac_capabilities &= ~(MAC_SYM_PAUSE |
> >MAC_ASYM_PAUSE);
> >> +     pl->config->mac_capabilities |= mac_pause;
> >> +
> >> +     phylink_set(pl->supported, Pause);
> >> +     phylink_set(pl->supported, Asym_Pause);
> >
> >I don't know the phylink code very well. Could you explain why these are
> >unconditional? It is not obvious to me.
> 
> Thanks for review.
> The unconditional phylink_set() is required precisely because of how phylink_validate() works. It acts as a necessary "reset" to prevent the permanent loss of capabilities in pl->supported across MTU changes.
> Consider the following scenario when MTU changes:
> 1. MTU 1500 -> 9000: The MAC loses Pause support. mac_pause becomes 0. When phylink_validate() is called, it correctly clears the Pause and Asym_Pause bits in pl->supported. This happens in the function call trace phylink_validate()=>phylink_validate_mac_and_pcs()=>phylink_validate_mask_caps()=>linkmode_and(supported, supported, mask), where mask represent the capability.
> 2. MTU 9000 -> 1500: The MAC regains Pause support. We update mac_pause back to 1. But if we simply call phylink_validate() at this point without unconditionally setting the bits first, it will take the current pl->supported (where Pause is 0 from step 1) as its input. The Pause capability will be permanently masked out and never restored.
> By unconditionally setting them here, we restore the baseline to 1, allowing phylink_validate() to correctly decide whether to keep them or clear them based on the current PHY/MAC capabilities.
 
Thanks for the explanation.

Reviewed-by: Andrew Lunn <andrew@xxxxxxx>

    Andrew