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

[Hal Rosenstock]

On Sun, 2006-02-12 at 02:52, Yael Kalka wrote:
> 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.

That looks to me like OSMV_SIM, OSMV_GEN1, and OSMV_VAPI. Can you
confirm ?

-- Hal

> 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.
> >  *
> >