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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20200930141909.GJ3996795@lunn.ch>
Date:   Wed, 30 Sep 2020 16:19:09 +0200
From:   Andrew Lunn <andrew@...n.ch>
To:     Michal Kubecek <mkubecek@...e.cz>
Cc:     Ido Schimmel <idosch@...sch.org>, netdev@...r.kernel.org,
        davem@...emloft.net, kuba@...nel.org, f.fainelli@...il.com,
        ayal@...dia.com, danieller@...dia.com, amcohen@...dia.com,
        mlxsw@...dia.com, Ido Schimmel <idosch@...dia.com>
Subject: Re: [RFC PATCH net] ethtool: Fix incompatibility between netlink and
 ioctl interfaces

> > 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.

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.

	Andrew