linux-rdma.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2 00/14] IB/srpt: Add RDMA/CM support
@ 2018-01-17  0:14 Bart Van Assche
       [not found] ` <20180117001418.7852-1-bart.vanassche-Sjgp3cTcYWE@public.gmane.org>
  0 siblings, 1 reply; 24+ messages in thread
From: Bart Van Assche @ 2018-01-17  0:14 UTC (permalink / raw)
  To: Jason Gunthorpe
  Cc: Doug Ledford, linux-rdma-u79uwXL29TY76Z2rM5mHXA, Bart Van Assche

Hello Jason and Doug,

This patch series not only adds RDMA/CM support to the SRP target driver but
also fixes a number of race conditions in that driver.

The RDMA/CM listener port number has to be specified as an ib_srpt kernel
module parameter. The default value for that parameter is zero which means
that RDMA/CM support is disabled.

Note: since this patch series uses the srp_login_req_rdma structure that was
introduced by the IB/srp RDMA/CM patch series, this series depends on the
IB/srp RDMA/CM patch series.

This patch series, just like v4 of the IB/srp RDMA/CM patch series, passes
Laurence Oberman's tests.

Please consider this patch series for inclusion in the upstream kernel.

Thanks,

Bart.

Changes compared to v1:
- Added patch "Fix a race condition related to wait list processing".
- Fixed the size of the character arrays used to store the initiator port ID
  and session name. This fixes a login failure that was reported by Laurence
  Oberman.

Bart Van Assche (14):
  IB/srpt: Make it safe to use RCU for srpt_device.rch_list
  IB/srpt: Rework srpt_disconnect_ch_sync()
  IB/srpt: Add P_Key support
  IB/srpt: One target per port
  IB/srpt: Use the source GID as session name
  IB/srpt: Rework multi-channel support
  IB/srpt: Simplify srpt_close_session()
  IB/srpt: Log all zero-length writes and completions
  IB/srpt: Fix login-related race conditions
  IB/srpt: Fix a race condition related to wait list processing
  IB/srpt: Avoid that wait list processing triggers command reordering
  IB/srpt: Prepare RDMA/CM support
  IB/srpt: Move the code for parsing struct ib_cm_req_event_param
  IB/srpt: Add RDMA/CM support

 drivers/infiniband/ulp/srpt/ib_srpt.c | 953 +++++++++++++++++++++++-----------
 drivers/infiniband/ulp/srpt/ib_srpt.h |  66 ++-
 2 files changed, 706 insertions(+), 313 deletions(-)

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

^ permalink raw reply	[flat|nested] 24+ messages in thread

* [PATCH v2 01/14] IB/srpt: Make it safe to use RCU for srpt_device.rch_list
       [not found] ` <20180117001418.7852-1-bart.vanassche-Sjgp3cTcYWE@public.gmane.org>
@ 2018-01-17  0:14   ` Bart Van Assche
  2018-01-17  0:14   ` [PATCH v2 02/14] IB/srpt: Rework srpt_disconnect_ch_sync() Bart Van Assche
                     ` (14 subsequent siblings)
  15 siblings, 0 replies; 24+ messages in thread
From: Bart Van Assche @ 2018-01-17  0:14 UTC (permalink / raw)
  To: Jason Gunthorpe
  Cc: Doug Ledford, linux-rdma-u79uwXL29TY76Z2rM5mHXA, Bart Van Assche

The next patch will iterate over rch_list from a context from which
it is not allowed to block. Hence make rch_list RCU-safe.

Signed-off-by: Bart Van Assche <bart.vanassche-Sjgp3cTcYWE@public.gmane.org>
---
 drivers/infiniband/ulp/srpt/ib_srpt.c | 13 ++++++++++---
 drivers/infiniband/ulp/srpt/ib_srpt.h |  2 ++
 2 files changed, 12 insertions(+), 3 deletions(-)

diff --git a/drivers/infiniband/ulp/srpt/ib_srpt.c b/drivers/infiniband/ulp/srpt/ib_srpt.c
index d78f60dcc2ba..4dd15378bc7c 100644
--- a/drivers/infiniband/ulp/srpt/ib_srpt.c
+++ b/drivers/infiniband/ulp/srpt/ib_srpt.c
@@ -1906,7 +1906,7 @@ static void srpt_free_ch(struct kref *kref)
 {
 	struct srpt_rdma_ch *ch = container_of(kref, struct srpt_rdma_ch, kref);
 
-	kfree(ch);
+	kfree_rcu(ch, rcu);
 }
 
 static void srpt_release_channel_work(struct work_struct *w)
@@ -1945,11 +1945,17 @@ static void srpt_release_channel_work(struct work_struct *w)
 			     srp_max_req_size, DMA_FROM_DEVICE);
 
 	mutex_lock(&sdev->mutex);
-	list_del_init(&ch->list);
+	list_del_rcu(&ch->list);
 	if (ch->release_done)
 		complete(ch->release_done);
 	mutex_unlock(&sdev->mutex);
 
+	synchronize_rcu();
+
+	mutex_lock(&sdev->mutex);
+	INIT_LIST_HEAD(&ch->list);
+	mutex_unlock(&sdev->mutex);
+
 	wake_up(&sdev->ch_releaseQ);
 
 	kref_put(&ch->kref, srpt_free_ch);
@@ -2064,6 +2070,7 @@ static int srpt_cm_req_recv(struct ib_cm_id *cm_id,
 		goto reject;
 	}
 
+	init_rcu_head(&ch->rcu);
 	kref_init(&ch->kref);
 	ch->zw_cqe.done = srpt_zerolength_write_done;
 	INIT_WORK(&ch->release_work, srpt_release_channel_work);
@@ -2190,7 +2197,7 @@ static int srpt_cm_req_recv(struct ib_cm_id *cm_id,
 	}
 
 	mutex_lock(&sdev->mutex);
-	list_add_tail(&ch->list, &sdev->rch_list);
+	list_add_tail_rcu(&ch->list, &sdev->rch_list);
 	mutex_unlock(&sdev->mutex);
 
 	goto out;
diff --git a/drivers/infiniband/ulp/srpt/ib_srpt.h b/drivers/infiniband/ulp/srpt/ib_srpt.h
index 11ce8c94a051..0ab59c60f2ef 100644
--- a/drivers/infiniband/ulp/srpt/ib_srpt.h
+++ b/drivers/infiniband/ulp/srpt/ib_srpt.h
@@ -244,6 +244,7 @@ enum rdma_ch_state {
  * @qp:            IB queue pair used for communicating over this channel.
  * @cq:            IB completion queue for this channel.
  * @zw_cqe:	   Zero-length write CQE.
+ * @rcu:           RCU head.
  * @kref:	   kref for this channel.
  * @rq_size:       IB receive queue size.
  * @max_rsp_size:  Maximum size of an RSP response message in bytes.
@@ -276,6 +277,7 @@ struct srpt_rdma_ch {
 	struct ib_qp		*qp;
 	struct ib_cq		*cq;
 	struct ib_cqe		zw_cqe;
+	struct rcu_head		rcu;
 	struct kref		kref;
 	int			rq_size;
 	u32			max_rsp_size;
-- 
2.15.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

^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [PATCH v2 02/14] IB/srpt: Rework srpt_disconnect_ch_sync()
       [not found] ` <20180117001418.7852-1-bart.vanassche-Sjgp3cTcYWE@public.gmane.org>
  2018-01-17  0:14   ` [PATCH v2 01/14] IB/srpt: Make it safe to use RCU for srpt_device.rch_list Bart Van Assche
@ 2018-01-17  0:14   ` Bart Van Assche
  2018-01-17  0:14   ` [PATCH v2 03/14] IB/srpt: Add P_Key support Bart Van Assche
                     ` (13 subsequent siblings)
  15 siblings, 0 replies; 24+ messages in thread
From: Bart Van Assche @ 2018-01-17  0:14 UTC (permalink / raw)
  To: Jason Gunthorpe
  Cc: Doug Ledford, linux-rdma-u79uwXL29TY76Z2rM5mHXA, Bart Van Assche

This patch fixes a use-after-free issue for ch->release_done when
running the SRP protocol on top of the rdma_rxe driver.

Signed-off-by: Bart Van Assche <bart.vanassche-Sjgp3cTcYWE@public.gmane.org>
---
 drivers/infiniband/ulp/srpt/ib_srpt.c | 45 ++++++++++++++++++-----------------
 drivers/infiniband/ulp/srpt/ib_srpt.h |  2 --
 2 files changed, 23 insertions(+), 24 deletions(-)

diff --git a/drivers/infiniband/ulp/srpt/ib_srpt.c b/drivers/infiniband/ulp/srpt/ib_srpt.c
index 4dd15378bc7c..5386b993daf9 100644
--- a/drivers/infiniband/ulp/srpt/ib_srpt.c
+++ b/drivers/infiniband/ulp/srpt/ib_srpt.c
@@ -1841,6 +1841,23 @@ static int srpt_disconnect_ch(struct srpt_rdma_ch *ch)
 	return ret;
 }
 
+static bool srpt_ch_closed(struct srpt_device *sdev, struct srpt_rdma_ch *ch)
+{
+	struct srpt_rdma_ch *ch2;
+	bool res = true;
+
+	rcu_read_lock();
+	list_for_each_entry(ch2, &sdev->rch_list, list) {
+		if (ch2 == ch) {
+			res = false;
+			break;
+		}
+	}
+	rcu_read_unlock();
+
+	return res;
+}
+
 /*
  * Send DREQ and wait for DREP. Return true if and only if this function
  * changed the state of @ch.
@@ -1848,31 +1865,24 @@ static int srpt_disconnect_ch(struct srpt_rdma_ch *ch)
 static bool srpt_disconnect_ch_sync(struct srpt_rdma_ch *ch)
 	__must_hold(&sdev->mutex)
 {
-	DECLARE_COMPLETION_ONSTACK(release_done);
 	struct srpt_device *sdev = ch->sport->sdev;
-	bool wait;
+	int ret;
 
 	lockdep_assert_held(&sdev->mutex);
 
 	pr_debug("ch %s-%d state %d\n", ch->sess_name, ch->qp->qp_num,
 		 ch->state);
 
-	WARN_ON(ch->release_done);
-	ch->release_done = &release_done;
-	wait = !list_empty(&ch->list);
-	srpt_disconnect_ch(ch);
+	ret = srpt_disconnect_ch(ch);
 	mutex_unlock(&sdev->mutex);
 
-	if (!wait)
-		goto out;
-
-	while (wait_for_completion_timeout(&release_done, 180 * HZ) == 0)
+	while (wait_event_timeout(sdev->ch_releaseQ, srpt_ch_closed(sdev, ch),
+				  5 * HZ) == 0)
 		pr_info("%s(%s-%d state %d): still waiting ...\n", __func__,
 			ch->sess_name, ch->qp->qp_num, ch->state);
 
-out:
 	mutex_lock(&sdev->mutex);
-	return wait;
+	return ret == 0;
 }
 
 static void srpt_set_enabled(struct srpt_port *sport, bool enabled)
@@ -1916,8 +1926,7 @@ static void srpt_release_channel_work(struct work_struct *w)
 	struct se_session *se_sess;
 
 	ch = container_of(w, struct srpt_rdma_ch, release_work);
-	pr_debug("%s: %s-%d; release_done = %p\n", __func__, ch->sess_name,
-		 ch->qp->qp_num, ch->release_done);
+	pr_debug("%s-%d\n", ch->sess_name, ch->qp->qp_num);
 
 	sdev = ch->sport->sdev;
 	BUG_ON(!sdev);
@@ -1946,14 +1955,6 @@ static void srpt_release_channel_work(struct work_struct *w)
 
 	mutex_lock(&sdev->mutex);
 	list_del_rcu(&ch->list);
-	if (ch->release_done)
-		complete(ch->release_done);
-	mutex_unlock(&sdev->mutex);
-
-	synchronize_rcu();
-
-	mutex_lock(&sdev->mutex);
-	INIT_LIST_HEAD(&ch->list);
 	mutex_unlock(&sdev->mutex);
 
 	wake_up(&sdev->ch_releaseQ);
diff --git a/drivers/infiniband/ulp/srpt/ib_srpt.h b/drivers/infiniband/ulp/srpt/ib_srpt.h
index 0ab59c60f2ef..67248338b4c9 100644
--- a/drivers/infiniband/ulp/srpt/ib_srpt.h
+++ b/drivers/infiniband/ulp/srpt/ib_srpt.h
@@ -270,7 +270,6 @@ enum rdma_ch_state {
  * @sess_name:     Session name.
  * @ini_guid:      Initiator port GUID.
  * @release_work:  Allows scheduling of srpt_release_channel().
- * @release_done:  Enables waiting for srpt_release_channel() completion.
  */
 struct srpt_rdma_ch {
 	struct ib_cm_id		*cm_id;
@@ -299,7 +298,6 @@ struct srpt_rdma_ch {
 	u8			sess_name[36];
 	u8			ini_guid[24];
 	struct work_struct	release_work;
-	struct completion	*release_done;
 };
 
 /**
-- 
2.15.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

^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [PATCH v2 03/14] IB/srpt: Add P_Key support
       [not found] ` <20180117001418.7852-1-bart.vanassche-Sjgp3cTcYWE@public.gmane.org>
  2018-01-17  0:14   ` [PATCH v2 01/14] IB/srpt: Make it safe to use RCU for srpt_device.rch_list Bart Van Assche
  2018-01-17  0:14   ` [PATCH v2 02/14] IB/srpt: Rework srpt_disconnect_ch_sync() Bart Van Assche
@ 2018-01-17  0:14   ` Bart Van Assche
  2018-01-17  0:14   ` [PATCH v2 04/14] IB/srpt: One target per port Bart Van Assche
                     ` (12 subsequent siblings)
  15 siblings, 0 replies; 24+ messages in thread
From: Bart Van Assche @ 2018-01-17  0:14 UTC (permalink / raw)
  To: Jason Gunthorpe
  Cc: Doug Ledford, linux-rdma-u79uwXL29TY76Z2rM5mHXA, Bart Van Assche

Process connection requests that use another P_Key than the default
correctly.

Signed-off-by: Bart Van Assche <bart.vanassche-Sjgp3cTcYWE@public.gmane.org>
---
 drivers/infiniband/ulp/srpt/ib_srpt.c | 14 +++++++++++---
 drivers/infiniband/ulp/srpt/ib_srpt.h |  2 ++
 2 files changed, 13 insertions(+), 3 deletions(-)

diff --git a/drivers/infiniband/ulp/srpt/ib_srpt.c b/drivers/infiniband/ulp/srpt/ib_srpt.c
index 5386b993daf9..cafa73083ee8 100644
--- a/drivers/infiniband/ulp/srpt/ib_srpt.c
+++ b/drivers/infiniband/ulp/srpt/ib_srpt.c
@@ -41,6 +41,7 @@
 #include <linux/string.h>
 #include <linux/delay.h>
 #include <linux/atomic.h>
+#include <rdma/ib_cache.h>
 #include <scsi/scsi_proto.h>
 #include <scsi/scsi_tcq.h>
 #include <target/target_core_base.h>
@@ -1057,7 +1058,12 @@ static int srpt_init_ch_qp(struct srpt_rdma_ch *ch, struct ib_qp *qp)
 	attr->qp_state = IB_QPS_INIT;
 	attr->qp_access_flags = IB_ACCESS_LOCAL_WRITE;
 	attr->port_num = ch->sport->port;
-	attr->pkey_index = 0;
+
+	ret = ib_find_cached_pkey(ch->sport->sdev->device, ch->sport->port,
+				  ch->pkey, &attr->pkey_index);
+	if (ret < 0)
+		pr_err("Translating pkey %#x failed (%d) - using index 0\n",
+		       ch->pkey, ret);
 
 	ret = ib_modify_qp(qp, attr,
 			   IB_QP_STATE | IB_QP_ACCESS_FLAGS | IB_QP_PORT |
@@ -1994,9 +2000,10 @@ static int srpt_cm_req_recv(struct ib_cm_id *cm_id,
 
 	it_iu_len = be32_to_cpu(req->req_it_iu_len);
 
-	pr_info("Received SRP_LOGIN_REQ with i_port_id %pI6, t_port_id %pI6 and it_iu_len %d on port %d (guid=%pI6)\n",
+	pr_info("Received SRP_LOGIN_REQ with i_port_id %pI6, t_port_id %pI6 and it_iu_len %d on port %d (guid=%pI6); pkey %#04x\n",
 		req->initiator_port_id, req->target_port_id, it_iu_len,
-		param->port, &sport->gid);
+		param->port, &sport->gid,
+		be16_to_cpu(param->primary_path->pkey));
 
 	rsp = kzalloc(sizeof(*rsp), GFP_KERNEL);
 	rej = kzalloc(sizeof(*rej), GFP_KERNEL);
@@ -2073,6 +2080,7 @@ static int srpt_cm_req_recv(struct ib_cm_id *cm_id,
 
 	init_rcu_head(&ch->rcu);
 	kref_init(&ch->kref);
+	ch->pkey = be16_to_cpu(param->primary_path->pkey);
 	ch->zw_cqe.done = srpt_zerolength_write_done;
 	INIT_WORK(&ch->release_work, srpt_release_channel_work);
 	memcpy(ch->i_port_id, req->initiator_port_id, 16);
diff --git a/drivers/infiniband/ulp/srpt/ib_srpt.h b/drivers/infiniband/ulp/srpt/ib_srpt.h
index 67248338b4c9..f830968e7fd4 100644
--- a/drivers/infiniband/ulp/srpt/ib_srpt.h
+++ b/drivers/infiniband/ulp/srpt/ib_srpt.h
@@ -266,6 +266,7 @@ enum rdma_ch_state {
  * @cmd_wait_list: List of SCSI commands that arrived before the RTU event. This
  *                 list contains struct srpt_ioctx elements and is protected
  *                 against concurrent modification by the cm_id spinlock.
+ * @pkey:          P_Key of the IB partition for this SRP channel.
  * @sess:          Session information associated with this SRP channel.
  * @sess_name:     Session name.
  * @ini_guid:      Initiator port GUID.
@@ -294,6 +295,7 @@ struct srpt_rdma_ch {
 	struct srpt_recv_ioctx	**ioctx_recv_ring;
 	struct list_head	list;
 	struct list_head	cmd_wait_list;
+	uint16_t		pkey;
 	struct se_session	*sess;
 	u8			sess_name[36];
 	u8			ini_guid[24];
-- 
2.15.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

^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [PATCH v2 04/14] IB/srpt: One target per port
       [not found] ` <20180117001418.7852-1-bart.vanassche-Sjgp3cTcYWE@public.gmane.org>
                     ` (2 preceding siblings ...)
  2018-01-17  0:14   ` [PATCH v2 03/14] IB/srpt: Add P_Key support Bart Van Assche
@ 2018-01-17  0:14   ` Bart Van Assche
  2018-01-17  0:14   ` [PATCH v2 05/14] IB/srpt: Use the source GID as session name Bart Van Assche
                     ` (11 subsequent siblings)
  15 siblings, 0 replies; 24+ messages in thread
From: Bart Van Assche @ 2018-01-17  0:14 UTC (permalink / raw)
  To: Jason Gunthorpe
  Cc: Doug Ledford, linux-rdma-u79uwXL29TY76Z2rM5mHXA, Bart Van Assche

In multipathing setups where a target system is equipped with
dual-port HCAs it is useful to have one connection per target port
instead of one connection per target HCA. Hence move the connection
list (rch_list) from struct srpt_device into struct srpt_port.

Signed-off-by: Bart Van Assche <bart.vanassche-Sjgp3cTcYWE@public.gmane.org>
---
 drivers/infiniband/ulp/srpt/ib_srpt.c | 98 +++++++++++++++++++----------------
 drivers/infiniband/ulp/srpt/ib_srpt.h | 16 +++---
 2 files changed, 61 insertions(+), 53 deletions(-)

diff --git a/drivers/infiniband/ulp/srpt/ib_srpt.c b/drivers/infiniband/ulp/srpt/ib_srpt.c
index cafa73083ee8..7893fc420794 100644
--- a/drivers/infiniband/ulp/srpt/ib_srpt.c
+++ b/drivers/infiniband/ulp/srpt/ib_srpt.c
@@ -1847,13 +1847,13 @@ static int srpt_disconnect_ch(struct srpt_rdma_ch *ch)
 	return ret;
 }
 
-static bool srpt_ch_closed(struct srpt_device *sdev, struct srpt_rdma_ch *ch)
+static bool srpt_ch_closed(struct srpt_port *sport, struct srpt_rdma_ch *ch)
 {
 	struct srpt_rdma_ch *ch2;
 	bool res = true;
 
 	rcu_read_lock();
-	list_for_each_entry(ch2, &sdev->rch_list, list) {
+	list_for_each_entry(ch2, &sport->rch_list, list) {
 		if (ch2 == ch) {
 			res = false;
 			break;
@@ -1871,33 +1871,32 @@ static bool srpt_ch_closed(struct srpt_device *sdev, struct srpt_rdma_ch *ch)
 static bool srpt_disconnect_ch_sync(struct srpt_rdma_ch *ch)
 	__must_hold(&sdev->mutex)
 {
-	struct srpt_device *sdev = ch->sport->sdev;
+	struct srpt_port *sport = ch->sport;
 	int ret;
 
-	lockdep_assert_held(&sdev->mutex);
+	lockdep_assert_held(&sport->mutex);
 
 	pr_debug("ch %s-%d state %d\n", ch->sess_name, ch->qp->qp_num,
 		 ch->state);
 
 	ret = srpt_disconnect_ch(ch);
-	mutex_unlock(&sdev->mutex);
+	mutex_unlock(&sport->mutex);
 
-	while (wait_event_timeout(sdev->ch_releaseQ, srpt_ch_closed(sdev, ch),
+	while (wait_event_timeout(sport->ch_releaseQ, srpt_ch_closed(sport, ch),
 				  5 * HZ) == 0)
 		pr_info("%s(%s-%d state %d): still waiting ...\n", __func__,
 			ch->sess_name, ch->qp->qp_num, ch->state);
 
-	mutex_lock(&sdev->mutex);
+	mutex_lock(&sport->mutex);
 	return ret == 0;
 }
 
 static void srpt_set_enabled(struct srpt_port *sport, bool enabled)
-	__must_hold(&sdev->mutex)
+	__must_hold(&sport->mutex)
 {
-	struct srpt_device *sdev = sport->sdev;
 	struct srpt_rdma_ch *ch;
 
-	lockdep_assert_held(&sdev->mutex);
+	lockdep_assert_held(&sport->mutex);
 
 	if (sport->enabled == enabled)
 		return;
@@ -1906,10 +1905,10 @@ static void srpt_set_enabled(struct srpt_port *sport, bool enabled)
 		return;
 
 again:
-	list_for_each_entry(ch, &sdev->rch_list, list) {
+	list_for_each_entry(ch, &sport->rch_list, list) {
 		if (ch->sport == sport) {
 			pr_info("%s: closing channel %s-%d\n",
-				sdev->device->name, ch->sess_name,
+				sport->sdev->device->name, ch->sess_name,
 				ch->qp->qp_num);
 			if (srpt_disconnect_ch_sync(ch))
 				goto again;
@@ -1929,6 +1928,7 @@ static void srpt_release_channel_work(struct work_struct *w)
 {
 	struct srpt_rdma_ch *ch;
 	struct srpt_device *sdev;
+	struct srpt_port *sport;
 	struct se_session *se_sess;
 
 	ch = container_of(w, struct srpt_rdma_ch, release_work);
@@ -1959,11 +1959,12 @@ static void srpt_release_channel_work(struct work_struct *w)
 			     sdev, ch->rq_size,
 			     srp_max_req_size, DMA_FROM_DEVICE);
 
-	mutex_lock(&sdev->mutex);
+	sport = ch->sport;
+	mutex_lock(&sport->mutex);
 	list_del_rcu(&ch->list);
-	mutex_unlock(&sdev->mutex);
+	mutex_unlock(&sport->mutex);
 
-	wake_up(&sdev->ch_releaseQ);
+	wake_up(&sport->ch_releaseQ);
 
 	kref_put(&ch->kref, srpt_free_ch);
 }
@@ -2036,9 +2037,9 @@ static int srpt_cm_req_recv(struct ib_cm_id *cm_id,
 	if ((req->req_flags & SRP_MTCH_ACTION) == SRP_MULTICHAN_SINGLE) {
 		rsp->rsp_flags = SRP_LOGIN_RSP_MULTICHAN_NO_CHAN;
 
-		mutex_lock(&sdev->mutex);
+		mutex_lock(&sport->mutex);
 
-		list_for_each_entry_safe(ch, tmp_ch, &sdev->rch_list, list) {
+		list_for_each_entry_safe(ch, tmp_ch, &sport->rch_list, list) {
 			if (!memcmp(ch->i_port_id, req->initiator_port_id, 16)
 			    && !memcmp(ch->t_port_id, req->target_port_id, 16)
 			    && param->port == ch->sport->port
@@ -2053,7 +2054,7 @@ static int srpt_cm_req_recv(struct ib_cm_id *cm_id,
 			}
 		}
 
-		mutex_unlock(&sdev->mutex);
+		mutex_unlock(&sport->mutex);
 
 	} else
 		rsp->rsp_flags = SRP_LOGIN_RSP_MULTICHAN_MAINTAINED;
@@ -2205,9 +2206,9 @@ static int srpt_cm_req_recv(struct ib_cm_id *cm_id,
 		goto release_channel;
 	}
 
-	mutex_lock(&sdev->mutex);
-	list_add_tail_rcu(&ch->list, &sdev->rch_list);
-	mutex_unlock(&sdev->mutex);
+	mutex_lock(&sport->mutex);
+	list_add_tail_rcu(&ch->list, &sport->rch_list);
+	mutex_unlock(&sport->mutex);
 
 	goto out;
 
@@ -2559,24 +2560,21 @@ static void srpt_refresh_port_work(struct work_struct *work)
 }
 
 /**
- * srpt_release_sdev - disable login and wait for associated channels
- * @sdev: SRPT HCA pointer.
+ * srpt_release_sport - disable login and wait for associated channels
+ * @sport: SRPT HCA port.
  */
-static int srpt_release_sdev(struct srpt_device *sdev)
+static int srpt_release_sport(struct srpt_port *sport)
 {
-	int i, res;
+	int res;
 
 	WARN_ON_ONCE(irqs_disabled());
 
-	BUG_ON(!sdev);
-
-	mutex_lock(&sdev->mutex);
-	for (i = 0; i < ARRAY_SIZE(sdev->port); i++)
-		srpt_set_enabled(&sdev->port[i], false);
-	mutex_unlock(&sdev->mutex);
+	mutex_lock(&sport->mutex);
+	srpt_set_enabled(sport, false);
+	mutex_unlock(&sport->mutex);
 
-	res = wait_event_interruptible(sdev->ch_releaseQ,
-				       list_empty_careful(&sdev->rch_list));
+	res = wait_event_interruptible(sport->ch_releaseQ,
+				       list_empty_careful(&sport->rch_list));
 	if (res)
 		pr_err("%s: interrupted.\n", __func__);
 
@@ -2704,9 +2702,7 @@ static void srpt_add_one(struct ib_device *device)
 		goto err;
 
 	sdev->device = device;
-	INIT_LIST_HEAD(&sdev->rch_list);
-	init_waitqueue_head(&sdev->ch_releaseQ);
-	mutex_init(&sdev->mutex);
+	mutex_init(&sdev->sdev_mutex);
 
 	sdev->pd = ib_alloc_pd(device, 0);
 	if (IS_ERR(sdev->pd))
@@ -2747,6 +2743,9 @@ static void srpt_add_one(struct ib_device *device)
 
 	for (i = 1; i <= sdev->device->phys_port_cnt; i++) {
 		sport = &sdev->port[i - 1];
+		INIT_LIST_HEAD(&sport->rch_list);
+		init_waitqueue_head(&sport->ch_releaseQ);
+		mutex_init(&sport->mutex);
 		sport->sdev = sdev;
 		sport->port = i;
 		sport->port_attrib.srp_max_rdma_size = DEFAULT_MAX_RDMA_SIZE;
@@ -2819,7 +2818,9 @@ static void srpt_remove_one(struct ib_device *device, void *client_data)
 	spin_lock(&srpt_dev_lock);
 	list_del(&sdev->list);
 	spin_unlock(&srpt_dev_lock);
-	srpt_release_sdev(sdev);
+
+	for (i = 0; i < sdev->device->phys_port_cnt; i++)
+		srpt_release_sport(&sdev->port[i]);
 
 	srpt_free_srq(sdev);
 
@@ -2905,11 +2906,11 @@ static void srpt_release_cmd(struct se_cmd *se_cmd)
 static void srpt_close_session(struct se_session *se_sess)
 {
 	struct srpt_rdma_ch *ch = se_sess->fabric_sess_ptr;
-	struct srpt_device *sdev = ch->sport->sdev;
+	struct srpt_port *sport = ch->sport;
 
-	mutex_lock(&sdev->mutex);
+	mutex_lock(&sport->mutex);
 	srpt_disconnect_ch_sync(ch);
-	mutex_unlock(&sdev->mutex);
+	mutex_unlock(&sport->mutex);
 }
 
 /**
@@ -3134,18 +3135,24 @@ static ssize_t srpt_tpg_attrib_use_srq_store(struct config_item *item,
 	if (val != !!val)
 		return -EINVAL;
 
-	ret = mutex_lock_interruptible(&sdev->mutex);
+	ret = mutex_lock_interruptible(&sdev->sdev_mutex);
 	if (ret < 0)
 		return ret;
+	ret = mutex_lock_interruptible(&sport->mutex);
+	if (ret < 0)
+		goto unlock_sdev;
 	enabled = sport->enabled;
 	/* Log out all initiator systems before changing 'use_srq'. */
 	srpt_set_enabled(sport, false);
 	sport->port_attrib.use_srq = val;
 	srpt_use_srq(sdev, sport->port_attrib.use_srq);
 	srpt_set_enabled(sport, enabled);
-	mutex_unlock(&sdev->mutex);
+	ret = count;
+	mutex_unlock(&sport->mutex);
+unlock_sdev:
+	mutex_unlock(&sdev->sdev_mutex);
 
-	return count;
+	return ret;
 }
 
 CONFIGFS_ATTR(srpt_tpg_attrib_,  srp_max_rdma_size);
@@ -3174,7 +3181,6 @@ static ssize_t srpt_tpg_enable_store(struct config_item *item,
 {
 	struct se_portal_group *se_tpg = to_tpg(item);
 	struct srpt_port *sport = srpt_tpg_to_sport(se_tpg);
-	struct srpt_device *sdev = sport->sdev;
 	unsigned long tmp;
         int ret;
 
@@ -3189,9 +3195,9 @@ static ssize_t srpt_tpg_enable_store(struct config_item *item,
 		return -EINVAL;
 	}
 
-	mutex_lock(&sdev->mutex);
+	mutex_lock(&sport->mutex);
 	srpt_set_enabled(sport, tmp);
-	mutex_unlock(&sdev->mutex);
+	mutex_unlock(&sport->mutex);
 
 	return count;
 }
diff --git a/drivers/infiniband/ulp/srpt/ib_srpt.h b/drivers/infiniband/ulp/srpt/ib_srpt.h
index f830968e7fd4..1434f0cd45f7 100644
--- a/drivers/infiniband/ulp/srpt/ib_srpt.h
+++ b/drivers/infiniband/ulp/srpt/ib_srpt.h
@@ -262,7 +262,7 @@ enum rdma_ch_state {
  * @state:         channel state. See also enum rdma_ch_state.
  * @ioctx_ring:    Send ring.
  * @ioctx_recv_ring: Receive I/O context ring.
- * @list:          Node for insertion in the srpt_device.rch_list list.
+ * @list:          Node in srpt_port.rch_list.
  * @cmd_wait_list: List of SCSI commands that arrived before the RTU event. This
  *                 list contains struct srpt_ioctx elements and is protected
  *                 against concurrent modification by the cm_id spinlock.
@@ -334,6 +334,9 @@ struct srpt_port_attrib {
  * @port_gid_tpg:  TPG associated with target port GID.
  * @port_gid_wwn:  WWN associated with target port GID.
  * @port_attrib:   Port attributes that can be accessed through configfs.
+ * @ch_releaseQ:   Enables waiting for removal from rch_list.
+ * @mutex:	   Protects rch_list.
+ * @rch_list:	   Channel list. See also srpt_rdma_ch.list.
  */
 struct srpt_port {
 	struct srpt_device	*sdev;
@@ -351,6 +354,9 @@ struct srpt_port {
 	struct se_portal_group	port_gid_tpg;
 	struct se_wwn		port_gid_wwn;
 	struct srpt_port_attrib port_attrib;
+	wait_queue_head_t	ch_releaseQ;
+	struct mutex		mutex;
+	struct list_head	rch_list;
 };
 
 /**
@@ -361,11 +367,9 @@ struct srpt_port {
  * @srq:           Per-HCA SRQ (shared receive queue).
  * @cm_id:         Connection identifier.
  * @srq_size:      SRQ size.
+ * @sdev_mutex:	   Serializes use_srq changes.
  * @use_srq:       Whether or not to use SRQ.
  * @ioctx_ring:    Per-HCA SRQ.
- * @rch_list:      Per-device channel list -- see also srpt_rdma_ch.list.
- * @ch_releaseQ:   Enables waiting for removal from rch_list.
- * @mutex:         Protects rch_list.
  * @port:          Information about the ports owned by this HCA.
  * @event_handler: Per-HCA asynchronous IB event handler.
  * @list:          Node in srpt_dev_list.
@@ -377,11 +381,9 @@ struct srpt_device {
 	struct ib_srq		*srq;
 	struct ib_cm_id		*cm_id;
 	int			srq_size;
+	struct mutex		sdev_mutex;
 	bool			use_srq;
 	struct srpt_recv_ioctx	**ioctx_ring;
-	struct list_head	rch_list;
-	wait_queue_head_t	ch_releaseQ;
-	struct mutex		mutex;
 	struct srpt_port	port[2];
 	struct ib_event_handler	event_handler;
 	struct list_head	list;
-- 
2.15.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

^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [PATCH v2 05/14] IB/srpt: Use the source GID as session name
       [not found] ` <20180117001418.7852-1-bart.vanassche-Sjgp3cTcYWE@public.gmane.org>
                     ` (3 preceding siblings ...)
  2018-01-17  0:14   ` [PATCH v2 04/14] IB/srpt: One target per port Bart Van Assche
@ 2018-01-17  0:14   ` Bart Van Assche
  2018-01-17  0:14   ` [PATCH v2 06/14] IB/srpt: Rework multi-channel support Bart Van Assche
                     ` (10 subsequent siblings)
  15 siblings, 0 replies; 24+ messages in thread
From: Bart Van Assche @ 2018-01-17  0:14 UTC (permalink / raw)
  To: Jason Gunthorpe
  Cc: Doug Ledford, linux-rdma-u79uwXL29TY76Z2rM5mHXA, Bart Van Assche

Use the source GID as session name instead of the initiator port ID
from the SRP login request. The only functional change in this patch
is that it changes the session name shown in debug messages.

Note: the fifth argument that is passed to target_alloc_session() is
what the SCSI target core uses as key for lookups in the ACL (access
control list) information.

Signed-off-by: Bart Van Assche <bart.vanassche-Sjgp3cTcYWE@public.gmane.org>
---
 drivers/infiniband/ulp/srpt/ib_srpt.c | 11 ++++++-----
 drivers/infiniband/ulp/srpt/ib_srpt.h |  4 +---
 2 files changed, 7 insertions(+), 8 deletions(-)

diff --git a/drivers/infiniband/ulp/srpt/ib_srpt.c b/drivers/infiniband/ulp/srpt/ib_srpt.c
index 7893fc420794..597fc0bc0734 100644
--- a/drivers/infiniband/ulp/srpt/ib_srpt.c
+++ b/drivers/infiniband/ulp/srpt/ib_srpt.c
@@ -1989,6 +1989,7 @@ static int srpt_cm_req_recv(struct ib_cm_id *cm_id,
 	struct srp_login_rej *rej;
 	struct ib_cm_rep_param *rep_param;
 	struct srpt_rdma_ch *ch, *tmp_ch;
+	char i_port_id[36];
 	u32 it_iu_len;
 	int i, ret = 0;
 
@@ -2143,9 +2144,9 @@ static int srpt_cm_req_recv(struct ib_cm_id *cm_id,
 		goto destroy_ib;
 	}
 
-	srpt_format_guid(ch->ini_guid, sizeof(ch->ini_guid),
+	srpt_format_guid(ch->sess_name, sizeof(ch->sess_name),
 			 &param->primary_path->dgid.global.interface_id);
-	snprintf(ch->sess_name, sizeof(ch->sess_name), "0x%016llx%016llx",
+	snprintf(i_port_id, sizeof(i_port_id), "0x%016llx%016llx",
 			be64_to_cpu(*(__be64 *)ch->i_port_id),
 			be64_to_cpu(*(__be64 *)(ch->i_port_id + 8)));
 
@@ -2154,16 +2155,16 @@ static int srpt_cm_req_recv(struct ib_cm_id *cm_id,
 	if (sport->port_guid_tpg.se_tpg_wwn)
 		ch->sess = target_alloc_session(&sport->port_guid_tpg, 0, 0,
 						TARGET_PROT_NORMAL,
-						ch->ini_guid, ch, NULL);
+						ch->sess_name, ch, NULL);
 	if (sport->port_gid_tpg.se_tpg_wwn && IS_ERR_OR_NULL(ch->sess))
 		ch->sess = target_alloc_session(&sport->port_gid_tpg, 0, 0,
-					TARGET_PROT_NORMAL, ch->sess_name, ch,
+					TARGET_PROT_NORMAL, i_port_id, ch,
 					NULL);
 	/* Retry without leading "0x" */
 	if (sport->port_gid_tpg.se_tpg_wwn && IS_ERR_OR_NULL(ch->sess))
 		ch->sess = target_alloc_session(&sport->port_gid_tpg, 0, 0,
 						TARGET_PROT_NORMAL,
-						ch->sess_name + 2, ch, NULL);
+						i_port_id + 2, ch, NULL);
 	if (IS_ERR_OR_NULL(ch->sess)) {
 		pr_info("Rejected login because no ACL has been configured yet for initiator %s.\n",
 			ch->sess_name);
diff --git a/drivers/infiniband/ulp/srpt/ib_srpt.h b/drivers/infiniband/ulp/srpt/ib_srpt.h
index 1434f0cd45f7..6c5a14ac7742 100644
--- a/drivers/infiniband/ulp/srpt/ib_srpt.h
+++ b/drivers/infiniband/ulp/srpt/ib_srpt.h
@@ -269,7 +269,6 @@ enum rdma_ch_state {
  * @pkey:          P_Key of the IB partition for this SRP channel.
  * @sess:          Session information associated with this SRP channel.
  * @sess_name:     Session name.
- * @ini_guid:      Initiator port GUID.
  * @release_work:  Allows scheduling of srpt_release_channel().
  */
 struct srpt_rdma_ch {
@@ -297,8 +296,7 @@ struct srpt_rdma_ch {
 	struct list_head	cmd_wait_list;
 	uint16_t		pkey;
 	struct se_session	*sess;
-	u8			sess_name[36];
-	u8			ini_guid[24];
+	u8			sess_name[24];
 	struct work_struct	release_work;
 };
 
-- 
2.15.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

^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [PATCH v2 06/14] IB/srpt: Rework multi-channel support
       [not found] ` <20180117001418.7852-1-bart.vanassche-Sjgp3cTcYWE@public.gmane.org>
                     ` (4 preceding siblings ...)
  2018-01-17  0:14   ` [PATCH v2 05/14] IB/srpt: Use the source GID as session name Bart Van Assche
@ 2018-01-17  0:14   ` Bart Van Assche
  2018-01-17  0:14   ` [PATCH v2 07/14] IB/srpt: Simplify srpt_close_session() Bart Van Assche
                     ` (9 subsequent siblings)
  15 siblings, 0 replies; 24+ messages in thread
From: Bart Van Assche @ 2018-01-17  0:14 UTC (permalink / raw)
  To: Jason Gunthorpe
  Cc: Doug Ledford, linux-rdma-u79uwXL29TY76Z2rM5mHXA, Bart Van Assche

Store initiator and target port ID's once per nexus instead of in each
channel data structure. This change simplifies the duplicate connection
check in srpt_cm_req_recv().

Signed-off-by: Bart Van Assche <bart.vanassche-Sjgp3cTcYWE@public.gmane.org>
---
 drivers/infiniband/ulp/srpt/ib_srpt.c | 186 ++++++++++++++++++++++++----------
 drivers/infiniband/ulp/srpt/ib_srpt.h |  34 +++++--
 2 files changed, 160 insertions(+), 60 deletions(-)

diff --git a/drivers/infiniband/ulp/srpt/ib_srpt.c b/drivers/infiniband/ulp/srpt/ib_srpt.c
index 597fc0bc0734..de3e77df146e 100644
--- a/drivers/infiniband/ulp/srpt/ib_srpt.c
+++ b/drivers/infiniband/ulp/srpt/ib_srpt.c
@@ -1849,16 +1849,20 @@ static int srpt_disconnect_ch(struct srpt_rdma_ch *ch)
 
 static bool srpt_ch_closed(struct srpt_port *sport, struct srpt_rdma_ch *ch)
 {
+	struct srpt_nexus *nexus;
 	struct srpt_rdma_ch *ch2;
 	bool res = true;
 
 	rcu_read_lock();
-	list_for_each_entry(ch2, &sport->rch_list, list) {
-		if (ch2 == ch) {
-			res = false;
-			break;
+	list_for_each_entry(nexus, &sport->nexus_list, entry) {
+		list_for_each_entry(ch2, &nexus->ch_list, list) {
+			if (ch2 == ch) {
+				res = false;
+				goto done;
+			}
 		}
 	}
+done:
 	rcu_read_unlock();
 
 	return res;
@@ -1891,30 +1895,78 @@ static bool srpt_disconnect_ch_sync(struct srpt_rdma_ch *ch)
 	return ret == 0;
 }
 
-static void srpt_set_enabled(struct srpt_port *sport, bool enabled)
-	__must_hold(&sport->mutex)
+static void __srpt_close_all_ch(struct srpt_port *sport)
 {
+	struct srpt_nexus *nexus;
 	struct srpt_rdma_ch *ch;
 
 	lockdep_assert_held(&sport->mutex);
 
-	if (sport->enabled == enabled)
-		return;
-	sport->enabled = enabled;
-	if (sport->enabled)
-		return;
+	list_for_each_entry(nexus, &sport->nexus_list, entry) {
+		list_for_each_entry(ch, &nexus->ch_list, list) {
+			if (srpt_disconnect_ch(ch) >= 0)
+				pr_info("Closing channel %s-%d because target %s_%d has been disabled\n",
+					ch->sess_name, ch->qp->qp_num,
+					sport->sdev->device->name, sport->port);
+			srpt_close_ch(ch);
+		}
+	}
+}
+
+/*
+ * Look up (i_port_id, t_port_id) in sport->nexus_list. Create an entry if
+ * it does not yet exist.
+ */
+static struct srpt_nexus *srpt_get_nexus(struct srpt_port *sport,
+					 const u8 i_port_id[16],
+					 const u8 t_port_id[16])
+{
+	struct srpt_nexus *nexus = NULL, *tmp_nexus = NULL, *n;
 
-again:
-	list_for_each_entry(ch, &sport->rch_list, list) {
-		if (ch->sport == sport) {
-			pr_info("%s: closing channel %s-%d\n",
-				sport->sdev->device->name, ch->sess_name,
-				ch->qp->qp_num);
-			if (srpt_disconnect_ch_sync(ch))
-				goto again;
+	for (;;) {
+		mutex_lock(&sport->mutex);
+		list_for_each_entry(n, &sport->nexus_list, entry) {
+			if (memcmp(n->i_port_id, i_port_id, 16) == 0 &&
+			    memcmp(n->t_port_id, t_port_id, 16) == 0) {
+				nexus = n;
+				break;
+			}
+		}
+		if (!nexus && tmp_nexus) {
+			list_add_tail_rcu(&tmp_nexus->entry,
+					  &sport->nexus_list);
+			swap(nexus, tmp_nexus);
 		}
+		mutex_unlock(&sport->mutex);
+
+		if (nexus)
+			break;
+		tmp_nexus = kzalloc(sizeof(*nexus), GFP_KERNEL);
+		if (!tmp_nexus) {
+			nexus = ERR_PTR(-ENOMEM);
+			break;
+		}
+		init_rcu_head(&tmp_nexus->rcu);
+		INIT_LIST_HEAD(&tmp_nexus->ch_list);
+		memcpy(tmp_nexus->i_port_id, i_port_id, 16);
+		memcpy(tmp_nexus->t_port_id, t_port_id, 16);
 	}
 
+	kfree(tmp_nexus);
+
+	return nexus;
+}
+
+static void srpt_set_enabled(struct srpt_port *sport, bool enabled)
+	__must_hold(&sport->mutex)
+{
+	lockdep_assert_held(&sport->mutex);
+
+	if (sport->enabled == enabled)
+		return;
+	sport->enabled = enabled;
+	if (!enabled)
+		__srpt_close_all_ch(sport);
 }
 
 static void srpt_free_ch(struct kref *kref)
@@ -1984,11 +2036,12 @@ static int srpt_cm_req_recv(struct ib_cm_id *cm_id,
 {
 	struct srpt_device *sdev = cm_id->context;
 	struct srpt_port *sport = &sdev->port[param->port - 1];
+	struct srpt_nexus *nexus;
 	struct srp_login_req *req;
-	struct srp_login_rsp *rsp;
-	struct srp_login_rej *rej;
-	struct ib_cm_rep_param *rep_param;
-	struct srpt_rdma_ch *ch, *tmp_ch;
+	struct srp_login_rsp *rsp = NULL;
+	struct srp_login_rej *rej = NULL;
+	struct ib_cm_rep_param *rep_param = NULL;
+	struct srpt_rdma_ch *ch;
 	char i_port_id[36];
 	u32 it_iu_len;
 	int i, ret = 0;
@@ -2007,6 +2060,13 @@ static int srpt_cm_req_recv(struct ib_cm_id *cm_id,
 		param->port, &sport->gid,
 		be16_to_cpu(param->primary_path->pkey));
 
+	nexus = srpt_get_nexus(sport, req->initiator_port_id,
+			       req->target_port_id);
+	if (IS_ERR(nexus)) {
+		ret = PTR_ERR(nexus);
+		goto out;
+	}
+
 	rsp = kzalloc(sizeof(*rsp), GFP_KERNEL);
 	rej = kzalloc(sizeof(*rej), GFP_KERNEL);
 	rep_param = kzalloc(sizeof(*rep_param), GFP_KERNEL);
@@ -2036,29 +2096,22 @@ static int srpt_cm_req_recv(struct ib_cm_id *cm_id,
 	}
 
 	if ((req->req_flags & SRP_MTCH_ACTION) == SRP_MULTICHAN_SINGLE) {
+		struct srpt_rdma_ch *ch2;
+
 		rsp->rsp_flags = SRP_LOGIN_RSP_MULTICHAN_NO_CHAN;
 
 		mutex_lock(&sport->mutex);
-
-		list_for_each_entry_safe(ch, tmp_ch, &sport->rch_list, list) {
-			if (!memcmp(ch->i_port_id, req->initiator_port_id, 16)
-			    && !memcmp(ch->t_port_id, req->target_port_id, 16)
-			    && param->port == ch->sport->port
-			    && param->listen_id == ch->sport->sdev->cm_id
-			    && ch->cm_id) {
-				if (srpt_disconnect_ch(ch) < 0)
-					continue;
-				pr_info("Relogin - closed existing channel %s\n",
-					ch->sess_name);
-				rsp->rsp_flags =
-					SRP_LOGIN_RSP_MULTICHAN_TERMINATED;
-			}
+		list_for_each_entry(ch2, &nexus->ch_list, list) {
+			if (srpt_disconnect_ch(ch2) < 0)
+				continue;
+			pr_info("Relogin - closed existing channel %s\n",
+				ch2->sess_name);
+			rsp->rsp_flags = SRP_LOGIN_RSP_MULTICHAN_TERMINATED;
 		}
-
 		mutex_unlock(&sport->mutex);
-
-	} else
+	} else {
 		rsp->rsp_flags = SRP_LOGIN_RSP_MULTICHAN_MAINTAINED;
+	}
 
 	if (*(__be64 *)req->target_port_id != cpu_to_be64(srpt_service_guid)
 	    || *(__be64 *)(req->target_port_id + 8) !=
@@ -2083,10 +2136,9 @@ static int srpt_cm_req_recv(struct ib_cm_id *cm_id,
 	init_rcu_head(&ch->rcu);
 	kref_init(&ch->kref);
 	ch->pkey = be16_to_cpu(param->primary_path->pkey);
+	ch->nexus = nexus;
 	ch->zw_cqe.done = srpt_zerolength_write_done;
 	INIT_WORK(&ch->release_work, srpt_release_channel_work);
-	memcpy(ch->i_port_id, req->initiator_port_id, 16);
-	memcpy(ch->t_port_id, req->target_port_id, 16);
 	ch->sport = &sdev->port[param->port - 1];
 	ch->cm_id = cm_id;
 	cm_id->context = ch;
@@ -2147,8 +2199,8 @@ static int srpt_cm_req_recv(struct ib_cm_id *cm_id,
 	srpt_format_guid(ch->sess_name, sizeof(ch->sess_name),
 			 &param->primary_path->dgid.global.interface_id);
 	snprintf(i_port_id, sizeof(i_port_id), "0x%016llx%016llx",
-			be64_to_cpu(*(__be64 *)ch->i_port_id),
-			be64_to_cpu(*(__be64 *)(ch->i_port_id + 8)));
+			be64_to_cpu(*(__be64 *)nexus->i_port_id),
+			be64_to_cpu(*(__be64 *)(nexus->i_port_id + 8)));
 
 	pr_debug("registering session %s\n", ch->sess_name);
 
@@ -2208,7 +2260,7 @@ static int srpt_cm_req_recv(struct ib_cm_id *cm_id,
 	}
 
 	mutex_lock(&sport->mutex);
-	list_add_tail_rcu(&ch->list, &sport->rch_list);
+	list_add_tail_rcu(&ch->list, &nexus->ch_list);
 	mutex_unlock(&sport->mutex);
 
 	goto out;
@@ -2560,13 +2612,28 @@ static void srpt_refresh_port_work(struct work_struct *work)
 	srpt_refresh_port(sport);
 }
 
+static bool srpt_ch_list_empty(struct srpt_port *sport)
+{
+	struct srpt_nexus *nexus;
+	bool res = true;
+
+	rcu_read_lock();
+	list_for_each_entry(nexus, &sport->nexus_list, entry)
+		if (!list_empty(&nexus->ch_list))
+			res = false;
+	rcu_read_unlock();
+
+	return res;
+}
+
 /**
  * srpt_release_sport - disable login and wait for associated channels
  * @sport: SRPT HCA port.
  */
 static int srpt_release_sport(struct srpt_port *sport)
 {
-	int res;
+	struct srpt_nexus *nexus, *next_n;
+	struct srpt_rdma_ch *ch;
 
 	WARN_ON_ONCE(irqs_disabled());
 
@@ -2574,10 +2641,27 @@ static int srpt_release_sport(struct srpt_port *sport)
 	srpt_set_enabled(sport, false);
 	mutex_unlock(&sport->mutex);
 
-	res = wait_event_interruptible(sport->ch_releaseQ,
-				       list_empty_careful(&sport->rch_list));
-	if (res)
-		pr_err("%s: interrupted.\n", __func__);
+	while (wait_event_timeout(sport->ch_releaseQ,
+				  srpt_ch_list_empty(sport), 5 * HZ) <= 0) {
+		pr_info("%s_%d: waiting for session unregistration ...\n",
+			sport->sdev->device->name, sport->port);
+		rcu_read_lock();
+		list_for_each_entry(nexus, &sport->nexus_list, entry) {
+			list_for_each_entry(ch, &nexus->ch_list, list) {
+				pr_info("%s-%d: state %s\n",
+					ch->sess_name, ch->qp->qp_num,
+					get_ch_state_name(ch->state));
+			}
+		}
+		rcu_read_unlock();
+	}
+
+	mutex_lock(&sport->mutex);
+	list_for_each_entry_safe(nexus, next_n, &sport->nexus_list, entry) {
+		list_del(&nexus->entry);
+		kfree_rcu(nexus, rcu);
+	}
+	mutex_unlock(&sport->mutex);
 
 	return 0;
 }
@@ -2744,7 +2828,7 @@ static void srpt_add_one(struct ib_device *device)
 
 	for (i = 1; i <= sdev->device->phys_port_cnt; i++) {
 		sport = &sdev->port[i - 1];
-		INIT_LIST_HEAD(&sport->rch_list);
+		INIT_LIST_HEAD(&sport->nexus_list);
 		init_waitqueue_head(&sport->ch_releaseQ);
 		mutex_init(&sport->mutex);
 		sport->sdev = sdev;
diff --git a/drivers/infiniband/ulp/srpt/ib_srpt.h b/drivers/infiniband/ulp/srpt/ib_srpt.h
index 6c5a14ac7742..59261d5de292 100644
--- a/drivers/infiniband/ulp/srpt/ib_srpt.h
+++ b/drivers/infiniband/ulp/srpt/ib_srpt.h
@@ -54,6 +54,8 @@
  */
 #define SRP_SERVICE_NAME_PREFIX		"SRP.T10:"
 
+struct srpt_nexus;
+
 enum {
 	/*
 	 * SRP IOControllerProfile attributes for SRP target ports that have
@@ -240,6 +242,7 @@ enum rdma_ch_state {
 
 /**
  * struct srpt_rdma_ch - RDMA channel
+ * @nexus:         I_T nexus this channel is associated with.
  * @cm_id:         IB CM ID associated with the channel.
  * @qp:            IB queue pair used for communicating over this channel.
  * @cq:            IB completion queue for this channel.
@@ -251,8 +254,6 @@ enum rdma_ch_state {
  * @sq_wr_avail:   number of work requests available in the send queue.
  * @sport:         pointer to the information of the HCA port used by this
  *                 channel.
- * @i_port_id:     128-bit initiator port identifier copied from SRP_LOGIN_REQ.
- * @t_port_id:     128-bit target port identifier copied from SRP_LOGIN_REQ.
  * @max_ti_iu_len: maximum target-to-initiator information unit length.
  * @req_lim:       request limit: maximum number of requests that may be sent
  *                 by the initiator without having received a response.
@@ -262,7 +263,7 @@ enum rdma_ch_state {
  * @state:         channel state. See also enum rdma_ch_state.
  * @ioctx_ring:    Send ring.
  * @ioctx_recv_ring: Receive I/O context ring.
- * @list:          Node in srpt_port.rch_list.
+ * @list:          Node in srpt_nexus.ch_list.
  * @cmd_wait_list: List of SCSI commands that arrived before the RTU event. This
  *                 list contains struct srpt_ioctx elements and is protected
  *                 against concurrent modification by the cm_id spinlock.
@@ -272,6 +273,7 @@ enum rdma_ch_state {
  * @release_work:  Allows scheduling of srpt_release_channel().
  */
 struct srpt_rdma_ch {
+	struct srpt_nexus	*nexus;
 	struct ib_cm_id		*cm_id;
 	struct ib_qp		*qp;
 	struct ib_cq		*cq;
@@ -282,8 +284,6 @@ struct srpt_rdma_ch {
 	u32			max_rsp_size;
 	atomic_t		sq_wr_avail;
 	struct srpt_port	*sport;
-	u8			i_port_id[16];
-	u8			t_port_id[16];
 	int			max_ti_iu_len;
 	atomic_t		req_lim;
 	atomic_t		req_lim_delta;
@@ -300,6 +300,22 @@ struct srpt_rdma_ch {
 	struct work_struct	release_work;
 };
 
+/**
+ * struct srpt_nexus - I_T nexus
+ * @rcu:       RCU head for this data structure.
+ * @entry:     srpt_port.nexus_list list node.
+ * @ch_list:   struct srpt_rdma_ch list. Protected by srpt_port.mutex.
+ * @i_port_id: 128-bit initiator port identifier copied from SRP_LOGIN_REQ.
+ * @t_port_id: 128-bit target port identifier copied from SRP_LOGIN_REQ.
+ */
+struct srpt_nexus {
+	struct rcu_head		rcu;
+	struct list_head	entry;
+	struct list_head	ch_list;
+	u8			i_port_id[16];
+	u8			t_port_id[16];
+};
+
 /**
  * struct srpt_port_attib - attributes for SRPT port
  * @srp_max_rdma_size: Maximum size of SRP RDMA transfers for new connections.
@@ -332,9 +348,9 @@ struct srpt_port_attrib {
  * @port_gid_tpg:  TPG associated with target port GID.
  * @port_gid_wwn:  WWN associated with target port GID.
  * @port_attrib:   Port attributes that can be accessed through configfs.
- * @ch_releaseQ:   Enables waiting for removal from rch_list.
- * @mutex:	   Protects rch_list.
- * @rch_list:	   Channel list. See also srpt_rdma_ch.list.
+ * @ch_releaseQ:   Enables waiting for removal from nexus_list.
+ * @mutex:	   Protects nexus_list.
+ * @nexus_list:	   Nexus list. See also srpt_nexus.entry.
  */
 struct srpt_port {
 	struct srpt_device	*sdev;
@@ -354,7 +370,7 @@ struct srpt_port {
 	struct srpt_port_attrib port_attrib;
 	wait_queue_head_t	ch_releaseQ;
 	struct mutex		mutex;
-	struct list_head	rch_list;
+	struct list_head	nexus_list;
 };
 
 /**
-- 
2.15.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

^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [PATCH v2 07/14] IB/srpt: Simplify srpt_close_session()
       [not found] ` <20180117001418.7852-1-bart.vanassche-Sjgp3cTcYWE@public.gmane.org>
                     ` (5 preceding siblings ...)
  2018-01-17  0:14   ` [PATCH v2 06/14] IB/srpt: Rework multi-channel support Bart Van Assche
@ 2018-01-17  0:14   ` Bart Van Assche
  2018-01-17  0:14   ` [PATCH v2 08/14] IB/srpt: Log all zero-length writes and completions Bart Van Assche
                     ` (8 subsequent siblings)
  15 siblings, 0 replies; 24+ messages in thread
From: Bart Van Assche @ 2018-01-17  0:14 UTC (permalink / raw)
  To: Jason Gunthorpe
  Cc: Doug Ledford, linux-rdma-u79uwXL29TY76Z2rM5mHXA, Bart Van Assche

Move a mutex lock and unlock statement from srpt_close_session()
into srpt_disconnect_ch_sync(). Since the previous patch removed
the last user of the return value of that function, change the
return value of srpt_disconnect_ch_sync() into void.

Signed-off-by: Bart Van Assche <bart.vanassche-Sjgp3cTcYWE@public.gmane.org>
---
 drivers/infiniband/ulp/srpt/ib_srpt.c | 19 ++++---------------
 1 file changed, 4 insertions(+), 15 deletions(-)

diff --git a/drivers/infiniband/ulp/srpt/ib_srpt.c b/drivers/infiniband/ulp/srpt/ib_srpt.c
index de3e77df146e..b248515a4fe4 100644
--- a/drivers/infiniband/ulp/srpt/ib_srpt.c
+++ b/drivers/infiniband/ulp/srpt/ib_srpt.c
@@ -1868,22 +1868,16 @@ static bool srpt_ch_closed(struct srpt_port *sport, struct srpt_rdma_ch *ch)
 	return res;
 }
 
-/*
- * Send DREQ and wait for DREP. Return true if and only if this function
- * changed the state of @ch.
- */
-static bool srpt_disconnect_ch_sync(struct srpt_rdma_ch *ch)
-	__must_hold(&sdev->mutex)
+/* Send DREQ and wait for DREP. */
+static void srpt_disconnect_ch_sync(struct srpt_rdma_ch *ch)
 {
 	struct srpt_port *sport = ch->sport;
-	int ret;
-
-	lockdep_assert_held(&sport->mutex);
 
 	pr_debug("ch %s-%d state %d\n", ch->sess_name, ch->qp->qp_num,
 		 ch->state);
 
-	ret = srpt_disconnect_ch(ch);
+	mutex_lock(&sport->mutex);
+	srpt_disconnect_ch(ch);
 	mutex_unlock(&sport->mutex);
 
 	while (wait_event_timeout(sport->ch_releaseQ, srpt_ch_closed(sport, ch),
@@ -1891,8 +1885,6 @@ static bool srpt_disconnect_ch_sync(struct srpt_rdma_ch *ch)
 		pr_info("%s(%s-%d state %d): still waiting ...\n", __func__,
 			ch->sess_name, ch->qp->qp_num, ch->state);
 
-	mutex_lock(&sport->mutex);
-	return ret == 0;
 }
 
 static void __srpt_close_all_ch(struct srpt_port *sport)
@@ -2991,11 +2983,8 @@ static void srpt_release_cmd(struct se_cmd *se_cmd)
 static void srpt_close_session(struct se_session *se_sess)
 {
 	struct srpt_rdma_ch *ch = se_sess->fabric_sess_ptr;
-	struct srpt_port *sport = ch->sport;
 
-	mutex_lock(&sport->mutex);
 	srpt_disconnect_ch_sync(ch);
-	mutex_unlock(&sport->mutex);
 }
 
 /**
-- 
2.15.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

^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [PATCH v2 08/14] IB/srpt: Log all zero-length writes and completions
       [not found] ` <20180117001418.7852-1-bart.vanassche-Sjgp3cTcYWE@public.gmane.org>
                     ` (6 preceding siblings ...)
  2018-01-17  0:14   ` [PATCH v2 07/14] IB/srpt: Simplify srpt_close_session() Bart Van Assche
@ 2018-01-17  0:14   ` Bart Van Assche
  2018-01-17  0:14   ` [PATCH v2 09/14] IB/srpt: Fix login-related race conditions Bart Van Assche
                     ` (7 subsequent siblings)
  15 siblings, 0 replies; 24+ messages in thread
From: Bart Van Assche @ 2018-01-17  0:14 UTC (permalink / raw)
  To: Jason Gunthorpe
  Cc: Doug Ledford, linux-rdma-u79uwXL29TY76Z2rM5mHXA, Bart Van Assche

The new pr_debug() statements are useful when debugging the ib_srpt
driver.

Signed-off-by: Bart Van Assche <bart.vanassche-Sjgp3cTcYWE@public.gmane.org>
---
 drivers/infiniband/ulp/srpt/ib_srpt.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/drivers/infiniband/ulp/srpt/ib_srpt.c b/drivers/infiniband/ulp/srpt/ib_srpt.c
index b248515a4fe4..866ff4be553c 100644
--- a/drivers/infiniband/ulp/srpt/ib_srpt.c
+++ b/drivers/infiniband/ulp/srpt/ib_srpt.c
@@ -840,6 +840,9 @@ static int srpt_zerolength_write(struct srpt_rdma_ch *ch)
 {
 	struct ib_send_wr wr, *bad_wr;
 
+	pr_debug("%s-%d: queued zerolength write\n", ch->sess_name,
+		 ch->qp->qp_num);
+
 	memset(&wr, 0, sizeof(wr));
 	wr.opcode = IB_WR_RDMA_WRITE;
 	wr.wr_cqe = &ch->zw_cqe;
@@ -851,6 +854,9 @@ static void srpt_zerolength_write_done(struct ib_cq *cq, struct ib_wc *wc)
 {
 	struct srpt_rdma_ch *ch = cq->cq_context;
 
+	pr_debug("%s-%d wc->status %d\n", ch->sess_name, ch->qp->qp_num,
+		 wc->status);
+
 	if (wc->status == IB_WC_SUCCESS) {
 		srpt_process_wait_list(ch);
 	} else {
@@ -1804,8 +1810,6 @@ static bool srpt_close_ch(struct srpt_rdma_ch *ch)
 		pr_err("%s-%d: changing queue pair into error state failed: %d\n",
 		       ch->sess_name, ch->qp->qp_num, ret);
 
-	pr_debug("%s-%d: queued zerolength write\n", ch->sess_name,
-		 ch->qp->qp_num);
 	ret = srpt_zerolength_write(ch);
 	if (ret < 0) {
 		pr_err("%s-%d: queuing zero-length write failed: %d\n",
-- 
2.15.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

^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [PATCH v2 09/14] IB/srpt: Fix login-related race conditions
       [not found] ` <20180117001418.7852-1-bart.vanassche-Sjgp3cTcYWE@public.gmane.org>
                     ` (7 preceding siblings ...)
  2018-01-17  0:14   ` [PATCH v2 08/14] IB/srpt: Log all zero-length writes and completions Bart Van Assche
@ 2018-01-17  0:14   ` Bart Van Assche
  2018-01-17  0:14   ` [PATCH v2 10/14] IB/srpt: Fix a race condition related to wait list processing Bart Van Assche
                     ` (6 subsequent siblings)
  15 siblings, 0 replies; 24+ messages in thread
From: Bart Van Assche @ 2018-01-17  0:14 UTC (permalink / raw)
  To: Jason Gunthorpe
  Cc: Doug Ledford, linux-rdma-u79uwXL29TY76Z2rM5mHXA, Bart Van Assche

Make sure that sport->mutex is not released between the duplicate
channel check, adding a channel to the channel list and performing
the sport enabled check. Avoid that srpt_disconnect_ch() can be
invoked concurrently with the ib_send_cm_rep() call by
srpt_cm_req_recv().

Signed-off-by: Bart Van Assche <bart.vanassche-Sjgp3cTcYWE@public.gmane.org>
---
 drivers/infiniband/ulp/srpt/ib_srpt.c | 195 +++++++++++++++++++---------------
 1 file changed, 111 insertions(+), 84 deletions(-)

diff --git a/drivers/infiniband/ulp/srpt/ib_srpt.c b/drivers/infiniband/ulp/srpt/ib_srpt.c
index 866ff4be553c..6278c4448061 100644
--- a/drivers/infiniband/ulp/srpt/ib_srpt.c
+++ b/drivers/infiniband/ulp/srpt/ib_srpt.c
@@ -2040,7 +2040,7 @@ static int srpt_cm_req_recv(struct ib_cm_id *cm_id,
 	struct srpt_rdma_ch *ch;
 	char i_port_id[36];
 	u32 it_iu_len;
-	int i, ret = 0;
+	int i, ret;
 
 	WARN_ON_ONCE(irqs_disabled());
 
@@ -2063,69 +2063,43 @@ static int srpt_cm_req_recv(struct ib_cm_id *cm_id,
 		goto out;
 	}
 
+	ret = -ENOMEM;
 	rsp = kzalloc(sizeof(*rsp), GFP_KERNEL);
 	rej = kzalloc(sizeof(*rej), GFP_KERNEL);
 	rep_param = kzalloc(sizeof(*rep_param), GFP_KERNEL);
-
-	if (!rsp || !rej || !rep_param) {
-		ret = -ENOMEM;
+	if (!rsp || !rej || !rep_param)
 		goto out;
-	}
 
+	ret = -EINVAL;
 	if (it_iu_len > srp_max_req_size || it_iu_len < 64) {
 		rej->reason = cpu_to_be32(
-			      SRP_LOGIN_REJ_REQ_IT_IU_LENGTH_TOO_LARGE);
-		ret = -EINVAL;
-		pr_err("rejected SRP_LOGIN_REQ because its"
-		       " length (%d bytes) is out of range (%d .. %d)\n",
+				SRP_LOGIN_REJ_REQ_IT_IU_LENGTH_TOO_LARGE);
+		pr_err("rejected SRP_LOGIN_REQ because its length (%d bytes) is out of range (%d .. %d)\n",
 		       it_iu_len, 64, srp_max_req_size);
 		goto reject;
 	}
 
 	if (!sport->enabled) {
-		rej->reason = cpu_to_be32(
-			      SRP_LOGIN_REJ_INSUFFICIENT_RESOURCES);
-		ret = -EINVAL;
-		pr_err("rejected SRP_LOGIN_REQ because the target port"
-		       " has not yet been enabled\n");
+		rej->reason = cpu_to_be32(SRP_LOGIN_REJ_INSUFFICIENT_RESOURCES);
+		pr_info("rejected SRP_LOGIN_REQ because target port %s_%d has not yet been enabled\n",
+			sport->sdev->device->name, param->port);
 		goto reject;
 	}
 
-	if ((req->req_flags & SRP_MTCH_ACTION) == SRP_MULTICHAN_SINGLE) {
-		struct srpt_rdma_ch *ch2;
-
-		rsp->rsp_flags = SRP_LOGIN_RSP_MULTICHAN_NO_CHAN;
-
-		mutex_lock(&sport->mutex);
-		list_for_each_entry(ch2, &nexus->ch_list, list) {
-			if (srpt_disconnect_ch(ch2) < 0)
-				continue;
-			pr_info("Relogin - closed existing channel %s\n",
-				ch2->sess_name);
-			rsp->rsp_flags = SRP_LOGIN_RSP_MULTICHAN_TERMINATED;
-		}
-		mutex_unlock(&sport->mutex);
-	} else {
-		rsp->rsp_flags = SRP_LOGIN_RSP_MULTICHAN_MAINTAINED;
-	}
-
 	if (*(__be64 *)req->target_port_id != cpu_to_be64(srpt_service_guid)
 	    || *(__be64 *)(req->target_port_id + 8) !=
 	       cpu_to_be64(srpt_service_guid)) {
 		rej->reason = cpu_to_be32(
-			      SRP_LOGIN_REJ_UNABLE_ASSOCIATE_CHANNEL);
-		ret = -ENOMEM;
-		pr_err("rejected SRP_LOGIN_REQ because it"
-		       " has an invalid target port identifier.\n");
+				SRP_LOGIN_REJ_UNABLE_ASSOCIATE_CHANNEL);
+		pr_err("rejected SRP_LOGIN_REQ because it has an invalid target port identifier.\n");
 		goto reject;
 	}
 
+	ret = -ENOMEM;
 	ch = kzalloc(sizeof(*ch), GFP_KERNEL);
 	if (!ch) {
-		rej->reason = cpu_to_be32(
-			      SRP_LOGIN_REJ_INSUFFICIENT_RESOURCES);
-		pr_err("rejected SRP_LOGIN_REQ because no memory.\n");
-		ret = -ENOMEM;
+		rej->reason = cpu_to_be32(SRP_LOGIN_REJ_INSUFFICIENT_RESOURCES);
+		pr_err("rejected SRP_LOGIN_REQ because out of memory.\n");
 		goto reject;
 	}
 
@@ -2153,8 +2127,11 @@ static int srpt_cm_req_recv(struct ib_cm_id *cm_id,
 		srpt_alloc_ioctx_ring(ch->sport->sdev, ch->rq_size,
 				      sizeof(*ch->ioctx_ring[0]),
 				      ch->max_rsp_size, DMA_TO_DEVICE);
-	if (!ch->ioctx_ring)
+	if (!ch->ioctx_ring) {
+		pr_err("rejected SRP_LOGIN_REQ because creating a new QP SQ ring failed.\n");
+		rej->reason = cpu_to_be32(SRP_LOGIN_REJ_INSUFFICIENT_RESOURCES);
 		goto free_ch;
+	}
 
 	INIT_LIST_HEAD(&ch->free_list);
 	for (i = 0; i < ch->rq_size; i++) {
@@ -2176,20 +2153,10 @@ static int srpt_cm_req_recv(struct ib_cm_id *cm_id,
 	}
 
 	ret = srpt_create_ch_ib(ch);
-	if (ret) {
-		rej->reason = cpu_to_be32(
-			      SRP_LOGIN_REJ_INSUFFICIENT_RESOURCES);
-		pr_err("rejected SRP_LOGIN_REQ because creating"
-		       " a new RDMA channel failed.\n");
-		goto free_recv_ring;
-	}
-
-	ret = srpt_ch_qp_rtr(ch, ch->qp);
 	if (ret) {
 		rej->reason = cpu_to_be32(SRP_LOGIN_REJ_INSUFFICIENT_RESOURCES);
-		pr_err("rejected SRP_LOGIN_REQ because enabling"
-		       " RTR failed (error code = %d)\n", ret);
-		goto destroy_ib;
+		pr_err("rejected SRP_LOGIN_REQ because creating a new RDMA channel failed.\n");
+		goto free_recv_ring;
 	}
 
 	srpt_format_guid(ch->sess_name, sizeof(ch->sess_name),
@@ -2214,11 +2181,51 @@ static int srpt_cm_req_recv(struct ib_cm_id *cm_id,
 						TARGET_PROT_NORMAL,
 						i_port_id + 2, ch, NULL);
 	if (IS_ERR_OR_NULL(ch->sess)) {
-		pr_info("Rejected login because no ACL has been configured yet for initiator %s.\n",
-			ch->sess_name);
-		rej->reason = cpu_to_be32((PTR_ERR(ch->sess) == -ENOMEM) ?
+		ret = PTR_ERR(ch->sess);
+		pr_info("Rejected login for initiator %s: ret = %d.\n",
+			ch->sess_name, ret);
+		rej->reason = cpu_to_be32(ret == -ENOMEM ?
 				SRP_LOGIN_REJ_INSUFFICIENT_RESOURCES :
 				SRP_LOGIN_REJ_CHANNEL_LIMIT_REACHED);
+		goto reject;
+	}
+
+	mutex_lock(&sport->mutex);
+
+	if ((req->req_flags & SRP_MTCH_ACTION) == SRP_MULTICHAN_SINGLE) {
+		struct srpt_rdma_ch *ch2;
+
+		rsp->rsp_flags = SRP_LOGIN_RSP_MULTICHAN_NO_CHAN;
+
+		list_for_each_entry(ch2, &nexus->ch_list, list) {
+			if (srpt_disconnect_ch(ch2) < 0)
+				continue;
+			pr_info("Relogin - closed existing channel %s\n",
+				ch2->sess_name);
+			rsp->rsp_flags = SRP_LOGIN_RSP_MULTICHAN_TERMINATED;
+		}
+	} else {
+		rsp->rsp_flags = SRP_LOGIN_RSP_MULTICHAN_MAINTAINED;
+	}
+
+	list_add_tail_rcu(&ch->list, &nexus->ch_list);
+
+	if (!sport->enabled) {
+		rej->reason = cpu_to_be32(
+				SRP_LOGIN_REJ_INSUFFICIENT_RESOURCES);
+		pr_info("rejected SRP_LOGIN_REQ because target %s_%d is not enabled\n",
+			sdev->device->name, param->port);
+		mutex_unlock(&sport->mutex);
+		goto reject;
+	}
+
+	mutex_unlock(&sport->mutex);
+
+	ret = srpt_ch_qp_rtr(ch, ch->qp);
+	if (ret) {
+		rej->reason = cpu_to_be32(SRP_LOGIN_REJ_INSUFFICIENT_RESOURCES);
+		pr_err("rejected SRP_LOGIN_REQ because enabling RTR failed (error code = %d)\n",
+		       ret);
 		goto destroy_ib;
 	}
 
@@ -2231,8 +2238,8 @@ static int srpt_cm_req_recv(struct ib_cm_id *cm_id,
 	rsp->max_it_iu_len = req->req_it_iu_len;
 	rsp->max_ti_iu_len = req->req_it_iu_len;
 	ch->max_ti_iu_len = it_iu_len;
-	rsp->buf_fmt = cpu_to_be16(SRP_BUF_FORMAT_DIRECT
-				   | SRP_BUF_FORMAT_INDIRECT);
+	rsp->buf_fmt = cpu_to_be16(SRP_BUF_FORMAT_DIRECT |
+				   SRP_BUF_FORMAT_INDIRECT);
 	rsp->req_lim_delta = cpu_to_be32(ch->rq_size);
 	atomic_set(&ch->req_lim, ch->rq_size);
 	atomic_set(&ch->req_lim_delta, 0);
@@ -2248,24 +2255,30 @@ static int srpt_cm_req_recv(struct ib_cm_id *cm_id,
 	rep_param->responder_resources = 4;
 	rep_param->initiator_depth = 4;
 
-	ret = ib_send_cm_rep(cm_id, rep_param);
-	if (ret) {
-		pr_err("sending SRP_LOGIN_REQ response failed"
-		       " (error code = %d)\n", ret);
-		goto release_channel;
-	}
-
+	/*
+	 * Hold the sport mutex while accepting a connection to avoid that
+	 * srpt_disconnect_ch() is invoked concurrently with this code.
+	 */
 	mutex_lock(&sport->mutex);
-	list_add_tail_rcu(&ch->list, &nexus->ch_list);
+	if (sport->enabled && ch->state == CH_CONNECTING)
+		ret = ib_send_cm_rep(cm_id, rep_param);
+	else
+		ret = -EINVAL;
 	mutex_unlock(&sport->mutex);
 
-	goto out;
+	switch (ret) {
+	case 0:
+		break;
+	case -EINVAL:
+		goto reject;
+	default:
+		rej->reason = cpu_to_be32(SRP_LOGIN_REJ_INSUFFICIENT_RESOURCES);
+		pr_err("sending SRP_LOGIN_REQ response failed (error code = %d)\n",
+		       ret);
+		goto reject;
+	}
 
-release_channel:
-	srpt_disconnect_ch(ch);
-	transport_deregister_session_configfs(ch->sess);
-	transport_deregister_session(ch->sess);
-	ch->sess = NULL;
+	goto out;
 
 destroy_ib:
 	srpt_destroy_ch_ib(ch);
@@ -2280,13 +2293,18 @@ static int srpt_cm_req_recv(struct ib_cm_id *cm_id,
 			     ch->sport->sdev, ch->rq_size,
 			     ch->max_rsp_size, DMA_TO_DEVICE);
 free_ch:
+	cm_id->context = NULL;
 	kfree(ch);
+	ch = NULL;
+
+	WARN_ON_ONCE(ret == 0);
 
 reject:
+	pr_info("Rejecting login with reason %#x\n", be32_to_cpu(rej->reason));
 	rej->opcode = SRP_LOGIN_REJ;
 	rej->tag = req->tag;
-	rej->buf_fmt = cpu_to_be16(SRP_BUF_FORMAT_DIRECT
-				   | SRP_BUF_FORMAT_INDIRECT);
+	rej->buf_fmt = cpu_to_be16(SRP_BUF_FORMAT_DIRECT |
+				   SRP_BUF_FORMAT_INDIRECT);
 
 	ib_send_cm_rej(cm_id, IB_CM_REJ_CONSUMER_DEFINED, NULL, 0,
 			     (void *)rej, sizeof(*rej));
@@ -2329,17 +2347,26 @@ static void srpt_cm_rtu_recv(struct srpt_rdma_ch *ch)
 {
 	int ret;
 
-	if (srpt_set_ch_state(ch, CH_LIVE)) {
-		ret = srpt_ch_qp_rts(ch, ch->qp);
-
-		if (ret == 0) {
-			/* Trigger wait list processing. */
-			ret = srpt_zerolength_write(ch);
-			WARN_ONCE(ret < 0, "%d\n", ret);
-		} else {
-			srpt_close_ch(ch);
-		}
+	ret = srpt_ch_qp_rts(ch, ch->qp);
+	if (ret < 0) {
+		pr_err("%s-%d: QP transition to RTS failed\n", ch->sess_name,
+		       ch->qp->qp_num);
+		srpt_close_ch(ch);
+		return;
 	}
+
+	/* Trigger wait list processing. */
+	ret = srpt_zerolength_write(ch);
+	WARN_ONCE(ret < 0, "%d\n", ret);
+
+	/*
+	 * Note: calling srpt_close_ch() if the transition to the LIVE state
+	 * fails is not necessary since that means that that function has
+	 * already been invoked from another thread.
+	 */
+	if (!srpt_set_ch_state(ch, CH_LIVE))
+		pr_err("%s-%d: channel transition to LIVE state failed\n",
+		       ch->sess_name, ch->qp->qp_num);
 }
 
 /**
-- 
2.15.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

^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [PATCH v2 10/14] IB/srpt: Fix a race condition related to wait list processing
       [not found] ` <20180117001418.7852-1-bart.vanassche-Sjgp3cTcYWE@public.gmane.org>
                     ` (8 preceding siblings ...)
  2018-01-17  0:14   ` [PATCH v2 09/14] IB/srpt: Fix login-related race conditions Bart Van Assche
@ 2018-01-17  0:14   ` Bart Van Assche
  2018-01-17  0:14   ` [PATCH v2 11/14] IB/srpt: Avoid that wait list processing triggers command reordering Bart Van Assche
                     ` (5 subsequent siblings)
  15 siblings, 0 replies; 24+ messages in thread
From: Bart Van Assche @ 2018-01-17  0:14 UTC (permalink / raw)
  To: Jason Gunthorpe
  Cc: Doug Ledford, linux-rdma-u79uwXL29TY76Z2rM5mHXA, Bart Van Assche

Wait list processing only occurs if the channel state >= CH_LIVE. Hence
set the channel state to CH_LIVE before triggering wait list processing
asynchronously.

Signed-off-by: Bart Van Assche <bart.vanassche-Sjgp3cTcYWE@public.gmane.org>
---
 drivers/infiniband/ulp/srpt/ib_srpt.c | 12 +++++++-----
 1 file changed, 7 insertions(+), 5 deletions(-)

diff --git a/drivers/infiniband/ulp/srpt/ib_srpt.c b/drivers/infiniband/ulp/srpt/ib_srpt.c
index 6278c4448061..372f1eb2fa49 100644
--- a/drivers/infiniband/ulp/srpt/ib_srpt.c
+++ b/drivers/infiniband/ulp/srpt/ib_srpt.c
@@ -2355,18 +2355,20 @@ static void srpt_cm_rtu_recv(struct srpt_rdma_ch *ch)
 		return;
 	}
 
-	/* Trigger wait list processing. */
-	ret = srpt_zerolength_write(ch);
-	WARN_ONCE(ret < 0, "%d\n", ret);
-
 	/*
 	 * Note: calling srpt_close_ch() if the transition to the LIVE state
 	 * fails is not necessary since that means that that function has
 	 * already been invoked from another thread.
 	 */
-	if (!srpt_set_ch_state(ch, CH_LIVE))
+	if (!srpt_set_ch_state(ch, CH_LIVE)) {
 		pr_err("%s-%d: channel transition to LIVE state failed\n",
 		       ch->sess_name, ch->qp->qp_num);
+		return;
+	}
+
+	/* Trigger wait list processing. */
+	ret = srpt_zerolength_write(ch);
+	WARN_ONCE(ret < 0, "%d\n", ret);
 }
 
 /**
-- 
2.15.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

^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [PATCH v2 11/14] IB/srpt: Avoid that wait list processing triggers command reordering
       [not found] ` <20180117001418.7852-1-bart.vanassche-Sjgp3cTcYWE@public.gmane.org>
                     ` (9 preceding siblings ...)
  2018-01-17  0:14   ` [PATCH v2 10/14] IB/srpt: Fix a race condition related to wait list processing Bart Van Assche
@ 2018-01-17  0:14   ` Bart Van Assche
  2018-01-17  0:14   ` [PATCH v2 12/14] IB/srpt: Prepare RDMA/CM support Bart Van Assche
                     ` (4 subsequent siblings)
  15 siblings, 0 replies; 24+ messages in thread
From: Bart Van Assche @ 2018-01-17  0:14 UTC (permalink / raw)
  To: Jason Gunthorpe
  Cc: Doug Ledford, linux-rdma-u79uwXL29TY76Z2rM5mHXA, Bart Van Assche

If a receive I/O context is removed from the wait list and
srpt_handle_new_iu() fails to allocate a send I/O context then
re-adding the receive I/O context to the wait list can cause
reordering. Avoid this by only removing a receive I/O context
from the wait list after allocating a send I/O context succeeded.

Signed-off-by: Bart Van Assche <bart.vanassche-Sjgp3cTcYWE@public.gmane.org>
---
 drivers/infiniband/ulp/srpt/ib_srpt.c | 82 ++++++++++++++++++++---------------
 drivers/infiniband/ulp/srpt/ib_srpt.h |  2 +
 2 files changed, 49 insertions(+), 35 deletions(-)

diff --git a/drivers/infiniband/ulp/srpt/ib_srpt.c b/drivers/infiniband/ulp/srpt/ib_srpt.c
index 372f1eb2fa49..5b2e74b1d808 100644
--- a/drivers/infiniband/ulp/srpt/ib_srpt.c
+++ b/drivers/infiniband/ulp/srpt/ib_srpt.c
@@ -1533,39 +1533,39 @@ static void srpt_handle_tsk_mgmt(struct srpt_rdma_ch *ch,
  * srpt_handle_new_iu - process a newly received information unit
  * @ch:    RDMA channel through which the information unit has been received.
  * @recv_ioctx: Receive I/O context associated with the information unit.
- * @send_ioctx: Send I/O context.
  */
-static void srpt_handle_new_iu(struct srpt_rdma_ch *ch,
-			       struct srpt_recv_ioctx *recv_ioctx,
-			       struct srpt_send_ioctx *send_ioctx)
+static bool
+srpt_handle_new_iu(struct srpt_rdma_ch *ch, struct srpt_recv_ioctx *recv_ioctx)
 {
+	struct srpt_send_ioctx *send_ioctx = NULL;
 	struct srp_cmd *srp_cmd;
+	bool res = false;
+	u8 opcode;
 
 	BUG_ON(!ch);
 	BUG_ON(!recv_ioctx);
 
+	if (unlikely(ch->state == CH_CONNECTING))
+		goto push;
+
 	ib_dma_sync_single_for_cpu(ch->sport->sdev->device,
 				   recv_ioctx->ioctx.dma, srp_max_req_size,
 				   DMA_FROM_DEVICE);
 
-	if (unlikely(ch->state == CH_CONNECTING))
-		goto out_wait;
-
-	if (unlikely(ch->state != CH_LIVE))
-		return;
-
 	srp_cmd = recv_ioctx->ioctx.buf;
-	if (srp_cmd->opcode == SRP_CMD || srp_cmd->opcode == SRP_TSK_MGMT) {
-		if (!send_ioctx) {
-			if (!list_empty(&ch->cmd_wait_list))
-				goto out_wait;
-			send_ioctx = srpt_get_send_ioctx(ch);
-		}
+	opcode = srp_cmd->opcode;
+	if (opcode == SRP_CMD || opcode == SRP_TSK_MGMT) {
+		send_ioctx = srpt_get_send_ioctx(ch);
 		if (unlikely(!send_ioctx))
-			goto out_wait;
+			goto push;
 	}
 
-	switch (srp_cmd->opcode) {
+	if (!list_empty(&recv_ioctx->wait_list)) {
+		WARN_ON_ONCE(!ch->processing_wait_list);
+		list_del_init(&recv_ioctx->wait_list);
+	}
+
+	switch (opcode) {
 	case SRP_CMD:
 		srpt_handle_cmd(ch, recv_ioctx, send_ioctx);
 		break;
@@ -1585,16 +1585,22 @@ static void srpt_handle_new_iu(struct srpt_rdma_ch *ch,
 		pr_err("Received SRP_RSP\n");
 		break;
 	default:
-		pr_err("received IU with unknown opcode 0x%x\n",
-		       srp_cmd->opcode);
+		pr_err("received IU with unknown opcode 0x%x\n", opcode);
 		break;
 	}
 
 	srpt_post_recv(ch->sport->sdev, ch, recv_ioctx);
-	return;
+	res = true;
 
-out_wait:
-	list_add_tail(&recv_ioctx->wait_list, &ch->cmd_wait_list);
+out:
+	return res;
+
+push:
+	if (list_empty(&recv_ioctx->wait_list)) {
+		WARN_ON_ONCE(ch->processing_wait_list);
+		list_add_tail(&recv_ioctx->wait_list, &ch->cmd_wait_list);
+	}
+	goto out;
 }
 
 static void srpt_recv_done(struct ib_cq *cq, struct ib_wc *wc)
@@ -1609,7 +1615,7 @@ static void srpt_recv_done(struct ib_cq *cq, struct ib_wc *wc)
 		req_lim = atomic_dec_return(&ch->req_lim);
 		if (unlikely(req_lim < 0))
 			pr_err("req_lim = %d < 0\n", req_lim);
-		srpt_handle_new_iu(ch, ioctx, NULL);
+		srpt_handle_new_iu(ch, ioctx);
 	} else {
 		pr_info_ratelimited("receiving failed for ioctx %p with status %d\n",
 				    ioctx, wc->status);
@@ -1623,19 +1629,21 @@ static void srpt_recv_done(struct ib_cq *cq, struct ib_wc *wc)
  */
 static void srpt_process_wait_list(struct srpt_rdma_ch *ch)
 {
-	struct srpt_send_ioctx *ioctx;
+	struct srpt_recv_ioctx *recv_ioctx, *tmp;
 
-	while (!list_empty(&ch->cmd_wait_list) &&
-	       ch->state >= CH_LIVE &&
-	       (ioctx = srpt_get_send_ioctx(ch)) != NULL) {
-		struct srpt_recv_ioctx *recv_ioctx;
+	WARN_ON_ONCE(ch->state == CH_CONNECTING);
 
-		recv_ioctx = list_first_entry(&ch->cmd_wait_list,
-					      struct srpt_recv_ioctx,
-					      wait_list);
-		list_del(&recv_ioctx->wait_list);
-		srpt_handle_new_iu(ch, recv_ioctx, ioctx);
+	if (list_empty(&ch->cmd_wait_list))
+		return;
+
+	WARN_ON_ONCE(ch->processing_wait_list);
+	ch->processing_wait_list = true;
+	list_for_each_entry_safe(recv_ioctx, tmp, &ch->cmd_wait_list,
+				 wait_list) {
+		if (!srpt_handle_new_iu(ch, recv_ioctx))
+			break;
 	}
+	ch->processing_wait_list = false;
 }
 
 /**
@@ -2150,6 +2158,8 @@ static int srpt_cm_req_recv(struct ib_cm_id *cm_id,
 			    cpu_to_be32(SRP_LOGIN_REJ_INSUFFICIENT_RESOURCES);
 			goto free_ring;
 		}
+		for (i = 0; i < ch->rq_size; i++)
+			INIT_LIST_HEAD(&ch->ioctx_recv_ring[i]->wait_list);
 	}
 
 	ret = srpt_create_ch_ib(ch);
@@ -2773,8 +2783,10 @@ static int srpt_alloc_srq(struct srpt_device *sdev)
 	sdev->use_srq = true;
 	sdev->srq = srq;
 
-	for (i = 0; i < sdev->srq_size; ++i)
+	for (i = 0; i < sdev->srq_size; ++i) {
+		INIT_LIST_HEAD(&sdev->ioctx_ring[i]->wait_list);
 		srpt_post_recv(sdev, NULL, sdev->ioctx_ring[i]);
+	}
 
 	return 0;
 }
diff --git a/drivers/infiniband/ulp/srpt/ib_srpt.h b/drivers/infiniband/ulp/srpt/ib_srpt.h
index 59261d5de292..10f9b2667ef2 100644
--- a/drivers/infiniband/ulp/srpt/ib_srpt.h
+++ b/drivers/infiniband/ulp/srpt/ib_srpt.h
@@ -261,6 +261,7 @@ enum rdma_ch_state {
  * @spinlock:      Protects free_list and state.
  * @free_list:     Head of list with free send I/O contexts.
  * @state:         channel state. See also enum rdma_ch_state.
+ * @processing_wait_list: Whether or not cmd_wait_list is being processed.
  * @ioctx_ring:    Send ring.
  * @ioctx_recv_ring: Receive I/O context ring.
  * @list:          Node in srpt_nexus.ch_list.
@@ -295,6 +296,7 @@ struct srpt_rdma_ch {
 	struct list_head	list;
 	struct list_head	cmd_wait_list;
 	uint16_t		pkey;
+	bool			processing_wait_list;
 	struct se_session	*sess;
 	u8			sess_name[24];
 	struct work_struct	release_work;
-- 
2.15.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

^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [PATCH v2 12/14] IB/srpt: Prepare RDMA/CM support
       [not found] ` <20180117001418.7852-1-bart.vanassche-Sjgp3cTcYWE@public.gmane.org>
                     ` (10 preceding siblings ...)
  2018-01-17  0:14   ` [PATCH v2 11/14] IB/srpt: Avoid that wait list processing triggers command reordering Bart Van Assche
@ 2018-01-17  0:14   ` Bart Van Assche
  2018-01-17  0:14   ` [PATCH v2 13/14] IB/srpt: Move the code for parsing struct ib_cm_req_event_param Bart Van Assche
                     ` (3 subsequent siblings)
  15 siblings, 0 replies; 24+ messages in thread
From: Bart Van Assche @ 2018-01-17  0:14 UTC (permalink / raw)
  To: Jason Gunthorpe
  Cc: Doug Ledford, linux-rdma-u79uwXL29TY76Z2rM5mHXA, Bart Van Assche

Introduce a union in struct srpt_rdma_ch for member variables that
depend on the type of connection manager. Avoid that error messages
report the IB/CM ID.

Signed-off-by: Bart Van Assche <bart.vanassche-Sjgp3cTcYWE@public.gmane.org>
---
 drivers/infiniband/ulp/srpt/ib_srpt.c | 32 ++++++++++++++++----------------
 drivers/infiniband/ulp/srpt/ib_srpt.h |  8 ++++++--
 2 files changed, 22 insertions(+), 18 deletions(-)

diff --git a/drivers/infiniband/ulp/srpt/ib_srpt.c b/drivers/infiniband/ulp/srpt/ib_srpt.c
index 5b2e74b1d808..f066fac19f13 100644
--- a/drivers/infiniband/ulp/srpt/ib_srpt.c
+++ b/drivers/infiniband/ulp/srpt/ib_srpt.c
@@ -215,12 +215,12 @@ static const char *get_ch_state_name(enum rdma_ch_state s)
  */
 static void srpt_qp_event(struct ib_event *event, struct srpt_rdma_ch *ch)
 {
-	pr_debug("QP event %d on cm_id=%p sess_name=%s state=%d\n",
-		 event->event, ch->cm_id, ch->sess_name, ch->state);
+	pr_debug("QP event %d on ch=%p sess_name=%s state=%d\n",
+		 event->event, ch, ch->sess_name, ch->state);
 
 	switch (event->event) {
 	case IB_EVENT_COMM_EST:
-		ib_cm_notify(ch->cm_id, event->event);
+		ib_cm_notify(ch->ib_cm.cm_id, event->event);
 		break;
 	case IB_EVENT_QP_LAST_WQE_REACHED:
 		pr_debug("%s-%d, state %s: received Last WQE event.\n",
@@ -1097,7 +1097,7 @@ static int srpt_ch_qp_rtr(struct srpt_rdma_ch *ch, struct ib_qp *qp)
 	int ret;
 
 	qp_attr.qp_state = IB_QPS_RTR;
-	ret = ib_cm_init_qp_attr(ch->cm_id, &qp_attr, &attr_mask);
+	ret = ib_cm_init_qp_attr(ch->ib_cm.cm_id, &qp_attr, &attr_mask);
 	if (ret)
 		goto out;
 
@@ -1127,7 +1127,7 @@ static int srpt_ch_qp_rts(struct srpt_rdma_ch *ch, struct ib_qp *qp)
 	int ret;
 
 	qp_attr.qp_state = IB_QPS_RTS;
-	ret = ib_cm_init_qp_attr(ch->cm_id, &qp_attr, &attr_mask);
+	ret = ib_cm_init_qp_attr(ch->ib_cm.cm_id, &qp_attr, &attr_mask);
 	if (ret)
 		goto out;
 
@@ -1509,9 +1509,9 @@ static void srpt_handle_tsk_mgmt(struct srpt_rdma_ch *ch,
 	srp_tsk = recv_ioctx->ioctx.buf;
 	cmd = &send_ioctx->cmd;
 
-	pr_debug("recv tsk_mgmt fn %d for task_tag %lld and cmd tag %lld"
-		 " cm_id %p sess %p\n", srp_tsk->tsk_mgmt_func,
-		 srp_tsk->task_tag, srp_tsk->tag, ch->cm_id, ch->sess);
+	pr_debug("recv tsk_mgmt fn %d for task_tag %lld and cmd tag %lld ch %p sess %p\n",
+		 srp_tsk->tsk_mgmt_func, srp_tsk->task_tag, srp_tsk->tag, ch,
+		 ch->sess);
 
 	srpt_set_cmd_state(send_ioctx, SRPT_STATE_MGMT);
 	send_ioctx->cmd.tag = srp_tsk->tag;
@@ -1762,9 +1762,9 @@ static int srpt_create_ch_ib(struct srpt_rdma_ch *ch)
 
 	atomic_set(&ch->sq_wr_avail, qp_init->cap.max_send_wr);
 
-	pr_debug("%s: max_cqe= %d max_sge= %d sq_size = %d cm_id= %p\n",
+	pr_debug("%s: max_cqe= %d max_sge= %d sq_size = %d ch= %p\n",
 		 __func__, ch->cq->cqe, qp_init->cap.max_send_sge,
-		 qp_init->cap.max_send_wr, ch->cm_id);
+		 qp_init->cap.max_send_wr, ch);
 
 	ret = srpt_init_ch_qp(ch, ch->qp);
 	if (ret)
@@ -1849,9 +1849,9 @@ static int srpt_disconnect_ch(struct srpt_rdma_ch *ch)
 	if (!srpt_set_ch_state(ch, CH_DISCONNECTING))
 		return -ENOTCONN;
 
-	ret = ib_send_cm_dreq(ch->cm_id, NULL, 0);
+	ret = ib_send_cm_dreq(ch->ib_cm.cm_id, NULL, 0);
 	if (ret < 0)
-		ret = ib_send_cm_drep(ch->cm_id, NULL, 0);
+		ret = ib_send_cm_drep(ch->ib_cm.cm_id, NULL, 0);
 
 	if (ret < 0 && srpt_close_ch(ch))
 		ret = 0;
@@ -2003,7 +2003,7 @@ static void srpt_release_channel_work(struct work_struct *w)
 	transport_deregister_session(se_sess);
 	ch->sess = NULL;
 
-	ib_destroy_cm_id(ch->cm_id);
+	ib_destroy_cm_id(ch->ib_cm.cm_id);
 
 	srpt_destroy_ch_ib(ch);
 
@@ -2118,7 +2118,7 @@ static int srpt_cm_req_recv(struct ib_cm_id *cm_id,
 	ch->zw_cqe.done = srpt_zerolength_write_done;
 	INIT_WORK(&ch->release_work, srpt_release_channel_work);
 	ch->sport = &sdev->port[param->port - 1];
-	ch->cm_id = cm_id;
+	ch->ib_cm.cm_id = cm_id;
 	cm_id->context = ch;
 	/*
 	 * ch->rq_size should be at least as large as the initiator queue
@@ -2239,8 +2239,8 @@ static int srpt_cm_req_recv(struct ib_cm_id *cm_id,
 		goto destroy_ib;
 	}
 
-	pr_debug("Establish connection sess=%p name=%s cm_id=%p\n", ch->sess,
-		 ch->sess_name, ch->cm_id);
+	pr_debug("Establish connection sess=%p name=%s ch=%p\n", ch->sess,
+		 ch->sess_name, ch);
 
 	/* create srp_login_response */
 	rsp->opcode = SRP_LOGIN_RSP;
diff --git a/drivers/infiniband/ulp/srpt/ib_srpt.h b/drivers/infiniband/ulp/srpt/ib_srpt.h
index 10f9b2667ef2..4d9199fd00dc 100644
--- a/drivers/infiniband/ulp/srpt/ib_srpt.h
+++ b/drivers/infiniband/ulp/srpt/ib_srpt.h
@@ -243,8 +243,8 @@ enum rdma_ch_state {
 /**
  * struct srpt_rdma_ch - RDMA channel
  * @nexus:         I_T nexus this channel is associated with.
- * @cm_id:         IB CM ID associated with the channel.
  * @qp:            IB queue pair used for communicating over this channel.
+ * @cm_id:         IB CM ID associated with the channel.
  * @cq:            IB completion queue for this channel.
  * @zw_cqe:	   Zero-length write CQE.
  * @rcu:           RCU head.
@@ -275,8 +275,12 @@ enum rdma_ch_state {
  */
 struct srpt_rdma_ch {
 	struct srpt_nexus	*nexus;
-	struct ib_cm_id		*cm_id;
 	struct ib_qp		*qp;
+	union {
+		struct {
+			struct ib_cm_id		*cm_id;
+		} ib_cm;
+	};
 	struct ib_cq		*cq;
 	struct ib_cqe		zw_cqe;
 	struct rcu_head		rcu;
-- 
2.15.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

^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [PATCH v2 13/14] IB/srpt: Move the code for parsing struct ib_cm_req_event_param
       [not found] ` <20180117001418.7852-1-bart.vanassche-Sjgp3cTcYWE@public.gmane.org>
                     ` (11 preceding siblings ...)
  2018-01-17  0:14   ` [PATCH v2 12/14] IB/srpt: Prepare RDMA/CM support Bart Van Assche
@ 2018-01-17  0:14   ` Bart Van Assche
  2018-01-17  0:14   ` [PATCH v2 14/14] IB/srpt: Add RDMA/CM support Bart Van Assche
                     ` (2 subsequent siblings)
  15 siblings, 0 replies; 24+ messages in thread
From: Bart Van Assche @ 2018-01-17  0:14 UTC (permalink / raw)
  To: Jason Gunthorpe
  Cc: Doug Ledford, linux-rdma-u79uwXL29TY76Z2rM5mHXA, Bart Van Assche

This patch does not change any functionality but makes srpt_cm_req_recv()
independent of the IB/CM and hence simplifies the patch that introduces
RDMA/CM support.

Signed-off-by: Bart Van Assche <bart.vanassche-Sjgp3cTcYWE@public.gmane.org>
---
 drivers/infiniband/ulp/srpt/ib_srpt.c | 49 +++++++++++++++++++++--------------
 1 file changed, 30 insertions(+), 19 deletions(-)

diff --git a/drivers/infiniband/ulp/srpt/ib_srpt.c b/drivers/infiniband/ulp/srpt/ib_srpt.c
index f066fac19f13..bf37816a1b12 100644
--- a/drivers/infiniband/ulp/srpt/ib_srpt.c
+++ b/drivers/infiniband/ulp/srpt/ib_srpt.c
@@ -2028,20 +2028,22 @@ static void srpt_release_channel_work(struct work_struct *w)
 /**
  * srpt_cm_req_recv - process the event IB_CM_REQ_RECEIVED
  * @cm_id: IB/CM connection identifier.
- * @param: IB/CM REQ parameters.
- * @private_data: IB/CM REQ private data.
+ * @port_num: Port through which the IB/CM REQ message was received.
+ * @pkey: P_Key of the incoming connection.
+ * @req: SRP login request.
+ * @src_addr: GID of the port that submitted the login request.
  *
  * Ownership of the cm_id is transferred to the target session if this
  * functions returns zero. Otherwise the caller remains the owner of cm_id.
  */
 static int srpt_cm_req_recv(struct ib_cm_id *cm_id,
-			    struct ib_cm_req_event_param *param,
-			    void *private_data)
+			    u8 port_num, __be16 pkey,
+			    const struct srp_login_req *req,
+			    const char *src_addr)
 {
 	struct srpt_device *sdev = cm_id->context;
-	struct srpt_port *sport = &sdev->port[param->port - 1];
+	struct srpt_port *sport = &sdev->port[port_num - 1];
 	struct srpt_nexus *nexus;
-	struct srp_login_req *req;
 	struct srp_login_rsp *rsp = NULL;
 	struct srp_login_rej *rej = NULL;
 	struct ib_cm_rep_param *rep_param = NULL;
@@ -2052,17 +2054,14 @@ static int srpt_cm_req_recv(struct ib_cm_id *cm_id,
 
 	WARN_ON_ONCE(irqs_disabled());
 
-	if (WARN_ON(!sdev || !private_data))
+	if (WARN_ON(!sdev || !req))
 		return -EINVAL;
 
-	req = (struct srp_login_req *)private_data;
-
 	it_iu_len = be32_to_cpu(req->req_it_iu_len);
 
 	pr_info("Received SRP_LOGIN_REQ with i_port_id %pI6, t_port_id %pI6 and it_iu_len %d on port %d (guid=%pI6); pkey %#04x\n",
 		req->initiator_port_id, req->target_port_id, it_iu_len,
-		param->port, &sport->gid,
-		be16_to_cpu(param->primary_path->pkey));
+		port_num, &sport->gid, be16_to_cpu(pkey));
 
 	nexus = srpt_get_nexus(sport, req->initiator_port_id,
 			       req->target_port_id);
@@ -2090,7 +2089,7 @@ static int srpt_cm_req_recv(struct ib_cm_id *cm_id,
 	if (!sport->enabled) {
 		rej->reason = cpu_to_be32(SRP_LOGIN_REJ_INSUFFICIENT_RESOURCES);
 		pr_info("rejected SRP_LOGIN_REQ because target port %s_%d has not yet been enabled\n",
-			sport->sdev->device->name, param->port);
+			sport->sdev->device->name, port_num);
 		goto reject;
 	}
 
@@ -2113,11 +2112,11 @@ static int srpt_cm_req_recv(struct ib_cm_id *cm_id,
 
 	init_rcu_head(&ch->rcu);
 	kref_init(&ch->kref);
-	ch->pkey = be16_to_cpu(param->primary_path->pkey);
+	ch->pkey = be16_to_cpu(pkey);
 	ch->nexus = nexus;
 	ch->zw_cqe.done = srpt_zerolength_write_done;
 	INIT_WORK(&ch->release_work, srpt_release_channel_work);
-	ch->sport = &sdev->port[param->port - 1];
+	ch->sport = sport;
 	ch->ib_cm.cm_id = cm_id;
 	cm_id->context = ch;
 	/*
@@ -2169,8 +2168,7 @@ static int srpt_cm_req_recv(struct ib_cm_id *cm_id,
 		goto free_recv_ring;
 	}
 
-	srpt_format_guid(ch->sess_name, sizeof(ch->sess_name),
-			 &param->primary_path->dgid.global.interface_id);
+	strlcpy(ch->sess_name, src_addr, sizeof(ch->sess_name));
 	snprintf(i_port_id, sizeof(i_port_id), "0x%016llx%016llx",
 			be64_to_cpu(*(__be64 *)nexus->i_port_id),
 			be64_to_cpu(*(__be64 *)(nexus->i_port_id + 8)));
@@ -2224,7 +2222,7 @@ static int srpt_cm_req_recv(struct ib_cm_id *cm_id,
 		rej->reason = cpu_to_be32(
 				SRP_LOGIN_REJ_INSUFFICIENT_RESOURCES);
 		pr_info("rejected SRP_LOGIN_REQ because target %s_%d is not enabled\n",
-			sdev->device->name, param->port);
+			sdev->device->name, port_num);
 		mutex_unlock(&sport->mutex);
 		goto reject;
 	}
@@ -2327,6 +2325,19 @@ static int srpt_cm_req_recv(struct ib_cm_id *cm_id,
 	return ret;
 }
 
+static int srpt_ib_cm_req_recv(struct ib_cm_id *cm_id,
+			       struct ib_cm_req_event_param *param,
+			       void *private_data)
+{
+	char sguid[40];
+
+	srpt_format_guid(sguid, sizeof(sguid),
+			 &param->primary_path->dgid.global.interface_id);
+
+	return srpt_cm_req_recv(cm_id, param->port, param->primary_path->pkey,
+				private_data, sguid);
+}
+
 static void srpt_cm_rej_recv(struct srpt_rdma_ch *ch,
 			     enum ib_cm_rej_reason reason,
 			     const u8 *private_data,
@@ -2401,8 +2412,8 @@ static int srpt_cm_handler(struct ib_cm_id *cm_id, struct ib_cm_event *event)
 	ret = 0;
 	switch (event->event) {
 	case IB_CM_REQ_RECEIVED:
-		ret = srpt_cm_req_recv(cm_id, &event->param.req_rcvd,
-				       event->private_data);
+		ret = srpt_ib_cm_req_recv(cm_id, &event->param.req_rcvd,
+					  event->private_data);
 		break;
 	case IB_CM_REJ_RECEIVED:
 		srpt_cm_rej_recv(ch, event->param.rej_rcvd.reason,
-- 
2.15.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

^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [PATCH v2 14/14] IB/srpt: Add RDMA/CM support
       [not found] ` <20180117001418.7852-1-bart.vanassche-Sjgp3cTcYWE@public.gmane.org>
                     ` (12 preceding siblings ...)
  2018-01-17  0:14   ` [PATCH v2 13/14] IB/srpt: Move the code for parsing struct ib_cm_req_event_param Bart Van Assche
@ 2018-01-17  0:14   ` Bart Van Assche
  2018-01-17 23:14   ` [PATCH v2 00/14] " Doug Ledford
  2018-01-17 23:33   ` Doug Ledford
  15 siblings, 0 replies; 24+ messages in thread
From: Bart Van Assche @ 2018-01-17  0:14 UTC (permalink / raw)
  To: Jason Gunthorpe
  Cc: Doug Ledford, linux-rdma-u79uwXL29TY76Z2rM5mHXA, Bart Van Assche

Add a kernel module parameter for configuring the port on which the
ib_srpt driver listens for incoming RDMA/CM connections. The default
value for this kernel module parameter is 0 which means "do not listen
for incoming RDMA/CM connections". Add RDMA/CM support to all code
that handles connection state changes. Modify srpt_init_nodeacl()
such that ACLs can be configured for IPv4 addresses.

Signed-off-by: Bart Van Assche <bart.vanassche-Sjgp3cTcYWE@public.gmane.org>
---
 drivers/infiniband/ulp/srpt/ib_srpt.c | 335 +++++++++++++++++++++++++++-------
 drivers/infiniband/ulp/srpt/ib_srpt.h |   6 +
 2 files changed, 279 insertions(+), 62 deletions(-)

diff --git a/drivers/infiniband/ulp/srpt/ib_srpt.c b/drivers/infiniband/ulp/srpt/ib_srpt.c
index bf37816a1b12..5ccc75c389e2 100644
--- a/drivers/infiniband/ulp/srpt/ib_srpt.c
+++ b/drivers/infiniband/ulp/srpt/ib_srpt.c
@@ -41,6 +41,7 @@
 #include <linux/string.h>
 #include <linux/delay.h>
 #include <linux/atomic.h>
+#include <linux/inet.h>
 #include <rdma/ib_cache.h>
 #include <scsi/scsi_proto.h>
 #include <scsi/scsi_tcq.h>
@@ -71,6 +72,10 @@ static u64 srpt_service_guid;
 static DEFINE_SPINLOCK(srpt_dev_lock);	/* Protects srpt_dev_list. */
 static LIST_HEAD(srpt_dev_list);	/* List of srpt_device structures. */
 
+static u16 rdma_cm_port;
+module_param(rdma_cm_port, short, 0444);
+MODULE_PARM_DESC(rdma_cm_port, "Port number RDMA/CM will bind to.");
+
 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,
@@ -92,6 +97,7 @@ MODULE_PARM_DESC(srpt_service_guid,
 		 " instead of using the node_guid of the first HCA.");
 
 static struct ib_client srpt_client;
+static struct rdma_cm_id *rdma_cm_id;
 static void srpt_release_cmd(struct se_cmd *se_cmd);
 static void srpt_free_ch(struct kref *kref);
 static int srpt_queue_status(struct se_cmd *cmd);
@@ -220,7 +226,10 @@ static void srpt_qp_event(struct ib_event *event, struct srpt_rdma_ch *ch)
 
 	switch (event->event) {
 	case IB_EVENT_COMM_EST:
-		ib_cm_notify(ch->ib_cm.cm_id, event->event);
+		if (ch->using_rdma_cm)
+			rdma_notify(ch->rdma_cm.cm_id, event->event);
+		else
+			ib_cm_notify(ch->ib_cm.cm_id, event->event);
 		break;
 	case IB_EVENT_QP_LAST_WQE_REACHED:
 		pr_debug("%s-%d, state %s: received Last WQE event.\n",
@@ -1057,6 +1066,8 @@ static int srpt_init_ch_qp(struct srpt_rdma_ch *ch, struct ib_qp *qp)
 	struct ib_qp_attr *attr;
 	int ret;
 
+	WARN_ON_ONCE(ch->using_rdma_cm);
+
 	attr = kzalloc(sizeof(*attr), GFP_KERNEL);
 	if (!attr)
 		return -ENOMEM;
@@ -1096,6 +1107,8 @@ static int srpt_ch_qp_rtr(struct srpt_rdma_ch *ch, struct ib_qp *qp)
 	int attr_mask;
 	int ret;
 
+	WARN_ON_ONCE(ch->using_rdma_cm);
+
 	qp_attr.qp_state = IB_QPS_RTR;
 	ret = ib_cm_init_qp_attr(ch->ib_cm.cm_id, &qp_attr, &attr_mask);
 	if (ret)
@@ -1746,18 +1759,33 @@ static int srpt_create_ch_ib(struct srpt_rdma_ch *ch)
 		qp_init->cap.max_recv_sge = qp_init->cap.max_send_sge;
 	}
 
-	ch->qp = ib_create_qp(sdev->pd, qp_init);
-	if (IS_ERR(ch->qp)) {
-		ret = PTR_ERR(ch->qp);
-		if (ret == -ENOMEM) {
-			sq_size /= 2;
-			if (sq_size >= MIN_SRPT_SQ_SIZE) {
-				ib_destroy_cq(ch->cq);
-				goto retry;
-			}
+	if (ch->using_rdma_cm) {
+		ret = rdma_create_qp(ch->rdma_cm.cm_id, sdev->pd, qp_init);
+		ch->qp = ch->rdma_cm.cm_id->qp;
+	} else {
+		ch->qp = ib_create_qp(sdev->pd, qp_init);
+		if (!IS_ERR(ch->qp)) {
+			ret = srpt_init_ch_qp(ch, ch->qp);
+			if (ret)
+				ib_destroy_qp(ch->qp);
+		} else {
+			ret = PTR_ERR(ch->qp);
+		}
+	}
+	if (ret) {
+		bool retry = sq_size > MIN_SRPT_SQ_SIZE;
+
+		if (retry) {
+			pr_debug("failed to create queue pair with sq_size = %d (%d) - retrying\n",
+				 sq_size, ret);
+			ib_free_cq(ch->cq);
+			sq_size = max(sq_size / 2, MIN_SRPT_SQ_SIZE);
+			goto retry;
+		} else {
+			pr_err("failed to create queue pair with sq_size = %d (%d)\n",
+			       sq_size, ret);
+			goto err_destroy_cq;
 		}
-		pr_err("failed to create_qp ret= %d\n", ret);
-		goto err_destroy_cq;
 	}
 
 	atomic_set(&ch->sq_wr_avail, qp_init->cap.max_send_wr);
@@ -1766,10 +1794,6 @@ static int srpt_create_ch_ib(struct srpt_rdma_ch *ch)
 		 __func__, ch->cq->cqe, qp_init->cap.max_send_sge,
 		 qp_init->cap.max_send_wr, ch);
 
-	ret = srpt_init_ch_qp(ch, ch->qp);
-	if (ret)
-		goto err_destroy_qp;
-
 	if (!sdev->use_srq)
 		for (i = 0; i < ch->rq_size; i++)
 			srpt_post_recv(sdev, ch, ch->ioctx_recv_ring[i]);
@@ -1778,9 +1802,8 @@ static int srpt_create_ch_ib(struct srpt_rdma_ch *ch)
 	kfree(qp_init);
 	return ret;
 
-err_destroy_qp:
-	ib_destroy_qp(ch->qp);
 err_destroy_cq:
+	ch->qp = NULL;
 	ib_free_cq(ch->cq);
 	goto out;
 }
@@ -1849,9 +1872,13 @@ static int srpt_disconnect_ch(struct srpt_rdma_ch *ch)
 	if (!srpt_set_ch_state(ch, CH_DISCONNECTING))
 		return -ENOTCONN;
 
-	ret = ib_send_cm_dreq(ch->ib_cm.cm_id, NULL, 0);
-	if (ret < 0)
-		ret = ib_send_cm_drep(ch->ib_cm.cm_id, NULL, 0);
+	if (ch->using_rdma_cm) {
+		ret = rdma_disconnect(ch->rdma_cm.cm_id);
+	} else {
+		ret = ib_send_cm_dreq(ch->ib_cm.cm_id, NULL, 0);
+		if (ret < 0)
+			ret = ib_send_cm_drep(ch->ib_cm.cm_id, NULL, 0);
+	}
 
 	if (ret < 0 && srpt_close_ch(ch))
 		ret = 0;
@@ -2003,7 +2030,10 @@ static void srpt_release_channel_work(struct work_struct *w)
 	transport_deregister_session(se_sess);
 	ch->sess = NULL;
 
-	ib_destroy_cm_id(ch->ib_cm.cm_id);
+	if (ch->using_rdma_cm)
+		rdma_destroy_id(ch->rdma_cm.cm_id);
+	else
+		ib_destroy_cm_id(ch->ib_cm.cm_id);
 
 	srpt_destroy_ch_ib(ch);
 
@@ -2027,26 +2057,33 @@ static void srpt_release_channel_work(struct work_struct *w)
 
 /**
  * srpt_cm_req_recv - process the event IB_CM_REQ_RECEIVED
- * @cm_id: IB/CM connection identifier.
- * @port_num: Port through which the IB/CM REQ message was received.
+ * @sdev: HCA through which the login request was received.
+ * @ib_cm_id: IB/CM connection identifier in case of IB/CM.
+ * @rdma_cm_id: RDMA/CM connection identifier in case of RDMA/CM.
+ * @port_num: Port through which the REQ message was received.
  * @pkey: P_Key of the incoming connection.
  * @req: SRP login request.
- * @src_addr: GID of the port that submitted the login request.
+ * @src_addr: GID (IB/CM) or IP address (RDMA/CM) of the port that submitted
+ * the login request.
  *
  * Ownership of the cm_id is transferred to the target session if this
- * functions returns zero. Otherwise the caller remains the owner of cm_id.
+ * function returns zero. Otherwise the caller remains the owner of cm_id.
  */
-static int srpt_cm_req_recv(struct ib_cm_id *cm_id,
+static int srpt_cm_req_recv(struct srpt_device *const sdev,
+			    struct ib_cm_id *ib_cm_id,
+			    struct rdma_cm_id *rdma_cm_id,
 			    u8 port_num, __be16 pkey,
 			    const struct srp_login_req *req,
 			    const char *src_addr)
 {
-	struct srpt_device *sdev = cm_id->context;
 	struct srpt_port *sport = &sdev->port[port_num - 1];
 	struct srpt_nexus *nexus;
 	struct srp_login_rsp *rsp = NULL;
 	struct srp_login_rej *rej = NULL;
-	struct ib_cm_rep_param *rep_param = NULL;
+	union {
+		struct rdma_conn_param rdma_cm;
+		struct ib_cm_rep_param ib_cm;
+	} *rep_param = NULL;
 	struct srpt_rdma_ch *ch;
 	char i_port_id[36];
 	u32 it_iu_len;
@@ -2117,8 +2154,14 @@ static int srpt_cm_req_recv(struct ib_cm_id *cm_id,
 	ch->zw_cqe.done = srpt_zerolength_write_done;
 	INIT_WORK(&ch->release_work, srpt_release_channel_work);
 	ch->sport = sport;
-	ch->ib_cm.cm_id = cm_id;
-	cm_id->context = ch;
+	if (ib_cm_id) {
+		ch->ib_cm.cm_id = ib_cm_id;
+		ib_cm_id->context = ch;
+	} else {
+		ch->using_rdma_cm = true;
+		ch->rdma_cm.cm_id = rdma_cm_id;
+		rdma_cm_id->context = ch;
+	}
 	/*
 	 * ch->rq_size should be at least as large as the initiator queue
 	 * depth to avoid that the initiator driver has to report QUEUE_FULL
@@ -2229,7 +2272,7 @@ static int srpt_cm_req_recv(struct ib_cm_id *cm_id,
 
 	mutex_unlock(&sport->mutex);
 
-	ret = srpt_ch_qp_rtr(ch, ch->qp);
+	ret = ch->using_rdma_cm ? 0 : srpt_ch_qp_rtr(ch, ch->qp);
 	if (ret) {
 		rej->reason = cpu_to_be32(SRP_LOGIN_REJ_INSUFFICIENT_RESOURCES);
 		pr_err("rejected SRP_LOGIN_REQ because enabling RTR failed (error code = %d)\n",
@@ -2253,25 +2296,38 @@ static int srpt_cm_req_recv(struct ib_cm_id *cm_id,
 	atomic_set(&ch->req_lim_delta, 0);
 
 	/* create cm reply */
-	rep_param->qp_num = ch->qp->qp_num;
-	rep_param->private_data = (void *)rsp;
-	rep_param->private_data_len = sizeof(*rsp);
-	rep_param->rnr_retry_count = 7;
-	rep_param->flow_control = 1;
-	rep_param->failover_accepted = 0;
-	rep_param->srq = 1;
-	rep_param->responder_resources = 4;
-	rep_param->initiator_depth = 4;
+	if (ch->using_rdma_cm) {
+		rep_param->rdma_cm.private_data = (void *)rsp;
+		rep_param->rdma_cm.private_data_len = sizeof(*rsp);
+		rep_param->rdma_cm.rnr_retry_count = 7;
+		rep_param->rdma_cm.flow_control = 1;
+		rep_param->rdma_cm.responder_resources = 4;
+		rep_param->rdma_cm.initiator_depth = 4;
+	} else {
+		rep_param->ib_cm.qp_num = ch->qp->qp_num;
+		rep_param->ib_cm.private_data = (void *)rsp;
+		rep_param->ib_cm.private_data_len = sizeof(*rsp);
+		rep_param->ib_cm.rnr_retry_count = 7;
+		rep_param->ib_cm.flow_control = 1;
+		rep_param->ib_cm.failover_accepted = 0;
+		rep_param->ib_cm.srq = 1;
+		rep_param->ib_cm.responder_resources = 4;
+		rep_param->ib_cm.initiator_depth = 4;
+	}
 
 	/*
 	 * Hold the sport mutex while accepting a connection to avoid that
 	 * srpt_disconnect_ch() is invoked concurrently with this code.
 	 */
 	mutex_lock(&sport->mutex);
-	if (sport->enabled && ch->state == CH_CONNECTING)
-		ret = ib_send_cm_rep(cm_id, rep_param);
-	else
+	if (sport->enabled && ch->state == CH_CONNECTING) {
+		if (ch->using_rdma_cm)
+			ret = rdma_accept(rdma_cm_id, &rep_param->rdma_cm);
+		else
+			ret = ib_send_cm_rep(ib_cm_id, &rep_param->ib_cm);
+	} else {
 		ret = -EINVAL;
+	}
 	mutex_unlock(&sport->mutex);
 
 	switch (ret) {
@@ -2301,7 +2357,8 @@ static int srpt_cm_req_recv(struct ib_cm_id *cm_id,
 			     ch->sport->sdev, ch->rq_size,
 			     ch->max_rsp_size, DMA_TO_DEVICE);
 free_ch:
-	cm_id->context = NULL;
+	if (ib_cm_id)
+		ib_cm_id->context = NULL;
 	kfree(ch);
 	ch = NULL;
 
@@ -2314,8 +2371,11 @@ static int srpt_cm_req_recv(struct ib_cm_id *cm_id,
 	rej->buf_fmt = cpu_to_be16(SRP_BUF_FORMAT_DIRECT |
 				   SRP_BUF_FORMAT_INDIRECT);
 
-	ib_send_cm_rej(cm_id, IB_CM_REJ_CONSUMER_DEFINED, NULL, 0,
-			     (void *)rej, sizeof(*rej));
+	if (rdma_cm_id)
+		rdma_reject(rdma_cm_id, rej, sizeof(*rej));
+	else
+		ib_send_cm_rej(ib_cm_id, IB_CM_REJ_CONSUMER_DEFINED, NULL, 0,
+			       rej, sizeof(*rej));
 
 out:
 	kfree(rep_param);
@@ -2334,10 +2394,61 @@ static int srpt_ib_cm_req_recv(struct ib_cm_id *cm_id,
 	srpt_format_guid(sguid, sizeof(sguid),
 			 &param->primary_path->dgid.global.interface_id);
 
-	return srpt_cm_req_recv(cm_id, param->port, param->primary_path->pkey,
+	return srpt_cm_req_recv(cm_id->context, cm_id, NULL, param->port,
+				param->primary_path->pkey,
 				private_data, sguid);
 }
 
+static const char *inet_ntop(const void *sa, char *dst, unsigned int size)
+{
+	switch (((struct sockaddr *)sa)->sa_family) {
+	case AF_INET:
+		snprintf(dst, size, "%pI4",
+			 &((struct sockaddr_in *)sa)->sin_addr);
+		break;
+	case AF_INET6:
+		snprintf(dst, size, "%pI6",
+			 &((struct sockaddr_in6 *)sa)->sin6_addr);
+		break;
+	default:
+		snprintf(dst, size, "???");
+		break;
+	}
+	return dst;
+}
+
+static int srpt_rdma_cm_req_recv(struct rdma_cm_id *cm_id,
+				 struct rdma_cm_event *event)
+{
+	struct srpt_device *sdev;
+	struct srp_login_req req;
+	const struct srp_login_req_rdma *req_rdma;
+	char src_addr[40];
+
+	sdev = ib_get_client_data(cm_id->device, &srpt_client);
+	if (!sdev)
+		return -ECONNREFUSED;
+
+	if (event->param.conn.private_data_len < sizeof(*req_rdma))
+		return -EINVAL;
+
+	/* Transform srp_login_req_rdma into srp_login_req. */
+	req_rdma = event->param.conn.private_data;
+	memset(&req, 0, sizeof(req));
+	req.opcode		= req_rdma->opcode;
+	req.tag			= req_rdma->tag;
+	req.req_it_iu_len	= req_rdma->req_it_iu_len;
+	req.req_buf_fmt		= req_rdma->req_buf_fmt;
+	req.req_flags		= req_rdma->req_flags;
+	memcpy(req.initiator_port_id, req_rdma->initiator_port_id, 16);
+	memcpy(req.target_port_id, req_rdma->target_port_id, 16);
+
+	inet_ntop(&cm_id->route.addr.src_addr, src_addr, sizeof(src_addr));
+
+	return srpt_cm_req_recv(sdev, NULL, cm_id, cm_id->port_num,
+				cm_id->route.path_rec->pkey, &req, src_addr);
+}
+
 static void srpt_cm_rej_recv(struct srpt_rdma_ch *ch,
 			     enum ib_cm_rej_reason reason,
 			     const u8 *private_data,
@@ -2361,14 +2472,14 @@ static void srpt_cm_rej_recv(struct srpt_rdma_ch *ch,
  * srpt_cm_rtu_recv - process an IB_CM_RTU_RECEIVED or USER_ESTABLISHED event
  * @ch: SRPT RDMA channel.
  *
- * An IB_CM_RTU_RECEIVED message indicates that the connection is established
- * and that the recipient may begin transmitting (RTU = ready to use).
+ * An RTU (ready to use) message indicates that the connection has been
+ * established and that the recipient may begin transmitting.
  */
 static void srpt_cm_rtu_recv(struct srpt_rdma_ch *ch)
 {
 	int ret;
 
-	ret = srpt_ch_qp_rts(ch, ch->qp);
+	ret = ch->using_rdma_cm ? 0 : srpt_ch_qp_rts(ch, ch->qp);
 	if (ret < 0) {
 		pr_err("%s-%d: QP transition to RTS failed\n", ch->sess_name,
 		       ch->qp->qp_num);
@@ -2455,6 +2566,49 @@ static int srpt_cm_handler(struct ib_cm_id *cm_id, struct ib_cm_event *event)
 	return ret;
 }
 
+static int srpt_rdma_cm_handler(struct rdma_cm_id *cm_id,
+				struct rdma_cm_event *event)
+{
+	struct srpt_rdma_ch *ch = cm_id->context;
+	int ret = 0;
+
+	switch (event->event) {
+	case RDMA_CM_EVENT_CONNECT_REQUEST:
+		ret = srpt_rdma_cm_req_recv(cm_id, event);
+		break;
+	case RDMA_CM_EVENT_REJECTED:
+		srpt_cm_rej_recv(ch, event->status,
+				 event->param.conn.private_data,
+				 event->param.conn.private_data_len);
+		break;
+	case RDMA_CM_EVENT_ESTABLISHED:
+		srpt_cm_rtu_recv(ch);
+		break;
+	case RDMA_CM_EVENT_DISCONNECTED:
+		if (ch->state < CH_DISCONNECTING)
+			srpt_disconnect_ch(ch);
+		else
+			srpt_close_ch(ch);
+		break;
+	case RDMA_CM_EVENT_TIMEWAIT_EXIT:
+		srpt_close_ch(ch);
+		break;
+	case RDMA_CM_EVENT_UNREACHABLE:
+		pr_info("Received CM REP error for ch %s-%d.\n", ch->sess_name,
+			ch->qp->qp_num);
+		break;
+	case RDMA_CM_EVENT_DEVICE_REMOVAL:
+	case RDMA_CM_EVENT_ADDR_CHANGE:
+		break;
+	default:
+		pr_err("received unrecognized RDMA CM event %d\n",
+		       event->event);
+		break;
+	}
+
+	return ret;
+}
+
 static int srpt_write_pending_status(struct se_cmd *se_cmd)
 {
 	struct srpt_send_ioctx *ioctx;
@@ -2826,7 +2980,7 @@ static void srpt_add_one(struct ib_device *device)
 {
 	struct srpt_device *sdev;
 	struct srpt_port *sport;
-	int i;
+	int i, ret;
 
 	pr_debug("device = %p\n", device);
 
@@ -2850,9 +3004,15 @@ static void srpt_add_one(struct ib_device *device)
 	if (!srpt_service_guid)
 		srpt_service_guid = be64_to_cpu(device->node_guid);
 
-	sdev->cm_id = ib_create_cm_id(device, srpt_cm_handler, sdev);
-	if (IS_ERR(sdev->cm_id))
-		goto err_ring;
+	if (rdma_port_get_link_layer(device, 1) == IB_LINK_LAYER_INFINIBAND)
+		sdev->cm_id = ib_create_cm_id(device, srpt_cm_handler, sdev);
+	if (IS_ERR(sdev->cm_id)) {
+		pr_info("ib_create_cm_id() failed: %ld\n",
+			PTR_ERR(sdev->cm_id));
+		sdev->cm_id = NULL;
+		if (!rdma_cm_id)
+			goto err_ring;
+	}
 
 	/* print out target login information */
 	pr_debug("Target login info: id_ext=%016llx,ioc_guid=%016llx,"
@@ -2865,8 +3025,14 @@ static void srpt_add_one(struct ib_device *device)
 	 * in the system as service_id; therefore, the target_id will change
 	 * if this HCA is gone bad and replaced by different HCA
 	 */
-	if (ib_cm_listen(sdev->cm_id, cpu_to_be64(srpt_service_guid), 0))
+	ret = sdev->cm_id ?
+		ib_cm_listen(sdev->cm_id, cpu_to_be64(srpt_service_guid), 0) :
+		0;
+	if (ret < 0) {
+		pr_err("ib_cm_listen() failed: %d (cm_id state = %d)\n", ret,
+		       sdev->cm_id->state);
 		goto err_cm;
+	}
 
 	INIT_IB_EVENT_HANDLER(&sdev->event_handler, sdev->device,
 			      srpt_event_handler);
@@ -2906,7 +3072,8 @@ static void srpt_add_one(struct ib_device *device)
 err_event:
 	ib_unregister_event_handler(&sdev->event_handler);
 err_cm:
-	ib_destroy_cm_id(sdev->cm_id);
+	if (sdev->cm_id)
+		ib_destroy_cm_id(sdev->cm_id);
 err_ring:
 	srpt_free_srq(sdev);
 	ib_dealloc_pd(sdev->pd);
@@ -2941,7 +3108,10 @@ static void srpt_remove_one(struct ib_device *device, void *client_data)
 	for (i = 0; i < sdev->device->phys_port_cnt; i++)
 		cancel_work_sync(&sdev->port[i].work);
 
-	ib_destroy_cm_id(sdev->cm_id);
+	if (sdev->cm_id)
+		ib_destroy_cm_id(sdev->cm_id);
+
+	ib_set_client_data(device, &srpt_client, NULL);
 
 	/*
 	 * Unregistering a target must happen after destroying sdev->cm_id
@@ -3105,18 +3275,25 @@ static int srpt_parse_i_port_id(u8 i_port_id[16], const char *name)
 	leading_zero_bytes = 16 - count;
 	memset(i_port_id, 0, leading_zero_bytes);
 	ret = hex2bin(i_port_id + leading_zero_bytes, p, count);
-	if (ret < 0)
-		pr_debug("hex2bin failed for srpt_parse_i_port_id: %d\n", ret);
+
 out:
 	return ret;
 }
 
 /*
- * configfs callback function invoked for
- * mkdir /sys/kernel/config/target/$driver/$port/$tpg/acls/$i_port_id
+ * configfs callback function invoked for mkdir
+ * /sys/kernel/config/target/$driver/$port/$tpg/acls/$i_port_id i_port_id must
+ * be an initiator port GUID, GID or IP address. See also the
+ * target_alloc_session() calls in this driver. Examples of valid initiator
+ * port IDs:
+ * 0x0000000000000000505400fffe4a0b7b
+ * 0000000000000000505400fffe4a0b7b
+ * 5054:00ff:fe4a:0b7b
+ * 192.168.122.76
  */
 static int srpt_init_nodeacl(struct se_node_acl *se_nacl, const char *name)
 {
+	struct sockaddr_storage sa;
 	u64 guid;
 	u8 i_port_id[16];
 	int ret;
@@ -3124,6 +3301,9 @@ static int srpt_init_nodeacl(struct se_node_acl *se_nacl, const char *name)
 	ret = srpt_parse_guid(&guid, name);
 	if (ret < 0)
 		ret = srpt_parse_i_port_id(i_port_id, name);
+	if (ret < 0)
+		ret = inet_pton_with_scope(&init_net, AF_UNSPEC, name, NULL,
+					   &sa);
 	if (ret < 0)
 		pr_err("invalid initiator port ID %s\n", name);
 	return ret;
@@ -3486,8 +3666,37 @@ static int __init srpt_init_module(void)
 		goto out_unregister_target;
 	}
 
+	if (rdma_cm_port) {
+		struct sockaddr_in addr;
+
+		rdma_cm_id = rdma_create_id(&init_net, srpt_rdma_cm_handler,
+					    NULL, RDMA_PS_TCP, IB_QPT_RC);
+		if (IS_ERR(rdma_cm_id)) {
+			rdma_cm_id = NULL;
+			pr_err("RDMA/CM ID creation failed\n");
+			goto out_unregister_client;
+		}
+
+		/* We will listen on any RDMA device. */
+		memset(&addr, 0, sizeof(addr));
+		addr.sin_family = AF_INET;
+		addr.sin_port = cpu_to_be16(rdma_cm_port);
+		if (rdma_bind_addr(rdma_cm_id, (void *)&addr)) {
+			pr_err("Binding RDMA/CM ID to port %u failed\n",
+			       rdma_cm_port);
+			goto out_unregister_client;
+		}
+
+		if (rdma_listen(rdma_cm_id, 128)) {
+			pr_err("rdma_listen() failed\n");
+			goto out_unregister_client;
+		}
+	}
+
 	return 0;
 
+out_unregister_client:
+	ib_unregister_client(&srpt_client);
 out_unregister_target:
 	target_unregister_template(&srpt_template);
 out:
@@ -3496,6 +3705,8 @@ static int __init srpt_init_module(void)
 
 static void __exit srpt_cleanup_module(void)
 {
+	if (rdma_cm_id)
+		rdma_destroy_id(rdma_cm_id);
 	ib_unregister_client(&srpt_client);
 	target_unregister_template(&srpt_template);
 }
diff --git a/drivers/infiniband/ulp/srpt/ib_srpt.h b/drivers/infiniband/ulp/srpt/ib_srpt.h
index 4d9199fd00dc..02883f8e9c71 100644
--- a/drivers/infiniband/ulp/srpt/ib_srpt.h
+++ b/drivers/infiniband/ulp/srpt/ib_srpt.h
@@ -42,6 +42,7 @@
 #include <rdma/ib_verbs.h>
 #include <rdma/ib_sa.h>
 #include <rdma/ib_cm.h>
+#include <rdma/rdma_cm.h>
 #include <rdma/rw.h>
 
 #include <scsi/srp.h>
@@ -261,6 +262,7 @@ enum rdma_ch_state {
  * @spinlock:      Protects free_list and state.
  * @free_list:     Head of list with free send I/O contexts.
  * @state:         channel state. See also enum rdma_ch_state.
+ * @using_rdma_cm: Whether the RDMA/CM or IB/CM is used for this channel.
  * @processing_wait_list: Whether or not cmd_wait_list is being processed.
  * @ioctx_ring:    Send ring.
  * @ioctx_recv_ring: Receive I/O context ring.
@@ -280,6 +282,9 @@ struct srpt_rdma_ch {
 		struct {
 			struct ib_cm_id		*cm_id;
 		} ib_cm;
+		struct {
+			struct rdma_cm_id	*cm_id;
+		} rdma_cm;
 	};
 	struct ib_cq		*cq;
 	struct ib_cqe		zw_cqe;
@@ -300,6 +305,7 @@ struct srpt_rdma_ch {
 	struct list_head	list;
 	struct list_head	cmd_wait_list;
 	uint16_t		pkey;
+	bool			using_rdma_cm;
 	bool			processing_wait_list;
 	struct se_session	*sess;
 	u8			sess_name[24];
-- 
2.15.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

^ permalink raw reply related	[flat|nested] 24+ messages in thread

* Re: [PATCH v2 00/14] IB/srpt: Add RDMA/CM support
       [not found] ` <20180117001418.7852-1-bart.vanassche-Sjgp3cTcYWE@public.gmane.org>
                     ` (13 preceding siblings ...)
  2018-01-17  0:14   ` [PATCH v2 14/14] IB/srpt: Add RDMA/CM support Bart Van Assche
@ 2018-01-17 23:14   ` Doug Ledford
       [not found]     ` <1516230870.3403.292.camel-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
  2018-01-17 23:33   ` Doug Ledford
  15 siblings, 1 reply; 24+ messages in thread
From: Doug Ledford @ 2018-01-17 23:14 UTC (permalink / raw)
  To: Bart Van Assche, Jason Gunthorpe; +Cc: linux-rdma-u79uwXL29TY76Z2rM5mHXA

[-- Attachment #1: Type: text/plain, Size: 781 bytes --]

On Tue, 2018-01-16 at 16:14 -0800, Bart Van Assche wrote:
> Hello Jason and Doug,
> 
> This patch series not only adds RDMA/CM support to the SRP target driver but
> also fixes a number of race conditions in that driver.
> 
> The RDMA/CM listener port number has to be specified as an ib_srpt kernel
> module parameter. The default value for that parameter is zero which means
> that RDMA/CM support is disabled.

Since srpt is already configured via the lIO framework, wouldn't that be
a better place for the listen port?  In fact, shouldn't it be part of a
portal like you have for iSERt?

-- 
Doug Ledford <dledford-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
    GPG KeyID: B826A3330E572FDD
    Key fingerprint = AE6B 1BDA 122B 23B4 265B  1274 B826 A333 0E57 2FDD

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH v2 00/14] IB/srpt: Add RDMA/CM support
       [not found] ` <20180117001418.7852-1-bart.vanassche-Sjgp3cTcYWE@public.gmane.org>
                     ` (14 preceding siblings ...)
  2018-01-17 23:14   ` [PATCH v2 00/14] " Doug Ledford
@ 2018-01-17 23:33   ` Doug Ledford
       [not found]     ` <1516231986.3403.296.camel-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
  15 siblings, 1 reply; 24+ messages in thread
From: Doug Ledford @ 2018-01-17 23:33 UTC (permalink / raw)
  To: Bart Van Assche, Jason Gunthorpe; +Cc: linux-rdma-u79uwXL29TY76Z2rM5mHXA

[-- Attachment #1: Type: text/plain, Size: 2083 bytes --]

On Tue, 2018-01-16 at 16:14 -0800, Bart Van Assche wrote:
> Hello Jason and Doug,
> 
> This patch series not only adds RDMA/CM support to the SRP target driver but
> also fixes a number of race conditions in that driver.
> 
> The RDMA/CM listener port number has to be specified as an ib_srpt kernel
> module parameter. The default value for that parameter is zero which means
> that RDMA/CM support is disabled.
> 
> Note: since this patch series uses the srp_login_req_rdma structure that was
> introduced by the IB/srp RDMA/CM patch series, this series depends on the
> IB/srp RDMA/CM patch series.
> 
> This patch series, just like v4 of the IB/srp RDMA/CM patch series, passes
> Laurence Oberman's tests.
> 
> Please consider this patch series for inclusion in the upstream kernel.
> 
> Thanks,
> 
> Bart.
> 
> Changes compared to v1:
> - Added patch "Fix a race condition related to wait list processing".
> - Fixed the size of the character arrays used to store the initiator port ID
>   and session name. This fixes a login failure that was reported by Laurence
>   Oberman.

Overall, this series looks mostly good.  I'm still wondering if the
configuration details need more work.  In particular, it seems the host
is lacking in the fundamental controls needed for implemented server
side ACLs in regards to RDMA_CM connections.  The current code assumes
it is safe to listen on the wildcard address on the target port, and I
don't think that's a safe assumption.  We might use different
vlans/pkeys to segment off different namespaces, and in that case we
would want to listen only on the vlans/pkeys that correspond to allowed
namespace clients.  So I think that needs correcting.  The first 11
patches of this series seem standalone and can go in now at this point. 
Would you agree?  If so, I'll pull those in while we discuss the
configuration stuff.

-- 
Doug Ledford <dledford-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
    GPG KeyID: B826A3330E572FDD
    Key fingerprint = AE6B 1BDA 122B 23B4 265B  1274 B826 A333 0E57 2FDD

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH v2 00/14] IB/srpt: Add RDMA/CM support
       [not found]     ` <1516231986.3403.296.camel-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
@ 2018-01-17 23:41       ` Bart Van Assche
       [not found]         ` <1516232517.2820.93.camel-Sjgp3cTcYWE@public.gmane.org>
  0 siblings, 1 reply; 24+ messages in thread
From: Bart Van Assche @ 2018-01-17 23:41 UTC (permalink / raw)
  To: jgg-VPRAkNaXOzVWk0Htik3J/w, dledford-H+wXaHxf7aLQT0dZR+AlfA
  Cc: linux-rdma-u79uwXL29TY76Z2rM5mHXA

On Wed, 2018-01-17 at 18:33 -0500, Doug Ledford wrote:
> The first 11
> patches of this series seem standalone and can go in now at this point. 
> Would you agree?  If so, I'll pull those in while we discuss the
> configuration stuff.

Hello Doug,

Please consider the first thirteen patches instead of only the first 11.
Neither patch 12 nor patch 13 introduces any code that controls which RDMA/CM
initiators are allowed to log in.

Thanks,

Bart.

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH v2 00/14] IB/srpt: Add RDMA/CM support
       [not found]         ` <1516232517.2820.93.camel-Sjgp3cTcYWE@public.gmane.org>
@ 2018-01-18  1:34           ` Doug Ledford
  0 siblings, 0 replies; 24+ messages in thread
From: Doug Ledford @ 2018-01-18  1:34 UTC (permalink / raw)
  To: Bart Van Assche, jgg-VPRAkNaXOzVWk0Htik3J/w
  Cc: linux-rdma-u79uwXL29TY76Z2rM5mHXA

[-- Attachment #1: Type: text/plain, Size: 807 bytes --]

On Wed, 2018-01-17 at 23:41 +0000, Bart Van Assche wrote:
> On Wed, 2018-01-17 at 18:33 -0500, Doug Ledford wrote:
> > The first 11
> > patches of this series seem standalone and can go in now at this point. 
> > Would you agree?  If so, I'll pull those in while we discuss the
> > configuration stuff.
> 
> Hello Doug,
> 
> Please consider the first thirteen patches instead of only the first 11.
> Neither patch 12 nor patch 13 introduces any code that controls which RDMA/CM
> initiators are allowed to log in.

I'm fine with that.  Patches 1-13 applied (with some tweaks to the
commit messages here and there).

-- 
Doug Ledford <dledford-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
    GPG KeyID: B826A3330E572FDD
    Key fingerprint = AE6B 1BDA 122B 23B4 265B  1274 B826 A333 0E57 2FDD

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH v2 00/14] IB/srpt: Add RDMA/CM support
       [not found]     ` <1516230870.3403.292.camel-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
@ 2018-01-30  0:23       ` Bart Van Assche
       [not found]         ` <1517271807.2687.65.camel-Sjgp3cTcYWE@public.gmane.org>
  0 siblings, 1 reply; 24+ messages in thread
From: Bart Van Assche @ 2018-01-30  0:23 UTC (permalink / raw)
  To: jgg-VPRAkNaXOzVWk0Htik3J/w, dledford-H+wXaHxf7aLQT0dZR+AlfA
  Cc: linux-rdma-u79uwXL29TY76Z2rM5mHXA

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset="utf-8", Size: 1162 bytes --]

On Wed, 2018-01-17 at 18:14 -0500, Doug Ledford wrote:
> On Tue, 2018-01-16 at 16:14 -0800, Bart Van Assche wrote:
> > Hello Jason and Doug,
> > 
> > This patch series not only adds RDMA/CM support to the SRP target driver but
> > also fixes a number of race conditions in that driver.
> > 
> > The RDMA/CM listener port number has to be specified as an ib_srpt kernel
> > module parameter. The default value for that parameter is zero which means
> > that RDMA/CM support is disabled.
> 
> Since srpt is already configured via the lIO framework, wouldn't that be
> a better place for the listen port?  In fact, shouldn't it be part of a
> portal like you have for iSERt?

Wouldn't that be overkill to have one listen port per RDMA port? I think
it will be easier for users if they have to configure the RDMA/CM port once
instead of one time per RDMA port. How about using the following location in
configfs for the RDMA/CM port:

/sys/kernel/config/target/srpt/discovery_auth/rdma_cm_port

Thanks,

Bart.N‹§²æìr¸›yúèšØb²X¬¶Ç§vØ^–)Þº{.nÇ+‰·¥Š{±­ÙšŠ{ayº\x1dʇڙë,j\a­¢f£¢·hš‹»öì\x17/oSc¾™Ú³9˜uÀ¦æå‰È&jw¨®\x03(­éšŽŠÝ¢j"ú\x1a¶^[m§ÿïêäz¹Þ–Šàþf£¢·hšˆ§~ˆmš

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH v2 00/14] IB/srpt: Add RDMA/CM support
       [not found]         ` <1517271807.2687.65.camel-Sjgp3cTcYWE@public.gmane.org>
@ 2018-01-30 17:43           ` Doug Ledford
       [not found]             ` <1517334206.27592.291.camel-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
  0 siblings, 1 reply; 24+ messages in thread
From: Doug Ledford @ 2018-01-30 17:43 UTC (permalink / raw)
  To: Bart Van Assche, jgg-VPRAkNaXOzVWk0Htik3J/w
  Cc: linux-rdma-u79uwXL29TY76Z2rM5mHXA

[-- Attachment #1: Type: text/plain, Size: 2040 bytes --]

On Tue, 2018-01-30 at 00:23 +0000, Bart Van Assche wrote:
> On Wed, 2018-01-17 at 18:14 -0500, Doug Ledford wrote:
> > On Tue, 2018-01-16 at 16:14 -0800, Bart Van Assche wrote:
> > > Hello Jason and Doug,
> > > 
> > > This patch series not only adds RDMA/CM support to the SRP target driver but
> > > also fixes a number of race conditions in that driver.
> > > 
> > > The RDMA/CM listener port number has to be specified as an ib_srpt kernel
> > > module parameter. The default value for that parameter is zero which means
> > > that RDMA/CM support is disabled.
> > 
> > Since srpt is already configured via the lIO framework, wouldn't that be
> > a better place for the listen port?  In fact, shouldn't it be part of a
> > portal like you have for iSERt?
> 
> Wouldn't that be overkill to have one listen port per RDMA port? I think
> it will be easier for users if they have to configure the RDMA/CM port once
> instead of one time per RDMA port. How about using the following location in
> configfs for the RDMA/CM port:
> 
> /sys/kernel/config/target/srpt/discovery_auth/rdma_cm_port

Hmmm...maybe the real answer here is to start considering how serious we
are about the RDMA_CM support in SRP.  If we're serious, should someone
contact IANNA about a reserving port number and just use whatever they
give us?  If we want to do the simple thing, a module option is fine,
while we decide on this issue, then I can see that being OK.  I'm more
OK with using configfs if there's a chance we won't get a well known
reserved port and the config option will stick around long term.  And I
tend to agree, per interface port configuration is probably not that
interesting.  But the ability to specify something other than the
wildcard IP address to listen on, and the ability to specify more than
one IP address to listen on, are.

-- 
Doug Ledford <dledford-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
    GPG KeyID: B826A3330E572FDD
    Key fingerprint = AE6B 1BDA 122B 23B4 265B  1274 B826 A333 0E57 2FDD

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH v2 00/14] IB/srpt: Add RDMA/CM support
       [not found]             ` <1517334206.27592.291.camel-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
@ 2018-01-30 18:19               ` Bart Van Assche
       [not found]                 ` <1517336389.2589.22.camel-Sjgp3cTcYWE@public.gmane.org>
  0 siblings, 1 reply; 24+ messages in thread
From: Bart Van Assche @ 2018-01-30 18:19 UTC (permalink / raw)
  To: jgg-VPRAkNaXOzVWk0Htik3J/w, dledford-H+wXaHxf7aLQT0dZR+AlfA
  Cc: linux-rdma-u79uwXL29TY76Z2rM5mHXA

On Tue, 2018-01-30 at 12:43 -0500, Doug Ledford wrote:
> On Tue, 2018-01-30 at 00:23 +0000, Bart Van Assche wrote:
> > On Wed, 2018-01-17 at 18:14 -0500, Doug Ledford wrote:
> > > On Tue, 2018-01-16 at 16:14 -0800, Bart Van Assche wrote:
> > > > Hello Jason and Doug,
> > > > 
> > > > This patch series not only adds RDMA/CM support to the SRP target driver but
> > > > also fixes a number of race conditions in that driver.
> > > > 
> > > > The RDMA/CM listener port number has to be specified as an ib_srpt kernel
> > > > module parameter. The default value for that parameter is zero which means
> > > > that RDMA/CM support is disabled.
> > > 
> > > Since srpt is already configured via the lIO framework, wouldn't that be
> > > a better place for the listen port?  In fact, shouldn't it be part of a
> > > portal like you have for iSERt?
> > 
> > Wouldn't that be overkill to have one listen port per RDMA port? I think
> > it will be easier for users if they have to configure the RDMA/CM port once
> > instead of one time per RDMA port. How about using the following location in
> > configfs for the RDMA/CM port:
> > 
> > /sys/kernel/config/target/srpt/discovery_auth/rdma_cm_port
> 
> Hmmm...maybe the real answer here is to start considering how serious we
> are about the RDMA_CM support in SRP.  If we're serious, should someone
> contact IANNA about a reserving port number and just use whatever they
> give us?  If we want to do the simple thing, a module option is fine,
> while we decide on this issue, then I can see that being OK.  I'm more
> OK with using configfs if there's a chance we won't get a well known
> reserved port and the config option will stick around long term.  And I
> tend to agree, per interface port configuration is probably not that
> interesting.  But the ability to specify something other than the
> wildcard IP address to listen on, and the ability to specify more than
> one IP address to listen on, are.

Hello Doug,

My motivation for posting the RDMA/CM patches on the linux-rdma mailing list
for the SRP initiator and target drivers was to allow others to run the
srp-test software on a setup that does not have any RDMA adapters, namely by
using the SoftRoCE driver. Since IANA has not yet assigned a port number to
SRP over RoCE I choose to make the port number configurable.

Personally I don't think we have to postpone the ib_srpt RDMA/CM patch until
IANA has assigned a port number. Once IANA has assigned a port number we
could e.g. modify the ib_srpt driver to enable RDMA/CM support by default
with the assigned port number (the patch I posted leaves RDMA/CM support
disabled by default).

Regarding configuring on which IP address to listen, one possibile approach
is to move the rdma_cm_port port number configfs attribute from
/sys/kernel/config/target/srpt/discovery_auth/rdma_cm_port to
/sys/kernel/config/target/srpt/$port/$port/attrib/rdma_cm_port where $port
is an RDMA port GUID or GID (both are accepted). That would allow to
enable/disable RDMA/CM support per RDMA adapter port. The ib_srpt driver
could e.g. create one listening rdma_cm_id per IP address that is associated
with an RDMA port.

One approach I have considered for allowing to configure per IP address
whether or not to accept incoming connections is to accept IP addresses as
RDMA port identifier ($port). However, that would complicate the ib_srpt
driver because there is code in that driver that assumes that there is a 1:1
relationship between RDMA ports and struct srpt_port instances. As an
example, the port[] array in struct srpt_device is an array with two elements.
Code like srpt_event_handler() and srpt_cm_req_recv() uses that array to
translate a port number into a struct srpt_port pointer.

Yet another approach would be to introduce a new directory hierarchy in
configfs for configuring IP access control. However, I don't think that would
be possible without modifying the LIO target core. And as you probably know
since several months the LIO target core maintainer is mostly interested in
bug fixes and not in new features or any other kind of patch that changes the
target core significantly.

Thanks,

Bart.

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH v2 00/14] IB/srpt: Add RDMA/CM support
       [not found]                 ` <1517336389.2589.22.camel-Sjgp3cTcYWE@public.gmane.org>
@ 2018-01-30 22:29                   ` Doug Ledford
       [not found]                     ` <1517351373.19117.6.camel-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
  0 siblings, 1 reply; 24+ messages in thread
From: Doug Ledford @ 2018-01-30 22:29 UTC (permalink / raw)
  To: Bart Van Assche, jgg-VPRAkNaXOzVWk0Htik3J/w
  Cc: linux-rdma-u79uwXL29TY76Z2rM5mHXA

[-- Attachment #1: Type: text/plain, Size: 4929 bytes --]

On Tue, 2018-01-30 at 18:19 +0000, Bart Van Assche wrote:
> On Tue, 2018-01-30 at 12:43 -0500, Doug Ledford wrote:
> > On Tue, 2018-01-30 at 00:23 +0000, Bart Van Assche wrote:
> > > On Wed, 2018-01-17 at 18:14 -0500, Doug Ledford wrote:
> > > > On Tue, 2018-01-16 at 16:14 -0800, Bart Van Assche wrote:
> > > > > Hello Jason and Doug,
> > > > > 
> > > > > This patch series not only adds RDMA/CM support to the SRP target driver but
> > > > > also fixes a number of race conditions in that driver.
> > > > > 
> > > > > The RDMA/CM listener port number has to be specified as an ib_srpt kernel
> > > > > module parameter. The default value for that parameter is zero which means
> > > > > that RDMA/CM support is disabled.
> > > > 
> > > > Since srpt is already configured via the lIO framework, wouldn't that be
> > > > a better place for the listen port?  In fact, shouldn't it be part of a
> > > > portal like you have for iSERt?
> > > 
> > > Wouldn't that be overkill to have one listen port per RDMA port? I think
> > > it will be easier for users if they have to configure the RDMA/CM port once
> > > instead of one time per RDMA port. How about using the following location in
> > > configfs for the RDMA/CM port:
> > > 
> > > /sys/kernel/config/target/srpt/discovery_auth/rdma_cm_port
> > 
> > Hmmm...maybe the real answer here is to start considering how serious we
> > are about the RDMA_CM support in SRP.  If we're serious, should someone
> > contact IANNA about a reserving port number and just use whatever they
> > give us?  If we want to do the simple thing, a module option is fine,
> > while we decide on this issue, then I can see that being OK.  I'm more
> > OK with using configfs if there's a chance we won't get a well known
> > reserved port and the config option will stick around long term.  And I
> > tend to agree, per interface port configuration is probably not that
> > interesting.  But the ability to specify something other than the
> > wildcard IP address to listen on, and the ability to specify more than
> > one IP address to listen on, are.
> 
> Hello Doug,
> 
> My motivation for posting the RDMA/CM patches on the linux-rdma mailing list
> for the SRP initiator and target drivers was to allow others to run the
> srp-test software on a setup that does not have any RDMA adapters, namely by
> using the SoftRoCE driver. Since IANA has not yet assigned a port number to
> SRP over RoCE I choose to make the port number configurable.

Sure, that makes sense.  I was just saying that if the intent is to
truly support this on anything other than an IB-like network, such as
actual iWARP or RoCE, then it's worth thinking about the long term
things that need done.

> Personally I don't think we have to postpone the ib_srpt RDMA/CM patch until
> IANA has assigned a port number.

No, we don't.

> Regarding configuring on which IP address to listen, one possibile approach
> is to move the rdma_cm_port port number configfs attribute from
> /sys/kernel/config/target/srpt/discovery_auth/rdma_cm_port to
> /sys/kernel/config/target/srpt/$port/$port/attrib/rdma_cm_port where $port
> is an RDMA port GUID or GID (both are accepted). That would allow to
> enable/disable RDMA/CM support per RDMA adapter port. The ib_srpt driver
> could e.g. create one listening rdma_cm_id per IP address that is associated
> with an RDMA port.

That would work as well.

> One approach I have considered for allowing to configure per IP address
> whether or not to accept incoming connections is to accept IP addresses as
> RDMA port identifier ($port). However, that would complicate the ib_srpt
> driver because there is code in that driver that assumes that there is a 1:1
> relationship between RDMA ports and struct srpt_port instances. As an
> example, the port[] array in struct srpt_device is an array with two elements.
> Code like srpt_event_handler() and srpt_cm_req_recv() uses that array to
> translate a port number into a struct srpt_port pointer.
> 
> Yet another approach would be to introduce a new directory hierarchy in
> configfs for configuring IP access control. However, I don't think that would
> be possible without modifying the LIO target core. And as you probably know
> since several months the LIO target core maintainer is mostly interested in
> bug fixes and not in new features or any other kind of patch that changes the
> target core significantly.

Since you envision this mostly as a testing tool, we can worry about
things like this later if it is decided to make it a "supported" item. 
It would be worth noting the testing status in some way that makes it
clear to users, maybe in docs or somewhere else.

-- 
Doug Ledford <dledford-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
    GPG KeyID: B826A3330E572FDD
    Key fingerprint = AE6B 1BDA 122B 23B4 265B  1274 B826 A333 0E57 2FDD

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH v2 00/14] IB/srpt: Add RDMA/CM support
       [not found]                     ` <1517351373.19117.6.camel-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
@ 2018-02-12 18:04                       ` Bart Van Assche
  0 siblings, 0 replies; 24+ messages in thread
From: Bart Van Assche @ 2018-02-12 18:04 UTC (permalink / raw)
  To: jgg-VPRAkNaXOzVWk0Htik3J/w, dledford-H+wXaHxf7aLQT0dZR+AlfA
  Cc: linux-rdma-u79uwXL29TY76Z2rM5mHXA

On Tue, 2018-01-30 at 17:29 -0500, Doug Ledford wrote:
> On Tue, 2018-01-30 at 18:19 +0000, Bart Van Assche wrote:
> > Regarding configuring on which IP address to listen, one possibile approach
> > is to move the rdma_cm_port port number configfs attribute from
> > /sys/kernel/config/target/srpt/discovery_auth/rdma_cm_port to
> > /sys/kernel/config/target/srpt/$port/$port/attrib/rdma_cm_port where $port
> > is an RDMA port GUID or GID (both are accepted). That would allow to
> > enable/disable RDMA/CM support per RDMA adapter port. The ib_srpt driver
> > could e.g. create one listening rdma_cm_id per IP address that is associated
> > with an RDMA port.
> 
> That would work as well.

Hello Doug,

Do you prefer that the RDMA/CM port number is configurable per RDMA port
(/sys/kernel/config/target/srpt/$port/$port/attrib/rdma_cm_port) or is a
single attribute for all ports sufficient
(/sys/kernel/config/target/srpt/discovery_auth/rdma_cm_port)? I'm asking this
since there is already an attribute that allows to enable/disable SRP target
support per RDMA port (/sys/kernel/config/target/srpt/$port/$port/enable).

Thanks,

Bart.



^ permalink raw reply	[flat|nested] 24+ messages in thread

end of thread, other threads:[~2018-02-12 18:04 UTC | newest]

Thread overview: 24+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-01-17  0:14 [PATCH v2 00/14] IB/srpt: Add RDMA/CM support Bart Van Assche
     [not found] ` <20180117001418.7852-1-bart.vanassche-Sjgp3cTcYWE@public.gmane.org>
2018-01-17  0:14   ` [PATCH v2 01/14] IB/srpt: Make it safe to use RCU for srpt_device.rch_list Bart Van Assche
2018-01-17  0:14   ` [PATCH v2 02/14] IB/srpt: Rework srpt_disconnect_ch_sync() Bart Van Assche
2018-01-17  0:14   ` [PATCH v2 03/14] IB/srpt: Add P_Key support Bart Van Assche
2018-01-17  0:14   ` [PATCH v2 04/14] IB/srpt: One target per port Bart Van Assche
2018-01-17  0:14   ` [PATCH v2 05/14] IB/srpt: Use the source GID as session name Bart Van Assche
2018-01-17  0:14   ` [PATCH v2 06/14] IB/srpt: Rework multi-channel support Bart Van Assche
2018-01-17  0:14   ` [PATCH v2 07/14] IB/srpt: Simplify srpt_close_session() Bart Van Assche
2018-01-17  0:14   ` [PATCH v2 08/14] IB/srpt: Log all zero-length writes and completions Bart Van Assche
2018-01-17  0:14   ` [PATCH v2 09/14] IB/srpt: Fix login-related race conditions Bart Van Assche
2018-01-17  0:14   ` [PATCH v2 10/14] IB/srpt: Fix a race condition related to wait list processing Bart Van Assche
2018-01-17  0:14   ` [PATCH v2 11/14] IB/srpt: Avoid that wait list processing triggers command reordering Bart Van Assche
2018-01-17  0:14   ` [PATCH v2 12/14] IB/srpt: Prepare RDMA/CM support Bart Van Assche
2018-01-17  0:14   ` [PATCH v2 13/14] IB/srpt: Move the code for parsing struct ib_cm_req_event_param Bart Van Assche
2018-01-17  0:14   ` [PATCH v2 14/14] IB/srpt: Add RDMA/CM support Bart Van Assche
2018-01-17 23:14   ` [PATCH v2 00/14] " Doug Ledford
     [not found]     ` <1516230870.3403.292.camel-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
2018-01-30  0:23       ` Bart Van Assche
     [not found]         ` <1517271807.2687.65.camel-Sjgp3cTcYWE@public.gmane.org>
2018-01-30 17:43           ` Doug Ledford
     [not found]             ` <1517334206.27592.291.camel-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
2018-01-30 18:19               ` Bart Van Assche
     [not found]                 ` <1517336389.2589.22.camel-Sjgp3cTcYWE@public.gmane.org>
2018-01-30 22:29                   ` Doug Ledford
     [not found]                     ` <1517351373.19117.6.camel-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
2018-02-12 18:04                       ` Bart Van Assche
2018-01-17 23:33   ` Doug Ledford
     [not found]     ` <1516231986.3403.296.camel-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
2018-01-17 23:41       ` Bart Van Assche
     [not found]         ` <1516232517.2820.93.camel-Sjgp3cTcYWE@public.gmane.org>
2018-01-18  1:34           ` Doug Ledford

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).