[ofa-general] Re: [PATCH for-2.6.21] mthca: QP reset race fixup

Michael S. Tsirkin mst at dev.mellanox.co.il
Wed Mar 21 05:34:34 PDT 2007


> Quoting Michael S. Tsirkin <mst at dev.mellanox.co.il>:
> Subject: Re: [PATCH for-2.6.21] mthca: QP reset race fixup
> 
> In hindsight, it was probably better to put qp_context directly in ib_wc
> instead of the qp pointer.
> 
> Then ipoib could set some flag in the structure pointed to by qp_context.
> 
> My guess this would be too big a change for 2.6.21. What do you think?


Here's how a patch to make the use after free fixable for ULPs that do polling
out of interrupt context (e.g. IPoIB with NAPI) would look like.

What do you think?

Warning: untested.

Signed-off-by: Michael S. Tsirkin <mst at dev.mellanox.co.il>

---

diff --git a/include/rdma/ib_verbs.h b/include/rdma/ib_verbs.h
index 765589f..4dc769a 100644
--- a/include/rdma/ib_verbs.h
+++ b/include/rdma/ib_verbs.h
@@ -420,7 +420,8 @@ struct ib_wc {
 	enum ib_wc_opcode	opcode;
 	u32			vendor_err;
 	u32			byte_len;
-	struct ib_qp	       *qp;
+	void		       *qp_context;
+	u32			qp_num;
 	__be32			imm_data;
 	u32			src_qp;
 	int			wc_flags;
diff --git a/drivers/infiniband/core/mad.c b/drivers/infiniband/core/mad.c
index 13efd41..7bad7ec 100644
--- a/drivers/infiniband/core/mad.c
+++ b/drivers/infiniband/core/mad.c
@@ -653,7 +653,8 @@ static void build_smp_wc(struct ib_qp *qp,
 	wc->pkey_index = pkey_index;
 	wc->byte_len = sizeof(struct ib_mad) + sizeof(struct ib_grh);
 	wc->src_qp = IB_QP0;
-	wc->qp = qp;
+	wc->qp_num = qp->qp_num;
+	wc->qp_context = qp->qp_context;
 	wc->slid = slid;
 	wc->sl = 0;
 	wc->dlid_path_bits = 0;
diff --git a/drivers/infiniband/core/uverbs_cmd.c b/drivers/infiniband/core/uverbs_cmd.c
index 4fd75af..b90e4a6 100644
--- a/drivers/infiniband/core/uverbs_cmd.c
+++ b/drivers/infiniband/core/uverbs_cmd.c
@@ -935,7 +935,7 @@ ssize_t ib_uverbs_poll_cq(struct ib_uverbs_file *file,
 		resp->wc[i].vendor_err 	   = wc[i].vendor_err;
 		resp->wc[i].byte_len 	   = wc[i].byte_len;
 		resp->wc[i].imm_data 	   = (__u32 __force) wc[i].imm_data;
-		resp->wc[i].qp_num 	   = wc[i].qp->qp_num;
+		resp->wc[i].qp_num 	   = wc[i].qp_num;
 		resp->wc[i].src_qp 	   = wc[i].src_qp;
 		resp->wc[i].wc_flags 	   = wc[i].wc_flags;
 		resp->wc[i].pkey_index 	   = wc[i].pkey_index;
diff --git a/drivers/infiniband/hw/amso1100/c2_cq.c b/drivers/infiniband/hw/amso1100/c2_cq.c
index 5175c99..835f7ed 100644
--- a/drivers/infiniband/hw/amso1100/c2_cq.c
+++ b/drivers/infiniband/hw/amso1100/c2_cq.c
@@ -153,7 +153,8 @@ static inline int c2_poll_one(struct c2_dev *c2dev,
 
 	entry->status = c2_cqe_status_to_openib(c2_wr_get_result(ce));
 	entry->wr_id = ce->hdr.context;
-	entry->qp = &qp->ibqp;
+	entry->qp_num = qp->ibqp.qp_num;
+	entry->qp_context = qp->ibqp.qp_context;
 	entry->wc_flags = 0;
 	entry->slid = 0;
 	entry->sl = 0;
diff --git a/drivers/infiniband/hw/ehca/ehca_reqs.c b/drivers/infiniband/hw/ehca/ehca_reqs.c
index 08d3f89..9c4e579 100644
--- a/drivers/infiniband/hw/ehca/ehca_reqs.c
+++ b/drivers/infiniband/hw/ehca/ehca_reqs.c
@@ -579,7 +579,8 @@ poll_cq_one_read_cqe:
 	} else
 		wc->status = IB_WC_SUCCESS;
 
-	wc->qp = NULL;
+	wc->qp_num = cqe->local_qp_number;
+	wc->qp_context = NULL;
 	wc->byte_len = cqe->nr_bytes_transferred;
 	wc->pkey_index = cqe->pkey_index;
 	wc->slid = cqe->rlid;
diff --git a/drivers/infiniband/hw/ipath/ipath_qp.c b/drivers/infiniband/hw/ipath/ipath_qp.c
index 64f07b1..a00230f 100644
--- a/drivers/infiniband/hw/ipath/ipath_qp.c
+++ b/drivers/infiniband/hw/ipath/ipath_qp.c
@@ -379,7 +379,8 @@ void ipath_error_qp(struct ipath_qp *qp, enum ib_wc_status err)
 	wc.vendor_err = 0;
 	wc.byte_len = 0;
 	wc.imm_data = 0;
-	wc.qp = &qp->ibqp;
+	wc.qp_num = qp->ibqp.qp_num;
+	wc.qp_context = qp->ibqp.qp_context;
 	wc.src_qp = 0;
 	wc.wc_flags = 0;
 	wc.pkey_index = 0;
diff --git a/drivers/infiniband/hw/ipath/ipath_rc.c b/drivers/infiniband/hw/ipath/ipath_rc.c
index 5ff20cb..74db045 100644
--- a/drivers/infiniband/hw/ipath/ipath_rc.c
+++ b/drivers/infiniband/hw/ipath/ipath_rc.c
@@ -702,7 +702,8 @@ void ipath_restart_rc(struct ipath_qp *qp, u32 psn, struct ib_wc *wc)
 		wc->opcode = ib_ipath_wc_opcode[wqe->wr.opcode];
 		wc->vendor_err = 0;
 		wc->byte_len = 0;
-		wc->qp = &qp->ibqp;
+		wc->qp_num = qp->ibqp.qp_num;
+		wc->qp_context = qp->ibqp.qp_context;
 		wc->src_qp = qp->remote_qpn;
 		wc->pkey_index = 0;
 		wc->slid = qp->remote_ah_attr.dlid;
diff --git a/drivers/infiniband/hw/ipath/ipath_ruc.c b/drivers/infiniband/hw/ipath/ipath_ruc.c
index e86cb17..ffe70ed 100644
--- a/drivers/infiniband/hw/ipath/ipath_ruc.c
+++ b/drivers/infiniband/hw/ipath/ipath_ruc.c
@@ -137,7 +137,8 @@ bad_lkey:
 	wc.vendor_err = 0;
 	wc.byte_len = 0;
 	wc.imm_data = 0;
-	wc.qp = &qp->ibqp;
+	wc.qp_num = qp->ibqp.qp_num;
+	wc.qp_context = qp->ibqp.qp_context;
 	wc.src_qp = 0;
 	wc.wc_flags = 0;
 	wc.pkey_index = 0;
diff --git a/drivers/infiniband/hw/ipath/ipath_uc.c b/drivers/infiniband/hw/ipath/ipath_uc.c
index 325d663..a03f538 100644
--- a/drivers/infiniband/hw/ipath/ipath_uc.c
+++ b/drivers/infiniband/hw/ipath/ipath_uc.c
@@ -49,7 +49,8 @@ static void complete_last_send(struct ipath_qp *qp, struct ipath_swqe *wqe,
 		wc->opcode = ib_ipath_wc_opcode[wqe->wr.opcode];
 		wc->vendor_err = 0;
 		wc->byte_len = wqe->length;
-		wc->qp = &qp->ibqp;
+		wc->qp_num = qp->ibqp.qp_num;
+		wc->qp_context = qp->ibqp.qp_context;
 		wc->src_qp = qp->remote_qpn;
 		wc->pkey_index = 0;
 		wc->slid = qp->remote_ah_attr.dlid;
diff --git a/drivers/infiniband/hw/ipath/ipath_ud.c b/drivers/infiniband/hw/ipath/ipath_ud.c
index 9a3e546..59fec70 100644
--- a/drivers/infiniband/hw/ipath/ipath_ud.c
+++ b/drivers/infiniband/hw/ipath/ipath_ud.c
@@ -66,7 +66,8 @@ bad_lkey:
 	wc.vendor_err = 0;
 	wc.byte_len = 0;
 	wc.imm_data = 0;
-	wc.qp = &qp->ibqp;
+	wc.qp_num = qp->ibqp.qp_num;
+	wc.qp_context = qp->ibqp.qp_context;
 	wc.src_qp = 0;
 	wc.wc_flags = 0;
 	wc.pkey_index = 0;
diff --git a/drivers/infiniband/hw/mthca/mthca_cmd.c b/drivers/infiniband/hw/mthca/mthca_cmd.c
index 7131446..4374c67 100644
--- a/drivers/infiniband/hw/mthca/mthca_cmd.c
+++ b/drivers/infiniband/hw/mthca/mthca_cmd.c
@@ -1858,7 +1858,7 @@ int mthca_MAD_IFC(struct mthca_dev *dev, int ignore_mkey, int ignore_bkey,
 
 		memset(inbox + 256, 0, 256);
 
-		MTHCA_PUT(inbox, in_wc->qp->qp_num, MAD_IFC_MY_QPN_OFFSET);
+		MTHCA_PUT(inbox, in_wc->qp_num, MAD_IFC_MY_QPN_OFFSET);
 		MTHCA_PUT(inbox, in_wc->src_qp,     MAD_IFC_RQPN_OFFSET);
 
 		val = in_wc->sl << 4;
diff --git a/drivers/infiniband/hw/mthca/mthca_cq.c b/drivers/infiniband/hw/mthca/mthca_cq.c
index e3c774b..808850a 100644
--- a/drivers/infiniband/hw/mthca/mthca_cq.c
+++ b/drivers/infiniband/hw/mthca/mthca_cq.c
@@ -538,7 +538,8 @@ static inline int mthca_poll_one(struct mthca_dev *dev,
 		}
 	}
 
-	entry->qp = &(*cur_qp)->ibqp;
+	entry->qp_num = (*cur_qp)->ibqp.qp_num;
+	entry->qp_context = (*cur_qp)->ibqp.qp_context;
 
 	if (is_send) {
 		wq = &(*cur_qp)->sq;
diff --git a/drivers/infiniband/hw/cxgb3/iwch_cq.c b/drivers/infiniband/hw/cxgb3/iwch_cq.c
index d7624c1..5a77dcc 100644
--- a/drivers/infiniband/hw/cxgb3/iwch_cq.c
+++ b/drivers/infiniband/hw/cxgb3/iwch_cq.c
@@ -79,7 +79,8 @@ static int iwch_poll_cq_one(struct iwch_dev *rhp, struct iwch_cq *chp,
 	ret = 1;
 
 	wc->wr_id = cookie;
-	wc->qp = &qhp->ibqp;
+	wc->qp_num = qhp->ibqp.qp_num;
+	wc->qp_context = qhp->ibqp.qp_context;
 	wc->vendor_err = CQE_STATUS(cqe);
 
 	PDBG("%s qpid 0x%x type %d opcode %d status 0x%x wrid hi 0x%x "

-- 
MST



More information about the general mailing list