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

[Yanjun Zhu <yanjun.zhu@linux.dev>]

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