On Wed, Sep 30, 2020 at 04:19:09PM +0200, Andrew Lunn wrote:

quoted

quoted

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