[ofa-general] Re: [PATCH] opensm: fix PortInfo update issues.

[Hal Rosenstock]

On Mon, 2007-11-12 at 23:09 +0200, Sasha Khapyorsky wrote:
> This fixes PortInfo update issues:
> - handle switch's port 0 (esp0 in link_mgr) as end port
> - remove check which is always FALSE (since *p_pi = *p_old_pi)
> 
> Signed-off-by: Sasha Khapyorsky <sashak at voltaire.com>
> ---
>  opensm/opensm/osm_lid_mgr.c  |   22 ++--------------------
>  opensm/opensm/osm_link_mgr.c |   13 ++-----------
>  2 files changed, 4 insertions(+), 31 deletions(-)
> 
> diff --git a/opensm/opensm/osm_lid_mgr.c b/opensm/opensm/osm_lid_mgr.c
> index d25605f..076d0e9 100644
> --- a/opensm/opensm/osm_lid_mgr.c
> +++ b/opensm/opensm/osm_lid_mgr.c
> @@ -959,15 +959,9 @@ __osm_lid_mgr_set_physp_pi(IN osm_lid_mgr_t * const p_mgr,
>  	 */
>  
>  	memset(payload, 0, IB_SMP_DATA_SIZE);
> -
> -	/* Correction by FUJITSU */
> -	if (port_num != 0)
> -		memcpy(payload, p_old_pi, sizeof(ib_port_info_t));
> +	memcpy(payload, p_old_pi, sizeof(ib_port_info_t));
>  
>  	/*
> -	   Correction following a bug injected by the previous
> -	   FUJITSU line:
> -
>  	   Should never write back a value that is bigger then 3 in
>  	   the PortPhysicalState field, so cannot simply copy!
>  
> @@ -976,19 +970,7 @@ __osm_lid_mgr_set_physp_pi(IN osm_lid_mgr_t * const p_mgr,
>  	   link down default state = polling
>  	   port state - no change
>  	 */
> -	/* these values can be set only for ca ports, so if we are
> -	   on a switch node, set these values to zero */
> -	if (osm_node_get_type(p_node) == IB_NODE_TYPE_SWITCH)
> -		p_pi->state_info2 = 0x0;
> -	else {
> -		p_pi->state_info2 = 0x02;
> -		/* Check to see if the value we are setting is different than
> -		   the value in the port_info. If it is, turn on send_set flag */
> -		if (ib_port_info_get_link_down_def_state(p_pi) !=
> -		    ib_port_info_get_link_down_def_state(p_old_pi))
> -			send_set = TRUE;
> -	}
> -
> +	p_pi->state_info2 = 0x02;

Isn't this a potential send_set change (as this could change LDDS) ?

>  	ib_port_info_set_port_state(p_pi, IB_LINK_NO_CHANGE);
>  
>  	p_pi->m_key = p_mgr->p_subn->opt.m_key;
> diff --git a/opensm/opensm/osm_link_mgr.c b/opensm/opensm/osm_link_mgr.c
> index b151c76..80e34b2 100644
> --- a/opensm/opensm/osm_link_mgr.c
> +++ b/opensm/opensm/osm_link_mgr.c
> @@ -166,27 +166,19 @@ __osm_link_mgr_set_physp_pi(IN osm_link_mgr_t * const p_mgr,
>  	p_old_pi = &p_physp->port_info;
>  
>  	memset(payload, 0, IB_SMP_DATA_SIZE);
> -
> -	/* Correction by FUJITSU */
>  	memcpy(payload, p_old_pi, sizeof(ib_port_info_t));
>  
>  	/*
> -	   Correction following a bug injected by the previous
> -	   FUJITSU line:
> -
>  	   Should never write back a value that is bigger then 3 in
>  	   the PortPhysicalState field - so can not simply copy!
>  
>  	   Actually we want to write there:
>  	   port physical state - no change,
>  	   link down default state = polling
> -	   port state - no change
> +	   port state - as requested.
>  	 */
>  	p_pi->state_info2 = 0x02;
> -	ib_port_info_set_port_state(p_pi, IB_LINK_NO_CHANGE);
> -	if (ib_port_info_get_link_down_def_state(p_pi) !=
> -	    ib_port_info_get_link_down_def_state(p_old_pi))
> -		send_set = TRUE;

Same as above.

> +	ib_port_info_set_port_state(p_pi, port_state);
>  
>  	/* we only change port fields if we do not change state */
>  	if (port_state == IB_LINK_NO_CHANGE) {
> @@ -358,7 +350,6 @@ __osm_link_mgr_set_physp_pi(IN osm_link_mgr_t * const p_mgr,
>  		 */
>  		context.pi_context.ignore_errors = FALSE;
>  
> -	ib_port_info_set_port_state(p_pi, port_state);
>  	if (port_state != IB_LINK_NO_CHANGE &&
>  	    ib_port_info_get_port_state(p_pi) !=
>  	    ib_port_info_get_port_state(p_old_pi)) {