Re: [PATCH 3/4] infiniband-diags: properly resolve node guids

Hal Rosenstock
2011-07-08T21:42:38Z

On 7/5/2011 3:08 PM, Ira Weiny wrote:

Can you explain what issue this solves or is it just to move to the SA
way to do this ?

>
> Signed-off-by: Ira Weiny <weiny2-i2BcT+NCU+M@public.gmane.org>
> ---
> include/ibdiag_common.h | 3 ++
> src/ibdiag_common.c | 55 +++++++++++++++++++++++++++++++++++++++++++++++
> src/iblinkinfo.c | 5 +--
> src/ibqueryerrors.c | 8 ++----
> 4 files changed, 63 insertions(+), 8 deletions(-)
>
> diff --git a/include/ibdiag_common.h b/include/ibdiag_common.h
> index 69cddfb..e107a43 100644
> --- a/include/ibdiag_common.h
> +++ b/include/ibdiag_common.h
> @@ -126,4 +126,7 @@ void sa_report_err(int status);
> comp_mask |= IB_##name##_COMPMASK_##mask; \
> }
>
> +int resolve_node_guid(ib_portid_t *port_id, char *node_guid,
> + struct ibmad_port *ibmad_port);
> +
> #endif /* _IBDIAG_COMMON_H_ */
> diff --git a/src/ibdiag_common.c b/src/ibdiag_common.c
> index 6d03a43..66dc132 100644
> --- a/src/ibdiag_common.c
> +++ b/src/ibdiag_common.c
> @@ -509,3 +509,58 @@ void sa_report_err(int status)
> fprintf(stderr, "ERROR: Query result returned 0x%04x, %s%s\n",
> status, sm_err_str, sa_err_str);
> }
> +
> +int resolve_node_guid(ib_portid_t *port_id, char *node_guid,
> + struct ibmad_port *ibmad_port)
> +{
> + ib_node_record_t nr;
> + ib_node_record_t *found_nr;
> + uint64_t comp_mask = 0;
> + struct sa_query_result result;
> + uint64_t guid = strtoull(node_guid, NULL, 0);
> + bind_handle_t h;
> + int ret = 0;
> +
> + if (guid == 0 || guid == ULLONG_MAX)
> + return -EINVAL;
> +
> + memset(&nr, 0, sizeof(nr));
> + CHECK_AND_SET_VAL(guid, 64, 0, nr.node_info.node_guid, NR, NODEGUID);
> +
> + if (!ibd_timeout)
> + ibd_timeout = MAD_DEF_TIMEOUT_MS;
> +
> + h = sa_get_bind_handle();
> + if (!h)
> + return -EIO;
> +
> + ret = sa_query(h, IB_MAD_METHOD_GET_TABLE, IB_SA_ATTR_NODERECORD, 0,
> + cl_hton64(comp_mask), 0, &nr, &result);
> + if (ret) {
> + fprintf(stderr, "Query SA failed: %s\n", ib_get_err_str(ret));
> + ret = -EIO;
> + goto free_bind_handle;
> + }
> +
> + if (result.status != IB_SA_MAD_STATUS_SUCCESS) {
> + sa_report_err(result.status);
> + ret = -EIO;
> + goto free_result;
> + }
> +
> + if (result.result_cnt > 1)
> + fprintf(stderr, "%s resolved to more than one port.\n",
> + node_guid);

Should the request just be a GET rather than GET_TABLE and avoid this
check ? I don't think multiple nodes can register with same Node GUID,
can they ? Also, I think it makes eliminates this check and the missing
0 check.

> +
> + found_nr = (ib_node_record_t *)sa_get_query_rec(result.p_result_madw,
> + 0);
> + guid = cl_ntoh64(found_nr->node_info.port_guid);
> + ret = ib_resolve_guid_via(port_id, &guid, NULL, ibd_timeout,
> + ibmad_port);

If you're avoiding the ib_resolve_portid_str_via routine, maybe also
avoid this here and do the SA PR lookup in sa query style code here ?

-- Hal

> +
> +free_result:
> + sa_free_result_mad(&result);
> +free_bind_handle:
> + sa_free_bind_handle(h);
> + return ret;
> +}
> diff --git a/src/iblinkinfo.c b/src/iblinkinfo.c
> index 8cf755d..2f69acf 100644
> --- a/src/iblinkinfo.c
> +++ b/src/iblinkinfo.c
> @@ -619,9 +619,8 @@ int main(int argc, char **argv)
> IBWARN("Failed to resolve %s; attempting full scan\n",
> dr_path);
> } else if (guid_str) {
> - if ((resolved =
> - ib_resolve_portid_str_via(&port_id, guid_str, IB_DEST_GUID,
> - NULL, ibmad_port)) < 0)
> + if ((resolved = resolve_node_guid(&port_id, guid_str,
> + ibmad_port)) < 0)
> IBWARN("Failed to resolve %s; attempting full scan\n",
> guid_str);
> }
> diff --git a/src/ibqueryerrors.c b/src/ibqueryerrors.c
> index 1bba0e0..4b59c6f 100644
> --- a/src/ibqueryerrors.c
> +++ b/src/ibqueryerrors.c
> @@ -914,11 +914,9 @@ int main(int argc, char **argv)
> IBWARN("Failed to resolve %s; attempting full scan",
> dr_path);
> } else if (node_guid_str) {
> - if ((resolved =
> - ib_resolve_portid_str_via(&portid, node_guid_str,
> - IB_DEST_GUID, ibd_sm_id,
> - ibmad_port)) < 0)
> - IBWARN("Failed to resolve %s; attempting full scan",
> + if ((resolved = resolve_node_guid(&portid, node_guid_str,
> + ibmad_port)) < 0)
> + IBWARN("Failed to resolve %s; attempting full scan\n",
> node_guid_str);
> }