linux-rdma.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Shannon Nelson <snelson@pensando.io>
To: Leon Romanovsky <leon@kernel.org>,
	"David S . Miller" <davem@davemloft.net>,
	Jakub Kicinski <kuba@kernel.org>
Cc: Leon Romanovsky <leonro@mellanox.com>,
	Michal Kalderon <michal.kalderon@marvell.com>,
	linux-netdev <netdev@vger.kernel.org>,
	RDMA mailing list <linux-rdma@vger.kernel.org>
Subject: Re: [PATCH net-next] net/core: Replace driver version to be kernel version
Date: Sun, 26 Jan 2020 10:56:17 -0800	[thread overview]
Message-ID: <43d43a45-18db-f959-7275-63c9976fdf40@pensando.io> (raw)
In-Reply-To: <20200123130541.30473-1-leon@kernel.org>

On 1/23/20 5:05 AM, Leon Romanovsky wrote:
> From: Leon Romanovsky<leonro@mellanox.com>
>
> In order to stop useless driver version bumps and unify output
> presented by ethtool -i, let's overwrite the version string.
>
> Before this change:
> [leonro@erver ~]$ ethtool -i eth0
> driver: virtio_net
> version: 1.0.0
> After this change:
> [leonro@server ~]$ ethtool -i eth0
> driver: virtio_net
> version: 5.5.0-rc6+
>
> Signed-off-by: Leon Romanovsky<leonro@mellanox.com>
> ---
> I wanted to change to VERMAGIC_STRING, but the output doesn't
> look pleasant to my taste and on my system is truncated to be
> "version: 5.5.0-rc6+ SMP mod_unload modve".
>
> After this patch, we can drop all those version assignments
> from the drivers.
>
> Inspired by nfp and hns code.
> ---
>   net/core/ethtool.c | 3 +++
>   1 file changed, 3 insertions(+)
>
> diff --git a/net/core/ethtool.c b/net/core/ethtool.c
> index cd9bc67381b2..3c6fb13a78bf 100644
> --- a/net/core/ethtool.c
> +++ b/net/core/ethtool.c
> @@ -17,6 +17,7 @@
>   #include <linux/phy.h>
>   #include <linux/bitops.h>
>   #include <linux/uaccess.h>
> +#include <linux/vermagic.h>
>   #include <linux/vmalloc.h>
>   #include <linux/sfp.h>
>   #include <linux/slab.h>
> @@ -776,6 +777,8 @@ static noinline_for_stack int ethtool_get_drvinfo(struct net_device *dev,
>   		return -EOPNOTSUPP;
>   	}
>
> +	strlcpy(info.version, UTS_RELEASE, sizeof(info.version));
> +
>   	/*
>   	 * this method of obtaining string set info is deprecated;
>   	 * Use ETHTOOL_GSSET_INFO instead.
> --
> 2.20.1
>

First of all, although I've seen some of the arguments about distros and 
their backporting, I still believe that the driver version number is 
useful.  In most cases it at least gets us in the ballpark of what 
generation the driver happens to be and is still useful. I'd really 
prefer that it is just left alone for the device manufactures and their 
support folks to deal with.

Fine, I'm sure I lose that argument since there's already been plenty of 
discussion about it.

Meanwhile, there is some non-zero number of support scripts and 
processes, possibly internal testing chains, that use that driver/vendor 
specific version information and will be broken by this change.  Small 
number?  Large number?  I don't know, but we're breaking them.

Sure, I probably easily lose that argument too, but it still should be 
stated.

This will end up affecting out-of-tree drivers as well, where it is 
useful to know what the version number is, most especially since it is 
different from what the kernel provided driver is.  How else are we to 
get this information out to the user?  If this feature gets squashed, 
we'll end up having to abuse some other mechanism so we can get the live 
information from the driver, and probably each vendor will find a 
different way to sneak it out, giving us more chaos than where we 
started.  At least the ethtool version field is a known and consistent 
place for the version info.

Of course, out-of-tree drivers are not first class citizens, so I 
probably lose that argument as well.

So if you are so all fired up about not allowing the drivers to report 
their own version number, then why report anything at all? Maybe just 
report a blank field.  As some have said, the uname info is already 
available else where, why are we sticking it here?

Personally, I think this is a rather arbitrary, heavy handed and 
unnecessary slam on the drivers, and will make support more difficult in 
the long run.

sln


  parent reply	other threads:[~2020-01-26 18:55 UTC|newest]

Thread overview: 30+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-01-23 13:05 Leon Romanovsky
2020-01-23 14:40 ` Jakub Kicinski
2020-01-23 14:54   ` Leon Romanovsky
2020-01-23 15:17     ` Jakub Kicinski
2020-01-25  9:13 ` David Miller
2020-01-26 18:56 ` Shannon Nelson [this message]
2020-01-26 19:41   ` Leon Romanovsky
2020-01-26 20:49     ` Jakub Kicinski
2020-01-26 21:08       ` Leon Romanovsky
2020-01-26 21:17         ` Shannon Nelson
2020-01-26 21:24           ` Leon Romanovsky
2020-01-26 22:12             ` Shannon Nelson
2020-01-26 22:22               ` Michal Kubecek
2020-01-26 22:57                 ` Shannon Nelson
2020-01-27  6:08                   ` Michal Kubecek
2020-01-27  6:42                     ` Leon Romanovsky
2020-01-27  5:18                 ` Leon Romanovsky
2020-01-26 21:33         ` Jakub Kicinski
2020-01-26 22:21           ` Shannon Nelson
2020-01-27  5:34             ` Leon Romanovsky
2020-01-27  6:45               ` Leon Romanovsky
2020-01-27 14:21                 ` Jakub Kicinski
2020-01-27 15:39                   ` Leon Romanovsky
2020-01-27  5:49           ` Leon Romanovsky
2020-01-27 12:21             ` David Miller
2020-01-27 12:42               ` Leon Romanovsky
2020-01-27 12:47                 ` David Miller
2020-01-27 17:57             ` Shannon Nelson
2020-01-26 20:52     ` Shannon Nelson
2020-01-26 21:19       ` Leon Romanovsky

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=43d43a45-18db-f959-7275-63c9976fdf40@pensando.io \
    --to=snelson@pensando.io \
    --cc=davem@davemloft.net \
    --cc=kuba@kernel.org \
    --cc=leon@kernel.org \
    --cc=leonro@mellanox.com \
    --cc=linux-rdma@vger.kernel.org \
    --cc=michal.kalderon@marvell.com \
    --cc=netdev@vger.kernel.org \
    --subject='Re: [PATCH net-next] net/core: Replace driver version to be kernel version' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).