netdev - Re: ethtool -s always return 0 even for errors

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] [day] [month] [year] [list]

Message-ID: <8d966634-f269-0bd3-83bc-627944c34814@windriver.com>
Date:   Mon, 27 Apr 2020 09:33:09 +0800
From:   Yi Zhao <yi.zhao@...driver.com>
To:     Michal Kubecek <mkubecek@...e.cz>, <netdev@...r.kernel.org>
CC:     <linville@...driver.com>
Subject: Re: ethtool -s always return 0 even for errors


On 4/23/20 9:36 PM, Michal Kubecek wrote:
> On Thu, Apr 23, 2020 at 05:45:47PM +0800, Yi Zhao wrote:
>> The ethtool -s returns 0 when it fails with an error (stderr):
>>
>> $ ethtool -s eth0 duplex full
>> Cannot advertise duplex full
>> $ echo $?
>> 0
>> $ ethtool -s eth0 speed 10
>> Cannot advertise speed 10
>> $ echo $?
>> 0
> These two are not really errors, just warnings. According to comments in
> the code, the idea was that requesting speed and/or duplex with
> autonegotiation enabled (either already enabled or requested to be
> enabled) and no explicit request for advertised modes (no "advertise"
> keyword), ethtool should enable exactly the modes (out of those
> supported by the device) which match requested speed and/or duplex
> value(s). The messages you see above are warnings that this logic is not
> implemented and all parameters are just passed to kernel and probably
> ignored (depends on the driver).
>
> Actually, with kernel 5.6 (with CONFIG_ETHTOOL_NETLINK=y) and ethtool
> built from git (or 5.6 once released), these commands work as expected:
>
> lion:/home/mike/work/git/ethtool # ./ethtool eth0
> Settings for eth0:
> ...
>          Advertised link modes:  10baseT/Half 10baseT/Full
>                                  100baseT/Half 100baseT/Full
>                                  1000baseT/Full
> ...
>          Auto-negotiation: on
> ...
> lion:/home/mike/work/git/ethtool # ./ethtool -s eth0 speed 100
> lion:/home/mike/work/git/ethtool # ./ethtool eth0
> Settings for eth0:
> ...
>          Advertised link modes:  100baseT/Half 100baseT/Full
> ...
> lion:/home/mike/work/git/ethtool # ./ethtool -s eth0 duplex full
> lion:/home/mike/work/git/ethtool # ./ethtool eth0
> Settings for eth0:
> ...
>          Advertised link modes:  10baseT/Full
>                                  100baseT/Full
>                                  1000baseT/Full
> ...
> lion:/home/mike/work/git/ethtool # ./ethtool -s eth0 speed 100 duplex full
> lion:/home/mike/work/git/ethtool # ./ethtool eth0
> Settings for eth0:
> ...
>          Advertised link modes:  100baseT/Full
> ...
>
>> $ ethtool -s eth1 duplex full
>> Cannot get current device settings: No such device
>>    not setting duplex
>> $ echo $?
>> 0
> The problem here is that for historical reasons, "ethtool -s" may issue
> up to three separate ioctl requests (or up to four netlink requests with
> new kernel and ethtool), depending on which parameters you request on
> command line. Each of them can either succeed or fail and you can even
> see multiple error messages:
>
> lion:/home/mike/work/git/ethtool # ethtool -s foo phyad 3 wol um msglvl 7
> Cannot get current device settings: No such device
>    not setting phy_address
> Cannot get current wake-on-lan settings: No such device
>    not setting wol
> Cannot get msglvl: No such device
>
> Currently, do_sset() always returns 0. While it certainly feels wrong to
> return 0 if all requests fail (including your case where there was only
> one request and it failed), it is much less obvious what should we
> return if some of the requests succeed and some fail.


Thank you for the detailed explanation.


//Yi

>
> Michal