[openib-general] Re: [PATCH] Opensm - fix LinkRecord get

[Hal Rosenstock]

On Wed, 2005-11-30 at 07:35, Yael Kalka wrote:
> Hi Hal,
> 
> During some tests I've noticed that in LinkRecord queries there are
> some bugs:
> 1. Trying to ensure the two physical ports are connected comparison
> isn't done correctly.
> 2. When __osm_lr_rcv_get_physp_link is called with physical ports not
> null - there is no check that the value returned is actually different
> than null. As a result we can get several links with the same value.
> 
> This patch fixes both issues.

Thanks. Applied.

Some minor comments below.

-- Hal

> Thanks,
> Yael
> 
> Signed-off-by:  Yael Kalka <yael at mellanox.co.il>
> 
> Index: opensm/osm_sa_link_record.c
> ===================================================================
> --- opensm/osm_sa_link_record.c	(revision 4231)
> +++ opensm/osm_sa_link_record.c	(working copy)
> @@ -235,7 +235,7 @@ __osm_lr_rcv_get_physp_link(
>          Ensure the two physp's are actually connected.
>          If not, bail out.
>        */
> -      if( osm_physp_get_remote( p_src_physp ) != p_src_physp )
> +      if( osm_physp_get_remote( p_src_physp ) != p_dest_physp )
>          goto Exit;

Should there be an error message here ?

>      }
>      else
> @@ -393,12 +393,16 @@ __osm_lr_rcv_get_port_links(
>          {
>            p_dest_physp = osm_port_get_phys_ptr( p_dest_port,
>                                                  dest_port_num );
> +          /* both physical ports should be with data */
> +          if (p_src_physp && p_dest_physp)
> +          {
>            __osm_lr_rcv_get_physp_link( p_rcv, p_lr, p_src_physp,
>                                         p_dest_physp, comp_mask,
>                                         p_list, p_req_physp );
>          }
>        }
>      }
> +    }

Formatting was off here (and similarly below)... I fixed it in the
change that was just committed.

>      else
>      {
>        /*
> @@ -412,17 +416,22 @@ __osm_lr_rcv_get_port_links(
>          if (port_num < p_src_port->physp_tbl_size) 
>          {          
>            p_src_physp = osm_port_get_phys_ptr( p_src_port, port_num );
> +          if (p_src_physp)
> +          {
>            __osm_lr_rcv_get_physp_link( p_rcv, p_lr, p_src_physp,
>                                         NULL, comp_mask, p_list,
>                                         p_req_physp );
>          }
>        }
> +      }
>        else
>        {
>          num_ports = osm_port_get_num_physp( p_src_port );
>          for( port_num = 1; port_num < num_ports; port_num++ )
>          {
>            p_src_physp = osm_port_get_phys_ptr( p_src_port, port_num );
> +          if (p_src_physp)
> +          {
>            __osm_lr_rcv_get_physp_link( p_rcv, p_lr, p_src_physp,
>                                         NULL, comp_mask, p_list,
>                                         p_req_physp );
> @@ -430,6 +439,7 @@ __osm_lr_rcv_get_port_links(
>        }
>      }
>    }
> +  }
>    else
>    {
>      if( p_dest_port )
> @@ -446,11 +456,14 @@ __osm_lr_rcv_get_port_links(
>          {
>            p_dest_physp = osm_port_get_phys_ptr(
>              p_dest_port, port_num );
> +          if (p_dest_physp)
> +          {
>            __osm_lr_rcv_get_physp_link( p_rcv, p_lr, NULL,
>                                         p_dest_physp, comp_mask,
>                                         p_list, p_req_physp );
>          }
>        }
> +      }
>        else
>        {
>          num_ports = osm_port_get_num_physp( p_dest_port );
> @@ -458,12 +471,15 @@ __osm_lr_rcv_get_port_links(
>          {
>            p_dest_physp = osm_port_get_phys_ptr(
>              p_dest_port, port_num );
> +          if (p_dest_physp)
> +          {
>            __osm_lr_rcv_get_physp_link( p_rcv, p_lr, NULL,
>                                         p_dest_physp, comp_mask,
>                                         p_list, p_req_physp );
>          }
>        }
>      }
> +    }
>      else
>      {
>        /*
>