From: Jakub Kicinski <email@example.com> To: Leon Romanovsky <firstname.lastname@example.org> Cc: Shannon Nelson <email@example.com>, "David S . Miller" <firstname.lastname@example.org>, Michal Kalderon <email@example.com>, linux-netdev <firstname.lastname@example.org>, RDMA mailing list <email@example.com> Subject: Re: [PATCH net-next] net/core: Replace driver version to be kernel version Date: Sun, 26 Jan 2020 13:33:53 -0800 [thread overview] Message-ID: <20200126133353.77f5cb7e@cakuba> (raw) In-Reply-To: <20200126210850.GB3870@unreal> On Sun, 26 Jan 2020 23:08:50 +0200, Leon Romanovsky wrote: > 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. > > 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). > > Shannon proposed to remove this field and it was me who said no :) Obviously, we can't remove fields from UAPI structs. > My plan is to overwrite ->version, delete all users and add > WARN_ONEC(strcpy(..->version_)...) inside net/ethtool/ to catch > abusers. What I was thinking just now was: initialize ->version to utsname before drivers are called, delete all upstream users, add a coccicheck for upstream drivers which try to report the version. > > 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 Right, upstream nfp reports kernel version (both in modinfo and ethtool) GitHub/compat/backport/out-of-tree reports kernel version in which the code was expected to appear in modinfo: https://github.com/Netronome/nfp-drv-kmods/commit/7ec15c47caf5dbdf1f9806410535ad5b7373ec34#diff-492d7fa4004d885a38cfa889ed1adbe7L1284 And git hash of the driver source plus out of tree marker in ethtool. That means it's out-of-tree driver which has to carry the extra code and require extra feeding. As backport should IMHO. > > > 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. Yeah... we all know it's not that simple :) The in-tree driver versions are meaningless and cause annoying churn when people arbitrarily bump them. If we can get people to stop doing that we'll be happy, that's all there is to it. Out of tree the field is useful, so we don't have to take it away just as a matter of principle. If we can't convince people to stop bringing the versions into the tree that'll be another story...
next prev parent reply other threads:[~2020-01-26 21:33 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 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 [this message] 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=20200126133353.77f5cb7e@cakuba \ --firstname.lastname@example.org \ --email@example.com \ --firstname.lastname@example.org \ --email@example.com \ --firstname.lastname@example.org \ --email@example.com \ --firstname.lastname@example.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).