lkml.org 
[lkml]   [2017]   [Dec]   [11]   [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
Mon, Dec 11, 2017 at 02:54:01PM CET, mkubecek@suse.cz wrote:
>Request the same information as ETHTOOL_GDRVINFO. This is read-only so that
>corresponding SET_DRVINFO exists but is only used in kernel replies. Rip
>the code to query the driver out of the legacy interface and move it to
>a new file ethtool_common.c so that both interfaces can use it.
>
>Signed-off-by: Michal Kubecek <mkubecek@suse.cz>
>---
> Documentation/networking/ethtool-netlink.txt | 30 ++++++++++-
> include/uapi/linux/ethtool_netlink.h | 21 ++++++++
> net/core/Makefile | 2 +-
> net/core/ethtool.c | 42 +++-------------
> net/core/ethtool_common.c | 46 +++++++++++++++++
> net/core/ethtool_common.h | 11 ++++
> net/core/ethtool_netlink.c | 75 ++++++++++++++++++++++++++++
> 7 files changed, 189 insertions(+), 38 deletions(-)
> create mode 100644 net/core/ethtool_common.c
> create mode 100644 net/core/ethtool_common.h
>
>diff --git a/Documentation/networking/ethtool-netlink.txt b/Documentation/networking/ethtool-netlink.txt
>index 893e5156f6a7..cb992180b211 100644
>--- a/Documentation/networking/ethtool-netlink.txt
>+++ b/Documentation/networking/ethtool-netlink.txt
>@@ -107,6 +107,9 @@ which the request applies.
> List of message types
> ---------------------
>
>+ ETHTOOL_CMD_GET_DRVINFO
>+ ETHTOOL_CMD_SET_DRVINFO response only
>+
> All constants use ETHTOOL_CMD_ prefix followed by "GET", "SET" or "ACT" to
> indicate the type.
>
>@@ -119,6 +122,31 @@ messages marked as "response only" in the table above.
> Later sections describe the format and semantics of these request messages.
>
>
>+GET_DRVINFO
>+-----------
>+
>+GET_DRVINFO request corresponds to ETHTOOL_GDRVINFO ioctl command and provides
>+basic driver information. The request doesn't use any attributes and flags,
>+info_mask and index field in request header are ignored. Kernel response
>+contents:
>+
>+ 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_*
?


>+ 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.

\
 
 \ /
  Last update: 2017-12-11 17:17    [W:0.135 / U:0.928 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site