diff mbox series

[for-next,03/17] RDMA/rxe: Isolate code to build request packet

Message ID 20221027185510.33808-4-rpearsonhpe@gmail.com (mailing list archive)
State Superseded
Headers show
Series RDMA/rxe: Enable scatter/gather support for skbs | expand

Commit Message

Bob Pearson Oct. 27, 2022, 6:54 p.m. UTC
Isolate all the code to build a request packet into a single
subroutine called rxe_init_req_packet().

Signed-off-by: Bob Pearson <rpearsonhpe@gmail.com>
---
 drivers/infiniband/sw/rxe/rxe_loc.h  |   2 +-
 drivers/infiniband/sw/rxe/rxe_net.c  |   6 +-
 drivers/infiniband/sw/rxe/rxe_req.c  | 121 ++++++++++++---------------
 drivers/infiniband/sw/rxe/rxe_resp.c |  11 +--
 4 files changed, 62 insertions(+), 78 deletions(-)

Comments

kernel test robot Oct. 30, 2022, 6:52 p.m. UTC | #1
Hi Bob,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on c9eeabac5e8d27a3f40280908e089058bab39edb]

url:    https://github.com/intel-lab-lkp/linux/commits/Bob-Pearson/RDMA-rxe-Enable-scatter-gather-support-for-skbs/20221028-025839
base:   c9eeabac5e8d27a3f40280908e089058bab39edb
patch link:    https://lore.kernel.org/r/20221027185510.33808-4-rpearsonhpe%40gmail.com
patch subject: [PATCH for-next 03/17] RDMA/rxe: Isolate code to build request packet
config: arm64-allyesconfig
compiler: clang version 16.0.0 (https://github.com/llvm/llvm-project 791a7ae1ba3efd6bca96338e10ffde557ba83920)
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # install arm64 cross compiling tool for clang build
        # apt-get install binutils-aarch64-linux-gnu
        # https://github.com/intel-lab-lkp/linux/commit/574d2b0517821f3c83c7962814b820a6880e2224
        git remote add linux-review https://github.com/intel-lab-lkp/linux
        git fetch --no-tags linux-review Bob-Pearson/RDMA-rxe-Enable-scatter-gather-support-for-skbs/20221028-025839
        git checkout 574d2b0517821f3c83c7962814b820a6880e2224
        # save the config file
        mkdir build_dir && cp config build_dir/.config
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=arm64 SHELL=/bin/bash drivers/infiniband/sw/rxe/

If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <lkp@intel.com>

All warnings (new ones prefixed by >>):

>> drivers/infiniband/sw/rxe/rxe_req.c:482:6: warning: variable 'skb' is used uninitialized whenever 'if' condition is true [-Wsometimes-uninitialized]
           if (unlikely(!av))
               ^~~~~~~~~~~~~
   include/linux/compiler.h:78:22: note: expanded from macro 'unlikely'
   # define unlikely(x)    __builtin_expect(!!(x), 0)
                           ^~~~~~~~~~~~~~~~~~~~~~~~~~
   drivers/infiniband/sw/rxe/rxe_req.c:523:6: note: uninitialized use occurs here
           if (skb)
               ^~~
   drivers/infiniband/sw/rxe/rxe_req.c:482:2: note: remove the 'if' if its condition is always false
           if (unlikely(!av))
           ^~~~~~~~~~~~~~~~~~
   drivers/infiniband/sw/rxe/rxe_req.c:465:21: note: initialize the variable 'skb' to silence this warning
           struct sk_buff *skb;
                              ^
                               = NULL
   1 warning generated.


vim +482 drivers/infiniband/sw/rxe/rxe_req.c

   458	
   459	static struct sk_buff *rxe_init_req_packet(struct rxe_qp *qp,
   460						   struct rxe_send_wqe *wqe,
   461						   int opcode, u32 payload,
   462						   struct rxe_pkt_info *pkt)
   463	{
   464		struct rxe_dev *rxe = to_rdev(qp->ibqp.device);
   465		struct sk_buff *skb;
   466		struct rxe_av *av;
   467		struct rxe_ah *ah;
   468		void *padp;
   469		int pad;
   470		int err = -EINVAL;
   471	
   472		pkt->rxe = rxe;
   473		pkt->opcode = opcode;
   474		pkt->qp = qp;
   475		pkt->psn = qp->req.psn;
   476		pkt->mask = rxe_opcode[opcode].mask;
   477		pkt->wqe = wqe;
   478		pkt->port_num = 1;
   479	
   480		/* get address vector and address handle for UD qps only */
   481		av = rxe_get_av(pkt, &ah);
 > 482		if (unlikely(!av))
   483			goto err_out;
   484	
   485		/* length from start of bth to end of icrc */
   486		pad = (-payload) & 0x3;
   487		pkt->paylen = rxe_opcode[opcode].length + payload +
   488							pad + RXE_ICRC_SIZE;
   489	
   490		/* init skb */
   491		skb = rxe_init_packet(rxe, av, pkt);
   492		if (unlikely(!skb))
   493			goto err_out;
   494	
   495		rxe_init_roce_hdrs(qp, wqe, pkt, pad);
   496	
   497		if (pkt->mask & RXE_WRITE_OR_SEND_MASK) {
   498			err = rxe_init_payload(qp, wqe, pkt, payload);
   499			if (err)
   500				goto err_out;
   501		}
   502	
   503		if (pad) {
   504			padp = payload_addr(pkt) + payload;
   505			memset(padp, 0, pad);
   506		}
   507	
   508		/* IP and UDP network headers */
   509		err = rxe_prepare(av, pkt, skb);
   510		if (err)
   511			goto err_out;
   512	
   513		if (ah)
   514			rxe_put(ah);
   515	
   516		return skb;
   517	
   518	err_out:
   519		if (err == -EFAULT)
   520			wqe->status = IB_WC_LOC_PROT_ERR;
   521		else
   522			wqe->status = IB_WC_LOC_QP_OP_ERR;
   523		if (skb)
   524			kfree_skb(skb);
   525		if (ah)
   526			rxe_put(ah);
   527	
   528		return NULL;
   529	}
   530
diff mbox series

Patch

diff --git a/drivers/infiniband/sw/rxe/rxe_loc.h b/drivers/infiniband/sw/rxe/rxe_loc.h
index c2a5c8814a48..574a6afc1199 100644
--- a/drivers/infiniband/sw/rxe/rxe_loc.h
+++ b/drivers/infiniband/sw/rxe/rxe_loc.h
@@ -92,7 +92,7 @@  void rxe_mw_cleanup(struct rxe_pool_elem *elem);
 
 /* rxe_net.c */
 struct sk_buff *rxe_init_packet(struct rxe_dev *rxe, struct rxe_av *av,
-				int paylen, struct rxe_pkt_info *pkt);
+				struct rxe_pkt_info *pkt);
 int rxe_prepare(struct rxe_av *av, struct rxe_pkt_info *pkt,
 		struct sk_buff *skb);
 int rxe_xmit_packet(struct rxe_qp *qp, struct rxe_pkt_info *pkt,
diff --git a/drivers/infiniband/sw/rxe/rxe_net.c b/drivers/infiniband/sw/rxe/rxe_net.c
index 35f327b9d4b8..1e4456f5cda2 100644
--- a/drivers/infiniband/sw/rxe/rxe_net.c
+++ b/drivers/infiniband/sw/rxe/rxe_net.c
@@ -443,7 +443,7 @@  int rxe_xmit_packet(struct rxe_qp *qp, struct rxe_pkt_info *pkt,
 }
 
 struct sk_buff *rxe_init_packet(struct rxe_dev *rxe, struct rxe_av *av,
-				int paylen, struct rxe_pkt_info *pkt)
+				struct rxe_pkt_info *pkt)
 {
 	unsigned int hdr_len;
 	struct sk_buff *skb = NULL;
@@ -468,7 +468,7 @@  struct sk_buff *rxe_init_packet(struct rxe_dev *rxe, struct rxe_av *av,
 		rcu_read_unlock();
 		goto out;
 	}
-	skb = alloc_skb(paylen + hdr_len + LL_RESERVED_SPACE(ndev),
+	skb = alloc_skb(pkt->paylen + hdr_len + LL_RESERVED_SPACE(ndev),
 			GFP_ATOMIC);
 
 	if (unlikely(!skb)) {
@@ -489,7 +489,7 @@  struct sk_buff *rxe_init_packet(struct rxe_dev *rxe, struct rxe_av *av,
 
 	pkt->rxe	= rxe;
 	pkt->port_num	= port_num;
-	pkt->hdr	= skb_put(skb, paylen);
+	pkt->hdr	= skb_put(skb, pkt->paylen);
 	pkt->mask	|= RXE_GRH_MASK;
 
 out:
diff --git a/drivers/infiniband/sw/rxe/rxe_req.c b/drivers/infiniband/sw/rxe/rxe_req.c
index 10a75f4e3608..8cc683ebf536 100644
--- a/drivers/infiniband/sw/rxe/rxe_req.c
+++ b/drivers/infiniband/sw/rxe/rxe_req.c
@@ -456,51 +456,76 @@  static int rxe_init_payload(struct rxe_qp *qp, struct rxe_send_wqe *wqe,
 	return err;
 }
 
-static struct sk_buff *init_req_packet(struct rxe_qp *qp,
-				       struct rxe_av *av,
-				       struct rxe_send_wqe *wqe,
-				       int opcode, u32 payload,
-				       struct rxe_pkt_info *pkt)
+static struct sk_buff *rxe_init_req_packet(struct rxe_qp *qp,
+					   struct rxe_send_wqe *wqe,
+					   int opcode, u32 payload,
+					   struct rxe_pkt_info *pkt)
 {
 	struct rxe_dev *rxe = to_rdev(qp->ibqp.device);
 	struct sk_buff *skb;
-	int pad = (-payload) & 0x3;
-	int paylen;
+	struct rxe_av *av;
+	struct rxe_ah *ah;
+	void *padp;
+	int pad;
+	int err = -EINVAL;
+
+	pkt->rxe = rxe;
+	pkt->opcode = opcode;
+	pkt->qp = qp;
+	pkt->psn = qp->req.psn;
+	pkt->mask = rxe_opcode[opcode].mask;
+	pkt->wqe = wqe;
+	pkt->port_num = 1;
+
+	/* get address vector and address handle for UD qps only */
+	av = rxe_get_av(pkt, &ah);
+	if (unlikely(!av))
+		goto err_out;
 
 	/* length from start of bth to end of icrc */
-	paylen = rxe_opcode[opcode].length + payload + pad + RXE_ICRC_SIZE;
-	pkt->paylen = paylen;
+	pad = (-payload) & 0x3;
+	pkt->paylen = rxe_opcode[opcode].length + payload +
+						pad + RXE_ICRC_SIZE;
 
 	/* init skb */
-	skb = rxe_init_packet(rxe, av, paylen, pkt);
+	skb = rxe_init_packet(rxe, av, pkt);
 	if (unlikely(!skb))
-		return NULL;
+		goto err_out;
 
 	rxe_init_roce_hdrs(qp, wqe, pkt, pad);
 
-	return skb;
-}
+	if (pkt->mask & RXE_WRITE_OR_SEND_MASK) {
+		err = rxe_init_payload(qp, wqe, pkt, payload);
+		if (err)
+			goto err_out;
+	}
 
-static int finish_packet(struct rxe_qp *qp, struct rxe_av *av,
-			 struct rxe_send_wqe *wqe, struct rxe_pkt_info *pkt,
-			 struct sk_buff *skb, u32 payload)
-{
-	int err;
+	if (pad) {
+		padp = payload_addr(pkt) + payload;
+		memset(padp, 0, pad);
+	}
 
+	/* IP and UDP network headers */
 	err = rxe_prepare(av, pkt, skb);
 	if (err)
-		return err;
+		goto err_out;
 
-	if (pkt->mask & RXE_WRITE_OR_SEND_MASK) {
-		err = rxe_init_payload(qp, wqe, pkt, payload);
-		if (bth_pad(pkt)) {
-			u8 *pad = payload_addr(pkt) + payload;
+	if (ah)
+		rxe_put(ah);
 
-			memset(pad, 0, bth_pad(pkt));
-		}
-	}
+	return skb;
 
-	return 0;
+err_out:
+	if (err == -EFAULT)
+		wqe->status = IB_WC_LOC_PROT_ERR;
+	else
+		wqe->status = IB_WC_LOC_QP_OP_ERR;
+	if (skb)
+		kfree_skb(skb);
+	if (ah)
+		rxe_put(ah);
+
+	return NULL;
 }
 
 static void update_wqe_state(struct rxe_qp *qp,
@@ -630,7 +655,6 @@  static int rxe_do_local_ops(struct rxe_qp *qp, struct rxe_send_wqe *wqe)
 int rxe_requester(void *arg)
 {
 	struct rxe_qp *qp = (struct rxe_qp *)arg;
-	struct rxe_dev *rxe = to_rdev(qp->ibqp.device);
 	struct rxe_pkt_info pkt;
 	struct sk_buff *skb;
 	struct rxe_send_wqe *wqe;
@@ -643,8 +667,6 @@  int rxe_requester(void *arg)
 	struct rxe_send_wqe rollback_wqe;
 	u32 rollback_psn;
 	struct rxe_queue *q = qp->sq.queue;
-	struct rxe_ah *ah;
-	struct rxe_av *av;
 
 	if (!rxe_get(qp))
 		return -EAGAIN;
@@ -753,44 +775,9 @@  int rxe_requester(void *arg)
 		payload = mtu;
 	}
 
-	pkt.rxe = rxe;
-	pkt.opcode = opcode;
-	pkt.qp = qp;
-	pkt.psn = qp->req.psn;
-	pkt.mask = rxe_opcode[opcode].mask;
-	pkt.wqe = wqe;
-
-	av = rxe_get_av(&pkt, &ah);
-	if (unlikely(!av)) {
-		pr_err("qp#%d Failed no address vector\n", qp_num(qp));
-		wqe->status = IB_WC_LOC_QP_OP_ERR;
-		goto err;
-	}
-
-	skb = init_req_packet(qp, av, wqe, opcode, payload, &pkt);
-	if (unlikely(!skb)) {
-		pr_err("qp#%d Failed allocating skb\n", qp_num(qp));
-		wqe->status = IB_WC_LOC_QP_OP_ERR;
-		if (ah)
-			rxe_put(ah);
-		goto err;
-	}
-
-	err = finish_packet(qp, av, wqe, &pkt, skb, payload);
-	if (unlikely(err)) {
-		pr_debug("qp#%d Error during finish packet\n", qp_num(qp));
-		if (err == -EFAULT)
-			wqe->status = IB_WC_LOC_PROT_ERR;
-		else
-			wqe->status = IB_WC_LOC_QP_OP_ERR;
-		kfree_skb(skb);
-		if (ah)
-			rxe_put(ah);
+	skb = rxe_init_req_packet(qp, wqe, opcode, payload, &pkt);
+	if (unlikely(!skb))
 		goto err;
-	}
-
-	if (ah)
-		rxe_put(ah);
 
 	/*
 	 * To prevent a race on wqe access between requester and completer,
diff --git a/drivers/infiniband/sw/rxe/rxe_resp.c b/drivers/infiniband/sw/rxe/rxe_resp.c
index 95d372db934d..a00885799619 100644
--- a/drivers/infiniband/sw/rxe/rxe_resp.c
+++ b/drivers/infiniband/sw/rxe/rxe_resp.c
@@ -665,22 +665,19 @@  static struct sk_buff *prepare_ack_packet(struct rxe_qp *qp,
 	int pad;
 	int err;
 
-	/*
-	 * allocate packet
-	 */
 	pad = (-payload) & 0x3;
 	paylen = rxe_opcode[opcode].length + payload + pad + RXE_ICRC_SIZE;
 
-	skb = rxe_init_packet(rxe, &qp->pri_av, paylen, ack);
-	if (!skb)
-		return NULL;
-
 	ack->qp = qp;
 	ack->opcode = opcode;
 	ack->mask = rxe_opcode[opcode].mask;
 	ack->paylen = paylen;
 	ack->psn = psn;
 
+	skb = rxe_init_packet(rxe, &qp->pri_av, ack);
+	if (!skb)
+		return NULL;
+
 	bth_init(ack, opcode, 0, 0, pad, IB_DEFAULT_PKEY_FULL,
 		 qp->attr.dest_qp_num, 0, psn);