* [PATCH net-next v1] net/core: Replace driver version to be kernel version
@ 2020-01-25 16:14 Leon Romanovsky
2020-01-25 16:55 ` Florian Fainelli
0 siblings, 1 reply; 6+ messages in thread
From: Leon Romanovsky @ 2020-01-25 16:14 UTC (permalink / raw)
To: David S. Miller, Jakub Kicinski
Cc: Leon Romanovsky, Michal Kalderon, linux-netdev, RDMA mailing list
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>
---
Changelog:
v1: Resend per-Dave's request
https://lore.kernel.org/linux-rdma/20200125.101311.1924780619716720495.davem@davemloft.net
No changes at all and applied cleanly on top of "3333e50b64fe Merge branch 'mlxsw-Offload-TBF'"
v0: https://lore.kernel.org/linux-rdma/20200123130541.30473-1-leon@kernel.org
---
net/ethtool/ioctl.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/net/ethtool/ioctl.c b/net/ethtool/ioctl.c
index 182bffbffa78..a403decacb6d 100644
--- a/net/ethtool/ioctl.c
+++ b/net/ethtool/ioctl.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>
@@ -666,6 +667,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.24.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH net-next v1] net/core: Replace driver version to be kernel version
2020-01-25 16:14 [PATCH net-next v1] net/core: Replace driver version to be kernel version Leon Romanovsky
@ 2020-01-25 16:55 ` Florian Fainelli
2020-01-25 18:49 ` Leon Romanovsky
0 siblings, 1 reply; 6+ messages in thread
From: Florian Fainelli @ 2020-01-25 16:55 UTC (permalink / raw)
To: Leon Romanovsky, David S. Miller, Jakub Kicinski, Michal Kubecek
Cc: Leon Romanovsky, Michal Kalderon, linux-netdev, RDMA mailing list
On 1/25/2020 8:14 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>> ---
> Changelog:
> v1: Resend per-Dave's request
> https://lore.kernel.org/linux-rdma/20200125.101311.1924780619716720495.davem@davemloft.net
> No changes at all and applied cleanly on top of "3333e50b64fe Merge branch 'mlxsw-Offload-TBF'"
> v0: https://lore.kernel.org/linux-rdma/20200123130541.30473-1-leon@kernel.org
There does not appear to be any explanation why we think this is a good
idea for *all* drivers, and not just the ones that are purely virtual?
Are you not concerned that this is ABI and that specific userland may be
relying on a specific info format and we could now be breaking their
version checks? I do not disagree that the version is not particularly
useful for in-tree kernel, but this is ABI, and breaking user-space is
usually a source of support questions.
> ---
> net/ethtool/ioctl.c | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/net/ethtool/ioctl.c b/net/ethtool/ioctl.c
> index 182bffbffa78..a403decacb6d 100644
> --- a/net/ethtool/ioctl.c
> +++ b/net/ethtool/ioctl.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>
> @@ -666,6 +667,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.24.1
>
--
Florian
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH net-next v1] net/core: Replace driver version to be kernel version
2020-01-25 16:55 ` Florian Fainelli
@ 2020-01-25 18:49 ` Leon Romanovsky
2020-01-25 19:24 ` Leon Romanovsky
0 siblings, 1 reply; 6+ messages in thread
From: Leon Romanovsky @ 2020-01-25 18:49 UTC (permalink / raw)
To: Florian Fainelli
Cc: David S. Miller, Jakub Kicinski, Michal Kubecek, Michal Kalderon,
linux-netdev, RDMA mailing list
On Sat, Jan 25, 2020 at 08:55:01AM -0800, Florian Fainelli wrote:
>
>
> On 1/25/2020 8:14 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>> ---
> > Changelog:
> > v1: Resend per-Dave's request
> > https://lore.kernel.org/linux-rdma/20200125.101311.1924780619716720495.davem@davemloft.net
> > No changes at all and applied cleanly on top of "3333e50b64fe Merge branch 'mlxsw-Offload-TBF'"
> > v0: https://lore.kernel.org/linux-rdma/20200123130541.30473-1-leon@kernel.org
>
> There does not appear to be any explanation why we think this is a good
> idea for *all* drivers, and not just the ones that are purely virtual?
We beat this dead horse too many times already, latest discussion and
justification can be found in that thread.
https://lore.kernel.org/linux-rdma/20200122152627.14903-1-michal.kalderon@marvell.com/T/#md460ff8f976c532a89d6860411c3c50bb811038b
However, it was discussed in ksummit mailing list too and overall
agreement that version exposed by in-tree modules are useless and
sometimes even worse. They mislead users to expect some features
or lack of them based on this arbitrary string.
>
> Are you not concerned that this is ABI and that specific userland may be
> relying on a specific info format and we could now be breaking their
> version checks? I do not disagree that the version is not particularly
> useful for in-tree kernel, but this is ABI, and breaking user-space is
> usually a source of support questions.
See this Linus's response:
"The unified policy is pretty much that version codes do not matter, do
not exist, and do not get updated.
Things are supposed to be backwards and forwards compatible, because
we don't accept breakage in user space anyway. So versioning is
pointless, and only causes problems."
https://lore.kernel.org/ksummit-discuss/CA+55aFx9A=5cc0QZ7CySC4F2K7eYaEfzkdYEc9JaNgCcV25=rg@mail.gmail.com/
I also don't think that declaring every print in the kernel as ABI is
good thing to do. We are not breaking binary ABI and continuing to
supply some sort of versioning, but in unified format and not in wild
west way like it is now.
So bottom line, if some REAL user space application (not test suites) relies
on specific version reported from ethtool, it is already broken and can't work
sanely for stable@, distros and upstream kernels.
Thanks
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH net-next v1] net/core: Replace driver version to be kernel version
2020-01-25 18:49 ` Leon Romanovsky
@ 2020-01-25 19:24 ` Leon Romanovsky
2020-01-26 3:11 ` Florian Fainelli
0 siblings, 1 reply; 6+ messages in thread
From: Leon Romanovsky @ 2020-01-25 19:24 UTC (permalink / raw)
To: Florian Fainelli
Cc: David S. Miller, Jakub Kicinski, Michal Kubecek, Michal Kalderon,
linux-netdev, RDMA mailing list
On Sat, Jan 25, 2020 at 08:49:58PM +0200, Leon Romanovsky wrote:
> On Sat, Jan 25, 2020 at 08:55:01AM -0800, Florian Fainelli wrote:
> >
> >
> > On 1/25/2020 8:14 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>> ---
> > > Changelog:
> > > v1: Resend per-Dave's request
> > > https://lore.kernel.org/linux-rdma/20200125.101311.1924780619716720495.davem@davemloft.net
> > > No changes at all and applied cleanly on top of "3333e50b64fe Merge branch 'mlxsw-Offload-TBF'"
> > > v0: https://lore.kernel.org/linux-rdma/20200123130541.30473-1-leon@kernel.org
> >
> > There does not appear to be any explanation why we think this is a good
> > idea for *all* drivers, and not just the ones that are purely virtual?
>
> We beat this dead horse too many times already, latest discussion and
> justification can be found in that thread.
> https://lore.kernel.org/linux-rdma/20200122152627.14903-1-michal.kalderon@marvell.com/T/#md460ff8f976c532a89d6860411c3c50bb811038b
>
> However, it was discussed in ksummit mailing list too and overall
> agreement that version exposed by in-tree modules are useless and
> sometimes even worse. They mislead users to expect some features
> or lack of them based on this arbitrary string.
>
> >
> > Are you not concerned that this is ABI and that specific userland may be
> > relying on a specific info format and we could now be breaking their
> > version checks? I do not disagree that the version is not particularly
> > useful for in-tree kernel, but this is ABI, and breaking user-space is
> > usually a source of support questions.
>
> See this Linus's response:
> "The unified policy is pretty much that version codes do not matter, do
> not exist, and do not get updated.
>
> Things are supposed to be backwards and forwards compatible, because
> we don't accept breakage in user space anyway. So versioning is
> pointless, and only causes problems."
> https://lore.kernel.org/ksummit-discuss/CA+55aFx9A=5cc0QZ7CySC4F2K7eYaEfzkdYEc9JaNgCcV25=rg@mail.gmail.com/
>
> I also don't think that declaring every print in the kernel as ABI is
> good thing to do. We are not breaking binary ABI and continuing to
> supply some sort of versioning, but in unified format and not in wild
> west way like it is now.
>
> So bottom line, if some REAL user space application (not test suites) relies
> on specific version reported from ethtool, it is already broken and can't work
> sanely for stable@, distros and upstream kernels.
And about support questions,
I'm already over-asked to update our mlx5 driver version every time some
of our developers adds new feature (every week or two), which is insane.
So I prefer to have one stable solution in the kernel.
Thanks
>
> Thanks
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH net-next v1] net/core: Replace driver version to be kernel version
2020-01-25 19:24 ` Leon Romanovsky
@ 2020-01-26 3:11 ` Florian Fainelli
2020-01-26 9:44 ` Leon Romanovsky
0 siblings, 1 reply; 6+ messages in thread
From: Florian Fainelli @ 2020-01-26 3:11 UTC (permalink / raw)
To: Leon Romanovsky
Cc: David S. Miller, Jakub Kicinski, Michal Kubecek, Michal Kalderon,
linux-netdev, RDMA mailing list
On 1/25/2020 11:24 AM, Leon Romanovsky wrote:
> On Sat, Jan 25, 2020 at 08:49:58PM +0200, Leon Romanovsky wrote:
>> On Sat, Jan 25, 2020 at 08:55:01AM -0800, Florian Fainelli wrote:
>>>
>>>
>>> On 1/25/2020 8:14 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>> ---
>>>> Changelog:
>>>> v1: Resend per-Dave's request
>>>> https://lore.kernel.org/linux-rdma/20200125.101311.1924780619716720495.davem@davemloft.net
>>>> No changes at all and applied cleanly on top of "3333e50b64fe Merge branch 'mlxsw-Offload-TBF'"
>>>> v0: https://lore.kernel.org/linux-rdma/20200123130541.30473-1-leon@kernel.org
>>>
>>> There does not appear to be any explanation why we think this is a good
>>> idea for *all* drivers, and not just the ones that are purely virtual?
>>
>> We beat this dead horse too many times already, latest discussion and
>> justification can be found in that thread.
>> https://lore.kernel.org/linux-rdma/20200122152627.14903-1-michal.kalderon@marvell.com/T/#md460ff8f976c532a89d6860411c3c50bb811038b
>>
>> However, it was discussed in ksummit mailing list too and overall
>> agreement that version exposed by in-tree modules are useless and
>> sometimes even worse. They mislead users to expect some features
>> or lack of them based on this arbitrary string.
>>
>>>
>>> Are you not concerned that this is ABI and that specific userland may be
>>> relying on a specific info format and we could now be breaking their
>>> version checks? I do not disagree that the version is not particularly
>>> useful for in-tree kernel, but this is ABI, and breaking user-space is
>>> usually a source of support questions.
>>
>> See this Linus's response:
>> "The unified policy is pretty much that version codes do not matter, do
>> not exist, and do not get updated.
>>
>> Things are supposed to be backwards and forwards compatible, because
>> we don't accept breakage in user space anyway. So versioning is
>> pointless, and only causes problems."
>> https://lore.kernel.org/ksummit-discuss/CA+55aFx9A=5cc0QZ7CySC4F2K7eYaEfzkdYEc9JaNgCcV25=rg@mail.gmail.com/
>>
>> I also don't think that declaring every print in the kernel as ABI is
>> good thing to do. We are not breaking binary ABI and continuing to
>> supply some sort of versioning, but in unified format and not in wild
>> west way like it is now.
>>
>> So bottom line, if some REAL user space application (not test suites) relies
>> on specific version reported from ethtool, it is already broken and can't work
>> sanely for stable@, distros and upstream kernels.
>
> And about support questions,
> I'm already over-asked to update our mlx5 driver version every time some
> of our developers adds new feature (every week or two), which is insane.
> So I prefer to have one stable solution in the kernel.
Fair enough, can you spin a new version which provides this background
discussion and links into your commit message?
--
Florian
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH net-next v1] net/core: Replace driver version to be kernel version
2020-01-26 3:11 ` Florian Fainelli
@ 2020-01-26 9:44 ` Leon Romanovsky
0 siblings, 0 replies; 6+ messages in thread
From: Leon Romanovsky @ 2020-01-26 9:44 UTC (permalink / raw)
To: Florian Fainelli
Cc: David S. Miller, Jakub Kicinski, Michal Kubecek, Michal Kalderon,
linux-netdev, RDMA mailing list
On Sat, Jan 25, 2020 at 07:11:49PM -0800, Florian Fainelli wrote:
>
>
> On 1/25/2020 11:24 AM, Leon Romanovsky wrote:
> > On Sat, Jan 25, 2020 at 08:49:58PM +0200, Leon Romanovsky wrote:
> >> On Sat, Jan 25, 2020 at 08:55:01AM -0800, Florian Fainelli wrote:
> >>>
> >>>
> >>> On 1/25/2020 8:14 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>> ---
> >>>> Changelog:
> >>>> v1: Resend per-Dave's request
> >>>> https://lore.kernel.org/linux-rdma/20200125.101311.1924780619716720495.davem@davemloft.net
> >>>> No changes at all and applied cleanly on top of "3333e50b64fe Merge branch 'mlxsw-Offload-TBF'"
> >>>> v0: https://lore.kernel.org/linux-rdma/20200123130541.30473-1-leon@kernel.org
> >>>
> >>> There does not appear to be any explanation why we think this is a good
> >>> idea for *all* drivers, and not just the ones that are purely virtual?
> >>
> >> We beat this dead horse too many times already, latest discussion and
> >> justification can be found in that thread.
> >> https://lore.kernel.org/linux-rdma/20200122152627.14903-1-michal.kalderon@marvell.com/T/#md460ff8f976c532a89d6860411c3c50bb811038b
> >>
> >> However, it was discussed in ksummit mailing list too and overall
> >> agreement that version exposed by in-tree modules are useless and
> >> sometimes even worse. They mislead users to expect some features
> >> or lack of them based on this arbitrary string.
> >>
> >>>
> >>> Are you not concerned that this is ABI and that specific userland may be
> >>> relying on a specific info format and we could now be breaking their
> >>> version checks? I do not disagree that the version is not particularly
> >>> useful for in-tree kernel, but this is ABI, and breaking user-space is
> >>> usually a source of support questions.
> >>
> >> See this Linus's response:
> >> "The unified policy is pretty much that version codes do not matter, do
> >> not exist, and do not get updated.
> >>
> >> Things are supposed to be backwards and forwards compatible, because
> >> we don't accept breakage in user space anyway. So versioning is
> >> pointless, and only causes problems."
> >> https://lore.kernel.org/ksummit-discuss/CA+55aFx9A=5cc0QZ7CySC4F2K7eYaEfzkdYEc9JaNgCcV25=rg@mail.gmail.com/
> >>
> >> I also don't think that declaring every print in the kernel as ABI is
> >> good thing to do. We are not breaking binary ABI and continuing to
> >> supply some sort of versioning, but in unified format and not in wild
> >> west way like it is now.
> >>
> >> So bottom line, if some REAL user space application (not test suites) relies
> >> on specific version reported from ethtool, it is already broken and can't work
> >> sanely for stable@, distros and upstream kernels.
> >
> > And about support questions,
> > I'm already over-asked to update our mlx5 driver version every time some
> > of our developers adds new feature (every week or two), which is insane.
> > So I prefer to have one stable solution in the kernel.
>
> Fair enough, can you spin a new version which provides this background
> discussion and links into your commit message?
Thanks for the feedback, I'm doing it now.
> --
> Florian
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2020-01-26 9:44 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-01-25 16:14 [PATCH net-next v1] net/core: Replace driver version to be kernel version Leon Romanovsky
2020-01-25 16:55 ` Florian Fainelli
2020-01-25 18:49 ` Leon Romanovsky
2020-01-25 19:24 ` Leon Romanovsky
2020-01-26 3:11 ` Florian Fainelli
2020-01-26 9:44 ` Leon Romanovsky
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).