libibmad: Fixes for failures when not all ports of HCA are connected
PATCH 1/1 libibmad: Fixes for failures when not all ports of HCA are connected
From: Brendan Doyle <[email protected]>
---
src/resolve.c | 22 ++++++++++++++++++---- src/rpc.c | 1 + src/sa.c | 2 ++ 3 files changed, 21 insertions(+), 4 deletions(-)
src/resolve.c +0 -0
@@ -40,6 +40,7 @@
#include <stdlib.h>
#include <string.h>
#include <arpa/inet.h>
+#include <errno.h>
#include <infiniband/umad.h>
#include <infiniband/mad.h>
@@ -57,10 +58,18 @@ int ib_resolve_smlid_via(ib_portid_t * sm_id, int timeout,
memset(sm_id, 0, sizeof(*sm_id));
- if (!smp_query_via(portinfo, &self, IB_ATTR_PORT_INFO, 0, 0, srcport))
+ if (!smp_query_via(portinfo, &self, IB_ATTR_PORT_INFO, 0, 0, srcport)) {
+ if (!errno)
+ errno = EIO; return -1;
+ }
mad_decode_field(portinfo, IB_PORT_SMLID_F, &lid);
+ if (lid == 0) {
+ if (!errno)
+ errno = EIO;
+ return -1;
+ }
mad_decode_field(portinfo, IB_PORT_SMSL_F, &sm_id->sl); return ib_portid_set(sm_id, lid, 0, 0);
@@ -95,21 +104,26 @@ int ib_resolve_guid_via(ib_portid_t * portid, uint64_t * guid,
ib_portid_t * sm_id, int timeout,
const struct ibmad_port *srcport)
{
- ib_portid_t sm_portid;
+ ib_portid_t sm_portid = { 0 };
uint8_t buf[IB_SA_DATA_SIZE] = { 0 };
ib_portid_t self = { 0 };
uint64_t selfguid, prefix;
ibmad_gid_t selfgid;
uint8_t nodeinfo[64];- if (!sm_id) {
+ if (!sm_id)
sm_id = &sm_portid;
+
+ if (!sm_id->lid) {
if (ib_resolve_smlid_via(sm_id, timeout, srcport) < 0)
return -1;
}- if (!smp_query_via(nodeinfo, &self, IB_ATTR_NODE_INFO, 0, 0, srcport))
+ if (!smp_query_via(nodeinfo, &self, IB_ATTR_NODE_INFO, 0, 0, srcport)) {
+ if (!errno)
+ errno = EIO; return -1; + } mad_decode_field(nodeinfo, IB_NODE_PORT_GUID_F, &selfguid); mad_set_field64(selfgid, 0, IB_GID_PREFIX_F, IB_DEFAULT_SUBN_PREFIX); mad_set_field64(selfgid, 0, IB_GID_GUID_F, selfguid);
src/rpc.c +0 -0
@@ -178,6 +178,7 @@ _do_madrpc(int port_id, void *sndbuf, void *rcvbuf, int agentid, int len,
IB_MAD_TRID_F) != trid); status = umad_status(rcvbuf); + errno = status; if (!status) return length; /* done */ if (status == ENOMEM)
src/sa.c +0 -0
@@ -38,6 +38,7 @@
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
+#include <errno.h>
#include <infiniband/mad.h>
#include "mad_internal.h"
@@ -56,6 +57,7 @@ uint8_t *sa_rpc_call(const struct ibmad_port *ibmad_port, void *rcvbuf,
if (portid->lid <= 0) {
IBWARN("only lid routes are supported");
+ errno = EIO;
return NULL;
}Signature · 7 lines
1.7.1 -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
Your commit message does not seem to have anything to do with the patch. Could you explain how returning errno from these functions "Fixes for failures when not all ports of HCA are connected"?
Furthermore, I'm reluctant to modify errno in this library. It is not documented and in general is poor form. I realize that the interface does not currently allow for an alternative. :-(
More comments below.
Quoted 31 lines · Boris Chiu
From: Brendan Doyle <[email protected]>
Signed-off-by Brendan Doyle <[email protected]>---
src/resolve.c | 22 ++++++++++++++++++---- src/rpc.c | 1 + src/sa.c | 2 ++ 3 files changed, 21 insertions(+), 4 deletions(-)src/resolve.c +3
-1@@ -40,6 +40,7 @@#include <stdlib.h> #include <string.h> #include <arpa/inet.h> +#include <errno.h> #include <infiniband/umad.h> #include <infiniband/mad.h> @@ -57,10 +58,18 @@ int ib_resolve_smlid_via(ib_portid_t * sm_id, int timeout,memset (sm_id,0 ,sizeof (*sm_id)); -if (!smp_query_via (portinfo, &self,IB_ATTR_PORT_INFO ,0 ,0 , srcport)) +if (!smp_query_via (portinfo, &self,IB_ATTR_PORT_INFO ,0 ,0 , srcport)) { +if (!errno)
src/resolve.c int ib_resolve_smlid_via(ib_portid_t * sm_id, int · line 75
Quoted 10 lines
src/resolve.c +7
-0+ errno =EIO ;return -1 ; + }mad_decode_field (portinfo,IB_PORT_SMLID_F , &lid); +if (lid ==0 ) { +if (!errno) + errno =EIO ; +return -1 ; + }
This may not be an error. A port which is down only requires PortState and PortPhyState to be valid.
Quoted 14 lines
mad_decode_field(portinfo, IB_PORT_SMSL_F, &sm_id->sl); return ib_portid_set(sm_id, lid, 0, 0);src/resolve.c +1
-1@@ -95,21 +104,26 @@ int ib_resolve_guid_via(ib_portid_t * portid, uint64_t * guid, ib_portid_t * sm_id, int timeout, const struct ibmad_port *srcport) { -ib_portid_t sm_portid; +ib_portid_t sm_portid = {0 };uint8_t buf[IB_SA_DATA_SIZE ] = {0 };ib_portid_t self = {0 };uint64_t selfguid, prefix;ibmad_gid_t selfgid;
src/resolve.c int ib_resolve_guid_via(ib_portid_t * portid, uint64_t · line 121
Quoted 10 lines
src/resolve.c +3
-1uint8_t nodeinfo[64 ]; -if (!sm_id) { +if (!sm_id) sm_id = &sm_portid; + +if (!sm_id->lid) {if (ib_resolve_smlid_via (sm_id, timeout, srcport) <0 )return -1 ; }
If you want ib_resolve_guid_via to resolve the SM for you, sm_id should be set to NULL. I don't see a reason to support an "invalid" sm_id port id.
Another note is that I have converted the diags to use internal functions which used umad* and SA queries to resolve GUID's. This is more appropriate in the long term.
Ira
Quoted 48 lines
src/resolve.c +4
-1-if (!smp_query_via (nodeinfo, &self,IB_ATTR_NODE_INFO ,0 ,0 , srcport)) +if (!smp_query_via (nodeinfo, &self,IB_ATTR_NODE_INFO ,0 ,0 , srcport)) { +if (!errno) + errno =EIO ;return -1 ; + }mad_decode_field (nodeinfo,IB_NODE_PORT_GUID_F , &selfguid);mad_set_field64(selfgid, 0, IB_GID_PREFIX_F, IB_DEFAULT_SUBN_PREFIX); mad_set_field64(selfgid, 0, IB_GID_GUID_F, selfguid);src/rpc.c +1
-0@@ -178,6 +178,7 @@ _do_madrpc(int port_id, void *sndbuf, void *rcvbuf, int agentid, int len, IB_MAD_TRID_F) != trid); status =umad_status (rcvbuf); + errno = status;if (!status)return length;/* done */ if (status == ENOMEM)src/sa.c +2
-1@@ -38,6 +38,7 @@#include <stdio.h> #include <stdlib.h> #include <string.h> +#include <errno.h> #include <infiniband/mad.h> #include "mad_internal.h" @@ -56,6 +57,7 @@ uint8_t *sa_rpc_call(const struct ibmad_port *ibmad_port, void *rcvbuf, if (portid->lid <= 0) {IBWARN ("only lid routes are supported" ); + errno =EIO ;return NULL ; } --
Quoted 3 lines
1.7.1
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
Quoted 1 line · Ira Weiny
Furthermore, I'm reluctant to modify errno in this library.
Yep, this should be avoided unless impossible...
Or.
Quoted 2 lines
It is not documented and in general is poor form. I realize that the interface does not currently allow for an alternative.:-(
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html