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

On Thu, Sep 2, 2021 at 4:27 PM yangx.jy@fujitsu.com
[off-list ref] wrote:

Hi Yanjun, Bob

Ping. :

Sorry. It is late to reply.
My concern is if this will introduce other risks or not.
If you can confirm that this will not introduce other risks,
I am fine with this commit.

Reviewed-by: Zhu Yanjun <zyjzyj2000@gmail.com>

Best Regards,
Xiao Yang
On 2021/8/25 17:44, Leon Romanovsky wrote:

quoted

On Mon, Aug 23, 2021 at 01:33:24AM +0000, yangx.jy@fujitsu.com wrote:

quoted

Hi Leon,

Could you review the patch?

There is no need, I trust to Zhu's and Bob's review.

Thanks

quoted

Best Regards,
Xiao Yang
On 2021/8/17 2:52, Bob Pearson wrote:

quoted

On 8/15/21 10:55 PM, Zhu Yanjun wrote:

quoted

On Sat, Aug 14, 2021 at 6:11 AM Bob Pearson[off-list ref]   wrote:

quoted

On 8/9/21 10:07 AM, Xiao Yang wrote:

quoted

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<redacted>

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<redacted>

The patch looks correct to me.