[openib-general] [RFC] [PATCHv2] mad: Add RMPP support foradditional management classes which support it

[Hal Rosenstock]

Hi Sean,

On Fri, 2006-03-17 at 13:03, Hal Rosenstock wrote:
> On Fri, 2006-03-17 at 13:00, Sean Hefty wrote:
> > >-static int data_offset(u8 mgmt_class)
> > >+int ib_get_rmpp_data_offset(u8 mgmt_class)
> > > {
> > > 	if (mgmt_class == IB_MGMT_CLASS_SUBN_ADM)
> > > 		return IB_MGMT_SA_HDR;
> > >+	else if ((mgmt_class == IB_MGMT_CLASS_DEVICE_MGMT) ||
> > >+		 (mgmt_class == IB_MGMT_CLASS_DEVICE_ADM) ||
> > >+		 (mgmt_class == IB_MGMT_CLASS_BIS))
> > >+		return IB_MGMT_DEVICE_HDR;
> > > 	else if ((mgmt_class >= IB_MGMT_CLASS_VENDOR_RANGE2_START) &&
> > > 		 (mgmt_class <= IB_MGMT_CLASS_VENDOR_RANGE2_END))
> > > 		return IB_MGMT_VENDOR_HDR;
> > > 	else
> > > 		return IB_MGMT_RMPP_HDR;
> > > }
> > >+EXPORT_SYMBOL(ib_get_rmpp_data_offset);
> > 
> > Is the base RMPP header (return value of IB_MGMT_RMPP_HDR) ever actually used by
> > any class?
> 
> With no class specific data ? I think that's possible but I don't think
> any classes do this.
> 
> > Maybe we should rename this routine to ib_get_mad_data_offset(), and
> > let it work on any type of MAD, versus RMPP only.  Then change IB_MGMT_RMPP_HDR
> > to IB_MGMT_MAD_HDR.
> 
> That can be done but it changes the the places in mad_rmpp.c where
> data_offset is called (albeit for classes where RMPP shouldn't be
> invoked). I didn't walk the code as to how this could occur...

I looked through the code and here's my take:
If we validate ib_register_mad_agent and ib_post_send_mad so that RMPP
is consistent with the MAD class being used, then I think what you say
about chaing data_offset makes sense. (In the case of register agent, it
is the management class in the registration request.) Does that make
sense to you ? If so, I will work up a patch for this approach.

-- Hal

> > >+	hdr_len = ib_get_rmpp_data_offset(rmpp_mad->mad_hdr.mgmt_class);
> > >+	if (hdr_len == IB_MGMT_RMPP_HDR) {
> > 
> > This looks like a check to see if the mgmt_class uses RMPP or not.
> 
> Yes, that's what it is doing.
> 
> > If we want
> > to keep ib_get_rmpp_data_offset as shown above, maybe we can check whether the
> > mad_agent uses RMPP, and only call ib_get_rmpp_data_offset if it does.
> 
> Yes, that would be clearer.
> 
> -- Hal
> 
> > - Sean
> 
> _______________________________________________
> openib-general mailing list
> openib-general at openib.org
> http://openib.org/mailman/listinfo/openib-general
> 
> To unsubscribe, please visit http://openib.org/mailman/listinfo/openib-general