[PATCH 0/3] IB/rxe: Three small fixes

[PATCH 0/3] IB/rxe: Three small fixes

[Andrew Boyer]

This patch series addresses two issues found during heavy traffic
testing as well as a recently-reported Smatch issue.

Andrew Boyer (3):
  IB/rxe: Use BTH_PSN_MASK when ACKing duplicate sends
  IB/rxe: Drop future atomic/read packets rather than retrying
  IB/rxe: Don't check for null ptr in send()

 drivers/infiniband/sw/rxe/rxe_comp.c | 2 +-
 drivers/infiniband/sw/rxe/rxe_net.c  | 3 +--
 drivers/infiniband/sw/rxe/rxe_resp.c | 3 ++-
 3 files changed, 4 insertions(+), 4 deletions(-)

-- 
1.8.3.1

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 1/3] IB/rxe: Use BTH_PSN_MASK when ACKing duplicate sends

[Andrew Boyer]

Signed-off-by: Andrew Boyer <andrew.boyer-8PEkshWhKlo@public.gmane.org>
---
 drivers/infiniband/sw/rxe/rxe_resp.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/infiniband/sw/rxe/rxe_resp.c b/drivers/infiniband/sw/rxe/rxe_resp.c
index 7a36ec9..3435eff 100644
--- a/drivers/infiniband/sw/rxe/rxe_resp.c
+++ b/drivers/infiniband/sw/rxe/rxe_resp.c
@@ -1070,12 +1070,13 @@ static enum resp_states duplicate_request(struct rxe_qp *qp,
 					  struct rxe_pkt_info *pkt)
 {
 	enum resp_states rc;
+	u32 prev_psn = (qp->resp.psn - 1) & BTH_PSN_MASK;
 
 	if (pkt->mask & RXE_SEND_MASK ||
 	    pkt->mask & RXE_WRITE_MASK) {
 		/* SEND. Ack again and cleanup. C9-105. */
 		if (bth_ack(pkt))
-			send_ack(qp, pkt, AETH_ACK_UNLIMITED, qp->resp.psn - 1);
+			send_ack(qp, pkt, AETH_ACK_UNLIMITED, prev_psn);
 		rc = RESPST_CLEANUP;
 		goto out;
 	} else if (pkt->mask & RXE_READ_MASK) {
-- 
1.8.3.1

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 2/3] IB/rxe: Drop future atomic/read packets rather than retrying

[Andrew Boyer]

If the completer is in the middle of a large read operation, one
lost packet can cause havoc. Going to COMPST_ERROR_RETRY will
cause the requester to resend the request. After that, any packet
from the first attempt still in the receive queue will be
interpreted as an error, restarting the error/retry sequence.
The transfer will quickly exhaust its retries.

This behavior is very noticeable when doing 512KB reads on a
QEMU system configured with 1500B MTU.

Also, a resent request here will prompt the responder on the
other side to immediately start resending, but the resent
packets will get stuck in the already-loaded receive queue and
will never be processed.

Rather than erroring out every time an unexpected future packet
arrives, just drop it. Eventually the retry timer will send a
duplicate request; the completer will be able to make progress since
the queue will start relatively empty.

Signed-off-by: Andrew Boyer <andrew.boyer-8PEkshWhKlo@public.gmane.org>
---
 drivers/infiniband/sw/rxe/rxe_comp.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/infiniband/sw/rxe/rxe_comp.c b/drivers/infiniband/sw/rxe/rxe_comp.c
index cd27cbd..d369f24 100644
--- a/drivers/infiniband/sw/rxe/rxe_comp.c
+++ b/drivers/infiniband/sw/rxe/rxe_comp.c
@@ -224,7 +224,7 @@ static inline enum comp_state check_psn(struct rxe_qp *qp,
 		else
 			return COMPST_DONE;
 	} else if ((diff > 0) && (wqe->mask & WR_ATOMIC_OR_READ_MASK)) {
-		return COMPST_ERROR_RETRY;
+		return COMPST_DONE;
 	} else {
 		return COMPST_CHECK_ACK;
 	}
-- 
1.8.3.1

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 3/3] IB/rxe: Don't check for null ptr in send()

[Andrew Boyer]

pkt->qp was already dereferenced earlier in the function.

Fixes Smatch complaint:
drivers/infiniband/sw/rxe/rxe_net.c:458 send()
	 warn: variable dereferenced before check 'pkt->qp' (see line 441)

Signed-off-by: Andrew Boyer <andrew.boyer-8PEkshWhKlo@public.gmane.org>
---
 drivers/infiniband/sw/rxe/rxe_net.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/infiniband/sw/rxe/rxe_net.c b/drivers/infiniband/sw/rxe/rxe_net.c
index 16967cd..342e781 100644
--- a/drivers/infiniband/sw/rxe/rxe_net.c
+++ b/drivers/infiniband/sw/rxe/rxe_net.c
@@ -455,8 +455,7 @@ static int send(struct rxe_dev *rxe, struct rxe_pkt_info *pkt,
 		return -EAGAIN;
 	}
 
-	if (pkt->qp)
-		atomic_inc(&pkt->qp->skb_out);
+	atomic_inc(&pkt->qp->skb_out);
 	kfree_skb(skb);
 
 	return 0;
-- 
1.8.3.1

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 0/3] IB/rxe: Three small fixes

[Doug Ledford]

[-- Attachment #1.1: Type: text/plain, Size: 782 bytes --]

On 12/22/2016 8:54 AM, Andrew Boyer wrote:
> This patch series addresses two issues found during heavy traffic
> testing as well as a recently-reported Smatch issue.
> 
> Andrew Boyer (3):
>   IB/rxe: Use BTH_PSN_MASK when ACKing duplicate sends
>   IB/rxe: Drop future atomic/read packets rather than retrying
>   IB/rxe: Don't check for null ptr in send()
> 
>  drivers/infiniband/sw/rxe/rxe_comp.c | 2 +-
>  drivers/infiniband/sw/rxe/rxe_net.c  | 3 +--
>  drivers/infiniband/sw/rxe/rxe_resp.c | 3 ++-
>  3 files changed, 4 insertions(+), 4 deletions(-)
> 

Thanks, series applied.

-- 
Doug Ledford <dledford-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
    GPG Key ID: B826A3330E572FDD
    Key fingerprint = AE6B 1BDA 122B 23B4 265B  1274 B826 A333 0E57 2FDD


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 884 bytes --]

Re: [PATCH 0/3] IB/rxe: Three small fixes

[Leon Romanovsky]

[-- Attachment #1: Type: text/plain, Size: 1032 bytes --]

On Thu, Dec 22, 2016 at 11:41:24AM -0500, Doug Ledford wrote:
> On 12/22/2016 8:54 AM, Andrew Boyer wrote:
> > This patch series addresses two issues found during heavy traffic
> > testing as well as a recently-reported Smatch issue.
> >
> > Andrew Boyer (3):
> >   IB/rxe: Use BTH_PSN_MASK when ACKing duplicate sends
> >   IB/rxe: Drop future atomic/read packets rather than retrying
> >   IB/rxe: Don't check for null ptr in send()
> >
> >  drivers/infiniband/sw/rxe/rxe_comp.c | 2 +-
> >  drivers/infiniband/sw/rxe/rxe_net.c  | 3 +--
> >  drivers/infiniband/sw/rxe/rxe_resp.c | 3 ++-
> >  3 files changed, 4 insertions(+), 4 deletions(-)
> >
>
> Thanks, series applied.

Hi Doug,

Thank you for such fast response, but can you please give enough time to
other reviewers/relevant maintainers to take a look on them before
applying?

Thanks

>
> --
> Doug Ledford <dledford-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
>     GPG Key ID: B826A3330E572FDD
>     Key fingerprint = AE6B 1BDA 122B 23B4 265B  1274 B826 A333 0E57 2FDD
>




[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH 0/3] IB/rxe: Three small fixes

[Doug Ledford]

[-- Attachment #1.1: Type: text/plain, Size: 1212 bytes --]

On 12/22/2016 12:18 PM, Leon Romanovsky wrote:
> On Thu, Dec 22, 2016 at 11:41:24AM -0500, Doug Ledford wrote:
>> On 12/22/2016 8:54 AM, Andrew Boyer wrote:
>>> This patch series addresses two issues found during heavy traffic
>>> testing as well as a recently-reported Smatch issue.
>>>
>>> Andrew Boyer (3):
>>>   IB/rxe: Use BTH_PSN_MASK when ACKing duplicate sends
>>>   IB/rxe: Drop future atomic/read packets rather than retrying
>>>   IB/rxe: Don't check for null ptr in send()
>>>
>>>  drivers/infiniband/sw/rxe/rxe_comp.c | 2 +-
>>>  drivers/infiniband/sw/rxe/rxe_net.c  | 3 +--
>>>  drivers/infiniband/sw/rxe/rxe_resp.c | 3 ++-
>>>  3 files changed, 4 insertions(+), 4 deletions(-)
>>>
>>
>> Thanks, series applied.
> 
> Hi Doug,
> 
> Thank you for such fast response, but can you please give enough time to
> other reviewers/relevant maintainers to take a look on them before
> applying?

If the patches look like they need review, I wait for review.  These did
not meet that criteria IMO.


-- 
Doug Ledford <dledford-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
    GPG Key ID: B826A3330E572FDD
    Key fingerprint = AE6B 1BDA 122B 23B4 265B  1274 B826 A333 0E57 2FDD


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 884 bytes --]

Re: [PATCH 0/3] IB/rxe: Three small fixes

[Leon Romanovsky]

[-- Attachment #1: Type: text/plain, Size: 1398 bytes --]

On Thu, Dec 22, 2016 at 01:33:43PM -0500, Doug Ledford wrote:
> On 12/22/2016 12:18 PM, Leon Romanovsky wrote:
> > On Thu, Dec 22, 2016 at 11:41:24AM -0500, Doug Ledford wrote:
> >> On 12/22/2016 8:54 AM, Andrew Boyer wrote:
> >>> This patch series addresses two issues found during heavy traffic
> >>> testing as well as a recently-reported Smatch issue.
> >>>
> >>> Andrew Boyer (3):
> >>>   IB/rxe: Use BTH_PSN_MASK when ACKing duplicate sends
> >>>   IB/rxe: Drop future atomic/read packets rather than retrying
> >>>   IB/rxe: Don't check for null ptr in send()
> >>>
> >>>  drivers/infiniband/sw/rxe/rxe_comp.c | 2 +-
> >>>  drivers/infiniband/sw/rxe/rxe_net.c  | 3 +--
> >>>  drivers/infiniband/sw/rxe/rxe_resp.c | 3 ++-
> >>>  3 files changed, 4 insertions(+), 4 deletions(-)
> >>>
> >>
> >> Thanks, series applied.
> >
> > Hi Doug,
> >
> > Thank you for such fast response, but can you please give enough time to
> > other reviewers/relevant maintainers to take a look on them before
> > applying?
>
> If the patches look like they need review, I wait for review.  These did
> not meet that criteria IMO.

Two of three are trivial, and another one is worth to take an extra time
to review.

Thanks

>
>
> --
> Doug Ledford <dledford-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
>     GPG Key ID: B826A3330E572FDD
>     Key fingerprint = AE6B 1BDA 122B 23B4 265B  1274 B826 A333 0E57 2FDD
>




[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH 0/3] IB/rxe: Three small fixes

[Doug Ledford]

[-- Attachment #1.1: Type: text/plain, Size: 1517 bytes --]

On 12/22/2016 2:05 PM, Leon Romanovsky wrote:
> On Thu, Dec 22, 2016 at 01:33:43PM -0500, Doug Ledford wrote:
>> On 12/22/2016 12:18 PM, Leon Romanovsky wrote:
>>> On Thu, Dec 22, 2016 at 11:41:24AM -0500, Doug Ledford wrote:
>>>> On 12/22/2016 8:54 AM, Andrew Boyer wrote:
>>>>> This patch series addresses two issues found during heavy traffic
>>>>> testing as well as a recently-reported Smatch issue.
>>>>>
>>>>> Andrew Boyer (3):
>>>>>   IB/rxe: Use BTH_PSN_MASK when ACKing duplicate sends
>>>>>   IB/rxe: Drop future atomic/read packets rather than retrying
>>>>>   IB/rxe: Don't check for null ptr in send()
>>>>>
>>>>>  drivers/infiniband/sw/rxe/rxe_comp.c | 2 +-
>>>>>  drivers/infiniband/sw/rxe/rxe_net.c  | 3 +--
>>>>>  drivers/infiniband/sw/rxe/rxe_resp.c | 3 ++-
>>>>>  3 files changed, 4 insertions(+), 4 deletions(-)
>>>>>
>>>>
>>>> Thanks, series applied.
>>>
>>> Hi Doug,
>>>
>>> Thank you for such fast response, but can you please give enough time to
>>> other reviewers/relevant maintainers to take a look on them before
>>> applying?
>>
>> If the patches look like they need review, I wait for review.  These did
>> not meet that criteria IMO.
> 
> Two of three are trivial, and another one is worth to take an extra time
> to review.

Which one did you think was not trivial?


-- 
Doug Ledford <dledford-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
    GPG Key ID: B826A3330E572FDD
    Key fingerprint = AE6B 1BDA 122B 23B4 265B  1274 B826 A333 0E57 2FDD


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 884 bytes --]

Re: [PATCH 0/3] IB/rxe: Three small fixes

[Leon Romanovsky]

[-- Attachment #1: Type: text/plain, Size: 1674 bytes --]

On Thu, Dec 22, 2016 at 02:19:50PM -0500, Doug Ledford wrote:
> On 12/22/2016 2:05 PM, Leon Romanovsky wrote:
> > On Thu, Dec 22, 2016 at 01:33:43PM -0500, Doug Ledford wrote:
> >> On 12/22/2016 12:18 PM, Leon Romanovsky wrote:
> >>> On Thu, Dec 22, 2016 at 11:41:24AM -0500, Doug Ledford wrote:
> >>>> On 12/22/2016 8:54 AM, Andrew Boyer wrote:
> >>>>> This patch series addresses two issues found during heavy traffic
> >>>>> testing as well as a recently-reported Smatch issue.
> >>>>>
> >>>>> Andrew Boyer (3):
> >>>>>   IB/rxe: Use BTH_PSN_MASK when ACKing duplicate sends
> >>>>>   IB/rxe: Drop future atomic/read packets rather than retrying
> >>>>>   IB/rxe: Don't check for null ptr in send()
> >>>>>
> >>>>>  drivers/infiniband/sw/rxe/rxe_comp.c | 2 +-
> >>>>>  drivers/infiniband/sw/rxe/rxe_net.c  | 3 +--
> >>>>>  drivers/infiniband/sw/rxe/rxe_resp.c | 3 ++-
> >>>>>  3 files changed, 4 insertions(+), 4 deletions(-)
> >>>>>
> >>>>
> >>>> Thanks, series applied.
> >>>
> >>> Hi Doug,
> >>>
> >>> Thank you for such fast response, but can you please give enough time to
> >>> other reviewers/relevant maintainers to take a look on them before
> >>> applying?
> >>
> >> If the patches look like they need review, I wait for review.  These did
> >> not meet that criteria IMO.
> >
> > Two of three are trivial, and another one is worth to take an extra time
> > to review.
>
> Which one did you think was not trivial?

Personaly for me, it was second patch from the series.

>
>
> --
> Doug Ledford <dledford-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
>     GPG Key ID: B826A3330E572FDD
>     Key fingerprint = AE6B 1BDA 122B 23B4 265B  1274 B826 A333 0E57 2FDD
>




[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH 0/3] IB/rxe: Three small fixes

[Doug Ledford]

[-- Attachment #1.1: Type: text/plain, Size: 1991 bytes --]

On 12/22/2016 3:23 PM, Leon Romanovsky wrote:
> On Thu, Dec 22, 2016 at 02:19:50PM -0500, Doug Ledford wrote:
>> On 12/22/2016 2:05 PM, Leon Romanovsky wrote:
>>> On Thu, Dec 22, 2016 at 01:33:43PM -0500, Doug Ledford wrote:
>>>> On 12/22/2016 12:18 PM, Leon Romanovsky wrote:
>>>>> On Thu, Dec 22, 2016 at 11:41:24AM -0500, Doug Ledford wrote:
>>>>>> On 12/22/2016 8:54 AM, Andrew Boyer wrote:
>>>>>>> This patch series addresses two issues found during heavy traffic
>>>>>>> testing as well as a recently-reported Smatch issue.
>>>>>>>
>>>>>>> Andrew Boyer (3):
>>>>>>>   IB/rxe: Use BTH_PSN_MASK when ACKing duplicate sends
>>>>>>>   IB/rxe: Drop future atomic/read packets rather than retrying
>>>>>>>   IB/rxe: Don't check for null ptr in send()
>>>>>>>
>>>>>>>  drivers/infiniband/sw/rxe/rxe_comp.c | 2 +-
>>>>>>>  drivers/infiniband/sw/rxe/rxe_net.c  | 3 +--
>>>>>>>  drivers/infiniband/sw/rxe/rxe_resp.c | 3 ++-
>>>>>>>  3 files changed, 4 insertions(+), 4 deletions(-)
>>>>>>>
>>>>>>
>>>>>> Thanks, series applied.
>>>>>
>>>>> Hi Doug,
>>>>>
>>>>> Thank you for such fast response, but can you please give enough time to
>>>>> other reviewers/relevant maintainers to take a look on them before
>>>>> applying?
>>>>
>>>> If the patches look like they need review, I wait for review.  These did
>>>> not meet that criteria IMO.
>>>
>>> Two of three are trivial, and another one is worth to take an extra time
>>> to review.
>>
>> Which one did you think was not trivial?
> 
> Personaly for me, it was second patch from the series.

That patch was found by experience.  Even if it isn't totally correct,
it improved the specific situation.  I'm fine taking that patch and if
the maintainer of rxe thinks there is a better way, I'm open to follow
on patches.


-- 
Doug Ledford <dledford-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
    GPG Key ID: B826A3330E572FDD
    Key fingerprint = AE6B 1BDA 122B 23B4 265B  1274 B826 A333 0E57 2FDD


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 884 bytes --]