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
_______________________________________________
openib-general mailing list
openib-general@openib.org
http://openib.org/mailman/listinfo/openib-general
To unsubscribe, please visit http://openib.org/mailman/listinfo/openib-general
