[openib-general] [RFC] [PATCH] mad: Add RMPP support for additionalmanagement classes

[Hal Rosenstock]

On Thu, 2006-03-16 at 23:09, Sean Hefty wrote:
> > /**
> >+ * ib_get_rmpp_data_offset - returns the data offset for a given
> >+ * management class.
> >+ * @mgmt_class: management class
> >+ *
> >+ * This routine returns the data offset in the MAD for the management
> >+ * class requested.
> >+ */
> >+int ib_get_rmpp_data_offset(u8 mgmt_class);
> 
> Maybe we should just make this an inline routine.

> >-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 0;
> 
> I think that this should return IB_MGMT_RMPP_HDR as the default, rather than 0,
> since that's the data offset.

Yes, that's the way it was but I'm wondering what is the data offset for
a management class which does not support RMPP ? Guess a better way of
phrasing that is "a management class which isn't supposed to support
RMPP yet apparently RMPP is active on" ? Maybe that's a separate issue
anyhow. That's why in user_mad.c::copy_recv_mad, the hdr_len returned
needs to be overwritten but perhaps there's an RMPP reason for it being
this way.

> >+static int data_offset(u8 mgmt_class)
> >+{
> >+	int offset;
> >+
> >+	offset = ib_get_rmpp_data_offset(mgmt_class);
> >+	if (!offset)
> > 		return IB_MGMT_RMPP_HDR;
> >+	return offset;
> > }
> 
> Then we can drop this routine.
> 
> > static void send_handler(struct ib_mad_agent *agent,
> > 			 struct ib_mad_send_wc *send_wc)
> > {
> >@@ -283,7 +272,9 @@ static ssize_t copy_recv_mad(char __user
> > 			 */
> > 			return -ENOSPC;
> > 		}
> >-		offset = data_offset(recv_buf->mad->mad_hdr.mgmt_class);
> >+		offset = ib_get_rmpp_data_offset(recv_buf->mad-
> >>mad_hdr.mgmt_class);
> >+		if (!offset)
> >+			offset = IB_MGMT_RMPP_HDR;
> 
> And remove the if statement here as well.
> 
> >+	hdr_len = ib_get_rmpp_data_offset(rmpp_mad->mad_hdr.mgmt_class);
> >+	if (hdr_len) {
> 
> But we'll need to adjust hdr_len here.  Maybe subtract off IB_MGMT_RMPP_HDR
> length, or compare hdr_len > IB_MGMT_RMPP_HDR.  (I'm not sure if hdr_len is used
> further down in this code, but can check that tomorrow.)

It is used further down in the code.

-- Hal

> 
> - Sean