[openib-general] RE: [PATCH] Opensm - clean osm_vendor_mlx_sa.c code

[Yael Kalka]

Hi Hal,
In answer to your questions:
1. This is still one code base for gen1 too. 
2. I don't think it is necessary to add osm_arbitrary_context_t in all
   vendors, just in the ones using it.
Yael


-----Original Message-----
From: Hal Rosenstock [mailto:halr at voltaire.com]
Sent: Thursday, February 09, 2006 9:09 PM
To: Yael Kalka
Cc: openib-general at openib.org; Eitan Zahavi
Subject: Re: [PATCH] Opensm - clean osm_vendor_mlx_sa.c code


Hi Yael,

On Mon, 2006-02-06 at 07:39, Yael Kalka wrote:
> Hi Hal,
> 
> Currently in osm_vendor_mlx_sa.c the sent context is saved arbitrarily
> as nodeInfo_context. This results in need for strange castings from
> long to pointer and vice-versa. The following patch adds another
> possible context - arbitrary context, which will be used in this case.

Thanks. Applied with one question below.

BTW, I have no way to test this (other than that things still work for
OpenIB). Is this still one code base for gen1 too ?

-- Hal

> Thanks,
> Yael
> 
> Signed-off-by:  Yael Kalka <yael at mellanox.co.il>
> 
> Index: libvendor/osm_vendor_mlx_sa.c
> ===================================================================
> --- libvendor/osm_vendor_mlx_sa.c	(revision 5307)
> +++ libvendor/osm_vendor_mlx_sa.c	(working copy)
> @@ -96,9 +96,9 @@ __osmv_sa_mad_rcv_cb(
>      goto Exit;
>    }
>  
> -  /* obtain the sent context since we store it during send in the ni_ctx */
> +  /* obtain the sent context */
>    p_query_req_copy =
> -	(osmv_query_req_t *)CAST_P2LONG(p_req_madw->context.ni_context.node_guid);
> +	(osmv_query_req_t *)(p_req_madw->context.arb_context.context1);
>  
>    /* provide the context of the original request in the result */
>    query_res.query_context = p_query_req_copy->query_context;
> @@ -207,7 +207,7 @@ __osmv_sa_mad_err_cb(
>  
>    /* Obtain the sent context etc */
>    p_query_req_copy =
> -	(osmv_query_req_t *)CAST_P2LONG(p_madw->context.ni_context.node_guid);
> +	(osmv_query_req_t *)(p_madw->context.arb_context.context1);
>  
>    /* provide the context of the original request in the result */
>    query_res.query_context = p_query_req_copy->query_context;
> @@ -561,10 +561,17 @@ __osmv_send_sa_req(
>    /*
>      Provide the address to send to
>    */
> +  /* Patch to handle IBAL - host order , where it should take destination lid in network order */
> +#ifdef OSM_VENDOR_INTF_AL
> +  p_madw->mad_addr.dest_lid = p_bind->sm_lid;
> +#else
>    p_madw->mad_addr.dest_lid = cl_hton16(p_bind->sm_lid);
> +#endif
>    p_madw->mad_addr.addr_type.smi.source_lid =
>      cl_hton16(p_bind->lid);
>    p_madw->mad_addr.addr_type.gsi.remote_qp = CL_HTON32(1);
> +  p_madw->mad_addr.addr_type.gsi.remote_qkey = IB_QP1_WELL_KNOWN_Q_KEY;
> +  p_madw->mad_addr.addr_type.gsi.pkey = IB_DEFAULT_PKEY;
>    p_madw->resp_expected = TRUE;
>    p_madw->fail_msg = CL_DISP_MSGID_NONE;
>  
> @@ -574,12 +581,11 @@ __osmv_send_sa_req(
>      Since we can not rely on the client to keep it arroud until
>      the response - we duplicate it and will later dispose it (in CB).
>      To store on the MADW we cast it into what opensm has:
> -    p_madw->context.ni_context.node_guid
> +    p_madw->context.arb_context.context1
>    */
>    p_query_req_copy = cl_malloc(sizeof(*p_query_req_copy));
>    *p_query_req_copy = *p_query_req;
> -  p_madw->context.ni_context.node_guid =
> -    (ib_net64_t)CAST_P2LONG(p_query_req_copy);
> +  p_madw->context.arb_context.context1 = p_query_req_copy;
>  
>    /* we can support async as well as sync calls */
>    sync = ((p_query_req->flags & OSM_SA_FLAGS_SYNC) == OSM_SA_FLAGS_SYNC);
> Index: include/opensm/osm_madw.h
> ===================================================================
> --- include/opensm/osm_madw.h	(revision 5307)
> +++ include/opensm/osm_madw.h	(working copy)
> @@ -315,6 +315,22 @@ typedef struct _osm_vla_context
>    boolean_t			   set_method;
>  } osm_vla_context_t;
>  /*********/
> +/****s* OpenSM: MAD Wrapper/osm_arbitrary_context_t
> +* NAME
> +*	osm_sa_context_t
> +*
> +* DESCRIPTION
> +*	Context needed by arbitrary recipient.
> +*
> +* SYNOPSIS
> +*/
> +typedef struct _osm_arbitrary_context
> +{
> +  void*             context1;
> +  void*             context2;
> +} osm_arbitrary_context_t;
> +/*********/
> +
>  /****s* OpenSM: MAD Wrapper/osm_madw_context_t
>  * NAME
>  *	osm_madw_context_t
> @@ -335,6 +351,7 @@ typedef union _osm_madw_context
>  	osm_smi_context_t		smi_context;
>  	osm_slvl_context_t	slvl_context;
>  	osm_pkey_context_t	pkey_context;
> +	osm_arbitrary_context_t arb_context;

Should this be carried for for all vendor layers or only the ones which
need this ?

>  } osm_madw_context_t;
>  /*********/
>  
> @@ -880,6 +897,34 @@ osm_madw_get_vla_context_ptr(
>  }
>  /*
>  * PARAMETERS
> +*	p_madw
> +*		[in] Pointer to an osm_madw_t object.
> +*
> +* RETURN VALUES
> +*	Pointer to the start of the context structure.
> +*
> +* NOTES
> +*
> +* SEE ALSO
> +*********/
> +
> +/****f* OpenSM: MAD Wrapper/osm_madw_get_arbitrary_context_ptr
> +* NAME
> +*	osm_madw_get_arbitrary_context_ptr
> +*
> +* DESCRIPTION
> +*	Gets a pointer to the arbitrary context in this MAD.
> +*
> +* SYNOPSIS
> +*/
> +static inline osm_arbitrary_context_t*
> +osm_madw_get_arbitrary_context_ptr(
> +	IN const osm_madw_t* const p_madw )
> +{
> +	return( (osm_arbitrary_context_t*)&p_madw->context );
> +}
> +/*
> +* PARAMETERS
>  *	p_madw
>  *		[in] Pointer to an osm_madw_t object.
>  *
>