All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v5] libibverbs: display gid type in ibv_devinfo
@ 2020-02-03 17:52 Devesh Sharma
  2020-02-03 17:53 ` Jason Gunthorpe
  2020-02-03 19:46 ` Leon Romanovsky
  0 siblings, 2 replies; 13+ messages in thread
From: Devesh Sharma @ 2020-02-03 17:52 UTC (permalink / raw)
  To: linux-rdma; +Cc: jgg, leon

It becomes difficult to make out from the output of ibv_devinfo
if a particular gid index is RoCE v2 or not.

Adding a string to the output of ibv_devinfo -v to display the
gid type at the end of gid.

The output would look something like below:
$ ibv_devinfo -v -d bnxt_re2
hca_id: bnxt_re2
 transport:             InfiniBand (0)
 fw_ver:                216.0.220.0
 node_guid:             b226:28ff:fed3:b0f0
 sys_image_guid:        b226:28ff:fed3:b0f0
  .
  .
  .
  .
       phys_state:      LINK_UP (5)
       GID[  0]:               fe80::b226:28ff:fed3:b0f0, IB/RoCE v1
       GID[  1]:               fe80::b226:28ff:fed3:b0f0, RoCE v2
       GID[  2]:               ::ffff:192.170.1.101, IB/RoCE v1
       GID[  3]:               ::ffff:192.170.1.101, RoCE v2
$
$

Reviewed-by: Jason Gunthrope <jgg@ziepe.ca>
Reviewed-by: Leon Romanovsky <leon@kernel.org>
Reviewed-by: Gal Pressman <galpress@amazon.com>
Reviewed-by: Parav Pandit <parav@mellanox.com>
Signed-off-by: Devesh Sharma <devesh.sharma@broadcom.com>
---
 libibverbs/driver.h           |  1 +
 libibverbs/examples/devinfo.c | 35 ++++++++++++++++++++++++-----------
 2 files changed, 25 insertions(+), 11 deletions(-)

diff --git a/libibverbs/driver.h b/libibverbs/driver.h
index a0e6f89..fc0699d 100644
--- a/libibverbs/driver.h
+++ b/libibverbs/driver.h
@@ -84,6 +84,7 @@ enum verbs_qp_mask {
 enum ibv_gid_type {
 	IBV_GID_TYPE_IB_ROCE_V1,
 	IBV_GID_TYPE_ROCE_V2,
+	IBV_GID_TYPE_INVALID
 };
 
 enum ibv_mr_type {
diff --git a/libibverbs/examples/devinfo.c b/libibverbs/examples/devinfo.c
index bf53eac..568712c 100644
--- a/libibverbs/examples/devinfo.c
+++ b/libibverbs/examples/devinfo.c
@@ -40,6 +40,7 @@
 #include <getopt.h>
 #include <endian.h>
 #include <inttypes.h>
+#include <arpa/inet.h>
 
 #include <infiniband/verbs.h>
 #include <infiniband/driver.h>
@@ -162,8 +163,19 @@ static const char *vl_str(uint8_t vl_num)
 	}
 }
 
+static const char *gid_type_str(enum ibv_gid_type type)
+{
+	switch (type) {
+	case IBV_GID_TYPE_IB_ROCE_V1: return "IB/RoCE v1";
+	case IBV_GID_TYPE_ROCE_V2: return "RoCE v2";
+	default: return "Invalid gid type";
+	}
+}
+
 static int print_all_port_gids(struct ibv_context *ctx, uint8_t port_num, int tbl_len)
 {
+	char gid_str[INET6_ADDRSTRLEN] = {};
+	enum ibv_gid_type type;
 	union ibv_gid gid;
 	int rc = 0;
 	int i;
@@ -175,17 +187,18 @@ static int print_all_port_gids(struct ibv_context *ctx, uint8_t port_num, int tb
 			       port_num, i);
 			return rc;
 		}
-		if (!null_gid(&gid))
-			printf("\t\t\tGID[%3d]:\t\t%02x%02x:%02x%02x:%02x%02x:%02x%02x:%02x%02x:%02x%02x:%02x%02x:%02x%02x\n",
-			       i,
-			       gid.raw[ 0], gid.raw[ 1],
-			       gid.raw[ 2], gid.raw[ 3],
-			       gid.raw[ 4], gid.raw[ 5],
-			       gid.raw[ 6], gid.raw[ 7],
-			       gid.raw[ 8], gid.raw[ 9],
-			       gid.raw[10], gid.raw[11],
-			       gid.raw[12], gid.raw[13],
-			       gid.raw[14], gid.raw[15]);
+
+		rc = ibv_query_gid_type(ctx, port_num, i, &type);
+		if (rc) {
+			rc = 0;
+			type = IBV_GID_TYPE_INVALID;
+		}
+
+		if (!null_gid(&gid)) {
+			inet_ntop(AF_INET6, gid.raw, gid_str, sizeof(gid_str));
+			printf("\t\t\tGID[%3d]:\t\t%s, %s\n", i, gid_str,
+			       gid_type_str(type));
+		}
 	}
 	return rc;
 }
-- 
1.8.3.1


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* Re: [PATCH v5] libibverbs: display gid type in ibv_devinfo
  2020-02-03 17:52 [PATCH v5] libibverbs: display gid type in ibv_devinfo Devesh Sharma
@ 2020-02-03 17:53 ` Jason Gunthorpe
  2020-02-03 18:01   ` Devesh Sharma
  2020-02-03 19:46 ` Leon Romanovsky
  1 sibling, 1 reply; 13+ messages in thread
From: Jason Gunthorpe @ 2020-02-03 17:53 UTC (permalink / raw)
  To: Devesh Sharma; +Cc: linux-rdma, leon

On Mon, Feb 03, 2020 at 12:52:04PM -0500, Devesh Sharma wrote:
> It becomes difficult to make out from the output of ibv_devinfo
> if a particular gid index is RoCE v2 or not.
> 
> Adding a string to the output of ibv_devinfo -v to display the
> gid type at the end of gid.
> 
> The output would look something like below:
> $ ibv_devinfo -v -d bnxt_re2
> hca_id: bnxt_re2
>  transport:             InfiniBand (0)
>  fw_ver:                216.0.220.0
>  node_guid:             b226:28ff:fed3:b0f0
>  sys_image_guid:        b226:28ff:fed3:b0f0
>   .
>   .
>   .
>   .
>        phys_state:      LINK_UP (5)
>        GID[  0]:               fe80::b226:28ff:fed3:b0f0, IB/RoCE v1
>        GID[  1]:               fe80::b226:28ff:fed3:b0f0, RoCE v2
>        GID[  2]:               ::ffff:192.170.1.101, IB/RoCE v1
>        GID[  3]:               ::ffff:192.170.1.101, RoCE v2

v1 GIDs are GIDs and should never be formed as IPv6 addreses..

Jason

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v5] libibverbs: display gid type in ibv_devinfo
  2020-02-03 17:53 ` Jason Gunthorpe
@ 2020-02-03 18:01   ` Devesh Sharma
  2020-02-03 18:04     ` Jason Gunthorpe
  0 siblings, 1 reply; 13+ messages in thread
From: Devesh Sharma @ 2020-02-03 18:01 UTC (permalink / raw)
  To: Jason Gunthorpe; +Cc: linux-rdma, Leon Romanovsky

On Mon, Feb 3, 2020 at 11:23 PM Jason Gunthorpe <jgg@mellanox.com> wrote:
>
> On Mon, Feb 03, 2020 at 12:52:04PM -0500, Devesh Sharma wrote:
> > It becomes difficult to make out from the output of ibv_devinfo
> > if a particular gid index is RoCE v2 or not.
> >
> > Adding a string to the output of ibv_devinfo -v to display the
> > gid type at the end of gid.
> >
> > The output would look something like below:
> > $ ibv_devinfo -v -d bnxt_re2
> > hca_id: bnxt_re2
> >  transport:             InfiniBand (0)
> >  fw_ver:                216.0.220.0
> >  node_guid:             b226:28ff:fed3:b0f0
> >  sys_image_guid:        b226:28ff:fed3:b0f0
> >   .
> >   .
> >   .
> >   .
> >        phys_state:      LINK_UP (5)
> >        GID[  0]:               fe80::b226:28ff:fed3:b0f0, IB/RoCE v1
> >        GID[  1]:               fe80::b226:28ff:fed3:b0f0, RoCE v2
> >        GID[  2]:               ::ffff:192.170.1.101, IB/RoCE v1
> >        GID[  3]:               ::ffff:192.170.1.101, RoCE v2
>
> v1 GIDs are GIDs and should never be formed as IPv6 addreses..
So, V1 gids would fall back to old style of display and there will be
one more check for gid-type inside the loop...
>
> Jason

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v5] libibverbs: display gid type in ibv_devinfo
  2020-02-03 18:01   ` Devesh Sharma
@ 2020-02-03 18:04     ` Jason Gunthorpe
  2020-02-03 18:09       ` Devesh Sharma
  2020-02-04  4:27       ` Parav Pandit
  0 siblings, 2 replies; 13+ messages in thread
From: Jason Gunthorpe @ 2020-02-03 18:04 UTC (permalink / raw)
  To: Devesh Sharma, Parav Pandit; +Cc: linux-rdma, Leon Romanovsky

On Mon, Feb 03, 2020 at 11:31:06PM +0530, Devesh Sharma wrote:
> On Mon, Feb 3, 2020 at 11:23 PM Jason Gunthorpe <jgg@mellanox.com> wrote:
> >
> > On Mon, Feb 03, 2020 at 12:52:04PM -0500, Devesh Sharma wrote:
> > > It becomes difficult to make out from the output of ibv_devinfo
> > > if a particular gid index is RoCE v2 or not.
> > >
> > > Adding a string to the output of ibv_devinfo -v to display the
> > > gid type at the end of gid.
> > >
> > > The output would look something like below:
> > > $ ibv_devinfo -v -d bnxt_re2
> > > hca_id: bnxt_re2
> > >  transport:             InfiniBand (0)
> > >  fw_ver:                216.0.220.0
> > >  node_guid:             b226:28ff:fed3:b0f0
> > >  sys_image_guid:        b226:28ff:fed3:b0f0
> > >   .
> > >   .
> > >   .
> > >   .
> > >        phys_state:      LINK_UP (5)
> > >        GID[  0]:               fe80::b226:28ff:fed3:b0f0, IB/RoCE v1
> > >        GID[  1]:               fe80::b226:28ff:fed3:b0f0, RoCE v2
> > >        GID[  2]:               ::ffff:192.170.1.101, IB/RoCE v1
> > >        GID[  3]:               ::ffff:192.170.1.101, RoCE v2
> >
> > v1 GIDs are GIDs and should never be formed as IPv6 addreses..
> So, V1 gids would fall back to old style of display and there will be
> one more check for gid-type inside the loop...

Yes

Parav should we show both the v6 and classic format for a v2 GID? ie

        GID[  3]:               0000:0000:0000:ffff:xxxx:xxxx:xxxx, RoCE v2
                                ::ffff:192.170.1.101

Lets also supress the 'IB/RoCE v1' string on !roce device. IB only has
one GID type, there is no reason to print anything

Jason

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v5] libibverbs: display gid type in ibv_devinfo
  2020-02-03 18:04     ` Jason Gunthorpe
@ 2020-02-03 18:09       ` Devesh Sharma
  2020-02-04  4:27       ` Parav Pandit
  1 sibling, 0 replies; 13+ messages in thread
From: Devesh Sharma @ 2020-02-03 18:09 UTC (permalink / raw)
  To: Jason Gunthorpe; +Cc: Parav Pandit, linux-rdma, Leon Romanovsky

On Mon, Feb 3, 2020 at 11:34 PM Jason Gunthorpe <jgg@mellanox.com> wrote:
>
> On Mon, Feb 03, 2020 at 11:31:06PM +0530, Devesh Sharma wrote:
> > On Mon, Feb 3, 2020 at 11:23 PM Jason Gunthorpe <jgg@mellanox.com> wrote:
> > >
> > > On Mon, Feb 03, 2020 at 12:52:04PM -0500, Devesh Sharma wrote:
> > > > It becomes difficult to make out from the output of ibv_devinfo
> > > > if a particular gid index is RoCE v2 or not.
> > > >
> > > > Adding a string to the output of ibv_devinfo -v to display the
> > > > gid type at the end of gid.
> > > >
> > > > The output would look something like below:
> > > > $ ibv_devinfo -v -d bnxt_re2
> > > > hca_id: bnxt_re2
> > > >  transport:             InfiniBand (0)
> > > >  fw_ver:                216.0.220.0
> > > >  node_guid:             b226:28ff:fed3:b0f0
> > > >  sys_image_guid:        b226:28ff:fed3:b0f0
> > > >   .
> > > >   .
> > > >   .
> > > >   .
> > > >        phys_state:      LINK_UP (5)
> > > >        GID[  0]:               fe80::b226:28ff:fed3:b0f0, IB/RoCE v1
> > > >        GID[  1]:               fe80::b226:28ff:fed3:b0f0, RoCE v2
> > > >        GID[  2]:               ::ffff:192.170.1.101, IB/RoCE v1
> > > >        GID[  3]:               ::ffff:192.170.1.101, RoCE v2
> > >
> > > v1 GIDs are GIDs and should never be formed as IPv6 addreses..
> > So, V1 gids would fall back to old style of display and there will be
> > one more check for gid-type inside the loop...
>
> Yes
Sure.
>
> Parav should we show both the v6 and classic format for a v2 GID? ie
>
>         GID[  3]:               0000:0000:0000:ffff:xxxx:xxxx:xxxx, RoCE v2
>                                 ::ffff:192.170.1.101
>
> Lets also supress the 'IB/RoCE v1' string on !roce device. IB only has
> one GID type, there is no reason to print anything
W.r.t. IB devices, this makes sense, While sending v1 I had a thought
in mind on IB devices.
>
> Jason

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v5] libibverbs: display gid type in ibv_devinfo
  2020-02-03 17:52 [PATCH v5] libibverbs: display gid type in ibv_devinfo Devesh Sharma
  2020-02-03 17:53 ` Jason Gunthorpe
@ 2020-02-03 19:46 ` Leon Romanovsky
  2020-02-04  5:07   ` Devesh Sharma
  1 sibling, 1 reply; 13+ messages in thread
From: Leon Romanovsky @ 2020-02-03 19:46 UTC (permalink / raw)
  To: Devesh Sharma; +Cc: linux-rdma, jgg

On Mon, Feb 03, 2020 at 12:52:04PM -0500, Devesh Sharma wrote:
> It becomes difficult to make out from the output of ibv_devinfo
> if a particular gid index is RoCE v2 or not.
>
> Adding a string to the output of ibv_devinfo -v to display the
> gid type at the end of gid.
>
> The output would look something like below:
> $ ibv_devinfo -v -d bnxt_re2
> hca_id: bnxt_re2
>  transport:             InfiniBand (0)
>  fw_ver:                216.0.220.0
>  node_guid:             b226:28ff:fed3:b0f0
>  sys_image_guid:        b226:28ff:fed3:b0f0
>   .
>   .
>   .
>   .
>        phys_state:      LINK_UP (5)
>        GID[  0]:               fe80::b226:28ff:fed3:b0f0, IB/RoCE v1
>        GID[  1]:               fe80::b226:28ff:fed3:b0f0, RoCE v2
>        GID[  2]:               ::ffff:192.170.1.101, IB/RoCE v1
>        GID[  3]:               ::ffff:192.170.1.101, RoCE v2
> $
> $
>
> Reviewed-by: Jason Gunthrope <jgg@ziepe.ca>
> Reviewed-by: Leon Romanovsky <leon@kernel.org>
> Reviewed-by: Gal Pressman <galpress@amazon.com>
> Reviewed-by: Parav Pandit <parav@mellanox.com>
> Signed-off-by: Devesh Sharma <devesh.sharma@broadcom.com>
> ---
>  libibverbs/driver.h           |  1 +
>  libibverbs/examples/devinfo.c | 35 ++++++++++++++++++++++++-----------
>  2 files changed, 25 insertions(+), 11 deletions(-)
>
> diff --git a/libibverbs/driver.h b/libibverbs/driver.h
> index a0e6f89..fc0699d 100644
> --- a/libibverbs/driver.h
> +++ b/libibverbs/driver.h
> @@ -84,6 +84,7 @@ enum verbs_qp_mask {
>  enum ibv_gid_type {
>  	IBV_GID_TYPE_IB_ROCE_V1,
>  	IBV_GID_TYPE_ROCE_V2,
> +	IBV_GID_TYPE_INVALID
>  };
>

Agree with Gal, this hunk shouldn't be in the patch at all.

Thanks

^ permalink raw reply	[flat|nested] 13+ messages in thread

* RE: [PATCH v5] libibverbs: display gid type in ibv_devinfo
  2020-02-03 18:04     ` Jason Gunthorpe
  2020-02-03 18:09       ` Devesh Sharma
@ 2020-02-04  4:27       ` Parav Pandit
  2020-02-04  5:08         ` Devesh Sharma
  2020-02-04  7:17         ` Leon Romanovsky
  1 sibling, 2 replies; 13+ messages in thread
From: Parav Pandit @ 2020-02-04  4:27 UTC (permalink / raw)
  To: Jason Gunthorpe, Devesh Sharma; +Cc: linux-rdma, Leon Romanovsky



> From: Jason Gunthorpe <jgg@mellanox.com>
> Sent: Monday, February 3, 2020 11:34 PM
> 
> On Mon, Feb 03, 2020 at 11:31:06PM +0530, Devesh Sharma wrote:
> > On Mon, Feb 3, 2020 at 11:23 PM Jason Gunthorpe <jgg@mellanox.com>
> wrote:
> > >
> > > On Mon, Feb 03, 2020 at 12:52:04PM -0500, Devesh Sharma wrote:
> > > > It becomes difficult to make out from the output of ibv_devinfo if
> > > > a particular gid index is RoCE v2 or not.
> > > >
> > > > Adding a string to the output of ibv_devinfo -v to display the gid
> > > > type at the end of gid.
> > > >
> > > > The output would look something like below:
> > > > $ ibv_devinfo -v -d bnxt_re2
> > > > hca_id: bnxt_re2
> > > >  transport:             InfiniBand (0)
> > > >  fw_ver:                216.0.220.0
> > > >  node_guid:             b226:28ff:fed3:b0f0
> > > >  sys_image_guid:        b226:28ff:fed3:b0f0
> > > >   .
> > > >   .
> > > >   .
> > > >   .
> > > >        phys_state:      LINK_UP (5)
> > > >        GID[  0]:               fe80::b226:28ff:fed3:b0f0, IB/RoCE v1
> > > >        GID[  1]:               fe80::b226:28ff:fed3:b0f0, RoCE v2
> > > >        GID[  2]:               ::ffff:192.170.1.101, IB/RoCE v1
> > > >        GID[  3]:               ::ffff:192.170.1.101, RoCE v2
> > >
> > > v1 GIDs are GIDs and should never be formed as IPv6 addreses..
> > So, V1 gids would fall back to old style of display and there will be
> > one more check for gid-type inside the loop...
> 
> Yes
> 
> Parav should we show both the v6 and classic format for a v2 GID? ie
> 
>         GID[  3]:               0000:0000:0000:ffff:xxxx:xxxx:xxxx, RoCE v2
>                                 ::ffff:192.170.1.101
> 
Due to lack of support of GID's netdev, v1/v2 type info in ibv_devinfo output, most users that I know of are using non upstream show_gids script.
So changing format here shouldn't break the existing users scripts.
There may be some scripts that may find this format different.
So I think printing both is likely a more safer option.

> Lets also supress the 'IB/RoCE v1' string on !roce device. IB only has one GID
> type, there is no reason to print anything
> 
> Jason

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v5] libibverbs: display gid type in ibv_devinfo
  2020-02-03 19:46 ` Leon Romanovsky
@ 2020-02-04  5:07   ` Devesh Sharma
  0 siblings, 0 replies; 13+ messages in thread
From: Devesh Sharma @ 2020-02-04  5:07 UTC (permalink / raw)
  To: Leon Romanovsky; +Cc: linux-rdma, Jason Gunthorpe

On Tue, Feb 4, 2020 at 1:16 AM Leon Romanovsky <leon@kernel.org> wrote:
>
> On Mon, Feb 03, 2020 at 12:52:04PM -0500, Devesh Sharma wrote:
> > It becomes difficult to make out from the output of ibv_devinfo
> > if a particular gid index is RoCE v2 or not.
> >
> > Adding a string to the output of ibv_devinfo -v to display the
> > gid type at the end of gid.
> >
> > The output would look something like below:
> > $ ibv_devinfo -v -d bnxt_re2
> > hca_id: bnxt_re2
> >  transport:             InfiniBand (0)
> >  fw_ver:                216.0.220.0
> >  node_guid:             b226:28ff:fed3:b0f0
> >  sys_image_guid:        b226:28ff:fed3:b0f0
> >   .
> >   .
> >   .
> >   .
> >        phys_state:      LINK_UP (5)
> >        GID[  0]:               fe80::b226:28ff:fed3:b0f0, IB/RoCE v1
> >        GID[  1]:               fe80::b226:28ff:fed3:b0f0, RoCE v2
> >        GID[  2]:               ::ffff:192.170.1.101, IB/RoCE v1
> >        GID[  3]:               ::ffff:192.170.1.101, RoCE v2
> > $
> > $
> >
> > Reviewed-by: Jason Gunthrope <jgg@ziepe.ca>
> > Reviewed-by: Leon Romanovsky <leon@kernel.org>
> > Reviewed-by: Gal Pressman <galpress@amazon.com>
> > Reviewed-by: Parav Pandit <parav@mellanox.com>
> > Signed-off-by: Devesh Sharma <devesh.sharma@broadcom.com>
> > ---
> >  libibverbs/driver.h           |  1 +
> >  libibverbs/examples/devinfo.c | 35 ++++++++++++++++++++++++-----------
> >  2 files changed, 25 insertions(+), 11 deletions(-)
> >
> > diff --git a/libibverbs/driver.h b/libibverbs/driver.h
> > index a0e6f89..fc0699d 100644
> > --- a/libibverbs/driver.h
> > +++ b/libibverbs/driver.h
> > @@ -84,6 +84,7 @@ enum verbs_qp_mask {
> >  enum ibv_gid_type {
> >       IBV_GID_TYPE_IB_ROCE_V1,
> >       IBV_GID_TYPE_ROCE_V2,
> > +     IBV_GID_TYPE_INVALID
> >  };
> >
>
> Agree with Gal, this hunk shouldn't be in the patch at all.
Okay, will change.
>
> Thanks

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v5] libibverbs: display gid type in ibv_devinfo
  2020-02-04  4:27       ` Parav Pandit
@ 2020-02-04  5:08         ` Devesh Sharma
  2020-02-04  7:17         ` Leon Romanovsky
  1 sibling, 0 replies; 13+ messages in thread
From: Devesh Sharma @ 2020-02-04  5:08 UTC (permalink / raw)
  To: Parav Pandit; +Cc: Jason Gunthorpe, linux-rdma, Leon Romanovsky

On Tue, Feb 4, 2020 at 9:57 AM Parav Pandit <parav@mellanox.com> wrote:
>
>
>
> > From: Jason Gunthorpe <jgg@mellanox.com>
> > Sent: Monday, February 3, 2020 11:34 PM
> >
> > On Mon, Feb 03, 2020 at 11:31:06PM +0530, Devesh Sharma wrote:
> > > On Mon, Feb 3, 2020 at 11:23 PM Jason Gunthorpe <jgg@mellanox.com>
> > wrote:
> > > >
> > > > On Mon, Feb 03, 2020 at 12:52:04PM -0500, Devesh Sharma wrote:
> > > > > It becomes difficult to make out from the output of ibv_devinfo if
> > > > > a particular gid index is RoCE v2 or not.
> > > > >
> > > > > Adding a string to the output of ibv_devinfo -v to display the gid
> > > > > type at the end of gid.
> > > > >
> > > > > The output would look something like below:
> > > > > $ ibv_devinfo -v -d bnxt_re2
> > > > > hca_id: bnxt_re2
> > > > >  transport:             InfiniBand (0)
> > > > >  fw_ver:                216.0.220.0
> > > > >  node_guid:             b226:28ff:fed3:b0f0
> > > > >  sys_image_guid:        b226:28ff:fed3:b0f0
> > > > >   .
> > > > >   .
> > > > >   .
> > > > >   .
> > > > >        phys_state:      LINK_UP (5)
> > > > >        GID[  0]:               fe80::b226:28ff:fed3:b0f0, IB/RoCE v1
> > > > >        GID[  1]:               fe80::b226:28ff:fed3:b0f0, RoCE v2
> > > > >        GID[  2]:               ::ffff:192.170.1.101, IB/RoCE v1
> > > > >        GID[  3]:               ::ffff:192.170.1.101, RoCE v2
> > > >
> > > > v1 GIDs are GIDs and should never be formed as IPv6 addreses..
> > > So, V1 gids would fall back to old style of display and there will be
> > > one more check for gid-type inside the loop...
> >
> > Yes
> >
> > Parav should we show both the v6 and classic format for a v2 GID? ie
> >
> >         GID[  3]:               0000:0000:0000:ffff:xxxx:xxxx:xxxx, RoCE v2
> >                                 ::ffff:192.170.1.101
> >
> Due to lack of support of GID's netdev, v1/v2 type info in ibv_devinfo output, most users that I know of are using non upstream show_gids script.
> So changing format here shouldn't break the existing users scripts.
> There may be some scripts that may find this format different.
> So I think printing both is likely a more safer option.
>
> > Lets also supress the 'IB/RoCE v1' string on !roce device. IB only has one GID
> > type, there is no reason to print anything
> >
Okay, so two changes and one from Gal's comment. Will change.
> > Jason

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v5] libibverbs: display gid type in ibv_devinfo
  2020-02-04  4:27       ` Parav Pandit
  2020-02-04  5:08         ` Devesh Sharma
@ 2020-02-04  7:17         ` Leon Romanovsky
  2020-02-04  7:25           ` Parav Pandit
  1 sibling, 1 reply; 13+ messages in thread
From: Leon Romanovsky @ 2020-02-04  7:17 UTC (permalink / raw)
  To: Parav Pandit; +Cc: Jason Gunthorpe, Devesh Sharma, linux-rdma

On Tue, Feb 04, 2020 at 04:27:45AM +0000, Parav Pandit wrote:
>
>
> > From: Jason Gunthorpe <jgg@mellanox.com>
> > Sent: Monday, February 3, 2020 11:34 PM
> >
> > On Mon, Feb 03, 2020 at 11:31:06PM +0530, Devesh Sharma wrote:
> > > On Mon, Feb 3, 2020 at 11:23 PM Jason Gunthorpe <jgg@mellanox.com>
> > wrote:
> > > >
> > > > On Mon, Feb 03, 2020 at 12:52:04PM -0500, Devesh Sharma wrote:
> > > > > It becomes difficult to make out from the output of ibv_devinfo if
> > > > > a particular gid index is RoCE v2 or not.
> > > > >
> > > > > Adding a string to the output of ibv_devinfo -v to display the gid
> > > > > type at the end of gid.
> > > > >
> > > > > The output would look something like below:
> > > > > $ ibv_devinfo -v -d bnxt_re2
> > > > > hca_id: bnxt_re2
> > > > >  transport:             InfiniBand (0)
> > > > >  fw_ver:                216.0.220.0
> > > > >  node_guid:             b226:28ff:fed3:b0f0
> > > > >  sys_image_guid:        b226:28ff:fed3:b0f0
> > > > >   .
> > > > >   .
> > > > >   .
> > > > >   .
> > > > >        phys_state:      LINK_UP (5)
> > > > >        GID[  0]:               fe80::b226:28ff:fed3:b0f0, IB/RoCE v1
> > > > >        GID[  1]:               fe80::b226:28ff:fed3:b0f0, RoCE v2
> > > > >        GID[  2]:               ::ffff:192.170.1.101, IB/RoCE v1
> > > > >        GID[  3]:               ::ffff:192.170.1.101, RoCE v2
> > > >
> > > > v1 GIDs are GIDs and should never be formed as IPv6 addreses..
> > > So, V1 gids would fall back to old style of display and there will be
> > > one more check for gid-type inside the loop...
> >
> > Yes
> >
> > Parav should we show both the v6 and classic format for a v2 GID? ie
> >
> >         GID[  3]:               0000:0000:0000:ffff:xxxx:xxxx:xxxx, RoCE v2
> >                                 ::ffff:192.170.1.101
> >
> Due to lack of support of GID's netdev, v1/v2 type info in ibv_devinfo output, most users that I know of are using non upstream show_gids script.
> So changing format here shouldn't break the existing users scripts.
> There may be some scripts that may find this format different.
> So I think printing both is likely a more safer option.

I don't understand this argument. Output from example tool (ibv_devinfo)
inside libibverbs can't be considered API and we can't live in constant
fear that some user script will break. Of course, we will try to keep it
stable, but there is no such promise.

Thanks

>
> > Lets also supress the 'IB/RoCE v1' string on !roce device. IB only has one GID
> > type, there is no reason to print anything
> >
> > Jason

^ permalink raw reply	[flat|nested] 13+ messages in thread

* RE: [PATCH v5] libibverbs: display gid type in ibv_devinfo
  2020-02-04  7:17         ` Leon Romanovsky
@ 2020-02-04  7:25           ` Parav Pandit
  2020-02-04  8:44             ` Devesh Sharma
  0 siblings, 1 reply; 13+ messages in thread
From: Parav Pandit @ 2020-02-04  7:25 UTC (permalink / raw)
  To: Leon Romanovsky; +Cc: Jason Gunthorpe, Devesh Sharma, linux-rdma

Hi Leon,

> From: Leon Romanovsky <leon@kernel.org>
> Sent: Tuesday, February 4, 2020 12:48 PM
> 
> On Tue, Feb 04, 2020 at 04:27:45AM +0000, Parav Pandit wrote:
> >
> >
> > > From: Jason Gunthorpe <jgg@mellanox.com>
> > > Sent: Monday, February 3, 2020 11:34 PM
> > >
> > > On Mon, Feb 03, 2020 at 11:31:06PM +0530, Devesh Sharma wrote:
> > > > On Mon, Feb 3, 2020 at 11:23 PM Jason Gunthorpe <jgg@mellanox.com>
> > > wrote:
> > > > >
> > > > > On Mon, Feb 03, 2020 at 12:52:04PM -0500, Devesh Sharma wrote:
> > > > > > It becomes difficult to make out from the output of
> > > > > > ibv_devinfo if a particular gid index is RoCE v2 or not.
> > > > > >
> > > > > > Adding a string to the output of ibv_devinfo -v to display the
> > > > > > gid type at the end of gid.
> > > > > >
> > > > > > The output would look something like below:
> > > > > > $ ibv_devinfo -v -d bnxt_re2
> > > > > > hca_id: bnxt_re2
> > > > > >  transport:             InfiniBand (0)
> > > > > >  fw_ver:                216.0.220.0
> > > > > >  node_guid:             b226:28ff:fed3:b0f0
> > > > > >  sys_image_guid:        b226:28ff:fed3:b0f0
> > > > > >   .
> > > > > >   .
> > > > > >   .
> > > > > >   .
> > > > > >        phys_state:      LINK_UP (5)
> > > > > >        GID[  0]:               fe80::b226:28ff:fed3:b0f0, IB/RoCE v1
> > > > > >        GID[  1]:               fe80::b226:28ff:fed3:b0f0, RoCE v2
> > > > > >        GID[  2]:               ::ffff:192.170.1.101, IB/RoCE v1
> > > > > >        GID[  3]:               ::ffff:192.170.1.101, RoCE v2
> > > > >
> > > > > v1 GIDs are GIDs and should never be formed as IPv6 addreses..
> > > > So, V1 gids would fall back to old style of display and there will
> > > > be one more check for gid-type inside the loop...
> > >
> > > Yes
> > >
> > > Parav should we show both the v6 and classic format for a v2 GID? ie
> > >
> > >         GID[  3]:               0000:0000:0000:ffff:xxxx:xxxx:xxxx, RoCE v2
> > >                                 ::ffff:192.170.1.101
> > >
> > Due to lack of support of GID's netdev, v1/v2 type info in ibv_devinfo output,
> most users that I know of are using non upstream show_gids script.
> > So changing format here shouldn't break the existing users scripts.
> > There may be some scripts that may find this format different.
> > So I think printing both is likely a more safer option.
> 
> I don't understand this argument. Output from example tool (ibv_devinfo)
> inside libibverbs can't be considered API and we can't live in constant fear that
> some user script will break. Of course, we will try to keep it stable, but there is
> no such promise.
> 
I agree with your point that ibv_devinfo output is not an API.
I haven't come across a user who uses ibv_devinfo output as an API given lack of info in it.
I really do not have any strong opinion to keep both format or single format.

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v5] libibverbs: display gid type in ibv_devinfo
  2020-02-04  7:25           ` Parav Pandit
@ 2020-02-04  8:44             ` Devesh Sharma
  2020-02-04  8:50               ` Devesh Sharma
  0 siblings, 1 reply; 13+ messages in thread
From: Devesh Sharma @ 2020-02-04  8:44 UTC (permalink / raw)
  To: Parav Pandit; +Cc: Leon Romanovsky, Jason Gunthorpe, linux-rdma

On Tue, Feb 4, 2020 at 12:55 PM Parav Pandit <parav@mellanox.com> wrote:
>
> Hi Leon,
>
> > From: Leon Romanovsky <leon@kernel.org>
> > Sent: Tuesday, February 4, 2020 12:48 PM
> >
> > On Tue, Feb 04, 2020 at 04:27:45AM +0000, Parav Pandit wrote:
> > >
> > >
> > > > From: Jason Gunthorpe <jgg@mellanox.com>
> > > > Sent: Monday, February 3, 2020 11:34 PM
> > > >
> > > > On Mon, Feb 03, 2020 at 11:31:06PM +0530, Devesh Sharma wrote:
> > > > > On Mon, Feb 3, 2020 at 11:23 PM Jason Gunthorpe <jgg@mellanox.com>
> > > > wrote:
> > > > > >
> > > > > > On Mon, Feb 03, 2020 at 12:52:04PM -0500, Devesh Sharma wrote:
> > > > > > > It becomes difficult to make out from the output of
> > > > > > > ibv_devinfo if a particular gid index is RoCE v2 or not.
> > > > > > >
> > > > > > > Adding a string to the output of ibv_devinfo -v to display the
> > > > > > > gid type at the end of gid.
> > > > > > >
> > > > > > > The output would look something like below:
> > > > > > > $ ibv_devinfo -v -d bnxt_re2
> > > > > > > hca_id: bnxt_re2
> > > > > > >  transport:             InfiniBand (0)
> > > > > > >  fw_ver:                216.0.220.0
> > > > > > >  node_guid:             b226:28ff:fed3:b0f0
> > > > > > >  sys_image_guid:        b226:28ff:fed3:b0f0
> > > > > > >   .
> > > > > > >   .
> > > > > > >   .
> > > > > > >   .
> > > > > > >        phys_state:      LINK_UP (5)
> > > > > > >        GID[  0]:               fe80::b226:28ff:fed3:b0f0, IB/RoCE v1
> > > > > > >        GID[  1]:               fe80::b226:28ff:fed3:b0f0, RoCE v2
> > > > > > >        GID[  2]:               ::ffff:192.170.1.101, IB/RoCE v1
> > > > > > >        GID[  3]:               ::ffff:192.170.1.101, RoCE v2
> > > > > >
> > > > > > v1 GIDs are GIDs and should never be formed as IPv6 addreses..
> > > > > So, V1 gids would fall back to old style of display and there will
> > > > > be one more check for gid-type inside the loop...
> > > >
> > > > Yes
> > > >
> > > > Parav should we show both the v6 and classic format for a v2 GID? ie
> > > >
> > > >         GID[  3]:               0000:0000:0000:ffff:xxxx:xxxx:xxxx, RoCE v2
> > > >                                 ::ffff:192.170.1.101
> > > >
> > > Due to lack of support of GID's netdev, v1/v2 type info in ibv_devinfo output,
> > most users that I know of are using non upstream show_gids script.
> > > So changing format here shouldn't break the existing users scripts.
> > > There may be some scripts that may find this format different.
> > > So I think printing both is likely a more safer option.
> >
> > I don't understand this argument. Output from example tool (ibv_devinfo)
> > inside libibverbs can't be considered API and we can't live in constant fear that
> > some user script will break. Of course, we will try to keep it stable, but there is
> > no such promise.
> >
> I agree with your point that ibv_devinfo output is not an API.
> I haven't come across a user who uses ibv_devinfo output as an API given lack of info in it.
> I really do not have any strong opinion to keep both format or single format.

To it looks dirty, if the tool would print both things would look
something like:
phys_state:             LINK_UP (5)
                        GID[  0]:
fe80:0000:0000:0000:b226:28ff:fed3:b0f0, IB/RoCE v1
                        GID[  1]:
fe80:0000:0000:0000:b226:28ff:fed3:b0f0, RoCE v2
                        GID[  1]:               fe80::b226:28ff:fed3:b0f0
                        GID[  2]:
0000:0000:0000:0000:0000:ffff:c0aa:0165, IB/RoCE v1
                        GID[  3]:
0000:0000:0000:0000:0000:ffff:c0aa:0165, RoCE v2
                        GID[  3]:               ::ffff:192.170.1.101

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v5] libibverbs: display gid type in ibv_devinfo
  2020-02-04  8:44             ` Devesh Sharma
@ 2020-02-04  8:50               ` Devesh Sharma
  0 siblings, 0 replies; 13+ messages in thread
From: Devesh Sharma @ 2020-02-04  8:50 UTC (permalink / raw)
  To: Parav Pandit; +Cc: Leon Romanovsky, Jason Gunthorpe, linux-rdma

On Tue, Feb 4, 2020 at 2:14 PM Devesh Sharma <devesh.sharma@broadcom.com> wrote:
>
> On Tue, Feb 4, 2020 at 12:55 PM Parav Pandit <parav@mellanox.com> wrote:
> >
> > Hi Leon,
> >
> > > From: Leon Romanovsky <leon@kernel.org>
> > > Sent: Tuesday, February 4, 2020 12:48 PM
> > >
> > > On Tue, Feb 04, 2020 at 04:27:45AM +0000, Parav Pandit wrote:
> > > >
> > > >
> > > > > From: Jason Gunthorpe <jgg@mellanox.com>
> > > > > Sent: Monday, February 3, 2020 11:34 PM
> > > > >
> > > > > On Mon, Feb 03, 2020 at 11:31:06PM +0530, Devesh Sharma wrote:
> > > > > > On Mon, Feb 3, 2020 at 11:23 PM Jason Gunthorpe <jgg@mellanox.com>
> > > > > wrote:
> > > > > > >
> > > > > > > On Mon, Feb 03, 2020 at 12:52:04PM -0500, Devesh Sharma wrote:
> > > > > > > > It becomes difficult to make out from the output of
> > > > > > > > ibv_devinfo if a particular gid index is RoCE v2 or not.
> > > > > > > >
> > > > > > > > Adding a string to the output of ibv_devinfo -v to display the
> > > > > > > > gid type at the end of gid.
> > > > > > > >
> > > > > > > > The output would look something like below:
> > > > > > > > $ ibv_devinfo -v -d bnxt_re2
> > > > > > > > hca_id: bnxt_re2
> > > > > > > >  transport:             InfiniBand (0)
> > > > > > > >  fw_ver:                216.0.220.0
> > > > > > > >  node_guid:             b226:28ff:fed3:b0f0
> > > > > > > >  sys_image_guid:        b226:28ff:fed3:b0f0
> > > > > > > >   .
> > > > > > > >   .
> > > > > > > >   .
> > > > > > > >   .
> > > > > > > >        phys_state:      LINK_UP (5)
> > > > > > > >        GID[  0]:               fe80::b226:28ff:fed3:b0f0, IB/RoCE v1
> > > > > > > >        GID[  1]:               fe80::b226:28ff:fed3:b0f0, RoCE v2
> > > > > > > >        GID[  2]:               ::ffff:192.170.1.101, IB/RoCE v1
> > > > > > > >        GID[  3]:               ::ffff:192.170.1.101, RoCE v2
> > > > > > >
> > > > > > > v1 GIDs are GIDs and should never be formed as IPv6 addreses..
> > > > > > So, V1 gids would fall back to old style of display and there will
> > > > > > be one more check for gid-type inside the loop...
> > > > >
> > > > > Yes
> > > > >
> > > > > Parav should we show both the v6 and classic format for a v2 GID? ie
> > > > >
> > > > >         GID[  3]:               0000:0000:0000:ffff:xxxx:xxxx:xxxx, RoCE v2
> > > > >                                 ::ffff:192.170.1.101
> > > > >
> > > > Due to lack of support of GID's netdev, v1/v2 type info in ibv_devinfo output,
> > > most users that I know of are using non upstream show_gids script.
> > > > So changing format here shouldn't break the existing users scripts.
> > > > There may be some scripts that may find this format different.
> > > > So I think printing both is likely a more safer option.
> > >
> > > I don't understand this argument. Output from example tool (ibv_devinfo)
> > > inside libibverbs can't be considered API and we can't live in constant fear that
> > > some user script will break. Of course, we will try to keep it stable, but there is
> > > no such promise.
> > >
> > I agree with your point that ibv_devinfo output is not an API.
> > I haven't come across a user who uses ibv_devinfo output as an API given lack of info in it.
> > I really do not have any strong opinion to keep both format or single format.
>
> To it looks dirty, if the tool would print both things would look
> something like:
> phys_state:             LINK_UP (5)
>                         GID[  0]:
> fe80:0000:0000:0000:b226:28ff:fed3:b0f0, IB/RoCE v1
>                         GID[  1]:
> fe80:0000:0000:0000:b226:28ff:fed3:b0f0, RoCE v2
>                         GID[  1]:               fe80::b226:28ff:fed3:b0f0
>                         GID[  2]:
> 0000:0000:0000:0000:0000:ffff:c0aa:0165, IB/RoCE v1
>                         GID[  3]:
> 0000:0000:0000:0000:0000:ffff:c0aa:0165, RoCE v2
>                         GID[  3]:               ::ffff:192.170.1.101

Re-post with re-formating:
phys_state:             LINK_UP (5)
                        GID[  0]:
fe80:0000:0000:0000:b226:28ff:fed3:b0f0, IB/RoCE v1
                        GID[  1]:
fe80:0000:0000:0000:b226:28ff:fed3:b0f0, RoCE v2
                        GID[  1]: fe80::b226:28ff:fed3:b0f0
                        GID[  2]:
0000:0000:0000:0000:0000:ffff:c0aa:0165, IB/RoCE v1
                        GID[  3]:
0000:0000:0000:0000:0000:ffff:c0aa:0165, RoCE v2
                        GID[  3]: ::ffff:192.170.1.101

^ permalink raw reply	[flat|nested] 13+ messages in thread

end of thread, other threads:[~2020-02-04  8:50 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-02-03 17:52 [PATCH v5] libibverbs: display gid type in ibv_devinfo Devesh Sharma
2020-02-03 17:53 ` Jason Gunthorpe
2020-02-03 18:01   ` Devesh Sharma
2020-02-03 18:04     ` Jason Gunthorpe
2020-02-03 18:09       ` Devesh Sharma
2020-02-04  4:27       ` Parav Pandit
2020-02-04  5:08         ` Devesh Sharma
2020-02-04  7:17         ` Leon Romanovsky
2020-02-04  7:25           ` Parav Pandit
2020-02-04  8:44             ` Devesh Sharma
2020-02-04  8:50               ` Devesh Sharma
2020-02-03 19:46 ` Leon Romanovsky
2020-02-04  5:07   ` Devesh Sharma

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.