[ofa-general] libibmad: ib_resolve_portid_str_via; Bug?

[Ira Weiny]

Sasha,

Below is a patch which fixes an issue I had when using
ib_resolve_portid_str_via.  When resolving via IB_DEST_GUID the
ib_resolve_guid_via function optionally uses the portid to attempt to set a
different subnet prefix.

IMO I don't think portid should be an in/out parameter in
ib_resolve_portid_str_via.  I happened to pass a portid object which was on
the stack and had some garbage data in it.  It took me a while to figure out
that ib_resolve_portid_str_via was attempting to use that garbage data.

To make this more clear I added ib_resolve_gid_via and another MAD_DEST type.

What do you think?  Right now the gid resolving is untested.

Ira



diff --git a/libibmad/include/infiniband/mad.h b/libibmad/include/infiniband/mad.h
index b6f4b60..ec76d0f 100644
--- a/libibmad/include/infiniband/mad.h
+++ b/libibmad/include/infiniband/mad.h
@@ -677,6 +677,7 @@ enum MAD_DEST {
        IB_DEST_DRPATH,
        IB_DEST_GUID,
        IB_DEST_DRSLID,
+       IB_DEST_GID
 };
 
 enum MAD_NODE_TYPE {
@@ -861,6 +862,9 @@ MAD_EXPORT int ib_resolve_smlid_via(ib_portid_t * sm_id, int timeout,
 MAD_EXPORT int ib_resolve_guid_via(ib_portid_t * portid, uint64_t * guid,
                                   ib_portid_t * sm_id, int timeout,
                                   const struct ibmad_port *srcport);
+MAD_EXPORT int ib_resolve_gid_via(ib_portid_t * portid, ibmad_gid_t gid,
+                                  ib_portid_t * sm_id, int timeout,
+                                  const struct ibmad_port *srcport);
 MAD_EXPORT int ib_resolve_portid_str_via(ib_portid_t * portid, char *addr_str,
                                         enum MAD_DEST dest, ib_portid_t * sm_id,
                                         const struct ibmad_port *srcport);
diff --git a/libibmad/src/libibmad.map b/libibmad/src/libibmad.map
index 6b77784..0ce3957 100644
--- a/libibmad/src/libibmad.map
+++ b/libibmad/src/libibmad.map
@@ -100,6 +100,7 @@ IBMAD_1.3 {
                ib_path_query_via;
                ib_resolve_smlid_via;
                ib_resolve_guid_via;
+               ib_resolve_gid_via;
                ib_resolve_portid_str_via;
                ib_resolve_self_via;
                mad_field_name;
diff --git a/libibmad/src/resolve.c b/libibmad/src/resolve.c
index f34c247..4d40b2b 100644
--- a/libibmad/src/resolve.c
+++ b/libibmad/src/resolve.c
@@ -68,6 +68,26 @@ int ib_resolve_smlid(ib_portid_t * sm_id, int timeout)
        return ib_resolve_smlid_via(sm_id, timeout, ibmp);
 }
 
+int ib_resolve_gid_via(ib_portid_t * portid, ibmad_gid_t gid,
+                       ib_portid_t * sm_id, int timeout,
+                       const struct ibmad_port *srcport)
+{
+       ib_portid_t sm_portid;
+       char buf[IB_SA_DATA_SIZE] = { 0 };
+
+       if (!sm_id) {
+               sm_id = &sm_portid;
+++ b/libibmad/src/libibmad.map
@@ -100,6 +100,7 @@ IBMAD_1.3 {
                ib_path_query_via;
                ib_resolve_smlid_via;
                ib_resolve_guid_via;
+               ib_resolve_gid_via;
                ib_resolve_portid_str_via;
                ib_resolve_self_via;
                mad_field_name;
diff --git a/libibmad/src/resolve.c b/libibmad/src/resolve.c
index f34c247..4d40b2b 100644
--- a/libibmad/src/resolve.c
+++ b/libibmad/src/resolve.c
@@ -68,6 +68,26 @@ int ib_resolve_smlid(ib_portid_t * sm_id, int timeout)
        return ib_resolve_smlid_via(sm_id, timeout, ibmp);
 }
 
+int ib_resolve_gid_via(ib_portid_t * portid, ibmad_gid_t gid,
+                       ib_portid_t * sm_id, int timeout,
+                       const struct ibmad_port *srcport)
+{
+       ib_portid_t sm_portid;
+       char buf[IB_SA_DATA_SIZE] = { 0 };
+
+       if (!sm_id) {
+               sm_id = &sm_portid;
+               if (ib_resolve_smlid_via(sm_id, timeout, srcport) < 0)
+                       return -1;
+       }
+
+       if ((portid->lid =
+            ib_path_query_via(srcport, gid, gid, sm_id, buf)) < 0)
+               return -1;
+
+       return 0;
+}
+
 int ib_resolve_guid_via(ib_portid_t * portid, uint64_t * guid,
                        ib_portid_t * sm_id, int timeout,
                        const struct ibmad_port *srcport)
@@ -80,11 +100,9 @@ int ib_resolve_guid_via(ib_portid_t * portid, uint64_t * guid,
                if (ib_resolve_smlid_via(sm_id, timeout, srcport) < 0)
                        return -1;
        }
-       if (*(uint64_t *) & portid->gid == 0)
-               mad_set_field64(portid->gid, 0, IB_GID_PREFIX_F,
-                               IB_DEFAULT_SUBN_PREFIX);
-       if (guid)
-               mad_set_field64(portid->gid, 0, IB_GID_GUID_F, *guid);
+
+       mad_set_field64(portid->gid, 0, IB_GID_PREFIX_F, IB_DEFAULT_SUBN_PREFIX);
+       mad_set_field64(portid->gid, 0, IB_GID_GUID_F, *guid);
 
        if ((portid->lid =
             ib_path_query_via(srcport, portid->gid, portid->gid, sm_id,
@@ -98,12 +116,15 @@ int ib_resolve_portid_str_via(ib_portid_t * portid, char *addr_str,
                              enum MAD_DEST dest_type, ib_portid_t * sm_id,
                              const struct ibmad_port *srcport)
 {
+       ibmad_gid_t gid;
        uint64_t guid;
        int lid;
        char *routepath;
        ib_portid_t selfportid = { 0 };
        int selfport = 0;
 
+       memset(portid, 0, sizeof *portid);
+
        switch (dest_type) {
        case IB_DEST_LID:
                lid = strtol(addr_str, 0, 0);
@@ -138,6 +159,10 @@ int ib_resolve_portid_str_via(ib_portid_t * portid, char *addr_str,
                        return -1;
                return 0;
 
+       case IB_DEST_GID:
+               if (inet_pton(AF_INET6, addr_str, &gid) <= 0)
+                       return -1;
+               return ib_resolve_gid_via(portid, gid, sm_id, 0, srcport);
        default:
                IBWARN("bad dest_type %d", dest_type);
        }