Thread (26 messages) 26 messages, 7 authors, 2017-12-18

Re: [RFC PATCH 5/9] ethtool: implement GET_DRVINFO message

From: Jiri Pirko <jiri@resnulli.us>
Date: 2017-12-11 16:16:14
Also in: lkml

Mon, Dec 11, 2017 at 02:54:01PM CET, mkubecek@suse.cz wrote:
quoted hunk ↗ jump to hunk
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 <redacted>
---
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.
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help