Re: [RFC PATCH net] ethtool: Fix incompatibility between netlink and ioctl interfaces

[Ido Schimmel <idosch@idosch.org>]

On Wed, Sep 30, 2020 at 04:19:09PM +0200, Andrew Lunn wrote:
> > > I don't think so. Doing:
> > > 
> > > # ethtool -s eth0 autoneg
> > > 
> > > Is a pretty established behavior to enable all the supported advertise
> > > bits.
> 
> I would disagree. phylib will return -EINVAL for this.

This has nothing to do with the kernel / phylib. With the ioctl
interface when you do:

# ethtool -s eth0 autoneg on

The ethtool user space utility will enable all the supported link modes:

https://git.kernel.org/pub/scm/network/ethtool/ethtool.git/tree/ethtool.c#n3170

For the netlink interface this is done by the kernel:

https://github.com/torvalds/linux/blob/master/net/ethtool/linkmodes.c#L2

But only if speed or duplex were specified:

https://github.com/torvalds/linux/blob/master/net/ethtool/linkmodes.c#L383

Which is a problem.

> 
> int phy_ethtool_ksettings_set(struct phy_device *phydev,
>                               const struct ethtool_link_ksettings *cmd)
> {
>         __ETHTOOL_DECLARE_LINK_MODE_MASK(advertising);
>         u8 autoneg = cmd->base.autoneg;
>         u8 duplex = cmd->base.duplex;
>         u32 speed = cmd->base.speed;
> 
> ...
>         linkmode_copy(advertising, cmd->link_modes.advertising);
> 
> ...
> 
>         if (autoneg == AUTONEG_ENABLE && linkmode_empty(advertising))
>                 return -EINVAL;
> 
> You have to pass a list of modes you want it to advertise. If you are
> using phylink and not a copper PHY, and autoneg, that means you are
> using in-band signalling. The same is imposed:
> 
>         /* If autonegotiation is enabled, we must have an advertisement */
>         if (config.an_enabled && phylink_is_empty_linkmode(config.advertising))
>                 return -EINVAL;
> 
> We have consistent behaviour whenever Linux is controlling the PHY
> because the core is imposing that behaviour. It would be nice if
> drivers ignoring the PHY core where consistent with this.

You will get an error from mlxsw as well (see example in the change
log).

> 
> 	Andrew