[openib-general] Re: [RFC] [PATCH] OpenSM: Protect against bad LIDs returned from faulty hardware/SMA

[Eitan Zahavi]

Hi Hal,

Good catch!
So I guess the osm_lid_mgr is broken when the given lid it out of
range.

My comment is nit picking but I think that a function that validate and modify
LID it got from the HW should have a more meaningful name to clarify the "modify".

I would propose:
osm_physp_fix_out_of_range_base_lid
osm_physp_validate_and_fix_base_lid
osm_physp_trim_base_lid_to_valid_range


Eitan


Hal Rosenstock wrote:
> OpenSM: Protect against bad LIDs returned from faulty hardware/SMA
> 
> Signed-off-by: Hal Rosenstock <halr at voltaire.com>
> 
> Index: include/opensm/osm_port.h
> ===================================================================
> --- include/opensm/osm_port.h	(revision 3590)
> +++ include/opensm/osm_port.h	(working copy)
> @@ -346,7 +346,7 @@ osm_physp_is_healthy(
>  *	Returns TRUE if the Physical Port has been maked as healthy
>  *	FALSE otherwise.
>  *  All physical ports are initialized as "healthy" but may be marked 
> -*  otherwise if a  received trap claims otherwise.
> +*  otherwise if a received trap claims otherwise.
>  *
>  * NOTES
>  *
> @@ -456,6 +456,44 @@ osm_physp_set_port_info(
>  *	Port, Physical Port
>  *********/
>  
> +/****f* OpenSM: Physical Port/osm_physp_validate_base_lid
> +* NAME
> +*  osm_physp_validate_base_lid
> +*
> +* DESCRIPTION
> +*  Validates the base LID in the Physical Port object.
> +*
> +* SYNOPSIS
> +*/
> +static inline ib_net16_t 
> +osm_physp_validate_base_lid(
> +	IN osm_physp_t* const p_physp )
> +{
> +	ib_net16_t orig_lid = 0;
> +
> +	CL_ASSERT( osm_physp_is_valid( p_physp ) );
> +	if ( cl_ntoh16( p_physp->port_info.base_lid ) >
> IB_LID_UCAST_END_HO )
> +	{
> +		orig_lid = p_physp->port_info.base_lid;
> +		p_physp->port_info.base_lid = 0;
> +	}
> +	return orig_lid;
> +}
> +/*
> +* PARAMETERS
> +*	p_physp
> +*		[in] Pointer to an osm_physp_t object.
> +*
> +* RETURN VALUES
> +*	Returns 0 if the base LID in the Physical port object is valid.
> +*	Returns original invalid LID otherwise.
> +*
> +* NOTES
> +*
> +* SEE ALSO
> +*	Port, Physical Port
> +*********/
> +
>  /****f* OpenSM: Physical Port/osm_physp_set_pkey_tbl
>  * NAME
>  *  osm_physp_set_pkey_tbl
> Index: opensm/osm_port_info_rcv.c
> ===================================================================
> --- opensm/osm_port_info_rcv.c	(revision 3590)
> +++ opensm/osm_port_info_rcv.c	(working copy)
> @@ -235,6 +235,7 @@ __osm_pi_rcv_process_switch_port(
>    osm_madw_context_t context;
>    osm_physp_t *p_remote_physp;
>    osm_node_t *p_remote_node;
> +  ib_net16_t orig_lid;
>    uint8_t port_num;
>    uint8_t remote_port_num;
>    osm_dr_path_t path;
> @@ -346,8 +347,13 @@ __osm_pi_rcv_process_switch_port(
>  
>    if (port_num == 0)
>    {
> -    /* This is a management port 0 */
> -		__osm_pi_rcv_process_endport(p_rcv, p_physp, p_pi);
> + 	/* This is switch management port 0 */
> +	if ( ( orig_lid = osm_physp_validate_base_lid( p_physp ) ) )
> +		osm_log( p_rcv->p_log, OSM_LOG_ERROR,
> +			 "__osm_pi_rcv_process_switch_port: ERR 0F04: "
> +			 "Invalid base LID 0x%x corrected.\n",
> +			 cl_ntoh16( orig_lid ) );
> +	__osm_pi_rcv_process_endport(p_rcv, p_physp, p_pi);
>    }
>  
>    OSM_LOG_EXIT( p_rcv->p_log );
> @@ -362,11 +368,18 @@ __osm_pi_rcv_process_ca_port(
>    IN osm_physp_t* const p_physp,
>    IN const ib_port_info_t* const p_pi )
>  {
> +  ib_net16_t orig_lid;
> +
>    OSM_LOG_ENTER( p_rcv->p_log, __osm_pi_rcv_process_ca_port );
>  
>    UNUSED_PARAM( p_node );
>  
>    osm_physp_set_port_info( p_physp, p_pi );
> +  if ( (orig_lid = osm_physp_validate_base_lid( p_physp ) ) )
> +    osm_log( p_rcv->p_log, OSM_LOG_ERROR,
> +	     "__osm_pi_rcv_process_ca_port: ERR 0F08: "
> +	     "Invalid base LID 0x%x corrected.\n",
> +	     cl_ntoh16 ( orig_lid ) );
>  
>    __osm_pi_rcv_process_endport(p_rcv, p_physp, p_pi);
>  
> @@ -382,6 +395,8 @@ __osm_pi_rcv_process_router_port(
>    IN osm_physp_t* const p_physp,
>    IN const ib_port_info_t* const p_pi )
>  {
> +  ib_net16_t orig_lid;
> +
>    OSM_LOG_ENTER( p_rcv->p_log, __osm_pi_rcv_process_router_port );
>  
>    UNUSED_PARAM( p_node );
> @@ -390,6 +405,11 @@ __osm_pi_rcv_process_router_port(
>      Update the PortInfo attribute.
>    */
>    osm_physp_set_port_info( p_physp, p_pi );
> +  if ( (orig_lid = osm_physp_validate_base_lid( p_physp ) ) )
> +    osm_log( p_rcv->p_log, OSM_LOG_ERROR,
> +	     "__osm_pi_rcv_process_router_port: ERR 0F09: "
> +	     "Invalid base LID 0x%x corrected.\n",
> +	     cl_ntoh16 ( orig_lid) );
>  
>    OSM_LOG_EXIT( p_rcv->p_log );
>  }
> 
>