All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH for-next v2 0/9] ICRC cleanup
@ 2021-07-07  4:00 Bob Pearson
  2021-07-07  4:00 ` [PATCH v2 1/9] RDMA/rxe: Move ICRC checking to a subroutine Bob Pearson
                   ` (10 more replies)
  0 siblings, 11 replies; 24+ messages in thread
From: Bob Pearson @ 2021-07-07  4:00 UTC (permalink / raw)
  To: jgg, zyjzyj2000, linux-rdma; +Cc: Bob Pearson

This series of patches is a cleanup of the ICRC code in the rxe driver.
The end result is to collect all the ICRC focused code into rxe_icrc.c
with three APIs that are used by the rest of the driver. One to initialize
the crypto engine used to compute crc32, and one each to generate and
check the ICRC in a packet.

Signed-off-by: Bob Pearson <rpearsonhpe@gmail.com>
---
v2:
  Split up the 5 patches in the first version into 9 patches which are
  each focused on a single change.
  Fixed sparse warnings in the first version.

Bob Pearson (9):
  RDMA/rxe: Move ICRC checking to a subroutine
  RDMA/rxe: Move rxe_xmit_packet to a subroutine
  RDMA/rxe: Fixup rxe_send and rxe_loopback
  RDMA/rxe: Move ICRC generation to a subroutine
  RDMA/rxe: Move rxe_crc32 to a subroutine
  RDMA/rxe: Fixup rxe_icrc_hdr
  RDMA/rxe: Move crc32 init code to rxe_icrc.c
  RDMA/rxe: Add kernel-doc comments to rxe_icrc.c
  RDMA/rxe: Fix types in rxe_icrc.c

 drivers/infiniband/sw/rxe/rxe.h       |  22 -----
 drivers/infiniband/sw/rxe/rxe_comp.c  |   4 +-
 drivers/infiniband/sw/rxe/rxe_icrc.c  | 124 +++++++++++++++++++++++++-
 drivers/infiniband/sw/rxe/rxe_loc.h   |  61 +++----------
 drivers/infiniband/sw/rxe/rxe_mr.c    |  22 +----
 drivers/infiniband/sw/rxe/rxe_net.c   |  59 ++++++++++--
 drivers/infiniband/sw/rxe/rxe_recv.c  |  23 +----
 drivers/infiniband/sw/rxe/rxe_req.c   |  13 +--
 drivers/infiniband/sw/rxe/rxe_resp.c  |  33 ++-----
 drivers/infiniband/sw/rxe/rxe_verbs.c |  11 +--
 10 files changed, 202 insertions(+), 170 deletions(-)

-- 
2.30.2


^ permalink raw reply	[flat|nested] 24+ messages in thread
* [PATCH v2 1/9] RDMA/rxe: Move ICRC checking to a subroutine
  2021-07-07  4:00 [PATCH for-next v2 0/9] ICRC cleanup Bob Pearson
@ 2021-07-07  4:00 ` Bob Pearson
  2021-07-13  7:27   ` Zhu Yanjun
  2021-07-16 15:55   ` Jason Gunthorpe
  2021-07-07  4:00 ` [PATCH v2 2/9] RDMA/rxe: Move rxe_xmit_packet " Bob Pearson
                   ` (9 subsequent siblings)
  10 siblings, 2 replies; 24+ messages in thread
From: Bob Pearson @ 2021-07-07  4:00 UTC (permalink / raw)
  To: jgg, zyjzyj2000, linux-rdma; +Cc: Bob Pearson

Move the code in rxe_recv() that checks the ICRC on incoming packets to a
subroutine rxe_check_icrc() and move that to rxe_icrc.c.

Signed-off-by: Bob Pearson <rpearsonhpe@gmail.com>
---
 drivers/infiniband/sw/rxe/rxe_icrc.c | 38 ++++++++++++++++++++++++++++
 drivers/infiniband/sw/rxe/rxe_loc.h  |  2 ++
 drivers/infiniband/sw/rxe/rxe_recv.c | 23 ++---------------
 3 files changed, 42 insertions(+), 21 deletions(-)

diff --git a/drivers/infiniband/sw/rxe/rxe_icrc.c b/drivers/infiniband/sw/rxe/rxe_icrc.c
index 66b2aad54bb7..d067841214be 100644
--- a/drivers/infiniband/sw/rxe/rxe_icrc.c
+++ b/drivers/infiniband/sw/rxe/rxe_icrc.c
@@ -67,3 +67,41 @@ u32 rxe_icrc_hdr(struct rxe_pkt_info *pkt, struct sk_buff *skb)
 			rxe_opcode[pkt->opcode].length - RXE_BTH_BYTES);
 	return crc;
 }
+
+/**
+ * rxe_icrc_check() - Compute ICRC for a packet and compare to the ICRC
+ *		      delivered in the packet.
+ * @skb: packet buffer
+ * @pkt: packet info
+ *
+ * Return: 0 if the values match else an error
+ */
+int rxe_icrc_check(struct sk_buff *skb, struct rxe_pkt_info *pkt)
+{
+	__be32 *icrcp;
+	u32 pkt_icrc;
+	u32 icrc;
+
+	icrcp = (__be32 *)(pkt->hdr + pkt->paylen - RXE_ICRC_SIZE);
+	pkt_icrc = be32_to_cpu(*icrcp);
+
+	icrc = rxe_icrc_hdr(pkt, skb);
+	icrc = rxe_crc32(pkt->rxe, icrc, (u8 *)payload_addr(pkt),
+				payload_size(pkt) + bth_pad(pkt));
+	icrc = (__force u32)cpu_to_be32(~icrc);
+
+	if (unlikely(icrc != pkt_icrc)) {
+		if (skb->protocol == htons(ETH_P_IPV6))
+			pr_warn_ratelimited("bad ICRC from %pI6c\n",
+					    &ipv6_hdr(skb)->saddr);
+		else if (skb->protocol == htons(ETH_P_IP))
+			pr_warn_ratelimited("bad ICRC from %pI4\n",
+					    &ip_hdr(skb)->saddr);
+		else
+			pr_warn_ratelimited("bad ICRC from unknown\n");
+
+		return -EINVAL;
+	}
+
+	return 0;
+}
diff --git a/drivers/infiniband/sw/rxe/rxe_loc.h b/drivers/infiniband/sw/rxe/rxe_loc.h
index 1ddb20855dee..015777e31ec9 100644
--- a/drivers/infiniband/sw/rxe/rxe_loc.h
+++ b/drivers/infiniband/sw/rxe/rxe_loc.h
@@ -193,7 +193,9 @@ int rxe_completer(void *arg);
 int rxe_requester(void *arg);
 int rxe_responder(void *arg);
 
+/* rxe_icrc.c */
 u32 rxe_icrc_hdr(struct rxe_pkt_info *pkt, struct sk_buff *skb);
+int rxe_icrc_check(struct sk_buff *skb, struct rxe_pkt_info *pkt);
 
 void rxe_resp_queue_pkt(struct rxe_qp *qp, struct sk_buff *skb);
 
diff --git a/drivers/infiniband/sw/rxe/rxe_recv.c b/drivers/infiniband/sw/rxe/rxe_recv.c
index 7a49e27da23a..6a6cc1fa90e4 100644
--- a/drivers/infiniband/sw/rxe/rxe_recv.c
+++ b/drivers/infiniband/sw/rxe/rxe_recv.c
@@ -361,8 +361,6 @@ void rxe_rcv(struct sk_buff *skb)
 	int err;
 	struct rxe_pkt_info *pkt = SKB_TO_PKT(skb);
 	struct rxe_dev *rxe = pkt->rxe;
-	__be32 *icrcp;
-	u32 calc_icrc, pack_icrc;
 
 	if (unlikely(skb->len < RXE_BTH_BYTES))
 		goto drop;
@@ -384,26 +382,9 @@ void rxe_rcv(struct sk_buff *skb)
 	if (unlikely(err))
 		goto drop;
 
-	/* Verify ICRC */
-	icrcp = (__be32 *)(pkt->hdr + pkt->paylen - RXE_ICRC_SIZE);
-	pack_icrc = be32_to_cpu(*icrcp);
-
-	calc_icrc = rxe_icrc_hdr(pkt, skb);
-	calc_icrc = rxe_crc32(rxe, calc_icrc, (u8 *)payload_addr(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))
-			pr_warn_ratelimited("bad ICRC from %pI6c\n",
-					    &ipv6_hdr(skb)->saddr);
-		else if (skb->protocol == htons(ETH_P_IP))
-			pr_warn_ratelimited("bad ICRC from %pI4\n",
-					    &ip_hdr(skb)->saddr);
-		else
-			pr_warn_ratelimited("bad ICRC from unknown\n");
-
+	err = rxe_icrc_check(skb, pkt);
+	if (unlikely(err))
 		goto drop;
-	}
 
 	rxe_counter_inc(rxe, RXE_CNT_RCVD_PKTS);
 
-- 
2.30.2


^ permalink raw reply related	[flat|nested] 24+ messages in thread
* [PATCH v2 2/9] RDMA/rxe: Move rxe_xmit_packet to a subroutine
  2021-07-07  4:00 [PATCH for-next v2 0/9] ICRC cleanup Bob Pearson
  2021-07-07  4:00 ` [PATCH v2 1/9] RDMA/rxe: Move ICRC checking to a subroutine Bob Pearson
@ 2021-07-07  4:00 ` Bob Pearson
  2021-07-14  3:24   ` Zhu Yanjun
  2021-07-07  4:00 ` [PATCH v2 3/9] RDMA/rxe: Fixup rxe_send and rxe_loopback Bob Pearson
                   ` (8 subsequent siblings)
  10 siblings, 1 reply; 24+ messages in thread
From: Bob Pearson @ 2021-07-07  4:00 UTC (permalink / raw)
  To: jgg, zyjzyj2000, linux-rdma; +Cc: Bob Pearson

rxe_xmit_packet() was an overlong inline subroutine. This patch moves it
into rxe_net.c as an ordinary subroutine.

Signed-off-by: Bob Pearson <rpearsonhpe@gmail.com>
---
 drivers/infiniband/sw/rxe/rxe_loc.h | 45 ++---------------------------
 drivers/infiniband/sw/rxe/rxe_net.c | 43 +++++++++++++++++++++++++++
 2 files changed, 45 insertions(+), 43 deletions(-)

diff --git a/drivers/infiniband/sw/rxe/rxe_loc.h b/drivers/infiniband/sw/rxe/rxe_loc.h
index 015777e31ec9..409d10f20948 100644
--- a/drivers/infiniband/sw/rxe/rxe_loc.h
+++ b/drivers/infiniband/sw/rxe/rxe_loc.h
@@ -104,6 +104,8 @@ int rxe_send(struct rxe_pkt_info *pkt, struct sk_buff *skb);
 struct sk_buff *rxe_init_packet(struct rxe_dev *rxe, struct rxe_av *av,
 				int paylen, struct rxe_pkt_info *pkt);
 int rxe_prepare(struct rxe_pkt_info *pkt, struct sk_buff *skb, u32 *crc);
+int rxe_xmit_packet(struct rxe_qp *qp, struct rxe_pkt_info *pkt,
+		    struct sk_buff *skb);
 const char *rxe_parent_name(struct rxe_dev *rxe, unsigned int port_num);
 int rxe_mcast_add(struct rxe_dev *rxe, union ib_gid *mgid);
 int rxe_mcast_delete(struct rxe_dev *rxe, union ib_gid *mgid);
@@ -206,47 +208,4 @@ static inline unsigned int wr_opcode_mask(int opcode, struct rxe_qp *qp)
 	return rxe_wr_opcode_info[opcode].mask[qp->ibqp.qp_type];
 }
 
-static inline int rxe_xmit_packet(struct rxe_qp *qp, struct rxe_pkt_info *pkt,
-				  struct sk_buff *skb)
-{
-	int err;
-	int is_request = pkt->mask & RXE_REQ_MASK;
-	struct rxe_dev *rxe = to_rdev(qp->ibqp.device);
-
-	if ((is_request && (qp->req.state != QP_STATE_READY)) ||
-	    (!is_request && (qp->resp.state != QP_STATE_READY))) {
-		pr_info("Packet dropped. QP is not in ready state\n");
-		goto drop;
-	}
-
-	if (pkt->mask & RXE_LOOPBACK_MASK) {
-		memcpy(SKB_TO_PKT(skb), pkt, sizeof(*pkt));
-		rxe_loopback(skb);
-		err = 0;
-	} else {
-		err = rxe_send(pkt, skb);
-	}
-
-	if (err) {
-		rxe->xmit_errors++;
-		rxe_counter_inc(rxe, RXE_CNT_SEND_ERR);
-		return err;
-	}
-
-	if ((qp_type(qp) != IB_QPT_RC) &&
-	    (pkt->mask & RXE_END_MASK)) {
-		pkt->wqe->state = wqe_state_done;
-		rxe_run_task(&qp->comp.task, 1);
-	}
-
-	rxe_counter_inc(rxe, RXE_CNT_SENT_PKTS);
-	goto done;
-
-drop:
-	kfree_skb(skb);
-	err = 0;
-done:
-	return err;
-}
-
 #endif /* RXE_LOC_H */
diff --git a/drivers/infiniband/sw/rxe/rxe_net.c b/drivers/infiniband/sw/rxe/rxe_net.c
index dec92928a1cd..c93a379a1b28 100644
--- a/drivers/infiniband/sw/rxe/rxe_net.c
+++ b/drivers/infiniband/sw/rxe/rxe_net.c
@@ -421,6 +421,49 @@ void rxe_loopback(struct sk_buff *skb)
 		rxe_rcv(skb);
 }
 
+int rxe_xmit_packet(struct rxe_qp *qp, struct rxe_pkt_info *pkt,
+		    struct sk_buff *skb)
+{
+	int err;
+	int is_request = pkt->mask & RXE_REQ_MASK;
+	struct rxe_dev *rxe = to_rdev(qp->ibqp.device);
+
+	if ((is_request && (qp->req.state != QP_STATE_READY)) ||
+	    (!is_request && (qp->resp.state != QP_STATE_READY))) {
+		pr_info("Packet dropped. QP is not in ready state\n");
+		goto drop;
+	}
+
+	if (pkt->mask & RXE_LOOPBACK_MASK) {
+		memcpy(SKB_TO_PKT(skb), pkt, sizeof(*pkt));
+		rxe_loopback(skb);
+		err = 0;
+	} else {
+		err = rxe_send(pkt, skb);
+	}
+
+	if (err) {
+		rxe->xmit_errors++;
+		rxe_counter_inc(rxe, RXE_CNT_SEND_ERR);
+		return err;
+	}
+
+	if ((qp_type(qp) != IB_QPT_RC) &&
+	    (pkt->mask & RXE_END_MASK)) {
+		pkt->wqe->state = wqe_state_done;
+		rxe_run_task(&qp->comp.task, 1);
+	}
+
+	rxe_counter_inc(rxe, RXE_CNT_SENT_PKTS);
+	goto done;
+
+drop:
+	kfree_skb(skb);
+	err = 0;
+done:
+	return err;
+}
+
 struct sk_buff *rxe_init_packet(struct rxe_dev *rxe, struct rxe_av *av,
 				int paylen, struct rxe_pkt_info *pkt)
 {
-- 
2.30.2


^ permalink raw reply related	[flat|nested] 24+ messages in thread
* [PATCH v2 3/9] RDMA/rxe: Fixup rxe_send and rxe_loopback
  2021-07-07  4:00 [PATCH for-next v2 0/9] ICRC cleanup Bob Pearson
  2021-07-07  4:00 ` [PATCH v2 1/9] RDMA/rxe: Move ICRC checking to a subroutine Bob Pearson
  2021-07-07  4:00 ` [PATCH v2 2/9] RDMA/rxe: Move rxe_xmit_packet " Bob Pearson
@ 2021-07-07  4:00 ` Bob Pearson
  2021-07-07  4:00 ` [PATCH v2 4/9] RDMA/rxe: Move ICRC generation to a subroutine Bob Pearson
                   ` (7 subsequent siblings)
  10 siblings, 0 replies; 24+ messages in thread
From: Bob Pearson @ 2021-07-07  4:00 UTC (permalink / raw)
  To: jgg, zyjzyj2000, linux-rdma; +Cc: Bob Pearson

Fixup rxe_send() and rxe_loopback() in rxe_net.c to have the same
calling sequence. This patch makes them static and have the same
parameter list and return value.

Signed-off-by: Bob Pearson <rpearsonhpe@gmail.com>
---
 drivers/infiniband/sw/rxe/rxe_loc.h |  2 --
 drivers/infiniband/sw/rxe/rxe_net.c | 28 ++++++++++++++--------------
 2 files changed, 14 insertions(+), 16 deletions(-)

diff --git a/drivers/infiniband/sw/rxe/rxe_loc.h b/drivers/infiniband/sw/rxe/rxe_loc.h
index 409d10f20948..5fc9abea88ca 100644
--- a/drivers/infiniband/sw/rxe/rxe_loc.h
+++ b/drivers/infiniband/sw/rxe/rxe_loc.h
@@ -99,8 +99,6 @@ struct rxe_mw *rxe_lookup_mw(struct rxe_qp *qp, int access, u32 rkey);
 void rxe_mw_cleanup(struct rxe_pool_entry *arg);
 
 /* rxe_net.c */
-void rxe_loopback(struct sk_buff *skb);
-int rxe_send(struct rxe_pkt_info *pkt, struct sk_buff *skb);
 struct sk_buff *rxe_init_packet(struct rxe_dev *rxe, struct rxe_av *av,
 				int paylen, struct rxe_pkt_info *pkt);
 int rxe_prepare(struct rxe_pkt_info *pkt, struct sk_buff *skb, u32 *crc);
diff --git a/drivers/infiniband/sw/rxe/rxe_net.c b/drivers/infiniband/sw/rxe/rxe_net.c
index c93a379a1b28..beaaec2e5a17 100644
--- a/drivers/infiniband/sw/rxe/rxe_net.c
+++ b/drivers/infiniband/sw/rxe/rxe_net.c
@@ -373,7 +373,7 @@ static void rxe_skb_tx_dtor(struct sk_buff *skb)
 	rxe_drop_ref(qp);
 }
 
-int rxe_send(struct rxe_pkt_info *pkt, struct sk_buff *skb)
+static int rxe_send(struct sk_buff *skb, struct rxe_pkt_info *pkt)
 {
 	int err;
 
@@ -406,19 +406,23 @@ int rxe_send(struct rxe_pkt_info *pkt, struct sk_buff *skb)
 /* fix up a send packet to match the packets
  * received from UDP before looping them back
  */
-void rxe_loopback(struct sk_buff *skb)
+static int rxe_loopback(struct sk_buff *skb, struct rxe_pkt_info *pkt)
 {
-	struct rxe_pkt_info *pkt = SKB_TO_PKT(skb);
+	memcpy(SKB_TO_PKT(skb), pkt, sizeof(*pkt));
 
 	if (skb->protocol == htons(ETH_P_IP))
 		skb_pull(skb, sizeof(struct iphdr));
 	else
 		skb_pull(skb, sizeof(struct ipv6hdr));
 
-	if (WARN_ON(!ib_device_try_get(&pkt->rxe->ib_dev)))
+	if (WARN_ON(!ib_device_try_get(&pkt->rxe->ib_dev))) {
 		kfree_skb(skb);
-	else
-		rxe_rcv(skb);
+		return -EIO;
+	}
+
+	rxe_rcv(skb);
+
+	return 0;
 }
 
 int rxe_xmit_packet(struct rxe_qp *qp, struct rxe_pkt_info *pkt,
@@ -434,14 +438,10 @@ int rxe_xmit_packet(struct rxe_qp *qp, struct rxe_pkt_info *pkt,
 		goto drop;
 	}
 
-	if (pkt->mask & RXE_LOOPBACK_MASK) {
-		memcpy(SKB_TO_PKT(skb), pkt, sizeof(*pkt));
-		rxe_loopback(skb);
-		err = 0;
-	} else {
-		err = rxe_send(pkt, skb);
-	}
-
+	if (pkt->mask & RXE_LOOPBACK_MASK)
+		err = rxe_loopback(skb, pkt);
+	else
+		err = rxe_send(skb, pkt);
 	if (err) {
 		rxe->xmit_errors++;
 		rxe_counter_inc(rxe, RXE_CNT_SEND_ERR);
-- 
2.30.2


^ permalink raw reply related	[flat|nested] 24+ messages in thread
* [PATCH v2 4/9] RDMA/rxe: Move ICRC generation to a subroutine
  2021-07-07  4:00 [PATCH for-next v2 0/9] ICRC cleanup Bob Pearson
                   ` (2 preceding siblings ...)
  2021-07-07  4:00 ` [PATCH v2 3/9] RDMA/rxe: Fixup rxe_send and rxe_loopback Bob Pearson
@ 2021-07-07  4:00 ` Bob Pearson
  2021-07-16 15:57   ` Jason Gunthorpe
  2021-07-07  4:00 ` [PATCH v2 5/9] RDMA/rxe: Move rxe_crc32 " Bob Pearson
                   ` (6 subsequent siblings)
  10 siblings, 1 reply; 24+ messages in thread
From: Bob Pearson @ 2021-07-07  4:00 UTC (permalink / raw)
  To: jgg, zyjzyj2000, linux-rdma; +Cc: Bob Pearson

Isolate ICRC generation into a single subroutine named rxe_generate_icrc()
in rxe_icrc.c. Remove scattered crc generation code from elsewhere.

Signed-off-by: Bob Pearson <rpearsonhpe@gmail.com>
---
 drivers/infiniband/sw/rxe/rxe_comp.c |  4 ++--
 drivers/infiniband/sw/rxe/rxe_icrc.c | 13 +++++++++++
 drivers/infiniband/sw/rxe/rxe_loc.h  | 10 ++++-----
 drivers/infiniband/sw/rxe/rxe_mr.c   | 22 ++++---------------
 drivers/infiniband/sw/rxe/rxe_net.c  |  6 ++---
 drivers/infiniband/sw/rxe/rxe_req.c  | 13 ++---------
 drivers/infiniband/sw/rxe/rxe_resp.c | 33 +++++++---------------------
 7 files changed, 37 insertions(+), 64 deletions(-)

diff --git a/drivers/infiniband/sw/rxe/rxe_comp.c b/drivers/infiniband/sw/rxe/rxe_comp.c
index 58ad9c2644f3..d2d802c776fd 100644
--- a/drivers/infiniband/sw/rxe/rxe_comp.c
+++ b/drivers/infiniband/sw/rxe/rxe_comp.c
@@ -349,7 +349,7 @@ static inline enum comp_state do_read(struct rxe_qp *qp,
 
 	ret = copy_data(qp->pd, IB_ACCESS_LOCAL_WRITE,
 			&wqe->dma, payload_addr(pkt),
-			payload_size(pkt), RXE_TO_MR_OBJ, NULL);
+			payload_size(pkt), RXE_TO_MR_OBJ);
 	if (ret) {
 		wqe->status = IB_WC_LOC_PROT_ERR;
 		return COMPST_ERROR;
@@ -371,7 +371,7 @@ static inline enum comp_state do_atomic(struct rxe_qp *qp,
 
 	ret = copy_data(qp->pd, IB_ACCESS_LOCAL_WRITE,
 			&wqe->dma, &atomic_orig,
-			sizeof(u64), RXE_TO_MR_OBJ, NULL);
+			sizeof(u64), RXE_TO_MR_OBJ);
 	if (ret) {
 		wqe->status = IB_WC_LOC_PROT_ERR;
 		return COMPST_ERROR;
diff --git a/drivers/infiniband/sw/rxe/rxe_icrc.c b/drivers/infiniband/sw/rxe/rxe_icrc.c
index d067841214be..08ab32eb6445 100644
--- a/drivers/infiniband/sw/rxe/rxe_icrc.c
+++ b/drivers/infiniband/sw/rxe/rxe_icrc.c
@@ -105,3 +105,16 @@ int rxe_icrc_check(struct sk_buff *skb, struct rxe_pkt_info *pkt)
 
 	return 0;
 }
+
+/* rxe_icrc_generate- compute ICRC for a packet. */
+void rxe_icrc_generate(struct sk_buff *skb, struct rxe_pkt_info *pkt)
+{
+	__be32 *icrcp;
+	u32 icrc;
+
+	icrcp = (__be32 *)(pkt->hdr + pkt->paylen - RXE_ICRC_SIZE);
+	icrc = rxe_icrc_hdr(pkt, skb);
+	icrc = rxe_crc32(pkt->rxe, icrc, (u8 *)payload_addr(pkt),
+				payload_size(pkt) + bth_pad(pkt));
+	*icrcp = (__force __be32)~icrc;
+}
diff --git a/drivers/infiniband/sw/rxe/rxe_loc.h b/drivers/infiniband/sw/rxe/rxe_loc.h
index 5fc9abea88ca..a832535fa35a 100644
--- a/drivers/infiniband/sw/rxe/rxe_loc.h
+++ b/drivers/infiniband/sw/rxe/rxe_loc.h
@@ -77,10 +77,9 @@ int rxe_mr_init_user(struct rxe_pd *pd, u64 start, u64 length, u64 iova,
 		     int access, struct rxe_mr *mr);
 int rxe_mr_init_fast(struct rxe_pd *pd, int max_pages, struct rxe_mr *mr);
 int rxe_mr_copy(struct rxe_mr *mr, u64 iova, void *addr, int length,
-		enum rxe_mr_copy_dir dir, u32 *crcp);
-int copy_data(struct rxe_pd *pd, int access,
-	      struct rxe_dma_info *dma, void *addr, int length,
-	      enum rxe_mr_copy_dir dir, u32 *crcp);
+		enum rxe_mr_copy_dir dir);
+int copy_data(struct rxe_pd *pd, int access, struct rxe_dma_info *dma,
+	      void *addr, int length, enum rxe_mr_copy_dir dir);
 void *iova_to_vaddr(struct rxe_mr *mr, u64 iova, int length);
 struct rxe_mr *lookup_mr(struct rxe_pd *pd, int access, u32 key,
 			 enum rxe_mr_lookup_type type);
@@ -101,7 +100,7 @@ void rxe_mw_cleanup(struct rxe_pool_entry *arg);
 /* rxe_net.c */
 struct sk_buff *rxe_init_packet(struct rxe_dev *rxe, struct rxe_av *av,
 				int paylen, struct rxe_pkt_info *pkt);
-int rxe_prepare(struct rxe_pkt_info *pkt, struct sk_buff *skb, u32 *crc);
+int rxe_prepare(struct rxe_pkt_info *pkt, struct sk_buff *skb);
 int rxe_xmit_packet(struct rxe_qp *qp, struct rxe_pkt_info *pkt,
 		    struct sk_buff *skb);
 const char *rxe_parent_name(struct rxe_dev *rxe, unsigned int port_num);
@@ -196,6 +195,7 @@ int rxe_responder(void *arg);
 /* rxe_icrc.c */
 u32 rxe_icrc_hdr(struct rxe_pkt_info *pkt, struct sk_buff *skb);
 int rxe_icrc_check(struct sk_buff *skb, struct rxe_pkt_info *pkt);
+void rxe_icrc_generate(struct sk_buff *skb, struct rxe_pkt_info *pkt);
 
 void rxe_resp_queue_pkt(struct rxe_qp *qp, struct sk_buff *skb);
 
diff --git a/drivers/infiniband/sw/rxe/rxe_mr.c b/drivers/infiniband/sw/rxe/rxe_mr.c
index 6aabcb4de235..ca48e285aaa7 100644
--- a/drivers/infiniband/sw/rxe/rxe_mr.c
+++ b/drivers/infiniband/sw/rxe/rxe_mr.c
@@ -279,11 +279,10 @@ void *iova_to_vaddr(struct rxe_mr *mr, u64 iova, int length)
 }
 
 /* copy data from a range (vaddr, vaddr+length-1) to or from
- * a mr object starting at iova. Compute incremental value of
- * crc32 if crcp is not zero. caller must hold a reference to mr
+ * a mr object starting at iova.
  */
 int rxe_mr_copy(struct rxe_mr *mr, u64 iova, void *addr, int length,
-		enum rxe_mr_copy_dir dir, u32 *crcp)
+		enum rxe_mr_copy_dir dir)
 {
 	int			err;
 	int			bytes;
@@ -293,7 +292,6 @@ int rxe_mr_copy(struct rxe_mr *mr, u64 iova, void *addr, int length,
 	int			m;
 	int			i;
 	size_t			offset;
-	u32			crc = crcp ? (*crcp) : 0;
 
 	if (length == 0)
 		return 0;
@@ -307,10 +305,6 @@ int rxe_mr_copy(struct rxe_mr *mr, u64 iova, void *addr, int length,
 
 		memcpy(dest, src, length);
 
-		if (crcp)
-			*crcp = rxe_crc32(to_rdev(mr->ibmr.device), *crcp, dest,
-					  length);
-
 		return 0;
 	}
 
@@ -341,10 +335,6 @@ int rxe_mr_copy(struct rxe_mr *mr, u64 iova, void *addr, int length,
 
 		memcpy(dest, src, bytes);
 
-		if (crcp)
-			crc = rxe_crc32(to_rdev(mr->ibmr.device), crc, dest,
-					bytes);
-
 		length	-= bytes;
 		addr	+= bytes;
 
@@ -359,9 +349,6 @@ int rxe_mr_copy(struct rxe_mr *mr, u64 iova, void *addr, int length,
 		}
 	}
 
-	if (crcp)
-		*crcp = crc;
-
 	return 0;
 
 err1:
@@ -377,8 +364,7 @@ int copy_data(
 	struct rxe_dma_info	*dma,
 	void			*addr,
 	int			length,
-	enum rxe_mr_copy_dir	dir,
-	u32			*crcp)
+	enum rxe_mr_copy_dir	dir)
 {
 	int			bytes;
 	struct rxe_sge		*sge	= &dma->sge[dma->cur_sge];
@@ -439,7 +425,7 @@ int copy_data(
 		if (bytes > 0) {
 			iova = sge->addr + offset;
 
-			err = rxe_mr_copy(mr, iova, addr, bytes, dir, crcp);
+			err = rxe_mr_copy(mr, iova, addr, bytes, dir);
 			if (err)
 				goto err2;
 
diff --git a/drivers/infiniband/sw/rxe/rxe_net.c b/drivers/infiniband/sw/rxe/rxe_net.c
index beaaec2e5a17..10c13dfebcbc 100644
--- a/drivers/infiniband/sw/rxe/rxe_net.c
+++ b/drivers/infiniband/sw/rxe/rxe_net.c
@@ -343,7 +343,7 @@ static int prepare6(struct rxe_pkt_info *pkt, struct sk_buff *skb)
 	return 0;
 }
 
-int rxe_prepare(struct rxe_pkt_info *pkt, struct sk_buff *skb, u32 *crc)
+int rxe_prepare(struct rxe_pkt_info *pkt, struct sk_buff *skb)
 {
 	int err = 0;
 
@@ -352,8 +352,6 @@ int rxe_prepare(struct rxe_pkt_info *pkt, struct sk_buff *skb, u32 *crc)
 	else if (skb->protocol == htons(ETH_P_IPV6))
 		err = prepare6(pkt, skb);
 
-	*crc = rxe_icrc_hdr(pkt, skb);
-
 	if (ether_addr_equal(skb->dev->dev_addr, rxe_get_av(pkt)->dmac))
 		pkt->mask |= RXE_LOOPBACK_MASK;
 
@@ -438,6 +436,8 @@ int rxe_xmit_packet(struct rxe_qp *qp, struct rxe_pkt_info *pkt,
 		goto drop;
 	}
 
+	rxe_icrc_generate(skb, pkt);
+
 	if (pkt->mask & RXE_LOOPBACK_MASK)
 		err = rxe_loopback(skb, pkt);
 	else
diff --git a/drivers/infiniband/sw/rxe/rxe_req.c b/drivers/infiniband/sw/rxe/rxe_req.c
index c57699cc6578..3894197a82f6 100644
--- a/drivers/infiniband/sw/rxe/rxe_req.c
+++ b/drivers/infiniband/sw/rxe/rxe_req.c
@@ -466,12 +466,9 @@ static int finish_packet(struct rxe_qp *qp, struct rxe_send_wqe *wqe,
 		       struct rxe_pkt_info *pkt, struct sk_buff *skb,
 		       int paylen)
 {
-	struct rxe_dev *rxe = to_rdev(qp->ibqp.device);
-	u32 crc = 0;
-	u32 *p;
 	int err;
 
-	err = rxe_prepare(pkt, skb, &crc);
+	err = rxe_prepare(pkt, skb);
 	if (err)
 		return err;
 
@@ -479,7 +476,6 @@ static int finish_packet(struct rxe_qp *qp, struct rxe_send_wqe *wqe,
 		if (wqe->wr.send_flags & IB_SEND_INLINE) {
 			u8 *tmp = &wqe->dma.inline_data[wqe->dma.sge_offset];
 
-			crc = rxe_crc32(rxe, crc, tmp, paylen);
 			memcpy(payload_addr(pkt), tmp, paylen);
 
 			wqe->dma.resid -= paylen;
@@ -487,8 +483,7 @@ static int finish_packet(struct rxe_qp *qp, struct rxe_send_wqe *wqe,
 		} else {
 			err = copy_data(qp->pd, 0, &wqe->dma,
 					payload_addr(pkt), paylen,
-					RXE_FROM_MR_OBJ,
-					&crc);
+					RXE_FROM_MR_OBJ);
 			if (err)
 				return err;
 		}
@@ -496,12 +491,8 @@ static int finish_packet(struct rxe_qp *qp, struct rxe_send_wqe *wqe,
 			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);
-
-	*p = ~crc;
 
 	return 0;
 }
diff --git a/drivers/infiniband/sw/rxe/rxe_resp.c b/drivers/infiniband/sw/rxe/rxe_resp.c
index 3743dc39b60c..685b8aebd627 100644
--- a/drivers/infiniband/sw/rxe/rxe_resp.c
+++ b/drivers/infiniband/sw/rxe/rxe_resp.c
@@ -536,7 +536,7 @@ static enum resp_states send_data_in(struct rxe_qp *qp, void *data_addr,
 	int err;
 
 	err = copy_data(qp->pd, IB_ACCESS_LOCAL_WRITE, &qp->resp.wqe->dma,
-			data_addr, data_len, RXE_TO_MR_OBJ, NULL);
+			data_addr, data_len, RXE_TO_MR_OBJ);
 	if (unlikely(err))
 		return (err == -ENOSPC) ? RESPST_ERR_LENGTH
 					: RESPST_ERR_MALFORMED_WQE;
@@ -552,7 +552,7 @@ static enum resp_states write_data_in(struct rxe_qp *qp,
 	int data_len = payload_size(pkt);
 
 	err = rxe_mr_copy(qp->resp.mr, qp->resp.va + qp->resp.offset,
-			  payload_addr(pkt), data_len, RXE_TO_MR_OBJ, NULL);
+			  payload_addr(pkt), data_len, RXE_TO_MR_OBJ);
 	if (err) {
 		rc = RESPST_ERR_RKEY_VIOLATION;
 		goto out;
@@ -613,13 +613,10 @@ static struct sk_buff *prepare_ack_packet(struct rxe_qp *qp,
 					  int opcode,
 					  int payload,
 					  u32 psn,
-					  u8 syndrome,
-					  u32 *crcp)
+					  u8 syndrome)
 {
 	struct rxe_dev *rxe = to_rdev(qp->ibqp.device);
 	struct sk_buff *skb;
-	u32 crc = 0;
-	u32 *p;
 	int paylen;
 	int pad;
 	int err;
@@ -651,20 +648,12 @@ static struct sk_buff *prepare_ack_packet(struct rxe_qp *qp,
 	if (ack->mask & RXE_ATMACK_MASK)
 		atmack_set_orig(ack, qp->resp.atomic_orig);
 
-	err = rxe_prepare(ack, skb, &crc);
+	err = rxe_prepare(ack, skb);
 	if (err) {
 		kfree_skb(skb);
 		return NULL;
 	}
 
-	if (crcp) {
-		/* CRC computation will be continued by the caller */
-		*crcp = crc;
-	} else {
-		p = payload_addr(ack) + payload + bth_pad(ack);
-		*p = ~crc;
-	}
-
 	return skb;
 }
 
@@ -682,8 +671,6 @@ static enum resp_states read_reply(struct rxe_qp *qp,
 	int opcode;
 	int err;
 	struct resp_res *res = qp->resp.res;
-	u32 icrc;
-	u32 *p;
 
 	if (!res) {
 		/* This is the first time we process that request. Get a
@@ -742,24 +729,20 @@ static enum resp_states read_reply(struct rxe_qp *qp,
 	payload = min_t(int, res->read.resid, mtu);
 
 	skb = prepare_ack_packet(qp, req_pkt, &ack_pkt, opcode, payload,
-				 res->cur_psn, AETH_ACK_UNLIMITED, &icrc);
+				 res->cur_psn, AETH_ACK_UNLIMITED);
 	if (!skb)
 		return RESPST_ERR_RNR;
 
 	err = rxe_mr_copy(res->read.mr, res->read.va, payload_addr(&ack_pkt),
-			  payload, RXE_FROM_MR_OBJ, &icrc);
+			  payload, RXE_FROM_MR_OBJ);
 	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;
 
 	err = rxe_xmit_packet(qp, &ack_pkt, skb);
 	if (err) {
@@ -984,7 +967,7 @@ static int send_ack(struct rxe_qp *qp, struct rxe_pkt_info *pkt,
 	struct sk_buff *skb;
 
 	skb = prepare_ack_packet(qp, pkt, &ack_pkt, IB_OPCODE_RC_ACKNOWLEDGE,
-				 0, psn, syndrome, NULL);
+				 0, psn, syndrome);
 	if (!skb) {
 		err = -ENOMEM;
 		goto err1;
@@ -1008,7 +991,7 @@ static int send_atomic_ack(struct rxe_qp *qp, struct rxe_pkt_info *pkt,
 
 	skb = prepare_ack_packet(qp, pkt, &ack_pkt,
 				 IB_OPCODE_RC_ATOMIC_ACKNOWLEDGE, 0, pkt->psn,
-				 syndrome, NULL);
+				 syndrome);
 	if (!skb) {
 		rc = -ENOMEM;
 		goto out;
-- 
2.30.2


^ permalink raw reply related	[flat|nested] 24+ messages in thread
* [PATCH v2 5/9] RDMA/rxe: Move rxe_crc32 to a subroutine
  2021-07-07  4:00 [PATCH for-next v2 0/9] ICRC cleanup Bob Pearson
                   ` (3 preceding siblings ...)
  2021-07-07  4:00 ` [PATCH v2 4/9] RDMA/rxe: Move ICRC generation to a subroutine Bob Pearson
@ 2021-07-07  4:00 ` Bob Pearson
  2021-07-07  4:00 ` [PATCH v2 6/9] RDMA/rxe: Fixup rxe_icrc_hdr Bob Pearson
                   ` (5 subsequent siblings)
  10 siblings, 0 replies; 24+ messages in thread
From: Bob Pearson @ 2021-07-07  4:00 UTC (permalink / raw)
  To: jgg, zyjzyj2000, linux-rdma; +Cc: Bob Pearson

Move rxe_crc32() from rxe.h to rxe_icrc.c as a static local function.

Signed-off-by: Bob Pearson <rpearsonhpe@gmail.com>
---
 drivers/infiniband/sw/rxe/rxe.h      | 21 ---------------------
 drivers/infiniband/sw/rxe/rxe_icrc.c | 21 +++++++++++++++++++++
 2 files changed, 21 insertions(+), 21 deletions(-)

diff --git a/drivers/infiniband/sw/rxe/rxe.h b/drivers/infiniband/sw/rxe/rxe.h
index 623fd17df02d..65a73c1c8b35 100644
--- a/drivers/infiniband/sw/rxe/rxe.h
+++ b/drivers/infiniband/sw/rxe/rxe.h
@@ -42,27 +42,6 @@
 
 extern bool rxe_initialized;
 
-static inline u32 rxe_crc32(struct rxe_dev *rxe,
-			    u32 crc, void *next, size_t len)
-{
-	u32 retval;
-	int err;
-
-	SHASH_DESC_ON_STACK(shash, rxe->tfm);
-
-	shash->tfm = rxe->tfm;
-	*(u32 *)shash_desc_ctx(shash) = crc;
-	err = crypto_shash_update(shash, next, len);
-	if (unlikely(err)) {
-		pr_warn_ratelimited("failed crc calculation, err: %d\n", err);
-		return crc32_le(crc, next, len);
-	}
-
-	retval = *(u32 *)shash_desc_ctx(shash);
-	barrier_data(shash_desc_ctx(shash));
-	return retval;
-}
-
 void rxe_set_mtu(struct rxe_dev *rxe, unsigned int dev_mtu);
 
 int rxe_add(struct rxe_dev *rxe, unsigned int mtu, const char *ibdev_name);
diff --git a/drivers/infiniband/sw/rxe/rxe_icrc.c b/drivers/infiniband/sw/rxe/rxe_icrc.c
index 08ab32eb6445..00916440f17b 100644
--- a/drivers/infiniband/sw/rxe/rxe_icrc.c
+++ b/drivers/infiniband/sw/rxe/rxe_icrc.c
@@ -7,6 +7,27 @@
 #include "rxe.h"
 #include "rxe_loc.h"
 
+static u32 rxe_crc32(struct rxe_dev *rxe, u32 crc, void *next, size_t len)
+{
+	u32 icrc;
+	int err;
+
+	SHASH_DESC_ON_STACK(shash, rxe->tfm);
+
+	shash->tfm = rxe->tfm;
+	*(u32 *)shash_desc_ctx(shash) = crc;
+	err = crypto_shash_update(shash, next, len);
+	if (unlikely(err)) {
+		pr_warn_ratelimited("failed crc calculation, err: %d\n", err);
+		return crc32_le(crc, next, len);
+	}
+
+	icrc = *(u32 *)shash_desc_ctx(shash);
+	barrier_data(shash_desc_ctx(shash));
+
+	return icrc;
+}
+
 /* Compute a partial ICRC for all the IB transport headers. */
 u32 rxe_icrc_hdr(struct rxe_pkt_info *pkt, struct sk_buff *skb)
 {
-- 
2.30.2


^ permalink raw reply related	[flat|nested] 24+ messages in thread
* [PATCH v2 6/9] RDMA/rxe: Fixup rxe_icrc_hdr
  2021-07-07  4:00 [PATCH for-next v2 0/9] ICRC cleanup Bob Pearson
                   ` (4 preceding siblings ...)
  2021-07-07  4:00 ` [PATCH v2 5/9] RDMA/rxe: Move rxe_crc32 " Bob Pearson
@ 2021-07-07  4:00 ` Bob Pearson
  2021-07-07  4:00 ` [PATCH v2 7/9] RDMA/rxe: Move crc32 init code to rxe_icrc.c Bob Pearson
                   ` (4 subsequent siblings)
  10 siblings, 0 replies; 24+ messages in thread
From: Bob Pearson @ 2021-07-07  4:00 UTC (permalink / raw)
  To: jgg, zyjzyj2000, linux-rdma; +Cc: Bob Pearson

rxe_icrc_hdr() in rxe_icrc.c is no longer shared. This patch makes it
static and changes the parameter list to match the other routines there.

Signed-off-by: Bob Pearson <rpearsonhpe@gmail.com>
---
 drivers/infiniband/sw/rxe/rxe_icrc.c | 6 +++---
 drivers/infiniband/sw/rxe/rxe_loc.h  | 1 -
 2 files changed, 3 insertions(+), 4 deletions(-)

diff --git a/drivers/infiniband/sw/rxe/rxe_icrc.c b/drivers/infiniband/sw/rxe/rxe_icrc.c
index 00916440f17b..777199517e9a 100644
--- a/drivers/infiniband/sw/rxe/rxe_icrc.c
+++ b/drivers/infiniband/sw/rxe/rxe_icrc.c
@@ -29,7 +29,7 @@ static u32 rxe_crc32(struct rxe_dev *rxe, u32 crc, void *next, size_t len)
 }
 
 /* Compute a partial ICRC for all the IB transport headers. */
-u32 rxe_icrc_hdr(struct rxe_pkt_info *pkt, struct sk_buff *skb)
+static u32 rxe_icrc_hdr(struct sk_buff *skb, struct rxe_pkt_info *pkt)
 {
 	unsigned int bth_offset = 0;
 	struct iphdr *ip4h = NULL;
@@ -106,7 +106,7 @@ int rxe_icrc_check(struct sk_buff *skb, struct rxe_pkt_info *pkt)
 	icrcp = (__be32 *)(pkt->hdr + pkt->paylen - RXE_ICRC_SIZE);
 	pkt_icrc = be32_to_cpu(*icrcp);
 
-	icrc = rxe_icrc_hdr(pkt, skb);
+	icrc = rxe_icrc_hdr(skb, pkt);
 	icrc = rxe_crc32(pkt->rxe, icrc, (u8 *)payload_addr(pkt),
 				payload_size(pkt) + bth_pad(pkt));
 	icrc = (__force u32)cpu_to_be32(~icrc);
@@ -134,7 +134,7 @@ void rxe_icrc_generate(struct sk_buff *skb, struct rxe_pkt_info *pkt)
 	u32 icrc;
 
 	icrcp = (__be32 *)(pkt->hdr + pkt->paylen - RXE_ICRC_SIZE);
-	icrc = rxe_icrc_hdr(pkt, skb);
+	icrc = rxe_icrc_hdr(skb, pkt);
 	icrc = rxe_crc32(pkt->rxe, icrc, (u8 *)payload_addr(pkt),
 				payload_size(pkt) + bth_pad(pkt));
 	*icrcp = (__force __be32)~icrc;
diff --git a/drivers/infiniband/sw/rxe/rxe_loc.h b/drivers/infiniband/sw/rxe/rxe_loc.h
index a832535fa35a..73a2c48a3160 100644
--- a/drivers/infiniband/sw/rxe/rxe_loc.h
+++ b/drivers/infiniband/sw/rxe/rxe_loc.h
@@ -193,7 +193,6 @@ int rxe_requester(void *arg);
 int rxe_responder(void *arg);
 
 /* rxe_icrc.c */
-u32 rxe_icrc_hdr(struct rxe_pkt_info *pkt, struct sk_buff *skb);
 int rxe_icrc_check(struct sk_buff *skb, struct rxe_pkt_info *pkt);
 void rxe_icrc_generate(struct sk_buff *skb, struct rxe_pkt_info *pkt);
 
-- 
2.30.2


^ permalink raw reply related	[flat|nested] 24+ messages in thread
* [PATCH v2 7/9] RDMA/rxe: Move crc32 init code to rxe_icrc.c
  2021-07-07  4:00 [PATCH for-next v2 0/9] ICRC cleanup Bob Pearson
                   ` (5 preceding siblings ...)
  2021-07-07  4:00 ` [PATCH v2 6/9] RDMA/rxe: Fixup rxe_icrc_hdr Bob Pearson
@ 2021-07-07  4:00 ` Bob Pearson
  2021-07-07  4:00 ` [PATCH v2 8/9] RDMA/rxe: Add kernel-doc comments " Bob Pearson
                   ` (3 subsequent siblings)
  10 siblings, 0 replies; 24+ messages in thread
From: Bob Pearson @ 2021-07-07  4:00 UTC (permalink / raw)
  To: jgg, zyjzyj2000, linux-rdma; +Cc: Bob Pearson

This patch collects the code from rxe_register_device() that sets
up the crc32 calculation into a subroutine rxe_icrc_init() in
rxe_icrc.c.

Signed-off-by: Bob Pearson <rpearsonhpe@gmail.com>
---
 drivers/infiniband/sw/rxe/rxe.h       |  1 -
 drivers/infiniband/sw/rxe/rxe_icrc.c  | 18 ++++++++++++++++++
 drivers/infiniband/sw/rxe/rxe_loc.h   |  1 +
 drivers/infiniband/sw/rxe/rxe_verbs.c | 11 +++--------
 4 files changed, 22 insertions(+), 9 deletions(-)

diff --git a/drivers/infiniband/sw/rxe/rxe.h b/drivers/infiniband/sw/rxe/rxe.h
index 65a73c1c8b35..1bb3fb618bf5 100644
--- a/drivers/infiniband/sw/rxe/rxe.h
+++ b/drivers/infiniband/sw/rxe/rxe.h
@@ -14,7 +14,6 @@
 
 #include <linux/module.h>
 #include <linux/skbuff.h>
-#include <linux/crc32.h>
 
 #include <rdma/ib_verbs.h>
 #include <rdma/ib_user_verbs.h>
diff --git a/drivers/infiniband/sw/rxe/rxe_icrc.c b/drivers/infiniband/sw/rxe/rxe_icrc.c
index 777199517e9a..62bcdfc8e96a 100644
--- a/drivers/infiniband/sw/rxe/rxe_icrc.c
+++ b/drivers/infiniband/sw/rxe/rxe_icrc.c
@@ -4,9 +4,27 @@
  * Copyright (c) 2015 System Fabric Works, Inc. All rights reserved.
  */
 
+#include <linux/crc32.h>
+
 #include "rxe.h"
 #include "rxe_loc.h"
 
+int rxe_icrc_init(struct rxe_dev *rxe)
+{
+	struct crypto_shash *tfm;
+
+	tfm = crypto_alloc_shash("crc32", 0, 0);
+	if (IS_ERR(tfm)) {
+		pr_warn("failed to init crc32 algorithm err:%ld\n",
+			       PTR_ERR(tfm));
+		return PTR_ERR(tfm);
+	}
+
+	rxe->tfm = tfm;
+
+	return 0;
+}
+
 static u32 rxe_crc32(struct rxe_dev *rxe, u32 crc, void *next, size_t len)
 {
 	u32 icrc;
diff --git a/drivers/infiniband/sw/rxe/rxe_loc.h b/drivers/infiniband/sw/rxe/rxe_loc.h
index 73a2c48a3160..f0c954575bde 100644
--- a/drivers/infiniband/sw/rxe/rxe_loc.h
+++ b/drivers/infiniband/sw/rxe/rxe_loc.h
@@ -193,6 +193,7 @@ int rxe_requester(void *arg);
 int rxe_responder(void *arg);
 
 /* rxe_icrc.c */
+int rxe_icrc_init(struct rxe_dev *rxe);
 int rxe_icrc_check(struct sk_buff *skb, struct rxe_pkt_info *pkt);
 void rxe_icrc_generate(struct sk_buff *skb, struct rxe_pkt_info *pkt);
 
diff --git a/drivers/infiniband/sw/rxe/rxe_verbs.c b/drivers/infiniband/sw/rxe/rxe_verbs.c
index c223959ac174..f7b1a1f64c13 100644
--- a/drivers/infiniband/sw/rxe/rxe_verbs.c
+++ b/drivers/infiniband/sw/rxe/rxe_verbs.c
@@ -1154,7 +1154,6 @@ int rxe_register_device(struct rxe_dev *rxe, const char *ibdev_name)
 {
 	int err;
 	struct ib_device *dev = &rxe->ib_dev;
-	struct crypto_shash *tfm;
 
 	strscpy(dev->node_desc, "rxe", sizeof(dev->node_desc));
 
@@ -1173,13 +1172,9 @@ int rxe_register_device(struct rxe_dev *rxe, const char *ibdev_name)
 	if (err)
 		return err;
 
-	tfm = crypto_alloc_shash("crc32", 0, 0);
-	if (IS_ERR(tfm)) {
-		pr_err("failed to allocate crc algorithm err:%ld\n",
-		       PTR_ERR(tfm));
-		return PTR_ERR(tfm);
-	}
-	rxe->tfm = tfm;
+	err = rxe_icrc_init(rxe);
+	if (err)
+		return err;
 
 	err = ib_register_device(dev, ibdev_name, NULL);
 	if (err)
-- 
2.30.2


^ permalink raw reply related	[flat|nested] 24+ messages in thread
* [PATCH v2 8/9] RDMA/rxe: Add kernel-doc comments to rxe_icrc.c
  2021-07-07  4:00 [PATCH for-next v2 0/9] ICRC cleanup Bob Pearson
                   ` (6 preceding siblings ...)
  2021-07-07  4:00 ` [PATCH v2 7/9] RDMA/rxe: Move crc32 init code to rxe_icrc.c Bob Pearson
@ 2021-07-07  4:00 ` Bob Pearson
  2021-07-07  4:00 ` [PATCH v2 9/9] RDMA/rxe: Fix types in rxe_icrc.c Bob Pearson
                   ` (2 subsequent siblings)
  10 siblings, 0 replies; 24+ messages in thread
From: Bob Pearson @ 2021-07-07  4:00 UTC (permalink / raw)
  To: jgg, zyjzyj2000, linux-rdma; +Cc: Bob Pearson

This patch adds kernel-doc style comments to rxe_icrc.c

Signed-off-by: Bob Pearson <rpearsonhpe@gmail.com>
---
 drivers/infiniband/sw/rxe/rxe_icrc.c | 32 +++++++++++++++++++++++++---
 1 file changed, 29 insertions(+), 3 deletions(-)

diff --git a/drivers/infiniband/sw/rxe/rxe_icrc.c b/drivers/infiniband/sw/rxe/rxe_icrc.c
index 62bcdfc8e96a..4473d38c171f 100644
--- a/drivers/infiniband/sw/rxe/rxe_icrc.c
+++ b/drivers/infiniband/sw/rxe/rxe_icrc.c
@@ -9,6 +9,12 @@
 #include "rxe.h"
 #include "rxe_loc.h"
 
+/**
+ * rxe_icrc_init() - Initialize crypto function for computing crc32
+ * @rxe: rdma_rxe device object
+ *
+ * Return: 0 on success else an error
+ */
 int rxe_icrc_init(struct rxe_dev *rxe)
 {
 	struct crypto_shash *tfm;
@@ -25,6 +31,15 @@ int rxe_icrc_init(struct rxe_dev *rxe)
 	return 0;
 }
 
+/**
+ * rxe_crc32() - Compute cumulative crc32 for a contiguous segment
+ * @rxe: rdma_rxe device object
+ * @crc: starting crc32 value from previous segments
+ * @next: starting address of current segment
+ * @len: length of current segment
+ *
+ * Return: the cumulative crc32 checksum
+ */
 static u32 rxe_crc32(struct rxe_dev *rxe, u32 crc, void *next, size_t len)
 {
 	u32 icrc;
@@ -46,7 +61,14 @@ static u32 rxe_crc32(struct rxe_dev *rxe, u32 crc, void *next, size_t len)
 	return icrc;
 }
 
-/* Compute a partial ICRC for all the IB transport headers. */
+/**
+ * rxe_icrc_hdr() - Compute the partial ICRC for the network and transport
+ *		  headers of a packet.
+ * @skb: packet buffer
+ * @pkt: packet information
+ *
+ * Return: the partial ICRC
+ */
 static u32 rxe_icrc_hdr(struct sk_buff *skb, struct rxe_pkt_info *pkt)
 {
 	unsigned int bth_offset = 0;
@@ -111,7 +133,7 @@ static u32 rxe_icrc_hdr(struct sk_buff *skb, struct rxe_pkt_info *pkt)
  * rxe_icrc_check() - Compute ICRC for a packet and compare to the ICRC
  *		      delivered in the packet.
  * @skb: packet buffer
- * @pkt: packet info
+ * @pkt: packet information
  *
  * Return: 0 if the values match else an error
  */
@@ -145,7 +167,11 @@ int rxe_icrc_check(struct sk_buff *skb, struct rxe_pkt_info *pkt)
 	return 0;
 }
 
-/* rxe_icrc_generate- compute ICRC for a packet. */
+/**
+ * rxe_icrc_generate() - compute ICRC for a packet.
+ * @skb: packet buffer
+ * @pkt: packet information
+ */
 void rxe_icrc_generate(struct sk_buff *skb, struct rxe_pkt_info *pkt)
 {
 	__be32 *icrcp;
-- 
2.30.2


^ permalink raw reply related	[flat|nested] 24+ messages in thread
* [PATCH v2 9/9] RDMA/rxe: Fix types in rxe_icrc.c
  2021-07-07  4:00 [PATCH for-next v2 0/9] ICRC cleanup Bob Pearson
                   ` (7 preceding siblings ...)
  2021-07-07  4:00 ` [PATCH v2 8/9] RDMA/rxe: Add kernel-doc comments " Bob Pearson
@ 2021-07-07  4:00 ` Bob Pearson
  2021-07-16 16:06   ` Jason Gunthorpe
  2021-07-08  7:36 ` [PATCH for-next v2 0/9] ICRC cleanup Zhu Yanjun
  2021-07-16 17:38 ` Jason Gunthorpe
  10 siblings, 1 reply; 24+ messages in thread
From: Bob Pearson @ 2021-07-07  4:00 UTC (permalink / raw)
  To: jgg, zyjzyj2000, linux-rdma; +Cc: Bob Pearson

Currently the ICRC is generated as a u32 type and then forced to a __be32
and stored into the ICRC field in the packet. The actual type of the ICRC
is __be32. This patch replaces u32 by __be32 and eliminates the casts.
The computation is exactly the same as the original but the types are
more consistent.

Signed-off-by: Bob Pearson <rpearsonhpe@gmail.com>
---
 drivers/infiniband/sw/rxe/rxe_icrc.c | 28 ++++++++++++++--------------
 1 file changed, 14 insertions(+), 14 deletions(-)

diff --git a/drivers/infiniband/sw/rxe/rxe_icrc.c b/drivers/infiniband/sw/rxe/rxe_icrc.c
index 4473d38c171f..e03af3012590 100644
--- a/drivers/infiniband/sw/rxe/rxe_icrc.c
+++ b/drivers/infiniband/sw/rxe/rxe_icrc.c
@@ -40,22 +40,22 @@ int rxe_icrc_init(struct rxe_dev *rxe)
  *
  * Return: the cumulative crc32 checksum
  */
-static u32 rxe_crc32(struct rxe_dev *rxe, u32 crc, void *next, size_t len)
+static __be32 rxe_crc32(struct rxe_dev *rxe, __be32 crc, void *next, size_t len)
 {
-	u32 icrc;
+	__be32 icrc;
 	int err;
 
 	SHASH_DESC_ON_STACK(shash, rxe->tfm);
 
 	shash->tfm = rxe->tfm;
-	*(u32 *)shash_desc_ctx(shash) = crc;
+	*(__be32 *)shash_desc_ctx(shash) = crc;
 	err = crypto_shash_update(shash, next, len);
 	if (unlikely(err)) {
 		pr_warn_ratelimited("failed crc calculation, err: %d\n", err);
-		return crc32_le(crc, next, len);
+		return (__force __be32)crc32_le((__force u32)crc, next, len);
 	}
 
-	icrc = *(u32 *)shash_desc_ctx(shash);
+	icrc = *(__be32 *)shash_desc_ctx(shash);
 	barrier_data(shash_desc_ctx(shash));
 
 	return icrc;
@@ -69,14 +69,14 @@ static u32 rxe_crc32(struct rxe_dev *rxe, u32 crc, void *next, size_t len)
  *
  * Return: the partial ICRC
  */
-static u32 rxe_icrc_hdr(struct sk_buff *skb, struct rxe_pkt_info *pkt)
+static __be32 rxe_icrc_hdr(struct sk_buff *skb, struct rxe_pkt_info *pkt)
 {
 	unsigned int bth_offset = 0;
 	struct iphdr *ip4h = NULL;
 	struct ipv6hdr *ip6h = NULL;
 	struct udphdr *udph;
 	struct rxe_bth *bth;
-	int crc;
+	__be32 crc;
 	int length;
 	int hdr_size = sizeof(struct udphdr) +
 		(skb->protocol == htons(ETH_P_IP) ?
@@ -91,7 +91,7 @@ static u32 rxe_icrc_hdr(struct sk_buff *skb, struct rxe_pkt_info *pkt)
 	/* This seed is the result of computing a CRC with a seed of
 	 * 0xfffffff and 8 bytes of 0xff representing a masked LRH.
 	 */
-	crc = 0xdebb20e3;
+	crc = (__force __be32)0xdebb20e3;
 
 	if (skb->protocol == htons(ETH_P_IP)) { /* IPv4 */
 		memcpy(pshdr, ip_hdr(skb), hdr_size);
@@ -140,16 +140,16 @@ static u32 rxe_icrc_hdr(struct sk_buff *skb, struct rxe_pkt_info *pkt)
 int rxe_icrc_check(struct sk_buff *skb, struct rxe_pkt_info *pkt)
 {
 	__be32 *icrcp;
-	u32 pkt_icrc;
-	u32 icrc;
+	__be32 pkt_icrc;
+	__be32 icrc;
 
 	icrcp = (__be32 *)(pkt->hdr + pkt->paylen - RXE_ICRC_SIZE);
-	pkt_icrc = be32_to_cpu(*icrcp);
+	pkt_icrc = *icrcp;
 
 	icrc = rxe_icrc_hdr(skb, pkt);
 	icrc = rxe_crc32(pkt->rxe, icrc, (u8 *)payload_addr(pkt),
 				payload_size(pkt) + bth_pad(pkt));
-	icrc = (__force u32)cpu_to_be32(~icrc);
+	icrc = ~icrc;
 
 	if (unlikely(icrc != pkt_icrc)) {
 		if (skb->protocol == htons(ETH_P_IPV6))
@@ -175,11 +175,11 @@ int rxe_icrc_check(struct sk_buff *skb, struct rxe_pkt_info *pkt)
 void rxe_icrc_generate(struct sk_buff *skb, struct rxe_pkt_info *pkt)
 {
 	__be32 *icrcp;
-	u32 icrc;
+	__be32 icrc;
 
 	icrcp = (__be32 *)(pkt->hdr + pkt->paylen - RXE_ICRC_SIZE);
 	icrc = rxe_icrc_hdr(skb, pkt);
 	icrc = rxe_crc32(pkt->rxe, icrc, (u8 *)payload_addr(pkt),
 				payload_size(pkt) + bth_pad(pkt));
-	*icrcp = (__force __be32)~icrc;
+	*icrcp = ~icrc;
 }
-- 
2.30.2


^ permalink raw reply related	[flat|nested] 24+ messages in thread
* Re: [PATCH for-next v2 0/9] ICRC cleanup
  2021-07-07  4:00 [PATCH for-next v2 0/9] ICRC cleanup Bob Pearson
                   ` (8 preceding siblings ...)
  2021-07-07  4:00 ` [PATCH v2 9/9] RDMA/rxe: Fix types in rxe_icrc.c Bob Pearson
@ 2021-07-08  7:36 ` Zhu Yanjun
  2021-07-16 17:38 ` Jason Gunthorpe
  10 siblings, 0 replies; 24+ messages in thread
From: Zhu Yanjun @ 2021-07-08  7:36 UTC (permalink / raw)
  To: Bob Pearson; +Cc: Jason Gunthorpe, RDMA mailing list

On Wed, Jul 7, 2021 at 12:01 PM Bob Pearson <rpearsonhpe@gmail.com> wrote:
>
> This series of patches is a cleanup of the ICRC code in the rxe driver.
> The end result is to collect all the ICRC focused code into rxe_icrc.c
> with three APIs that are used by the rest of the driver. One to initialize
> the crypto engine used to compute crc32, and one each to generate and
> check the ICRC in a packet.
>
> Signed-off-by: Bob Pearson <rpearsonhpe@gmail.com>
> ---
> v2:
>   Split up the 5 patches in the first version into 9 patches which are
>   each focused on a single change.
>   Fixed sparse warnings in the first version.
>
> Bob Pearson (9):
>   RDMA/rxe: Move ICRC checking to a subroutine
>   RDMA/rxe: Move rxe_xmit_packet to a subroutine
>   RDMA/rxe: Fixup rxe_send and rxe_loopback
>   RDMA/rxe: Move ICRC generation to a subroutine
>   RDMA/rxe: Move rxe_crc32 to a subroutine
>   RDMA/rxe: Fixup rxe_icrc_hdr
>   RDMA/rxe: Move crc32 init code to rxe_icrc.c
>   RDMA/rxe: Add kernel-doc comments to rxe_icrc.c
>   RDMA/rxe: Fix types in rxe_icrc.c

Thanks. It seems that these patches can pass rdma-core tests.
I will delve into these patches soon.

Zhu Yanjun

>
>  drivers/infiniband/sw/rxe/rxe.h       |  22 -----
>  drivers/infiniband/sw/rxe/rxe_comp.c  |   4 +-
>  drivers/infiniband/sw/rxe/rxe_icrc.c  | 124 +++++++++++++++++++++++++-
>  drivers/infiniband/sw/rxe/rxe_loc.h   |  61 +++----------
>  drivers/infiniband/sw/rxe/rxe_mr.c    |  22 +----
>  drivers/infiniband/sw/rxe/rxe_net.c   |  59 ++++++++++--
>  drivers/infiniband/sw/rxe/rxe_recv.c  |  23 +----
>  drivers/infiniband/sw/rxe/rxe_req.c   |  13 +--
>  drivers/infiniband/sw/rxe/rxe_resp.c  |  33 ++-----
>  drivers/infiniband/sw/rxe/rxe_verbs.c |  11 +--
>  10 files changed, 202 insertions(+), 170 deletions(-)
>
> --
> 2.30.2
>

^ permalink raw reply	[flat|nested] 24+ messages in thread
* Re: [PATCH v2 1/9] RDMA/rxe: Move ICRC checking to a subroutine
  2021-07-07  4:00 ` [PATCH v2 1/9] RDMA/rxe: Move ICRC checking to a subroutine Bob Pearson
@ 2021-07-13  7:27   ` Zhu Yanjun
  2021-07-16 15:55   ` Jason Gunthorpe
  1 sibling, 0 replies; 24+ messages in thread
From: Zhu Yanjun @ 2021-07-13  7:27 UTC (permalink / raw)
  To: Bob Pearson; +Cc: Jason Gunthorpe, RDMA mailing list

On Wed, Jul 7, 2021 at 12:01 PM Bob Pearson <rpearsonhpe@gmail.com> wrote:
>
> Move the code in rxe_recv() that checks the ICRC on incoming packets to a
> subroutine rxe_check_icrc() and move that to rxe_icrc.c.

In this commit, a new function rxe_icrc_check is created with the
original source code.

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

Zhu Yanjun

>
> Signed-off-by: Bob Pearson <rpearsonhpe@gmail.com>
> ---
>  drivers/infiniband/sw/rxe/rxe_icrc.c | 38 ++++++++++++++++++++++++++++
>  drivers/infiniband/sw/rxe/rxe_loc.h  |  2 ++
>  drivers/infiniband/sw/rxe/rxe_recv.c | 23 ++---------------
>  3 files changed, 42 insertions(+), 21 deletions(-)
>
> diff --git a/drivers/infiniband/sw/rxe/rxe_icrc.c b/drivers/infiniband/sw/rxe/rxe_icrc.c
> index 66b2aad54bb7..d067841214be 100644
> --- a/drivers/infiniband/sw/rxe/rxe_icrc.c
> +++ b/drivers/infiniband/sw/rxe/rxe_icrc.c
> @@ -67,3 +67,41 @@ u32 rxe_icrc_hdr(struct rxe_pkt_info *pkt, struct sk_buff *skb)
>                         rxe_opcode[pkt->opcode].length - RXE_BTH_BYTES);
>         return crc;
>  }
> +
> +/**
> + * rxe_icrc_check() - Compute ICRC for a packet and compare to the ICRC
> + *                   delivered in the packet.
> + * @skb: packet buffer
> + * @pkt: packet info
> + *
> + * Return: 0 if the values match else an error
> + */
> +int rxe_icrc_check(struct sk_buff *skb, struct rxe_pkt_info *pkt)
> +{
> +       __be32 *icrcp;
> +       u32 pkt_icrc;
> +       u32 icrc;
> +
> +       icrcp = (__be32 *)(pkt->hdr + pkt->paylen - RXE_ICRC_SIZE);
> +       pkt_icrc = be32_to_cpu(*icrcp);
> +
> +       icrc = rxe_icrc_hdr(pkt, skb);
> +       icrc = rxe_crc32(pkt->rxe, icrc, (u8 *)payload_addr(pkt),
> +                               payload_size(pkt) + bth_pad(pkt));
> +       icrc = (__force u32)cpu_to_be32(~icrc);
> +
> +       if (unlikely(icrc != pkt_icrc)) {
> +               if (skb->protocol == htons(ETH_P_IPV6))
> +                       pr_warn_ratelimited("bad ICRC from %pI6c\n",
> +                                           &ipv6_hdr(skb)->saddr);
> +               else if (skb->protocol == htons(ETH_P_IP))
> +                       pr_warn_ratelimited("bad ICRC from %pI4\n",
> +                                           &ip_hdr(skb)->saddr);
> +               else
> +                       pr_warn_ratelimited("bad ICRC from unknown\n");
> +
> +               return -EINVAL;
> +       }
> +
> +       return 0;
> +}
> diff --git a/drivers/infiniband/sw/rxe/rxe_loc.h b/drivers/infiniband/sw/rxe/rxe_loc.h
> index 1ddb20855dee..015777e31ec9 100644
> --- a/drivers/infiniband/sw/rxe/rxe_loc.h
> +++ b/drivers/infiniband/sw/rxe/rxe_loc.h
> @@ -193,7 +193,9 @@ int rxe_completer(void *arg);
>  int rxe_requester(void *arg);
>  int rxe_responder(void *arg);
>
> +/* rxe_icrc.c */
>  u32 rxe_icrc_hdr(struct rxe_pkt_info *pkt, struct sk_buff *skb);
> +int rxe_icrc_check(struct sk_buff *skb, struct rxe_pkt_info *pkt);
>
>  void rxe_resp_queue_pkt(struct rxe_qp *qp, struct sk_buff *skb);
>
> diff --git a/drivers/infiniband/sw/rxe/rxe_recv.c b/drivers/infiniband/sw/rxe/rxe_recv.c
> index 7a49e27da23a..6a6cc1fa90e4 100644
> --- a/drivers/infiniband/sw/rxe/rxe_recv.c
> +++ b/drivers/infiniband/sw/rxe/rxe_recv.c
> @@ -361,8 +361,6 @@ void rxe_rcv(struct sk_buff *skb)
>         int err;
>         struct rxe_pkt_info *pkt = SKB_TO_PKT(skb);
>         struct rxe_dev *rxe = pkt->rxe;
> -       __be32 *icrcp;
> -       u32 calc_icrc, pack_icrc;
>
>         if (unlikely(skb->len < RXE_BTH_BYTES))
>                 goto drop;
> @@ -384,26 +382,9 @@ void rxe_rcv(struct sk_buff *skb)
>         if (unlikely(err))
>                 goto drop;
>
> -       /* Verify ICRC */
> -       icrcp = (__be32 *)(pkt->hdr + pkt->paylen - RXE_ICRC_SIZE);
> -       pack_icrc = be32_to_cpu(*icrcp);
> -
> -       calc_icrc = rxe_icrc_hdr(pkt, skb);
> -       calc_icrc = rxe_crc32(rxe, calc_icrc, (u8 *)payload_addr(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))
> -                       pr_warn_ratelimited("bad ICRC from %pI6c\n",
> -                                           &ipv6_hdr(skb)->saddr);
> -               else if (skb->protocol == htons(ETH_P_IP))
> -                       pr_warn_ratelimited("bad ICRC from %pI4\n",
> -                                           &ip_hdr(skb)->saddr);
> -               else
> -                       pr_warn_ratelimited("bad ICRC from unknown\n");
> -
> +       err = rxe_icrc_check(skb, pkt);
> +       if (unlikely(err))
>                 goto drop;
> -       }
>
>         rxe_counter_inc(rxe, RXE_CNT_RCVD_PKTS);
>
> --
> 2.30.2
>

^ permalink raw reply	[flat|nested] 24+ messages in thread
* Re: [PATCH v2 2/9] RDMA/rxe: Move rxe_xmit_packet to a subroutine
  2021-07-07  4:00 ` [PATCH v2 2/9] RDMA/rxe: Move rxe_xmit_packet " Bob Pearson
@ 2021-07-14  3:24   ` Zhu Yanjun
  0 siblings, 0 replies; 24+ messages in thread
From: Zhu Yanjun @ 2021-07-14  3:24 UTC (permalink / raw)
  To: Bob Pearson; +Cc: Jason Gunthorpe, RDMA mailing list

On Wed, Jul 7, 2021 at 12:01 PM Bob Pearson <rpearsonhpe@gmail.com> wrote:
>
> rxe_xmit_packet() was an overlong inline subroutine. This patch moves it
> into rxe_net.c as an ordinary subroutine.

In this commit, the function rxe_xmit_packet is moved from rxe_loc.h
to rxe_net.c.
No other changes.

I am fine with this. Thanks.

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

Zhu Yanjun

>
> Signed-off-by: Bob Pearson <rpearsonhpe@gmail.com>
> ---
>  drivers/infiniband/sw/rxe/rxe_loc.h | 45 ++---------------------------
>  drivers/infiniband/sw/rxe/rxe_net.c | 43 +++++++++++++++++++++++++++
>  2 files changed, 45 insertions(+), 43 deletions(-)
>
> diff --git a/drivers/infiniband/sw/rxe/rxe_loc.h b/drivers/infiniband/sw/rxe/rxe_loc.h
> index 015777e31ec9..409d10f20948 100644
> --- a/drivers/infiniband/sw/rxe/rxe_loc.h
> +++ b/drivers/infiniband/sw/rxe/rxe_loc.h
> @@ -104,6 +104,8 @@ int rxe_send(struct rxe_pkt_info *pkt, struct sk_buff *skb);
>  struct sk_buff *rxe_init_packet(struct rxe_dev *rxe, struct rxe_av *av,
>                                 int paylen, struct rxe_pkt_info *pkt);
>  int rxe_prepare(struct rxe_pkt_info *pkt, struct sk_buff *skb, u32 *crc);
> +int rxe_xmit_packet(struct rxe_qp *qp, struct rxe_pkt_info *pkt,
> +                   struct sk_buff *skb);
>  const char *rxe_parent_name(struct rxe_dev *rxe, unsigned int port_num);
>  int rxe_mcast_add(struct rxe_dev *rxe, union ib_gid *mgid);
>  int rxe_mcast_delete(struct rxe_dev *rxe, union ib_gid *mgid);
> @@ -206,47 +208,4 @@ static inline unsigned int wr_opcode_mask(int opcode, struct rxe_qp *qp)
>         return rxe_wr_opcode_info[opcode].mask[qp->ibqp.qp_type];
>  }
>
> -static inline int rxe_xmit_packet(struct rxe_qp *qp, struct rxe_pkt_info *pkt,
> -                                 struct sk_buff *skb)
> -{
> -       int err;
> -       int is_request = pkt->mask & RXE_REQ_MASK;
> -       struct rxe_dev *rxe = to_rdev(qp->ibqp.device);
> -
> -       if ((is_request && (qp->req.state != QP_STATE_READY)) ||
> -           (!is_request && (qp->resp.state != QP_STATE_READY))) {
> -               pr_info("Packet dropped. QP is not in ready state\n");
> -               goto drop;
> -       }
> -
> -       if (pkt->mask & RXE_LOOPBACK_MASK) {
> -               memcpy(SKB_TO_PKT(skb), pkt, sizeof(*pkt));
> -               rxe_loopback(skb);
> -               err = 0;
> -       } else {
> -               err = rxe_send(pkt, skb);
> -       }
> -
> -       if (err) {
> -               rxe->xmit_errors++;
> -               rxe_counter_inc(rxe, RXE_CNT_SEND_ERR);
> -               return err;
> -       }
> -
> -       if ((qp_type(qp) != IB_QPT_RC) &&
> -           (pkt->mask & RXE_END_MASK)) {
> -               pkt->wqe->state = wqe_state_done;
> -               rxe_run_task(&qp->comp.task, 1);
> -       }
> -
> -       rxe_counter_inc(rxe, RXE_CNT_SENT_PKTS);
> -       goto done;
> -
> -drop:
> -       kfree_skb(skb);
> -       err = 0;
> -done:
> -       return err;
> -}
> -
>  #endif /* RXE_LOC_H */
> diff --git a/drivers/infiniband/sw/rxe/rxe_net.c b/drivers/infiniband/sw/rxe/rxe_net.c
> index dec92928a1cd..c93a379a1b28 100644
> --- a/drivers/infiniband/sw/rxe/rxe_net.c
> +++ b/drivers/infiniband/sw/rxe/rxe_net.c
> @@ -421,6 +421,49 @@ void rxe_loopback(struct sk_buff *skb)
>                 rxe_rcv(skb);
>  }
>
> +int rxe_xmit_packet(struct rxe_qp *qp, struct rxe_pkt_info *pkt,
> +                   struct sk_buff *skb)
> +{
> +       int err;
> +       int is_request = pkt->mask & RXE_REQ_MASK;
> +       struct rxe_dev *rxe = to_rdev(qp->ibqp.device);
> +
> +       if ((is_request && (qp->req.state != QP_STATE_READY)) ||
> +           (!is_request && (qp->resp.state != QP_STATE_READY))) {
> +               pr_info("Packet dropped. QP is not in ready state\n");
> +               goto drop;
> +       }
> +
> +       if (pkt->mask & RXE_LOOPBACK_MASK) {
> +               memcpy(SKB_TO_PKT(skb), pkt, sizeof(*pkt));
> +               rxe_loopback(skb);
> +               err = 0;
> +       } else {
> +               err = rxe_send(pkt, skb);
> +       }
> +
> +       if (err) {
> +               rxe->xmit_errors++;
> +               rxe_counter_inc(rxe, RXE_CNT_SEND_ERR);
> +               return err;
> +       }
> +
> +       if ((qp_type(qp) != IB_QPT_RC) &&
> +           (pkt->mask & RXE_END_MASK)) {
> +               pkt->wqe->state = wqe_state_done;
> +               rxe_run_task(&qp->comp.task, 1);
> +       }
> +
> +       rxe_counter_inc(rxe, RXE_CNT_SENT_PKTS);
> +       goto done;
> +
> +drop:
> +       kfree_skb(skb);
> +       err = 0;
> +done:
> +       return err;
> +}
> +
>  struct sk_buff *rxe_init_packet(struct rxe_dev *rxe, struct rxe_av *av,
>                                 int paylen, struct rxe_pkt_info *pkt)
>  {
> --
> 2.30.2
>

^ permalink raw reply	[flat|nested] 24+ messages in thread
* Re: [PATCH v2 1/9] RDMA/rxe: Move ICRC checking to a subroutine
  2021-07-07  4:00 ` [PATCH v2 1/9] RDMA/rxe: Move ICRC checking to a subroutine Bob Pearson
  2021-07-13  7:27   ` Zhu Yanjun
@ 2021-07-16 15:55   ` Jason Gunthorpe
  1 sibling, 0 replies; 24+ messages in thread
From: Jason Gunthorpe @ 2021-07-16 15:55 UTC (permalink / raw)
  To: Bob Pearson; +Cc: zyjzyj2000, linux-rdma

On Tue, Jul 06, 2021 at 11:00:33PM -0500, Bob Pearson wrote:
> Move the code in rxe_recv() that checks the ICRC on incoming packets to a
> subroutine rxe_check_icrc() and move that to rxe_icrc.c.
> 
> Signed-off-by: Bob Pearson <rpearsonhpe@gmail.com>
> Reviewed-by: Zhu Yanjun <zyjzyj2000@gmail.com>
>  drivers/infiniband/sw/rxe/rxe_icrc.c | 38 ++++++++++++++++++++++++++++
>  drivers/infiniband/sw/rxe/rxe_loc.h  |  2 ++
>  drivers/infiniband/sw/rxe/rxe_recv.c | 23 ++---------------
>  3 files changed, 42 insertions(+), 21 deletions(-)
> 
> diff --git a/drivers/infiniband/sw/rxe/rxe_icrc.c b/drivers/infiniband/sw/rxe/rxe_icrc.c
> index 66b2aad54bb7..d067841214be 100644
> +++ b/drivers/infiniband/sw/rxe/rxe_icrc.c
> @@ -67,3 +67,41 @@ u32 rxe_icrc_hdr(struct rxe_pkt_info *pkt, struct sk_buff *skb)
>  			rxe_opcode[pkt->opcode].length - RXE_BTH_BYTES);
>  	return crc;
>  }
> +
> +/**
> + * rxe_icrc_check() - Compute ICRC for a packet and compare to the ICRC
> + *		      delivered in the packet.
> + * @skb: packet buffer
> + * @pkt: packet info
> + *
> + * Return: 0 if the values match else an error
> + */
> +int rxe_icrc_check(struct sk_buff *skb, struct rxe_pkt_info *pkt)
> +{
> +	__be32 *icrcp;
> +	u32 pkt_icrc;
> +	u32 icrc;
> +
> +	icrcp = (__be32 *)(pkt->hdr + pkt->paylen - RXE_ICRC_SIZE);
> +	pkt_icrc = be32_to_cpu(*icrcp);
> +
> +	icrc = rxe_icrc_hdr(pkt, skb);
> +	icrc = rxe_crc32(pkt->rxe, icrc, (u8 *)payload_addr(pkt),
> +				payload_size(pkt) + bth_pad(pkt));
> +	icrc = (__force u32)cpu_to_be32(~icrc);

This isn't right, the return value of rxe_crc32 is a __b32 so icrc
here should be a __b32 too

> +	if (unlikely(icrc != pkt_icrc)) {

And this should be written

if (unlikely(be32_to_cpu(~icrc) != pkt_icrc)) {

Or, alternatively you can skip both of the be32_to_cpu's and just
directly compare the __be32 types

Also, I'm not sure the rxe_crc32 byteswapping is right either, the
crc32 shash implementation is storing a __le32 in the shash_desc_ctx()
but this code reads it out as a __be32 ? On BE that will gain a
byteswap, is it OK?

Jason

^ permalink raw reply	[flat|nested] 24+ messages in thread
* Re: [PATCH v2 4/9] RDMA/rxe: Move ICRC generation to a subroutine
  2021-07-07  4:00 ` [PATCH v2 4/9] RDMA/rxe: Move ICRC generation to a subroutine Bob Pearson
@ 2021-07-16 15:57   ` Jason Gunthorpe
  2021-07-16 16:08     ` Bob Pearson
  0 siblings, 1 reply; 24+ messages in thread
From: Jason Gunthorpe @ 2021-07-16 15:57 UTC (permalink / raw)
  To: Bob Pearson; +Cc: zyjzyj2000, linux-rdma

On Tue, Jul 06, 2021 at 11:00:36PM -0500, Bob Pearson wrote:

> +/* rxe_icrc_generate- compute ICRC for a packet. */
> +void rxe_icrc_generate(struct sk_buff *skb, struct rxe_pkt_info *pkt)
> +{
> +	__be32 *icrcp;
> +	u32 icrc;
> +
> +	icrcp = (__be32 *)(pkt->hdr + pkt->paylen - RXE_ICRC_SIZE);
> +	icrc = rxe_icrc_hdr(pkt, skb);
> +	icrc = rxe_crc32(pkt->rxe, icrc, (u8 *)payload_addr(pkt),
> +				payload_size(pkt) + bth_pad(pkt));
> +	*icrcp = (__force __be32)~icrc;
> +}

Same comment here, the u32 icrc should be a  __be32 because that is
what rxe_crc32 returns, no force

Jason

^ permalink raw reply	[flat|nested] 24+ messages in thread
* Re: [PATCH v2 9/9] RDMA/rxe: Fix types in rxe_icrc.c
  2021-07-07  4:00 ` [PATCH v2 9/9] RDMA/rxe: Fix types in rxe_icrc.c Bob Pearson
@ 2021-07-16 16:06   ` Jason Gunthorpe
  0 siblings, 0 replies; 24+ messages in thread
From: Jason Gunthorpe @ 2021-07-16 16:06 UTC (permalink / raw)
  To: Bob Pearson; +Cc: zyjzyj2000, linux-rdma

On Tue, Jul 06, 2021 at 11:00:41PM -0500, Bob Pearson wrote:
> Currently the ICRC is generated as a u32 type and then forced to a __be32
> and stored into the ICRC field in the packet. The actual type of the ICRC
> is __be32. This patch replaces u32 by __be32 and eliminates the casts.
> The computation is exactly the same as the original but the types are
> more consistent.
> 
> Signed-off-by: Bob Pearson <rpearsonhpe@gmail.com>
> ---
>  drivers/infiniband/sw/rxe/rxe_icrc.c | 28 ++++++++++++++--------------
>  1 file changed, 14 insertions(+), 14 deletions(-)

Oh, well Ok, it mostly gets fixed up here

>  	shash->tfm = rxe->tfm;
> -	*(u32 *)shash_desc_ctx(shash) = crc;
> +	*(__be32 *)shash_desc_ctx(shash) = crc;
>  	err = crypto_shash_update(shash, next, len);
>  	if (unlikely(err)) {
>  		pr_warn_ratelimited("failed crc calculation, err: %d\n", err);
> -		return crc32_le(crc, next, len);
> +		return (__force __be32)crc32_le((__force u32)crc, next, len);
>  	}

But all this makes my head ache, I'm skeptical it is OK, but isn't any
worse

> @@ -91,7 +91,7 @@ static u32 rxe_icrc_hdr(struct sk_buff *skb, struct rxe_pkt_info *pkt)
>  	/* This seed is the result of computing a CRC with a seed of
>  	 * 0xfffffff and 8 bytes of 0xff representing a masked LRH.
>  	 */
> -	crc = 0xdebb20e3;
> +	crc = (__force __be32)0xdebb20e3;

Eg should this be cpu_to_be32(0xe320bbde) ?

Hard to know without a BE system to check it out on

Jason

^ permalink raw reply	[flat|nested] 24+ messages in thread
* Re: [PATCH v2 4/9] RDMA/rxe: Move ICRC generation to a subroutine
  2021-07-16 15:57   ` Jason Gunthorpe
@ 2021-07-16 16:08     ` Bob Pearson
  2021-07-16 16:29       ` Jason Gunthorpe
  0 siblings, 1 reply; 24+ messages in thread
From: Bob Pearson @ 2021-07-16 16:08 UTC (permalink / raw)
  To: Jason Gunthorpe; +Cc: zyjzyj2000, linux-rdma

On 7/16/21 10:57 AM, Jason Gunthorpe wrote:
> On Tue, Jul 06, 2021 at 11:00:36PM -0500, Bob Pearson wrote:
> 
>> +/* rxe_icrc_generate- compute ICRC for a packet. */
>> +void rxe_icrc_generate(struct sk_buff *skb, struct rxe_pkt_info *pkt)
>> +{
>> +	__be32 *icrcp;
>> +	u32 icrc;
>> +
>> +	icrcp = (__be32 *)(pkt->hdr + pkt->paylen - RXE_ICRC_SIZE);
>> +	icrc = rxe_icrc_hdr(pkt, skb);
>> +	icrc = rxe_crc32(pkt->rxe, icrc, (u8 *)payload_addr(pkt),
>> +				payload_size(pkt) + bth_pad(pkt));
>> +	*icrcp = (__force __be32)~icrc;
>> +}
> 
> Same comment here, the u32 icrc should be a  __be32 because that is
> what rxe_crc32 returns, no force
> 
> Jason
> 

I agree. The last patch in the series tries to make sense of the byte order.
Here I was trying to take baby steps and just move the code without changing anything.
It makes the thing easier for Zhu to review because no logic changed just where the code is.
However as you point out it doesn't really make sense on the face of it. There isn't any
really good resolution because both the hardware and software versions of the crc32 calculation
are clearly labeled __le but they are stuffed into the ICRC which is clearly identified as __be.
The problem is that it works i.e. interoperates with ConnectX. I would love a conversation with one
of the IBA architects to resolve this.

Bob


^ permalink raw reply	[flat|nested] 24+ messages in thread
* Re: [PATCH v2 4/9] RDMA/rxe: Move ICRC generation to a subroutine
  2021-07-16 16:08     ` Bob Pearson
@ 2021-07-16 16:29       ` Jason Gunthorpe
  2021-07-16 16:38         ` Pearson, Robert B
  0 siblings, 1 reply; 24+ messages in thread
From: Jason Gunthorpe @ 2021-07-16 16:29 UTC (permalink / raw)
  To: Bob Pearson; +Cc: zyjzyj2000, linux-rdma

On Fri, Jul 16, 2021 at 11:08:42AM -0500, Bob Pearson wrote:
> On 7/16/21 10:57 AM, Jason Gunthorpe wrote:
> > On Tue, Jul 06, 2021 at 11:00:36PM -0500, Bob Pearson wrote:
> > 
> >> +/* rxe_icrc_generate- compute ICRC for a packet. */
> >> +void rxe_icrc_generate(struct sk_buff *skb, struct rxe_pkt_info *pkt)
> >> +{
> >> +	__be32 *icrcp;
> >> +	u32 icrc;
> >> +
> >> +	icrcp = (__be32 *)(pkt->hdr + pkt->paylen - RXE_ICRC_SIZE);
> >> +	icrc = rxe_icrc_hdr(pkt, skb);
> >> +	icrc = rxe_crc32(pkt->rxe, icrc, (u8 *)payload_addr(pkt),
> >> +				payload_size(pkt) + bth_pad(pkt));
> >> +	*icrcp = (__force __be32)~icrc;
> >> +}
> > 
> > Same comment here, the u32 icrc should be a  __be32 because that is
> > what rxe_crc32 returns, no force
> > 
> > Jason
> > 
> 
> I agree. The last patch in the series tries to make sense of the byte order.
> Here I was trying to take baby steps and just move the code without changing anything.
> It makes the thing easier for Zhu to review because no logic changed just where the code is.
> However as you point out it doesn't really make sense on the face of it. There isn't any
> really good resolution because both the hardware and software versions of the crc32 calculation
> are clearly labeled __le but they are stuffed into the ICRC which is clearly identified as __be.
> The problem is that it works i.e. interoperates with ConnectX. I would love a conversation with one
> of the IBA architects to resolve this.

CRC's are complicated. There are 2 ways to feed the bits into the
LSFR (left or right) and at least 4 ways to represent the output.

Depending on how you design the LSFR and the algorithm you inherently
get one of the combinations.

Since rxe is using crc_le, and works, it is somehow setup that the
input bits are in the right order but the output is reversed. So

  cpu_to_be32(byteswap(crc_le()))

Looks like the right equation.

On LE two byteswaps cancel and you an get away with naked casting. On
BE it looks like a swap will be missing?

SHASH adds an additional cpu_to_le32() hidden inside the crypto
code. That would make the expected sequence this:

  cpu_to_be32(byteswap(le32_to_cpu(cpu_to_le32(crc_le32())))

Now things look better. It is confusing because the bytes output by
the SHASH are called "LE", and for some versions of the crc32 they may
be, however for IBTA this memory is in what we'd call BE layout. So
just casting the memory image above to BE is always fine.

The above will generate 0 swaps on LE and 1 swap on BE, vs no swaps on
BE for the naked crc32_le() call.

Most likely this confusion is a defect in the design of the CRC that
is being papered over by swaps.

You'd have to get out a qemu running a be PPC/ARM to check it out
properly, but looks to me like the shash is working, the naked
crc32_le is missing a swap, and loading the initial non-zero/non-FF
constants is missing a swap.

Jason

^ permalink raw reply	[flat|nested] 24+ messages in thread
* RE: [PATCH v2 4/9] RDMA/rxe: Move ICRC generation to a subroutine
  2021-07-16 16:29       ` Jason Gunthorpe
@ 2021-07-16 16:38         ` Pearson, Robert B
  0 siblings, 0 replies; 24+ messages in thread
From: Pearson, Robert B @ 2021-07-16 16:38 UTC (permalink / raw)
  To: Jason Gunthorpe, Bob Pearson; +Cc: zyjzyj2000, linux-rdma

I know. If you look at <Linux>/lib/crc32.c, I'm the current author, but it is now replaced by the crypto engines.
It was a nightmare if I recall. -- Bob

-----Original Message-----
From: Jason Gunthorpe <jgg@nvidia.com> 
Sent: Friday, July 16, 2021 11:29 AM
To: Bob Pearson <rpearsonhpe@gmail.com>
Cc: zyjzyj2000@gmail.com; linux-rdma@vger.kernel.org
Subject: Re: [PATCH v2 4/9] RDMA/rxe: Move ICRC generation to a subroutine

On Fri, Jul 16, 2021 at 11:08:42AM -0500, Bob Pearson wrote:
> On 7/16/21 10:57 AM, Jason Gunthorpe wrote:
> > On Tue, Jul 06, 2021 at 11:00:36PM -0500, Bob Pearson wrote:
> > 
> >> +/* rxe_icrc_generate- compute ICRC for a packet. */ void 
> >> +rxe_icrc_generate(struct sk_buff *skb, struct rxe_pkt_info *pkt) {
> >> +	__be32 *icrcp;
> >> +	u32 icrc;
> >> +
> >> +	icrcp = (__be32 *)(pkt->hdr + pkt->paylen - RXE_ICRC_SIZE);
> >> +	icrc = rxe_icrc_hdr(pkt, skb);
> >> +	icrc = rxe_crc32(pkt->rxe, icrc, (u8 *)payload_addr(pkt),
> >> +				payload_size(pkt) + bth_pad(pkt));
> >> +	*icrcp = (__force __be32)~icrc;
> >> +}
> > 
> > Same comment here, the u32 icrc should be a  __be32 because that is 
> > what rxe_crc32 returns, no force
> > 
> > Jason
> > 
> 
> I agree. The last patch in the series tries to make sense of the byte order.
> Here I was trying to take baby steps and just move the code without changing anything.
> It makes the thing easier for Zhu to review because no logic changed just where the code is.
> However as you point out it doesn't really make sense on the face of 
> it. There isn't any really good resolution because both the hardware 
> and software versions of the crc32 calculation are clearly labeled __le but they are stuffed into the ICRC which is clearly identified as __be.
> The problem is that it works i.e. interoperates with ConnectX. I would 
> love a conversation with one of the IBA architects to resolve this.

CRC's are complicated. There are 2 ways to feed the bits into the LSFR (left or right) and at least 4 ways to represent the output.

Depending on how you design the LSFR and the algorithm you inherently get one of the combinations.

Since rxe is using crc_le, and works, it is somehow setup that the input bits are in the right order but the output is reversed. So

  cpu_to_be32(byteswap(crc_le()))

Looks like the right equation.

On LE two byteswaps cancel and you an get away with naked casting. On BE it looks like a swap will be missing?

SHASH adds an additional cpu_to_le32() hidden inside the crypto code. That would make the expected sequence this:

  cpu_to_be32(byteswap(le32_to_cpu(cpu_to_le32(crc_le32())))

Now things look better. It is confusing because the bytes output by the SHASH are called "LE", and for some versions of the crc32 they may be, however for IBTA this memory is in what we'd call BE layout. So just casting the memory image above to BE is always fine.

The above will generate 0 swaps on LE and 1 swap on BE, vs no swaps on BE for the naked crc32_le() call.

Most likely this confusion is a defect in the design of the CRC that is being papered over by swaps.

You'd have to get out a qemu running a be PPC/ARM to check it out properly, but looks to me like the shash is working, the naked crc32_le is missing a swap, and loading the initial non-zero/non-FF constants is missing a swap.

Jason

^ permalink raw reply	[flat|nested] 24+ messages in thread
* Re: [PATCH for-next v2 0/9] ICRC cleanup
  2021-07-07  4:00 [PATCH for-next v2 0/9] ICRC cleanup Bob Pearson
                   ` (9 preceding siblings ...)
  2021-07-08  7:36 ` [PATCH for-next v2 0/9] ICRC cleanup Zhu Yanjun
@ 2021-07-16 17:38 ` Jason Gunthorpe
  2021-07-19  8:42   ` Zhu Yanjun
  10 siblings, 1 reply; 24+ messages in thread
From: Jason Gunthorpe @ 2021-07-16 17:38 UTC (permalink / raw)
  To: Bob Pearson; +Cc: zyjzyj2000, linux-rdma

On Tue, Jul 06, 2021 at 11:00:32PM -0500, Bob Pearson wrote:
> This series of patches is a cleanup of the ICRC code in the rxe driver.
> The end result is to collect all the ICRC focused code into rxe_icrc.c
> with three APIs that are used by the rest of the driver. One to initialize
> the crypto engine used to compute crc32, and one each to generate and
> check the ICRC in a packet.
> 
> Signed-off-by: Bob Pearson <rpearsonhpe@gmail.com>
> ---
> v2:
>   Split up the 5 patches in the first version into 9 patches which are
>   each focused on a single change.
>   Fixed sparse warnings in the first version.
> 
> Bob Pearson (9):
>   RDMA/rxe: Move ICRC checking to a subroutine
>   RDMA/rxe: Move rxe_xmit_packet to a subroutine
>   RDMA/rxe: Fixup rxe_send and rxe_loopback
>   RDMA/rxe: Move ICRC generation to a subroutine
>   RDMA/rxe: Move rxe_crc32 to a subroutine
>   RDMA/rxe: Fixup rxe_icrc_hdr
>   RDMA/rxe: Move crc32 init code to rxe_icrc.c
>   RDMA/rxe: Add kernel-doc comments to rxe_icrc.c
>   RDMA/rxe: Fix types in rxe_icrc.c

Applied to for-next, thanks

Jason

^ permalink raw reply	[flat|nested] 24+ messages in thread
* Re: [PATCH for-next v2 0/9] ICRC cleanup
  2021-07-16 17:38 ` Jason Gunthorpe
@ 2021-07-19  8:42   ` Zhu Yanjun
  2021-07-19 15:53     ` Olga Kornievskaia
  0 siblings, 1 reply; 24+ messages in thread
From: Zhu Yanjun @ 2021-07-19  8:42 UTC (permalink / raw)
  To: Jason Gunthorpe; +Cc: Bob Pearson, RDMA mailing list

On Sat, Jul 17, 2021 at 1:38 AM Jason Gunthorpe <jgg@nvidia.com> wrote:
>
> On Tue, Jul 06, 2021 at 11:00:32PM -0500, Bob Pearson wrote:
> > This series of patches is a cleanup of the ICRC code in the rxe driver.
> > The end result is to collect all the ICRC focused code into rxe_icrc.c
> > with three APIs that are used by the rest of the driver. One to initialize
> > the crypto engine used to compute crc32, and one each to generate and
> > check the ICRC in a packet.
> >
> > Signed-off-by: Bob Pearson <rpearsonhpe@gmail.com>
> > ---
> > v2:
> >   Split up the 5 patches in the first version into 9 patches which are
> >   each focused on a single change.
> >   Fixed sparse warnings in the first version.
> >
> > Bob Pearson (9):
> >   RDMA/rxe: Move ICRC checking to a subroutine
> >   RDMA/rxe: Move rxe_xmit_packet to a subroutine
> >   RDMA/rxe: Fixup rxe_send and rxe_loopback
> >   RDMA/rxe: Move ICRC generation to a subroutine
> >   RDMA/rxe: Move rxe_crc32 to a subroutine
> >   RDMA/rxe: Fixup rxe_icrc_hdr
> >   RDMA/rxe: Move crc32 init code to rxe_icrc.c
> >   RDMA/rxe: Add kernel-doc comments to rxe_icrc.c
> >   RDMA/rxe: Fix types in rxe_icrc.c
>
> Applied to for-next, thanks

Hi, Jason && Bob

I confronted a problem.
One hosts with this patch series, A
other hosts, without this patch series, B

I run rping between A <-------> B.

I confronted the following errors, and rping can not succeed.
"
...
[ 1848.251273] rdma_rxe: bad ICRC from 172.16.1.61
[ 1971.750691] rdma_rxe: bad ICRC from 172.16.1.61
...
"
It seems that this patch series breaks the Backward compatibility of RXE.

Not sure if it is a problem. Please comment.

Thanks a lot.
Zhu Yanjun

>
> Jason

^ permalink raw reply	[flat|nested] 24+ messages in thread
* Re: [PATCH for-next v2 0/9] ICRC cleanup
  2021-07-19  8:42   ` Zhu Yanjun
@ 2021-07-19 15:53     ` Olga Kornievskaia
  2021-07-19 17:10       ` Jason Gunthorpe
  0 siblings, 1 reply; 24+ messages in thread
From: Olga Kornievskaia @ 2021-07-19 15:53 UTC (permalink / raw)
  To: Zhu Yanjun; +Cc: Jason Gunthorpe, Bob Pearson, RDMA mailing list

On Mon, Jul 19, 2021 at 5:16 AM Zhu Yanjun <zyjzyj2000@gmail.com> wrote:
>
> On Sat, Jul 17, 2021 at 1:38 AM Jason Gunthorpe <jgg@nvidia.com> wrote:
> >
> > On Tue, Jul 06, 2021 at 11:00:32PM -0500, Bob Pearson wrote:
> > > This series of patches is a cleanup of the ICRC code in the rxe driver.
> > > The end result is to collect all the ICRC focused code into rxe_icrc.c
> > > with three APIs that are used by the rest of the driver. One to initialize
> > > the crypto engine used to compute crc32, and one each to generate and
> > > check the ICRC in a packet.
> > >
> > > Signed-off-by: Bob Pearson <rpearsonhpe@gmail.com>
> > > ---
> > > v2:
> > >   Split up the 5 patches in the first version into 9 patches which are
> > >   each focused on a single change.
> > >   Fixed sparse warnings in the first version.
> > >
> > > Bob Pearson (9):
> > >   RDMA/rxe: Move ICRC checking to a subroutine
> > >   RDMA/rxe: Move rxe_xmit_packet to a subroutine
> > >   RDMA/rxe: Fixup rxe_send and rxe_loopback
> > >   RDMA/rxe: Move ICRC generation to a subroutine
> > >   RDMA/rxe: Move rxe_crc32 to a subroutine
> > >   RDMA/rxe: Fixup rxe_icrc_hdr
> > >   RDMA/rxe: Move crc32 init code to rxe_icrc.c
> > >   RDMA/rxe: Add kernel-doc comments to rxe_icrc.c
> > >   RDMA/rxe: Fix types in rxe_icrc.c
> >
> > Applied to for-next, thanks
>
> Hi, Jason && Bob
>
> I confronted a problem.
> One hosts with this patch series, A
> other hosts, without this patch series, B
>
> I run rping between A <-------> B.
>
> I confronted the following errors, and rping can not succeed.
> "
> ...
> [ 1848.251273] rdma_rxe: bad ICRC from 172.16.1.61
> [ 1971.750691] rdma_rxe: bad ICRC from 172.16.1.61
> ...
> "
> It seems that this patch series breaks the Backward compatibility of RXE.
>
> Not sure if it is a problem. Please comment.
>
> Thanks a lot.
> Zhu Yanjun

I would like to second this post. I was about to submit a new post
asking about why rxe isn't working and if it's known. I'm trying to
figure out when/what patch broke things. From the stand point of
NFSoRDMA, it stopped working and I have discovered that simple rping
doesn't work as well. The last known working release was 5.11.



>
> >
> > Jason

^ permalink raw reply	[flat|nested] 24+ messages in thread
* Re: [PATCH for-next v2 0/9] ICRC cleanup
  2021-07-19 15:53     ` Olga Kornievskaia
@ 2021-07-19 17:10       ` Jason Gunthorpe
  2021-07-19 21:26         ` Bob Pearson
  0 siblings, 1 reply; 24+ messages in thread
From: Jason Gunthorpe @ 2021-07-19 17:10 UTC (permalink / raw)
  To: Olga Kornievskaia; +Cc: Zhu Yanjun, Bob Pearson, RDMA mailing list

On Mon, Jul 19, 2021 at 11:53:20AM -0400, Olga Kornievskaia wrote:
> On Mon, Jul 19, 2021 at 5:16 AM Zhu Yanjun <zyjzyj2000@gmail.com> wrote:
> >
> > On Sat, Jul 17, 2021 at 1:38 AM Jason Gunthorpe <jgg@nvidia.com> wrote:
> > >
> > > On Tue, Jul 06, 2021 at 11:00:32PM -0500, Bob Pearson wrote:
> > > > This series of patches is a cleanup of the ICRC code in the rxe driver.
> > > > The end result is to collect all the ICRC focused code into rxe_icrc.c
> > > > with three APIs that are used by the rest of the driver. One to initialize
> > > > the crypto engine used to compute crc32, and one each to generate and
> > > > check the ICRC in a packet.
> > > >
> > > > Signed-off-by: Bob Pearson <rpearsonhpe@gmail.com>
> > > > v2:
> > > >   Split up the 5 patches in the first version into 9 patches which are
> > > >   each focused on a single change.
> > > >   Fixed sparse warnings in the first version.
> > > >
> > > > Bob Pearson (9):
> > > >   RDMA/rxe: Move ICRC checking to a subroutine
> > > >   RDMA/rxe: Move rxe_xmit_packet to a subroutine
> > > >   RDMA/rxe: Fixup rxe_send and rxe_loopback
> > > >   RDMA/rxe: Move ICRC generation to a subroutine
> > > >   RDMA/rxe: Move rxe_crc32 to a subroutine
> > > >   RDMA/rxe: Fixup rxe_icrc_hdr
> > > >   RDMA/rxe: Move crc32 init code to rxe_icrc.c
> > > >   RDMA/rxe: Add kernel-doc comments to rxe_icrc.c
> > > >   RDMA/rxe: Fix types in rxe_icrc.c
> > >
> > > Applied to for-next, thanks
> >
> > Hi, Jason && Bob
> >
> > I confronted a problem.
> > One hosts with this patch series, A
> > other hosts, without this patch series, B
> >
> > I run rping between A <-------> B.
> >
> > I confronted the following errors, and rping can not succeed.
> > "
> > ...
> > [ 1848.251273] rdma_rxe: bad ICRC from 172.16.1.61
> > [ 1971.750691] rdma_rxe: bad ICRC from 172.16.1.61
> > ...
> > "
> > It seems that this patch series breaks the Backward compatibility of RXE.
> >
> > Not sure if it is a problem. Please comment.
> >
> > Thanks a lot.
> > Zhu Yanjun
> 
> I would like to second this post. I was about to submit a new post
> asking about why rxe isn't working and if it's known. I'm trying to
> figure out when/what patch broke things. From the stand point of
> NFSoRDMA, it stopped working and I have discovered that simple rping
> doesn't work as well. The last known working release was 5.11.

This isn't included in v5.13 or v5.14, so it can't be this series

You should send Bob and Zhu your non-working report

Jason

^ permalink raw reply	[flat|nested] 24+ messages in thread
* Re: [PATCH for-next v2 0/9] ICRC cleanup
  2021-07-19 17:10       ` Jason Gunthorpe
@ 2021-07-19 21:26         ` Bob Pearson
  0 siblings, 0 replies; 24+ messages in thread
From: Bob Pearson @ 2021-07-19 21:26 UTC (permalink / raw)
  To: Jason Gunthorpe, Olga Kornievskaia; +Cc: Zhu Yanjun, RDMA mailing list

On 7/19/21 12:10 PM, Jason Gunthorpe wrote:
> On Mon, Jul 19, 2021 at 11:53:20AM -0400, Olga Kornievskaia wrote:
>> On Mon, Jul 19, 2021 at 5:16 AM Zhu Yanjun <zyjzyj2000@gmail.com> wrote:
>>>
>>> On Sat, Jul 17, 2021 at 1:38 AM Jason Gunthorpe <jgg@nvidia.com> wrote:
>>>>
>>>> On Tue, Jul 06, 2021 at 11:00:32PM -0500, Bob Pearson wrote:
>>>>> This series of patches is a cleanup of the ICRC code in the rxe driver.
>>>>> The end result is to collect all the ICRC focused code into rxe_icrc.c
>>>>> with three APIs that are used by the rest of the driver. One to initialize
>>>>> the crypto engine used to compute crc32, and one each to generate and
>>>>> check the ICRC in a packet.
>>>>>
>>>>> Signed-off-by: Bob Pearson <rpearsonhpe@gmail.com>
>>>>> v2:
>>>>>   Split up the 5 patches in the first version into 9 patches which are
>>>>>   each focused on a single change.
>>>>>   Fixed sparse warnings in the first version.
>>>>>
>>>>> Bob Pearson (9):
>>>>>   RDMA/rxe: Move ICRC checking to a subroutine
>>>>>   RDMA/rxe: Move rxe_xmit_packet to a subroutine
>>>>>   RDMA/rxe: Fixup rxe_send and rxe_loopback
>>>>>   RDMA/rxe: Move ICRC generation to a subroutine
>>>>>   RDMA/rxe: Move rxe_crc32 to a subroutine
>>>>>   RDMA/rxe: Fixup rxe_icrc_hdr
>>>>>   RDMA/rxe: Move crc32 init code to rxe_icrc.c
>>>>>   RDMA/rxe: Add kernel-doc comments to rxe_icrc.c
>>>>>   RDMA/rxe: Fix types in rxe_icrc.c
>>>>
>>>> Applied to for-next, thanks
>>>
>>> Hi, Jason && Bob
>>>
>>> I confronted a problem.
>>> One hosts with this patch series, A
>>> other hosts, without this patch series, B
>>>
>>> I run rping between A <-------> B.
>>>
>>> I confronted the following errors, and rping can not succeed.
>>> "
>>> ...
>>> [ 1848.251273] rdma_rxe: bad ICRC from 172.16.1.61
>>> [ 1971.750691] rdma_rxe: bad ICRC from 172.16.1.61
>>> ...
>>> "
>>> It seems that this patch series breaks the Backward compatibility of RXE.
>>>
>>> Not sure if it is a problem. Please comment.
>>>
>>> Thanks a lot.
>>> Zhu Yanjun
>>
>> I would like to second this post. I was about to submit a new post
>> asking about why rxe isn't working and if it's known. I'm trying to
>> figure out when/what patch broke things. From the stand point of
>> NFSoRDMA, it stopped working and I have discovered that simple rping
>> doesn't work as well. The last known working release was 5.11.
> 
> This isn't included in v5.13 or v5.14, so it can't be this series
> 
> You should send Bob and Zhu your non-working report
> 
> Jason
> 
I'm working on finding what went wrong for Zhu. SHould have something in the next couple of hours.

Bob

^ permalink raw reply	[flat|nested] 24+ messages in thread
end of thread, other threads:[~2021-07-19 23:37 UTC | newest]

Thread overview: 24+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-07-07  4:00 [PATCH for-next v2 0/9] ICRC cleanup Bob Pearson
2021-07-07  4:00 ` [PATCH v2 1/9] RDMA/rxe: Move ICRC checking to a subroutine Bob Pearson
2021-07-13  7:27   ` Zhu Yanjun
2021-07-16 15:55   ` Jason Gunthorpe
2021-07-07  4:00 ` [PATCH v2 2/9] RDMA/rxe: Move rxe_xmit_packet " Bob Pearson
2021-07-14  3:24   ` Zhu Yanjun
2021-07-07  4:00 ` [PATCH v2 3/9] RDMA/rxe: Fixup rxe_send and rxe_loopback Bob Pearson
2021-07-07  4:00 ` [PATCH v2 4/9] RDMA/rxe: Move ICRC generation to a subroutine Bob Pearson
2021-07-16 15:57   ` Jason Gunthorpe
2021-07-16 16:08     ` Bob Pearson
2021-07-16 16:29       ` Jason Gunthorpe
2021-07-16 16:38         ` Pearson, Robert B
2021-07-07  4:00 ` [PATCH v2 5/9] RDMA/rxe: Move rxe_crc32 " Bob Pearson
2021-07-07  4:00 ` [PATCH v2 6/9] RDMA/rxe: Fixup rxe_icrc_hdr Bob Pearson
2021-07-07  4:00 ` [PATCH v2 7/9] RDMA/rxe: Move crc32 init code to rxe_icrc.c Bob Pearson
2021-07-07  4:00 ` [PATCH v2 8/9] RDMA/rxe: Add kernel-doc comments " Bob Pearson
2021-07-07  4:00 ` [PATCH v2 9/9] RDMA/rxe: Fix types in rxe_icrc.c Bob Pearson
2021-07-16 16:06   ` Jason Gunthorpe
2021-07-08  7:36 ` [PATCH for-next v2 0/9] ICRC cleanup Zhu Yanjun
2021-07-16 17:38 ` Jason Gunthorpe
2021-07-19  8:42   ` Zhu Yanjun
2021-07-19 15:53     ` Olga Kornievskaia
2021-07-19 17:10       ` Jason Gunthorpe
2021-07-19 21:26         ` Bob Pearson

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.