[openib-general] ib_mad.c comments

Sean Hefty mshefty at ichips.intel.com
Wed Sep 8 16:47:39 PDT 2004


Here's a list of comments from reviewing ib_mad.c.  I'll use this list as kind of my to do list for the GSI.  Several of these can be delayed when implementing.  After we meet tomorrow, I will begin creating patches.  Overall, it's a good start.

ib_mad_reg():
	- Need to lock when checking/setting version/class/methods.
	- Need to support registrations for "all" methods of a given class.
	  (We may want the initial implementation to only do this for now,
	  to shorten the development time.)
	- Should we reference qp0 and qp1 with the registration?
	- Need to ensure unique tids in case of wrapping.
	
ib_mad_post_send():
	- We should return the error code from ib_post_send in order to
	  handle overruns differently.
	- The print level should be lowered from error.
	- Should we avoid casting the list_head to a structure where possible?

allocate_method_table():
	- Can just use memset to clear the table.

check_class_table():
	- Has an extra '{'.

ib_mad_recv_done_handler():
ib_mad_send_done_handler():
	- Not sure why these calls search for the corresponding work request.

ib_mad_post_receive_mads():
	- I think we can just pass &qp0 or &qp1, rather than a type to
	  ib_mad_post_receive_mad.
	- Print level should be lowered from error
	- We can track the number of posted receives to avoid posting overruns.

struct ib_mad_device_private:
	- If we make qp0 and qp1 an array, it may simply the code and remove
	  several checks from the code.

ib_mad_device_open():
	- A nit, but it's logically initializing a port on the device.
	- Remove +20 to CQ size.
	- We could change to using 1 PD/device, versus 1 PD/port.
	- Not sure if we need to support max_sge on the send queue.  This may
	  be substantially larger than what we need.  At a minimum, I think
	  that we need 2 for optimal RMPP support.  I'm not sure where the
	  trade-off is between SGE versus copying into a single buffer lies.

- Sean
-- 



More information about the general mailing list