from rmpp.c::ib_create_send_mad ... if (mad_agent->rmpp_version) { struct ib_rmpp_mad *rmpp_mad; rmpp_mad = (struct ib_rmpp_mad *)send_buf->mad; rmpp_mad->rmpp_hdr.paylen_newwin = cpu_to_be32(hdr_len - offsetof(struct ib_rmpp_mad, data) + data_len); }
Should the above also be based on whether RMPP is active in the MAD to be sent ? I guess this depends on whether one MAD agent can be shared for RMPP and non RMPP sends.
ib_create_send_mad doesn't have the information to know if the MAD will active RMPP or not, just that the header is present. Hmm... I was originally thinking that it wouldn't matter if this field was set or not, but if RMPP is not active, the field is supposed to be set to 0.
I can think of a couple of options here. We can extend this API to indicate if RMPP is active or not. Have the user clear this field if RMPP is not active. Or let the RMPP code clear the field when sending the MAD...
It might even make sense to have calls to initialize the MAD and RMPP header information given a list of parameters.
Also from rmpp.c::ib_create_send_mad:
send_buf->send_wr.wr.ud.remote_qkey = IB_QP_SET_QKEY;
Should the remote_qkey be a passed in parameter to this routine ?
The user could override this once the MAD has been returned, but I don't object to adding this...
from mad_rmpp.c: static int data_offset(u8 mgmt_class)
{
if (mgmt_class == IB_MGMT_CLASS_SUBN_ADM)
return offsetof(struct ib_sa_mad, data);
else if ((mgmt_class >= IB_MGMT_CLASS_VENDOR_RANGE2_START) &&
(mgmt_class <= IB_MGMT_CLASS_VENDOR_RANGE2_END))
return offsetof(struct ib_vendor_mad, data);
else
return offsetof(struct ib_rmpp_mad, data);
}
Seems like there are a number of places where vendor range 2 is checked. There already is a (static) function for this in mad.c. Should this be used (and made global) ?
I thought about this too, but didn't think that this check was complex or long enough to create a function for it... I doubt many clients outside of the MAD layer itself would need such a check however, so sharing it between mad.c and mad_rmpp.c seems fine, but I doubt we need an exposed API.
- Sean _______________________________________________ openib-general mailing list [email protected] http://openib.org/mailman/listinfo/openib-general
To unsubscribe, please visit http://openib.org/mailman/listinfo/openib-general
