Re: [PATCH net-next v5 13/22] ethtool: provide driver/device information in GET_INFO request

From: Michal Kubecek <mkubecek@suse.cz>
To: Jiri Pirko <jiri@resnulli.us>
Cc: David Miller <davem@davemloft.net>,
	netdev@vger.kernel.org,
	Jakub Kicinski <jakub.kicinski@netronome.com>,
	Andrew Lunn <andrew@lunn.ch>,
	Florian Fainelli <f.fainelli@gmail.com>,
	John Linville <linville@tuxdriver.com>,
	Stephen Hemminger <stephen@networkplumber.org>,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH net-next v5 13/22] ethtool: provide driver/device information in GET_INFO request
Date: Wed, 27 Mar 2019 23:25:54 +0100	[thread overview]
Message-ID: <20190327222554.GV26076@unicorn.suse.cz> (raw)
In-Reply-To: <20190327201411.GH14297@nanopsycho>

On Wed, Mar 27, 2019 at 09:14:11PM +0100, Jiri Pirko wrote:
> Mon, Mar 25, 2019 at 06:08:33PM CET, mkubecek@suse.cz wrote:
> >+
> >+Kernel response contents:
> >+
> >+    ETHA_INFO_DEV		(nested)	device identification
> >+    ETHA_INFO_DRVINFO		(nested)	driver information
> >+        ETHA_DRVINFO_DRIVER		(string)	driver name
> >+        ETHA_DRVINFO_FWVERSION		(string)	firmware version
> >+        ETHA_DRVINFO_BUSINFO		(string)	device bus address
> >+        ETHA_DRVINFO_EROM_VER		(string)	expansion ROM version
> 
> These are already very nicely supported in devlink. No need to duplicate
> here.

They are supported by devlink as an interface. But devlink itself is
only supported by few NIC drivers at the moment:

mike@unicorn:~/work/git/net-next> grep -r devlink_ops drivers/net/
drivers/net/ethernet/mellanox/mlx4/main.c:static const struct devlink_ops mlx4_devlink_ops = {
drivers/net/ethernet/mellanox/mlx4/main.c:      devlink = devlink_alloc(&mlx4_devlink_ops, sizeof(*priv));
drivers/net/ethernet/mellanox/mlxsw/core.c:static const struct devlink_ops mlxsw_devlink_ops = {
drivers/net/ethernet/mellanox/mlxsw/core.c:             devlink = devlink_alloc(&mlxsw_devlink_ops, alloc_size);
drivers/net/ethernet/mellanox/mlx5/core/main.c:static const struct devlink_ops mlx5_devlink_ops = {
drivers/net/ethernet/mellanox/mlx5/core/main.c: devlink = devlink_alloc(&mlx5_devlink_ops, sizeof(*dev));
drivers/net/ethernet/cavium/liquidio/lio_main.c:static const struct devlink_ops liquidio_devlink_ops = {
drivers/net/ethernet/cavium/liquidio/lio_main.c:        devlink = devlink_alloc(&liquidio_devlink_ops,
drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.c:static const struct devlink_ops bnxt_dl_ops = {
drivers/net/ethernet/netronome/nfp/nfp_main.c:  devlink = devlink_alloc(&nfp_devlink_ops, sizeof(*pf));
drivers/net/ethernet/netronome/nfp/nfp_main.h:extern const struct devlink_ops nfp_devlink_ops;
drivers/net/ethernet/netronome/nfp/nfp_devlink.c:const struct devlink_ops nfp_devlink_ops = {
drivers/net/netdevsim/devlink.c:static const struct devlink_ops nsim_devlink_ops = {
drivers/net/netdevsim/devlink.c:        devlink = devlink_alloc(&nsim_devlink_ops, 0);

That's 6 drivers from 4 vendors (if I don't count netdevsim). And I did
not check if all of them do actually provide the information shown
above. On the other hand:

mike@unicorn:~/work/git/net-next> egrep -r '\.get_drvinfo' drivers/net/ | wc -l
240

Some of these 240 lines assign the same handler but not enough to make
me optimistic about being able to implement "ethtool -i <dev>" using
devlink interface in near future (say few months or one year).

I'm all for implementing new features which are are related to physical
device (ASIC) rather than network interface only in devlink (at the
level of kernel-userspace interface). But for features already provided
by ethtool (userspace utility) I can't help seeing the state of devlink
support in NIC drivers as a serious blocker.

Michal