ethtool -s always return 0 even for errors

ethtool -s always return 0 even for errors

[Yi Zhao]

Hi,

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
$ ethtool -s eth1 duplex full
Cannot get current device settings: No such device
  not setting duplex
$ echo $?
0


Is this a correct behavior of ethtool?

Thanks,
Yi

Re: ethtool -s always return 0 even for errors

[Michal Kubecek]

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.

Michal

Re: ethtool -s always return 0 even for errors

[Yi Zhao]

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