Re: [PATCH v3 07/28] IB/Verbs: Reform IB-ulp ipoib

[Jason Gunthorpe]

On Mon, Apr 13, 2015 at 03:46:03PM -0400, ira.weiny wrote:

> > This doesn't quite look right, it should be 'goto error1'
> 
> Looks like you replied to the wrong patch.  ??  I don't see error1 in ipoib_add_one.

> For the ib_cm module...

Right, sorry.
 
> Yes I think it should go to "error1".  However, see below...
> 
> This is the type of clean up error which would be avoided if a call to
> cap_ib_cm_dev() were done at the top of the function.

So what does cap_ib_cm_dev return in your example:

> Dev
> 	port 1 : cap_is_cm == true
> 	port 2 : cap_is_cm == false
> 	port 3 : cap_is_cm == true

True? Then the code is still broken, having cap_ib_cm_dev doesn't help
anything.

If we make it possible to be per port then it has to be fixed.

If you want to argue the above example is illegal and port 2 has to be
on a different device, I'd be interested to see what that looks like.

Thinking about it some more, cap_foo_dev only makes sense if all ports
are either true or false. Mixed is a BUG.

That seems reasonable, and solves the #10 problem, but we should
enforce this invariant during device register.

Typically the ports seem to be completely orthogonal (like SA), so in those
cases the _dev and restriction makes no sense.

CM seems to be different, so it should probably enforce its rules

Jason
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/