[PATCH] RDMA/rxe: Check the last packet by RXE_END_MASK

[PATCH] RDMA/rxe: Check the last packet by RXE_END_MASK

[Xiao Yang]

It's wrong to check the last packet by RXE_COMP_MASK because the flag
is to indicate if responder needs to generate a completion.

Fixes: 9fcd67d1772c ("IB/rxe: increment msn only when completing a request")
Fixes: 8700e3e7c485 ("Soft RoCE driver")
Signed-off-by: Xiao Yang <yangx.jy@fujitsu.com>
---
 drivers/infiniband/sw/rxe/rxe_resp.c | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/drivers/infiniband/sw/rxe/rxe_resp.c b/drivers/infiniband/sw/rxe/rxe_resp.c
index e8f435fa6e4d..380934e38923 100644
--- a/drivers/infiniband/sw/rxe/rxe_resp.c
+++ b/drivers/infiniband/sw/rxe/rxe_resp.c
@@ -814,6 +814,10 @@ static enum resp_states execute(struct rxe_qp *qp, struct rxe_pkt_info *pkt)
 			return RESPST_ERR_INVALIDATE_RKEY;
 	}
 
+	if (pkt->mask & RXE_END_MASK)
+		/* We successfully processed this new request. */
+		qp->resp.msn++;
+
 	/* next expected psn, read handles this separately */
 	qp->resp.psn = (pkt->psn + 1) & BTH_PSN_MASK;
 	qp->resp.ack_psn = qp->resp.psn;
@@ -821,11 +825,9 @@ static enum resp_states execute(struct rxe_qp *qp, struct rxe_pkt_info *pkt)
 	qp->resp.opcode = pkt->opcode;
 	qp->resp.status = IB_WC_SUCCESS;
 
-	if (pkt->mask & RXE_COMP_MASK) {
-		/* We successfully processed this new request. */
-		qp->resp.msn++;
+	if (pkt->mask & RXE_COMP_MASK)
 		return RESPST_COMPLETE;
-	} else if (qp_type(qp) == IB_QPT_RC)
+	else if (qp_type(qp) == IB_QPT_RC)
 		return RESPST_ACKNOWLEDGE;
 	else
 		return RESPST_CLEANUP;
-- 
2.25.1

Re: [PATCH] RDMA/rxe: Check the last packet by RXE_END_MASK

[Jason Gunthorpe]

On Wed, Dec 29, 2021 at 11:44:38AM +0800, Xiao Yang wrote:
> It's wrong to check the last packet by RXE_COMP_MASK because the flag
> is to indicate if responder needs to generate a completion.
> 
> Fixes: 9fcd67d1772c ("IB/rxe: increment msn only when completing a request")
> Fixes: 8700e3e7c485 ("Soft RoCE driver")
> Signed-off-by: Xiao Yang <yangx.jy@fujitsu.com>
> ---
>  drivers/infiniband/sw/rxe/rxe_resp.c | 10 ++++++----
>  1 file changed, 6 insertions(+), 4 deletions(-)

Bob/Zhu is this OK?

> diff --git a/drivers/infiniband/sw/rxe/rxe_resp.c b/drivers/infiniband/sw/rxe/rxe_resp.c
> index e8f435fa6e4d..380934e38923 100644
> --- a/drivers/infiniband/sw/rxe/rxe_resp.c
> +++ b/drivers/infiniband/sw/rxe/rxe_resp.c
> @@ -814,6 +814,10 @@ static enum resp_states execute(struct rxe_qp *qp, struct rxe_pkt_info *pkt)
>  			return RESPST_ERR_INVALIDATE_RKEY;
>  	}
>  
> +	if (pkt->mask & RXE_END_MASK)
> +		/* We successfully processed this new request. */
> +		qp->resp.msn++;
> +
>  	/* next expected psn, read handles this separately */
>  	qp->resp.psn = (pkt->psn + 1) & BTH_PSN_MASK;
>  	qp->resp.ack_psn = qp->resp.psn;
> @@ -821,11 +825,9 @@ static enum resp_states execute(struct rxe_qp *qp, struct rxe_pkt_info *pkt)
>  	qp->resp.opcode = pkt->opcode;
>  	qp->resp.status = IB_WC_SUCCESS;
>  
> -	if (pkt->mask & RXE_COMP_MASK) {
> -		/* We successfully processed this new request. */
> -		qp->resp.msn++;
> +	if (pkt->mask & RXE_COMP_MASK)
>  		return RESPST_COMPLETE;
> -	} else if (qp_type(qp) == IB_QPT_RC)
> +	else if (qp_type(qp) == IB_QPT_RC)
>  		return RESPST_ACKNOWLEDGE;
>  	else
>  		return RESPST_CLEANUP;
> -- 
> 2.25.1
> 
> 
> 

Re: [PATCH] RDMA/rxe: Check the last packet by RXE_END_MASK

[Robert Pearson]

Yes.

On Wed, Jan 5, 2022 at 6:40 PM Jason Gunthorpe <jgg@nvidia.com> wrote:
>
> On Wed, Dec 29, 2021 at 11:44:38AM +0800, Xiao Yang wrote:
> > It's wrong to check the last packet by RXE_COMP_MASK because the flag
> > is to indicate if responder needs to generate a completion.
> >
> > Fixes: 9fcd67d1772c ("IB/rxe: increment msn only when completing a request")
> > Fixes: 8700e3e7c485 ("Soft RoCE driver")
> > Signed-off-by: Xiao Yang <yangx.jy@fujitsu.com>
> > ---
> >  drivers/infiniband/sw/rxe/rxe_resp.c | 10 ++++++----
> >  1 file changed, 6 insertions(+), 4 deletions(-)
>
> Bob/Zhu is this OK?
>
> > diff --git a/drivers/infiniband/sw/rxe/rxe_resp.c b/drivers/infiniband/sw/rxe/rxe_resp.c
> > index e8f435fa6e4d..380934e38923 100644
> > --- a/drivers/infiniband/sw/rxe/rxe_resp.c
> > +++ b/drivers/infiniband/sw/rxe/rxe_resp.c
> > @@ -814,6 +814,10 @@ static enum resp_states execute(struct rxe_qp *qp, struct rxe_pkt_info *pkt)
> >                       return RESPST_ERR_INVALIDATE_RKEY;
> >       }
> >
> > +     if (pkt->mask & RXE_END_MASK)
> > +             /* We successfully processed this new request. */
> > +             qp->resp.msn++;
> > +
> >       /* next expected psn, read handles this separately */
> >       qp->resp.psn = (pkt->psn + 1) & BTH_PSN_MASK;
> >       qp->resp.ack_psn = qp->resp.psn;
> > @@ -821,11 +825,9 @@ static enum resp_states execute(struct rxe_qp *qp, struct rxe_pkt_info *pkt)
> >       qp->resp.opcode = pkt->opcode;
> >       qp->resp.status = IB_WC_SUCCESS;
> >
> > -     if (pkt->mask & RXE_COMP_MASK) {
> > -             /* We successfully processed this new request. */
> > -             qp->resp.msn++;
> > +     if (pkt->mask & RXE_COMP_MASK)
> >               return RESPST_COMPLETE;
> > -     } else if (qp_type(qp) == IB_QPT_RC)
> > +     else if (qp_type(qp) == IB_QPT_RC)
> >               return RESPST_ACKNOWLEDGE;
> >       else
> >               return RESPST_CLEANUP;
> > --
> > 2.25.1
> >
> >
> >

Re: [PATCH] RDMA/rxe: Check the last packet by RXE_END_MASK

[Yanjun Zhu]

在 2022/1/6 8:40, Jason Gunthorpe 写道:
> On Wed, Dec 29, 2021 at 11:44:38AM +0800, Xiao Yang wrote:
>> It's wrong to check the last packet by RXE_COMP_MASK because the flag
>> is to indicate if responder needs to generate a completion.
>>
>> Fixes: 9fcd67d1772c ("IB/rxe: increment msn only when completing a request")
>> Fixes: 8700e3e7c485 ("Soft RoCE driver")
>> Signed-off-by: Xiao Yang <yangx.jy@fujitsu.com>
>> ---
>>   drivers/infiniband/sw/rxe/rxe_resp.c | 10 ++++++----
>>   1 file changed, 6 insertions(+), 4 deletions(-)
> Bob/Zhu is this OK?

Add david.marchand@6wind.com.


 From the following from the commit log of 9fcd67d1772c ("IB/rxe: 
increment msn only when completing a request")

"

     Logically, the requester associates a sequential Send Sequence Number
     (SSN) with each WQE posted to the send queue. The SSN bears a one-
     to-one relationship to the MSN returned by the responder in each re-
     sponse packet. Therefore, when the requester receives a response, 
it in-
     terprets the MSN as representing the SSN of the most recent request
     completed by the responder to determine which send WQE(s) can be
     completed."

"

It seems that it does not mean to check the last packet. It means that 
it receives a MSN response.

Please David Marchand <david.marchand@6wind.com> to check this commit.

Thanks a lot.

Zhu Yanjun

>
>> diff --git a/drivers/infiniband/sw/rxe/rxe_resp.c b/drivers/infiniband/sw/rxe/rxe_resp.c
>> index e8f435fa6e4d..380934e38923 100644
>> --- a/drivers/infiniband/sw/rxe/rxe_resp.c
>> +++ b/drivers/infiniband/sw/rxe/rxe_resp.c
>> @@ -814,6 +814,10 @@ static enum resp_states execute(struct rxe_qp *qp, struct rxe_pkt_info *pkt)
>>   			return RESPST_ERR_INVALIDATE_RKEY;
>>   	}
>>   
>> +	if (pkt->mask & RXE_END_MASK)
>> +		/* We successfully processed this new request. */
>> +		qp->resp.msn++;
>> +
>>   	/* next expected psn, read handles this separately */
>>   	qp->resp.psn = (pkt->psn + 1) & BTH_PSN_MASK;
>>   	qp->resp.ack_psn = qp->resp.psn;
>> @@ -821,11 +825,9 @@ static enum resp_states execute(struct rxe_qp *qp, struct rxe_pkt_info *pkt)
>>   	qp->resp.opcode = pkt->opcode;
>>   	qp->resp.status = IB_WC_SUCCESS;
>>   
>> -	if (pkt->mask & RXE_COMP_MASK) {
>> -		/* We successfully processed this new request. */
>> -		qp->resp.msn++;
>> +	if (pkt->mask & RXE_COMP_MASK)
>>   		return RESPST_COMPLETE;
>> -	} else if (qp_type(qp) == IB_QPT_RC)
>> +	else if (qp_type(qp) == IB_QPT_RC)
>>   		return RESPST_ACKNOWLEDGE;
>>   	else
>>   		return RESPST_CLEANUP;
>> -- 
>> 2.25.1
>>
>>
>>

Re: [PATCH] RDMA/rxe: Check the last packet by RXE_END_MASK

[yangx.jy]

On 2022/1/7 19:49, Yanjun Zhu wrote:
> It seems that it does not mean to check the last packet. It means that 
> it receives a MSN response. 
Hi Yanjun,

Checking the last packet is a way to indicate that responder has 
completed an entire request(including multiple packets) and then 
increases a msn.

Best Regards,
Xiao Yang

Re: [PATCH] RDMA/rxe: Check the last packet by RXE_END_MASK

[Yanjun Zhu]

在 2022/1/10 13:17, yangx.jy@fujitsu.com 写道:
> On 2022/1/7 19:49, Yanjun Zhu wrote:
>> It seems that it does not mean to check the last packet. It means that
>> it receives a MSN response.
> Hi Yanjun,
>
> Checking the last packet is a way to indicate that responder has
> completed an entire request(including multiple packets) and then
> increases a msn.

Hi, Xiao

What does the msn mean?

Thanks a lot.

Zhu Yanjun

>
> Best Regards,
> Xiao Yang

Re: [PATCH] RDMA/rxe: Check the last packet by RXE_END_MASK

[Haakon Bugge]

> On 11 Jan 2022, at 15:42, Yanjun Zhu <yanjun.zhu@linux.dev> wrote:
> 
> 
> 在 2022/1/10 13:17, yangx.jy@fujitsu.com 写道:
>> On 2022/1/7 19:49, Yanjun Zhu wrote:
>>> It seems that it does not mean to check the last packet. It means that
>>> it receives a MSN response.
>> Hi Yanjun,
>> 
>> Checking the last packet is a way to indicate that responder has
>> completed an entire request(including multiple packets) and then
>> increases a msn.
> 
> Hi, Xiao
> 
> What does the msn mean?

Message Sequence Number.


Thxs, Håkon

> 
> Thanks a lot.
> 
> Zhu Yanjun
> 
>> 
>> Best Regards,
>> Xiao Yang

Re: [PATCH] RDMA/rxe: Check the last packet by RXE_END_MASK

[Yanjun Zhu]

在 2022/1/11 23:16, Haakon Bugge 写道:
>
>> On 11 Jan 2022, at 15:42, Yanjun Zhu <yanjun.zhu@linux.dev> wrote:
>>
>>
>> 在 2022/1/10 13:17, yangx.jy@fujitsu.com 写道:
>>> On 2022/1/7 19:49, Yanjun Zhu wrote:
>>>> It seems that it does not mean to check the last packet. It means that
>>>> it receives a MSN response.
>>> Hi Yanjun,
>>>
>>> Checking the last packet is a way to indicate that responder has
>>> completed an entire request(including multiple packets) and then
>>> increases a msn.
>> Hi, Xiao
>>
>> What does the msn mean?
> Message Sequence Number.

Thanks, Haakon

I am reading the following from the spec.

"

C9-148: An HCA responder using Reliable Connection service shall initialize

its MSN value to zero. The responder shall increment its MSN
whenever it has successfully completed processing a new, valid request
message. The MSN shall not be incremented for duplicate requests. The
incremented MSN shall be returned in the last or only packet of an RDMA
READ or Atomic response. For RDMA READ requests, the responder
may increment its MSN after it has completed validating the request and
before it has begun transmitting any of the requested data, and may return
the incremented MSN in the AETH of the first response packet. The MSN
shall be incremented only once for any given request message.

"

It seems that the above describe how to handle MSN increment in details.

Zhu Yanjun

>
>
> Thxs, Håkon
>
>> Thanks a lot.
>>
>> Zhu Yanjun
>>
>>> Best Regards,
>>> Xiao Yang

Re: [PATCH] RDMA/rxe: Check the last packet by RXE_END_MASK

[yangx.jy]

On 2022/1/12 9:19, Yanjun Zhu wrote:
>
> 在 2022/1/11 23:16, Haakon Bugge 写道:
>>
>>> On 11 Jan 2022, at 15:42, Yanjun Zhu <yanjun.zhu@linux.dev> wrote:
>>>
>>>
>>> 在 2022/1/10 13:17, yangx.jy@fujitsu.com 写道:
>>>> On 2022/1/7 19:49, Yanjun Zhu wrote:
>>>>> It seems that it does not mean to check the last packet. It means 
>>>>> that
>>>>> it receives a MSN response.
>>>> Hi Yanjun,
>>>>
>>>> Checking the last packet is a way to indicate that responder has
>>>> completed an entire request(including multiple packets) and then
>>>> increases a msn.
>>> Hi, Xiao
>>>
>>> What does the msn mean?
>> Message Sequence Number.
>
> Thanks, Haakon
>
> I am reading the following from the spec.
>
> "
>
> C9-148: An HCA responder using Reliable Connection service shall 
> initialize
>
> its MSN value to zero. The responder shall increment its MSN
> whenever it has successfully completed processing a new, valid request
> message. The MSN shall not be incremented for duplicate requests. The
> incremented MSN shall be returned in the last or only packet of an RDMA
> READ or Atomic response. For RDMA READ requests, the responder
> may increment its MSN after it has completed validating the request and
> before it has begun transmitting any of the requested data, and may 
> return
> the incremented MSN in the AETH of the first response packet. The MSN
> shall be incremented only once for any given request message.
>
> "
>
> It seems that the above describe how to handle MSN increment in details.
Hi Yanjun,

Sorry for the late reply.

Right, 9.7.7.1 GENERATING MSN VALUE section explains Message Sequence 
Number(MSN).

Best Regards,
Xiao Yang
>
> Zhu Yanjun
>
>>
>>
>> Thxs, Håkon
>>
>>> Thanks a lot.
>>>
>>> Zhu Yanjun
>>>
>>>> Best Regards,
>>>> Xiao Yang

Re: [PATCH] RDMA/rxe: Check the last packet by RXE_END_MASK

[Yanjun Zhu]

在 2022/1/12 11:58, yangx.jy@fujitsu.com 写道:
> On 2022/1/12 9:19, Yanjun Zhu wrote:
>> 在 2022/1/11 23:16, Haakon Bugge 写道:
>>>> On 11 Jan 2022, at 15:42, Yanjun Zhu <yanjun.zhu@linux.dev> wrote:
>>>>
>>>>
>>>> 在 2022/1/10 13:17, yangx.jy@fujitsu.com 写道:
>>>>> On 2022/1/7 19:49, Yanjun Zhu wrote:
>>>>>> It seems that it does not mean to check the last packet. It means
>>>>>> that
>>>>>> it receives a MSN response.
>>>>> Hi Yanjun,
>>>>>
>>>>> Checking the last packet is a way to indicate that responder has
>>>>> completed an entire request(including multiple packets) and then
>>>>> increases a msn.
>>>> Hi, Xiao
>>>>
>>>> What does the msn mean?
>>> Message Sequence Number.
>> Thanks, Haakon
>>
>> I am reading the following from the spec.
>>
>> "
>>
>> C9-148: An HCA responder using Reliable Connection service shall
>> initialize
>>
>> its MSN value to zero. The responder shall increment its MSN
>> whenever it has successfully completed processing a new, valid request
>> message. The MSN shall not be incremented for duplicate requests. The
>> incremented MSN shall be returned in the last or only packet of an RDMA
>> READ or Atomic response. For RDMA READ requests, the responder
>> may increment its MSN after it has completed validating the request and
>> before it has begun transmitting any of the requested data, and may
>> return
>> the incremented MSN in the AETH of the first response packet. The MSN
>> shall be incremented only once for any given request message.
>>
>> "
>>
>> It seems that the above describe how to handle MSN increment in details.
> Hi Yanjun,
>
> Sorry for the late reply.
>
> Right, 9.7.7.1 GENERATING MSN VALUE section explains Message Sequence

Does your commit take into account of duplicate requests?

Zhu Yanjun

> Number(MSN).
>
> Best Regards,
> Xiao Yang
>> Zhu Yanjun
>>
>>>
>>> Thxs, Håkon
>>>
>>>> Thanks a lot.
>>>>
>>>> Zhu Yanjun
>>>>
>>>>> Best Regards,
>>>>> Xiao Yang

Re: [PATCH] RDMA/rxe: Check the last packet by RXE_END_MASK

[yangx.jy]

On 2022/1/15 11:38, Yanjun Zhu wrote:
>
> 在 2022/1/12 11:58, yangx.jy@fujitsu.com 写道:
>> On 2022/1/12 9:19, Yanjun Zhu wrote:
>>> 在 2022/1/11 23:16, Haakon Bugge 写道:
>>>>> On 11 Jan 2022, at 15:42, Yanjun Zhu <yanjun.zhu@linux.dev> wrote:
>>>>>
>>>>>
>>>>> 在 2022/1/10 13:17, yangx.jy@fujitsu.com 写道:
>>>>>> On 2022/1/7 19:49, Yanjun Zhu wrote:
>>>>>>> It seems that it does not mean to check the last packet. It means
>>>>>>> that
>>>>>>> it receives a MSN response.
>>>>>> Hi Yanjun,
>>>>>>
>>>>>> Checking the last packet is a way to indicate that responder has
>>>>>> completed an entire request(including multiple packets) and then
>>>>>> increases a msn.
>>>>> Hi, Xiao
>>>>>
>>>>> What does the msn mean?
>>>> Message Sequence Number.
>>> Thanks, Haakon
>>>
>>> I am reading the following from the spec.
>>>
>>> "
>>>
>>> C9-148: An HCA responder using Reliable Connection service shall
>>> initialize
>>>
>>> its MSN value to zero. The responder shall increment its MSN
>>> whenever it has successfully completed processing a new, valid request
>>> message. The MSN shall not be incremented for duplicate requests. The
>>> incremented MSN shall be returned in the last or only packet of an RDMA
>>> READ or Atomic response. For RDMA READ requests, the responder
>>> may increment its MSN after it has completed validating the request and
>>> before it has begun transmitting any of the requested data, and may
>>> return
>>> the incremented MSN in the AETH of the first response packet. The MSN
>>> shall be incremented only once for any given request message.
>>>
>>> "
>>>
>>> It seems that the above describe how to handle MSN increment in 
>>> details.
>> Hi Yanjun,
>>
>> Sorry for the late reply.
>>
>> Right, 9.7.7.1 GENERATING MSN VALUE section explains Message Sequence
>
> Does your commit take into account of duplicate requests?
Hi Yanjun,

Responder will check duplicate requests by check_psn() and process them 
by duplicate_request().
According to the logic of duplicate_request(), responder doesn't 
increase msn for duplicate requests.

Best Regards,
Xiao Yang
>
> Zhu Yanjun
>
>> Number(MSN).
>>
>> Best Regards,
>> Xiao Yang
>>> Zhu Yanjun
>>>
>>>>
>>>> Thxs, Håkon
>>>>
>>>>> Thanks a lot.
>>>>>
>>>>> Zhu Yanjun
>>>>>
>>>>>> Best Regards,
>>>>>> Xiao Yang

Re: [PATCH] RDMA/rxe: Check the last packet by RXE_END_MASK

[Yanjun Zhu]

在 2022/1/18 10:20, yangx.jy@fujitsu.com 写道:
> On 2022/1/15 11:38, Yanjun Zhu wrote:
>> 在 2022/1/12 11:58, yangx.jy@fujitsu.com 写道:
>>> On 2022/1/12 9:19, Yanjun Zhu wrote:
>>>> 在 2022/1/11 23:16, Haakon Bugge 写道:
>>>>>> On 11 Jan 2022, at 15:42, Yanjun Zhu <yanjun.zhu@linux.dev> wrote:
>>>>>>
>>>>>>
>>>>>> 在 2022/1/10 13:17, yangx.jy@fujitsu.com 写道:
>>>>>>> On 2022/1/7 19:49, Yanjun Zhu wrote:
>>>>>>>> It seems that it does not mean to check the last packet. It means
>>>>>>>> that
>>>>>>>> it receives a MSN response.
>>>>>>> Hi Yanjun,
>>>>>>>
>>>>>>> Checking the last packet is a way to indicate that responder has
>>>>>>> completed an entire request(including multiple packets) and then
>>>>>>> increases a msn.
>>>>>> Hi, Xiao
>>>>>>
>>>>>> What does the msn mean?
>>>>> Message Sequence Number.
>>>> Thanks, Haakon
>>>>
>>>> I am reading the following from the spec.
>>>>
>>>> "
>>>>
>>>> C9-148: An HCA responder using Reliable Connection service shall
>>>> initialize
>>>>
>>>> its MSN value to zero. The responder shall increment its MSN
>>>> whenever it has successfully completed processing a new, valid request
>>>> message. The MSN shall not be incremented for duplicate requests. The
>>>> incremented MSN shall be returned in the last or only packet of an RDMA
>>>> READ or Atomic response. For RDMA READ requests, the responder
>>>> may increment its MSN after it has completed validating the request and
>>>> before it has begun transmitting any of the requested data, and may
>>>> return
>>>> the incremented MSN in the AETH of the first response packet. The MSN
>>>> shall be incremented only once for any given request message.
>>>>
>>>> "
>>>>
>>>> It seems that the above describe how to handle MSN increment in
>>>> details.
>>> Hi Yanjun,
>>>
>>> Sorry for the late reply.
>>>
>>> Right, 9.7.7.1 GENERATING MSN VALUE section explains Message Sequence

"

...

Since the responder may choose to coalesce acknowledges, a single 
response packet may in fact acknowledge
several request messages. Thus, when it receives a new MSN, the 
requester begins evaluating WQEs on its send queue beginning with the
oldest outstanding WQE and progressing forward.

...

"

In the above, several request messages come. From the SPEC, msn should 
increase based on the number of request messages.

Can your commit handle the above case?


Zhu Yanjun

>> Does your commit take into account of duplicate requests?
> Hi Yanjun,
>
> Responder will check duplicate requests by check_psn() and process them
> by duplicate_request().
> According to the logic of duplicate_request(), responder doesn't
> increase msn for duplicate requests.
>
> Best Regards,
> Xiao Yang
>> Zhu Yanjun
>>
>>> Number(MSN).
>>>
>>> Best Regards,
>>> Xiao Yang
>>>> Zhu Yanjun
>>>>
>>>>> Thxs, Håkon
>>>>>
>>>>>> Thanks a lot.
>>>>>>
>>>>>> Zhu Yanjun
>>>>>>
>>>>>>> Best Regards,
>>>>>>> Xiao Yang

Re: [PATCH] RDMA/rxe: Check the last packet by RXE_END_MASK

[yangx.jy]

On 2022/1/19 22:08, Yanjun Zhu wrote:
> "
>
> ...
>
> Since the responder may choose to coalesce acknowledges, a single 
> response packet may in fact acknowledge
> several request messages. Thus, when it receives a new MSN, the 
> requester begins evaluating WQEs on its send queue beginning with the
> oldest outstanding WQE and progressing forward.
>
> ...
>
> "
>
> In the above, several request messages come. From the SPEC, msn should 
> increase based on the number of request messages.
Hi Yanjun，

Current logic shows that posting a WQE on SQ increases SSN (SSN++) and 
processing a WQE on responder successfully increases MSN (MSN++).
I think current SoftRoce doesn't implement the rule you metioned.

To be honest, the rule is not clear for me. when and how many 
acknowledges of several request messages can we coalesce?
>
> Can your commit handle the above case?
No.

Best Regards,
Xiao Yang
>
>
> Zhu Yanjun

Re: [PATCH] RDMA/rxe: Check the last packet by RXE_END_MASK

[Yanjun Zhu]

在 2022/1/20 17:33, yangx.jy@fujitsu.com 写道:
> On 2022/1/19 22:08, Yanjun Zhu wrote:
>> "
>>
>> ...
>>
>> Since the responder may choose to coalesce acknowledges, a single
>> response packet may in fact acknowledge
>> several request messages. Thus, when it receives a new MSN, the
>> requester begins evaluating WQEs on its send queue beginning with the
>> oldest outstanding WQE and progressing forward.
>>
>> ...
>>
>> "
>>
>> In the above, several request messages come. From the SPEC, msn should
>> increase based on the number of request messages.
> Hi Yanjun，
>
> Current logic shows that posting a WQE on SQ increases SSN (SSN++) and
> processing a WQE on responder successfully increases MSN (MSN++).
> I think current SoftRoce doesn't implement the rule you metioned.
>
> To be honest, the rule is not clear for me. when and how many
> acknowledges of several request messages can we coalesce?

Hi, Jason Gunthorpe && Leon Romanovsky

In Mellanox RDMA NIC, how this rule is implemented?

Thanks a lot.

Zhu Yanjun

>> Can your commit handle the above case?
> No.
>
> Best Regards,
> Xiao Yang
>>
>> Zhu Yanjun

Re: [PATCH] RDMA/rxe: Check the last packet by RXE_END_MASK

[Jason Gunthorpe]

On Wed, Dec 29, 2021 at 11:44:38AM +0800, Xiao Yang wrote:
> It's wrong to check the last packet by RXE_COMP_MASK because the flag
> is to indicate if responder needs to generate a completion.
> 
> Fixes: 9fcd67d1772c ("IB/rxe: increment msn only when completing a request")
> Fixes: 8700e3e7c485 ("Soft RoCE driver")
> Signed-off-by: Xiao Yang <yangx.jy@fujitsu.com>
> ---
>  drivers/infiniband/sw/rxe/rxe_resp.c | 10 ++++++----
>  1 file changed, 6 insertions(+), 4 deletions(-)

Applied to for-next, thanks

Jason