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.

> +     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.

> +         (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.


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

Reply via email to