[openib-general] RE: [PATCH 4/4] opensm: no need to wait for pkey_mgr

Eitan Zahavi eitan at mellanox.co.il
Tue May 2 01:55:47 PDT 2006


Hi Sasha,

This patch breaks the basic concept for OpenSM to report "SUBNET UP"
only if all SubnMgt.Set were sent successfully. I do not think we want
to remove this feature.

In detail (you probably know that so it is just clarification for the
audience):
OpenSM uses SMP MADs to initialize the fabric components. SMP delivery
reliability is provided through the fact each Set gets a GetResp. But
the actual delivery of the SMP is not guaranteed. The layers below
OpenSM provide "retry" capability such that if a GetResp is not received
in some time window the Set is resent. 
However, there is no guarantee that even with multiple retries the
packet will reach the destination. But in that case OpenSM eventually
receives a transaction timeout error.

When OpenSM fails to set some of the fabric components it does not give
up.
Instead it will report "errors during initialization" and will restart
its fabric configuration sequence. 

This patch breaks this concept: if OpenSM does not wait for the
partition setting to occur before completing the fabric configuration
"FULL SWEEP" it will not report "errors during initialization" nor start
a new sweep.

Eitan Zahavi
Design Technology Director
Mellanox Technologies LTD
Tel:+972-4-9097208
Fax:+972-4-9593245
P.O. Box 586 Yokneam 20692 ISRAEL


> -----Original Message-----
> From: Sasha Khapyorsky [mailto:sashak at voltaire.com]
> Sent: Sunday, April 23, 2006 5:26 PM
> To: Hal Rosenstock
> Cc: openib-general at openib.org; Eitan Zahavi; Ofer Gigi; Yael Kalka
> Subject: [PATCH 4/4] opensm: no need to wait for pkey_mgr
> 
> 
> Don't wait for pkey tables update responses in partition manager -
> we may just continue resweep process.
> 
> Signed-off-by: Sasha Khapyorsky <sashak at voltaire.com>
> ---
> 
>  osm/opensm/osm_pkey_mgr.c  |   66
+++++++++++++++++---------------------------
>  osm/opensm/osm_state_mgr.c |   41 ++-------------------------
>  2 files changed, 29 insertions(+), 78 deletions(-)
> 
> diff --git a/osm/opensm/osm_pkey_mgr.c b/osm/opensm/osm_pkey_mgr.c
> index da8dfa8..167b4c1 100644
> --- a/osm/opensm/osm_pkey_mgr.c
> +++ b/osm/opensm/osm_pkey_mgr.c
> @@ -135,7 +135,8 @@ pkey_mgr_enforce_partition(
>   * does not exist. Update existed entry when membership was changed.
>   */
> 
> -static void pkey_mgr_process_physical_port(
> +static void
> +pkey_mgr_process_physical_port(
>     IN osm_log_t *p_log,
>     IN const osm_req_t *p_req,
>     IN const ib_net16_t pkey,
> @@ -210,7 +211,7 @@ static void pkey_mgr_process_physical_po
> 
>
/**********************************************************************
>
**********************************************************************/
> -static boolean_t
> +static void
>  pkey_mgr_update_peer_port(
>     osm_log_t *p_log,
>     const osm_req_t *p_req,
> @@ -227,22 +228,21 @@ pkey_mgr_update_peer_port(
>     uint16_t block_index;
>     uint16_t num_of_blocks;
>     ib_api_status_t status = IB_SUCCESS;
> -   boolean_t ret_val = FALSE;
> 
>     p = osm_port_get_default_phys_ptr( p_port );
>     if ( !osm_physp_is_valid( p ) )
> -      return FALSE;
> +      return;
>     peer = osm_physp_get_remote( p );
>     if ( !peer || !osm_physp_is_valid( peer ) )
> -      return FALSE;
> +      return;
>     p_node = osm_physp_get_node_ptr( peer );
>     if ( osm_node_get_type( p_node ) != IB_NODE_TYPE_SWITCH )
> -      return FALSE;
> +      return;
> 
>     p_sw = osm_get_switch_by_guid( p_subn, osm_node_get_node_guid(
p_node ));
>     if (!p_sw || !(p_si = osm_switch_get_si_ptr( p_sw )) ||
>  	  !p_si->enforce_cap)
> -      return FALSE;
> +      return;
> 
>     if (pkey_mgr_enforce_partition( p_req, peer, enforce ) !=
IB_SUCCESS) {
>        osm_log( p_log, OSM_LOG_ERROR,
> @@ -254,7 +254,7 @@ pkey_mgr_update_peer_port(
>     }
> 
>     if (enforce == FALSE)
> -      return FALSE;
> +      return;
> 
>     p_pkey_tbl = osm_physp_get_pkey_tbl( p );
>     p_peer_pkey_tbl = osm_physp_get_pkey_tbl( peer );
> @@ -271,36 +271,30 @@ pkey_mgr_update_peer_port(
>           status =
>              pkey_mgr_update_pkey_entry( p_req, peer, block,
block_index );
>           if ( status == IB_SUCCESS )
> -	    ret_val = TRUE;
> +            osm_log( p_log, OSM_LOG_VERBOSE,
> +                     "pkey_mgr_update_peer_port: "
> +                     "pkey table block %u was updated for node
0x%016" PRIx64
> +                     " port %u\n",
> +                     block_index,
> +		     cl_ntoh64( osm_node_get_node_guid( p_node ) ),
> +                     osm_physp_get_port_num( peer ) );
>           else
>              osm_log( p_log, OSM_LOG_ERROR,
>                       "pkey_mgr_update_peer_port: "
>                       "pkey_mgr_update_pkey_entry() failed to update "
> -                     "pkey table block %d for node 0x%016" PRIx64
> +                     "pkey table block %u for node 0x%016" PRIx64
>                       " port %u\n",
>                       block_index,
>                       cl_ntoh64( osm_node_get_node_guid( p_node ) ),
>                       osm_physp_get_port_num( peer ) );
>        }
>     }
> -
> -   if ( ret_val == TRUE &&
> -        osm_log_is_active( p_log, OSM_LOG_VERBOSE ) )
> -   {
> -      osm_log( p_log, OSM_LOG_VERBOSE,
> -               "pkey_mgr_update_peer_port: "
> -               "pkey table was updated for node 0x%016" PRIx64
> -               " port %u\n",
> -               cl_ntoh64( osm_node_get_node_guid( p_node ) ),
> -               osm_physp_get_port_num( peer ) );
> -   }
> -
> -   return ret_val;
>  }
> 
>
/**********************************************************************
>
**********************************************************************/
> -static boolean_t pkey_mgr_update_port(
> +static void
> +pkey_mgr_update_port(
>     osm_log_t *p_log,
>     osm_req_t *p_req,
>     const osm_port_t * const p_port )
> @@ -312,11 +306,10 @@ static boolean_t pkey_mgr_update_port(
>     uint16_t block_index;
>     uint16_t num_of_blocks;
>     ib_api_status_t status;
> -   boolean_t ret_val = FALSE;
> 
>     p = osm_port_get_default_phys_ptr( p_port );
>     if ( !osm_physp_is_valid( p ) )
> -      return FALSE;
> +      return;
> 
>     p_pkey_tbl = osm_physp_get_pkey_tbl(p);
>     num_of_blocks = osm_pkey_tbl_get_num_blocks( p_pkey_tbl );
> @@ -331,9 +324,7 @@ static boolean_t pkey_mgr_update_port(
> 
>        status =
>              pkey_mgr_update_pkey_entry( p_req, p, new_block,
block_index );
> -      if (status == IB_SUCCESS)
> -         ret_val = TRUE;
> -      else
> +      if (status != IB_SUCCESS)
>           osm_log( p_log, OSM_LOG_ERROR,
>                    "pkey_mgr_update_port:  "
>                    "pkey_mgr_update_pkey_entry() failed to update "
> @@ -342,8 +333,6 @@ static boolean_t pkey_mgr_update_port(
>                    cl_ntoh64( osm_node_get_node_guid( p_node ) ),
>                    osm_physp_get_port_num( p ) );
>     }
> -
> -   return ret_val;
>  }
> 
>
/**********************************************************************
> @@ -385,7 +374,6 @@ osm_pkey_mgr_process(
>     cl_map_item_t *p_next;
>     osm_prtn_t *p_prtn;
>     osm_port_t *p_port;
> -   osm_signal_t signal = OSM_SIGNAL_DONE;
>     osm_physp_t *p_physp;
> 
>     CL_ASSERT( p_osm );
> @@ -428,18 +416,16 @@ osm_pkey_mgr_process(
>     {
>        p_port = ( osm_port_t * ) p_next;
>        p_next = cl_qmap_next( p_next );
> -      if (pkey_mgr_update_port(&p_osm->log, &p_osm->sm.req, p_port))
> -         signal = OSM_SIGNAL_DONE_PENDING;
> +      pkey_mgr_update_port(&p_osm->log, &p_osm->sm.req, p_port);
>        if (osm_node_get_type( osm_port_get_parent_node( p_port ) ) !=
> -           	IB_NODE_TYPE_SWITCH &&
> -                pkey_mgr_update_peer_port( &p_osm->log,
&p_osm->sm.req,
> -		                           &p_osm->subn, p_port,
> -
!p_osm->subn.opt.no_partition_enforcement ))
> -         signal = OSM_SIGNAL_DONE_PENDING;
> +           	IB_NODE_TYPE_SWITCH )
> +          pkey_mgr_update_peer_port( &p_osm->log, &p_osm->sm.req,
> +                                     &p_osm->subn, p_port,
> +
!p_osm->subn.opt.no_partition_enforcement );
>     }
> 
>   _err:
>     CL_PLOCK_RELEASE( &p_osm->lock );
>     OSM_LOG_EXIT( &p_osm->log );
> -   return ( signal );
> +   return OSM_SIGNAL_DONE;
>  }
> diff --git a/osm/opensm/osm_state_mgr.c b/osm/opensm/osm_state_mgr.c
> index 1aefc0b..194e51e 100644
> --- a/osm/opensm/osm_state_mgr.c
> +++ b/osm/opensm/osm_state_mgr.c
> @@ -2232,8 +2232,10 @@ osm_state_mgr_process(
>                 osm_sm_state_mgr_process( p_mgr->p_sm_state_mgr,
>
OSM_SM_SIGNAL_DISCOVERY_COMPLETED );
> 
> -            /* the returned signal might be DONE or DONE_PENDING */
> +            /* the returned signal will be always DONE */
>              signal = osm_pkey_mgr_process( p_mgr->p_subn->p_osm );
> +	    p_mgr->state = OSM_SM_STATE_SET_PKEY_DONE;
> +
>              break;
> 
>           default:
> @@ -2243,43 +2245,6 @@ osm_state_mgr_process(
>           }
>           break;
> 
> -      case OSM_SM_STATE_SET_PKEY:
> -        switch ( signal )
> -        {
> -        case OSM_SIGNAL_DONE:
> -          p_mgr->state = OSM_SM_STATE_SET_PKEY_DONE;
> -          break;
> -
> -        case OSM_SIGNAL_DONE_PENDING:
> -          /*
> -           * There are outstanding transactions, so we
> -           * must wait for the wire to clear.
> -           */
> -          p_mgr->state = OSM_SM_STATE_SET_PKEY_WAIT;
> -          signal = OSM_SIGNAL_NONE;
> -          break;
> -
> -        default:
> -          __osm_state_mgr_signal_error( p_mgr, signal );
> -          signal = OSM_SIGNAL_NONE;
> -          break;
> -        }
> -        break;
> -
> -      case OSM_SM_STATE_SET_PKEY_WAIT:
> -        switch ( signal )
> -        {
> -        case OSM_SIGNAL_NO_PENDING_TRANSACTIONS:
> -          p_mgr->state = OSM_SM_STATE_SET_PKEY_DONE;
> -          break;
> -
> -        default:
> -          __osm_state_mgr_signal_error( p_mgr, signal );
> -          signal = OSM_SIGNAL_NONE;
> -          break;
> -        }
> -        break;
> -
>        case OSM_SM_STATE_SET_PKEY_DONE:
>          switch ( signal )
>          {



More information about the general mailing list