[openib-general] InfiniPath driver announcement

Grant Grundler iod00d at hp.com
Wed Sep 28 16:17:41 PDT 2005


On Wed, Sep 28, 2005 at 12:50:07PM -0700, Robert Walsh wrote:
> Hi all,
> 
> PathScale is pleased to announce the availability of OpenIB drivers for
> the InfiniPath HCA.  The complete code for this first release of the
> driver has been checked into the OpenIB repository under the
> svn/gen2/branches/ipath directory.
>         
> Comments and feedback are welcome.

Cool!
Congrats!

Here is some initial feedback on infiniband/hw/ipath/ib_ipath/ipath_openib.c.
I pulled the source using
	svn co https://www.openib.org/svn/gen2/branches/ipath

o Please Remove #ifdef LINUX_VERSION_CODE checks.
  Those are welcome in a seperate patch for backports.

o use of cmp24() seems hokey in several cases:
	o "<<8" and then compare against 0x100?
	o "cmp24(psn, qp->s_next_psn) >= 0" where psn is u32?
	  I think the "int" cast guarantees the shift left
	  is an arithmetic opertation and not a logical one.
	  This seems correct but non-intuitive given the
	  operands and fact that the subtraction is done
	  starting with u32 in many cases.
	o ditto for cmp24(credit,...) (credit is u32)

o Why all the "#pragma weak" statements?
  AFAICT, This doesn't exist in any other part of the kernel.

o lots of things are named ib_openib_*.
  Please rename those ib_ipath or something like that.

o in ipath_multicast_detach():
	        /* Find the GID in the mcast table. */
		for (n = mcast_tree.rb_node;;) {

  would be better as:
	n = mcast_tree.rb_nod;
	while (1) {

sorry, out of time...but something to start with.

hth,
grant



More information about the general mailing list