Re: [PATCH] Make rxe driver to calculate correct byte_len on receiving side when work completion is generated with IB_WC_RECV_RDMA_WITH_IMM opcode.

[Yanjun Zhu <yanjun.zhu@oracle.com>]

On 2019/7/8 9:46, Konstantin Taranov wrote:
> On Mon, 8 Jul 2019 07:35:24 +0800
> Zhu Yanjun <yanjun.zhu@oracle.com> wrote:
>
>> 在 2019/7/8 5:23, Konstantin Taranov 写道:
>>> On Wed, 3 Jul 2019 09:24:54 +0800
>>> Yanjun Zhu <yanjun.zhu@oracle.com> wrote:
>>>   
>>>> On 2019/6/27 22:06, Konstantin Taranov wrote:
>>>>> Make softRoce to calculate correct byte_len on receiving side when work completion
>>>>> is generated with IB_WC_RECV_RDMA_WITH_IMM opcode.
>>>>>
>>>>> According to documentation byte_len must indicate the number of written
>>>>> bytes, whereas it was always equal to zero for IB_WC_RECV_RDMA_WITH_IMM opcode.
>>>> With roce NIC, what is the byte_len? Thanks a lot.
>>> byte_len is a field of a work completion (struct ib_uverbs_wc or struct ibv_wc). It is defined in verbs and stores
>>> the number of written bytes to the destination memory. In case of IB_WC_RECV_RDMA_WITH_IMM
>>> completion event, the field byte_len must store the number of written bytes for incoming
>>> RDMA_WRITE_WITH_IMM request.
>> Cool. Thanks for your explanations.
>>
>> The above is the test result of physical RoCE NIC?
>>
> Yes. When I use physical nics, the byte_len indicates the number of received bytes.
> It is also fully complies with what is written in https://www.rdmamojo.com/2013/02/15/ibv_poll_cq/ about the byte_len field.

Nice. I am fine with this patch.

Thanks a lot.

Zhu Yanjun

>   
>
>
>> Thanks.
>>
>> Zhu Yanjun
>>
>>>   
>>>> Zhu Yanjun
>>>>   
>>>>> The patch proposes to remember the length of an RDMA request from the RETH header, and use it
>>>>> as byte_len when the work completion with IB_WC_RECV_RDMA_WITH_IMM opcode is generated.
>>>>>
>>>>> Signed-off-by: Konstantin Taranov <konstantin.taranov@inf.ethz.ch>
>>>>> ---
>>>>>     drivers/infiniband/sw/rxe/rxe_resp.c  | 5 ++++-
>>>>>     drivers/infiniband/sw/rxe/rxe_verbs.h | 1 +
>>>>>     2 files changed, 5 insertions(+), 1 deletion(-)
>>>>>
>>>>> diff --git a/drivers/infiniband/sw/rxe/rxe_resp.c b/drivers/infiniband/sw/rxe/rxe_resp.c
>>>>> index aca9f60f9b21..1cbfbd98eb22 100644
>>>>> --- a/drivers/infiniband/sw/rxe/rxe_resp.c
>>>>> +++ b/drivers/infiniband/sw/rxe/rxe_resp.c
>>>>> @@ -431,6 +431,7 @@ static enum resp_states check_rkey(struct rxe_qp *qp,
>>>>>     			qp->resp.va = reth_va(pkt);
>>>>>     			qp->resp.rkey = reth_rkey(pkt);
>>>>>     			qp->resp.resid = reth_len(pkt);
>>>>> +			qp->resp.length = reth_len(pkt);
>>>>>     		}
>>>>>     		access = (pkt->mask & RXE_READ_MASK) ? IB_ACCESS_REMOTE_READ
>>>>>     						     : IB_ACCESS_REMOTE_WRITE;
>>>>> @@ -856,7 +857,9 @@ static enum resp_states do_complete(struct rxe_qp *qp,
>>>>>     				pkt->mask & RXE_WRITE_MASK) ?
>>>>>     					IB_WC_RECV_RDMA_WITH_IMM : IB_WC_RECV;
>>>>>     		wc->vendor_err = 0;
>>>>> -		wc->byte_len = wqe->dma.length - wqe->dma.resid;
>>>>> +		wc->byte_len = (pkt->mask & RXE_IMMDT_MASK &&
>>>>> +				pkt->mask & RXE_WRITE_MASK) ?
>>>>> +					qp->resp.length : wqe->dma.length - wqe->dma.resid;
>>>>>     
>>>>>     		/* fields after byte_len are different between kernel and user
>>>>>     		 * space
>>>>> diff --git a/drivers/infiniband/sw/rxe/rxe_verbs.h b/drivers/infiniband/sw/rxe/rxe_verbs.h
>>>>> index e8be7f44e3be..28bfb3ece104 100644
>>>>> --- a/drivers/infiniband/sw/rxe/rxe_verbs.h
>>>>> +++ b/drivers/infiniband/sw/rxe/rxe_verbs.h
>>>>> @@ -213,6 +213,7 @@ struct rxe_resp_info {
>>>>>     	struct rxe_mem		*mr;
>>>>>     	u32			resid;
>>>>>     	u32			rkey;
>>>>> +	u32			length;
>>>>>     	u64			atomic_orig;
>>>>>     
>>>>>     	/* SRQ only */