* [PATCH rdma-next 1/2] RDMA/ipoib: Don't set constant driver version
2020-02-20 7:12 [PATCH rdma-next 0/2] Drop driver version in favor of default ethtool Leon Romanovsky
@ 2020-02-20 7:12 ` Leon Romanovsky
2020-02-20 13:34 ` Dennis Dalessandro
2020-02-20 7:12 ` [PATCH rdma-next 2/2] RDMA/opa_vnic: Delete " Leon Romanovsky
2020-02-27 20:42 ` [PATCH rdma-next 0/2] Drop driver version in favor of default ethtool Jason Gunthorpe
2 siblings, 1 reply; 8+ messages in thread
From: Leon Romanovsky @ 2020-02-20 7:12 UTC (permalink / raw)
To: Doug Ledford, Jason Gunthorpe
Cc: Leon Romanovsky, RDMA mailing list, Dennis Dalessandro
From: Leon Romanovsky <leonro@mellanox.com>
There is no need to set driver version in in-tree kernel code.
Signed-off-by: Leon Romanovsky <leonro@mellanox.com>
---
drivers/infiniband/ulp/ipoib/ipoib.h | 2 --
drivers/infiniband/ulp/ipoib/ipoib_ethtool.c | 3 ---
drivers/infiniband/ulp/ipoib/ipoib_main.c | 4 ----
3 files changed, 9 deletions(-)
diff --git a/drivers/infiniband/ulp/ipoib/ipoib.h b/drivers/infiniband/ulp/ipoib/ipoib.h
index 2aa3457a30ce..e188a95984b5 100644
--- a/drivers/infiniband/ulp/ipoib/ipoib.h
+++ b/drivers/infiniband/ulp/ipoib/ipoib.h
@@ -838,6 +838,4 @@ extern int ipoib_debug_level;
#define IPOIB_QPN(ha) (be32_to_cpup((__be32 *) ha) & 0xffffff)
-extern const char ipoib_driver_version[];
-
#endif /* _IPOIB_H */
diff --git a/drivers/infiniband/ulp/ipoib/ipoib_ethtool.c b/drivers/infiniband/ulp/ipoib/ipoib_ethtool.c
index 63e4f9d15fd9..a47097d4577c 100644
--- a/drivers/infiniband/ulp/ipoib/ipoib_ethtool.c
+++ b/drivers/infiniband/ulp/ipoib/ipoib_ethtool.c
@@ -68,9 +68,6 @@ static void ipoib_get_drvinfo(struct net_device *netdev,
strlcpy(drvinfo->bus_info, dev_name(priv->ca->dev.parent),
sizeof(drvinfo->bus_info));
- strlcpy(drvinfo->version, ipoib_driver_version,
- sizeof(drvinfo->version));
-
strlcpy(drvinfo->driver, "ib_ipoib", sizeof(drvinfo->driver));
}
diff --git a/drivers/infiniband/ulp/ipoib/ipoib_main.c b/drivers/infiniband/ulp/ipoib/ipoib_main.c
index f630be064435..9fac1dbc8197 100644
--- a/drivers/infiniband/ulp/ipoib/ipoib_main.c
+++ b/drivers/infiniband/ulp/ipoib/ipoib_main.c
@@ -52,10 +52,6 @@
#include <linux/inetdevice.h>
#include <rdma/ib_cache.h>
-#define DRV_VERSION "1.0.0"
-
-const char ipoib_driver_version[] = DRV_VERSION;
-
MODULE_AUTHOR("Roland Dreier");
MODULE_DESCRIPTION("IP-over-InfiniBand net driver");
MODULE_LICENSE("Dual BSD/GPL");
--
2.24.1
^ permalink raw reply related [flat|nested] 8+ messages in thread* Re: [PATCH rdma-next 1/2] RDMA/ipoib: Don't set constant driver version
2020-02-20 7:12 ` [PATCH rdma-next 1/2] RDMA/ipoib: Don't set constant driver version Leon Romanovsky
@ 2020-02-20 13:34 ` Dennis Dalessandro
2020-02-20 13:46 ` Leon Romanovsky
0 siblings, 1 reply; 8+ messages in thread
From: Dennis Dalessandro @ 2020-02-20 13:34 UTC (permalink / raw)
To: Leon Romanovsky, Doug Ledford, Jason Gunthorpe
Cc: Leon Romanovsky, RDMA mailing list
On 2/20/2020 2:12 AM, Leon Romanovsky wrote:
> From: Leon Romanovsky <leonro@mellanox.com>
>
> There is no need to set driver version in in-tree kernel code.
>
> Signed-off-by: Leon Romanovsky <leonro@mellanox.com>
> ---
> drivers/infiniband/ulp/ipoib/ipoib.h | 2 --
> drivers/infiniband/ulp/ipoib/ipoib_ethtool.c | 3 ---
> drivers/infiniband/ulp/ipoib/ipoib_main.c | 4 ----
> 3 files changed, 9 deletions(-)
>
Same comments as the other patch, can we just remove the field from the
drvinfo struct altogether.
Reviewed-by: Dennis Dalessandro <dennis.dalessandro@intel.com>
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH rdma-next 1/2] RDMA/ipoib: Don't set constant driver version
2020-02-20 13:34 ` Dennis Dalessandro
@ 2020-02-20 13:46 ` Leon Romanovsky
0 siblings, 0 replies; 8+ messages in thread
From: Leon Romanovsky @ 2020-02-20 13:46 UTC (permalink / raw)
To: Dennis Dalessandro; +Cc: Doug Ledford, Jason Gunthorpe, RDMA mailing list
On Thu, Feb 20, 2020 at 08:34:00AM -0500, Dennis Dalessandro wrote:
> On 2/20/2020 2:12 AM, Leon Romanovsky wrote:
> > From: Leon Romanovsky <leonro@mellanox.com>
> >
> > There is no need to set driver version in in-tree kernel code.
> >
> > Signed-off-by: Leon Romanovsky <leonro@mellanox.com>
> > ---
> > drivers/infiniband/ulp/ipoib/ipoib.h | 2 --
> > drivers/infiniband/ulp/ipoib/ipoib_ethtool.c | 3 ---
> > drivers/infiniband/ulp/ipoib/ipoib_main.c | 4 ----
> > 3 files changed, 9 deletions(-)
> >
>
> Same comments as the other patch, can we just remove the field from the
> drvinfo struct altogether.
Ahh, and extra thing.
I put default version in ->version before calling to the driver. It
allows for out-of-tree drivers overwrite that field and continue to
manage their internal versions.
Thanks
>
> Reviewed-by: Dennis Dalessandro <dennis.dalessandro@intel.com>
>
^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH rdma-next 2/2] RDMA/opa_vnic: Delete driver version
2020-02-20 7:12 [PATCH rdma-next 0/2] Drop driver version in favor of default ethtool Leon Romanovsky
2020-02-20 7:12 ` [PATCH rdma-next 1/2] RDMA/ipoib: Don't set constant driver version Leon Romanovsky
@ 2020-02-20 7:12 ` Leon Romanovsky
2020-02-20 13:32 ` Dennis Dalessandro
2020-02-27 20:42 ` [PATCH rdma-next 0/2] Drop driver version in favor of default ethtool Jason Gunthorpe
2 siblings, 1 reply; 8+ messages in thread
From: Leon Romanovsky @ 2020-02-20 7:12 UTC (permalink / raw)
To: Doug Ledford, Jason Gunthorpe
Cc: Leon Romanovsky, RDMA mailing list, Dennis Dalessandro
From: Leon Romanovsky <leonro@mellanox.com>
The default version provided by "ethtool -i" it the correct way
to identify Driver version. There is no need to overwrite it.
Signed-off-by: Leon Romanovsky <leonro@mellanox.com>
---
drivers/infiniband/ulp/opa_vnic/opa_vnic_ethtool.c | 2 --
drivers/infiniband/ulp/opa_vnic/opa_vnic_internal.h | 1 -
drivers/infiniband/ulp/opa_vnic/opa_vnic_vema.c | 5 -----
3 files changed, 8 deletions(-)
diff --git a/drivers/infiniband/ulp/opa_vnic/opa_vnic_ethtool.c b/drivers/infiniband/ulp/opa_vnic/opa_vnic_ethtool.c
index 8ad7da989a0e..42d557dff19d 100644
--- a/drivers/infiniband/ulp/opa_vnic/opa_vnic_ethtool.c
+++ b/drivers/infiniband/ulp/opa_vnic/opa_vnic_ethtool.c
@@ -125,8 +125,6 @@ static void vnic_get_drvinfo(struct net_device *netdev,
struct ethtool_drvinfo *drvinfo)
{
strlcpy(drvinfo->driver, opa_vnic_driver_name, sizeof(drvinfo->driver));
- strlcpy(drvinfo->version, opa_vnic_driver_version,
- sizeof(drvinfo->version));
strlcpy(drvinfo->bus_info, dev_name(netdev->dev.parent),
sizeof(drvinfo->bus_info));
}
diff --git a/drivers/infiniband/ulp/opa_vnic/opa_vnic_internal.h b/drivers/infiniband/ulp/opa_vnic/opa_vnic_internal.h
index 6dbc08e1a6a6..dd942dd642bd 100644
--- a/drivers/infiniband/ulp/opa_vnic/opa_vnic_internal.h
+++ b/drivers/infiniband/ulp/opa_vnic/opa_vnic_internal.h
@@ -292,7 +292,6 @@ struct opa_vnic_mac_tbl_node {
hlist_for_each_entry(obj, &name[bkt], member)
extern char opa_vnic_driver_name[];
-extern const char opa_vnic_driver_version[];
struct opa_vnic_adapter *opa_vnic_add_netdev(struct ib_device *ibdev,
u8 port_num, u8 vport_num);
diff --git a/drivers/infiniband/ulp/opa_vnic/opa_vnic_vema.c b/drivers/infiniband/ulp/opa_vnic/opa_vnic_vema.c
index be5befd92d16..6e8d650c17c7 100644
--- a/drivers/infiniband/ulp/opa_vnic/opa_vnic_vema.c
+++ b/drivers/infiniband/ulp/opa_vnic/opa_vnic_vema.c
@@ -59,9 +59,7 @@
#include "opa_vnic_internal.h"
-#define DRV_VERSION "1.0"
char opa_vnic_driver_name[] = "opa_vnic";
-const char opa_vnic_driver_version[] = DRV_VERSION;
/*
* The trap service level is kept in bits 3 to 7 in the trap_sl_rsvd
@@ -1041,9 +1039,6 @@ static int __init opa_vnic_init(void)
{
int rc;
- pr_info("OPA Virtual Network Driver - v%s\n",
- opa_vnic_driver_version);
-
rc = ib_register_client(&opa_vnic_client);
if (rc)
pr_err("VNIC driver register failed %d\n", rc);
--
2.24.1
^ permalink raw reply related [flat|nested] 8+ messages in thread* Re: [PATCH rdma-next 2/2] RDMA/opa_vnic: Delete driver version
2020-02-20 7:12 ` [PATCH rdma-next 2/2] RDMA/opa_vnic: Delete " Leon Romanovsky
@ 2020-02-20 13:32 ` Dennis Dalessandro
2020-02-20 13:44 ` Leon Romanovsky
0 siblings, 1 reply; 8+ messages in thread
From: Dennis Dalessandro @ 2020-02-20 13:32 UTC (permalink / raw)
To: Leon Romanovsky, Doug Ledford, Jason Gunthorpe
Cc: Leon Romanovsky, RDMA mailing list
On 2/20/2020 2:12 AM, Leon Romanovsky wrote:
> From: Leon Romanovsky <leonro@mellanox.com>
>
> The default version provided by "ethtool -i" it the correct way
> to identify Driver version. There is no need to overwrite it.
>
> Signed-off-by: Leon Romanovsky <leonro@mellanox.com>
> ---
> drivers/infiniband/ulp/opa_vnic/opa_vnic_ethtool.c | 2 --
> drivers/infiniband/ulp/opa_vnic/opa_vnic_internal.h | 1 -
> drivers/infiniband/ulp/opa_vnic/opa_vnic_vema.c | 5 -----
> 3 files changed, 8 deletions(-)
>
> diff --git a/drivers/infiniband/ulp/opa_vnic/opa_vnic_ethtool.c b/drivers/infiniband/ulp/opa_vnic/opa_vnic_ethtool.c
> index 8ad7da989a0e..42d557dff19d 100644
> --- a/drivers/infiniband/ulp/opa_vnic/opa_vnic_ethtool.c
> +++ b/drivers/infiniband/ulp/opa_vnic/opa_vnic_ethtool.c
> @@ -125,8 +125,6 @@ static void vnic_get_drvinfo(struct net_device *netdev,
> struct ethtool_drvinfo *drvinfo)
> {
> strlcpy(drvinfo->driver, opa_vnic_driver_name, sizeof(drvinfo->driver));
> - strlcpy(drvinfo->version, opa_vnic_driver_version,
> - sizeof(drvinfo->version));
> strlcpy(drvinfo->bus_info, dev_name(netdev->dev.parent),
> sizeof(drvinfo->bus_info));
> }
Is there a patch series to get rid of drvinfo->version? Seems to me if
we don't want drivers to set it then we don't need it to begin with do we?
Regardless I don't have any objections to the patch. We've been down
this road with version numbers and I believe this was added to vnic
specifically to fill in something for ethtool.
Reviewed-by: Dennis Dalessandro <dennis.dalessandro@intel.com>
-Denny
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [PATCH rdma-next 2/2] RDMA/opa_vnic: Delete driver version
2020-02-20 13:32 ` Dennis Dalessandro
@ 2020-02-20 13:44 ` Leon Romanovsky
0 siblings, 0 replies; 8+ messages in thread
From: Leon Romanovsky @ 2020-02-20 13:44 UTC (permalink / raw)
To: Dennis Dalessandro; +Cc: Doug Ledford, Jason Gunthorpe, RDMA mailing list
On Thu, Feb 20, 2020 at 08:32:35AM -0500, Dennis Dalessandro wrote:
> On 2/20/2020 2:12 AM, Leon Romanovsky wrote:
> > From: Leon Romanovsky <leonro@mellanox.com>
> >
> > The default version provided by "ethtool -i" it the correct way
> > to identify Driver version. There is no need to overwrite it.
> >
> > Signed-off-by: Leon Romanovsky <leonro@mellanox.com>
> > ---
> > drivers/infiniband/ulp/opa_vnic/opa_vnic_ethtool.c | 2 --
> > drivers/infiniband/ulp/opa_vnic/opa_vnic_internal.h | 1 -
> > drivers/infiniband/ulp/opa_vnic/opa_vnic_vema.c | 5 -----
> > 3 files changed, 8 deletions(-)
> >
> > diff --git a/drivers/infiniband/ulp/opa_vnic/opa_vnic_ethtool.c b/drivers/infiniband/ulp/opa_vnic/opa_vnic_ethtool.c
> > index 8ad7da989a0e..42d557dff19d 100644
> > --- a/drivers/infiniband/ulp/opa_vnic/opa_vnic_ethtool.c
> > +++ b/drivers/infiniband/ulp/opa_vnic/opa_vnic_ethtool.c
> > @@ -125,8 +125,6 @@ static void vnic_get_drvinfo(struct net_device *netdev,
> > struct ethtool_drvinfo *drvinfo)
> > {
> > strlcpy(drvinfo->driver, opa_vnic_driver_name, sizeof(drvinfo->driver));
> > - strlcpy(drvinfo->version, opa_vnic_driver_version,
> > - sizeof(drvinfo->version));
> > strlcpy(drvinfo->bus_info, dev_name(netdev->dev.parent),
> > sizeof(drvinfo->bus_info));
> > }
>
> Is there a patch series to get rid of drvinfo->version? Seems to me if we
> don't want drivers to set it then we don't need it to begin with do we?
Unfortunately struct ethtool_drvinfo is defined in include/uapi/linux/ethtool.h
and we can't change it without breaking ethtool.
My WIP in progress branch (based on net-next) is located here:
https://git.kernel.org/pub/scm/linux/kernel/git/leon/linux-rdma.git/log/?h=ethtool
At the end of my journey, we will have checkpatch.pl patch and update of
coding style.
>
> Regardless I don't have any objections to the patch. We've been down this
> road with version numbers and I believe this was added to vnic specifically
> to fill in something for ethtool.
And now, you will be able to see real version :)
>
> Reviewed-by: Dennis Dalessandro <dennis.dalessandro@intel.com>
Thanks
>
> -Denny
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH rdma-next 0/2] Drop driver version in favor of default ethtool
2020-02-20 7:12 [PATCH rdma-next 0/2] Drop driver version in favor of default ethtool Leon Romanovsky
2020-02-20 7:12 ` [PATCH rdma-next 1/2] RDMA/ipoib: Don't set constant driver version Leon Romanovsky
2020-02-20 7:12 ` [PATCH rdma-next 2/2] RDMA/opa_vnic: Delete " Leon Romanovsky
@ 2020-02-27 20:42 ` Jason Gunthorpe
2 siblings, 0 replies; 8+ messages in thread
From: Jason Gunthorpe @ 2020-02-27 20:42 UTC (permalink / raw)
To: Leon Romanovsky
Cc: Doug Ledford, Leon Romanovsky, RDMA mailing list, Dennis Dalessandro
On Thu, Feb 20, 2020 at 09:12:37AM +0200, Leon Romanovsky wrote:
> From: Leon Romanovsky <leonro@mellanox.com>
>
> Following the change in ethtool to provide default driver version which
> is aligned with kernel in use, drop the static assignments of driver
> versions in IPoIB and OPA_VNIC.
>
> Thanks
>
> Leon Romanovsky (2):
> RDMA/ipoib: Don't set constant driver version
> RDMA/opa_vnic: Delete driver version
Applied to for-next
Jason
^ permalink raw reply [flat|nested] 8+ messages in thread