netdev - Re: [PATCH] ethtool: Add ETHTOOL_RESET support via --reset command

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: <f9f26d96-5707-dcc9-58b1-c2636211c6a3@mellanox.com>
Date:   Tue, 5 Dec 2017 10:21:23 +0200
From:   Gal Pressman <galp@...lanox.com>
To:     Scott Branden <scott.branden@...adcom.com>,
        Gal Pressman <galp@...lanox.com>,
        "John W. Linville" <linville@...driver.com>
Cc:     BCM Kernel Feedback <bcm-kernel-feedback-list@...adcom.com>,
        Steve Lin <steven.lin1@...adcom.com>,
        Michael Chan <michael.chan@...adcom.com>,
        netdev@...r.kernel.org
Subject: Re: [PATCH] ethtool: Add ETHTOOL_RESET support via --reset command

On 04-Dec-17 22:16, Scott Branden wrote:
> Hi Gal,
>
> I do not understand you're comment - questions inline
>
>
> On 17-12-03 12:07 AM, Gal Pressman wrote:
>> On 30-Nov-17 21:24, Scott Branden wrote:
>>> Add ETHTOOL_RESET support via --reset command.
>>>
>>> ie.  ethtool --reset DEVNAME <flagname(s)>
>>>
>>> flagnames currently match the ETH_RESET_xxx names:
>>> mgmt,irq,dma,filter,offload,mac,phy,ram,dedicated,all
>>>
>>> Alternatively, you can specific component bitfield directly using
>>> ethtool --reset DEVNAME flags %x
>>>
>>> Signed-off-by: Scott Branden <scott.branden@...adcom.com>
>>> ---
>>>   ethtool.8.in | 55 ++++++++++++++++++++++++++++++++++++++++++++-
>>>   ethtool.c    | 73 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>>>   2 files changed, 127 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/ethtool.8.in b/ethtool.8.in
>>> index 6ad3065..925cfe3 100644
>>> --- a/ethtool.8.in
>>> +++ b/ethtool.8.in
>>> @@ -355,6 +355,20 @@ ethtool \- query or control network driver and hardware settings
>>>   .B ethtool \-\-get\-phy\-tunable
>>>   .I devname
>>>   .RB [ downshift ]
>>> +.HP
>>> +.B ethtool \-\-reset
>>> +.I devname
>>> +.BN flags
>>> +.B [mgmt]
>>> +.B [irq]
>>> +.B [dma]
>>> +.B [filter]
>>> +.B [offload]
>>> +.B [mac]
>>> +.B [phy]
>>> +.B [ram]
>>> +.B [dedicated]
>>> +.B [all]
>>>   .
>> Nit:
>> Usually, the brackets formatting is different than the keyword inside them.
> Could you please explain what the formatting should be then?  Any of the options listed are possible components that can be selected.  At least one must be chosen.  Should the brackets be removed then?
The brackets are fine, IMO .B [mgmt] should be replaced with .RB [ mgmt ].
Looks better, and keeps us consistent with other flags (--get-phy-tunable for example).