lkml.org 
[lkml]   [2017]   [Dec]   [13]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [RFC PATCH 5/9] ethtool: implement GET_DRVINFO message
Wed, Dec 13, 2017 at 12:54:39AM CET, mkubecek@suse.cz wrote:
>On Mon, Dec 11, 2017 at 05:16:01PM +0100, Jiri Pirko wrote:
>> Mon, Dec 11, 2017 at 02:54:01PM CET, mkubecek@suse.cz wrote:
>> >+
>> >+ ETHA_DRVINFO_DRIVER (string) driver name
>> >+ ETHA_DRVINFO_VERSION (string) driver version
>>
>> You use 3 prefixes:
>> ETHTOOL_ for cmd
>> ETHA_ for attrs
>> ethnl_ for function
>>
>> I suggest to sync this, perhaps to:
>> ETHNL_CMD_*
>> ETHNL_ATTR_*
>> ethnl_*
>> ?
>
>Switching to ETHNL_CMD_* is certainly a good idea. For attributes, I'm a
>bit worried that ETHNL_ATTR_ prefix might make it even harder to fit
>into the 80 characters per line limit. I'll try and see what it would
>look like.
>
>> >+ ETHA_DRVINFO_FWVERSION (string) firmware version
>> >+ ETHA_DRVINFO_BUSINFO (string) device bus address
>> >+ ETHA_DRVINFO_EROM_VER (string) expansion ROM version
>> >+ ETHA_DRVINFO_N_PRIV_FLAGS (u32) number of private flags
>> >+ ETHA_DRVINFO_N_STATS (u32) number of device stats
>> >+ ETHA_DRVINFO_TESTINFO_LEN (u32) number of test results
>> >+ ETHA_DRVINFO_EEDUMP_LEN (u32) EEPROM dump size
>> >+ ETHA_DRVINFO_REGDUMP_LEN (u32) register dump size
>>
>> We are now working on providing various fw memory regions dump in
>> devlink. It makes sense to have it in devlink for couple of reasons:
>> 1) per-asic, not netdev specific, therefore does not really make sense
>> to have netdev as handle, but rather devlink handle.
>> 2) snapshot support - we need to provide support for getting snapshot
>> (for example on failure), transferring to user and deleting it
>> (remove from driver memory).
>>
>> Also, driver name, version, fwversion, etc is per-asic. Would make sense
>> to have it in devlink as well.
>>
>> I think this is great opprotunity to move things where they should be to
>> be alligned with the current world and kernel infrastructure.
>
>IMHO this depends on the question whether we are going to rework also
>the interface between kernel and NIC drivers (currently ethtool_ops).
>If we are, it would make good sense to split the information in both
>interfaces. If not, it doesn't seem to make userspace do two queries,
>each getting one part of the same information provided by NIC.

Yeah, agreed. I believe we should.


>
>Also, I must admit that one thing I dislike about the iw tool is that it
>pushes me to distinguish between operations on "phy" and "dev". While
>I understand that it makes perfect sense for someone familiar with the
>internals, it's quite annoying for an occasional "consumer". It would be
>sad if we created a set of perfectly logical and clean tools and
>(almost) 20 years later, people still used old ioctl based ethtool
>because "it's what they are used to and it works just fine for them"
>(like they do with ifconfig, netstat or brctl).

Good documentation should take care of that.



>
>Michal

\
 
 \ /
  Last update: 2017-12-13 07:57    [W:0.078 / U:1.880 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site