[openib-general] RE: [PATCH 3 of 3] mad: large RMPP support, Round 2

[Jack Morgenstein]

On Tuesday 21 February 2006 20:01, Sean Hefty wrote:
> > I did it this way to avoid using kzalloc for all segment allocations, or
> > kmalloc/memset for all.  This way, only the last segment is "memset"'ed
> > and only in the padding area.
>
> The API states that the buffer returned from ib_create_send_mad will be
> cleared. We either need to clear the buffer, or update the documentation.
>

Either way is OK by me.  Note that ib_create_send_mad does allocate the "base"
MAD with kzalloc.  Its just the RMPP segments that are allocated with kmalloc 
and are not initially cleared.

> > [JPM]: This was done to hide segment implementation details from the
> > user_mad layer. You are correct that we can get rid of the size field,
> > and store it only once.
> > However, I think that the proper place for this value is the
> > "ib_mad_send_wr_private" structure, which also holds the list pointer.
> > How about if we change
> > struct ib_mad_multipacket_seg
> > *ib_mad_get_multipacket_seg(struct ib_mad_send_buf *send_buf, int
> > seg_num)
> >
> > to also return the segment size:
>
> Since the user needs to know the segment size, it makes more sense to me to
> just expose it through ib_mad_send_buf.
>

I just thought that by returning the segment size in the procedure call, we 
preserve the option to easily support multiple segment sizes within a single
RMPP message.  However, if you think that we will never need such an option,
I don't object to exposing the segment size in the ib_mad_send_buf structure.

> > [JPM]:  You are correct.  In fact, the test just before this one is also
> > performed in ib_create_send_mad(), and may also be deleted (this was not
> > part of the patch):
> >
> > 	if (rmpp_active && !agent->rmpp_version) {
> > 		ret = -EINVAL;
> > 		goto err_ah;
> > 	}
>
> I will remove these, and submit a patch for any other changes.  Thanks.
>
> - Sean