[PATCH RFC net-next 1/5] net: stmmac: call phylink_start() and phylink_stop() in XDP functions

[Andrew Lunn]

On Fri, Feb 28, 2025 at 03:31:22PM +0800, Furong Xu wrote:
> On Thu, 27 Feb 2025 15:05:02 +0000
> "Russell King (Oracle)" <rmk+kernel at armlinux.org.uk> wrote:
> 
> > Phylink does not permit drivers to mess with the netif carrier, as
> > this will de-synchronise phylink with the MAC driver. Moreover,
> > setting and clearing the TE and RE bits via stmmac_mac_set() in this
> > path is also wrong as the link may not be up.
> > 
> > Replace the netif_carrier_on(), netif_carrier_off() and
> > stmmac_mac_set() calls with the appropriate phylink_start() and
> > phylink_stop() calls, thereby allowing phylink to manage the netif
> > carrier and TE/RE bits through the .mac_link_up() and .mac_link_down()
> > methods.
> > 
> > Note that RE should only be set after the DMA is ready to avoid the
> > receive FIFO between the MAC and DMA blocks overflowing, so
> > phylink_start() needs to be placed after DMA has been started.
> > 
> > Signed-off-by: Russell King (Oracle) <rmk+kernel at armlinux.org.uk>
 
> XDP programs work like a charm both before and after this patch.
> 
> Tested-by: Furong Xu <0x1207 at gmail.com>

Thanks for testing this.

Could you give a little details of how you actually tested it?

The issues here is, when is the link set admin up, requiring that
phylink triggers an autoneg etc.

For plain old TCP/IP, you at some point use:

ip link set eth42 up

which will cause the core to call the drivers open() method, which
then triggers phylnk.

The carrier manipulation which this code replaces seems to suggest you
can load an XDP program while the interface is admin down, and that
action of loading the program will implicitly set the carrier up.

Did you test loading an XDP program on an interface which is admin
down?

	Andrew