[ofa-general] Re: [PATCH] opensm: PortInfo set decision flow simplification

[Hal Rosenstock]

Hi Sasha,

On Sat, 2007-11-10 at 16:51 +0200, Sasha Khapyorsky wrote:
> This simplifies (but doesn't change) flow for PortInfo set decision in
> lid and link mgrs - mostly to make the code more readable.
> 
> Signed-off-by: Sasha Khapyorsky <sashak at voltaire.com>
> ---
>  opensm/opensm/osm_lid_mgr.c  |    9 +++++----
>  opensm/opensm/osm_link_mgr.c |   15 ++++++---------
>  2 files changed, 11 insertions(+), 13 deletions(-)
> 
> diff --git a/opensm/opensm/osm_lid_mgr.c b/opensm/opensm/osm_lid_mgr.c
> index 9527185..9da6fcf 100644
> --- a/opensm/opensm/osm_lid_mgr.c
> +++ b/opensm/opensm/osm_lid_mgr.c
> @@ -1184,9 +1184,11 @@ __osm_lid_mgr_set_physp_pi(IN osm_lid_mgr_t * const p_mgr,
>  	   3. got_set_resp on the physical port is FALSE. This means we haven't seen
>  	   this port before and we need to send Set of PortInfo to it.
>  	 */
> -	if (send_set || p_mgr->p_subn->first_time_master_sweep == TRUE ||
> -	    p_physp->got_set_resp == FALSE) {
> +	if (p_mgr->p_subn->first_time_master_sweep == TRUE ||
> +	    p_physp->got_set_resp == FALSE)
> +		send_set = TRUE;
>  
> +	if (send_set) {
>  		p_mgr->send_set_reqs = TRUE;
>  		status = osm_req_set(p_mgr->p_req,
>  				     osm_physp_get_dr_path_ptr(p_physp),
> @@ -1199,8 +1201,7 @@ __osm_lid_mgr_set_physp_pi(IN osm_lid_mgr_t * const p_mgr,
>  
>        Exit:
>  	OSM_LOG_EXIT(p_mgr->p_log);
> -	return (send_set || p_mgr->p_subn->first_time_master_sweep == TRUE ||
> -		p_physp->got_set_resp == FALSE);
> +	return send_set;
>  }
>  
>  /**********************************************************************
> diff --git a/opensm/opensm/osm_link_mgr.c b/opensm/opensm/osm_link_mgr.c
> index 19d03d9..b151c76 100644
> --- a/opensm/opensm/osm_link_mgr.c
> +++ b/opensm/opensm/osm_link_mgr.c
> @@ -389,15 +389,12 @@ __osm_link_mgr_set_physp_pi(IN osm_link_mgr_t * const p_mgr,
>  	   b. got_set_resp on the physical port is FALSE. This means we haven't
>  	   seen this port before - need to send PortInfoSet to it.
>  	 */
> -	if (send_set ||
> -	    (osm_node_get_type(p_node) != IB_NODE_TYPE_SWITCH
> -	     && p_physp->got_set_resp == FALSE)
> -	    || (osm_node_get_type(p_node) == IB_NODE_TYPE_SWITCH
> -		&& port_num == 0 && p_physp->got_set_resp == FALSE)
> -	    || (osm_node_get_type(p_node) == IB_NODE_TYPE_SWITCH
> -		&& port_num != 0
> -		&& (p_mgr->p_subn->first_time_master_sweep == TRUE
> -		    || p_physp->got_set_resp == FALSE))) {
> +	if (p_physp->got_set_resp == FALSE
> +	    || (osm_node_get_type(p_node) == IB_NODE_TYPE_SWITCH && port_num
> +		&& p_mgr->p_subn->first_time_master_sweep == TRUE))

This doesn't look logically the same to me. I think it sets send_set in
some cases where it wasn't before.

-- Hal

> +		send_set = TRUE;
> +
> +	if (send_set) {
>  		p_mgr->send_set_reqs = TRUE;
>  		status = osm_req_set(p_mgr->p_req,
>  				     osm_physp_get_dr_path_ptr(p_physp),