[PATCH rdma-next 0/2] Drop driver version in favor of default ethtool

[PATCH rdma-next 0/2] Drop driver version in favor of default ethtool

[Leon Romanovsky]

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

 drivers/infiniband/ulp/ipoib/ipoib.h                | 2 --
 drivers/infiniband/ulp/ipoib/ipoib_ethtool.c        | 3 ---
 drivers/infiniband/ulp/ipoib/ipoib_main.c           | 4 ----
 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 -----
 6 files changed, 17 deletions(-)

--
2.24.1

[PATCH rdma-next 1/2] RDMA/ipoib: Don't set constant driver version

[Leon Romanovsky]

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

[PATCH rdma-next 2/2] RDMA/opa_vnic: Delete driver version

[Leon Romanovsky]

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

Re: [PATCH rdma-next 2/2] RDMA/opa_vnic: Delete driver version

[Dennis Dalessandro]

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

Re: [PATCH rdma-next 1/2] RDMA/ipoib: Don't set constant driver version

[Dennis Dalessandro]

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>

Re: [PATCH rdma-next 2/2] RDMA/opa_vnic: Delete driver version

[Leon Romanovsky]

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

Re: [PATCH rdma-next 1/2] RDMA/ipoib: Don't set constant driver version

[Leon Romanovsky]

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>
>

Re: [PATCH rdma-next 0/2] Drop driver version in favor of default ethtool

[Jason Gunthorpe]

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