Hi Al, On Mon, Sep 19, 2011 at 6:06 PM, Albert Chu <[email protected]> wrote: > The following patches add a new tool ibccquery to infiniband-diags. It > supports the querying of various congestion control settings. Related > updates to libibmad are also included.
Looks good to me :-) Just a few comments below: Attaching rather than inlining patches makes it harder to comment. On 0001-Add-support-for-congestion-control-mads.patch, is ib_rpc_cc_t really needed ? Couldn't mkey in existing rpc struct just be reused/overloaded for this (and change comment to indicate mkey or cckey) and then some code could be eliminated ? On 0001-Support-ibccquery-congestion-control-query-tool.patch, I'm worried about the following: + /* XXX: Q3/2010 errata lists first entry offset at 80, but we assume + * will be updated to 96 once CurrentTimeStamp field is word aligned. + * In addition, assume max 13 log events instead of 16. Due to + * errata changes increasing size of CA log event, 16 log events is + * no longer possible to fit in max MAD size. + */ As far as the 13 v. 16 entries, this appears correct to me (MAD size) but I'm concerned about changing the offset from 80 to 96 for better alignment as this is putting the cart before the horse a little as since these changes have not been finalized AFAIK at the IBTA. Also, would you comment on what testing has been done with this ? -- Hal > Al > > -- > Albert Chu > [email protected] > Computer Scientist > High Performance Systems Division > Lawrence Livermore National Laboratory > > -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
