Linux-RDMA Archive on lore.kernel.org
 help / color / Atom feed
* [PATCH 1/2] Update mailmap info for Steve Wise
@ 2019-12-03  2:03 Steve Wise
  2019-12-03  2:03 ` [PATCH 2/2] rxe: correctly calculate iCRC for unaligned payloads Steve Wise
  0 siblings, 1 reply; 5+ messages in thread
From: Steve Wise @ 2019-12-03  2:03 UTC (permalink / raw)
  To: linux-rdma; +Cc: Steve Wise

Signed-off-by: Steve Wise <larrystevenwise@gmail.com>
---
 .mailmap | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/.mailmap b/.mailmap
index 83d7e750c2fc..ef79a4a6c258 100644
--- a/.mailmap
+++ b/.mailmap
@@ -267,3 +267,5 @@ Gustavo Padovan <gustavo@las.ic.unicamp.br>
 Gustavo Padovan <padovan@profusion.mobi>
 Changbin Du <changbin.du@intel.com> <changbin.du@intel.com>
 Changbin Du <changbin.du@intel.com> <changbin.du@gmail.com>
+Steve Wise <larrystevenwise@gmail.com> <swise@chelsio.com>
+Steve Wise <larrystevenwise@gmail.com> <swise@opengridcomputing.com>
-- 
2.20.1


^ permalink raw reply	[flat|nested] 5+ messages in thread

* [PATCH 2/2] rxe: correctly calculate iCRC for unaligned payloads
  2019-12-03  2:03 [PATCH 1/2] Update mailmap info for Steve Wise Steve Wise
@ 2019-12-03  2:03 ` Steve Wise
  2019-12-03 16:25   ` Bart Van Assche
  0 siblings, 1 reply; 5+ messages in thread
From: Steve Wise @ 2019-12-03  2:03 UTC (permalink / raw)
  To: linux-rdma; +Cc: Steve Wise, bvanassche

If RoCE PDUs being sent or received contain pad bytes, then the iCRC
is miscalculated resulting PDUs being emitted by RXE with an incorrect
iCRC, as well as ingress PDUs being dropped due to erroneously detecting
a bad iCRC in the PDU.  The fix is to include the pad bytes, if any,
in iCRC computations.

CC: bvanassche@acm.org,3100102071@zju.edu.cn,leon@kernel.org
Signed-off-by: Steve Wise <larrystevenwise@gmail.com>
---
 drivers/infiniband/sw/rxe/rxe_recv.c | 2 +-
 drivers/infiniband/sw/rxe/rxe_req.c  | 6 ++++++
 drivers/infiniband/sw/rxe/rxe_resp.c | 7 +++++++
 3 files changed, 14 insertions(+), 1 deletion(-)

diff --git a/drivers/infiniband/sw/rxe/rxe_recv.c b/drivers/infiniband/sw/rxe/rxe_recv.c
index f9a492ed900b..831ad578a7b2 100644
--- a/drivers/infiniband/sw/rxe/rxe_recv.c
+++ b/drivers/infiniband/sw/rxe/rxe_recv.c
@@ -389,7 +389,7 @@ void rxe_rcv(struct sk_buff *skb)
 
 	calc_icrc = rxe_icrc_hdr(pkt, skb);
 	calc_icrc = rxe_crc32(rxe, calc_icrc, (u8 *)payload_addr(pkt),
-			      payload_size(pkt));
+			      payload_size(pkt) + bth_pad(pkt));
 	calc_icrc = (__force u32)cpu_to_be32(~calc_icrc);
 	if (unlikely(calc_icrc != pack_icrc)) {
 		if (skb->protocol == htons(ETH_P_IPV6))
diff --git a/drivers/infiniband/sw/rxe/rxe_req.c b/drivers/infiniband/sw/rxe/rxe_req.c
index c5d9b558fa90..e5031172c019 100644
--- a/drivers/infiniband/sw/rxe/rxe_req.c
+++ b/drivers/infiniband/sw/rxe/rxe_req.c
@@ -500,6 +500,12 @@ static int fill_packet(struct rxe_qp *qp, struct rxe_send_wqe *wqe,
 			if (err)
 				return err;
 		}
+		if (bth_pad(pkt)) {
+			u8 *pad = payload_addr(pkt) + paylen;
+
+			memset(pad, 0, bth_pad(pkt));
+			crc = rxe_crc32(rxe, crc, pad, bth_pad(pkt));
+		}
 	}
 	p = payload_addr(pkt) + paylen + bth_pad(pkt);
 
diff --git a/drivers/infiniband/sw/rxe/rxe_resp.c b/drivers/infiniband/sw/rxe/rxe_resp.c
index 1cbfbd98eb22..c4a8195bf670 100644
--- a/drivers/infiniband/sw/rxe/rxe_resp.c
+++ b/drivers/infiniband/sw/rxe/rxe_resp.c
@@ -732,6 +732,13 @@ static enum resp_states read_reply(struct rxe_qp *qp,
 	if (err)
 		pr_err("Failed copying memory\n");
 
+	if (bth_pad(&ack_pkt)) {
+		struct rxe_dev *rxe = to_rdev(qp->ibqp.device);
+		u8 *pad = payload_addr(&ack_pkt) + payload;
+
+		memset(pad, 0, bth_pad(&ack_pkt));
+		icrc = rxe_crc32(rxe, icrc, pad, bth_pad(&ack_pkt));
+	}
 	p = payload_addr(&ack_pkt) + payload + bth_pad(&ack_pkt);
 	*p = ~icrc;
 
-- 
2.20.1


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH 2/2] rxe: correctly calculate iCRC for unaligned payloads
  2019-12-03  2:03 ` [PATCH 2/2] rxe: correctly calculate iCRC for unaligned payloads Steve Wise
@ 2019-12-03 16:25   ` Bart Van Assche
  2019-12-03 18:32     ` Steve Wise
  2019-12-04  0:46     ` Doug Ledford
  0 siblings, 2 replies; 5+ messages in thread
From: Bart Van Assche @ 2019-12-03 16:25 UTC (permalink / raw)
  To: Steve Wise, linux-rdma, 3100102071, leon

On 12/2/19 6:03 PM, Steve Wise wrote:
> If RoCE PDUs being sent or received contain pad bytes, then the iCRC
> is miscalculated resulting PDUs being emitted by RXE with an incorrect
> iCRC, as well as ingress PDUs being dropped due to erroneously detecting
> a bad iCRC in the PDU.  The fix is to include the pad bytes, if any,
> in iCRC computations.

Should this description mention that this patch breaks compatibility 
with SoftRoCE drivers that do not include this fix? Do we need a kernel 
module parameter that allows to select either the old or the new behavior?

> CC: bvanassche@acm.org,3100102071@zju.edu.cn,leon@kernel.org

Should this Cc-line perhaps be converted into three Cc-lines?

Otherwise this patch looks fine to me.

Thanks,

Bart.

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH 2/2] rxe: correctly calculate iCRC for unaligned payloads
  2019-12-03 16:25   ` Bart Van Assche
@ 2019-12-03 18:32     ` Steve Wise
  2019-12-04  0:46     ` Doug Ledford
  1 sibling, 0 replies; 5+ messages in thread
From: Steve Wise @ 2019-12-03 18:32 UTC (permalink / raw)
  To: Bart Van Assche; +Cc: linux-rdma, wangqi, Leon Romanovsky

On Tue, Dec 3, 2019 at 10:25 AM Bart Van Assche <bvanassche@acm.org> wrote:
>
> On 12/2/19 6:03 PM, Steve Wise wrote:
> > If RoCE PDUs being sent or received contain pad bytes, then the iCRC
> > is miscalculated resulting PDUs being emitted by RXE with an incorrect
> > iCRC, as well as ingress PDUs being dropped due to erroneously detecting
> > a bad iCRC in the PDU.  The fix is to include the pad bytes, if any,
> > in iCRC computations.
>
> Should this description mention that this patch breaks compatibility
> with SoftRoCE drivers that do not include this fix? Do we need a kernel
> module parameter that allows to select either the old or the new behavior?
>

Good point.  I defer to others on how they want to handle that.

> > CC: bvanassche@acm.org,3100102071@zju.edu.cn,leon@kernel.org
>
> Should this Cc-line perhaps be converted into three Cc-lines?

Yea I screwed this up.   I really didn't want this in the commit log
vs just CCing on the email submission, but I was having issues with
git send-email.  Pilot error. ;)


Stevo

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH 2/2] rxe: correctly calculate iCRC for unaligned payloads
  2019-12-03 16:25   ` Bart Van Assche
  2019-12-03 18:32     ` Steve Wise
@ 2019-12-04  0:46     ` Doug Ledford
  1 sibling, 0 replies; 5+ messages in thread
From: Doug Ledford @ 2019-12-04  0:46 UTC (permalink / raw)
  To: Bart Van Assche, Steve Wise, linux-rdma, 3100102071, leon

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

On Tue, 2019-12-03 at 08:25 -0800, Bart Van Assche wrote:
> On 12/2/19 6:03 PM, Steve Wise wrote:
> > If RoCE PDUs being sent or received contain pad bytes, then the iCRC
> > is miscalculated resulting PDUs being emitted by RXE with an
> > incorrect
> > iCRC, as well as ingress PDUs being dropped due to erroneously
> > detecting
> > a bad iCRC in the PDU.  The fix is to include the pad bytes, if any,
> > in iCRC computations.
> 
> Should this description mention that this patch breaks compatibility 
> with SoftRoCE drivers that do not include this fix? Do we need a
> kernel 
> module parameter that allows to select either the old or the new
> behavior?

No.  The original soft-RoCE driver was supposed to be compatible with
hardware devices.  Because of this bug, it obviously wasn't.  This is a
bug fix, and we do not need to do anything to be compatible with the
broken behavior.  Instead, it just needs noting that the soft-RoCE
implementation in prior kernels has a known wire format bug that impacts
communications with both fixed versions of the driver and real hardware
devices.

> > CC: bvanassche@acm.org,3100102071@zju.edu.cn,leon@kernel.org
> 
> Should this Cc-line perhaps be converted into three Cc-lines?
> 
> Otherwise this patch looks fine to me.
> 
> Thanks,
> 
> Bart.
> 

-- 
Doug Ledford <dledford@redhat.com>
    GPG KeyID: B826A3330E572FDD
    Fingerprint = AE6B 1BDA 122B 23B4 265B  1274 B826 A333 0E57 2FDD

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, back to index

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-12-03  2:03 [PATCH 1/2] Update mailmap info for Steve Wise Steve Wise
2019-12-03  2:03 ` [PATCH 2/2] rxe: correctly calculate iCRC for unaligned payloads Steve Wise
2019-12-03 16:25   ` Bart Van Assche
2019-12-03 18:32     ` Steve Wise
2019-12-04  0:46     ` Doug Ledford

Linux-RDMA Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-rdma/0 linux-rdma/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-rdma linux-rdma/ https://lore.kernel.org/linux-rdma \
		linux-rdma@vger.kernel.org
	public-inbox-index linux-rdma

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-rdma


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git