Message-ID: <20200126210850.GB3870@unreal>
Date:   Sun, 26 Jan 2020 23:08:50 +0200
From:   Leon Romanovsky <leon@...nel.org>
To:     Jakub Kicinski <kuba@...nel.org>
Cc:     Shannon Nelson <snelson@...sando.io>,
        "David S . Miller" <davem@...emloft.net>,
        Michal Kalderon <michal.kalderon@...vell.com>,
        linux-netdev <netdev@...r.kernel.org>,
        RDMA mailing list <linux-rdma@...r.kernel.org>
Subject: Re: [PATCH net-next] net/core: Replace driver version to be kernel
 version

On Sun, Jan 26, 2020 at 12:49:57PM -0800, Jakub Kicinski wrote:
> On Sun, 26 Jan 2020 21:41:10 +0200, Leon Romanovsky wrote:
> > > 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.
> >
> > The thing is that leaving this field as empty, for sure will break all
> > applications. I have a feeling that it can be close to 100% hit rate.
> > So, kernel version was chosen as an option, because it is already
> > successfully in use by at least two drivers (nfp and hns).
>
> Shannon does have a point that out of tree drivers still make use of
> this field. Perhaps it would be a more suitable first step to print the
> kernel version as default and add a comment saying upstream modules
> shouldn't overwrite it (perhaps one day CI can catch new violators).

Jakub,

Shannon proposed to remove this field and it was me who said no :)
My plan is to overwrite ->version, delete all users and add
WARN_ONEC(strcpy(..->version_)...) inside net/ethtool/ to catch
abusers.

>
> The NFP reports the git hash of the driver source plus the string
> "(oot)" for out-of-tree:
>
> https://github.com/Netronome/nfp-drv-kmods/blob/master/src/Kbuild#L297
> https://github.com/Netronome/nfp-drv-kmods/blob/master/src/Kbuild#L315

I was inspired by upstream code.
https://elixir.bootlin.com/linux/v5.5-rc7/source/drivers/net/ethernet/netronome/nfp/nfp_net_ethtool.c#L184

>
> > Leaving to deal with driver version to vendors is not an option too,
> > because they prove for more than once that they are not capable to
> > define user visible interfaces. It comes due to their natural believe
> > that their company is alone in the world and user visible interface
> > should be suitable for them only.
> >
> > It is already impossible for users to distinguish properly versions
> > of different vendors, because they use arbitrary strings with some
> > numbers.
>
> That is true. But reporting the kernel version without even as much as
> in-tree/out-of-tree indication makes the field entirely meaningless.

The long-standing policy in kernel that we don't really care about
out-of-tree code.

Thanks

Powered by blists - more mailing lists