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]

Message-ID: <20181216130309.GD2063@slim.localdomain>
Date:   Sun, 16 Dec 2018 13:03:09 -0500
From:   Vivien Didelot <vivien.didelot@...il.com>
To:     Andrew Lunn <andrew@...n.ch>
Cc:     Florian Fainelli <f.fainelli@...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

Hi Andrew,

On Sat, 15 Dec 2018 18:48:49 +0100, Andrew Lunn <andrew@...n.ch> wrote:
> > > 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.
> 
> 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?

Indeed we could have DSA driver specific names, including or not a
prefix such as "dsa-", as in "(dsa-)mv88e6xxx". However I choose not
to go that way for the moment since casting the ethtool registers to
u16 and checking regs[3] isn't destructive. I felt like changing the
"dsa" driver name can be part of a future iteration, but I'd be glad
to make the change now if you guys prefer.


Thanks,
Vivien