[ofa-general] Re: [PATCH] ipoib large send offload

[Roland Dreier]

 > Add LSO supprt to ipoib

This is a perfect example of what I mentioned before about splitting
up patches.  This patch should be three: "add LSO support to core,"
"Use LSO in IPoIB" and "implement LSO in mlx4."

 > +	[IB_WR_LSO]			= __constant_cpu_to_be32(MLX4_OPCODE_LSO),

Why a new opcode vs. just a send flag?  Seems like a strange interface
to present to the consumer.

 > +
 > +			if (wr->opcode == IB_WR_LSO) {
 > +				int halign;
 > +
 > +				memcpy(((struct mlx4_lso_seg *)wqe)->header,
 > +				       wr->wr.ud.header, wr->wr.ud.hlen);
 > +				wmb();
 > +				((struct mlx4_lso_seg *)wqe)->mss_hdr_size =
 > +					cpu_to_be32(((wr->wr.ud.mss - wr->wr.ud.hlen) << 16) |
 > +						    wr->wr.ud.hlen);
 > +
 > +				halign = ALIGN(wr->wr.ud.hlen, 16);
 > +				wqe += halign;
 > +				size += halign >> 4;
 > +
 > +				if (unlikely(wr->num_sge > qp->sq.max_gs - (halign >> 4))) {
 > +					err = -EINVAL;
 > +					*bad_wr = wr;
 > +					goto out;
 > +				}
 > +			}
 > +

Please try to wrap this up into a helper function so the post send
function doesn't get too cluttered.

 > @@ -1365,6 +1389,7 @@ int mlx4_ib_post_send(struct ib_qp *ibqp
 >  		ctrl->owner_opcode = mlx4_ib_opcode[wr->opcode] |
 >  			(ind & qp->sq.wqe_cnt ? cpu_to_be32(1 << 31) : 0);
 >  
 > +
 >  		/*
 >  		 * We can improve latency by not stamping the last
 >  		 * send queue WQE until after ringing the doorbell, so

Please try to avoid extranoeous changes like this.

 > +//		printk("%s: [%d] addr = 0x%llx, size = %d\n", __func__, i, addr, frag->size);

seems like some debugging code escaped into the wild.

 - R.