| Message ID | 1580967842-11220-1-git-send-email-devesh.sharma@broadcom.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [v7] libibverbs: display gid type in ibv_devinfo | expand |
On 2/5/2020 21:44, 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:0000:0000:0000:b226:28ff:fed3:b0f0, IB/RoCE v1 > GID[ 1]: fe80::b226:28ff:fed3:b0f0, RoCE v2 > GID[ 2]: 0000:0000:0000:0000:0000:ffff:c0aa:0165, 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/examples/devinfo.c | 61 ++++++++++++++++++++++++++++++++++--------- > 1 file changed, 49 insertions(+), 12 deletions(-) > > diff --git a/libibverbs/examples/devinfo.c b/libibverbs/examples/devinfo.c > index bf53eac..7e2015b 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,12 +163,50 @@ static const char *vl_str(uint8_t vl_num) > } > } > > -static int print_all_port_gids(struct ibv_context *ctx, uint8_t port_num, int tbl_len) > +#define DEVINFO_INVALID_GID_TYPE 2 > +static const char *gid_type_str(enum ibv_gid_type type) > { > + switch (type) { > + case IBV_GID_TYPE_IB_ROCE_V1: return "IB/RoCE v1"; You call this function only if link layer is ethernet, why do you need the "IB/" part? > + case IBV_GID_TYPE_ROCE_V2: return "RoCE v2"; > + default: return "Invalid gid type"; > + } > +} > + > +static void print_formated_gid(union ibv_gid *gid, int i, > + enum ibv_gid_type type, int ll) > +{ > + char gid_str[INET6_ADDRSTRLEN] = {}; > + char str[20] = {}; > + > + if (ll == IBV_LINK_LAYER_ETHERNET) > + sprintf(str, ", %s", gid_type_str(type)); > + > + if (type == IBV_GID_TYPE_IB_ROCE_V1) > + printf("\t\t\tGID[%3d]:\t\t%02x%02x:%02x%02x:%02x%02x:%02x%02x:%02x%02x:%02x%02x:%02x%02x:%02x%02x%s\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], str); > + > + if (type == IBV_GID_TYPE_ROCE_V2) { > + inet_ntop(AF_INET6, gid->raw, gid_str, sizeof(gid_str)); > + printf("\t\t\tGID[%3d]:\t\t%s%s\n", i, gid_str, str); > + } > +} > + > +static int print_all_port_gids(struct ibv_context *ctx, > + struct ibv_port_attr *port_attr, > + uint8_t port_num) > +{ > + enum ibv_gid_type type; > union ibv_gid gid; > + int tbl_len; > int rc = 0; > int i; > > + tbl_len = port_attr->gid_tbl_len; > for (i = 0; i < tbl_len; i++) { > rc = ibv_query_gid(ctx, port_num, i, &gid); > if (rc) { > @@ -175,17 +214,15 @@ static int print_all_port_gids(struct ibv_context *ctx, uint8_t port_num, int tb > port_num, i); > return rc; > } > + > + rc = ibv_query_gid_type(ctx, port_num, i, &type); I really dislike this, what if in the future a new gid type will be added (I know, I know, what are the chances :) ) old rdma-core won't display anything in that index (while today it displays anything as long as the gid isn't a null gid). Mark > + if (rc) { > + rc = 0; > + type = DEVINFO_INVALID_GID_TYPE; > + } > 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]); > + print_formated_gid(&gid, i, type, > + port_attr->link_layer); > } > return rc; > } > @@ -614,7 +651,7 @@ static int print_hca_cap(struct ibv_device *ib_dev, uint8_t ib_port) > printf("\t\t\tphys_state:\t\t%s (%d)\n", > port_phy_state_str(port_attr.phys_state), port_attr.phys_state); > > - if (print_all_port_gids(ctx, port, port_attr.gid_tbl_len)) > + if (print_all_port_gids(ctx, &port_attr, port)) > goto cleanup; > } > printf("\n"); >
On Thu, Feb 06, 2020 at 12:44:02AM -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:0000:0000:0000:b226:28ff:fed3:b0f0, IB/RoCE v1 > GID[ 1]: fe80::b226:28ff:fed3:b0f0, RoCE v2 > GID[ 2]: 0000:0000:0000:0000:0000:ffff:c0aa:0165, 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> Please don't add my Reviewed-by, before I explicitly wrote it. Thanks
On Thu, Feb 6, 2020 at 2:08 PM Leon Romanovsky <leon@kernel.org> wrote: > > On Thu, Feb 06, 2020 at 12:44:02AM -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:0000:0000:0000:b226:28ff:fed3:b0f0, IB/RoCE v1 > > GID[ 1]: fe80::b226:28ff:fed3:b0f0, RoCE v2 > > GID[ 2]: 0000:0000:0000:0000:0000:ffff:c0aa:0165, 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> > > Please don't add my Reviewed-by, before I explicitly wrote it. > Noted! > > Thanks
On Thu, Feb 6, 2020 at 11:38 AM Mark Bloch <markb@mellanox.com> wrote: > > > > On 2/5/2020 21:44, 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:0000:0000:0000:b226:28ff:fed3:b0f0, IB/RoCE v1 > > GID[ 1]: fe80::b226:28ff:fed3:b0f0, RoCE v2 > > GID[ 2]: 0000:0000:0000:0000:0000:ffff:c0aa:0165, 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/examples/devinfo.c | 61 ++++++++++++++++++++++++++++++++++--------- > > 1 file changed, 49 insertions(+), 12 deletions(-) > > > > diff --git a/libibverbs/examples/devinfo.c b/libibverbs/examples/devinfo.c > > index bf53eac..7e2015b 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,12 +163,50 @@ static const char *vl_str(uint8_t vl_num) > > } > > } > > > > -static int print_all_port_gids(struct ibv_context *ctx, uint8_t port_num, int tbl_len) > > +#define DEVINFO_INVALID_GID_TYPE 2 > > +static const char *gid_type_str(enum ibv_gid_type type) > > { > > + switch (type) { > > + case IBV_GID_TYPE_IB_ROCE_V1: return "IB/RoCE v1"; > > You call this function only if link layer is ethernet, why do you need the "IB/" part? Jason do you have any suggestion here, "IB/ROCE v1" is the standard string rdma-cm and /sys/class/infiniband/bnxt_re0/ports/1/gid_attrs/types/* use to distinguish v2 gid from v1/IB gids. > > > > + case IBV_GID_TYPE_ROCE_V2: return "RoCE v2"; > > + default: return "Invalid gid type"; > > + } > > +} > > + > > +static void print_formated_gid(union ibv_gid *gid, int i, > > + enum ibv_gid_type type, int ll) > > +{ > > + char gid_str[INET6_ADDRSTRLEN] = {}; > > + char str[20] = {}; > > + > > + if (ll == IBV_LINK_LAYER_ETHERNET) > > + sprintf(str, ", %s", gid_type_str(type)); > > + > > + if (type == IBV_GID_TYPE_IB_ROCE_V1) > > + printf("\t\t\tGID[%3d]:\t\t%02x%02x:%02x%02x:%02x%02x:%02x%02x:%02x%02x:%02x%02x:%02x%02x:%02x%02x%s\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], str); > > + > > + if (type == IBV_GID_TYPE_ROCE_V2) { > > + inet_ntop(AF_INET6, gid->raw, gid_str, sizeof(gid_str)); > > + printf("\t\t\tGID[%3d]:\t\t%s%s\n", i, gid_str, str); > > + } > > +} > > + > > +static int print_all_port_gids(struct ibv_context *ctx, > > + struct ibv_port_attr *port_attr, > > + uint8_t port_num) > > +{ > > + enum ibv_gid_type type; > > union ibv_gid gid; > > + int tbl_len; > > int rc = 0; > > int i; > > > > + tbl_len = port_attr->gid_tbl_len; > > for (i = 0; i < tbl_len; i++) { > > rc = ibv_query_gid(ctx, port_num, i, &gid); > > if (rc) { > > @@ -175,17 +214,15 @@ static int print_all_port_gids(struct ibv_context *ctx, uint8_t port_num, int tb > > port_num, i); > > return rc; > > } > > + > > + rc = ibv_query_gid_type(ctx, port_num, i, &type); > > I really dislike this, what if in the future a new gid type will be added (I know, I know, what are > the chances :) ) > > old rdma-core won't display anything in that index (while today it displays anything as long > as the gid isn't a null gid). Parav, do you have any suggestions here? > > Mark > > > + if (rc) { > > + rc = 0; > > + type = DEVINFO_INVALID_GID_TYPE; > > + } > > 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]); > > + print_formated_gid(&gid, i, type, > > + port_attr->link_layer); > > } > > return rc; > > } > > @@ -614,7 +651,7 @@ static int print_hca_cap(struct ibv_device *ib_dev, uint8_t ib_port) > > printf("\t\t\tphys_state:\t\t%s (%d)\n", > > port_phy_state_str(port_attr.phys_state), port_attr.phys_state); > > > > - if (print_all_port_gids(ctx, port, port_attr.gid_tbl_len)) > > + if (print_all_port_gids(ctx, &port_attr, port)) > > goto cleanup; > > } > > printf("\n"); > >
On Fri, Feb 07, 2020 at 06:07:34PM +0530, Devesh Sharma wrote: > > > -static int print_all_port_gids(struct ibv_context *ctx, uint8_t port_num, int tbl_len) > > > +#define DEVINFO_INVALID_GID_TYPE 2 > > > +static const char *gid_type_str(enum ibv_gid_type type) > > > { > > > + switch (type) { > > > + case IBV_GID_TYPE_IB_ROCE_V1: return "IB/RoCE v1"; > > > > You call this function only if link layer is ethernet, why do you need the "IB/" part? > Jason do you have any suggestion here, "IB/ROCE v1" is the standard > string rdma-cm and > /sys/class/infiniband/bnxt_re0/ports/1/gid_attrs/types/* use to > distinguish v2 gid from v1/IB gids. I would ignore the sysfs If you know for sure it is RoCE v1 then say so IB doesn't have types so it shouldn't show anything Jason
On Fri, Feb 7, 2020 at 7:43 PM Jason Gunthorpe <jgg@mellanox.com> wrote: > > On Fri, Feb 07, 2020 at 06:07:34PM +0530, Devesh Sharma wrote: > > > > -static int print_all_port_gids(struct ibv_context *ctx, uint8_t port_num, int tbl_len) > > > > +#define DEVINFO_INVALID_GID_TYPE 2 > > > > +static const char *gid_type_str(enum ibv_gid_type type) > > > > { > > > > + switch (type) { > > > > + case IBV_GID_TYPE_IB_ROCE_V1: return "IB/RoCE v1"; > > > > > > You call this function only if link layer is ethernet, why do you need the "IB/" part? > > Jason do you have any suggestion here, "IB/ROCE v1" is the standard > > string rdma-cm and > > /sys/class/infiniband/bnxt_re0/ports/1/gid_attrs/types/* use to > > distinguish v2 gid from v1/IB gids. > > I would ignore the sysfs > > If you know for sure it is RoCE v1 then say so > > IB doesn't have types so it shouldn't show anything Sent out a v8 for this change. > > Jason
diff --git a/libibverbs/examples/devinfo.c b/libibverbs/examples/devinfo.c index bf53eac..7e2015b 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,12 +163,50 @@ static const char *vl_str(uint8_t vl_num) } } -static int print_all_port_gids(struct ibv_context *ctx, uint8_t port_num, int tbl_len) +#define DEVINFO_INVALID_GID_TYPE 2 +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 void print_formated_gid(union ibv_gid *gid, int i, + enum ibv_gid_type type, int ll) +{ + char gid_str[INET6_ADDRSTRLEN] = {}; + char str[20] = {}; + + if (ll == IBV_LINK_LAYER_ETHERNET) + sprintf(str, ", %s", gid_type_str(type)); + + if (type == IBV_GID_TYPE_IB_ROCE_V1) + printf("\t\t\tGID[%3d]:\t\t%02x%02x:%02x%02x:%02x%02x:%02x%02x:%02x%02x:%02x%02x:%02x%02x:%02x%02x%s\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], str); + + if (type == IBV_GID_TYPE_ROCE_V2) { + inet_ntop(AF_INET6, gid->raw, gid_str, sizeof(gid_str)); + printf("\t\t\tGID[%3d]:\t\t%s%s\n", i, gid_str, str); + } +} + +static int print_all_port_gids(struct ibv_context *ctx, + struct ibv_port_attr *port_attr, + uint8_t port_num) +{ + enum ibv_gid_type type; union ibv_gid gid; + int tbl_len; int rc = 0; int i; + tbl_len = port_attr->gid_tbl_len; for (i = 0; i < tbl_len; i++) { rc = ibv_query_gid(ctx, port_num, i, &gid); if (rc) { @@ -175,17 +214,15 @@ static int print_all_port_gids(struct ibv_context *ctx, uint8_t port_num, int tb port_num, i); return rc; } + + rc = ibv_query_gid_type(ctx, port_num, i, &type); + if (rc) { + rc = 0; + type = DEVINFO_INVALID_GID_TYPE; + } 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]); + print_formated_gid(&gid, i, type, + port_attr->link_layer); } return rc; } @@ -614,7 +651,7 @@ static int print_hca_cap(struct ibv_device *ib_dev, uint8_t ib_port) printf("\t\t\tphys_state:\t\t%s (%d)\n", port_phy_state_str(port_attr.phys_state), port_attr.phys_state); - if (print_all_port_gids(ctx, port, port_attr.gid_tbl_len)) + if (print_all_port_gids(ctx, &port_attr, port)) goto cleanup; } printf("\n");