netdev - Re: Ethtool question

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]

Date:   Tue, 17 Oct 2017 10:49:18 +0300
From:   Gal Pressman <galp@...lanox.com>
To:     Ben Greear <greearb@...delatech.com>,
        Roopa Prabhu <roopa@...ulusnetworks.com>
Cc:     David Miller <davem@...emloft.net>,
        "John W. Linville" <linville@...driver.com>,
        "netdev@...r.kernel.org" <netdev@...r.kernel.org>
Subject: Re: Ethtool question

On 16-Oct-17 21:10, Ben Greear wrote:
> On 10/12/2017 03:00 PM, Roopa Prabhu wrote:
>> On Thu, Oct 12, 2017 at 2:45 PM, Ben Greear <greearb@...delatech.com> wrote:
>>> On 10/11/2017 01:49 PM, David Miller wrote:
>>>>
>>>> From: "John W. Linville" <linville@...driver.com>
>>>> Date: Wed, 11 Oct 2017 16:44:07 -0400
>>>>
>>>>> On Wed, Oct 11, 2017 at 09:51:56AM -0700, Ben Greear wrote:
>>>>>>
>>>>>> I noticed today that setting some ethtool settings to the same value
>>>>>> returns an error code.  I would think this should silently return
>>>>>> success instead?  Makes it easier to call it from scripts this way:
>>>>>>
>>>>>> [root@...313-6477 lanforge]# ethtool -L eth3 combined 1
>>>>>> combined unmodified, ignoring
>>>>>> no channel parameters changed, aborting
>>>>>> current values: tx 0 rx 0 other 1 combined 1
>>>>>> [root@...313-6477 lanforge]# echo $?
>>>>>> 1
>>>>>
>>>>>
>>>>> I just had this discussion a couple of months ago with someone. My
>>>>> initial feeling was like you, a no-op is not a failure. But someone
>>>>> convinced me otherwise...I will now endeavour to remember who that
>>>>> was and how they convinced me...
>>>>>
>>>>> Anyone else have input here?
>>>>
>>>>
>>>> I guess this usually happens when drivers don't support changing the
>>>> settings at all.  So they just make their ethtool operation for the
>>>> 'set' always return an error.
>>>>
>>>> We could have a generic ethtool helper that does "get" and then if the
>>>> "set" request is identical just return zero.
>>>>
>>>> But from another perspective, the error returned from the "set" in this
>>>> situation also indicates to the user that the driver does not support
>>>> the "set" operation which has value and meaning in and of itself.  And
>>>> we'd lose that with the given suggestion.
>>>
>>>
>>> In my case, the driver (igb) does support the set, my program just made the
>>> same
>>> ethtool call several times and it fails after the initial change (that
>>> actually
>>> changes something), as best as I can figure.
>>
>>
>> This error is returned by ethtool user-space. It does a get, check and
>> then set if user has requested changes.
>>
>
> So, should we fix ethtool to return 0 in this case instead of an error code?
>
> I think so.  If the driver itself returns an error, then probably return the
> error code and/or fix the driver as seems appropriate.
>
> Thanks,
> Ben
>

FWIW, seems like the code isn't consistent in this matter.
Setting pause/channels/coalescing with the same values will return an error code, but setting EEE will return success.