| Message ID | 1580752324-24742-1-git-send-email-devesh.sharma@broadcom.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [v5] libibverbs: display gid type in ibv_devinfo | expand |
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
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
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
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
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
> 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
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
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
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
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.
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
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
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; }
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(-)