[PATCH 0/9] ib_srpt: Changes from RFC for v3.2-rc1 mainline merge

[PATCH 0/9] ib_srpt: Changes from RFC for v3.2-rc1 mainline merge

[Nicholas A. Bellinger]

From: Nicholas Bellinger <nab-IzHhD5pYlfBP7FQvKIMDCQ@public.gmane.org>

Hi Roland, Bart & Co.

The following series are patches from the last round of discussion for
the v3.2-rc1 mainline merge of ib_srpt. This includes the removal and/or
conversion of ib_srpt module parameters into per port configfs attributes,
inclusion of two bugfix patches from the SCST tree, and some other minor
changes as requested by Bart.

This leaves srp_max_req_size, srpt_srq_size, and srpt_service_guid as
module parameters.  The first two are currently required because they
are called in sprt_add_one() before the setup of individual srpt_port
endpoints, and the third apparently still needs to be global and would
not function as a per port configfs attribute.

Please have a look, and i'll be sending out a full patch again shortly
on top of target-pending.git with the other target-core patches headed 
or v3.2-rc1.

Thanks,

--nab

Bart Van Assche (2):
  ib_srpt: Fix potential out-of-bounds array access
  ib_srpt: Avoid failed multipart RDMA transfers

Nicholas Bellinger (7):
  ib_srpt: Fix srpt_alloc_fabric_acl failure case return value
  ib_srpt: Update comments to reference $driver/$port layout
  ib_srpt: Fix sport->port_guid formatting code
  ib_srpt: Remove legacy use_port_guid_in_session_name module parameter
  ib_srpt: Convert srp_max_rdma_size into per port configfs attribute
  ib_srpt: Convert srp_max_rsp_size into per port configfs attribute
  ib_srpt: Convert srpt_sq_size into per port configfs attribute

 drivers/infiniband/ulp/srpt/ib_srpt.c |  351 +++++++++++++++++++++-----------
 drivers/infiniband/ulp/srpt/ib_srpt.h |   41 ++++-
 2 files changed, 267 insertions(+), 125 deletions(-)

-- 
1.7.2.5

--
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 1/9] ib_srpt: Fix potential out-of-bounds array access

[Nicholas A. Bellinger]

From: Bart Van Assche <bvanassche@acm.org>

This patch fixes a potential out-of-bounds array access in
srpt_map_sg_to_ib_sge().  This is bugfix port from SCST svn r3262
as recommended by Bart Van Assche for the initial ib_srpt merge.

Cc: Bart Van Assche <bvanassche@acm.org>
Cc: Roland Dreier <roland@purestorage.com>
Signed-off-by: Nicholas A. Bellinger <nab@linux-iscsi.org>
---
 drivers/infiniband/ulp/srpt/ib_srpt.c |    7 ++++---
 1 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/drivers/infiniband/ulp/srpt/ib_srpt.c b/drivers/infiniband/ulp/srpt/ib_srpt.c
index e412a35..383994d 100644
--- a/drivers/infiniband/ulp/srpt/ib_srpt.c
+++ b/drivers/infiniband/ulp/srpt/ib_srpt.c
@@ -1142,7 +1142,8 @@ static int srpt_map_sg_to_ib_sge(struct srpt_rdma_ch *ch,
 	if (ioctx->rdma_ius && ioctx->n_rdma_ius)
 		nrdma = ioctx->n_rdma_ius;
 	else {
-		nrdma = count / SRPT_DEF_SG_PER_WQE + ioctx->n_rbuf;
+		nrdma = (count + SRPT_DEF_SG_PER_WQE - 1) / SRPT_DEF_SG_PER_WQE
+			+ ioctx->n_rbuf;
 
 		ioctx->rdma_ius = kzalloc(nrdma * sizeof *riu, GFP_KERNEL);
 		if (!ioctx->rdma_ius)
@@ -1258,11 +1259,11 @@ static int srpt_map_sg_to_ib_sge(struct srpt_rdma_ch *ch,
 			}
 
 			++k;
-			if (k == riu->sge_cnt && rsize > 0) {
+			if (k == riu->sge_cnt && rsize > 0 && tsize > 0) {
 				++riu;
 				sge = riu->sge;
 				k = 0;
-			} else if (rsize > 0)
+			} else if (rsize > 0 && tsize > 0)
 				++sge;
 		}
 	}
-- 
1.7.2.5

[PATCH 2/9] ib_srpt: Avoid failed multipart RDMA transfers

[Nicholas A. Bellinger]

From: Bart Van Assche <bvanassche@acm.org>

Multipart RDMA transfers can fail after one or more but not all RDMA
transfers have been initiated because either an IB cable has been pulled
or the ib_srpt kernel module has been unloaded while an RDMA transfer is
being setup.

This is a bugfix port from SCST svn r3632 as recommended by Bart Van
Assche.

Cc: Bart Van Assche <bvanassche@acm.org>
Cc: Roland Dreier <roland@purestorage.com>
Signed-off-by: Nicholas A. Bellinger <nab@linux-iscsi.org>
---
 drivers/infiniband/ulp/srpt/ib_srpt.c |  101 +++++++++++++++++++++-----------
 drivers/infiniband/ulp/srpt/ib_srpt.h |   26 +++++++-
 2 files changed, 88 insertions(+), 39 deletions(-)

diff --git a/drivers/infiniband/ulp/srpt/ib_srpt.c b/drivers/infiniband/ulp/srpt/ib_srpt.c
index 383994d..6aba709 100644
--- a/drivers/infiniband/ulp/srpt/ib_srpt.c
+++ b/drivers/infiniband/ulp/srpt/ib_srpt.c
@@ -807,7 +807,7 @@ static int srpt_post_recv(struct srpt_device *sdev,
 	struct ib_recv_wr wr, *bad_wr;
 
 	BUG_ON(!sdev);
-	wr.wr_id = encode_wr_id(IB_WC_RECV, ioctx->ioctx.index);
+	wr.wr_id = encode_wr_id(SRPT_RECV, ioctx->ioctx.index);
 
 	list.addr = ioctx->ioctx.dma;
 	list.length = srp_max_req_size;
@@ -849,7 +849,7 @@ static int srpt_post_send(struct srpt_rdma_ch *ch,
 	list.lkey = sdev->mr->lkey;
 
 	wr.next = NULL;
-	wr.wr_id = encode_wr_id(IB_WC_SEND, ioctx->ioctx.index);
+	wr.wr_id = encode_wr_id(SRPT_SEND, ioctx->ioctx.index);
 	wr.sg_list = &list;
 	wr.num_sge = 1;
 	wr.opcode = IB_WR_SEND;
@@ -1494,17 +1494,26 @@ static void srpt_handle_send_comp(struct srpt_rdma_ch *ch,
  * check_stop_free() callback.
  */
 static void srpt_handle_rdma_comp(struct srpt_rdma_ch *ch,
-				  struct srpt_send_ioctx *ioctx)
+				  struct srpt_send_ioctx *ioctx,
+				  enum srpt_opcode opcode)
 {
 	WARN_ON(ioctx->n_rdma <= 0);
 	atomic_add(ioctx->n_rdma, &ch->sq_wr_avail);
 
-	if (srpt_test_and_set_cmd_state(ioctx, SRPT_STATE_NEED_DATA,
-					SRPT_STATE_DATA_IN))
-		transport_generic_handle_data(&ioctx->cmd);
-	else
-		printk(KERN_ERR "%s[%d]: wrong state = %d\n", __func__,
-		       __LINE__, srpt_get_cmd_state(ioctx));
+	if (opcode == SRPT_RDMA_READ_LAST) {
+		if (srpt_test_and_set_cmd_state(ioctx, SRPT_STATE_NEED_DATA,
+						SRPT_STATE_DATA_IN))
+			transport_generic_handle_data(&ioctx->cmd);
+		else
+			printk(KERN_ERR "%s[%d]: wrong state = %d\n", __func__,
+			       __LINE__, srpt_get_cmd_state(ioctx));
+	} else if (opcode == SRPT_RDMA_ABORT) {
+		ioctx->rdma_aborted = true;
+	} else {
+		WARN_ON(opcode != SRPT_RDMA_READ_LAST);
+		printk(KERN_ERR "%s[%d]: scmnd == NULL (opcode %d)", __func__,
+				__LINE__, opcode);
+	}
 }
 
 /**
@@ -1512,7 +1521,7 @@ static void srpt_handle_rdma_comp(struct srpt_rdma_ch *ch,
  */
 static void srpt_handle_rdma_err_comp(struct srpt_rdma_ch *ch,
 				      struct srpt_send_ioctx *ioctx,
-				      u8 opcode)
+				      enum srpt_opcode opcode)
 {
 	struct se_cmd *cmd;
 	enum srpt_command_state state;
@@ -1520,7 +1529,7 @@ static void srpt_handle_rdma_err_comp(struct srpt_rdma_ch *ch,
 	cmd = &ioctx->cmd;
 	state = srpt_get_cmd_state(ioctx);
 	switch (opcode) {
-	case IB_WC_RDMA_READ:
+	case SRPT_RDMA_READ_LAST:
 		if (ioctx->n_rdma <= 0) {
 			printk(KERN_ERR "Received invalid RDMA read"
 			       " error completion with idx %d\n",
@@ -1534,9 +1543,8 @@ static void srpt_handle_rdma_err_comp(struct srpt_rdma_ch *ch,
 			printk(KERN_ERR "%s[%d]: wrong state = %d\n",
 			       __func__, __LINE__, state);
 		break;
-	case IB_WC_RDMA_WRITE:
-		atomic_set(&ioctx->cmd.transport_lun_stop,
-			   1);
+	case SRPT_RDMA_WRITE_LAST:
+		atomic_set(&ioctx->cmd.transport_lun_stop, 1);
 		break;
 	default:
 		printk(KERN_ERR "%s[%d]: opcode = %u\n", __func__,
@@ -2041,33 +2049,32 @@ static void srpt_process_send_completion(struct ib_cq *cq,
 {
 	struct srpt_send_ioctx *send_ioctx;
 	uint32_t index;
-	u8 opcode;
+	enum srpt_opcode opcode;
 
 	index = idx_from_wr_id(wc->wr_id);
 	opcode = opcode_from_wr_id(wc->wr_id);
 	send_ioctx = ch->ioctx_ring[index];
 	if (wc->status == IB_WC_SUCCESS) {
-		if (opcode == IB_WC_SEND)
+		if (opcode == SRPT_SEND)
 			srpt_handle_send_comp(ch, send_ioctx);
 		else {
-			WARN_ON(wc->opcode != IB_WC_RDMA_READ);
-			srpt_handle_rdma_comp(ch, send_ioctx);
+			WARN_ON(opcode != SRPT_RDMA_ABORT &&
+				wc->opcode != IB_WC_RDMA_READ);
+			srpt_handle_rdma_comp(ch, send_ioctx, opcode);
 		}
 	} else {
-		if (opcode == IB_WC_SEND) {
+		if (opcode == SRPT_SEND) {
 			printk(KERN_INFO "sending response for idx %u failed"
 			       " with status %d\n", index, wc->status);
 			srpt_handle_send_err_comp(ch, wc->wr_id);
-		} else {
-			printk(KERN_INFO "RDMA %s for idx %u failed with status"
-			       " %d\n", opcode == IB_WC_RDMA_READ ? "read"
-			       : opcode == IB_WC_RDMA_WRITE ? "write"
-			       : "???", index, wc->status);
+		} else if (opcode != SRPT_RDMA_MID) {
+			printk(KERN_INFO "RDMA t %d for idx %u failed with"
+				" status %d", opcode, index, wc->status);
 			srpt_handle_rdma_err_comp(ch, send_ioctx, opcode);
 		}
 	}
 
-	while (unlikely(opcode == IB_WC_SEND
+	while (unlikely(opcode == SRPT_SEND
 			&& !list_empty(&ch->cmd_wait_list)
 			&& srpt_get_ch_state(ch) == CH_LIVE
 			&& (send_ioctx = srpt_get_send_ioctx(ch)) != NULL)) {
@@ -2091,7 +2098,7 @@ static void srpt_process_completion(struct ib_cq *cq, struct srpt_rdma_ch *ch)
 	ib_req_notify_cq(cq, IB_CQ_NEXT_COMP);
 	while ((n = ib_poll_cq(cq, ARRAY_SIZE(ch->wc), wc)) > 0) {
 		for (i = 0; i < n; i++) {
-			if (opcode_from_wr_id(wc[i].wr_id) & IB_WC_RECV)
+			if (opcode_from_wr_id(wc[i].wr_id) == SRPT_RECV)
 				srpt_process_rcv_completion(cq, ch, &wc[i]);
 			else
 				srpt_process_send_completion(cq, ch, &wc[i]);
@@ -2882,32 +2889,37 @@ static int srpt_perform_rdmas(struct srpt_rdma_ch *ch,
 	int ret;
 	int sq_wr_avail;
 	enum dma_data_direction dir;
+	const int n_rdma = ioctx->n_rdma;
 
 	dir = ioctx->cmd.data_direction;
 	if (dir == DMA_TO_DEVICE) {
 		/* write */
 		ret = -ENOMEM;
-		sq_wr_avail = atomic_sub_return(ioctx->n_rdma,
-						 &ch->sq_wr_avail);
+		sq_wr_avail = atomic_sub_return(n_rdma, &ch->sq_wr_avail);
 		if (sq_wr_avail < 0) {
 			printk(KERN_WARNING "IB send queue full (needed %d)\n",
-			       ioctx->n_rdma);
+			       n_rdma);
 			goto out;
 		}
 	}
 
+	ioctx->rdma_aborted = false;
 	ret = 0;
 	riu = ioctx->rdma_ius;
 	memset(&wr, 0, sizeof wr);
 
-	for (i = 0; i < ioctx->n_rdma; ++i, ++riu) {
+	for (i = 0; i < n_rdma; ++i, ++riu) {
 		if (dir == DMA_FROM_DEVICE) {
 			wr.opcode = IB_WR_RDMA_WRITE;
-			wr.wr_id = encode_wr_id(IB_WC_RDMA_WRITE,
+			wr.wr_id = encode_wr_id(i == n_rdma - 1 ?
+						SRPT_RDMA_WRITE_LAST :
+						SRPT_RDMA_MID,
 						ioctx->ioctx.index);
 		} else {
 			wr.opcode = IB_WR_RDMA_READ;
-			wr.wr_id = encode_wr_id(IB_WC_RDMA_READ,
+			wr.wr_id = encode_wr_id(i == n_rdma - 1 ?
+						SRPT_RDMA_READ_LAST :
+						SRPT_RDMA_MID,
 						ioctx->ioctx.index);
 		}
 		wr.next = NULL;
@@ -2917,17 +2929,36 @@ static int srpt_perform_rdmas(struct srpt_rdma_ch *ch,
 		wr.sg_list = riu->sge;
 
 		/* only get completion event for the last rdma write */
-		if (i == (ioctx->n_rdma - 1) && dir == DMA_TO_DEVICE)
+		if (i == (n_rdma - 1) && dir == DMA_TO_DEVICE)
 			wr.send_flags = IB_SEND_SIGNALED;
 
 		ret = ib_post_send(ch->qp, &wr, &bad_wr);
 		if (ret)
-			goto out;
+			break;
 	}
 
+	if (ret)
+		printk(KERN_ERR "%s[%d]: ib_post_send() returned %d for %d/%d",
+				 __func__, __LINE__, ret, i, n_rdma);
+	if (ret && i > 0) {
+		wr.num_sge = 0;
+		wr.wr_id = encode_wr_id(SRPT_RDMA_ABORT, ioctx->ioctx.index);
+		wr.send_flags = IB_SEND_SIGNALED;
+		while (ch->state == CH_LIVE &&
+			ib_post_send(ch->qp, &wr, &bad_wr) != 0) {
+			printk(KERN_INFO "Trying to abort failed RDMA transfer [%d]",
+				ioctx->ioctx.index);
+			msleep(1000);
+		}
+		while (ch->state != CH_RELEASING && !ioctx->rdma_aborted) {
+			printk(KERN_INFO "Waiting until RDMA abort finished [%d]",
+				ioctx->ioctx.index);
+			msleep(1000);
+		}
+	}
 out:
 	if (unlikely(dir == DMA_TO_DEVICE && ret < 0))
-		atomic_add(ioctx->n_rdma, &ch->sq_wr_avail);
+		atomic_add(n_rdma, &ch->sq_wr_avail);
 	return ret;
 }
 
diff --git a/drivers/infiniband/ulp/srpt/ib_srpt.h b/drivers/infiniband/ulp/srpt/ib_srpt.h
index 045fb7b..59ee2d7 100644
--- a/drivers/infiniband/ulp/srpt/ib_srpt.h
+++ b/drivers/infiniband/ulp/srpt/ib_srpt.h
@@ -127,12 +127,27 @@ enum {
 	DEFAULT_MAX_RDMA_SIZE = 65536,
 };
 
+enum srpt_opcode {
+	SRPT_RECV,
+	SRPT_SEND,
+	SRPT_RDMA_MID,
+	SRPT_RDMA_ABORT,
+	SRPT_RDMA_READ_LAST,
+	SRPT_RDMA_WRITE_LAST,
+};
+
 static inline u64 encode_wr_id(u8 opcode, u32 idx)
-{ return ((u64)opcode << 32) | idx; }
-static inline u8 opcode_from_wr_id(u64 wr_id)
-{ return wr_id >> 32; }
+{
+	return ((u64)opcode << 32) | idx;
+}
+static inline enum srpt_opcode opcode_from_wr_id(u64 wr_id)
+{
+	return wr_id >> 32;
+}
 static inline u32 idx_from_wr_id(u64 wr_id)
-{ return (u32)wr_id; }
+{
+	return (u32)wr_id;
+}
 
 struct rdma_iu {
 	u64		raddr;
@@ -204,6 +219,8 @@ struct srpt_recv_ioctx {
  * @tag:         Tag of the received SRP information unit.
  * @spinlock:    Protects 'state'.
  * @state:       I/O context state.
+ * @rdma_aborted: If initiating a multipart RDMA transfer failed, whether
+ * 		 the already initiated transfers have finished.
  * @cmd:         Target core command data structure.
  * @sense_data:  SCSI sense data.
  */
@@ -218,6 +235,7 @@ struct srpt_send_ioctx {
 	struct list_head	free_list;
 	spinlock_t		spinlock;
 	enum srpt_command_state	state;
+	bool			rdma_aborted;
 	struct se_cmd		cmd;
 	struct completion	tx_done;
 	u64			tag;
-- 
1.7.2.5

[PATCH 3/9] ib_srpt: Fix srpt_alloc_fabric_acl failure case return value

[Nicholas A. Bellinger]

From: Nicholas Bellinger <nab@linux-iscsi.org>

This patch fixes a bug in srpt_make_nodeacl(), where a failure of
srpt_alloc_fabric_acl() was not setting a proper error return code.

Reported-by: Bart Van Assche <bvanassche@acm.org>
Cc: Bart Van Assche <bvanassche@acm.org>
Cc: Roland Dreier <roland@purestorage.com>
Signed-off-by: Nicholas A. Bellinger <nab@linux-iscsi.org>
---
 drivers/infiniband/ulp/srpt/ib_srpt.c |    4 +++-
 1 files changed, 3 insertions(+), 1 deletions(-)

diff --git a/drivers/infiniband/ulp/srpt/ib_srpt.c b/drivers/infiniband/ulp/srpt/ib_srpt.c
index 6aba709..fee78b0 100644
--- a/drivers/infiniband/ulp/srpt/ib_srpt.c
+++ b/drivers/infiniband/ulp/srpt/ib_srpt.c
@@ -3679,8 +3679,10 @@ static struct se_node_acl *srpt_make_nodeacl(struct se_portal_group *tpg,
 	}
 
 	se_nacl_new = srpt_alloc_fabric_acl(tpg);
-	if (!se_nacl_new)
+	if (!se_nacl_new) {
+		ret = -ENOMEM;
 		goto err;
+	}
 	/*
 	 * nacl_new may be released by core_tpg_add_initiator_node_acl()
 	 * when converting a node ACL from demo mode to explict
-- 
1.7.2.5

[PATCH 4/9] ib_srpt: Update comments to reference $driver/$port layout

[Nicholas A. Bellinger]

From: Nicholas Bellinger <nab@linux-iscsi.org>

Reported-by: Bart Van Assche <bvanassche@acm.org>
Cc: Bart Van Assche <bvanassche@acm.org>
Cc: Roland Dreier <roland@purestorage.com>
Signed-off-by: Nicholas A. Bellinger <nab@linux-iscsi.org>
---
 drivers/infiniband/ulp/srpt/ib_srpt.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/infiniband/ulp/srpt/ib_srpt.c b/drivers/infiniband/ulp/srpt/ib_srpt.c
index fee78b0..6e19816 100644
--- a/drivers/infiniband/ulp/srpt/ib_srpt.c
+++ b/drivers/infiniband/ulp/srpt/ib_srpt.c
@@ -3805,7 +3805,7 @@ static void srpt_drop_tpg(struct se_portal_group *tpg)
 
 /**
  * configfs callback invoked for
- * mkdir /sys/kernel/config/target/$driver/$target
+ * mkdir /sys/kernel/config/target/$driver/$port
  */
 static struct se_wwn *srpt_make_tport(struct target_fabric_configfs *tf,
 				      struct config_group *group,
@@ -3828,7 +3828,7 @@ err:
 
 /**
  * configfs callback invoked for
- * rmdir /sys/kernel/config/target/$driver/$target
+ * rmdir /sys/kernel/config/target/$driver/$port
  */
 static void srpt_drop_tport(struct se_wwn *wwn)
 {
-- 
1.7.2.5

[PATCH 5/9] ib_srpt: Fix sport->port_guid formatting code

[Nicholas A. Bellinger]

From: Nicholas Bellinger <nab@linux-iscsi.org>

This patch fixes the sport->port_guid formatting code in srpt_add_one() to
properly use sport->gid.global.interface_id instead of device->node_guid w/
a port offset.  This requires using ib_query_gid() from srpt_refresh_port(),
so the sport->port_guid assignment has been moved after srpt_refresh_port().

Reported-by: Bart Van Assche <bvanassche@acm.org>
Cc: Bart Van Assche <bvanassche@acm.org>
Cc: Roland Dreier <roland@purestorage.com>
Signed-off-by: Nicholas A. Bellinger <nab@linux-iscsi.org>
---
 drivers/infiniband/ulp/srpt/ib_srpt.c |    9 +++------
 1 files changed, 3 insertions(+), 6 deletions(-)

diff --git a/drivers/infiniband/ulp/srpt/ib_srpt.c b/drivers/infiniband/ulp/srpt/ib_srpt.c
index 6e19816..ae9fd0e 100644
--- a/drivers/infiniband/ulp/srpt/ib_srpt.c
+++ b/drivers/infiniband/ulp/srpt/ib_srpt.c
@@ -3325,17 +3325,14 @@ static void srpt_add_one(struct ib_device *device)
 		INIT_LIST_HEAD(&sport->port_acl_list);
 		spin_lock_init(&sport->port_acl_lock);
 
-		sprintf(sport->port_guid, "0x0000000000000000%04x%04x%04x%04x",
-                                be16_to_cpu(((__be16 *)&device->node_guid)[0]),
-                                be16_to_cpu(((__be16 *)&device->node_guid)[1]),
-                                be16_to_cpu(((__be16 *)&device->node_guid)[2]),
-                                be16_to_cpu(((__be16 *)&device->node_guid)[3]) + i);
-
 		if (srpt_refresh_port(sport)) {
 			printk(KERN_ERR "MAD registration failed for %s-%d.\n",
 			       srpt_sdev_name(sdev), i);
 			goto err_ring;
 		}
+		snprintf(sport->port_guid, sizeof(sport->port_guid),
+				"0x0000000000000000%016llx",
+				be64_to_cpu(sport->gid.global.interface_id));
 	}
 
 	spin_lock(&srpt_dev_lock);
-- 
1.7.2.5

[PATCH 6/9] ib_srpt: Remove legacy use_port_guid_in_session_name module parameter

[Nicholas A. Bellinger]

From: Nicholas Bellinger <nab@linux-iscsi.org>

This patch removes the legacy use_port_guid_in_session_name module parameter
that is no longer required in modern ib_srpt code.

Reported-by: Roland Dreier <roland@purestorage.com>
Cc: Roland Dreier <roland@purestorage.com>
Cc: Bart Van Assche <bvanassche@acm.org>
Signed-off-by: Nicholas A. Bellinger <nab@linux-iscsi.org>
---
 drivers/infiniband/ulp/srpt/ib_srpt.c |   36 +++++---------------------------
 1 files changed, 6 insertions(+), 30 deletions(-)

diff --git a/drivers/infiniband/ulp/srpt/ib_srpt.c b/drivers/infiniband/ulp/srpt/ib_srpt.c
index ae9fd0e..5c61ba4 100644
--- a/drivers/infiniband/ulp/srpt/ib_srpt.c
+++ b/drivers/infiniband/ulp/srpt/ib_srpt.c
@@ -103,12 +103,6 @@ module_param(srpt_sq_size, int, 0444);
 MODULE_PARM_DESC(srpt_sq_size,
 		 "Per-channel send queue (SQ) size.");
 
-static bool use_port_guid_in_session_name;
-module_param(use_port_guid_in_session_name, bool, 0444);
-MODULE_PARM_DESC(use_port_guid_in_session_name,
-		 "Use target port ID in the session name such that"
-		 " redundant paths between multiport systems can be masked.");
-
 static int srpt_get_u64_x(char *buffer, struct kernel_param *kp)
 {
 	return sprintf(buffer, "0x%016llx", *(u64 *)kp->arg);
@@ -2605,30 +2599,12 @@ static int srpt_cm_req_recv(struct ib_cm_id *cm_id,
 		       " RTR failed (error code = %d)\n", ret);
 		goto destroy_ib;
 	}
-
-	if (use_port_guid_in_session_name) {
-		/*
-		 * If the kernel module parameter use_port_guid_in_session_name
-		 * has been specified, use a combination of the target port
-		 * GUID and the initiator port ID as the session name. This
-		 * was the original behavior of the SRP target implementation
-		 * (i.e. before the SRPT was included in OFED 1.3).
-		 */
-		snprintf(ch->sess_name, sizeof(ch->sess_name),
-			 "0x%016llx%016llx",
-			 be64_to_cpu(*(__be64 *)
-				&sdev->port[param->port - 1].gid.raw[8]),
-			 be64_to_cpu(*(__be64 *)(ch->i_port_id + 8)));
-	} else {
-		/*
-		 * Default behavior: use the initator port identifier as the
-		 * session name.
-		 */
-		snprintf(ch->sess_name, sizeof(ch->sess_name),
-			 "0x%016llx%016llx",
-			 be64_to_cpu(*(__be64 *)ch->i_port_id),
-			 be64_to_cpu(*(__be64 *)(ch->i_port_id + 8)));
-	}
+	/*
+	 * Use the initator port identifier as the session name.
+	 */
+	snprintf(ch->sess_name, sizeof(ch->sess_name), "0x%016llx%016llx",
+			be64_to_cpu(*(__be64 *)ch->i_port_id),
+			be64_to_cpu(*(__be64 *)(ch->i_port_id + 8)));
 
 	pr_debug("registering session %s\n", ch->sess_name);
 
-- 
1.7.2.5

[PATCH 7/9] ib_srpt: Convert srp_max_rdma_size into per port configfs attribute

[Nicholas A. Bellinger]

From: Nicholas Bellinger <nab@linux-iscsi.org>

This patch converts the srp_max_rdma_size module parameter into a per
endpoint configfs attribute.  This includes adding the necessary bits
for show + store attributes w/ min/max bounds checking, and updating
srpt_get_ioc() to accept a struct srpt_port parameter.

Reported-by: Roland Dreier <roland@purestorage.com>
Cc: Roland Dreier <roland@purestorage.com>
Cc: Bart Van Assche <bvanassche@acm.org>
Signed-off-by: Nicholas A. Bellinger <nab@linux-iscsi.org>
---
 drivers/infiniband/ulp/srpt/ib_srpt.c |   60 ++++++++++++++++++++++++++++-----
 drivers/infiniband/ulp/srpt/ib_srpt.h |   10 +++++
 2 files changed, 61 insertions(+), 9 deletions(-)

diff --git a/drivers/infiniband/ulp/srpt/ib_srpt.c b/drivers/infiniband/ulp/srpt/ib_srpt.c
index 5c61ba4..80ef5af 100644
--- a/drivers/infiniband/ulp/srpt/ib_srpt.c
+++ b/drivers/infiniband/ulp/srpt/ib_srpt.c
@@ -78,11 +78,6 @@ static u64 srpt_service_guid;
 static spinlock_t srpt_dev_lock;       /* Protects srpt_dev_list. */
 static struct list_head srpt_dev_list; /* List of srpt_device structures. */
 
-static unsigned srp_max_rdma_size = DEFAULT_MAX_RDMA_SIZE;
-module_param(srp_max_rdma_size, int, 0744);
-MODULE_PARM_DESC(srp_max_rdma_size,
-		 "Maximum size of SRP RDMA transfers for new connections.");
-
 static unsigned srp_max_req_size = DEFAULT_MAX_REQ_SIZE;
 module_param(srp_max_req_size, int, 0444);
 MODULE_PARM_DESC(srp_max_req_size,
@@ -342,9 +337,10 @@ static void srpt_get_iou(struct ib_dm_mad *mad)
  * Architecture Specification. See also section B.7, table B.7 in the SRP
  * r16a document.
  */
-static void srpt_get_ioc(struct srpt_device *sdev, u32 slot,
+static void srpt_get_ioc(struct srpt_port *sport, u32 slot,
 			 struct ib_dm_mad *mad)
 {
+	struct srpt_device *sdev = sport->sdev;
 	struct ib_dm_ioc_profile *iocp;
 
 	iocp = (struct ib_dm_ioc_profile *)mad->data;
@@ -376,7 +372,7 @@ static void srpt_get_ioc(struct srpt_device *sdev, u32 slot,
 	iocp->send_queue_depth = cpu_to_be16(sdev->srq_size);
 	iocp->rdma_read_depth = 4;
 	iocp->send_size = cpu_to_be32(srp_max_req_size);
-	iocp->rdma_size = cpu_to_be32(min(max(srp_max_rdma_size, 256U),
+	iocp->rdma_size = cpu_to_be32(min(sport->port_attrib.srp_max_rdma_size,
 					  1U << 24));
 	iocp->num_svc_entries = 1;
 	iocp->op_cap_mask = SRP_SEND_TO_IOC | SRP_SEND_FROM_IOC |
@@ -445,7 +441,7 @@ static void srpt_mgmt_method_get(struct srpt_port *sp, struct ib_mad *rq_mad,
 		break;
 	case DM_ATTR_IOC_PROFILE:
 		slot = be32_to_cpu(rq_mad->mad_hdr.attr_mod);
-		srpt_get_ioc(sp->sdev, slot, rsp_mad);
+		srpt_get_ioc(sp, slot, rsp_mad);
 		break;
 	case DM_ATTR_SVC_ENTRIES:
 		slot = be32_to_cpu(rq_mad->mad_hdr.attr_mod);
@@ -3297,6 +3293,7 @@ static void srpt_add_one(struct ib_device *device)
 		sport = &sdev->port[i - 1];
 		sport->sdev = sdev;
 		sport->port = i;
+		sport->port_attrib.srp_max_rdma_size = DEFAULT_MAX_RDMA_SIZE;
 		INIT_WORK(&sport->work, srpt_refresh_port_work);
 		INIT_LIST_HEAD(&sport->port_acl_list);
 		spin_lock_init(&sport->port_acl_lock);
@@ -3700,6 +3697,51 @@ static void srpt_drop_nodeacl(struct se_node_acl *se_nacl)
 	srpt_release_fabric_acl(NULL, se_nacl);
 }
 
+static ssize_t srpt_tpg_attrib_show_srp_max_rdma_size(
+	struct se_portal_group *se_tpg,
+	char *page)
+{
+	struct srpt_port *sport = container_of(se_tpg, struct srpt_port, port_tpg_1);
+
+	return sprintf(page, "%u\n", sport->port_attrib.srp_max_rdma_size);
+}
+
+static ssize_t srpt_tpg_attrib_store_srp_max_rdma_size(
+	struct se_portal_group *se_tpg,
+	const char *page,
+	size_t count)
+{
+	struct srpt_port *sport = container_of(se_tpg, struct srpt_port, port_tpg_1);
+	unsigned long val;
+	int ret;
+	
+	ret = strict_strtoul(page, 0, &val);
+	if (ret < 0) {
+		pr_err("strict_strtoul() failed with ret: %d\n", ret);
+		return -EINVAL;
+	}
+	if (val > MAX_SRPT_RDMA_SIZE) {
+		pr_err("val: %lu exceeds MAX_SRPT_RDMA_SIZE: %d\n", val,
+			MAX_SRPT_RDMA_SIZE);
+		return -EINVAL;
+	}
+	if (val < DEFAULT_MAX_RDMA_SIZE) {
+		pr_err("val: %lu smaller than DEFAULT_MAX_RDMA_SIZE: %d\n",
+			val, DEFAULT_MAX_RDMA_SIZE);
+		return -EINVAL;
+	}
+	sport->port_attrib.srp_max_rdma_size = val;
+	
+	return count;
+}
+
+TF_TPG_ATTRIB_ATTR(srpt, srp_max_rdma_size, S_IRUGO | S_IWUSR);
+
+static struct configfs_attribute *srpt_tpg_attrib_attrs[] = {
+	&srpt_tpg_attrib_srp_max_rdma_size.attr,
+	NULL,
+};
+
 static ssize_t srpt_tpg_show_enable(
 	struct se_portal_group *se_tpg,
 	char *page)
@@ -3937,7 +3979,7 @@ static int __init srpt_init_module(void)
 	 */
 	srpt_target->tf_cit_tmpl.tfc_wwn_cit.ct_attrs = srpt_wwn_attrs;
 	srpt_target->tf_cit_tmpl.tfc_tpg_base_cit.ct_attrs = srpt_tpg_attrs;
-	srpt_target->tf_cit_tmpl.tfc_tpg_attrib_cit.ct_attrs = NULL;
+	srpt_target->tf_cit_tmpl.tfc_tpg_attrib_cit.ct_attrs = srpt_tpg_attrib_attrs;
 	srpt_target->tf_cit_tmpl.tfc_tpg_param_cit.ct_attrs = NULL;
 	srpt_target->tf_cit_tmpl.tfc_tpg_np_base_cit.ct_attrs = NULL;
 	srpt_target->tf_cit_tmpl.tfc_tpg_nacl_base_cit.ct_attrs = NULL;
diff --git a/drivers/infiniband/ulp/srpt/ib_srpt.h b/drivers/infiniband/ulp/srpt/ib_srpt.h
index 59ee2d7..c9caa90 100644
--- a/drivers/infiniband/ulp/srpt/ib_srpt.h
+++ b/drivers/infiniband/ulp/srpt/ib_srpt.h
@@ -114,6 +114,7 @@ enum {
 	MIN_SRPT_SRQ_SIZE = 4,
 	DEFAULT_SRPT_SRQ_SIZE = 4095,
 	MAX_SRPT_SRQ_SIZE = 65535,
+	MAX_SRPT_RDMA_SIZE = 256U,
 
 	MIN_MAX_REQ_SIZE = 996,
 	DEFAULT_MAX_REQ_SIZE
@@ -326,6 +327,14 @@ struct srpt_rdma_ch {
 };
 
 /**
+ * struct srpt_port_attib - Attributes for SRPT port
+ * @srp_max_rdma_size: Maximum size of SRP RDMA transfers for new connections.
+ */
+struct srpt_port_attrib {
+	u32			srp_max_rdma_size;
+};
+
+/**
  * struct srpt_port - Information associated by SRPT with a single IB port.
  * @sdev:      backpointer to the HCA information.
  * @mad_agent: per-port management datagram processing information.
@@ -355,6 +364,7 @@ struct srpt_port {
 	struct se_portal_group	port_tpg_1;
 	struct se_wwn		port_wwn;
 	struct list_head	port_acl_list;
+	struct srpt_port_attrib port_attrib;
 };
 
 /**
-- 
1.7.2.5

[PATCH 8/9] ib_srpt: Convert srp_max_rsp_size into per port configfs attribute

[Nicholas A. Bellinger]

From: Nicholas Bellinger <nab@linux-iscsi.org>

This patch converts the srp_max_rsp_size module parameter into a per
port configfs attribute.  This includes adding the necessary bits for
show + store attributes w/ min/max bounds checking, and dropping
the legacy WARN_ON() as sport_port is not available for all callers
of srpt_alloc_ioctx_ring() and srpt_free_ioctx_ring().

It also drops the legacy srp_max_rsp_size module parameter checks
in srpt_init_module(), and adds new MAX_SRPT_RSP_SIZE = 1024 for
proper bounds checking in srpt_tpg_attrib_store_srp_max_rsp_size().

Reported-by: Roland Dreier <roland@purestorage.com>
Cc: Roland Dreier <roland@purestorage.com>
Cc: Bart Van Assche <bvanassche@acm.org>
Signed-off-by: Nicholas A. Bellinger <nab@linux-iscsi.org>
---
 drivers/infiniband/ulp/srpt/ib_srpt.c |   67 +++++++++++++++++++++++---------
 drivers/infiniband/ulp/srpt/ib_srpt.h |    3 +
 2 files changed, 51 insertions(+), 19 deletions(-)

diff --git a/drivers/infiniband/ulp/srpt/ib_srpt.c b/drivers/infiniband/ulp/srpt/ib_srpt.c
index 80ef5af..f4a900b 100644
--- a/drivers/infiniband/ulp/srpt/ib_srpt.c
+++ b/drivers/infiniband/ulp/srpt/ib_srpt.c
@@ -83,11 +83,6 @@ module_param(srp_max_req_size, int, 0444);
 MODULE_PARM_DESC(srp_max_req_size,
 		 "Maximum size of SRP request messages in bytes.");
 
-static unsigned int srp_max_rsp_size = DEFAULT_MAX_RSP_SIZE;
-module_param(srp_max_rsp_size, int, 0444);
-MODULE_PARM_DESC(srp_max_rsp_size,
-		 "Maximum size of SRP response messages in bytes.");
-
 static int srpt_srq_size = DEFAULT_SRPT_SRQ_SIZE;
 module_param(srpt_srq_size, int, 0444);
 MODULE_PARM_DESC(srpt_srq_size,
@@ -687,7 +682,6 @@ static struct srpt_ioctx **srpt_alloc_ioctx_ring(struct srpt_device *sdev,
 
 	WARN_ON(ioctx_size != sizeof(struct srpt_recv_ioctx)
 		&& ioctx_size != sizeof(struct srpt_send_ioctx));
-	WARN_ON(dma_size != srp_max_req_size && dma_size != srp_max_rsp_size);
 
 	ring = kmalloc(ring_size * sizeof(ring[0]), GFP_KERNEL);
 	if (!ring)
@@ -717,8 +711,6 @@ static void srpt_free_ioctx_ring(struct srpt_ioctx **ioctx_ring,
 {
 	int i;
 
-	WARN_ON(dma_size != srp_max_req_size && dma_size != srp_max_rsp_size);
-
 	for (i = 0; i < ring_size; ++i)
 		srpt_free_ioctx(sdev, ioctx_ring[i], dma_size, dir);
 	kfree(ioctx_ring);
@@ -2384,7 +2376,8 @@ static void srpt_release_channel_work(struct work_struct *w)
 
 	srpt_free_ioctx_ring((struct srpt_ioctx **)ch->ioctx_ring,
 			     ch->sport->sdev, ch->rq_size,
-			     srp_max_rsp_size, DMA_TO_DEVICE);
+			     ch->sport->port_attrib.srp_max_rsp_size,
+			     DMA_TO_DEVICE);
 
 	spin_lock_irq(&sdev->spinlock);
 	list_del(&ch->list);
@@ -2568,7 +2561,8 @@ static int srpt_cm_req_recv(struct ib_cm_id *cm_id,
 	ch->ioctx_ring = (struct srpt_send_ioctx **)
 		srpt_alloc_ioctx_ring(ch->sport->sdev, ch->rq_size,
 				      sizeof(*ch->ioctx_ring[0]),
-				      srp_max_rsp_size, DMA_TO_DEVICE);
+				      ch->sport->port_attrib.srp_max_rsp_size,
+				      DMA_TO_DEVICE);
 	if (!ch->ioctx_ring)
 		goto free_ch;
 
@@ -2676,8 +2670,8 @@ destroy_ib:
 free_ring:
 	srpt_free_ioctx_ring((struct srpt_ioctx **)ch->ioctx_ring,
 			     ch->sport->sdev, ch->rq_size,
-			     srp_max_rsp_size, DMA_TO_DEVICE);
-
+			     ch->sport->port_attrib.srp_max_rsp_size,
+			     DMA_TO_DEVICE);
 free_ch:
 	kfree(ch);
 
@@ -3294,6 +3288,7 @@ static void srpt_add_one(struct ib_device *device)
 		sport->sdev = sdev;
 		sport->port = i;
 		sport->port_attrib.srp_max_rdma_size = DEFAULT_MAX_RDMA_SIZE;
+		sport->port_attrib.srp_max_rsp_size = DEFAULT_MAX_RSP_SIZE;
 		INIT_WORK(&sport->work, srpt_refresh_port_work);
 		INIT_LIST_HEAD(&sport->port_acl_list);
 		spin_lock_init(&sport->port_acl_lock);
@@ -3737,8 +3732,49 @@ static ssize_t srpt_tpg_attrib_store_srp_max_rdma_size(
 
 TF_TPG_ATTRIB_ATTR(srpt, srp_max_rdma_size, S_IRUGO | S_IWUSR);
 
+static ssize_t srpt_tpg_attrib_show_srp_max_rsp_size(
+	struct se_portal_group *se_tpg,
+	char *page)
+{
+	struct srpt_port *sport = container_of(se_tpg, struct srpt_port, port_tpg_1);
+
+	return sprintf(page, "%u\n", sport->port_attrib.srp_max_rsp_size);
+}
+
+static ssize_t srpt_tpg_attrib_store_srp_max_rsp_size(
+	struct se_portal_group *se_tpg,
+	const char *page,
+	size_t count)
+{
+	struct srpt_port *sport = container_of(se_tpg, struct srpt_port, port_tpg_1);
+	unsigned long val;
+	int ret;
+
+	ret = strict_strtoul(page, 0, &val);
+	if (ret < 0) {
+		pr_err("strict_strtoul() failed with ret: %d\n", ret);
+		return -EINVAL;
+	}
+	if (val > MAX_SRPT_RSP_SIZE) {
+		pr_err("val: %lu exceeds MAX_SRPT_RSP_SIZE: %d\n", val,
+			MAX_SRPT_RSP_SIZE);
+		return -EINVAL;
+	}
+	if (val < MIN_MAX_RSP_SIZE) {
+		pr_err("val: %lu smaller than MIN_MAX_RSP_SIZE: %d\n", val,
+			MIN_MAX_RSP_SIZE);
+		return -EINVAL;
+	}
+	sport->port_attrib.srp_max_rsp_size = val;
+
+	return count;
+}
+
+TF_TPG_ATTRIB_ATTR(srpt, srp_max_rsp_size, S_IRUGO | S_IWUSR);
+
 static struct configfs_attribute *srpt_tpg_attrib_attrs[] = {
 	&srpt_tpg_attrib_srp_max_rdma_size.attr,
+	&srpt_tpg_attrib_srp_max_rsp_size.attr,
 	NULL,
 };
 
@@ -3937,13 +3973,6 @@ static int __init srpt_init_module(void)
 		goto out;
 	}
 
-	if (srp_max_rsp_size < MIN_MAX_RSP_SIZE) {
-		printk(KERN_ERR "invalid value %d for kernel module parameter"
-		       " srp_max_rsp_size -- must be at least %d.\n",
-		       srp_max_rsp_size, MIN_MAX_RSP_SIZE);
-		goto out;
-	}
-
 	if (srpt_srq_size < MIN_SRPT_SRQ_SIZE
 	    || srpt_srq_size > MAX_SRPT_SRQ_SIZE) {
 		printk(KERN_ERR "invalid value %d for kernel module parameter"
diff --git a/drivers/infiniband/ulp/srpt/ib_srpt.h b/drivers/infiniband/ulp/srpt/ib_srpt.h
index c9caa90..27b1027 100644
--- a/drivers/infiniband/ulp/srpt/ib_srpt.h
+++ b/drivers/infiniband/ulp/srpt/ib_srpt.h
@@ -115,6 +115,7 @@ enum {
 	DEFAULT_SRPT_SRQ_SIZE = 4095,
 	MAX_SRPT_SRQ_SIZE = 65535,
 	MAX_SRPT_RDMA_SIZE = 256U,
+	MAX_SRPT_RSP_SIZE = 1024,
 
 	MIN_MAX_REQ_SIZE = 996,
 	DEFAULT_MAX_REQ_SIZE
@@ -329,9 +330,11 @@ struct srpt_rdma_ch {
 /**
  * struct srpt_port_attib - Attributes for SRPT port
  * @srp_max_rdma_size: Maximum size of SRP RDMA transfers for new connections.
+ * @srp_max_rsp_size: Maximum size of SRP response messages in bytes.
  */
 struct srpt_port_attrib {
 	u32			srp_max_rdma_size;
+	u32			srp_max_rsp_size;
 };
 
 /**
-- 
1.7.2.5

[PATCH 9/9] ib_srpt: Convert srpt_sq_size into per port configfs attribute

[Nicholas A. Bellinger]

From: Nicholas Bellinger <nab@linux-iscsi.org>

This patch converts the srpt_sq_size module parameter into a per
port configfs attribute.  This includes adding the necessary bits for
show + store attributes w/ min/max bounds checking, and dropping the
legacy srp_max_rsp_size module parameter checks in srpt_init_module().

Reported-by: Roland Dreier <roland@purestorage.com>
Cc: Roland Dreier <roland@purestorage.com>
Cc: Bart Van Assche <bvanassche@acm.org>
Signed-off-by: Nicholas A. Bellinger <nab@linux-iscsi.org>
---
 drivers/infiniband/ulp/srpt/ib_srpt.c |   63 ++++++++++++++++++++++++--------
 drivers/infiniband/ulp/srpt/ib_srpt.h |    2 +
 2 files changed, 49 insertions(+), 16 deletions(-)

diff --git a/drivers/infiniband/ulp/srpt/ib_srpt.c b/drivers/infiniband/ulp/srpt/ib_srpt.c
index f4a900b..9e1c5e0 100644
--- a/drivers/infiniband/ulp/srpt/ib_srpt.c
+++ b/drivers/infiniband/ulp/srpt/ib_srpt.c
@@ -88,11 +88,6 @@ module_param(srpt_srq_size, int, 0444);
 MODULE_PARM_DESC(srpt_srq_size,
 		 "Shared receive queue (SRQ) size.");
 
-static int srpt_sq_size = DEF_SRPT_SQ_SIZE;
-module_param(srpt_sq_size, int, 0444);
-MODULE_PARM_DESC(srpt_sq_size,
-		 "Per-channel send queue (SQ) size.");
-
 static int srpt_get_u64_x(char *buffer, struct kernel_param *kp)
 {
 	return sprintf(buffer, "0x%016llx", *(u64 *)kp->arg);
@@ -2133,7 +2128,8 @@ static int srpt_compl_thread(void *arg)
 static int srpt_create_ch_ib(struct srpt_rdma_ch *ch)
 {
 	struct ib_qp_init_attr *qp_init;
-	struct srpt_device *sdev = ch->sport->sdev;
+	struct srpt_port *sport = ch->sport;
+	struct srpt_device *sdev = sport->sdev;
 	int ret;
 
 	WARN_ON(ch->rq_size < 1);
@@ -2144,11 +2140,11 @@ static int srpt_create_ch_ib(struct srpt_rdma_ch *ch)
 		goto out;
 
 	ch->cq = ib_create_cq(sdev->device, srpt_completion, NULL, ch,
-			      ch->rq_size + srpt_sq_size, 0);
+			      ch->rq_size + sport->port_attrib.srp_sq_size, 0);
 	if (IS_ERR(ch->cq)) {
 		ret = PTR_ERR(ch->cq);
 		printk(KERN_ERR "failed to create CQ cqe= %d ret= %d\n",
-		       ch->rq_size + srpt_sq_size, ret);
+		       ch->rq_size + sport->port_attrib.srp_sq_size, ret);
 		goto out;
 	}
 
@@ -2160,7 +2156,7 @@ static int srpt_create_ch_ib(struct srpt_rdma_ch *ch)
 	qp_init->srq = sdev->srq;
 	qp_init->sq_sig_type = IB_SIGNAL_REQ_WR;
 	qp_init->qp_type = IB_QPT_RC;
-	qp_init->cap.max_send_wr = srpt_sq_size;
+	qp_init->cap.max_send_wr = sport->port_attrib.srp_sq_size;
 	qp_init->cap.max_send_sge = SRPT_DEF_SG_PER_WQE;
 
 	ch->qp = ib_create_qp(sdev->pd, qp_init);
@@ -3289,6 +3285,7 @@ static void srpt_add_one(struct ib_device *device)
 		sport->port = i;
 		sport->port_attrib.srp_max_rdma_size = DEFAULT_MAX_RDMA_SIZE;
 		sport->port_attrib.srp_max_rsp_size = DEFAULT_MAX_RSP_SIZE;
+		sport->port_attrib.srp_sq_size = DEF_SRPT_SQ_SIZE;
 		INIT_WORK(&sport->work, srpt_refresh_port_work);
 		INIT_LIST_HEAD(&sport->port_acl_list);
 		spin_lock_init(&sport->port_acl_lock);
@@ -3772,9 +3769,50 @@ static ssize_t srpt_tpg_attrib_store_srp_max_rsp_size(
 
 TF_TPG_ATTRIB_ATTR(srpt, srp_max_rsp_size, S_IRUGO | S_IWUSR);
 
+static ssize_t srpt_tpg_attrib_show_srp_sq_size(
+	struct se_portal_group *se_tpg,
+	char *page)
+{
+	struct srpt_port *sport = container_of(se_tpg, struct srpt_port, port_tpg_1);
+
+	return sprintf(page, "%u\n", sport->port_attrib.srp_sq_size);
+}
+
+static ssize_t srpt_tpg_attrib_store_srp_sq_size(
+	struct se_portal_group *se_tpg,
+	const char *page,
+	size_t count)
+{
+	struct srpt_port *sport = container_of(se_tpg, struct srpt_port, port_tpg_1);
+	unsigned long val;
+	int ret;
+
+	ret = strict_strtoul(page, 0, &val);
+	if (ret < 0) {
+		pr_err("strict_strtoul() failed with ret: %d\n", ret);
+		return -EINVAL;
+	}
+	if (val > MAX_SRPT_SRQ_SIZE) {
+		pr_err("val: %lu exceeds MAX_SRPT_SRQ_SIZE: %d\n", val,
+			MAX_SRPT_SRQ_SIZE);
+		return -EINVAL;
+	}
+	if (val < MIN_SRPT_SRQ_SIZE) {
+		pr_err("val: %lu smaller than MIN_SRPT_SRQ_SIZE: %d\n", val,
+			MIN_SRPT_SRQ_SIZE);
+		return -EINVAL;
+	}
+	sport->port_attrib.srp_sq_size = val;
+
+	return count;
+}
+
+TF_TPG_ATTRIB_ATTR(srpt, srp_sq_size, S_IRUGO | S_IWUSR);
+
 static struct configfs_attribute *srpt_tpg_attrib_attrs[] = {
 	&srpt_tpg_attrib_srp_max_rdma_size.attr,
 	&srpt_tpg_attrib_srp_max_rsp_size.attr,
+	&srpt_tpg_attrib_srp_sq_size.attr,
 	NULL,
 };
 
@@ -3981,13 +4019,6 @@ static int __init srpt_init_module(void)
 		goto out;
 	}
 
-	if (srpt_sq_size < MIN_SRPT_SQ_SIZE) {
-		printk(KERN_ERR "invalid value %d for kernel module parameter"
-		       " srpt_sq_size -- must be at least %d.\n", srpt_srq_size,
-		       MIN_SRPT_SQ_SIZE);
-		goto out;
-	}
-
 	spin_lock_init(&srpt_dev_lock);
 	INIT_LIST_HEAD(&srpt_dev_list);
 
diff --git a/drivers/infiniband/ulp/srpt/ib_srpt.h b/drivers/infiniband/ulp/srpt/ib_srpt.h
index 27b1027..1b607ae 100644
--- a/drivers/infiniband/ulp/srpt/ib_srpt.h
+++ b/drivers/infiniband/ulp/srpt/ib_srpt.h
@@ -331,10 +331,12 @@ struct srpt_rdma_ch {
  * struct srpt_port_attib - Attributes for SRPT port
  * @srp_max_rdma_size: Maximum size of SRP RDMA transfers for new connections.
  * @srp_max_rsp_size: Maximum size of SRP response messages in bytes.
+ * @srp_sq_size: Shared receive queue (SRQ) size.
  */
 struct srpt_port_attrib {
 	u32			srp_max_rdma_size;
 	u32			srp_max_rsp_size;
+	u32			srp_sq_size;
 };
 
 /**
-- 
1.7.2.5

Re: [PATCH 7/9] ib_srpt: Convert srp_max_rdma_size into per port configfs attribute

[Bart Van Assche]

On Mon, Oct 24, 2011 at 7:33 AM, Nicholas A. Bellinger
<nab-IzHhD5pYlfBP7FQvKIMDCQ@public.gmane.org> wrote:
> This patch converts the srp_max_rdma_size module parameter into a per
> endpoint configfs attribute.  This includes adding the necessary bits
> for show + store attributes w/ min/max bounds checking, and updating
> srpt_get_ioc() to accept a struct srpt_port parameter.
>
> [ ... ]
>
> -static void srpt_get_ioc(struct srpt_device *sdev, u32 slot,
> +static void srpt_get_ioc(struct srpt_port *sport, u32 slot,
>                         struct ib_dm_mad *mad)

The SRP spec says that there should be only one value for
iocp->rdma_size per I/O controller. This patch breaks that rule. I'm
not sure it's a good idea to introduce such behavior changes.

Bart.
--
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 6/9] ib_srpt: Remove legacy use_port_guid_in_session_name module parameter

[Bart Van Assche]

On Mon, Oct 24, 2011 at 7:33 AM, Nicholas A. Bellinger
<nab-IzHhD5pYlfBP7FQvKIMDCQ@public.gmane.org> wrote:
> This patch removes the legacy use_port_guid_in_session_name module parameter
> that is no longer required in modern ib_srpt code.

The patch looks fine to me but the description could be improved: the
"use_port_guid_in_session_name" had been introduced in the past to
work around a limitation of the Windows SRP initiator in multipath
setups. Apparently with recent Windows SRP initiator versions setting
that parameter is no longer necessary.

Bart.
--
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 7/9] ib_srpt: Convert srp_max_rdma_size into per port configfs attribute

[Nicholas A. Bellinger]

On Mon, 2011-10-24 at 18:34 +0200, Bart Van Assche wrote:
> On Mon, Oct 24, 2011 at 7:33 AM, Nicholas A. Bellinger
> <nab@linux-iscsi.org> wrote:
> > This patch converts the srp_max_rdma_size module parameter into a per
> > endpoint configfs attribute.  This includes adding the necessary bits
> > for show + store attributes w/ min/max bounds checking, and updating
> > srpt_get_ioc() to accept a struct srpt_port parameter.
> >
> > [ ... ]
> >
> > -static void srpt_get_ioc(struct srpt_device *sdev, u32 slot,
> > +static void srpt_get_ioc(struct srpt_port *sport, u32 slot,
> >                         struct ib_dm_mad *mad)
> 
> The SRP spec says that there should be only one value for
> iocp->rdma_size per I/O controller. This patch breaks that rule. I'm
> not sure it's a good idea to introduce such behavior changes.
> 

Hi Bart,

This is something that Roland asked me to persue, so i'll have to defer
to his judgment if he want's to kick this back out to a module parameter
or not.

--nab

Re: [PATCH 9/9] ib_srpt: Convert srpt_sq_size into per port configfs attribute

[Bart Van Assche]

On Mon, Oct 24, 2011 at 7:33 AM, Nicholas A. Bellinger
<nab-IzHhD5pYlfBP7FQvKIMDCQ@public.gmane.org> wrote:
> @@ -2144,11 +2140,11 @@ static int srpt_create_ch_ib(struct srpt_rdma_ch *ch)
>                goto out;
>
>        ch->cq = ib_create_cq(sdev->device, srpt_completion, NULL, ch,
> -                             ch->rq_size + srpt_sq_size, 0);
> +                             ch->rq_size + sport->port_attrib.srp_sq_size, 0);
>        if (IS_ERR(ch->cq)) {
>                ret = PTR_ERR(ch->cq);
>                printk(KERN_ERR "failed to create CQ cqe= %d ret= %d\n",
> -                      ch->rq_size + srpt_sq_size, ret);
> +                      ch->rq_size + sport->port_attrib.srp_sq_size, ret);
>                goto out;
>        }
>
> @@ -2160,7 +2156,7 @@ static int srpt_create_ch_ib(struct srpt_rdma_ch *ch)
>        qp_init->srq = sdev->srq;
>        qp_init->sq_sig_type = IB_SIGNAL_REQ_WR;
>        qp_init->qp_type = IB_QPT_RC;
> -       qp_init->cap.max_send_wr = srpt_sq_size;
> +       qp_init->cap.max_send_wr = sport->port_attrib.srp_sq_size;
>        qp_init->cap.max_send_sge = SRPT_DEF_SG_PER_WQE;

This patch introduces a race condition: if
sport->port_attrib.srp_sq_size is modified from user space while an
SRP login is ongoing then different values of that parameter can be
used in each of the above statements that read that parameter while
they all should use the same value.

Bart.
--
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 9/9] ib_srpt: Convert srpt_sq_size into per port configfs attribute

[Nicholas A. Bellinger]

On Mon, 2011-10-24 at 20:32 +0200, Bart Van Assche wrote:
> On Mon, Oct 24, 2011 at 7:33 AM, Nicholas A. Bellinger
> <nab@linux-iscsi.org> wrote:
> > @@ -2144,11 +2140,11 @@ static int srpt_create_ch_ib(struct srpt_rdma_ch *ch)
> >                goto out;
> >
> >        ch->cq = ib_create_cq(sdev->device, srpt_completion, NULL, ch,
> > -                             ch->rq_size + srpt_sq_size, 0);
> > +                             ch->rq_size + sport->port_attrib.srp_sq_size, 0);
> >        if (IS_ERR(ch->cq)) {
> >                ret = PTR_ERR(ch->cq);
> >                printk(KERN_ERR "failed to create CQ cqe= %d ret= %d\n",
> > -                      ch->rq_size + srpt_sq_size, ret);
> > +                      ch->rq_size + sport->port_attrib.srp_sq_size, ret);
> >                goto out;
> >        }
> >
> > @@ -2160,7 +2156,7 @@ static int srpt_create_ch_ib(struct srpt_rdma_ch *ch)
> >        qp_init->srq = sdev->srq;
> >        qp_init->sq_sig_type = IB_SIGNAL_REQ_WR;
> >        qp_init->qp_type = IB_QPT_RC;
> > -       qp_init->cap.max_send_wr = srpt_sq_size;
> > +       qp_init->cap.max_send_wr = sport->port_attrib.srp_sq_size;
> >        qp_init->cap.max_send_sge = SRPT_DEF_SG_PER_WQE;
> 
> This patch introduces a race condition: if
> sport->port_attrib.srp_sq_size is modified from user space while an
> SRP login is ongoing then different values of that parameter can be
> used in each of the above statements that read that parameter while
> they all should use the same value.
> 

Fair point.  Fixing this up now.

Also, please stop dropping relevant list CC'ed from my patches.

Thanks, 

--nab

Re: [PATCH 8/9] ib_srpt: Convert srp_max_rsp_size into per port configfs attribute

[Bart Van Assche]

On Mon, Oct 24, 2011 at 7:33 AM, Nicholas A. Bellinger
<nab-IzHhD5pYlfBP7FQvKIMDCQ@public.gmane.org> wrote:
> +static ssize_t srpt_tpg_attrib_store_srp_max_rsp_size(
> +       struct se_portal_group *se_tpg,
> +       const char *page,
> +       size_t count)
> +{
> +       struct srpt_port *sport = container_of(se_tpg, struct srpt_port, port_tpg_1);
> +       unsigned long val;
> +       int ret;
> +
> +       ret = strict_strtoul(page, 0, &val);

If the data "page" points at only consists of digits, the above
strict_strtoul() call will trigger a past-end-of-buffer read. Also,
isn't kstrto*() preferred over strict_strto*() ?

Bart.
--
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 8/9] ib_srpt: Convert srp_max_rsp_size into per port configfs attribute

[Nicholas A. Bellinger]

On Mon, 2011-10-24 at 21:44 +0200, Bart Van Assche wrote:
> On Mon, Oct 24, 2011 at 7:33 AM, Nicholas A. Bellinger
> <nab-IzHhD5pYlfBP7FQvKIMDCQ@public.gmane.org> wrote:
> > +static ssize_t srpt_tpg_attrib_store_srp_max_rsp_size(
> > +       struct se_portal_group *se_tpg,
> > +       const char *page,
> > +       size_t count)
> > +{
> > +       struct srpt_port *sport = container_of(se_tpg, struct srpt_port, port_tpg_1);
> > +       unsigned long val;
> > +       int ret;
> > +
> > +       ret = strict_strtoul(page, 0, &val);
> 
> If the data "page" points at only consists of digits, the above
> strict_strtoul() call will trigger a past-end-of-buffer read.

I don't understand what you mean here.  Can you provide a test case to
demonstrate please..?

> Also, isn't kstrto*() preferred over strict_strto*() ?
> 

Not AFAIK.

--nab


--
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 5/9] ib_srpt: Fix sport->port_guid formatting code

[Bart Van Assche]

On Mon, Oct 24, 2011 at 7:33 AM, Nicholas A. Bellinger
<nab-IzHhD5pYlfBP7FQvKIMDCQ@public.gmane.org> wrote:
> +               snprintf(sport->port_guid, sizeof(sport->port_guid),
> +                               "0x0000000000000000%016llx",
> +                               be64_to_cpu(sport->gid.global.interface_id));

If I interpret Roland's e-mail correctly Roland asked to use GID table
entry 0 (http://www.spinics.net/lists/linux-rdma/msg09751.html) ?

Bart.
--
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 8/9] ib_srpt: Convert srp_max_rsp_size into per port configfs attribute

[Bart Van Assche]

On Mon, Oct 24, 2011 at 9:49 PM, Nicholas A. Bellinger
<nab-IzHhD5pYlfBP7FQvKIMDCQ@public.gmane.org> wrote:
> On Mon, 2011-10-24 at 21:44 +0200, Bart Van Assche wrote:
>> On Mon, Oct 24, 2011 at 7:33 AM, Nicholas A. Bellinger
>> <nab-IzHhD5pYlfBP7FQvKIMDCQ@public.gmane.org> wrote:
>> > +static ssize_t srpt_tpg_attrib_store_srp_max_rsp_size(
>> > +       struct se_portal_group *se_tpg,
>> > +       const char *page,
>> > +       size_t count)
>> > +{
>> > +       struct srpt_port *sport = container_of(se_tpg, struct srpt_port, port_tpg_1);
>> > +       unsigned long val;
>> > +       int ret;
>> > +
>> > +       ret = strict_strtoul(page, 0, &val);
>>
>> If the data "page" points at only consists of digits, the above
>> strict_strtoul() call will trigger a past-end-of-buffer read.
>
> I don't understand what you mean here.  Can you provide a test case to
> demonstrate please..?

echo -n "345" >$configfs_path_of_parameter.

Bart.
--
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 8/9] ib_srpt: Convert srp_max_rsp_size into per port configfs attribute

[Nicholas A. Bellinger]

On Mon, 2011-10-24 at 21:58 +0200, Bart Van Assche wrote:
> On Mon, Oct 24, 2011 at 9:49 PM, Nicholas A. Bellinger
> <nab@linux-iscsi.org> wrote:
> > On Mon, 2011-10-24 at 21:44 +0200, Bart Van Assche wrote:
> >> On Mon, Oct 24, 2011 at 7:33 AM, Nicholas A. Bellinger
> >> <nab@linux-iscsi.org> wrote:
> >> > +static ssize_t srpt_tpg_attrib_store_srp_max_rsp_size(
> >> > +       struct se_portal_group *se_tpg,
> >> > +       const char *page,
> >> > +       size_t count)
> >> > +{
> >> > +       struct srpt_port *sport = container_of(se_tpg, struct srpt_port, port_tpg_1);
> >> > +       unsigned long val;
> >> > +       int ret;
> >> > +
> >> > +       ret = strict_strtoul(page, 0, &val);
> >>
> >> If the data "page" points at only consists of digits, the above
> >> strict_strtoul() call will trigger a past-end-of-buffer read.
> >
> > I don't understand what you mean here.  Can you provide a test case to
> > demonstrate please..?
> 
> echo -n "345" >$configfs_path_of_parameter.
> 

Still not sure what your getting at here..?

root@tifa:/sys/kernel/config/target/srpt/0x00000000000000000002c903000e8acd/tpgt_1/attrib# ls -la
total 0
drwxr-xr-x 2 root root    0 Oct 24 13:01 .
drwxr-xr-x 7 root root    0 Oct 24 12:58 ..
-rw-r--r-- 1 root root 4096 Oct 24 12:58 srp_max_rdma_size
-rw-r--r-- 1 root root 4096 Oct 24 13:01 srp_max_rsp_size
-rw-r--r-- 1 root root 4096 Oct 24 12:58 srp_sq_size

root@tifa:/sys/kernel/config/target/srpt/0x00000000000000000002c903000e8acd/tpgt_1/attrib# cat srp_max_rsp_size
256
root@tifa:/sys/kernel/config/target/srpt/0x00000000000000000002c903000e8acd/tpgt_1/attrib# echo -n "240" > srp_max_rsp_size
root@tifa:/sys/kernel/config/target/srpt/0x00000000000000000002c903000e8acd/tpgt_1/attrib# cat srp_max_rsp_size
240

Re: [PATCH 8/9] ib_srpt: Convert srp_max_rsp_size into per port configfs attribute

[Bart Van Assche]

On Mon, Oct 24, 2011 at 10:05 PM, Nicholas A. Bellinger
<nab-IzHhD5pYlfBP7FQvKIMDCQ@public.gmane.org> wrote:
> On Mon, 2011-10-24 at 21:58 +0200, Bart Van Assche wrote:
>> On Mon, Oct 24, 2011 at 9:49 PM, Nicholas A. Bellinger
>> <nab-IzHhD5pYlfBP7FQvKIMDCQ@public.gmane.org> wrote:
>> > On Mon, 2011-10-24 at 21:44 +0200, Bart Van Assche wrote:
>> >> On Mon, Oct 24, 2011 at 7:33 AM, Nicholas A. Bellinger
>> >> <nab-IzHhD5pYlfBP7FQvKIMDCQ@public.gmane.org> wrote:
>> >> > +static ssize_t srpt_tpg_attrib_store_srp_max_rsp_size(
>> >> > +       struct se_portal_group *se_tpg,
>> >> > +       const char *page,
>> >> > +       size_t count)
>> >> > +{
>> >> > +       struct srpt_port *sport = container_of(se_tpg, struct srpt_port, port_tpg_1);
>> >> > +       unsigned long val;
>> >> > +       int ret;
>> >> > +
>> >> > +       ret = strict_strtoul(page, 0, &val);
>> >>
>> >> If the data "page" points at only consists of digits, the above
>> >> strict_strtoul() call will trigger a past-end-of-buffer read.
>> >
>> > I don't understand what you mean here.  Can you provide a test case to
>> > demonstrate please..?
>>
>> echo -n "345" >$configfs_path_of_parameter.
>
> Still not sure what your getting at here..?

Only the data in page[0..count-1] is guaranteed to be initialized.
strict_strtoul() will read until it either finds whitespace or a
binary zero, so if the data in page[] does neither contain whitespace
nor a binary zero then strict_strtoul() will read past the end of the
data in page[]. There may be any data at page[count], including a
valid digit.

Bart.
--
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 8/9] ib_srpt: Convert srp_max_rsp_size into per port configfs attribute

[Bart Van Assche]

On Mon, Oct 24, 2011 at 9:49 PM, Nicholas A. Bellinger
<nab-IzHhD5pYlfBP7FQvKIMDCQ@public.gmane.org> wrote:
> On Mon, 2011-10-24 at 21:44 +0200, Bart Van Assche wrote:
>> Also, isn't kstrto*() preferred over strict_strto*() ?
>
> Not AFAIK.

This is what I found in the description of commit 33ee3b2:

    kstrto*: converting strings to integers done (hopefully) right

    1. simple_strto*() do not contain overflow checks and crufty,
       libc way to indicate failure.
    2. strict_strto*() also do not have overflow checks but the name and
       comments pretend they do.
    3. Both families have only "long long" and "long" variants,
       but users want strtou8()
    4. Both "simple" and "strict" prefixes are wrong:
       Simple doesn't exactly say what's so simple, strict should not exist
       because conversion should be strict by default.

    The solution is to use "k" prefix and add convertors for more types.
    Enter
        kstrtoull()
        ...

Bart.
--
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 8/9] ib_srpt: Convert srp_max_rsp_size into per port configfs attribute

[Nicholas A. Bellinger]

On Mon, 2011-10-24 at 22:11 +0200, Bart Van Assche wrote:
> On Mon, Oct 24, 2011 at 10:05 PM, Nicholas A. Bellinger
> <nab@linux-iscsi.org> wrote:
> > On Mon, 2011-10-24 at 21:58 +0200, Bart Van Assche wrote:
> >> On Mon, Oct 24, 2011 at 9:49 PM, Nicholas A. Bellinger
> >> <nab@linux-iscsi.org> wrote:
> >> > On Mon, 2011-10-24 at 21:44 +0200, Bart Van Assche wrote:
> >> >> On Mon, Oct 24, 2011 at 7:33 AM, Nicholas A. Bellinger
> >> >> <nab@linux-iscsi.org> wrote:
> >> >> > +static ssize_t srpt_tpg_attrib_store_srp_max_rsp_size(
> >> >> > +       struct se_portal_group *se_tpg,
> >> >> > +       const char *page,
> >> >> > +       size_t count)
> >> >> > +{
> >> >> > +       struct srpt_port *sport = container_of(se_tpg, struct srpt_port, port_tpg_1);
> >> >> > +       unsigned long val;
> >> >> > +       int ret;
> >> >> > +
> >> >> > +       ret = strict_strtoul(page, 0, &val);
> >> >>
> >> >> If the data "page" points at only consists of digits, the above
> >> >> strict_strtoul() call will trigger a past-end-of-buffer read.
> >> >
> >> > I don't understand what you mean here.  Can you provide a test case to
> >> > demonstrate please..?
> >>
> >> echo -n "345" >$configfs_path_of_parameter.
> >
> > Still not sure what your getting at here..?
> 
> Only the data in page[0..count-1] is guaranteed to be initialized.
> strict_strtoul() will read until it either finds whitespace or a
> binary zero, so if the data in page[] does neither contain whitespace
> nor a binary zero then strict_strtoul() will read past the end of the
> data in page[]. There may be any data at page[count], including a
> valid digit.
> 

That is part obvious.  The point your missing is that configfs is
already sanitizing the the incoming buffer in fs/configfs/file.c to work
with strict_strtoul() here:

static int
fill_write_buffer(struct configfs_buffer * buffer, const char __user * buf, size_t count)
{
        int error;

        if (!buffer->page)
                buffer->page = (char *)__get_free_pages(GFP_KERNEL, 0);
        if (!buffer->page)
                return -ENOMEM;

        if (count >= SIMPLE_ATTR_SIZE)
                count = SIMPLE_ATTR_SIZE - 1;
        error = copy_from_user(buffer->page,buf,count);
        buffer->needs_read_fill = 1;
        /* if buf is assumed to contain a string, terminate it by \0,
         * so e.g. sscanf() can scan the string easily */
        buffer->page[count] = 0;
        return error ? -EFAULT : count;
}

Re: [PATCH 8/9] ib_srpt: Convert srp_max_rsp_size into per port configfs attribute

[Nicholas A. Bellinger]

On Mon, 2011-10-24 at 22:16 +0200, Bart Van Assche wrote:
> On Mon, Oct 24, 2011 at 9:49 PM, Nicholas A. Bellinger
> <nab-IzHhD5pYlfBP7FQvKIMDCQ@public.gmane.org> wrote:
> > On Mon, 2011-10-24 at 21:44 +0200, Bart Van Assche wrote:
> >> Also, isn't kstrto*() preferred over strict_strto*() ?
> >
> > Not AFAIK.
> 
> This is what I found in the description of commit 33ee3b2:
> 
>     kstrto*: converting strings to integers done (hopefully) right
> 
>     1. simple_strto*() do not contain overflow checks and crufty,
>        libc way to indicate failure.
>     2. strict_strto*() also do not have overflow checks but the name and
>        comments pretend they do.
>     3. Both families have only "long long" and "long" variants,
>        but users want strtou8()
>     4. Both "simple" and "strict" prefixes are wrong:
>        Simple doesn't exactly say what's so simple, strict should not exist
>        because conversion should be strict by default.
> 
>     The solution is to use "k" prefix and add convertors for more types.
>     Enter
>         kstrtoull()
>         ...

Fair enough.  I'll convert this separately after the merge considering
it's a very minor improvement, and not a bugfix.

--nab



--
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 5/9] ib_srpt: Fix sport->port_guid formatting code

[Nicholas A. Bellinger]

On Mon, 2011-10-24 at 21:57 +0200, Bart Van Assche wrote:
> On Mon, Oct 24, 2011 at 7:33 AM, Nicholas A. Bellinger
> <nab@linux-iscsi.org> wrote:
> > +               snprintf(sport->port_guid, sizeof(sport->port_guid),
> > +                               "0x0000000000000000%016llx",
> > +                               be64_to_cpu(sport->gid.global.interface_id));
> 
> If I interpret Roland's e-mail correctly Roland asked to use GID table
> entry 0 (http://www.spinics.net/lists/linux-rdma/msg09751.html) ?
> 

I'll defer to Roland on this, as the port->gid.global.interface_id you
recommended does exactly what we need.  I'm happy to test another code
snippet if you have one handy.

--nab

Re: [PATCH 7/9] ib_srpt: Convert srp_max_rdma_size into per port configfs attribute

[Nicholas A. Bellinger]

On Mon, 2011-10-24 at 05:33 +0000, Nicholas A. Bellinger wrote:
> From: Nicholas Bellinger <nab@linux-iscsi.org>
> 
> This patch converts the srp_max_rdma_size module parameter into a per
> endpoint configfs attribute.  This includes adding the necessary bits
> for show + store attributes w/ min/max bounds checking, and updating
> srpt_get_ioc() to accept a struct srpt_port parameter.
> 
> Reported-by: Roland Dreier <roland@purestorage.com>
> Cc: Roland Dreier <roland@purestorage.com>
> Cc: Bart Van Assche <bvanassche@acm.org>
> Signed-off-by: Nicholas A. Bellinger <nab@linux-iscsi.org>
> ---
>  drivers/infiniband/ulp/srpt/ib_srpt.c |   60 ++++++++++++++++++++++++++++-----
>  drivers/infiniband/ulp/srpt/ib_srpt.h |   10 +++++
>  2 files changed, 61 insertions(+), 9 deletions(-)
> 

<SNIP>

> diff --git a/drivers/infiniband/ulp/srpt/ib_srpt.h b/drivers/infiniband/ulp/srpt/ib_srpt.h
> index 59ee2d7..c9caa90 100644
> --- a/drivers/infiniband/ulp/srpt/ib_srpt.h
> +++ b/drivers/infiniband/ulp/srpt/ib_srpt.h
> @@ -114,6 +114,7 @@ enum {
>  	MIN_SRPT_SRQ_SIZE = 4,
>  	DEFAULT_SRPT_SRQ_SIZE = 4095,
>  	MAX_SRPT_SRQ_SIZE = 65535,
> +	MAX_SRPT_RDMA_SIZE = 256U,
>  

Hey btw, what should the proper MAX_SRPT_RDMA_SIZE be here..?

--nab

Re: [PATCH 7/9] ib_srpt: Convert srp_max_rdma_size into per port configfs attribute

[Nicholas A. Bellinger]

On Mon, 2011-10-24 at 13:29 -0700, Nicholas A. Bellinger wrote:
> On Mon, 2011-10-24 at 05:33 +0000, Nicholas A. Bellinger wrote:
> > From: Nicholas Bellinger <nab-IzHhD5pYlfBP7FQvKIMDCQ@public.gmane.org>
> > 
> > This patch converts the srp_max_rdma_size module parameter into a per
> > endpoint configfs attribute.  This includes adding the necessary bits
> > for show + store attributes w/ min/max bounds checking, and updating
> > srpt_get_ioc() to accept a struct srpt_port parameter.
> > 
> > Reported-by: Roland Dreier <roland-BHEL68pLQRGGvPXPguhicg@public.gmane.org>
> > Cc: Roland Dreier <roland-BHEL68pLQRGGvPXPguhicg@public.gmane.org>
> > Cc: Bart Van Assche <bvanassche-HInyCGIudOg@public.gmane.org>
> > Signed-off-by: Nicholas A. Bellinger <nab-IzHhD5pYlfBP7FQvKIMDCQ@public.gmane.org>
> > ---
> >  drivers/infiniband/ulp/srpt/ib_srpt.c |   60 ++++++++++++++++++++++++++++-----
> >  drivers/infiniband/ulp/srpt/ib_srpt.h |   10 +++++
> >  2 files changed, 61 insertions(+), 9 deletions(-)
> > 
> 
> <SNIP>
> 
> > diff --git a/drivers/infiniband/ulp/srpt/ib_srpt.h b/drivers/infiniband/ulp/srpt/ib_srpt.h
> > index 59ee2d7..c9caa90 100644
> > --- a/drivers/infiniband/ulp/srpt/ib_srpt.h
> > +++ b/drivers/infiniband/ulp/srpt/ib_srpt.h
> > @@ -114,6 +114,7 @@ enum {
> >  	MIN_SRPT_SRQ_SIZE = 4,
> >  	DEFAULT_SRPT_SRQ_SIZE = 4095,
> >  	MAX_SRPT_SRQ_SIZE = 65535,
> > +	MAX_SRPT_RDMA_SIZE = 256U,
> >  
> 
> Hey btw, what should the proper MAX_SRPT_RDMA_SIZE be here..?
> 

Ping on this Bart..?

Thanks,

--nab

--
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 7/9] ib_srpt: Convert srp_max_rdma_size into per port configfs attribute

[Bart Van Assche]

On Mon, Oct 24, 2011 at 10:29 PM, Nicholas A. Bellinger
<nab-IzHhD5pYlfBP7FQvKIMDCQ@public.gmane.org> wrote:
>> @@ -114,6 +114,7 @@ enum {
>>       MIN_SRPT_SRQ_SIZE = 4,
>>       DEFAULT_SRPT_SRQ_SIZE = 4095,
>>       MAX_SRPT_SRQ_SIZE = 65535,
>> +     MAX_SRPT_RDMA_SIZE = 256U,
>>
>
> Hey btw, what should the proper MAX_SRPT_RDMA_SIZE be here..?

As you can see in the ib_srpt source code, the largest value filled in
for this parameter in the IOC profile is 1U << 24.

Bart.
--
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 7/9] ib_srpt: Convert srp_max_rdma_size into per port configfs attribute

[Nicholas A. Bellinger]

On Tue, 2011-10-25 at 12:32 +0200, Bart Van Assche wrote:
> On Mon, Oct 24, 2011 at 10:29 PM, Nicholas A. Bellinger
> <nab-IzHhD5pYlfBP7FQvKIMDCQ@public.gmane.org> wrote:
> >> @@ -114,6 +114,7 @@ enum {
> >>       MIN_SRPT_SRQ_SIZE = 4,
> >>       DEFAULT_SRPT_SRQ_SIZE = 4095,
> >>       MAX_SRPT_SRQ_SIZE = 65535,
> >> +     MAX_SRPT_RDMA_SIZE = 256U,
> >>
> >
> > Hey btw, what should the proper MAX_SRPT_RDMA_SIZE be here..?
> 
> As you can see in the ib_srpt source code, the largest value filled in
> for this parameter in the IOC profile is 1U << 24.

Thanks Bart.  Fixing this up now.

--nab


--
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 5/9] ib_srpt: Fix sport->port_guid formatting code

[Bart Van Assche]

On Mon, Oct 24, 2011 at 10:25 PM, Nicholas A. Bellinger
<nab-IzHhD5pYlfBP7FQvKIMDCQ@public.gmane.org> wrote:
> On Mon, 2011-10-24 at 21:57 +0200, Bart Van Assche wrote:
>> On Mon, Oct 24, 2011 at 7:33 AM, Nicholas A. Bellinger
>> <nab-IzHhD5pYlfBP7FQvKIMDCQ@public.gmane.org> wrote:
>> > +               snprintf(sport->port_guid, sizeof(sport->port_guid),
>> > +                               "0x0000000000000000%016llx",
>> > +                               be64_to_cpu(sport->gid.global.interface_id));
>>
>> If I interpret Roland's e-mail correctly Roland asked to use GID table
>> entry 0 (http://www.spinics.net/lists/linux-rdma/msg09751.html) ?
>>
>
> I'll defer to Roland on this, as the port->gid.global.interface_id you
> recommended does exactly what we need.  I'm happy to test another code
> snippet if you have one handy.

Something like this should generate GID 0 in ASCII format (not tested
in any way, and using separators will help to make the output easier
to read):

snprintf(sport->port_gid, sizeof(sport->port_gid),
            "0x%016llx%016llx",
             be64_to_cpu(sport->gid.global.subnet_prefix),
             be64_to_cpu(sport->gid.global.interface_id));

Bart.
--
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 5/9] ib_srpt: Fix sport->port_guid formatting code

[Nicholas A. Bellinger]

On Wed, 2011-10-26 at 20:23 +0200, Bart Van Assche wrote:
> On Mon, Oct 24, 2011 at 10:25 PM, Nicholas A. Bellinger
> <nab-IzHhD5pYlfBP7FQvKIMDCQ@public.gmane.org> wrote:
> > On Mon, 2011-10-24 at 21:57 +0200, Bart Van Assche wrote:
> >> On Mon, Oct 24, 2011 at 7:33 AM, Nicholas A. Bellinger
> >> <nab-IzHhD5pYlfBP7FQvKIMDCQ@public.gmane.org> wrote:
> >> > +               snprintf(sport->port_guid, sizeof(sport->port_guid),
> >> > +                               "0x0000000000000000%016llx",
> >> > +                               be64_to_cpu(sport->gid.global.interface_id));
> >>
> >> If I interpret Roland's e-mail correctly Roland asked to use GID table
> >> entry 0 (http://www.spinics.net/lists/linux-rdma/msg09751.html) ?
> >>
> >
> > I'll defer to Roland on this, as the port->gid.global.interface_id you
> > recommended does exactly what we need.  I'm happy to test another code
> > snippet if you have one handy.
> 
> Something like this should generate GID 0 in ASCII format (not tested
> in any way, and using separators will help to make the output easier
> to read):
> 
> snprintf(sport->port_gid, sizeof(sport->port_gid),
>             "0x%016llx%016llx",
>              be64_to_cpu(sport->gid.global.subnet_prefix),
>              be64_to_cpu(sport->gid.global.interface_id));
> 

So adding the subnet_prefix here works as expected, and as previously
mentioned I'm OK to change the /var/target/fabric/ib_srpt.spec
formatting in rtslib for SRP target ports to accommodate this..

But the bit that now looks kinda odd with this change is that NodeACLs
would still not be using the subnet_prefix.  Here's a bit of targetcli
output to demonstrate:

/ib_srpt> info
Fabric module name: ib_srpt
ConfigFS path: /sys/kernel/config/target/srpt
Allowed WWNs list (free type): 0xfe800000000000000002c903000e8ace, 0xfe800000000000000002c903000e8acd
Fabric module specfile: /var/target/fabric/ib_srpt.spec
Fabric module features: acls
Corresponding kernel module: ib_srpt
/ib_srpt> ls
o- ib_srpt ......................................................... [2 Targets]
  o- 0xfe800000000000000002c903000e8acd .............................. [enabled]
  | o- acls ............................................................ [1 ACL]
  | | o- 0x00000000000000000002c903000e8be9 .................... [1 Mapped LUNs]
  | |   o- mapped_lun0 ............................................. [lun0 (rw)]
  | o- luns ........................................................... [1 LUNs]
  |   o- lun0 ................................... [iblock/scsi_debug (/dev/sdb)]
  o- 0xfe800000000000000002c903000e8ace .............................. [enabled]
    o- acls ............................................................ [1 ACL]
    | o- 0x00000000000000000002c903000e8bea .................... [1 Mapped LUNs]
    |   o- mapped_lun0 ............................................. [lun0 (rw)]
    o- luns ........................................................... [1 LUNs]
      o- lun0 .................................. [iblock/scsi_debug2 (/dev/sdc)]
/ib_srpt> 

So I don't see an easy way to get subnet_prefix for the incoming
initiator port in srpt_cm_req_recv(), as what's being passed in from
srp_login_req->initiator_port_id seems to have subnet_prefix already
stripped off..?

So the two question here for Roland and yourself are:  Is using a real
subnet prefix for target ports, but not the explict NodeACLs for
initiator ports acceptable for an initial merge..?  If not, how is it
possible to obtain the remote initiator port's subnet_prefix during
login in srpt_cm_req_recv() so we can keep subnet_prefix usage
consistent between target ports and explict NodeACL initiator ports
in /sys/kernel/config/target/srpt/

Thank you,
.
--nab


--
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