Wed, Dec 13, 2017 at 12:54:39AM CET, mkube...@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, mkube...@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
