[PATCH 0/29v2] InfiniBand core update

[PATCH 0/29v2] InfiniBand core update

[Hal Rosenstock]

This is version 2 of a patch series to get the Infiniband core up to
date. Aside from bug fixes, the following new functionality is also
introduced: 

	MAD (Management Datagram) support for RMPP
			(Reliable MultiPacket Protocol)
	CM (Communications Manager) support
	User CM support
	User MAD changes for RMPP
	Service Record support in SA query

Thanks.

-- Hal

Re: [PATCH 0/29v2] InfiniBand core update

[David S. Miller]

NO!!!! No more huge patch bombs to linux-kernel@vger.kernel.org
people!  Condense your changes into more manageable pieces
or only submit smaller groups of changes at a time.

You people doing this are absolutely killing vger.kernel.org.

Every patch you post has to go to 5000+ subscribers, so please
keep this in mind when posting multiple patches.  If you feel
the need to send, say, more than 15 patches at once, reconsider.

Re: [PATCH 0/29v2] InfiniBand core update

[Andrew Morton]

Hal Rosenstock <halr@voltaire.com> wrote:
>
> This is version 2 of a patch series to get the Infiniband core up to
> date.

Well that was interesting.

- All the patches had mangled headers:

-- linux-2.6.13-rc2-mm1-16/...
+++ linux-2.6.13-rc2-mm1-17/...

  instead of

--- linux-2.6.13-rc2-mm1-16/...
+++ linux-2.6.13-rc2-mm1-17/...

  Which I fixed up.

- The second patch didn't apply.  I fixed that too.

- The patches add lots of trailing whitespace.  I habitually trim that
  off, figuring that any downstream merging hassle which that causes is
  just punishment ;)


Please check that it all landed OK in the next -mm.

I've hung on to Tom Duffy's patch pending a test compilation.

I'll tentatively consider this material to be not-for-2.6.13?

Re: [PATCH 0/29v2] InfiniBand core update

[Hal Rosenstock]

On Mon, 2005-07-11 at 20:05, Andrew Morton wrote:
> Hal Rosenstock <halr@voltaire.com> wrote:
> >
> > This is version 2 of a patch series to get the Infiniband core up to
> > date.
> 
> Well that was interesting.
> 
> - All the patches had mangled headers:
> 
> -- linux-2.6.13-rc2-mm1-16/...
> +++ linux-2.6.13-rc2-mm1-17/...
> 
>   instead of
> 
> --- linux-2.6.13-rc2-mm1-16/...
> +++ linux-2.6.13-rc2-mm1-17/...

Not sure how that happened.

>   Which I fixed up.

Thanks.

> - The second patch didn't apply.  I fixed that too.

Not sure how this was broken. Thanks for fixing this.

> - The patches add lots of trailing whitespace.  I habitually trim that
>   off, figuring that any downstream merging hassle which that causes is
>   just punishment ;)

I'll work on merging this back downstream :-(

> Please check that it all landed OK in the next -mm.

Will do.

> I've hung on to Tom Duffy's patch pending a test compilation.
> 
> I'll tentatively consider this material to be not-for-2.6.13?

Presuming that "this material" refers to the patch to add the kernel CM
implementation, if kernel CM does not make 2.6.13, then user CM should
not either as it is dependent on it.

-- Hal

Re: [PATCH 0/29v2] InfiniBand core update

[Andrew Morton]

Hal Rosenstock <halr@voltaire.com> wrote:
>
> > I'll tentatively consider this material to be not-for-2.6.13?
> 
>  Presuming that "this material" refers to the patch to add the kernel CM
>  implementation, if kernel CM does not make 2.6.13, then user CM should
>  not either as it is dependent on it.

Well I was asking.  Do you guys think that this material is appropriate to
and safe enough for 2.6.13?

What are "user CM" and "kernel CM"?

Re: [PATCH 0/29v2] InfiniBand core update

[David S. Miller]

Please acknowledge that you understand how inappropriate
and problem causing your huge patch bomb was today to this
mailing list.

It is nearly 8 hours later, and vger.kernel.org is still
trying mightily to spit out all of your patches to the
5000+ people subscribed to linux-kernel.

There is about ~6 hour posting latency as a result of all
of this.

Re: [PATCH 0/29v2] InfiniBand core update

[Hal Rosenstock]

On Mon, 2005-07-11 at 23:12, David S. Miller wrote:
> Please acknowledge that you understand how inappropriate
> and problem causing your huge patch bomb was today to this
> mailing list.
> 
> It is nearly 8 hours later, and vger.kernel.org is still
> trying mightily to spit out all of your patches to the
> 5000+ people subscribed to linux-kernel.
> 
> There is about ~6 hour posting latency as a result of all
> of this.

I understand the problem it caused. The MAD layer of InfiniBand was 
quite a bit behind and we wanted to catch up. I tried to make them in
smaller manageable pieces and that was why there were so many patches in
the series. In the future, I will endeavor to make the changes smaller.
Sorry for the problems this caused.

-- Hal

Re: [PATCH 0/29v2] InfiniBand core update

[Hal Rosenstock]

On Mon, 2005-07-11 at 23:11, Andrew Morton wrote:
> Well I was asking.  Do you guys think that this material is appropriate to
> and safe enough for 2.6.13?

We think so.

> What are "user CM" and "kernel CM"?

CM is the InfiniBand Communications Manager. It is (primarily)
responsible for setting up and managing connections between endpoints. 
Connections. IB communication occurs over the queue pairs (QPs) set up
by the CM protocol.

kernel CM is CM functionality in the kernel. user CM is to enable
userspace applications to utilize this functionality.

-- Hal

Re: [PATCH 0/29v2] InfiniBand core update

[Hal Rosenstock]

On Mon, 2005-07-11 at 23:11, Andrew Morton wrote:
> Well I was asking.  Do you guys think that this material is appropriate to
> and safe enough for 2.6.13?

I used your versions of the patches (Tom's ucm one is needed and you
added that). I also back ported the trailing whitespace elimination
changes.

I just completed a regression test including uCM, CM, RMPP, OpenSM,
IPoIB and it looks good to me.

I'll check this again when the next -mm comes out.

Thanks.

-- Hal

Re: [PATCH 0/29v2] InfiniBand core update

[Andrew Morton]

Hal Rosenstock <halr@voltaire.com> wrote:
>
> On Mon, 2005-07-11 at 23:11, Andrew Morton wrote:
> > Well I was asking.  Do you guys think that this material is appropriate to
> > and safe enough for 2.6.13?
> 
> I used your versions of the patches (Tom's ucm one is needed and you
> added that). I also back ported the trailing whitespace elimination
> changes.
> 
> I just completed a regression test including uCM, CM, RMPP, OpenSM,
> IPoIB and it looks good to me.
> 

OK, well the timing of a merge is mainly up to you guys, especially as the
subsystem is pretty raw and you're the only people who use it ;)

Two things from a quick scan:

a) In many places the patch does

	if (p)
		kfree(p);

   But kfree(0) is permitted.  The cleanup police will be after you at
   some stage - it'd be best to fix those things up immediately.

b) The patch exports a ton of symbols to non-GPL modules:

+EXPORT_SYMBOL(ib_create_ah_from_wc);
+EXPORT_SYMBOL(ib_modify_mad);
+EXPORT_SYMBOL(ib_create_send_mad);
+EXPORT_SYMBOL(ib_free_send_mad);
+EXPORT_SYMBOL(ib_coalesce_recv_mad);
+EXPORT_SYMBOL(ib_sa_service_rec_query);
+EXPORT_SYMBOL(ib_create_cm_id);
+EXPORT_SYMBOL(ib_destroy_cm_id);
+EXPORT_SYMBOL(ib_cm_listen);
+EXPORT_SYMBOL(ib_send_cm_req);
+EXPORT_SYMBOL(ib_send_cm_rep);
+EXPORT_SYMBOL(ib_send_cm_rtu);
+EXPORT_SYMBOL(ib_send_cm_dreq);
+EXPORT_SYMBOL(ib_send_cm_drep);
+EXPORT_SYMBOL(ib_send_cm_rej);
+EXPORT_SYMBOL(ib_send_cm_mra);
+EXPORT_SYMBOL(ib_send_cm_lap);
+EXPORT_SYMBOL(ib_send_cm_apr);
+EXPORT_SYMBOL(ib_send_cm_sidr_req);
+EXPORT_SYMBOL(ib_send_cm_sidr_rep);
+EXPORT_SYMBOL(ib_cm_establish);
+EXPORT_SYMBOL(ib_cm_init_qp_attr);

   Why?

Re: [PATCH 0/29v2] InfiniBand core update

[Hal Rosenstock]

On Tue, 2005-07-12 at 18:38, Andrew Morton wrote:
> OK, well the timing of a merge is mainly up to you guys, especially as the
> subsystem is pretty raw and you're the only people who use it ;)
> 
> Two things from a quick scan:
> 
> a) In many places the patch does
> 
> 	if (p)
> 		kfree(p);
> 
>    But kfree(0) is permitted.  The cleanup police will be after you at
>    some stage - it'd be best to fix those things up immediately.

I'll/We'll work on eradicating these and pushing a patch for this
upstream.

> b) The patch exports a ton of symbols to non-GPL modules:
> 
> +EXPORT_SYMBOL(ib_create_ah_from_wc);
> +EXPORT_SYMBOL(ib_modify_mad);
> +EXPORT_SYMBOL(ib_create_send_mad);
> +EXPORT_SYMBOL(ib_free_send_mad);
> +EXPORT_SYMBOL(ib_coalesce_recv_mad);
> +EXPORT_SYMBOL(ib_sa_service_rec_query);
> +EXPORT_SYMBOL(ib_create_cm_id);
> +EXPORT_SYMBOL(ib_destroy_cm_id);
> +EXPORT_SYMBOL(ib_cm_listen);
> +EXPORT_SYMBOL(ib_send_cm_req);
> +EXPORT_SYMBOL(ib_send_cm_rep);
> +EXPORT_SYMBOL(ib_send_cm_rtu);
> +EXPORT_SYMBOL(ib_send_cm_dreq);
> +EXPORT_SYMBOL(ib_send_cm_drep);
> +EXPORT_SYMBOL(ib_send_cm_rej);
> +EXPORT_SYMBOL(ib_send_cm_mra);
> +EXPORT_SYMBOL(ib_send_cm_lap);
> +EXPORT_SYMBOL(ib_send_cm_apr);
> +EXPORT_SYMBOL(ib_send_cm_sidr_req);
> +EXPORT_SYMBOL(ib_send_cm_sidr_rep);
> +EXPORT_SYMBOL(ib_cm_establish);
> +EXPORT_SYMBOL(ib_cm_init_qp_attr);
> 
>    Why?

I think that is because OpenIB has a dual GPL/BSD license.

-- Hal