Re: [PATCH v2] providers/rxe: Set the correct value of resid for inline data

[Bob Pearson <rpearsonhpe@gmail.com>]

On 9/2/21 3:27 AM, yangx.jy@fujitsu.com wrote:
> Hi Yanjun, Bob
> 
> Ping. :-)
> 
> Best Regards,
> Xiao Yang
> On 2021/8/25 17:44, Leon Romanovsky wrote:
>> On Mon, Aug 23, 2021 at 01:33:24AM +0000, yangx.jy@fujitsu.com wrote:
>>> Hi Leon,
>>>
>>> Could you review the patch?
>> There is no need, I trust to Zhu's and Bob's review.
>>
>> Thanks
>>
>>> Best Regards,
>>> Xiao Yang
>>> On 2021/8/17 2:52, Bob Pearson wrote:
>>>> On 8/15/21 10:55 PM, Zhu Yanjun wrote:
>>>>> On Sat, Aug 14, 2021 at 6:11 AM Bob Pearson<rpearsonhpe@gmail.com>   wrote:
>>>>>> On 8/9/21 10:07 AM, Xiao Yang wrote:
>>>>>>> Resid indicates the residual length of transmitted bytes but current
>>>>>>> rxe sets it to zero for inline data at the beginning.  In this case,
>>>>>>> request will transmit zero byte to responder wrongly.
>>>>>>>
>>>>>>> Resid should be set to the total length of transmitted bytes at the
>>>>>>> beginning.
>>>>>>>
>>>>>>> Note:
>>>>>>> Just remove the useless setting of resid in init_send_wqe().
>>>>>>>
>>>>>>> Fixes: 1a894ca10105 ("Providers/rxe: Implement ibv_create_qp_ex verb")
>>>>>>> Fixes: 8337db5df125 ("Providers/rxe: Implement memory windows")
>>>>>>> Signed-off-by: Xiao Yang<yangx.jy@fujitsu.com>
>>>>>>> ---
>>>>>>>    providers/rxe/rxe.c | 5 ++---
>>>>>>>    1 file changed, 2 insertions(+), 3 deletions(-)
>>>>>>>
>>>>>>> diff --git a/providers/rxe/rxe.c b/providers/rxe/rxe.c
>>>>>>> index 3c3ea8bb..3533a325 100644
>>>>>>> --- a/providers/rxe/rxe.c
>>>>>>> +++ b/providers/rxe/rxe.c
>>>>>>> @@ -1004,7 +1004,7 @@ static void wr_set_inline_data(struct ibv_qp_ex *ibqp, void *addr,
>>>>>>>
>>>>>>>         memcpy(wqe->dma.inline_data, addr, length);
>>>>>>>         wqe->dma.length = length;
>>>>>>> -     wqe->dma.resid = 0;
>>>>>>> +     wqe->dma.resid = length;
>>>>>>>    }
>>>>>>>
>>>>>>>    static void wr_set_inline_data_list(struct ibv_qp_ex *ibqp, size_t num_buf,
>>>>>>> @@ -1035,6 +1035,7 @@ static void wr_set_inline_data_list(struct ibv_qp_ex *ibqp, size_t num_buf,
>>>>>>>         }
>>>>>>>
>>>>>>>         wqe->dma.length = tot_length;
>>>>>>> +     wqe->dma.resid = tot_length;
>>>>>>>    }
>>>>>>>
>>>>>>>    static void wr_set_sge(struct ibv_qp_ex *ibqp, uint32_t lkey, uint64_t addr,
>>>>>>> @@ -1473,8 +1474,6 @@ static int init_send_wqe(struct rxe_qp *qp, struct rxe_wq *sq,
>>>>>>>         if (ibwr->send_flags&   IBV_SEND_INLINE) {
>>>>>>>                 uint8_t *inline_data = wqe->dma.inline_data;
>>>>>>>
>>>>>>> -             wqe->dma.resid = 0;
>>>>>>> -
>>>>>>>                 for (i = 0; i<   num_sge; i++) {
>>>>>>>                         memcpy(inline_data,
>>>>>>>                                (uint8_t *)(long)ibwr->sg_list[i].addr,
>>>>>>>
>>>>>> Signed-off-by: Bob Pearson<rpearsonhpe@gmail.com>
>>>>> The Signed-off-by: tag indicates that the signer was involved in the
>>>>> development of the patch, or that he/she was in the patch’s delivery
>>>>> path.
>>>>>
>>>>> Zhu Yanjun
>>>>>
>>>> Sorry, my misunderstanding. Then I want to say
>>>>
>>>> Reviewed-by: Bob Pearson<rpearsonhpe@gmail.com>
>>>>
>>>> The patch looks correct to me.

Hi,

What are you looking for? I reviewed the patch (above) and agree with you that it makes sense.
But it's not up to me to accept it. That would be Jason or Zhu.

BTW until this is fixed I it looks like inline WRs are broken for the extended QP API.

Bob