[ofa-general] [PATCH 6/10 v1] IB/mlx4: Add LSO support

[Eli Cohen]

On Wed, 2008-04-02 at 08:26 -0700, Roland Dreier wrote:
> Not sure I follow.  Given that we have
> 
> struct mlx4_lso_seg {
> 	__be32			mss_hdr_size;
> 	__be32			header[0];
> };
> 
> I don't see much difference between my proposal
> 
> > 	halign = ALIGN(wr->wr.ud.hlen + sizeof *wqe, 16);
> 
> and yours
> 
> >	halign = ALIGN(wr->wr.ud.hlen + 4, 16);
> 
> since isn't sizeof *wqe == 4?

Right, I missed that.

> 
>  > I don't think so, at least in the case that hlen equals 48 which is a
>  > valid one since the total length used by the LSO segment would be 48 + 4
>  > which requires 4 * 16 bytes chunks. If we'd use the above statement the
>  > send would fail.
> 
> But the point is that the current code would only bump the wqe pointer
> by 48 bytes and the last 4 bytes of the header would be overwritten by
> the next data segment.
> 

Given the fact that sizeof *wqe == 4 then what you propose seems to be a
correct approach. But I do think that this is equivalent but looks
cleaner:

-       unsigned halign = ALIGN(wr->wr.ud.hlen, 16);
+       unsigned halign = ALIGN(wr->wr.ud.hlen + sizeof *wqe, 16);


-       if (unlikely(wr->wr.ud.hlen) > 60)
+       if (unlikely(halign > 64))
                return -EINVAL;