[ofa-general] [PATCH 1/2] IB/ipath - fix UD send with immediate

[Roland Dreier]

 > This fixes a small bug which incorrectly calculated the header size
 > for UD send with immediate and therefore dropped packets.

What's the bug?  It's not clear how this patch fixes anything...

 > @@ -301,8 +301,6 @@ int ipath_make_ud_req(struct ipath_qp *qp)
 >  
 >  	/* header size in 32-bit words LRH+BTH+DETH = (8+12+8)/4. */
 >  	qp->s_hdrwords = 7;
 > -	if (wqe->wr.opcode == IB_WR_SEND_WITH_IMM)
 > -		qp->s_hdrwords++;
 >  	qp->s_cur_size = wqe->length;
 >  	qp->s_cur_sge = &qp->s_sge;
 >  	qp->s_wqe = wqe;
 > @@ -327,6 +325,7 @@ int ipath_make_ud_req(struct ipath_qp *qp)
 >  		ohdr = &qp->s_hdr.u.oth;
 >  	}
 >  	if (wqe->wr.opcode == IB_WR_SEND_WITH_IMM) {
 > +		qp->s_hdrwords++;
 >  		ohdr->u.ud.imm_data = wqe->wr.imm_data;

This looks like it doesn't make any difference, since I don't see any
place qp->s_hdrwords is used in between the old location and the new
location of the increment...

 > +	/*
 > +	 * The opcode is in the low byte when its in network order
 > +	 * (top byte when in host order).
 > +	 */
 > +	opcode = be32_to_cpu(ohdr->bth[0]) >> 24;
 > +	if (qp->ibqp.qp_num > 1 &&
 > +	    opcode == IB_OPCODE_UD_SEND_ONLY_WITH_IMMEDIATE) {
 > +		if (header_in_data) {
 > +			wc.imm_data = *(__be32 *) data;
 > +			data += sizeof(__be32);
 > +		} else
 > +			wc.imm_data = ohdr->u.ud.imm_data;
 > +		wc.wc_flags = IB_WC_WITH_IMM;
 > +		hdrsize += sizeof(u32);
 > +	} else if (opcode == IB_OPCODE_UD_SEND_ONLY) {
 > +		wc.imm_data = 0;
 > +		wc.wc_flags = 0;
 > +	} else {
 > +		dev->n_pkt_drops++;
 > +		goto bail;
 > +	}
 > +
 >  	/* Get the number of bytes the message was padded by. */
 >  	pad = (be32_to_cpu(ohdr->bth[0]) >> 20) & 3;
 >  	if (unlikely(tlen < (hdrsize + pad + 4))) {

I guess the bug is that you need to set hdrsize to the length
including the immediate data before this last test?

And this bug causes problems because MVAPICH uses UD sends with
immediate data??  How bad is the impact of this -- do we really need
it for 2.6.24?

By the way, is there anything in the IB spec that forbids sends with
immediate data on QP1?  I'm wondering why your code treats QP1
differently here.

 - R.