netdev - Re: [PATCH 0/7] ethtool: add pretty dump for DSA mv88e6xxx drivers

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]

Date:   Sun, 16 Dec 2018 10:02:27 -0800
From:   Florian Fainelli <f.fainelli@...il.com>
To:     Andrew Lunn <andrew@...n.ch>
Cc:     Vivien Didelot <vivien.didelot@...il.com>, netdev@...r.kernel.org,
        Chris Healy <cphealy@...il.com>,
        "John W . Linville" <linville@...driver.com>
Subject: Re: [PATCH 0/7] ethtool: add pretty dump for DSA mv88e6xxx drivers

Le 12/15/18 à 9:48 AM, Andrew Lunn a écrit :
> On Sat, Dec 15, 2018 at 09:28:41AM -0800, Florian Fainelli wrote:
>> Hi Vivien,
>>
>> Le 12/14/18 à 6:50 PM, Vivien Didelot a écrit :
>>> This patch series adds support to pretty dump the registers of user
>>> ports created by the kernel "dsa" subsystem.
>>>
>>> The first patch adds the base support for "dsa" interfaces.
>>>
>>> The second patch adds the boilerplate for the "mv88e6xxx" DSA driver,
>>> all using 32 registers of 16 bits, the switch ID being available in
>>> the port identification register 3. Support for other DSA drivers such
>>> as "b53" or "ksz" can be added similarly later. Because the different
>>> switches supported by mv88e6xxx have slightly different register layout,
>>> we keep it simple and stupid by providing one dump function per switch.
>>
>> This looks good to me, the only "concern" is that mv88e6xxx set
>> regs->version = 0, while we could probably put the switch model in there
>> directly which would avoid other drivers to have to put the chip ID in
>> regs[3] since that may, or may not be convenient.
> 
> Hi Florian, Vivien
> 
> I was wondering about that. Having this all under 'dsa' seems too
> granular. It would be better if we could have 'mv88e6xxx', 'b53',
> 'ksz', etc. That might need a new DSA driver op to get the driver name
> which we then use for the slave?

We could indeed do that, the other option would be return something
like: "dsa-mv88e6xxx" or "dsa-b53" in ethtool::get_drv_info() but this
might be breaking ABI since "dsa" has been commonly established for the
past 10 years or so.

Changing things within ethtool is reasonably easy, but we probably need
to decide now whether kernel changes are requited to assist with
companion ethtool changes.
-- 
Florian