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
