[ofa-general] Re: PATCH 1/1 - fix kernel crash in mad.c when IB_MAD_RESULT_(SUCCESS|CONSUMED) returned

[Dave Olson]

On Tue, 20 May 2008, Roland Dreier wrote:

|  >         case IB_MAD_RESULT_SUCCESS | IB_MAD_RESULT_CONSUMED:
|  >                 kmem_cache_free(ib_mad_cache, mad_priv);
|  > -               break;
|  > +               kfree(local);
|  > +               goto out;
| 
| Seems you need to set ret = 1 here?  Otherwise I think ib_post_send_mad
| will continue handling the send even though the packet was supposedly
| consumed.

Yes,  you are right about the fact that it should be set, but apparently
all callers are simply checking for a return value > 0, because the
packet is only sent once (return values > 1 have no defined meaning so
I'm not surprised the callers just check > 0).

Do you want me to resubmit it that way, or do you want to make the
change?

| Also as a side note, I think handle_outgoing_dr_smp() would be clearer
| if rather than having
| 
| out:
| 	return ret;
| 
| and then doing stuff like
| 
| 	ret = -EINVAL;
| 	goto out;
| 
| the code just did "return -EINVAL;"
| 
| Maybe I'll do that cleanup for 2.6.27.

Seems reasonable enough to me.

Dave Olson
dave.olson at qlogic.com