[ofw] [PATCH 4/4] MAD: fix issues routing vendor MADs

[Leonid Keller]

Thank you.
You may commit all this patch series. 

> -----Original Message-----
> From: Sean Hefty [mailto:sean.hefty at intel.com] 
> Sent: Tuesday, May 12, 2009 6:25 PM
> To: Leonid Keller; ofw at lists.openfabrics.org
> Subject: RE: [ofw] [PATCH 4/4] MAD: fix issues routing vendor MADs
> 
> >Could you elaborate on removing IB_MCLASS_SUBN_ADM and some other 
> >management classes and on the changes in GMP handling ?
> 
> Unless I missed something, the handling should be the same.  
> See details below.
> 
> >> ); diff -up -r -X \mshefty\scm\winof\trunk\docs\dontdiff.txt
> >> -I '\$Id:' trunk\core\al/kernel/al_smi.c 
> >> branches\winverbs\core\al/kernel/al_smi.c
> >> --- trunk\core\al/kernel/al_smi.c	2009-01-26
> >> 09:34:30.250875000 -0800
> >> +++ branches\winverbs\core\al/kernel/al_smi.c	2009-05-07
> >> 15:53:12.270907500 -0700
> >> @@ -184,12 +184,12 @@ mad_route_t
> >>  route_recv_dm_mad(
> >>  	IN				ib_mad_element_t*
> >> 		p_mad_element );
> >>
> >> -mad_route_t
> >> -route_recv_gmp(
> >> +static mad_route_t
> >> +route_recv_bm(
> 
> Rename route_recv_gmp to route_recv_bm.  This core of this 
> function was only really called for BM MADs.
> 
> >>  	IN				ib_mad_element_t*
> >> 		p_mad_element );
> >>
> >> -mad_route_t
> >> -route_recv_gmp_attr(
> >> +static mad_route_t
> >> +route_recv_perf(
> 
> This isn't a rename.  Route_recv_gmp_attr folds into 
> route_recv_bm.  A new call is added to check for perf MADs.
> 
> >> p_mad_wr,IB_WCS_SUCCESS); @@ -2937,35 +2950,19 @@ process_mad_recv(
> >>  			break;
> >>
> >>  		case IB_MCLASS_PERF:
> >> -			/* Process the received GMP. */
> >> -			switch( p_mad_element->p_mad_buf->method )
> >> -			{
> >> -			case IB_MAD_METHOD_GET:
> >> -			case IB_MAD_METHOD_SET:
> >> -				route = ROUTE_LOCAL;
> >> -				break;
> >> -			default:
> >> -				break;
> >> -			}
> >> +			route = route_recv_perf( p_mad_element );
> 
> The perf MAD handling code above goes into the new 
> route_recv_perf call.  It just makes the code prettier.
> 
> >>  			break;
> >>
> >>  		case IB_MCLASS_BM:
> >> -			route = route_recv_gmp( p_mad_element );
> >> +			route = route_recv_bm( p_mad_element );
> 
> We're dealing with BM MADs here only.
> 
> >>  			break;
> >>
> >> -		case IB_MCLASS_SUBN_ADM:
> >> -		case IB_MCLASS_DEV_MGMT:
> >> -		case IB_MCLASS_COMM_MGMT:
> >> -		case IB_MCLASS_SNMP:
> >> +		case IB_MLX_VENDOR_CLASS1:
> >> +		case IB_MLX_VENDOR_CLASS2:
> >> +			route = ROUTE_LOCAL;
> >>  			break;
> >>
> >>  		default:
> >> -			/* Route vendor specific MADs to the
> >> HCA provider. */
> >> -			if( ib_class_is_vendor_specific(
> >> -				p_mad_element->p_mad_buf->mgmt_class ) )
> >> -			{
> >> -				route = route_recv_gmp( p_mad_element );
> 
> This is the only other call to route_recv_gmp, and it is 
> limited to vendor specific MADs.  The MLX vendor class MADs 
> are routed locally.  All other vendor defined MADs are 
> dispatched normally.  This is a change to routing of vendor MADs.
> 
> >> @@ -3090,72 +3087,38 @@ route_recv_smp_attr(  }
> >>
> >>
> >> -/*
> >> - * Route a received GMP.
> >> - */
> >> -mad_route_t
> >> -route_recv_gmp(
> >> +static mad_route_t
> >> +route_recv_bm(
> >>  	IN				ib_mad_element_t*
> >> 		p_mad_element )
> >>  {
> >> -	mad_route_t				route;
> >> -
> >> -	AL_ENTER( AL_DBG_SMI );
> >> -
> >> -	CL_ASSERT( p_mad_element );
> >> -
> >> -	/* Process the received GMP. */
> >>  	switch( p_mad_element->p_mad_buf->method )
> >>  	{
> >>  	case IB_MAD_METHOD_GET:
> >>  	case IB_MAD_METHOD_SET:
> >> -		/* Route vendor specific MADs to the HCA provider. */
> >> -		if( ib_class_is_vendor_specific(
> >> -			p_mad_element->p_mad_buf->mgmt_class ) )
> >> -		{
> >> -			route = ROUTE_LOCAL;
> >> -		}
> >> -		else
> >> -		{
> >> -			route = route_recv_gmp_attr( p_mad_element );
> >> -		}
> 
> Based on the two places where route_recv_gmp is called, 
> route_recv_gmp_attr only ends up being called for BM MADs.
> 
> >> +		if( p_mad_element->p_mad_buf->attr_id ==
> >> IB_MAD_ATTR_CLASS_PORT_INFO )
> >> +			return ROUTE_LOCAL;
> 
> route_recv_gmp_attr returns local based on this check.  I 
> merged the two calls.
> 
> >>  		break;
> >> -
> >>  	default:
> >> -		route = ROUTE_DISPATCHER;
> >>  		break;
> >>  	}
> >> -
> >> -	AL_EXIT( AL_DBG_SMI );
> >> -	return route;
> >> +	return ROUTE_DISPATCHER;
> >>  }
> >>
> >> -
> >> -
> >> -/*
> >> - * Route received GMP attributes.
> >> - */
> >> -mad_route_t
> >> -route_recv_gmp_attr(
> >> +static mad_route_t
> >> +route_recv_perf(
> >>  	IN				ib_mad_element_t*
> >> 		p_mad_element )
> >>  {
> >> -	mad_route_t				route;
> >> -
> >> -	AL_ENTER( AL_DBG_SMI );
> >> -
> >> -	CL_ASSERT( p_mad_element );
> >> -
> >> -	/* Process the received GMP attributes. */
> >> -	if( p_mad_element->p_mad_buf->attr_id ==
> >> IB_MAD_ATTR_CLASS_PORT_INFO )
> >> -		route = ROUTE_LOCAL;
> 
> This is the all moved into route_recv_bm
> 
> >> -	else
> >> -		route = ROUTE_DISPATCHER;
> >> -
> >> -	AL_EXIT( AL_DBG_SMI );
> >> -	return route;
> >> +	switch( p_mad_element->p_mad_buf->method )
> >> +	{
> >> +	case IB_MAD_METHOD_GET:
> >> +	case IB_MAD_METHOD_SET:
> >> +		return ROUTE_LOCAL;
> >> +	default:
> >> +		break;
> >> +	}
> >> +	return ROUTE_DISPATCHER;
> >>  }
> 
> This is the perf MAD handling code moved into a new call.
> 
> Hope that answers your question.
> 
> - Sean
> 
>