On 1/28/2015 1:27 PM, Weiny, Ira wrote: > Sorry for the delay. > >> -----Original Message----- >> From: linux-rdma-ow...@vger.kernel.org [mailto:linux-rdma- >> ow...@vger.kernel.org] On Behalf Of Hal Rosenstock >> Sent: Tuesday, January 20, 2015 5:00 AM >> To: Weiny, Ira >> Cc: Dan Ben-Yosef; linux-rdma (linux-rdma@vger.kernel.org) >> Subject: [PATCH infiniband-diags] ibtracert.c: Remove checking the port 0 >> state >> for BasePort0 switches >> >> From: Dan Ben Yosef <da...@mellanox.com> >> >> The port 0 state check is needed only for HCA/Routers and >> EnhancedPort0 switches. >> >> For BasePort0 switches, the PortState field in the PortInfo attribute for >> port0 is >> undefined (not used). >> >> Signed-off-by: Dan Ben Yosef <da...@mellanox.com> >> --- >> src/ibtracert.c | 47 ++++++++++++++++++++++++++++++++++++++++------- >> 1 files changed, 40 insertions(+), 7 deletions(-) >> >> diff --git a/src/ibtracert.c b/src/ibtracert.c index d32968b..326200b 100644 >> --- a/src/ibtracert.c >> +++ b/src/ibtracert.c >> @@ -92,6 +92,7 @@ struct Switch { >> int mccap; >> int linearFDBtop; >> int fdb_base; >> + int enhsp0; >> int8_t fdb[64]; >> char switchinfo[64]; >> }; >> @@ -114,6 +115,17 @@ struct Node { >> Node *nodesdist[MAXHOPS]; >> uint64_t target_portguid; >> >> +static int is_route_inactive_port0(Node * node, Port * port, Switch * >> +sw) { > > I would prefer this function be named something like: > > port_inactive_not_bsp0 > > The current name is confusing as to what the logic is.
How about is_port_inactive ? >> + int res = 0; >> + /* not active for HCA and for enhanced port0 Switches */ >> + if (port->state != 4 && > > Please use #defines here even though the original code did not. This is an issue throughout infiniband-diags. I think it should be addressed by separate patch(es) for this. >> + (node->type != IB_NODE_SWITCH || >> + (node->type == IB_NODE_SWITCH && sw->enhsp0))) >> + res = 1; >> + return res; >> +} >> + >> static int get_node(Node * node, Port * port, ib_portid_t * portid) { >> void *pi = port->portinfo, *ni = node->nodeinfo, *nd = node->nodedesc; >> @@ -164,6 +176,7 @@ static int switch_lookup(Switch * sw, ib_portid_t * >> portid, int lid) >> >> mad_decode_field(si, IB_SW_LINEAR_FDB_CAP_F, &sw->linearcap); >> mad_decode_field(si, IB_SW_LINEAR_FDB_TOP_F, &sw- >>> linearFDBtop); >> + mad_decode_field(si, IB_SW_ENHANCED_PORT0_F, &sw->enhsp0); >> >> if (lid >= sw->linearcap && lid > sw->linearFDBtop) >> return -1; >> @@ -248,7 +261,7 @@ static int find_route(ib_portid_t * from, ib_portid_t * >> to, int dump) >> Port *port, fromport, toport, nextport; >> Switch sw; >> int maxhops = MAXHOPS; >> - int portnum, outport; >> + int portnum, outport = 255, next_sw_outport = 255; >> >> memset(&fromnode,0,sizeof(Node)); >> memset(&tonode,0,sizeof(Node)); >> @@ -274,20 +287,28 @@ static int find_route(ib_portid_t * from, ib_portid_t * >> to, int dump) >> portnum = port->portnum; >> >> dump_endnode(dump, "From", node, port); >> + if (node->type == IB_NODE_SWITCH) { >> + next_sw_outport = switch_lookup(&sw, from, to->lid); >> + if (next_sw_outport < 0 || next_sw_outport > node->numports) >> { >> + /* needed to print the port in badtbl */ >> + outport = next_sw_outport; >> + goto badtbl; >> + } >> + } >> >> while (maxhops--) { >> - if (port->state != 4) >> + /* checking if the port state isn't active. >> + * The "sw" argument is only relevant when the port is a >> + * switch port for HCAs this argument is ignored */ > > This comment should go above the function signature and be enhanced to > clarify that it is checking for port state inactive except for BSP0 which has > no state. Will do that in next version of the patch. -- Hal > > Ira > > >> + if (is_route_inactive_port0(node, port, &sw)) >> goto badport; >> >> if (sameport(port, &toport)) >> break; /* found */ >> >> - outport = portnum; >> if (node->type == IB_NODE_SWITCH) { >> DEBUG("switch node"); >> - if ((outport = switch_lookup(&sw, from, to->lid)) < 0 || >> - outport > node->numports) >> - goto badtbl; >> + outport = next_sw_outport; >> >> if (extend_dpath(&from->drpath, outport) < 0) >> goto badpath; >> @@ -307,6 +328,7 @@ static int find_route(ib_portid_t * from, ib_portid_t * >> to, int dump) >> (node->type == IB_NODE_ROUTER)) { >> int ca_src = 0; >> >> + outport = portnum; >> DEBUG("ca or router node"); >> if (!sameport(port, &fromport)) { >> IBWARN >> @@ -335,8 +357,19 @@ static int find_route(ib_portid_t * from, ib_portid_t * >> to, int dump) >> nextport.portnum = >> from->drpath.p[from->drpath.cnt + 1]; >> } >> + /* only if the next node is a switch, get switch info */ >> + if (nextnode.type == IB_NODE_SWITCH) { >> + next_sw_outport = switch_lookup(&sw, from, to->lid); >> + if (next_sw_outport < 0 || >> + next_sw_outport > nextnode.numports) { >> + /* needed to print the port in badtbl */ >> + outport = next_sw_outport; >> + goto badtbl; >> + } >> + } >> + >> port = &nextport; >> - if (port->state != 4) >> + if (is_route_inactive_port0(&nextnode, port, &sw)) >> goto badoutport; >> node = &nextnode; >> portnum = port->portnum; >> -- >> 1.7.8.2 >> >> -- >> To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the >> body >> of a message to majord...@vger.kernel.org More majordomo info at >> http://vger.kernel.org/majordomo-info.html > -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html