[PATCH v1 00/13] T10-PI support for iSER initiator

[PATCH v1 00/13] T10-PI support for iSER initiator

[Sagi Grimberg]

Hey Roland, Nic and Co

This patchset adds T10 protection information offload support over
RDMA signature verbs API. This set, along with the iSER target set,
allow end-to-end protection information passthrough and validation.
The patchset was tested against Linux SCSI target with iSER DIF
support applied.

The enablement of iSER DIF support is currently controlled with
module parameters (similar to lpfc for example) which make them
global. In the next phase we can consider passing these parameters
from iscsid nl messages, which would make them per-connection.

The approach I took with respect to escelating protection information
errors was minimal iSCSI intervention in protection information affairs.
I added libiscsi a hook asking the transport to check the protection
information status, and construct the proper sense data in case of errors
(the alternative of letting the transport to construct sense data seemed
much less appealing).

Note that this patchset comes on top of a pending patch for iSER to suppress
fastreg completions (http://marc.info/?l=linux-rdma&m=139047309831997&w=2).

v0 patches are available in target-pending git repo (branch rdma-dif) and
passed 0-DAY testing.

Roland, I would like to hear your feedback on this.

The set is ordered by the following:
- Preperation patches (non/minor functionality changes).
- Add protection information execution support.
- Add protection information status check facilities.
- Publish T10-DIF support to SCSI midlayer according to
  IB device capabilities.

Changes from v0:
- Fix protection information dma registration for unaligned scatterlists
  which may happen when the block layer merges bios.
- Don't fail connections on devices without DIF support - warn and continue
  without DIF.
- reword FR -> FastReg

Alex Tabachnik (2):
  IB/iser: Introduce pi_enable, pi_guard module parameters
  IB/iser: Initialize T10-PI resources

Sagi Grimberg (11):
  IB/iser: Avoid FRWR notation, use fastreg instead
  IB/iser: Push the desicion what memory key to use into fast_reg_mr
    routine
  IB/iser: Move fast_reg_descriptor initialization to a function
  IB/iser: Keep IB device attributes under iser_device
  IB/iser: Replace fastreg descriptor valid bool with indicators
    container
  IB/iser: Generalize iser_unmap_task_data and
    finalize_rdma_unaligned_sg
  IB/iser: Generalize fall_to_bounce_buf routine
  IB/iser: Support T10-PI operations
  SCSI/libiscsi: Add check_protection callback for transports
  IB/iser: Implement check_protection
  IB/iser: Publish T10-PI support to SCSI midlayer

 drivers/infiniband/ulp/iser/iscsi_iser.c     |   46 +++-
 drivers/infiniband/ulp/iser/iscsi_iser.h     |   71 ++++-
 drivers/infiniband/ulp/iser/iser_initiator.c |   98 +++++-
 drivers/infiniband/ulp/iser/iser_memory.c    |  445 +++++++++++++++++++-------
 drivers/infiniband/ulp/iser/iser_verbs.c     |  285 ++++++++++++-----
 drivers/scsi/libiscsi.c                      |   32 ++
 include/scsi/libiscsi.h                      |    4 +
 include/scsi/scsi_transport_iscsi.h          |    1 +
 8 files changed, 769 insertions(+), 213 deletions(-)

[PATCH v1 01/13] IB/iser: Avoid FRWR notation, use fastreg instead

[Sagi Grimberg]

FRWR stands for "fast registration work request". We
want to avoid calling the fastreg pool with that name,
instead we name it fastreg which stands for "fast registration".

This pool will include more elements in the future, so
it is a good idea to generalize the name.

Signed-off-by: Sagi Grimberg <sagig-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
Signed-off-by: Alex Tabachnik <alext-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
---
 drivers/infiniband/ulp/iser/iscsi_iser.h  |   20 ++++---
 drivers/infiniband/ulp/iser/iser_memory.c |   28 +++++-----
 drivers/infiniband/ulp/iser/iser_verbs.c  |   84 ++++++++++++++--------------
 3 files changed, 67 insertions(+), 65 deletions(-)

diff --git a/drivers/infiniband/ulp/iser/iscsi_iser.h b/drivers/infiniband/ulp/iser/iscsi_iser.h
index e1a01c6..ca161df 100644
--- a/drivers/infiniband/ulp/iser/iscsi_iser.h
+++ b/drivers/infiniband/ulp/iser/iscsi_iser.h
@@ -138,7 +138,7 @@
 #define ISER_WSV			0x08
 #define ISER_RSV			0x04
 
-#define ISER_FRWR_LI_WRID		0xffffffffffffffffULL
+#define ISER_FASTREG_LI_WRID		0xffffffffffffffffULL
 
 struct iser_hdr {
 	u8      flags;
@@ -312,6 +312,8 @@ struct iser_conn {
 	unsigned int 		     rx_desc_head;
 	struct iser_rx_desc	     *rx_descs;
 	struct ib_recv_wr	     rx_wr[ISER_MIN_POSTED_RX];
+
+	/* Connection memory registration pool */
 	union {
 		struct {
 			struct ib_fmr_pool      *pool;	   /* pool of IB FMRs         */
@@ -321,8 +323,8 @@ struct iser_conn {
 		struct {
 			struct list_head	pool;
 			int			pool_size;
-		} frwr;
-	} fastreg;
+		} fastreg;
+	};
 };
 
 struct iscsi_iser_conn {
@@ -408,8 +410,8 @@ void iser_finalize_rdma_unaligned_sg(struct iscsi_iser_task *task,
 
 int  iser_reg_rdma_mem_fmr(struct iscsi_iser_task *task,
 			   enum iser_data_dir cmd_dir);
-int  iser_reg_rdma_mem_frwr(struct iscsi_iser_task *task,
-			    enum iser_data_dir cmd_dir);
+int  iser_reg_rdma_mem_fastreg(struct iscsi_iser_task *task,
+			       enum iser_data_dir cmd_dir);
 
 int  iser_connect(struct iser_conn   *ib_conn,
 		  struct sockaddr_in *src_addr,
@@ -422,8 +424,8 @@ int  iser_reg_page_vec(struct iser_conn     *ib_conn,
 
 void iser_unreg_mem_fmr(struct iscsi_iser_task *iser_task,
 			enum iser_data_dir cmd_dir);
-void iser_unreg_mem_frwr(struct iscsi_iser_task *iser_task,
-			 enum iser_data_dir cmd_dir);
+void iser_unreg_mem_fastreg(struct iscsi_iser_task *iser_task,
+			    enum iser_data_dir cmd_dir);
 
 int  iser_post_recvl(struct iser_conn *ib_conn);
 int  iser_post_recvm(struct iser_conn *ib_conn, int count);
@@ -440,6 +442,6 @@ int  iser_initialize_task_headers(struct iscsi_task *task,
 int iser_alloc_rx_descriptors(struct iser_conn *ib_conn, struct iscsi_session *session);
 int iser_create_fmr_pool(struct iser_conn *ib_conn, unsigned cmds_max);
 void iser_free_fmr_pool(struct iser_conn *ib_conn);
-int iser_create_frwr_pool(struct iser_conn *ib_conn, unsigned cmds_max);
-void iser_free_frwr_pool(struct iser_conn *ib_conn);
+int iser_create_fastreg_pool(struct iser_conn *ib_conn, unsigned cmds_max);
+void iser_free_fastreg_pool(struct iser_conn *ib_conn);
 #endif
diff --git a/drivers/infiniband/ulp/iser/iser_memory.c b/drivers/infiniband/ulp/iser/iser_memory.c
index f770179..6e9b7bc 100644
--- a/drivers/infiniband/ulp/iser/iser_memory.c
+++ b/drivers/infiniband/ulp/iser/iser_memory.c
@@ -422,8 +422,8 @@ int iser_reg_rdma_mem_fmr(struct iscsi_iser_task *iser_task,
 			 (unsigned long)regd_buf->reg.va,
 			 (unsigned long)regd_buf->reg.len);
 	} else { /* use FMR for multiple dma entries */
-		iser_page_vec_build(mem, ib_conn->fastreg.fmr.page_vec, ibdev);
-		err = iser_reg_page_vec(ib_conn, ib_conn->fastreg.fmr.page_vec,
+		iser_page_vec_build(mem, ib_conn->fmr.page_vec, ibdev);
+		err = iser_reg_page_vec(ib_conn, ib_conn->fmr.page_vec,
 					&regd_buf->reg);
 		if (err && err != -EAGAIN) {
 			iser_data_buf_dump(mem, ibdev);
@@ -431,12 +431,12 @@ int iser_reg_rdma_mem_fmr(struct iscsi_iser_task *iser_task,
 				 mem->dma_nents,
 				 ntoh24(iser_task->desc.iscsi_header.dlength));
 			iser_err("page_vec: data_size = 0x%x, length = %d, offset = 0x%x\n",
-				 ib_conn->fastreg.fmr.page_vec->data_size,
-				 ib_conn->fastreg.fmr.page_vec->length,
-				 ib_conn->fastreg.fmr.page_vec->offset);
-			for (i = 0; i < ib_conn->fastreg.fmr.page_vec->length; i++)
+				 ib_conn->fmr.page_vec->data_size,
+				 ib_conn->fmr.page_vec->length,
+				 ib_conn->fmr.page_vec->offset);
+			for (i = 0; i < ib_conn->fmr.page_vec->length; i++)
 				iser_err("page_vec[%d] = 0x%llx\n", i,
-					 (unsigned long long) ib_conn->fastreg.fmr.page_vec->pages[i]);
+					 (unsigned long long) ib_conn->fmr.page_vec->pages[i]);
 		}
 		if (err)
 			return err;
@@ -457,7 +457,7 @@ static int iser_fast_reg_mr(struct fast_reg_descriptor *desc,
 
 	if (!desc->valid) {
 		memset(&inv_wr, 0, sizeof(inv_wr));
-		inv_wr.wr_id = ISER_FRWR_LI_WRID;
+		inv_wr.wr_id = ISER_FASTREG_LI_WRID;
 		inv_wr.opcode = IB_WR_LOCAL_INV;
 		inv_wr.ex.invalidate_rkey = desc->data_mr->rkey;
 		wr = &inv_wr;
@@ -468,7 +468,7 @@ static int iser_fast_reg_mr(struct fast_reg_descriptor *desc,
 
 	/* Prepare FASTREG WR */
 	memset(&fastreg_wr, 0, sizeof(fastreg_wr));
-	fastreg_wr.wr_id = ISER_FRWR_LI_WRID;
+	fastreg_wr.wr_id = ISER_FASTREG_LI_WRID;
 	fastreg_wr.opcode = IB_WR_FAST_REG_MR;
 	fastreg_wr.wr.fast_reg.iova_start = desc->data_frpl->page_list[0] + offset;
 	fastreg_wr.wr.fast_reg.page_list = desc->data_frpl;
@@ -503,13 +503,13 @@ static int iser_fast_reg_mr(struct fast_reg_descriptor *desc,
 }
 
 /**
- * iser_reg_rdma_mem_frwr - Registers memory intended for RDMA,
+ * iser_reg_rdma_mem_fastreg - Registers memory intended for RDMA,
  * using Fast Registration WR (if possible) obtaining rkey and va
  *
  * returns 0 on success, errno code on failure
  */
-int iser_reg_rdma_mem_frwr(struct iscsi_iser_task *iser_task,
-			   enum iser_data_dir cmd_dir)
+int iser_reg_rdma_mem_fastreg(struct iscsi_iser_task *iser_task,
+			      enum iser_data_dir cmd_dir)
 {
 	struct iser_conn *ib_conn = iser_task->iser_conn->ib_conn;
 	struct iser_device *device = ib_conn->device;
@@ -544,7 +544,7 @@ int iser_reg_rdma_mem_frwr(struct iscsi_iser_task *iser_task,
 		regd_buf->reg.is_mr = 0;
 	} else {
 		spin_lock_irqsave(&ib_conn->lock, flags);
-		desc = list_first_entry(&ib_conn->fastreg.frwr.pool,
+		desc = list_first_entry(&ib_conn->fastreg.pool,
 					struct fast_reg_descriptor, list);
 		list_del(&desc->list);
 		spin_unlock_irqrestore(&ib_conn->lock, flags);
@@ -567,7 +567,7 @@ int iser_reg_rdma_mem_frwr(struct iscsi_iser_task *iser_task,
 	return 0;
 err_reg:
 	spin_lock_irqsave(&ib_conn->lock, flags);
-	list_add_tail(&desc->list, &ib_conn->fastreg.frwr.pool);
+	list_add_tail(&desc->list, &ib_conn->fastreg.pool);
 	spin_unlock_irqrestore(&ib_conn->lock, flags);
 	return err;
 }
diff --git a/drivers/infiniband/ulp/iser/iser_verbs.c b/drivers/infiniband/ulp/iser/iser_verbs.c
index 7bdb811..dc5a0b4 100644
--- a/drivers/infiniband/ulp/iser/iser_verbs.c
+++ b/drivers/infiniband/ulp/iser/iser_verbs.c
@@ -94,13 +94,13 @@ static int iser_create_device_ib_res(struct iser_device *device)
 		device->iser_unreg_rdma_mem = iser_unreg_mem_fmr;
 	} else
 	if (dev_attr->device_cap_flags & IB_DEVICE_MEM_MGT_EXTENSIONS) {
-		iser_info("FRWR supported, using FRWR for registration\n");
-		device->iser_alloc_rdma_reg_res = iser_create_frwr_pool;
-		device->iser_free_rdma_reg_res = iser_free_frwr_pool;
-		device->iser_reg_rdma_mem = iser_reg_rdma_mem_frwr;
-		device->iser_unreg_rdma_mem = iser_unreg_mem_frwr;
+		iser_info("FastReg supported, using FastReg for registration\n");
+		device->iser_alloc_rdma_reg_res = iser_create_fastreg_pool;
+		device->iser_free_rdma_reg_res = iser_free_fastreg_pool;
+		device->iser_reg_rdma_mem = iser_reg_rdma_mem_fastreg;
+		device->iser_unreg_rdma_mem = iser_unreg_mem_fastreg;
 	} else {
-		iser_err("IB device does not support FMRs nor FRWRs, can't register memory\n");
+		iser_err("IB device does not support FMRs nor FastRegs, can't register memory\n");
 		goto dev_attr_err;
 	}
 
@@ -221,13 +221,13 @@ int iser_create_fmr_pool(struct iser_conn *ib_conn, unsigned cmds_max)
 	struct ib_fmr_pool_param params;
 	int ret = -ENOMEM;
 
-	ib_conn->fastreg.fmr.page_vec = kmalloc(sizeof(struct iser_page_vec) +
-						(sizeof(u64)*(ISCSI_ISER_SG_TABLESIZE + 1)),
-						GFP_KERNEL);
-	if (!ib_conn->fastreg.fmr.page_vec)
+	ib_conn->fmr.page_vec = kmalloc(sizeof(*ib_conn->fmr.page_vec) +
+					(sizeof(u64)*(ISCSI_ISER_SG_TABLESIZE + 1)),
+					GFP_KERNEL);
+	if (!ib_conn->fmr.page_vec)
 		return ret;
 
-	ib_conn->fastreg.fmr.page_vec->pages = (u64 *)(ib_conn->fastreg.fmr.page_vec + 1);
+	ib_conn->fmr.page_vec->pages = (u64 *)(ib_conn->fmr.page_vec + 1);
 
 	params.page_shift        = SHIFT_4K;
 	/* when the first/last SG element are not start/end *
@@ -243,16 +243,16 @@ int iser_create_fmr_pool(struct iser_conn *ib_conn, unsigned cmds_max)
 				    IB_ACCESS_REMOTE_WRITE |
 				    IB_ACCESS_REMOTE_READ);
 
-	ib_conn->fastreg.fmr.pool = ib_create_fmr_pool(device->pd, &params);
-	if (!IS_ERR(ib_conn->fastreg.fmr.pool))
+	ib_conn->fmr.pool = ib_create_fmr_pool(device->pd, &params);
+	if (!IS_ERR(ib_conn->fmr.pool))
 		return 0;
 
 	/* no FMR => no need for page_vec */
-	kfree(ib_conn->fastreg.fmr.page_vec);
-	ib_conn->fastreg.fmr.page_vec = NULL;
+	kfree(ib_conn->fmr.page_vec);
+	ib_conn->fmr.page_vec = NULL;
 
-	ret = PTR_ERR(ib_conn->fastreg.fmr.pool);
-	ib_conn->fastreg.fmr.pool = NULL;
+	ret = PTR_ERR(ib_conn->fmr.pool);
+	ib_conn->fmr.pool = NULL;
 	if (ret != -ENOSYS) {
 		iser_err("FMR allocation failed, err %d\n", ret);
 		return ret;
@@ -268,30 +268,30 @@ int iser_create_fmr_pool(struct iser_conn *ib_conn, unsigned cmds_max)
 void iser_free_fmr_pool(struct iser_conn *ib_conn)
 {
 	iser_info("freeing conn %p fmr pool %p\n",
-		  ib_conn, ib_conn->fastreg.fmr.pool);
+		  ib_conn, ib_conn->fmr.pool);
 
-	if (ib_conn->fastreg.fmr.pool != NULL)
-		ib_destroy_fmr_pool(ib_conn->fastreg.fmr.pool);
+	if (ib_conn->fmr.pool != NULL)
+		ib_destroy_fmr_pool(ib_conn->fmr.pool);
 
-	ib_conn->fastreg.fmr.pool = NULL;
+	ib_conn->fmr.pool = NULL;
 
-	kfree(ib_conn->fastreg.fmr.page_vec);
-	ib_conn->fastreg.fmr.page_vec = NULL;
+	kfree(ib_conn->fmr.page_vec);
+	ib_conn->fmr.page_vec = NULL;
 }
 
 /**
- * iser_create_frwr_pool - Creates pool of fast_reg descriptors
+ * iser_create_fastreg_pool - Creates pool of fast_reg descriptors
  * for fast registration work requests.
  * returns 0 on success, or errno code on failure
  */
-int iser_create_frwr_pool(struct iser_conn *ib_conn, unsigned cmds_max)
+int iser_create_fastreg_pool(struct iser_conn *ib_conn, unsigned cmds_max)
 {
 	struct iser_device	*device = ib_conn->device;
 	struct fast_reg_descriptor	*desc;
 	int i, ret;
 
-	INIT_LIST_HEAD(&ib_conn->fastreg.frwr.pool);
-	ib_conn->fastreg.frwr.pool_size = 0;
+	INIT_LIST_HEAD(&ib_conn->fastreg.pool);
+	ib_conn->fastreg.pool_size = 0;
 	for (i = 0; i < cmds_max; i++) {
 		desc = kmalloc(sizeof(*desc), GFP_KERNEL);
 		if (!desc) {
@@ -316,8 +316,8 @@ int iser_create_frwr_pool(struct iser_conn *ib_conn, unsigned cmds_max)
 			goto fast_reg_mr_failure;
 		}
 		desc->valid = true;
-		list_add_tail(&desc->list, &ib_conn->fastreg.frwr.pool);
-		ib_conn->fastreg.frwr.pool_size++;
+		list_add_tail(&desc->list, &ib_conn->fastreg.pool);
+		ib_conn->fastreg.pool_size++;
 	}
 
 	return 0;
@@ -327,24 +327,24 @@ fast_reg_mr_failure:
 fast_reg_page_failure:
 	kfree(desc);
 err:
-	iser_free_frwr_pool(ib_conn);
+	iser_free_fastreg_pool(ib_conn);
 	return ret;
 }
 
 /**
- * iser_free_frwr_pool - releases the pool of fast_reg descriptors
+ * iser_free_fastreg_pool - releases the pool of fast_reg descriptors
  */
-void iser_free_frwr_pool(struct iser_conn *ib_conn)
+void iser_free_fastreg_pool(struct iser_conn *ib_conn)
 {
 	struct fast_reg_descriptor *desc, *tmp;
 	int i = 0;
 
-	if (list_empty(&ib_conn->fastreg.frwr.pool))
+	if (list_empty(&ib_conn->fastreg.pool))
 		return;
 
-	iser_info("freeing conn %p frwr pool\n", ib_conn);
+	iser_info("freeing conn %p fr pool\n", ib_conn);
 
-	list_for_each_entry_safe(desc, tmp, &ib_conn->fastreg.frwr.pool, list) {
+	list_for_each_entry_safe(desc, tmp, &ib_conn->fastreg.pool, list) {
 		list_del(&desc->list);
 		ib_free_fast_reg_page_list(desc->data_frpl);
 		ib_dereg_mr(desc->data_mr);
@@ -352,9 +352,9 @@ void iser_free_frwr_pool(struct iser_conn *ib_conn)
 		++i;
 	}
 
-	if (i < ib_conn->fastreg.frwr.pool_size)
+	if (i < ib_conn->fastreg.pool_size)
 		iser_warn("pool still has %d regions registered\n",
-			  ib_conn->fastreg.frwr.pool_size - i);
+			  ib_conn->fastreg.pool_size - i);
 }
 
 /**
@@ -801,7 +801,7 @@ int iser_reg_page_vec(struct iser_conn     *ib_conn,
 	page_list = page_vec->pages;
 	io_addr	  = page_list[0];
 
-	mem  = ib_fmr_pool_map_phys(ib_conn->fastreg.fmr.pool,
+	mem  = ib_fmr_pool_map_phys(ib_conn->fmr.pool,
 				    page_list,
 				    page_vec->length,
 				    io_addr);
@@ -855,8 +855,8 @@ void iser_unreg_mem_fmr(struct iscsi_iser_task *iser_task,
 	reg->mem_h = NULL;
 }
 
-void iser_unreg_mem_frwr(struct iscsi_iser_task *iser_task,
-			 enum iser_data_dir cmd_dir)
+void iser_unreg_mem_fastreg(struct iscsi_iser_task *iser_task,
+			    enum iser_data_dir cmd_dir)
 {
 	struct iser_mem_reg *reg = &iser_task->rdma_regd[cmd_dir].reg;
 	struct iser_conn *ib_conn = iser_task->iser_conn->ib_conn;
@@ -868,7 +868,7 @@ void iser_unreg_mem_frwr(struct iscsi_iser_task *iser_task,
 	reg->mem_h = NULL;
 	reg->is_mr = 0;
 	spin_lock_bh(&ib_conn->lock);
-	list_add_tail(&desc->list, &ib_conn->fastreg.frwr.pool);
+	list_add_tail(&desc->list, &ib_conn->fastreg.pool);
 	spin_unlock_bh(&ib_conn->lock);
 }
 
@@ -999,7 +999,7 @@ static int iser_drain_tx_cq(struct iser_device  *device, int cq_index)
 		} else {
 			iser_err("tx id %llx status %d vend_err %x\n",
 				 wc.wr_id, wc.status, wc.vendor_err);
-			if (wc.wr_id != ISER_FRWR_LI_WRID) {
+			if (wc.wr_id != ISER_FASTREG_LI_WRID) {
 				atomic_dec(&ib_conn->post_send_buf_count);
 				iser_handle_comp_error(tx_desc, ib_conn);
 			}
-- 
1.7.1

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

[PATCH v1 02/13] IB/iser: Push the desicion what memory key to use into fast_reg_mr routine

[Sagi Grimberg]

This is a preparation step for T10-PI offload support. We prefer to push
the desicion of which mkey to use (global or fastreg) to iser_fast_reg_mr.
We choose to do this since it in T10-PI we may need to register for
protection buffers and in this case we wish to simplify iser_fast_reg_mr
instead of repeating the logic of which key to use.

This patch does not change any functionality.

Signed-off-by: Sagi Grimberg <sagig@mellanox.com>
Signed-off-by: Alex Tabachnik <alext@mellanox.com>
---
 drivers/infiniband/ulp/iser/iser_memory.c |  101 +++++++++++++++++------------
 1 files changed, 59 insertions(+), 42 deletions(-)

diff --git a/drivers/infiniband/ulp/iser/iser_memory.c b/drivers/infiniband/ulp/iser/iser_memory.c
index 6e9b7bc..d25587e 100644
--- a/drivers/infiniband/ulp/iser/iser_memory.c
+++ b/drivers/infiniband/ulp/iser/iser_memory.c
@@ -444,16 +444,40 @@ int iser_reg_rdma_mem_fmr(struct iscsi_iser_task *iser_task,
 	return 0;
 }
 
-static int iser_fast_reg_mr(struct fast_reg_descriptor *desc,
-			    struct iser_conn *ib_conn,
+static int iser_fast_reg_mr(struct iscsi_iser_task *iser_task,
 			    struct iser_regd_buf *regd_buf,
-			    u32 offset, unsigned int data_size,
-			    unsigned int page_list_len)
+			    struct iser_data_buf *mem,
+			    struct ib_sge *sge)
 {
+	struct fast_reg_descriptor *desc = regd_buf->reg.mem_h;
+	struct iser_conn *ib_conn = iser_task->iser_conn->ib_conn;
+	struct iser_device *device = ib_conn->device;
+	struct ib_device *ibdev = device->ib_device;
 	struct ib_send_wr fastreg_wr, inv_wr;
 	struct ib_send_wr *bad_wr, *wr = NULL;
 	u8 key;
-	int ret;
+	int ret, offset, size, plen;
+
+	/* if there a single dma entry, dma mr suffices */
+	if (mem->dma_nents == 1) {
+		struct scatterlist *sg = (struct scatterlist *)mem->buf;
+
+		sge->lkey = device->mr->lkey;
+		sge->addr   = ib_sg_dma_address(ibdev, &sg[0]);
+		sge->length  = ib_sg_dma_len(ibdev, &sg[0]);
+
+		iser_dbg("Single DMA entry: lkey=0x%x, addr=0x%llx, length=0x%x\n",
+			 sge->lkey, sge->addr, sge->length);
+		return 0;
+	}
+
+	plen = iser_sg_to_page_vec(mem, device->ib_device,
+				   desc->data_frpl->page_list,
+				   &offset, &size);
+	if (plen * SIZE_4K < size) {
+		iser_err("fast reg page_list too short to hold this SG\n");
+		return -EINVAL;
+	}
 
 	if (!desc->valid) {
 		memset(&inv_wr, 0, sizeof(inv_wr));
@@ -472,9 +496,9 @@ static int iser_fast_reg_mr(struct fast_reg_descriptor *desc,
 	fastreg_wr.opcode = IB_WR_FAST_REG_MR;
 	fastreg_wr.wr.fast_reg.iova_start = desc->data_frpl->page_list[0] + offset;
 	fastreg_wr.wr.fast_reg.page_list = desc->data_frpl;
-	fastreg_wr.wr.fast_reg.page_list_len = page_list_len;
+	fastreg_wr.wr.fast_reg.page_list_len = plen;
 	fastreg_wr.wr.fast_reg.page_shift = SHIFT_4K;
-	fastreg_wr.wr.fast_reg.length = data_size;
+	fastreg_wr.wr.fast_reg.length = size;
 	fastreg_wr.wr.fast_reg.rkey = desc->data_mr->rkey;
 	fastreg_wr.wr.fast_reg.access_flags = (IB_ACCESS_LOCAL_WRITE  |
 					       IB_ACCESS_REMOTE_WRITE |
@@ -492,12 +516,9 @@ static int iser_fast_reg_mr(struct fast_reg_descriptor *desc,
 	}
 	desc->valid = false;
 
-	regd_buf->reg.mem_h = desc;
-	regd_buf->reg.lkey = desc->data_mr->lkey;
-	regd_buf->reg.rkey = desc->data_mr->rkey;
-	regd_buf->reg.va = desc->data_frpl->page_list[0] + offset;
-	regd_buf->reg.len = data_size;
-	regd_buf->reg.is_mr = 1;
+	sge->lkey = desc->data_mr->lkey;
+	sge->addr = desc->data_frpl->page_list[0] + offset;
+	sge->length = size;
 
 	return ret;
 }
@@ -516,11 +537,10 @@ int iser_reg_rdma_mem_fastreg(struct iscsi_iser_task *iser_task,
 	struct ib_device *ibdev = device->ib_device;
 	struct iser_data_buf *mem = &iser_task->data[cmd_dir];
 	struct iser_regd_buf *regd_buf = &iser_task->rdma_regd[cmd_dir];
-	struct fast_reg_descriptor *desc;
-	unsigned int data_size, page_list_len;
+	struct fast_reg_descriptor *desc = NULL;
+	struct ib_sge data_sge;
 	int err, aligned_len;
 	unsigned long flags;
-	u32 offset;
 
 	aligned_len = iser_data_buf_aligned_len(mem, ibdev);
 	if (aligned_len != mem->dma_nents) {
@@ -533,41 +553,38 @@ int iser_reg_rdma_mem_fastreg(struct iscsi_iser_task *iser_task,
 		mem = &iser_task->data_copy[cmd_dir];
 	}
 
-	/* if there a single dma entry, dma mr suffices */
-	if (mem->dma_nents == 1) {
-		struct scatterlist *sg = (struct scatterlist *)mem->buf;
-
-		regd_buf->reg.lkey = device->mr->lkey;
-		regd_buf->reg.rkey = device->mr->rkey;
-		regd_buf->reg.len  = ib_sg_dma_len(ibdev, &sg[0]);
-		regd_buf->reg.va   = ib_sg_dma_address(ibdev, &sg[0]);
-		regd_buf->reg.is_mr = 0;
-	} else {
+	if (mem->dma_nents != 1) {
 		spin_lock_irqsave(&ib_conn->lock, flags);
 		desc = list_first_entry(&ib_conn->fastreg.pool,
 					struct fast_reg_descriptor, list);
 		list_del(&desc->list);
 		spin_unlock_irqrestore(&ib_conn->lock, flags);
-		page_list_len = iser_sg_to_page_vec(mem, device->ib_device,
-						    desc->data_frpl->page_list,
-						    &offset, &data_size);
-
-		if (page_list_len * SIZE_4K < data_size) {
-			iser_err("fast reg page_list too short to hold this SG\n");
-			err = -EINVAL;
-			goto err_reg;
-		}
+		regd_buf->reg.mem_h = desc;
+	}
 
-		err = iser_fast_reg_mr(desc, ib_conn, regd_buf,
-				       offset, data_size, page_list_len);
-		if (err)
-			goto err_reg;
+	err = iser_fast_reg_mr(iser_task, regd_buf, mem, &data_sge);
+	if (err)
+		goto err_reg;
+
+	if (desc) {
+		regd_buf->reg.rkey = desc->data_mr->rkey;
+		regd_buf->reg.is_mr = 1;
+	} else {
+		regd_buf->reg.rkey = device->mr->rkey;
+		regd_buf->reg.is_mr = 0;
 	}
 
+	regd_buf->reg.lkey = data_sge.lkey;
+	regd_buf->reg.va = data_sge.addr;
+	regd_buf->reg.len = data_sge.length;
+
 	return 0;
 err_reg:
-	spin_lock_irqsave(&ib_conn->lock, flags);
-	list_add_tail(&desc->list, &ib_conn->fastreg.pool);
-	spin_unlock_irqrestore(&ib_conn->lock, flags);
+	if (desc) {
+		spin_lock_irqsave(&ib_conn->lock, flags);
+		list_add_tail(&desc->list, &ib_conn->fastreg.pool);
+		spin_unlock_irqrestore(&ib_conn->lock, flags);
+	}
+
 	return err;
 }
-- 
1.7.1

[PATCH v1 03/13] IB/iser: Move fast_reg_descriptor initialization to a function

[Sagi Grimberg]

fastreg descriptor will include protection information context.
In order to place the logic in one place we introduce
iser_create_fr_desc function.

This patch does not change any functionality.

Signed-off-by: Sagi Grimberg <sagig@mellanox.com>
Signed-off-by: Alex Tabachnik <alext@mellanox.com>
---
 drivers/infiniband/ulp/iser/iser_verbs.c |   58 ++++++++++++++++++++---------
 1 files changed, 40 insertions(+), 18 deletions(-)

diff --git a/drivers/infiniband/ulp/iser/iser_verbs.c b/drivers/infiniband/ulp/iser/iser_verbs.c
index dc5a0b4..9569e40 100644
--- a/drivers/infiniband/ulp/iser/iser_verbs.c
+++ b/drivers/infiniband/ulp/iser/iser_verbs.c
@@ -279,6 +279,39 @@ void iser_free_fmr_pool(struct iser_conn *ib_conn)
 	ib_conn->fmr.page_vec = NULL;
 }
 
+static int
+iser_create_fastreg_desc(struct ib_device *ib_device, struct ib_pd *pd,
+			 struct fast_reg_descriptor *desc)
+{
+	int ret;
+
+	desc->data_frpl = ib_alloc_fast_reg_page_list(ib_device,
+						      ISCSI_ISER_SG_TABLESIZE + 1);
+	if (IS_ERR(desc->data_frpl)) {
+		ret = PTR_ERR(desc->data_frpl);
+		iser_err("Failed to allocate ib_fast_reg_page_list err=%d\n",
+			 ret);
+		return PTR_ERR(desc->data_frpl);
+	}
+
+	desc->data_mr = ib_alloc_fast_reg_mr(pd, ISCSI_ISER_SG_TABLESIZE + 1);
+	if (IS_ERR(desc->data_mr)) {
+		ret = PTR_ERR(desc->data_mr);
+		iser_err("Failed to allocate ib_fast_reg_mr err=%d\n", ret);
+		goto fast_reg_mr_failure;
+	}
+	iser_info("Create fr_desc %p page_list %p\n",
+		  desc, desc->data_frpl->page_list);
+	desc->valid = true;
+
+	return 0;
+
+fast_reg_mr_failure:
+	ib_free_fast_reg_page_list(desc->data_frpl);
+
+	return ret;
+}
+
 /**
  * iser_create_fastreg_pool - Creates pool of fast_reg descriptors
  * for fast registration work requests.
@@ -300,32 +333,21 @@ int iser_create_fastreg_pool(struct iser_conn *ib_conn, unsigned cmds_max)
 			goto err;
 		}
 
-		desc->data_frpl = ib_alloc_fast_reg_page_list(device->ib_device,
-							 ISCSI_ISER_SG_TABLESIZE + 1);
-		if (IS_ERR(desc->data_frpl)) {
-			ret = PTR_ERR(desc->data_frpl);
-			iser_err("Failed to allocate ib_fast_reg_page_list err=%d\n", ret);
-			goto fast_reg_page_failure;
+		ret = iser_create_fastreg_desc(device->ib_device,
+					       device->pd, desc);
+		if (ret) {
+			iser_err("Failed to create fastreg descriptor err=%d\n",
+				 ret);
+			kfree(desc);
+			goto err;
 		}
 
-		desc->data_mr = ib_alloc_fast_reg_mr(device->pd,
-						     ISCSI_ISER_SG_TABLESIZE + 1);
-		if (IS_ERR(desc->data_mr)) {
-			ret = PTR_ERR(desc->data_mr);
-			iser_err("Failed to allocate ib_fast_reg_mr err=%d\n", ret);
-			goto fast_reg_mr_failure;
-		}
-		desc->valid = true;
 		list_add_tail(&desc->list, &ib_conn->fastreg.pool);
 		ib_conn->fastreg.pool_size++;
 	}
 
 	return 0;
 
-fast_reg_mr_failure:
-	ib_free_fast_reg_page_list(desc->data_frpl);
-fast_reg_page_failure:
-	kfree(desc);
 err:
 	iser_free_fastreg_pool(ib_conn);
 	return ret;
-- 
1.7.1

[PATCH v1 04/13] IB/iser: Keep IB device attributes under iser_device

[Sagi Grimberg]

For T10-PI offload support, we will need to know the
device signature offload capability upon every connection
establishment.

This patch does not change any functionality.

Signed-off-by: Sagi Grimberg <sagig@mellanox.com>
Signed-off-by: Alex Tabachnik <alext@mellanox.com>
---
 drivers/infiniband/ulp/iser/iscsi_iser.h |    1 +
 drivers/infiniband/ulp/iser/iser_verbs.c |   18 ++++++------------
 2 files changed, 7 insertions(+), 12 deletions(-)

diff --git a/drivers/infiniband/ulp/iser/iscsi_iser.h b/drivers/infiniband/ulp/iser/iscsi_iser.h
index ca161df..b4290f5 100644
--- a/drivers/infiniband/ulp/iser/iscsi_iser.h
+++ b/drivers/infiniband/ulp/iser/iscsi_iser.h
@@ -260,6 +260,7 @@ struct iscsi_iser_task;
 struct iser_device {
 	struct ib_device             *ib_device;
 	struct ib_pd	             *pd;
+	struct ib_device_attr	     dev_attr;
 	struct ib_cq	             *rx_cq[ISER_MAX_CQ];
 	struct ib_cq	             *tx_cq[ISER_MAX_CQ];
 	struct ib_mr	             *mr;
diff --git a/drivers/infiniband/ulp/iser/iser_verbs.c b/drivers/infiniband/ulp/iser/iser_verbs.c
index 9569e40..95fcfca 100644
--- a/drivers/infiniband/ulp/iser/iser_verbs.c
+++ b/drivers/infiniband/ulp/iser/iser_verbs.c
@@ -71,17 +71,14 @@ static void iser_event_handler(struct ib_event_handler *handler,
  */
 static int iser_create_device_ib_res(struct iser_device *device)
 {
-	int i, j;
 	struct iser_cq_desc *cq_desc;
-	struct ib_device_attr *dev_attr;
+	struct ib_device_attr *dev_attr = &device->dev_attr;
+	int ret, i, j;
 
-	dev_attr = kmalloc(sizeof(*dev_attr), GFP_KERNEL);
-	if (!dev_attr)
-		return -ENOMEM;
-
-	if (ib_query_device(device->ib_device, dev_attr)) {
+	ret = ib_query_device(device->ib_device, dev_attr);
+	if (ret) {
 		pr_warn("Query device failed for %s\n", device->ib_device->name);
-		goto dev_attr_err;
+		return ret;
 	}
 
 	/* Assign function handles  - based on FMR support */
@@ -101,7 +98,7 @@ static int iser_create_device_ib_res(struct iser_device *device)
 		device->iser_unreg_rdma_mem = iser_unreg_mem_fastreg;
 	} else {
 		iser_err("IB device does not support FMRs nor FastRegs, can't register memory\n");
-		goto dev_attr_err;
+		return -1;
 	}
 
 	device->cqs_used = min(ISER_MAX_CQ, device->ib_device->num_comp_vectors);
@@ -158,7 +155,6 @@ static int iser_create_device_ib_res(struct iser_device *device)
 	if (ib_register_event_handler(&device->event_handler))
 		goto handler_err;
 
-	kfree(dev_attr);
 	return 0;
 
 handler_err:
@@ -178,8 +174,6 @@ pd_err:
 	kfree(device->cq_desc);
 cq_desc_err:
 	iser_err("failed to allocate an IB resource\n");
-dev_attr_err:
-	kfree(dev_attr);
 	return -1;
 }
 
-- 
1.7.1

[PATCH v1 05/13] IB/iser: Replace fastreg descriptor valid bool with indicators container

[Sagi Grimberg]

In T10-PI support we will have memory keys for protection
buffers and signature transactions. We prefer to compact
indicators rather than keeping multiple bools.

This commit does not change any functionality.

Signed-off-by: Sagi Grimberg <sagig@mellanox.com>
Signed-off-by: Alex Tabachnik <alext@mellanox.com>
---
 drivers/infiniband/ulp/iser/iscsi_iser.h  |    8 ++++++--
 drivers/infiniband/ulp/iser/iser_memory.c |    4 ++--
 drivers/infiniband/ulp/iser/iser_verbs.c  |    2 +-
 3 files changed, 9 insertions(+), 5 deletions(-)

diff --git a/drivers/infiniband/ulp/iser/iscsi_iser.h b/drivers/infiniband/ulp/iser/iscsi_iser.h
index b4290f5..5660714 100644
--- a/drivers/infiniband/ulp/iser/iscsi_iser.h
+++ b/drivers/infiniband/ulp/iser/iscsi_iser.h
@@ -280,13 +280,17 @@ struct iser_device {
 							    enum iser_data_dir cmd_dir);
 };
 
+enum iser_reg_indicator {
+	ISER_DATA_KEY_VALID = 1 << 0,
+};
+
 struct fast_reg_descriptor {
 	struct list_head		  list;
 	/* For fast registration - FRWR */
 	struct ib_mr			 *data_mr;
 	struct ib_fast_reg_page_list     *data_frpl;
-	/* Valid for fast registration flag */
-	bool				  valid;
+	/* registration indicators container */
+	u8				  reg_indicators;
 };
 
 struct iser_conn {
diff --git a/drivers/infiniband/ulp/iser/iser_memory.c b/drivers/infiniband/ulp/iser/iser_memory.c
index d25587e..a7a0d3e 100644
--- a/drivers/infiniband/ulp/iser/iser_memory.c
+++ b/drivers/infiniband/ulp/iser/iser_memory.c
@@ -479,7 +479,7 @@ static int iser_fast_reg_mr(struct iscsi_iser_task *iser_task,
 		return -EINVAL;
 	}
 
-	if (!desc->valid) {
+	if (!(desc->reg_indicators & ISER_DATA_KEY_VALID)) {
 		memset(&inv_wr, 0, sizeof(inv_wr));
 		inv_wr.wr_id = ISER_FASTREG_LI_WRID;
 		inv_wr.opcode = IB_WR_LOCAL_INV;
@@ -514,7 +514,7 @@ static int iser_fast_reg_mr(struct iscsi_iser_task *iser_task,
 		iser_err("fast registration failed, ret:%d\n", ret);
 		return ret;
 	}
-	desc->valid = false;
+	desc->reg_indicators &= ~ISER_DATA_KEY_VALID;
 
 	sge->lkey = desc->data_mr->lkey;
 	sge->addr = desc->data_frpl->page_list[0] + offset;
diff --git a/drivers/infiniband/ulp/iser/iser_verbs.c b/drivers/infiniband/ulp/iser/iser_verbs.c
index 95fcfca..6a5f424 100644
--- a/drivers/infiniband/ulp/iser/iser_verbs.c
+++ b/drivers/infiniband/ulp/iser/iser_verbs.c
@@ -296,7 +296,7 @@ iser_create_fastreg_desc(struct ib_device *ib_device, struct ib_pd *pd,
 	}
 	iser_info("Create fr_desc %p page_list %p\n",
 		  desc, desc->data_frpl->page_list);
-	desc->valid = true;
+	desc->reg_indicators |= ISER_DATA_KEY_VALID;
 
 	return 0;
 
-- 
1.7.1

[PATCH v1 06/13] IB/iser: Generalize iser_unmap_task_data and finalize_rdma_unaligned_sg

[Sagi Grimberg]

This routines operates on data buffers and may also work with
protection infomation buffers. So we generalize them to handle
an iser_data_buf which can be the command data or command protection
information.

This patch does not change any functionality.

Signed-off-by: Sagi Grimberg <sagig-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
---
 drivers/infiniband/ulp/iser/iscsi_iser.h     |    9 ++++--
 drivers/infiniband/ulp/iser/iser_initiator.c |   37 +++++++++++++++++-------
 drivers/infiniband/ulp/iser/iser_memory.c    |   40 ++++++++++---------------
 3 files changed, 48 insertions(+), 38 deletions(-)

diff --git a/drivers/infiniband/ulp/iser/iscsi_iser.h b/drivers/infiniband/ulp/iser/iscsi_iser.h
index 5660714..623defa 100644
--- a/drivers/infiniband/ulp/iser/iscsi_iser.h
+++ b/drivers/infiniband/ulp/iser/iscsi_iser.h
@@ -410,8 +410,10 @@ void iser_task_rdma_finalize(struct iscsi_iser_task *task);
 
 void iser_free_rx_descriptors(struct iser_conn *ib_conn);
 
-void iser_finalize_rdma_unaligned_sg(struct iscsi_iser_task *task,
-				     enum iser_data_dir         cmd_dir);
+void iser_finalize_rdma_unaligned_sg(struct iscsi_iser_task *iser_task,
+				     struct iser_data_buf *mem,
+				     struct iser_data_buf *mem_copy,
+				     enum iser_data_dir cmd_dir);
 
 int  iser_reg_rdma_mem_fmr(struct iscsi_iser_task *task,
 			   enum iser_data_dir cmd_dir);
@@ -441,7 +443,8 @@ int iser_dma_map_task_data(struct iscsi_iser_task *iser_task,
 			    enum   iser_data_dir       iser_dir,
 			    enum   dma_data_direction  dma_dir);
 
-void iser_dma_unmap_task_data(struct iscsi_iser_task *iser_task);
+void iser_dma_unmap_task_data(struct iscsi_iser_task *iser_task,
+			      struct iser_data_buf *data);
 int  iser_initialize_task_headers(struct iscsi_task *task,
 			struct iser_tx_desc *tx_desc);
 int iser_alloc_rx_descriptors(struct iser_conn *ib_conn, struct iscsi_session *session);
diff --git a/drivers/infiniband/ulp/iser/iser_initiator.c b/drivers/infiniband/ulp/iser/iser_initiator.c
index 334f34b..58e14c7 100644
--- a/drivers/infiniband/ulp/iser/iser_initiator.c
+++ b/drivers/infiniband/ulp/iser/iser_initiator.c
@@ -644,27 +644,42 @@ void iser_task_rdma_init(struct iscsi_iser_task *iser_task)
 void iser_task_rdma_finalize(struct iscsi_iser_task *iser_task)
 {
 	struct iser_device *device = iser_task->iser_conn->ib_conn->device;
-	int is_rdma_aligned = 1;
+	int is_rdma_data_aligned = 1;
 
 	/* if we were reading, copy back to unaligned sglist,
 	 * anyway dma_unmap and free the copy
 	 */
 	if (iser_task->data_copy[ISER_DIR_IN].copy_buf != NULL) {
-		is_rdma_aligned = 0;
-		iser_finalize_rdma_unaligned_sg(iser_task, ISER_DIR_IN);
+		is_rdma_data_aligned = 0;
+		iser_finalize_rdma_unaligned_sg(iser_task,
+						&iser_task->data[ISER_DIR_IN],
+						&iser_task->data_copy[ISER_DIR_IN],
+						ISER_DIR_IN);
 	}
+
 	if (iser_task->data_copy[ISER_DIR_OUT].copy_buf != NULL) {
-		is_rdma_aligned = 0;
-		iser_finalize_rdma_unaligned_sg(iser_task, ISER_DIR_OUT);
+		is_rdma_data_aligned = 0;
+		iser_finalize_rdma_unaligned_sg(iser_task,
+						&iser_task->data[ISER_DIR_OUT],
+						&iser_task->data_copy[ISER_DIR_OUT],
+						ISER_DIR_OUT);
 	}
 
-	if (iser_task->dir[ISER_DIR_IN])
+	if (iser_task->dir[ISER_DIR_IN]) {
 		device->iser_unreg_rdma_mem(iser_task, ISER_DIR_IN);
+		if (is_rdma_data_aligned)
+			iser_dma_unmap_task_data(iser_task,
+						 &iser_task->data[ISER_DIR_IN]);
 
-	if (iser_task->dir[ISER_DIR_OUT])
-		device->iser_unreg_rdma_mem(iser_task, ISER_DIR_OUT);
+	}
 
-       /* if the data was unaligned, it was already unmapped and then copied */
-       if (is_rdma_aligned)
-		iser_dma_unmap_task_data(iser_task);
+	if (iser_task->dir[ISER_DIR_OUT]) {
+		device->iser_unreg_rdma_mem(iser_task, ISER_DIR_OUT);
+		if (is_rdma_data_aligned)
+			iser_dma_unmap_task_data(iser_task,
+						 &iser_task->data[ISER_DIR_OUT]);
+		if (prot_count && is_rdma_prot_aligned)
+			iser_dma_unmap_task_data(iser_task,
+						 &iser_task->prot[ISER_DIR_OUT]);
+	}
 }
diff --git a/drivers/infiniband/ulp/iser/iser_memory.c b/drivers/infiniband/ulp/iser/iser_memory.c
index a7a0d3e..a933508 100644
--- a/drivers/infiniband/ulp/iser/iser_memory.c
+++ b/drivers/infiniband/ulp/iser/iser_memory.c
@@ -105,17 +105,18 @@ static int iser_start_rdma_unaligned_sg(struct iscsi_iser_task *iser_task,
 /**
  * iser_finalize_rdma_unaligned_sg
  */
+
 void iser_finalize_rdma_unaligned_sg(struct iscsi_iser_task *iser_task,
-				     enum iser_data_dir         cmd_dir)
+				     struct iser_data_buf *data,
+				     struct iser_data_buf *data_copy,
+				     enum iser_data_dir cmd_dir)
 {
 	struct ib_device *dev;
-	struct iser_data_buf *mem_copy;
 	unsigned long  cmd_data_len;
 
 	dev = iser_task->iser_conn->ib_conn->device->ib_device;
-	mem_copy = &iser_task->data_copy[cmd_dir];
 
-	ib_dma_unmap_sg(dev, &mem_copy->sg_single, 1,
+	ib_dma_unmap_sg(dev, &data_copy->sg_single, 1,
 			(cmd_dir == ISER_DIR_OUT) ?
 			DMA_TO_DEVICE : DMA_FROM_DEVICE);
 
@@ -127,10 +128,10 @@ void iser_finalize_rdma_unaligned_sg(struct iscsi_iser_task *iser_task,
 		int i;
 
 		/* copy back read RDMA to unaligned sg */
-		mem	= mem_copy->copy_buf;
+		mem = data_copy->copy_buf;
 
-		sgl	= (struct scatterlist *)iser_task->data[ISER_DIR_IN].buf;
-		sg_size = iser_task->data[ISER_DIR_IN].size;
+		sgl = (struct scatterlist *)data->buf;
+		sg_size = data->size;
 
 		p = mem;
 		for_each_sg(sgl, sg, sg_size, i) {
@@ -143,15 +144,15 @@ void iser_finalize_rdma_unaligned_sg(struct iscsi_iser_task *iser_task,
 		}
 	}
 
-	cmd_data_len = iser_task->data[cmd_dir].data_len;
+	cmd_data_len = data->data_len;
 
 	if (cmd_data_len > ISER_KMALLOC_THRESHOLD)
-		free_pages((unsigned long)mem_copy->copy_buf,
+		free_pages((unsigned long)data_copy->copy_buf,
 			   ilog2(roundup_pow_of_two(cmd_data_len)) - PAGE_SHIFT);
 	else
-		kfree(mem_copy->copy_buf);
+		kfree(data_copy->copy_buf);
 
-	mem_copy->copy_buf = NULL;
+	data_copy->copy_buf = NULL;
 }
 
 #define IS_4K_ALIGNED(addr)	((((unsigned long)addr) & ~MASK_4K) == 0)
@@ -329,22 +330,13 @@ int iser_dma_map_task_data(struct iscsi_iser_task *iser_task,
 	return 0;
 }
 
-void iser_dma_unmap_task_data(struct iscsi_iser_task *iser_task)
+void iser_dma_unmap_task_data(struct iscsi_iser_task *iser_task,
+			      struct iser_data_buf *data)
 {
 	struct ib_device *dev;
-	struct iser_data_buf *data;
 
 	dev = iser_task->iser_conn->ib_conn->device->ib_device;
-
-	if (iser_task->dir[ISER_DIR_IN]) {
-		data = &iser_task->data[ISER_DIR_IN];
-		ib_dma_unmap_sg(dev, data->buf, data->size, DMA_FROM_DEVICE);
-	}
-
-	if (iser_task->dir[ISER_DIR_OUT]) {
-		data = &iser_task->data[ISER_DIR_OUT];
-		ib_dma_unmap_sg(dev, data->buf, data->size, DMA_TO_DEVICE);
-	}
+	ib_dma_unmap_sg(dev, data->buf, data->size, DMA_FROM_DEVICE);
 }
 
 static int fall_to_bounce_buf(struct iscsi_iser_task *iser_task,
@@ -363,7 +355,7 @@ static int fall_to_bounce_buf(struct iscsi_iser_task *iser_task,
 		iser_data_buf_dump(mem, ibdev);
 
 	/* unmap the command data before accessing it */
-	iser_dma_unmap_task_data(iser_task);
+	iser_dma_unmap_task_data(iser_task, &iser_task->data[cmd_dir]);
 
 	/* allocate copy buf, if we are writing, copy the */
 	/* unaligned scatterlist, dma map the copy        */
-- 
1.7.1

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

[PATCH v1 07/13] IB/iser: Generalize fall_to_bounce_buf routine

[Sagi Grimberg]

Unaligned SG-lists may also happen for protection
information. Genrelize bounce buffer routine to handle
any iser_data_buf which may be data and/or protection.

This patch does not change any functionality.

Signed-off-by: Sagi Grimberg <sagig-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
---
 drivers/infiniband/ulp/iser/iser_memory.c |   53 ++++++++++++++++-------------
 1 files changed, 29 insertions(+), 24 deletions(-)

diff --git a/drivers/infiniband/ulp/iser/iser_memory.c b/drivers/infiniband/ulp/iser/iser_memory.c
index a933508..2c3f4b1 100644
--- a/drivers/infiniband/ulp/iser/iser_memory.c
+++ b/drivers/infiniband/ulp/iser/iser_memory.c
@@ -45,13 +45,19 @@
  * iser_start_rdma_unaligned_sg
  */
 static int iser_start_rdma_unaligned_sg(struct iscsi_iser_task *iser_task,
+					struct iser_data_buf *data,
+					struct iser_data_buf *data_copy,
 					enum iser_data_dir cmd_dir)
 {
-	int dma_nents;
-	struct ib_device *dev;
+	struct ib_device *dev = iser_task->iser_conn->ib_conn->device->ib_device;
+	struct scatterlist *sgl = (struct scatterlist *)data->buf;
+	struct scatterlist *sg;
 	char *mem = NULL;
-	struct iser_data_buf *data = &iser_task->data[cmd_dir];
-	unsigned long  cmd_data_len = data->data_len;
+	unsigned long  cmd_data_len = 0;
+	int dma_nents, i;
+
+	for_each_sg(sgl, sg, data->size, i)
+		cmd_data_len += ib_sg_dma_len(dev, sg);
 
 	if (cmd_data_len > ISER_KMALLOC_THRESHOLD)
 		mem = (void *)__get_free_pages(GFP_ATOMIC,
@@ -61,17 +67,16 @@ static int iser_start_rdma_unaligned_sg(struct iscsi_iser_task *iser_task,
 
 	if (mem == NULL) {
 		iser_err("Failed to allocate mem size %d %d for copying sglist\n",
-			 data->size,(int)cmd_data_len);
+			 data->size, (int)cmd_data_len);
 		return -ENOMEM;
 	}
 
 	if (cmd_dir == ISER_DIR_OUT) {
 		/* copy the unaligned sg the buffer which is used for RDMA */
-		struct scatterlist *sgl = (struct scatterlist *)data->buf;
-		struct scatterlist *sg;
 		int i;
 		char *p, *from;
 
+		sgl = (struct scatterlist *)data->buf;
 		p = mem;
 		for_each_sg(sgl, sg, data->size, i) {
 			from = kmap_atomic(sg_page(sg));
@@ -83,22 +88,19 @@ static int iser_start_rdma_unaligned_sg(struct iscsi_iser_task *iser_task,
 		}
 	}
 
-	sg_init_one(&iser_task->data_copy[cmd_dir].sg_single, mem, cmd_data_len);
-	iser_task->data_copy[cmd_dir].buf  =
-		&iser_task->data_copy[cmd_dir].sg_single;
-	iser_task->data_copy[cmd_dir].size = 1;
+	sg_init_one(&data_copy->sg_single, mem, cmd_data_len);
+	data_copy->buf = &data_copy->sg_single;
+	data_copy->size = 1;
+	data_copy->copy_buf = mem;
 
-	iser_task->data_copy[cmd_dir].copy_buf  = mem;
-
-	dev = iser_task->iser_conn->ib_conn->device->ib_device;
-	dma_nents = ib_dma_map_sg(dev,
-				  &iser_task->data_copy[cmd_dir].sg_single,
-				  1,
+	dma_nents = ib_dma_map_sg(dev, &data_copy->sg_single, 1,
 				  (cmd_dir == ISER_DIR_OUT) ?
 				  DMA_TO_DEVICE : DMA_FROM_DEVICE);
 	BUG_ON(dma_nents == 0);
 
-	iser_task->data_copy[cmd_dir].dma_nents = dma_nents;
+	data_copy->dma_nents = dma_nents;
+	data_copy->data_len = cmd_data_len;
+
 	return 0;
 }
 
@@ -341,11 +343,12 @@ void iser_dma_unmap_task_data(struct iscsi_iser_task *iser_task,
 
 static int fall_to_bounce_buf(struct iscsi_iser_task *iser_task,
 			      struct ib_device *ibdev,
+			      struct iser_data_buf *mem,
+			      struct iser_data_buf *mem_copy,
 			      enum iser_data_dir cmd_dir,
 			      int aligned_len)
 {
 	struct iscsi_conn    *iscsi_conn = iser_task->iser_conn->iscsi_conn;
-	struct iser_data_buf *mem = &iser_task->data[cmd_dir];
 
 	iscsi_conn->fmr_unalign_cnt++;
 	iser_warn("rdma alignment violation (%d/%d aligned) or FMR not supported\n",
@@ -355,12 +358,12 @@ static int fall_to_bounce_buf(struct iscsi_iser_task *iser_task,
 		iser_data_buf_dump(mem, ibdev);
 
 	/* unmap the command data before accessing it */
-	iser_dma_unmap_task_data(iser_task, &iser_task->data[cmd_dir]);
+	iser_dma_unmap_task_data(iser_task, mem);
 
 	/* allocate copy buf, if we are writing, copy the */
 	/* unaligned scatterlist, dma map the copy        */
-	if (iser_start_rdma_unaligned_sg(iser_task, cmd_dir) != 0)
-			return -ENOMEM;
+	if (iser_start_rdma_unaligned_sg(iser_task, mem, mem_copy, cmd_dir) != 0)
+		return -ENOMEM;
 
 	return 0;
 }
@@ -388,7 +391,8 @@ int iser_reg_rdma_mem_fmr(struct iscsi_iser_task *iser_task,
 
 	aligned_len = iser_data_buf_aligned_len(mem, ibdev);
 	if (aligned_len != mem->dma_nents) {
-		err = fall_to_bounce_buf(iser_task, ibdev,
+		err = fall_to_bounce_buf(iser_task, ibdev, mem,
+					 &iser_task->data_copy[cmd_dir],
 					 cmd_dir, aligned_len);
 		if (err) {
 			iser_err("failed to allocate bounce buffer\n");
@@ -536,7 +540,8 @@ int iser_reg_rdma_mem_fastreg(struct iscsi_iser_task *iser_task,
 
 	aligned_len = iser_data_buf_aligned_len(mem, ibdev);
 	if (aligned_len != mem->dma_nents) {
-		err = fall_to_bounce_buf(iser_task, ibdev,
+		err = fall_to_bounce_buf(iser_task, ibdev, mem,
+					 &iser_task->data_copy[cmd_dir],
 					 cmd_dir, aligned_len);
 		if (err) {
 			iser_err("failed to allocate bounce buffer\n");
-- 
1.7.1

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

[PATCH v1 08/13] IB/iser: Introduce pi_enable, pi_guard module parameters

[Sagi Grimberg]

From: Alex Tabachnik <alext-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>

Use modparams to activate protection information support.

pi_enable bool: Based on this parameter iSER will know
if it should support T10-PI. We don't want to do this by
default as it requires to allocate and initiatlize extra
resources. In case pi_enable=N, iSER won't publish to
SCSI midlayer any DIF capabilities.

pi_guard int: Based on this parameter iSER will publish
DIX guard type support to SCSI midlayer. 0 means CRC is
allowed to be passed in DIX buffers, 1 (or non-zero)
means IP-CSUM is allowed to be passed in DIX buffers.
Note that over the wire, only CRC is allowed.

In the next phase, it is worth considering passing these
parameters from iscsid via nlmsg. This will allow these
parameters to be connection based rather than global.

Signed-off-by: Alex Tabachnik <alext-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
Signed-off-by: Sagi Grimberg <sagig-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
---
 drivers/infiniband/ulp/iser/iscsi_iser.c |    8 ++++++++
 drivers/infiniband/ulp/iser/iscsi_iser.h |    3 +++
 drivers/infiniband/ulp/iser/iser_verbs.c |   13 +++++++++++++
 3 files changed, 24 insertions(+), 0 deletions(-)

diff --git a/drivers/infiniband/ulp/iser/iscsi_iser.c b/drivers/infiniband/ulp/iser/iscsi_iser.c
index dd03cfe..cfa952e 100644
--- a/drivers/infiniband/ulp/iser/iscsi_iser.c
+++ b/drivers/infiniband/ulp/iser/iscsi_iser.c
@@ -82,6 +82,8 @@ static unsigned int iscsi_max_lun = 512;
 module_param_named(max_lun, iscsi_max_lun, uint, S_IRUGO);
 
 int iser_debug_level = 0;
+bool iser_pi_enable = false;
+int iser_pi_guard = 0;
 
 MODULE_DESCRIPTION("iSER (iSCSI Extensions for RDMA) Datamover");
 MODULE_LICENSE("Dual BSD/GPL");
@@ -91,6 +93,12 @@ MODULE_VERSION(DRV_VER);
 module_param_named(debug_level, iser_debug_level, int, 0644);
 MODULE_PARM_DESC(debug_level, "Enable debug tracing if > 0 (default:disabled)");
 
+module_param_named(pi_enable, iser_pi_enable, bool, 0644);
+MODULE_PARM_DESC(pi_enable, "Enable T10-PI offload support (default:disabled)");
+
+module_param_named(pi_guard, iser_pi_guard, int, 0644);
+MODULE_PARM_DESC(pi_guard, "T10-PI guard_type, 0:CRC|1:IP_CSUM (default:CRC)");
+
 struct iser_global ig;
 
 void
diff --git a/drivers/infiniband/ulp/iser/iscsi_iser.h b/drivers/infiniband/ulp/iser/iscsi_iser.h
index 623defa..011003f 100644
--- a/drivers/infiniband/ulp/iser/iscsi_iser.h
+++ b/drivers/infiniband/ulp/iser/iscsi_iser.h
@@ -317,6 +317,7 @@ struct iser_conn {
 	unsigned int 		     rx_desc_head;
 	struct iser_rx_desc	     *rx_descs;
 	struct ib_recv_wr	     rx_wr[ISER_MIN_POSTED_RX];
+	bool			     pi_support;
 
 	/* Connection memory registration pool */
 	union {
@@ -371,6 +372,8 @@ struct iser_global {
 
 extern struct iser_global ig;
 extern int iser_debug_level;
+extern bool iser_pi_enable;
+extern int iser_pi_guard;
 
 /* allocate connection resources needed for rdma functionality */
 int iser_conn_set_full_featured_mode(struct iscsi_conn *conn);
diff --git a/drivers/infiniband/ulp/iser/iser_verbs.c b/drivers/infiniband/ulp/iser/iser_verbs.c
index 6a5f424..4c27f55 100644
--- a/drivers/infiniband/ulp/iser/iser_verbs.c
+++ b/drivers/infiniband/ulp/iser/iser_verbs.c
@@ -607,6 +607,19 @@ static int iser_addr_handler(struct rdma_cm_id *cma_id)
 	ib_conn = (struct iser_conn *)cma_id->context;
 	ib_conn->device = device;
 
+	/* connection T10-PI support */
+	if (iser_pi_enable) {
+		if (!(device->dev_attr.device_cap_flags &
+		      IB_DEVICE_SIGNATURE_HANDOVER)) {
+			iser_warn("T10-PI requested but not supported on %s, "
+				  "continue without T10-PI\n",
+				  ib_conn->device->ib_device->name);
+			ib_conn->pi_support = false;
+		} else {
+			ib_conn->pi_support = true;
+		}
+	}
+
 	ret = rdma_resolve_route(cma_id, 1000);
 	if (ret) {
 		iser_err("resolve route failed: %d\n", ret);
-- 
1.7.1

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

[PATCH v1 09/13] IB/iser: Initialize T10-PI resources

[Sagi Grimberg]

From: Alex Tabachnik <alext@mellanox.com>

During connection establishment we also initiatlize
T10-PI resources (QP, PI contexts) in order to support
SCSI's protection operations.

Signed-off-by: Alex Tabachnik <alext@mellanox.com>
Signed-off-by: Sagi Grimberg <sagig@mellanox.com>
---
 drivers/infiniband/ulp/iser/iscsi_iser.h |   21 ++++++++-
 drivers/infiniband/ulp/iser/iser_verbs.c |   77 +++++++++++++++++++++++++++---
 2 files changed, 90 insertions(+), 8 deletions(-)

diff --git a/drivers/infiniband/ulp/iser/iscsi_iser.h b/drivers/infiniband/ulp/iser/iscsi_iser.h
index 011003f..99fc8b8 100644
--- a/drivers/infiniband/ulp/iser/iscsi_iser.h
+++ b/drivers/infiniband/ulp/iser/iscsi_iser.h
@@ -134,6 +134,15 @@
 					ISER_MAX_TX_MISC_PDUS        + \
 					ISER_MAX_RX_MISC_PDUS)
 
+/* Max registration work requests per command */
+#define ISER_MAX_REG_WR_PER_CMD		5
+
+/* For Signature we don't support DATAOUTs so no need to make room for them */
+#define ISER_QP_SIG_MAX_REQ_DTOS	(ISER_DEF_XMIT_CMDS_MAX	*       \
+					(1 + ISER_MAX_REG_WR_PER_CMD) + \
+					ISER_MAX_TX_MISC_PDUS         + \
+					ISER_MAX_RX_MISC_PDUS)
+
 #define ISER_VER			0x10
 #define ISER_WSV			0x08
 #define ISER_RSV			0x04
@@ -281,7 +290,16 @@ struct iser_device {
 };
 
 enum iser_reg_indicator {
-	ISER_DATA_KEY_VALID = 1 << 0,
+	ISER_DATA_KEY_VALID	= 1 << 0,
+	ISER_PROT_KEY_VALID	= 1 << 1,
+	ISER_SIG_KEY_VALID	= 1 << 2,
+	ISER_FASTREG_PROTECTED	= 1 << 3,
+};
+
+struct iser_pi_context {
+	struct ib_mr                   *prot_mr;
+	struct ib_fast_reg_page_list   *prot_frpl;
+	struct ib_mr                   *sig_mr;
 };
 
 struct fast_reg_descriptor {
@@ -289,6 +307,7 @@ struct fast_reg_descriptor {
 	/* For fast registration - FRWR */
 	struct ib_mr			 *data_mr;
 	struct ib_fast_reg_page_list     *data_frpl;
+	struct iser_pi_context		 *pi_ctx;
 	/* registration indicators container */
 	u8				  reg_indicators;
 };
diff --git a/drivers/infiniband/ulp/iser/iser_verbs.c b/drivers/infiniband/ulp/iser/iser_verbs.c
index 4c27f55..0404c71 100644
--- a/drivers/infiniband/ulp/iser/iser_verbs.c
+++ b/drivers/infiniband/ulp/iser/iser_verbs.c
@@ -275,7 +275,7 @@ void iser_free_fmr_pool(struct iser_conn *ib_conn)
 
 static int
 iser_create_fastreg_desc(struct ib_device *ib_device, struct ib_pd *pd,
-			 struct fast_reg_descriptor *desc)
+			 bool pi_enable, struct fast_reg_descriptor *desc)
 {
 	int ret;
 
@@ -294,12 +294,64 @@ iser_create_fastreg_desc(struct ib_device *ib_device, struct ib_pd *pd,
 		iser_err("Failed to allocate ib_fast_reg_mr err=%d\n", ret);
 		goto fast_reg_mr_failure;
 	}
+	desc->reg_indicators |= ISER_DATA_KEY_VALID;
+
+	if (pi_enable) {
+		struct ib_mr_init_attr mr_init_attr = {0};
+		struct iser_pi_context *pi_ctx = NULL;
+
+		desc->pi_ctx = kzalloc(sizeof(*desc->pi_ctx), GFP_KERNEL);
+		if (!desc->pi_ctx) {
+			iser_err("Failed to allocate pi context\n");
+			ret = -ENOMEM;
+			goto pi_ctx_alloc_failure;
+		}
+		pi_ctx = desc->pi_ctx;
+
+		pi_ctx->prot_frpl = ib_alloc_fast_reg_page_list(ib_device,
+						    ISCSI_ISER_SG_TABLESIZE);
+		if (IS_ERR(pi_ctx->prot_frpl)) {
+			ret = PTR_ERR(pi_ctx->prot_frpl);
+			iser_err("Failed to allocate prot frpl ret=%d\n",
+				 ret);
+			goto prot_frpl_failure;
+		}
+
+		pi_ctx->prot_mr = ib_alloc_fast_reg_mr(pd,
+						ISCSI_ISER_SG_TABLESIZE + 1);
+		if (IS_ERR(pi_ctx->prot_mr)) {
+			ret = PTR_ERR(pi_ctx->prot_mr);
+			iser_err("Failed to allocate prot frmr ret=%d\n",
+				 ret);
+			goto prot_mr_failure;
+		}
+		desc->reg_indicators |= ISER_PROT_KEY_VALID;
+
+		mr_init_attr.max_reg_descriptors = 2;
+		mr_init_attr.flags |= IB_MR_SIGNATURE_EN;
+		pi_ctx->sig_mr = ib_create_mr(pd, &mr_init_attr);
+		if (IS_ERR(pi_ctx->sig_mr)) {
+			ret = PTR_ERR(pi_ctx->sig_mr);
+			iser_err("Failed to allocate signature enabled mr err=%d\n",
+				 ret);
+			goto sig_mr_failure;
+		}
+		desc->reg_indicators |= ISER_SIG_KEY_VALID;
+	}
+	desc->reg_indicators &= ~ISER_FASTREG_PROTECTED;
+
 	iser_info("Create fr_desc %p page_list %p\n",
 		  desc, desc->data_frpl->page_list);
-	desc->reg_indicators |= ISER_DATA_KEY_VALID;
 
 	return 0;
-
+sig_mr_failure:
+	ib_dereg_mr(desc->pi_ctx->prot_mr);
+prot_mr_failure:
+	ib_free_fast_reg_page_list(desc->pi_ctx->prot_frpl);
+prot_frpl_failure:
+	kfree(desc->pi_ctx);
+pi_ctx_alloc_failure:
+	ib_dereg_mr(desc->data_mr);
 fast_reg_mr_failure:
 	ib_free_fast_reg_page_list(desc->data_frpl);
 
@@ -320,15 +372,15 @@ int iser_create_fastreg_pool(struct iser_conn *ib_conn, unsigned cmds_max)
 	INIT_LIST_HEAD(&ib_conn->fastreg.pool);
 	ib_conn->fastreg.pool_size = 0;
 	for (i = 0; i < cmds_max; i++) {
-		desc = kmalloc(sizeof(*desc), GFP_KERNEL);
+		desc = kzalloc(sizeof(*desc), GFP_KERNEL);
 		if (!desc) {
 			iser_err("Failed to allocate a new fast_reg descriptor\n");
 			ret = -ENOMEM;
 			goto err;
 		}
 
-		ret = iser_create_fastreg_desc(device->ib_device,
-					       device->pd, desc);
+		ret = iser_create_fastreg_desc(device->ib_device, device->pd,
+					       ib_conn->pi_support, desc);
 		if (ret) {
 			iser_err("Failed to create fastreg descriptor err=%d\n",
 				 ret);
@@ -364,6 +416,12 @@ void iser_free_fastreg_pool(struct iser_conn *ib_conn)
 		list_del(&desc->list);
 		ib_free_fast_reg_page_list(desc->data_frpl);
 		ib_dereg_mr(desc->data_mr);
+		if (desc->pi_ctx) {
+			ib_free_fast_reg_page_list(desc->pi_ctx->prot_frpl);
+			ib_dereg_mr(desc->pi_ctx->prot_mr);
+			ib_destroy_mr(desc->pi_ctx->sig_mr);
+			kfree(desc->pi_ctx);
+		}
 		kfree(desc);
 		++i;
 	}
@@ -405,12 +463,17 @@ static int iser_create_ib_conn_res(struct iser_conn *ib_conn)
 	init_attr.qp_context	= (void *)ib_conn;
 	init_attr.send_cq	= device->tx_cq[min_index];
 	init_attr.recv_cq	= device->rx_cq[min_index];
-	init_attr.cap.max_send_wr  = ISER_QP_MAX_REQ_DTOS;
 	init_attr.cap.max_recv_wr  = ISER_QP_MAX_RECV_DTOS;
 	init_attr.cap.max_send_sge = 2;
 	init_attr.cap.max_recv_sge = 1;
 	init_attr.sq_sig_type	= IB_SIGNAL_REQ_WR;
 	init_attr.qp_type	= IB_QPT_RC;
+	if (ib_conn->pi_support) {
+		init_attr.cap.max_send_wr = ISER_QP_SIG_MAX_REQ_DTOS;
+		init_attr.create_flags |= IB_QP_CREATE_SIGNATURE_EN;
+	} else {
+		init_attr.cap.max_send_wr  = ISER_QP_MAX_REQ_DTOS;
+	}
 
 	ret = rdma_create_qp(ib_conn->cma_id, device->pd, &init_attr);
 	if (ret)
-- 
1.7.1

[PATCH v1 10/13] IB/iser: Support T10-PI operations

[Sagi Grimberg]

Add logic to initialize protection information entities.
Upon each iSCSI task, we keep the scsi_cmnd in order to
query the scsi protection operations and reference to
protection buffers.

Modify iser_fast_reg_mr to receive indication weather it
is registering the data or protection buffers.

In addition Introduce iser_reg_sig_mr which performs fast
registration work-request for a signature enabled memory region
(IB_WR_REG_SIG_MR). In this routine we set all the protection
relevants for the device to offload protection data-transfer
and verification.

Signed-off-by: Sagi Grimberg <sagig@mellanox.com>
Signed-off-by: Alex Tabachnik <alext@mellanox.com>
---
 drivers/infiniband/ulp/iser/iscsi_iser.c     |    2 +
 drivers/infiniband/ulp/iser/iscsi_iser.h     |    9 +
 drivers/infiniband/ulp/iser/iser_initiator.c |   63 ++++++-
 drivers/infiniband/ulp/iser/iser_memory.c    |  257 +++++++++++++++++++++++---
 4 files changed, 304 insertions(+), 27 deletions(-)

diff --git a/drivers/infiniband/ulp/iser/iscsi_iser.c b/drivers/infiniband/ulp/iser/iscsi_iser.c
index cfa952e..a64b878 100644
--- a/drivers/infiniband/ulp/iser/iscsi_iser.c
+++ b/drivers/infiniband/ulp/iser/iscsi_iser.c
@@ -184,6 +184,8 @@ iscsi_iser_task_init(struct iscsi_task *task)
 
 	iser_task->command_sent = 0;
 	iser_task_rdma_init(iser_task);
+	iser_task->sc = task->sc;
+
 	return 0;
 }
 
diff --git a/drivers/infiniband/ulp/iser/iscsi_iser.h b/drivers/infiniband/ulp/iser/iscsi_iser.h
index 99fc8b8..fce5409 100644
--- a/drivers/infiniband/ulp/iser/iscsi_iser.h
+++ b/drivers/infiniband/ulp/iser/iscsi_iser.h
@@ -46,6 +46,8 @@
 #include <linux/printk.h>
 #include <scsi/libiscsi.h>
 #include <scsi/scsi_transport_iscsi.h>
+#include <scsi/scsi_cmnd.h>
+#include <scsi/scsi_device.h>
 
 #include <linux/interrupt.h>
 #include <linux/wait.h>
@@ -289,6 +291,10 @@ struct iser_device {
 							    enum iser_data_dir cmd_dir);
 };
 
+#define ISER_CHECK_GUARD	0xc0
+#define ISER_CHECK_REFTAG	0x0f
+#define ISER_CHECK_APPTAG	0x30
+
 enum iser_reg_indicator {
 	ISER_DATA_KEY_VALID	= 1 << 0,
 	ISER_PROT_KEY_VALID	= 1 << 1,
@@ -361,11 +367,14 @@ struct iscsi_iser_task {
 	struct iser_tx_desc          desc;
 	struct iscsi_iser_conn	     *iser_conn;
 	enum iser_task_status 	     status;
+	struct scsi_cmnd	     *sc;
 	int                          command_sent;  /* set if command  sent  */
 	int                          dir[ISER_DIRS_NUM];      /* set if dir use*/
 	struct iser_regd_buf         rdma_regd[ISER_DIRS_NUM];/* regd rdma buf */
 	struct iser_data_buf         data[ISER_DIRS_NUM];     /* orig. data des*/
 	struct iser_data_buf         data_copy[ISER_DIRS_NUM];/* contig. copy  */
+	struct iser_data_buf         prot[ISER_DIRS_NUM];     /* prot desc     */
+	struct iser_data_buf         prot_copy[ISER_DIRS_NUM];/* prot copy     */
 };
 
 struct iser_page_vec {
diff --git a/drivers/infiniband/ulp/iser/iser_initiator.c b/drivers/infiniband/ulp/iser/iser_initiator.c
index 58e14c7..7fd95fe 100644
--- a/drivers/infiniband/ulp/iser/iser_initiator.c
+++ b/drivers/infiniband/ulp/iser/iser_initiator.c
@@ -62,6 +62,17 @@ static int iser_prepare_read_cmd(struct iscsi_task *task,
 	if (err)
 		return err;
 
+	if (scsi_prot_sg_count(iser_task->sc)) {
+		struct iser_data_buf *pbuf_in = &iser_task->prot[ISER_DIR_IN];
+
+		err = iser_dma_map_task_data(iser_task,
+					     pbuf_in,
+					     ISER_DIR_IN,
+					     DMA_FROM_DEVICE);
+		if (err)
+			return err;
+	}
+
 	if (edtl > iser_task->data[ISER_DIR_IN].data_len) {
 		iser_err("Total data length: %ld, less than EDTL: "
 			 "%d, in READ cmd BHS itt: %d, conn: 0x%p\n",
@@ -113,6 +124,17 @@ iser_prepare_write_cmd(struct iscsi_task *task,
 	if (err)
 		return err;
 
+	if (scsi_prot_sg_count(iser_task->sc)) {
+		struct iser_data_buf *pbuf_out = &iser_task->prot[ISER_DIR_OUT];
+
+		err = iser_dma_map_task_data(iser_task,
+					     pbuf_out,
+					     ISER_DIR_OUT,
+					     DMA_TO_DEVICE);
+		if (err)
+			return err;
+	}
+
 	if (edtl > iser_task->data[ISER_DIR_OUT].data_len) {
 		iser_err("Total data length: %ld, less than EDTL: %d, "
 			 "in WRITE cmd BHS itt: %d, conn: 0x%p\n",
@@ -368,7 +390,7 @@ int iser_send_command(struct iscsi_conn *conn,
 	struct iscsi_iser_task *iser_task = task->dd_data;
 	unsigned long edtl;
 	int err;
-	struct iser_data_buf *data_buf;
+	struct iser_data_buf *data_buf, *prot_buf;
 	struct iscsi_scsi_req *hdr = (struct iscsi_scsi_req *)task->hdr;
 	struct scsi_cmnd *sc  =  task->sc;
 	struct iser_tx_desc *tx_desc = &iser_task->desc;
@@ -379,18 +401,26 @@ int iser_send_command(struct iscsi_conn *conn,
 	tx_desc->type = ISCSI_TX_SCSI_COMMAND;
 	iser_create_send_desc(iser_conn->ib_conn, tx_desc);
 
-	if (hdr->flags & ISCSI_FLAG_CMD_READ)
+	if (hdr->flags & ISCSI_FLAG_CMD_READ) {
 		data_buf = &iser_task->data[ISER_DIR_IN];
-	else
+		prot_buf = &iser_task->prot[ISER_DIR_IN];
+	} else {
 		data_buf = &iser_task->data[ISER_DIR_OUT];
+		prot_buf = &iser_task->prot[ISER_DIR_OUT];
+	}
 
 	if (scsi_sg_count(sc)) { /* using a scatter list */
 		data_buf->buf  = scsi_sglist(sc);
 		data_buf->size = scsi_sg_count(sc);
 	}
-
 	data_buf->data_len = scsi_bufflen(sc);
 
+	if (scsi_prot_sg_count(sc)) {
+		prot_buf->buf  = scsi_prot_sglist(sc);
+		prot_buf->size = scsi_prot_sg_count(sc);
+		prot_buf->data_len = sc->prot_sdb->length;
+	}
+
 	if (hdr->flags & ISCSI_FLAG_CMD_READ) {
 		err = iser_prepare_read_cmd(task, edtl);
 		if (err)
@@ -635,6 +665,9 @@ void iser_task_rdma_init(struct iscsi_iser_task *iser_task)
 	iser_task->data[ISER_DIR_IN].data_len  = 0;
 	iser_task->data[ISER_DIR_OUT].data_len = 0;
 
+	iser_task->prot[ISER_DIR_IN].data_len  = 0;
+	iser_task->prot[ISER_DIR_OUT].data_len = 0;
+
 	memset(&iser_task->rdma_regd[ISER_DIR_IN], 0,
 	       sizeof(struct iser_regd_buf));
 	memset(&iser_task->rdma_regd[ISER_DIR_OUT], 0,
@@ -645,6 +678,8 @@ void iser_task_rdma_finalize(struct iscsi_iser_task *iser_task)
 {
 	struct iser_device *device = iser_task->iser_conn->ib_conn->device;
 	int is_rdma_data_aligned = 1;
+	int is_rdma_prot_aligned = 1;
+	int prot_count = scsi_prot_sg_count(iser_task->sc);
 
 	/* if we were reading, copy back to unaligned sglist,
 	 * anyway dma_unmap and free the copy
@@ -665,12 +700,30 @@ void iser_task_rdma_finalize(struct iscsi_iser_task *iser_task)
 						ISER_DIR_OUT);
 	}
 
+	if (iser_task->prot_copy[ISER_DIR_IN].copy_buf != NULL) {
+		is_rdma_prot_aligned = 0;
+		iser_finalize_rdma_unaligned_sg(iser_task,
+						&iser_task->prot[ISER_DIR_IN],
+						&iser_task->prot_copy[ISER_DIR_IN],
+						ISER_DIR_IN);
+	}
+
+	if (iser_task->prot_copy[ISER_DIR_OUT].copy_buf != NULL) {
+		is_rdma_prot_aligned = 0;
+		iser_finalize_rdma_unaligned_sg(iser_task,
+						&iser_task->prot[ISER_DIR_OUT],
+						&iser_task->prot_copy[ISER_DIR_OUT],
+						ISER_DIR_OUT);
+	}
+
 	if (iser_task->dir[ISER_DIR_IN]) {
 		device->iser_unreg_rdma_mem(iser_task, ISER_DIR_IN);
 		if (is_rdma_data_aligned)
 			iser_dma_unmap_task_data(iser_task,
 						 &iser_task->data[ISER_DIR_IN]);
-
+		if (prot_count && is_rdma_prot_aligned)
+			iser_dma_unmap_task_data(iser_task,
+						 &iser_task->prot[ISER_DIR_IN]);
 	}
 
 	if (iser_task->dir[ISER_DIR_OUT]) {
diff --git a/drivers/infiniband/ulp/iser/iser_memory.c b/drivers/infiniband/ulp/iser/iser_memory.c
index 2c3f4b1..0995565 100644
--- a/drivers/infiniband/ulp/iser/iser_memory.c
+++ b/drivers/infiniband/ulp/iser/iser_memory.c
@@ -440,15 +440,180 @@ int iser_reg_rdma_mem_fmr(struct iscsi_iser_task *iser_task,
 	return 0;
 }
 
+static inline enum ib_t10_dif_type
+scsi2ib_prot_type(unsigned char prot_type)
+{
+	switch (prot_type) {
+	case SCSI_PROT_DIF_TYPE0:
+		return IB_T10DIF_NONE;
+	case SCSI_PROT_DIF_TYPE1:
+		return IB_T10DIF_TYPE1;
+	case SCSI_PROT_DIF_TYPE2:
+		return IB_T10DIF_TYPE2;
+	case SCSI_PROT_DIF_TYPE3:
+		return IB_T10DIF_TYPE3;
+	default:
+		return IB_T10DIF_NONE;
+	}
+}
+
+
+static int
+iser_set_sig_attrs(struct scsi_cmnd *sc, struct ib_sig_attrs *sig_attrs)
+{
+	unsigned char scsi_ptype = scsi_get_prot_type(sc);
+
+	sig_attrs->mem.sig_type = IB_SIG_TYPE_T10_DIF;
+	sig_attrs->wire.sig_type = IB_SIG_TYPE_T10_DIF;
+	sig_attrs->mem.sig.dif.pi_interval = sc->device->sector_size;
+	sig_attrs->wire.sig.dif.pi_interval = sc->device->sector_size;
+
+	switch (scsi_get_prot_op(sc)) {
+	case SCSI_PROT_WRITE_INSERT:
+	case SCSI_PROT_READ_STRIP:
+		sig_attrs->mem.sig.dif.type = IB_T10DIF_NONE;
+		sig_attrs->wire.sig.dif.type = scsi2ib_prot_type(scsi_ptype);
+		sig_attrs->wire.sig.dif.bg_type = IB_T10DIF_CRC;
+		sig_attrs->wire.sig.dif.ref_tag = scsi_get_lba(sc) &
+						  0xffffffff;
+		break;
+	case SCSI_PROT_READ_INSERT:
+	case SCSI_PROT_WRITE_STRIP:
+		sig_attrs->mem.sig.dif.type = scsi2ib_prot_type(scsi_ptype);
+		sig_attrs->mem.sig.dif.bg_type = IB_T10DIF_CRC;
+		sig_attrs->mem.sig.dif.ref_tag = scsi_get_lba(sc) &
+						 0xffffffff;
+		sig_attrs->wire.sig.dif.type = IB_T10DIF_NONE;
+		break;
+	case SCSI_PROT_READ_PASS:
+	case SCSI_PROT_WRITE_PASS:
+		sig_attrs->mem.sig.dif.type = scsi2ib_prot_type(scsi_ptype);
+		sig_attrs->mem.sig.dif.bg_type = IB_T10DIF_CRC;
+		sig_attrs->mem.sig.dif.ref_tag = scsi_get_lba(sc) &
+						 0xffffffff;
+		sig_attrs->wire.sig.dif.type = scsi2ib_prot_type(scsi_ptype);
+		sig_attrs->wire.sig.dif.bg_type = IB_T10DIF_CRC;
+		sig_attrs->wire.sig.dif.ref_tag = scsi_get_lba(sc) &
+						  0xffffffff;
+		break;
+	default:
+		iser_err("Unsupported PI operation %d\n",
+			 scsi_get_prot_op(sc));
+		return -EINVAL;
+	}
+	return 0;
+}
+
+
+static int
+iser_set_prot_checks(struct scsi_cmnd *sc, u8 *mask)
+{
+	switch (scsi_get_prot_type(sc)) {
+	case SCSI_PROT_DIF_TYPE0:
+		*mask = 0x0;
+		break;
+	case SCSI_PROT_DIF_TYPE1:
+	case SCSI_PROT_DIF_TYPE2:
+		*mask = ISER_CHECK_GUARD | ISER_CHECK_REFTAG;
+		break;
+	case SCSI_PROT_DIF_TYPE3:
+		*mask = ISER_CHECK_GUARD;
+		break;
+	default:
+		iser_err("Unsupported protection type %d\n",
+			 scsi_get_prot_type(sc));
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
+static int
+iser_reg_sig_mr(struct iscsi_iser_task *iser_task,
+		struct fast_reg_descriptor *desc, struct ib_sge *data_sge,
+		struct ib_sge *prot_sge, struct ib_sge *sig_sge)
+{
+	struct iser_conn *iser_conn = iser_task->iser_conn->ib_conn;
+	struct iser_pi_context *pi_ctx = desc->pi_ctx;
+	struct ib_send_wr sig_wr, inv_wr;
+	struct ib_send_wr *bad_wr, *wr = NULL;
+	struct ib_sig_attrs sig_attrs;
+	int ret;
+	u32 key;
+
+	memset(&sig_attrs, 0, sizeof(sig_attrs));
+	ret = iser_set_sig_attrs(iser_task->sc, &sig_attrs);
+	if (ret)
+		goto err;
+
+	ret = iser_set_prot_checks(iser_task->sc, &sig_attrs.check_mask);
+	if (ret)
+		goto err;
+
+	if (!(desc->reg_indicators & ISER_SIG_KEY_VALID)) {
+		memset(&inv_wr, 0, sizeof(inv_wr));
+		inv_wr.opcode = IB_WR_LOCAL_INV;
+		inv_wr.wr_id = ISER_FASTREG_LI_WRID;
+		inv_wr.ex.invalidate_rkey = pi_ctx->sig_mr->rkey;
+		wr = &inv_wr;
+		/* Bump the key */
+		key = (u8)(pi_ctx->sig_mr->rkey & 0x000000FF);
+		ib_update_fast_reg_key(pi_ctx->sig_mr, ++key);
+	}
+
+	memset(&sig_wr, 0, sizeof(sig_wr));
+	sig_wr.opcode = IB_WR_REG_SIG_MR;
+	sig_wr.wr_id = ISER_FASTREG_LI_WRID;
+	sig_wr.sg_list = data_sge;
+	sig_wr.num_sge = 1;
+	sig_wr.wr.sig_handover.sig_attrs = &sig_attrs;
+	sig_wr.wr.sig_handover.sig_mr = pi_ctx->sig_mr;
+	if (scsi_prot_sg_count(iser_task->sc))
+		sig_wr.wr.sig_handover.prot = prot_sge;
+	sig_wr.wr.sig_handover.access_flags = IB_ACCESS_LOCAL_WRITE |
+					      IB_ACCESS_REMOTE_READ |
+					      IB_ACCESS_REMOTE_WRITE;
+
+	if (!wr)
+		wr = &sig_wr;
+	else
+		wr->next = &sig_wr;
+
+	ret = ib_post_send(iser_conn->qp, wr, &bad_wr);
+	if (ret) {
+		iser_err("reg_sig_mr failed, ret:%d\n", ret);
+		goto err;
+	}
+	desc->reg_indicators &= ~ISER_SIG_KEY_VALID;
+
+	sig_sge->lkey = pi_ctx->sig_mr->lkey;
+	sig_sge->addr = 0;
+	sig_sge->length = data_sge->length + prot_sge->length;
+	if (scsi_get_prot_op(iser_task->sc) == SCSI_PROT_WRITE_INSERT ||
+	    scsi_get_prot_op(iser_task->sc) == SCSI_PROT_READ_STRIP) {
+		sig_sge->length += (data_sge->length /
+				   iser_task->sc->device->sector_size) * 8;
+	}
+
+	iser_dbg("sig_sge: addr: 0x%llx  length: %u lkey: 0x%x\n",
+		 sig_sge->addr, sig_sge->length,
+		 sig_sge->lkey);
+err:
+	return ret;
+}
+
 static int iser_fast_reg_mr(struct iscsi_iser_task *iser_task,
 			    struct iser_regd_buf *regd_buf,
 			    struct iser_data_buf *mem,
+			    enum iser_reg_indicator ind,
 			    struct ib_sge *sge)
 {
 	struct fast_reg_descriptor *desc = regd_buf->reg.mem_h;
 	struct iser_conn *ib_conn = iser_task->iser_conn->ib_conn;
 	struct iser_device *device = ib_conn->device;
 	struct ib_device *ibdev = device->ib_device;
+	struct ib_mr *mr;
+	struct ib_fast_reg_page_list *frpl;
 	struct ib_send_wr fastreg_wr, inv_wr;
 	struct ib_send_wr *bad_wr, *wr = NULL;
 	u8 key;
@@ -467,35 +632,42 @@ static int iser_fast_reg_mr(struct iscsi_iser_task *iser_task,
 		return 0;
 	}
 
-	plen = iser_sg_to_page_vec(mem, device->ib_device,
-				   desc->data_frpl->page_list,
+	if (ind == ISER_DATA_KEY_VALID) {
+		mr = desc->data_mr;
+		frpl = desc->data_frpl;
+	} else {
+		mr = desc->pi_ctx->prot_mr;
+		frpl = desc->pi_ctx->prot_frpl;
+	}
+
+	plen = iser_sg_to_page_vec(mem, device->ib_device, frpl->page_list,
 				   &offset, &size);
 	if (plen * SIZE_4K < size) {
 		iser_err("fast reg page_list too short to hold this SG\n");
 		return -EINVAL;
 	}
 
-	if (!(desc->reg_indicators & ISER_DATA_KEY_VALID)) {
+	if (!(desc->reg_indicators & ind)) {
 		memset(&inv_wr, 0, sizeof(inv_wr));
 		inv_wr.wr_id = ISER_FASTREG_LI_WRID;
 		inv_wr.opcode = IB_WR_LOCAL_INV;
-		inv_wr.ex.invalidate_rkey = desc->data_mr->rkey;
+		inv_wr.ex.invalidate_rkey = mr->rkey;
 		wr = &inv_wr;
 		/* Bump the key */
-		key = (u8)(desc->data_mr->rkey & 0x000000FF);
-		ib_update_fast_reg_key(desc->data_mr, ++key);
+		key = (u8)(mr->rkey & 0x000000FF);
+		ib_update_fast_reg_key(mr, ++key);
 	}
 
 	/* Prepare FASTREG WR */
 	memset(&fastreg_wr, 0, sizeof(fastreg_wr));
 	fastreg_wr.wr_id = ISER_FASTREG_LI_WRID;
 	fastreg_wr.opcode = IB_WR_FAST_REG_MR;
-	fastreg_wr.wr.fast_reg.iova_start = desc->data_frpl->page_list[0] + offset;
-	fastreg_wr.wr.fast_reg.page_list = desc->data_frpl;
+	fastreg_wr.wr.fast_reg.iova_start = frpl->page_list[0] + offset;
+	fastreg_wr.wr.fast_reg.page_list = frpl;
 	fastreg_wr.wr.fast_reg.page_list_len = plen;
 	fastreg_wr.wr.fast_reg.page_shift = SHIFT_4K;
 	fastreg_wr.wr.fast_reg.length = size;
-	fastreg_wr.wr.fast_reg.rkey = desc->data_mr->rkey;
+	fastreg_wr.wr.fast_reg.rkey = mr->rkey;
 	fastreg_wr.wr.fast_reg.access_flags = (IB_ACCESS_LOCAL_WRITE  |
 					       IB_ACCESS_REMOTE_WRITE |
 					       IB_ACCESS_REMOTE_READ);
@@ -510,10 +682,10 @@ static int iser_fast_reg_mr(struct iscsi_iser_task *iser_task,
 		iser_err("fast registration failed, ret:%d\n", ret);
 		return ret;
 	}
-	desc->reg_indicators &= ~ISER_DATA_KEY_VALID;
+	desc->reg_indicators &= ~ind;
 
-	sge->lkey = desc->data_mr->lkey;
-	sge->addr = desc->data_frpl->page_list[0] + offset;
+	sge->lkey = mr->lkey;
+	sge->addr = frpl->page_list[0] + offset;
 	sge->length = size;
 
 	return ret;
@@ -550,7 +722,8 @@ int iser_reg_rdma_mem_fastreg(struct iscsi_iser_task *iser_task,
 		mem = &iser_task->data_copy[cmd_dir];
 	}
 
-	if (mem->dma_nents != 1) {
+	if (mem->dma_nents != 1 ||
+	    scsi_get_prot_op(iser_task->sc) != SCSI_PROT_NORMAL) {
 		spin_lock_irqsave(&ib_conn->lock, flags);
 		desc = list_first_entry(&ib_conn->fastreg.pool,
 					struct fast_reg_descriptor, list);
@@ -559,21 +732,61 @@ int iser_reg_rdma_mem_fastreg(struct iscsi_iser_task *iser_task,
 		regd_buf->reg.mem_h = desc;
 	}
 
-	err = iser_fast_reg_mr(iser_task, regd_buf, mem, &data_sge);
+	err = iser_fast_reg_mr(iser_task, regd_buf, mem,
+			       ISER_DATA_KEY_VALID, &data_sge);
 	if (err)
 		goto err_reg;
 
-	if (desc) {
-		regd_buf->reg.rkey = desc->data_mr->rkey;
+	if (scsi_get_prot_op(iser_task->sc) != SCSI_PROT_NORMAL) {
+		struct ib_sge prot_sge, sig_sge;
+
+		memset(&prot_sge, 0, sizeof(prot_sge));
+		if (scsi_prot_sg_count(iser_task->sc)) {
+			mem = &iser_task->prot[cmd_dir];
+			aligned_len = iser_data_buf_aligned_len(mem, ibdev);
+			if (aligned_len != mem->dma_nents) {
+				err = fall_to_bounce_buf(iser_task, ibdev, mem,
+							 &iser_task->prot_copy[cmd_dir],
+							 cmd_dir, aligned_len);
+				if (err) {
+					iser_err("failed to allocate bounce buffer\n");
+					return err;
+				}
+				mem = &iser_task->prot_copy[cmd_dir];
+			}
+
+			err = iser_fast_reg_mr(iser_task, regd_buf, mem,
+					       ISER_PROT_KEY_VALID, &prot_sge);
+			if (err)
+				goto err_reg;
+		}
+
+		err = iser_reg_sig_mr(iser_task, desc, &data_sge,
+				      &prot_sge, &sig_sge);
+		if (err) {
+			iser_err("Failed to register signature mr\n");
+			return err;
+		}
+		desc->reg_indicators |= ISER_FASTREG_PROTECTED;
+
+		regd_buf->reg.lkey = sig_sge.lkey;
+		regd_buf->reg.rkey = desc->pi_ctx->sig_mr->rkey;
+		regd_buf->reg.va = sig_sge.addr;
+		regd_buf->reg.len = sig_sge.length;
 		regd_buf->reg.is_mr = 1;
 	} else {
-		regd_buf->reg.rkey = device->mr->rkey;
-		regd_buf->reg.is_mr = 0;
-	}
+		if (desc) {
+			regd_buf->reg.rkey = desc->data_mr->rkey;
+			regd_buf->reg.is_mr = 1;
+		} else {
+			regd_buf->reg.rkey = device->mr->rkey;
+			regd_buf->reg.is_mr = 0;
+		}
 
-	regd_buf->reg.lkey = data_sge.lkey;
-	regd_buf->reg.va = data_sge.addr;
-	regd_buf->reg.len = data_sge.length;
+		regd_buf->reg.lkey = data_sge.lkey;
+		regd_buf->reg.va = data_sge.addr;
+		regd_buf->reg.len = data_sge.length;
+	}
 
 	return 0;
 err_reg:
-- 
1.7.1

[PATCH v1 11/13] SCSI/libiscsi: Add check_protection callback for transports

[Sagi Grimberg]

iSCSI needs to be at least aware that a task involves protection
information. In case it does, after the transaction completed
libiscsi will ask the transport to check the protection status
of the transaction.

Unlike transport errors, DIF errors should not prevent successful
completion of the transaction from the transport point of view,
but should be escelated to scsi mid-layer when constructing the
scsi result and sense data.

check_protection routine will return the ascq corresponding to the
DIF error that occured (or 0 if no error happened).

return ascq:
- 0x1: GUARD_CHECK_FAILED
- 0x2: APPTAG_CHECK_FAILED
- 0x3: REFTAG_CHECK_FAILED

Signed-off-by: Sagi Grimberg <sagig-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
Signed-off-by: Alex Tabachnik <alext-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
---
 drivers/scsi/libiscsi.c             |   32 ++++++++++++++++++++++++++++++++
 include/scsi/libiscsi.h             |    4 ++++
 include/scsi/scsi_transport_iscsi.h |    1 +
 3 files changed, 37 insertions(+), 0 deletions(-)

diff --git a/drivers/scsi/libiscsi.c b/drivers/scsi/libiscsi.c
index 4046241..a58a6bb 100644
--- a/drivers/scsi/libiscsi.c
+++ b/drivers/scsi/libiscsi.c
@@ -395,6 +395,10 @@ static int iscsi_prep_scsi_cmd_pdu(struct iscsi_task *task)
 		if (rc)
 			return rc;
 	}
+
+	if (scsi_get_prot_op(sc) != SCSI_PROT_NORMAL)
+		task->protected = true;
+
 	if (sc->sc_data_direction == DMA_TO_DEVICE) {
 		unsigned out_len = scsi_out(sc)->length;
 		struct iscsi_r2t_info *r2t = &task->unsol_r2t;
@@ -823,6 +827,33 @@ static void iscsi_scsi_cmd_rsp(struct iscsi_conn *conn, struct iscsi_hdr *hdr,
 
 	sc->result = (DID_OK << 16) | rhdr->cmd_status;
 
+	if (task->protected) {
+		sector_t sector;
+		u8 ascq;
+
+		/**
+		 * Transports that didn't implement check_protection
+		 * callback but still published T10-PI support to scsi-mid
+		 * deserve this BUG_ON.
+		 **/
+		 BUG_ON(!session->tt->check_protection);
+
+		ascq = session->tt->check_protection(task, &sector);
+		if (ascq) {
+			sc->result = DRIVER_SENSE << 24 | DID_ABORT << 16 |
+				     SAM_STAT_CHECK_CONDITION;
+			scsi_build_sense_buffer(1, sc->sense_buffer,
+						ILLEGAL_REQUEST, 0x10, ascq);
+			sc->sense_buffer[7] = 0xc; /* Additional sense length */
+			sc->sense_buffer[8] = 0;   /* Information desc type */
+			sc->sense_buffer[9] = 0xa; /* Additional desc length */
+			sc->sense_buffer[10] = 0x80; /* Validity bit */
+
+			put_unaligned_be64(sector, &sc->sense_buffer[12]);
+			goto out;
+		}
+	}
+
 	if (rhdr->response != ISCSI_STATUS_CMD_COMPLETED) {
 		sc->result = DID_ERROR << 16;
 		goto out;
@@ -1567,6 +1598,7 @@ static inline struct iscsi_task *iscsi_alloc_task(struct iscsi_conn *conn,
 	task->have_checked_conn = false;
 	task->last_timeout = jiffies;
 	task->last_xfer = jiffies;
+	task->protected = false;
 	INIT_LIST_HEAD(&task->running);
 	return task;
 }
diff --git a/include/scsi/libiscsi.h b/include/scsi/libiscsi.h
index 309f513..1457c26 100644
--- a/include/scsi/libiscsi.h
+++ b/include/scsi/libiscsi.h
@@ -133,6 +133,10 @@ struct iscsi_task {
 	unsigned long		last_xfer;
 	unsigned long		last_timeout;
 	bool			have_checked_conn;
+
+	/* T10 protection information */
+	bool			protected;
+
 	/* state set/tested under session->lock */
 	int			state;
 	atomic_t		refcount;
diff --git a/include/scsi/scsi_transport_iscsi.h b/include/scsi/scsi_transport_iscsi.h
index 88640a4..2555ee5 100644
--- a/include/scsi/scsi_transport_iscsi.h
+++ b/include/scsi/scsi_transport_iscsi.h
@@ -167,6 +167,7 @@ struct iscsi_transport {
 				 struct iscsi_bus_flash_conn *fnode_conn);
 	int (*logout_flashnode_sid) (struct iscsi_cls_session *cls_sess);
 	int (*get_host_stats) (struct Scsi_Host *shost, char *buf, int len);
+	u8 (*check_protection)(struct iscsi_task *task, sector_t *sector);
 };
 
 /*
-- 
1.7.1

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

[PATCH v1 12/13] IB/iser: Implement check_protection

[Sagi Grimberg]

Once the iSCSI transaction is completed we must
imeplement check_protection in order to notify
on DIF errors that may have occured.

The routine boils down to calling ib_check_mr_status
to get the signature status of the transaction.

Signed-off-by: Sagi Grimberg <sagig@mellanox.com>
Signed-off-by: Alex Tabachnik <alext@mellanox.com>
---
 drivers/infiniband/ulp/iser/iscsi_iser.c |   13 ++++++++
 drivers/infiniband/ulp/iser/iscsi_iser.h |    2 +
 drivers/infiniband/ulp/iser/iser_verbs.c |   45 ++++++++++++++++++++++++++++++
 3 files changed, 60 insertions(+), 0 deletions(-)

diff --git a/drivers/infiniband/ulp/iser/iscsi_iser.c b/drivers/infiniband/ulp/iser/iscsi_iser.c
index a64b878..f13d7e9 100644
--- a/drivers/infiniband/ulp/iser/iscsi_iser.c
+++ b/drivers/infiniband/ulp/iser/iscsi_iser.c
@@ -306,6 +306,18 @@ static void iscsi_iser_cleanup_task(struct iscsi_task *task)
 	}
 }
 
+static u8 iscsi_iser_check_protection(struct iscsi_task *task, sector_t *sector)
+{
+	struct iscsi_iser_task *iser_task = task->dd_data;
+
+	if (iser_task->dir[ISER_DIR_IN])
+		return iser_check_task_pi_status(iser_task, ISER_DIR_IN,
+						 sector);
+	else
+		return iser_check_task_pi_status(iser_task, ISER_DIR_OUT,
+						 sector);
+}
+
 static struct iscsi_cls_conn *
 iscsi_iser_conn_create(struct iscsi_cls_session *cls_session, uint32_t conn_idx)
 {
@@ -742,6 +754,7 @@ static struct iscsi_transport iscsi_iser_transport = {
 	.xmit_task		= iscsi_iser_task_xmit,
 	.cleanup_task		= iscsi_iser_cleanup_task,
 	.alloc_pdu		= iscsi_iser_pdu_alloc,
+	.check_protection	= iscsi_iser_check_protection,
 	/* recovery */
 	.session_recovery_timedout = iscsi_session_recovery_timedout,
 
diff --git a/drivers/infiniband/ulp/iser/iscsi_iser.h b/drivers/infiniband/ulp/iser/iscsi_iser.h
index fce5409..95f291f 100644
--- a/drivers/infiniband/ulp/iser/iscsi_iser.h
+++ b/drivers/infiniband/ulp/iser/iscsi_iser.h
@@ -483,4 +483,6 @@ int iser_create_fmr_pool(struct iser_conn *ib_conn, unsigned cmds_max);
 void iser_free_fmr_pool(struct iser_conn *ib_conn);
 int iser_create_fastreg_pool(struct iser_conn *ib_conn, unsigned cmds_max);
 void iser_free_fastreg_pool(struct iser_conn *ib_conn);
+u8 iser_check_task_pi_status(struct iscsi_iser_task *iser_task,
+			     enum iser_data_dir cmd_dir, sector_t *sector);
 #endif
diff --git a/drivers/infiniband/ulp/iser/iser_verbs.c b/drivers/infiniband/ulp/iser/iser_verbs.c
index 0404c71..617d490 100644
--- a/drivers/infiniband/ulp/iser/iser_verbs.c
+++ b/drivers/infiniband/ulp/iser/iser_verbs.c
@@ -1153,3 +1153,48 @@ static void iser_cq_callback(struct ib_cq *cq, void *cq_context)
 
 	tasklet_schedule(&device->cq_tasklet[cq_index]);
 }
+
+u8 iser_check_task_pi_status(struct iscsi_iser_task *iser_task,
+			     enum iser_data_dir cmd_dir, sector_t *sector)
+{
+	struct iser_mem_reg *reg = &iser_task->rdma_regd[cmd_dir].reg;
+	struct fast_reg_descriptor *desc = reg->mem_h;
+	unsigned long sector_size = iser_task->sc->device->sector_size;
+	struct ib_mr_status mr_status;
+	int ret;
+
+	if (desc && desc->reg_indicators & ISER_FASTREG_PROTECTED) {
+		desc->reg_indicators &= ~ISER_FASTREG_PROTECTED;
+		ret = ib_check_mr_status(desc->pi_ctx->sig_mr,
+					 IB_MR_CHECK_SIG_STATUS, &mr_status);
+		if (ret) {
+			pr_err("ib_check_mr_status failed, ret %d\n", ret);
+			goto err;
+		}
+
+		if (mr_status.fail_status & IB_MR_CHECK_SIG_STATUS) {
+			*sector = mr_status.sig_err.sig_err_offset;
+
+			do_div(*sector, sector_size + 8);
+			pr_err("PI error found type %d at sector %lx "
+			       "expected %x vs actual %x\n",
+			       mr_status.sig_err.err_type, *sector,
+			       mr_status.sig_err.expected,
+			       mr_status.sig_err.actual);
+
+			switch (mr_status.sig_err.err_type) {
+			case IB_SIG_BAD_GUARD:
+				return 0x1;
+			case IB_SIG_BAD_REFTAG:
+				return 0x3;
+			case IB_SIG_BAD_APPTAG:
+				return 0x2;
+			}
+		}
+	}
+
+	return 0;
+err:
+	/* Not alot we can do here, return ambiguous guard error */
+	return 0x1;
+}
-- 
1.7.1

[PATCH v1 13/13] IB/iser: Publish T10-PI support to SCSI midlayer

[Sagi Grimberg]

After allocating a scsi_host we set protection types
and guard type supported.

Signed-off-by: Sagi Grimberg <sagig@mellanox.com>
Signed-off-by: Alex Tabachnik <alext@mellanox.com>
---
 drivers/infiniband/ulp/iser/iscsi_iser.c |   23 ++++++++++++++++++++++-
 1 files changed, 22 insertions(+), 1 deletions(-)

diff --git a/drivers/infiniband/ulp/iser/iscsi_iser.c b/drivers/infiniband/ulp/iser/iscsi_iser.c
index f13d7e9..a0ec2d0 100644
--- a/drivers/infiniband/ulp/iser/iscsi_iser.c
+++ b/drivers/infiniband/ulp/iser/iscsi_iser.c
@@ -435,6 +435,17 @@ static void iscsi_iser_session_destroy(struct iscsi_cls_session *cls_session)
 	iscsi_host_free(shost);
 }
 
+static inline unsigned int
+iser_dif_prot_caps(int prot_caps)
+{
+	return ((prot_caps & IB_PROT_T10DIF_TYPE_1) ? SHOST_DIF_TYPE1_PROTECTION |
+						      SHOST_DIX_TYPE1_PROTECTION : 0) |
+	       ((prot_caps & IB_PROT_T10DIF_TYPE_2) ? SHOST_DIF_TYPE2_PROTECTION |
+						      SHOST_DIX_TYPE2_PROTECTION : 0) |
+	       ((prot_caps & IB_PROT_T10DIF_TYPE_3) ? SHOST_DIF_TYPE3_PROTECTION |
+						      SHOST_DIX_TYPE3_PROTECTION : 0);
+}
+
 static struct iscsi_cls_session *
 iscsi_iser_session_create(struct iscsi_endpoint *ep,
 			  uint16_t cmds_max, uint16_t qdepth,
@@ -459,8 +470,18 @@ iscsi_iser_session_create(struct iscsi_endpoint *ep,
 	 * older userspace tools (before 2.0-870) did not pass us
 	 * the leading conn's ep so this will be NULL;
 	 */
-	if (ep)
+	if (ep) {
 		ib_conn = ep->dd_data;
+		if (ib_conn->pi_support) {
+			u32 sig_caps = ib_conn->device->dev_attr.sig_prot_cap;
+
+			scsi_host_set_prot(shost, iser_dif_prot_caps(sig_caps));
+			if (iser_pi_guard)
+				scsi_host_set_guard(shost, SHOST_DIX_GUARD_IP);
+			else
+				scsi_host_set_guard(shost, SHOST_DIX_GUARD_CRC);
+		}
+	}
 
 	if (iscsi_host_add(shost,
 			   ep ? ib_conn->device->ib_device->dma_device : NULL))
-- 
1.7.1

Re: [PATCH v1 11/13] SCSI/libiscsi: Add check_protection callback for transports

[Mike Christie]

On 02/27/2014 05:13 AM, Sagi Grimberg wrote:
> diff --git a/drivers/scsi/libiscsi.c b/drivers/scsi/libiscsi.c
> index 4046241..a58a6bb 100644
> --- a/drivers/scsi/libiscsi.c
> +++ b/drivers/scsi/libiscsi.c
> @@ -395,6 +395,10 @@ static int iscsi_prep_scsi_cmd_pdu(struct iscsi_task *task)
>  		if (rc)
>  			return rc;
>  	}
> +
> +	if (scsi_get_prot_op(sc) != SCSI_PROT_NORMAL)
> +		task->protected = true;
> +
>  	if (sc->sc_data_direction == DMA_TO_DEVICE) {
>  		unsigned out_len = scsi_out(sc)->length;
>  		struct iscsi_r2t_info *r2t = &task->unsol_r2t;
> @@ -823,6 +827,33 @@ static void iscsi_scsi_cmd_rsp(struct iscsi_conn *conn, struct iscsi_hdr *hdr,
>  
>  	sc->result = (DID_OK << 16) | rhdr->cmd_status;
>  
> +	if (task->protected) {
> +		sector_t sector;
> +		u8 ascq;
> +
> +		/**
> +		 * Transports that didn't implement check_protection
> +		 * callback but still published T10-PI support to scsi-mid
> +		 * deserve this BUG_ON.
> +		 **/
> +		 BUG_ON(!session->tt->check_protection);

Extra space before BUG_ON.

> +
> +		ascq = session->tt->check_protection(task, &sector);
> +		if (ascq) {
> +			sc->result = DRIVER_SENSE << 24 | DID_ABORT << 16 |
> +				     SAM_STAT_CHECK_CONDITION;

I am not sure what the reason for the DID_ABORT is here. I do not think
we want that, because we just want scsi-ml to evaluate the sense error
part of the failure. It works ok today, but the DID_ABORT error can
possibly be evaluated before the sense so you might miss passing that
info to upper layers.


> +			scsi_build_sense_buffer(1, sc->sense_buffer,
> +						ILLEGAL_REQUEST, 0x10, ascq);
> +			sc->sense_buffer[7] = 0xc; /* Additional sense length */
> +			sc->sense_buffer[8] = 0;   /* Information desc type */
> +			sc->sense_buffer[9] = 0xa; /* Additional desc length */
> +			sc->sense_buffer[10] = 0x80; /* Validity bit */
> +
> +			put_unaligned_be64(sector, &sc->sense_buffer[12]);
> +			goto out;
> +		}
> +	}
> +
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v1 10/13] IB/iser: Support T10-PI operations

[Mike Christie]

On 02/27/2014 05:13 AM, Sagi Grimberg wrote:
> diff --git a/drivers/infiniband/ulp/iser/iser_initiator.c b/drivers/infiniband/ulp/iser/iser_initiator.c
> index 58e14c7..7fd95fe 100644
> --- a/drivers/infiniband/ulp/iser/iser_initiator.c
> +++ b/drivers/infiniband/ulp/iser/iser_initiator.c
> @@ -62,6 +62,17 @@ static int iser_prepare_read_cmd(struct iscsi_task *task,
>  	if (err)
>  		return err;
>  
> +	if (scsi_prot_sg_count(iser_task->sc)) {
> +		struct iser_data_buf *pbuf_in = &iser_task->prot[ISER_DIR_IN];
> +
> +		err = iser_dma_map_task_data(iser_task,
> +					     pbuf_in,
> +					     ISER_DIR_IN,
> +					     DMA_FROM_DEVICE);
> +		if (err)
> +			return err;
> +	}
> +
>  	if (edtl > iser_task->data[ISER_DIR_IN].data_len) {
>  		iser_err("Total data length: %ld, less than EDTL: "
>  			 "%d, in READ cmd BHS itt: %d, conn: 0x%p\n",
> @@ -113,6 +124,17 @@ iser_prepare_write_cmd(struct iscsi_task *task,
>  	if (err)
>  		return err;
>  
> +	if (scsi_prot_sg_count(iser_task->sc)) {
> +		struct iser_data_buf *pbuf_out = &iser_task->prot[ISER_DIR_OUT];
> +
> +		err = iser_dma_map_task_data(iser_task,
> +					     pbuf_out,
> +					     ISER_DIR_OUT,
> +					     DMA_TO_DEVICE);
> +		if (err)
> +			return err;
> +	}
> +


The xmit_task callout does handle failures like EINVAL. If the above map
calls fail then you would get infinite retries. You would currently want
to do the mapping in the init_task callout instead.

If it makes it easier on the driver implementation then it is ok to
modify the xmit_task callers so that they handle multiple error codes
for drivers like iser that have the xmit_task callout called from
iscsi_queuecommand.

Re: [PATCH v1 11/13] SCSI/libiscsi: Add check_protection callback for transports

[Sagi Grimberg]

On 3/3/2014 6:41 AM, Mike Christie wrote:
> On 02/27/2014 05:13 AM, Sagi Grimberg wrote:
>> diff --git a/drivers/scsi/libiscsi.c b/drivers/scsi/libiscsi.c
>> index 4046241..a58a6bb 100644
>> --- a/drivers/scsi/libiscsi.c
>> +++ b/drivers/scsi/libiscsi.c
>> @@ -395,6 +395,10 @@ static int iscsi_prep_scsi_cmd_pdu(struct iscsi_task *task)
>>   		if (rc)
>>   			return rc;
>>   	}
>> +
>> +	if (scsi_get_prot_op(sc) != SCSI_PROT_NORMAL)
>> +		task->protected = true;
>> +
>>   	if (sc->sc_data_direction == DMA_TO_DEVICE) {
>>   		unsigned out_len = scsi_out(sc)->length;
>>   		struct iscsi_r2t_info *r2t = &task->unsol_r2t;
>> @@ -823,6 +827,33 @@ static void iscsi_scsi_cmd_rsp(struct iscsi_conn *conn, struct iscsi_hdr *hdr,
>>   
>>   	sc->result = (DID_OK << 16) | rhdr->cmd_status;
>>   
>> +	if (task->protected) {
>> +		sector_t sector;
>> +		u8 ascq;
>> +
>> +		/**
>> +		 * Transports that didn't implement check_protection
>> +		 * callback but still published T10-PI support to scsi-mid
>> +		 * deserve this BUG_ON.
>> +		 **/
>> +		 BUG_ON(!session->tt->check_protection);
> Extra space before BUG_ON.

I'll add.

>> +
>> +		ascq = session->tt->check_protection(task, &sector);
>> +		if (ascq) {
>> +			sc->result = DRIVER_SENSE << 24 | DID_ABORT << 16 |
>> +				     SAM_STAT_CHECK_CONDITION;
> I am not sure what the reason for the DID_ABORT is here. I do not think
> we want that, because we just want scsi-ml to evaluate the sense error
> part of the failure. It works ok today, but the DID_ABORT error can
> possibly be evaluated before the sense so you might miss passing that
> info to upper layers.

That makes sense to me. I can remove DID_ABORT.

>
>> +			scsi_build_sense_buffer(1, sc->sense_buffer,
>> +						ILLEGAL_REQUEST, 0x10, ascq);
>> +			sc->sense_buffer[7] = 0xc; /* Additional sense length */
>> +			sc->sense_buffer[8] = 0;   /* Information desc type */
>> +			sc->sense_buffer[9] = 0xa; /* Additional desc length */
>> +			sc->sense_buffer[10] = 0x80; /* Validity bit */
>> +
>> +			put_unaligned_be64(sector, &sc->sense_buffer[12]);
>> +			goto out;
>> +		}
>> +	}
>> +
> --
> To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v1 10/13] IB/iser: Support T10-PI operations

[Sagi Grimberg]

On 3/3/2014 6:44 AM, Mike Christie wrote:
> On 02/27/2014 05:13 AM, Sagi Grimberg wrote:
>> diff --git a/drivers/infiniband/ulp/iser/iser_initiator.c b/drivers/infiniband/ulp/iser/iser_initiator.c
>> index 58e14c7..7fd95fe 100644
>> --- a/drivers/infiniband/ulp/iser/iser_initiator.c
>> +++ b/drivers/infiniband/ulp/iser/iser_initiator.c
>> @@ -62,6 +62,17 @@ static int iser_prepare_read_cmd(struct iscsi_task *task,
>>   	if (err)
>>   		return err;
>>   
>> +	if (scsi_prot_sg_count(iser_task->sc)) {
>> +		struct iser_data_buf *pbuf_in = &iser_task->prot[ISER_DIR_IN];
>> +
>> +		err = iser_dma_map_task_data(iser_task,
>> +					     pbuf_in,
>> +					     ISER_DIR_IN,
>> +					     DMA_FROM_DEVICE);
>> +		if (err)
>> +			return err;
>> +	}
>> +
>>   	if (edtl > iser_task->data[ISER_DIR_IN].data_len) {
>>   		iser_err("Total data length: %ld, less than EDTL: "
>>   			 "%d, in READ cmd BHS itt: %d, conn: 0x%p\n",
>> @@ -113,6 +124,17 @@ iser_prepare_write_cmd(struct iscsi_task *task,
>>   	if (err)
>>   		return err;
>>   
>> +	if (scsi_prot_sg_count(iser_task->sc)) {
>> +		struct iser_data_buf *pbuf_out = &iser_task->prot[ISER_DIR_OUT];
>> +
>> +		err = iser_dma_map_task_data(iser_task,
>> +					     pbuf_out,
>> +					     ISER_DIR_OUT,
>> +					     DMA_TO_DEVICE);
>> +		if (err)
>> +			return err;
>> +	}
>> +
>
> The xmit_task callout does handle failures like EINVAL. If the above map
> calls fail then you would get infinite retries. You would currently want
> to do the mapping in the init_task callout instead.

Same applies to the data dma_mapping that already exist in xmit_task today.
I can move it to init_task, but to me, dma_mapping is not really an init 
operation
but part of xmit stage.

> If it makes it easier on the driver implementation then it is ok to
> modify the xmit_task callers so that they handle multiple error codes
> for drivers like iser that have the xmit_task callout called from
> iscsi_queuecommand.

This sounds somewhat better to me.

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

Re: [PATCH v1 10/13] IB/iser: Support T10-PI operations

[Or Gerlitz]

On 03/03/2014 06:44, Mike Christie wrote:
> The xmit_task callout does handle failures like EINVAL. If the above map
> calls fail then you would get infinite retries. You would currently want
> to do the mapping in the init_task callout instead.
>
> If it makes it easier on the driver implementation then it is ok to
> modify the xmit_task callers so that they handle multiple error codes
> for drivers like iser that have the xmit_task callout called from
> iscsi_queuecommand.

Mike,

After looking on the code with Sagi,  it seems to us that the correct 
way to go here, would be to enhance in iscsi_queuecommand the processing 
of the result returned by session->tt->xmit_task(task) to behave in a 
similar manner to how the return value of iscsi_prep_scsi_cmd_pdu() is 
treated. E.g for errors such as ENOMEM and EGAIN take the "reject" flow 
which would cause the SCSI midlayer to retry the command and for other 
return values go to the "fault" flow which will cause the ML to abort 
the command.

Or.


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

Re: [PATCH v1 10/13] IB/iser: Support T10-PI operations

[Sagi Grimberg]

On 3/4/2014 11:38 AM, Or Gerlitz wrote:
> On 03/03/2014 06:44, Mike Christie wrote:
>> The xmit_task callout does handle failures like EINVAL. If the above map
>> calls fail then you would get infinite retries. You would currently want
>> to do the mapping in the init_task callout instead.
>>
>> If it makes it easier on the driver implementation then it is ok to
>> modify the xmit_task callers so that they handle multiple error codes
>> for drivers like iser that have the xmit_task callout called from
>> iscsi_queuecommand.
>
> Mike,
>
> After looking on the code with Sagi,  it seems to us that the correct 
> way to go here, would be to enhance in iscsi_queuecommand the 
> processing of the result returned by session->tt->xmit_task(task) to 
> behave in a similar manner to how the return value of 
> iscsi_prep_scsi_cmd_pdu() is treated. E.g for errors such as ENOMEM 
> and EGAIN take the "reject" flow which would cause the SCSI midlayer 
> to retry the command and for other return values go to the "fault" 
> flow which will cause the ML to abort the command.
>
> Or.
>

Yes, we were thinking about the following:
--- a/drivers/scsi/libiscsi.c
+++ b/drivers/scsi/libiscsi.c
@@ -1707,10 +1707,17 @@ int iscsi_queuecommand(struct Scsi_Host *host, 
struct scsi_cmnd *sc)
                                 goto prepd_fault;
                         }
                 }
-               if (session->tt->xmit_task(task)) {
-                       session->cmdsn--;
-                       reason = FAILURE_SESSION_NOT_READY;
-                       goto prepd_reject;
+
+               reason = session->tt->xmit_task(task);
+               if (reason) {
+                       if (reason == -ENOMEM || reason == -EAGAIN) {
+                               session->cmdsn--;
+                               reason = FAILURE_SESSION_NOT_READY;
+                               goto prepd_reject;
+                       } else {
+                               sc->result = DID_ABORT << 16;
+                               goto prepd_fault;
+                       }
                 }
         } else {
                 list_add_tail(&task->running, &conn->cmdqueue);

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

Re: [PATCH v1 10/13] IB/iser: Support T10-PI operations

[Or Gerlitz]

On 04/03/2014 11:59, Sagi Grimberg wrote:
> On 3/4/2014 11:38 AM, Or Gerlitz wrote:
>> On 03/03/2014 06:44, Mike Christie wrote:
>>> The xmit_task callout does handle failures like EINVAL. If the above 
>>> map
>>> calls fail then you would get infinite retries. You would currently 
>>> want
>>> to do the mapping in the init_task callout instead.
>>>
>>> If it makes it easier on the driver implementation then it is ok to
>>> modify the xmit_task callers so that they handle multiple error codes
>>> for drivers like iser that have the xmit_task callout called from
>>> iscsi_queuecommand.
>>
>> Mike,
>>
>> After looking on the code with Sagi,  it seems to us that the correct 
>> way to go here, would be to enhance in iscsi_queuecommand the 
>> processing of the result returned by session->tt->xmit_task(task) to 
>> behave in a similar manner to how the return value of 
>> iscsi_prep_scsi_cmd_pdu() is treated. E.g for errors such as ENOMEM 
>> and EGAIN take the "reject" flow which would cause the SCSI midlayer 
>> to retry the command and for other return values go to the "fault" 
>> flow which will cause the ML to abort the command.
>>
>> Or.
>>
>
> Yes, we were thinking about the following:
> --- a/drivers/scsi/libiscsi.c
> +++ b/drivers/scsi/libiscsi.c
> @@ -1707,10 +1707,17 @@ int iscsi_queuecommand(struct Scsi_Host *host, 
> struct scsi_cmnd *sc)
>                                 goto prepd_fault;
>                         }
>                 }
> -               if (session->tt->xmit_task(task)) {
> -                       session->cmdsn--;
> -                       reason = FAILURE_SESSION_NOT_READY;
> -                       goto prepd_reject;
> +
> +               reason = session->tt->xmit_task(task);
> +               if (reason) {
> +                       if (reason == -ENOMEM || reason == -EAGAIN) {
> +                               session->cmdsn--;

I am pretty sure this has to be done anyway, no matter why the xmit_task 
callback failed

> + reason = FAILURE_SESSION_NOT_READY;
> +                               goto prepd_reject;
> +                       } else {
> +                               sc->result = DID_ABORT << 16;
> +                               goto prepd_fault;
> +                       }
>                 }
>         } else {
>                 list_add_tail(&task->running, &conn->cmdqueue);
>
> -- 
> To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
> the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

Re: [PATCH v1 10/13] IB/iser: Support T10-PI operations

[Sagi Grimberg]

On 3/4/2014 1:25 PM, Or Gerlitz wrote:
> On 04/03/2014 11:59, Sagi Grimberg wrote:
>> On 3/4/2014 11:38 AM, Or Gerlitz wrote:
>>> On 03/03/2014 06:44, Mike Christie wrote:
>>>> The xmit_task callout does handle failures like EINVAL. If the 
>>>> above map
>>>> calls fail then you would get infinite retries. You would currently 
>>>> want
>>>> to do the mapping in the init_task callout instead.
>>>>
>>>> If it makes it easier on the driver implementation then it is ok to
>>>> modify the xmit_task callers so that they handle multiple error codes
>>>> for drivers like iser that have the xmit_task callout called from
>>>> iscsi_queuecommand.
>>>
>>> Mike,
>>>
>>> After looking on the code with Sagi,  it seems to us that the 
>>> correct way to go here, would be to enhance in iscsi_queuecommand 
>>> the processing of the result returned by 
>>> session->tt->xmit_task(task) to behave in a similar manner to how 
>>> the return value of iscsi_prep_scsi_cmd_pdu() is treated. E.g for 
>>> errors such as ENOMEM and EGAIN take the "reject" flow which would 
>>> cause the SCSI midlayer to retry the command and for other return 
>>> values go to the "fault" flow which will cause the ML to abort the 
>>> command.
>>>
>>> Or.
>>>
>>
>> Yes, we were thinking about the following:
>> --- a/drivers/scsi/libiscsi.c
>> +++ b/drivers/scsi/libiscsi.c
>> @@ -1707,10 +1707,17 @@ int iscsi_queuecommand(struct Scsi_Host 
>> *host, struct scsi_cmnd *sc)
>>                                 goto prepd_fault;
>>                         }
>>                 }
>> -               if (session->tt->xmit_task(task)) {
>> -                       session->cmdsn--;
>> -                       reason = FAILURE_SESSION_NOT_READY;
>> -                       goto prepd_reject;
>> +
>> +               reason = session->tt->xmit_task(task);
>> +               if (reason) {
>> +                       if (reason == -ENOMEM || reason == -EAGAIN) {
>> +                               session->cmdsn--;
>
> I am pretty sure this has to be done anyway, no matter why the 
> xmit_task callback failed

Even if we abort? this just follows the same logic as 
iscsi_prep_scsi_cmd_pdu error flow.

>
>> + reason = FAILURE_SESSION_NOT_READY;
>> +                               goto prepd_reject;
>> +                       } else {
>> +                               sc->result = DID_ABORT << 16;
>> +                               goto prepd_fault;
>> +                       }
>>                 }
>>         } else {
>>                 list_add_tail(&task->running, &conn->cmdqueue);
>>
>> -- 
>> To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at http://vger.kernel.org/majordomo-info.html
>

Re: [PATCH v1 10/13] IB/iser: Support T10-PI operations

[Or Gerlitz]

On 04/03/2014 16:44, Sagi Grimberg wrote:
>>> @@ -1707,10 +1707,17 @@ int iscsi_queuecommand(struct Scsi_Host 
>>> *host, struct scsi_cmnd *sc)
>>>                                 goto prepd_fault;
>>>                         }
>>>                 }
>>> -               if (session->tt->xmit_task(task)) {
>>> -                       session->cmdsn--;
>>> -                       reason = FAILURE_SESSION_NOT_READY;
>>> -                       goto prepd_reject;
>>> +
>>> +               reason = session->tt->xmit_task(task);
>>> +               if (reason) {
>>> +                       if (reason == -ENOMEM || reason == -EAGAIN) {
>>> +                               session->cmdsn--;
>>
>> I am pretty sure this has to be done anyway, no matter why the 
>> xmit_task callback failed
>
> Even if we abort? this just follows the same logic as 
> iscsi_prep_scsi_cmd_pdu error flow.

yes, take a 2nd look on iscsi_prep_scsi_cmd_pdu and you'll see that all 
the possible error cases take place **before** session->cmdsn is 
incremented
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v1 10/13] IB/iser: Support T10-PI operations

[Sagi Grimberg]

On 3/4/2014 6:16 PM, Or Gerlitz wrote:
> On 04/03/2014 16:44, Sagi Grimberg wrote:
>>>> @@ -1707,10 +1707,17 @@ int iscsi_queuecommand(struct Scsi_Host 
>>>> *host, struct scsi_cmnd *sc)
>>>>                                 goto prepd_fault;
>>>>                         }
>>>>                 }
>>>> -               if (session->tt->xmit_task(task)) {
>>>> -                       session->cmdsn--;
>>>> -                       reason = FAILURE_SESSION_NOT_READY;
>>>> -                       goto prepd_reject;
>>>> +
>>>> +               reason = session->tt->xmit_task(task);
>>>> +               if (reason) {
>>>> +                       if (reason == -ENOMEM || reason == -EAGAIN) {
>>>> +                               session->cmdsn--;
>>>
>>> I am pretty sure this has to be done anyway, no matter why the 
>>> xmit_task callback failed
>>
>> Even if we abort? this just follows the same logic as 
>> iscsi_prep_scsi_cmd_pdu error flow.
>
> yes, take a 2nd look on iscsi_prep_scsi_cmd_pdu and you'll see that 
> all the possible error cases take place **before** session->cmdsn is 
> incremented

Ahhh, yes... So dec the cmdsn regardless.

Mike, are you on board with this?

Sagi.

Re: [PATCH v1 10/13] IB/iser: Support T10-PI operations

[Mike Christie]

On 03/04/2014 03:59 AM, Sagi Grimberg wrote:
> On 3/4/2014 11:38 AM, Or Gerlitz wrote:
>> On 03/03/2014 06:44, Mike Christie wrote:
>>> The xmit_task callout does handle failures like EINVAL. If the above map
>>> calls fail then you would get infinite retries. You would currently want
>>> to do the mapping in the init_task callout instead.
>>>
>>> If it makes it easier on the driver implementation then it is ok to
>>> modify the xmit_task callers so that they handle multiple error codes
>>> for drivers like iser that have the xmit_task callout called from
>>> iscsi_queuecommand.
>>
>> Mike,
>>
>> After looking on the code with Sagi,  it seems to us that the correct
>> way to go here, would be to enhance in iscsi_queuecommand the
>> processing of the result returned by session->tt->xmit_task(task) to
>> behave in a similar manner to how the return value of
>> iscsi_prep_scsi_cmd_pdu() is treated. E.g for errors such as ENOMEM
>> and EGAIN take the "reject" flow which would cause the SCSI midlayer
>> to retry the command and for other return values go to the "fault"
>> flow which will cause the ML to abort the command.
>>
>> Or.
>>
> 
> Yes, we were thinking about the following:
> --- a/drivers/scsi/libiscsi.c
> +++ b/drivers/scsi/libiscsi.c
> @@ -1707,10 +1707,17 @@ int iscsi_queuecommand(struct Scsi_Host *host,
> struct scsi_cmnd *sc)
>                                 goto prepd_fault;
>                         }
>                 }
> -               if (session->tt->xmit_task(task)) {
> -                       session->cmdsn--;
> -                       reason = FAILURE_SESSION_NOT_READY;
> -                       goto prepd_reject;
> +
> +               reason = session->tt->xmit_task(task);
> +               if (reason) {
> +                       if (reason == -ENOMEM || reason == -EAGAIN) {
> +                               session->cmdsn--;
> +                               reason = FAILURE_SESSION_NOT_READY;
> +                               goto prepd_reject;
> +                       } else {
> +                               sc->result = DID_ABORT << 16;
> +                               goto prepd_fault;
> +                       }

Or is correct.

If iscsi_prep_scsi_cmd_pdu is successful then it will increment cmdsn.

In this code path for xmit_task above, we assume that the driver can
either take the entire command and can send it here, or return error and
we requeue. For your new error case where we cannot send the command due
to a hard failure so we want to fail the command, then we still need to
decrement the cmdsn or there would be a hole since it was never put on
the wire.

Also, it is probably safest to check for the error code you are adding
support for:

reason = session->tt->xmit_task(task);
if (reason) {
	session->cmdsn--;

	if (reason == -EINVAL) {
		sc->result = DID_ABORT << 16;
		goto prepd_fault;
	} else {
		sc->result = DID_ABORT << 16;
		goto prepd_fault;
	}
}
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html