[RFC,5/9] iscsi: set netns for iscsi_tcp hosts

Message ID	566c527d12f6ed56eeb40952fef7431a0ccdc78f.1675876735.git.lduncan@suse.com (mailing list archive)
State	Deferred
Headers	show Return-Path: <linux-scsi-owner@vger.kernel.org> From: Lee Duncan <leeman.duncan@gmail.com> To: linux-scsi@vger.kernel.org, open-iscsi@googlegroups.com, netdev@vger.kernel.org Cc: Lee Duncan <lduncan@suse.com>, Chris Leech <cleech@redhat.com> Subject: [RFC PATCH 5/9] iscsi: set netns for iscsi_tcp hosts Date: Wed, 8 Feb 2023 09:40:53 -0800 Message-Id: <566c527d12f6ed56eeb40952fef7431a0ccdc78f.1675876735.git.lduncan@suse.com> In-Reply-To: <cover.1675876731.git.lduncan@suse.com> References: <cover.1675876731.git.lduncan@suse.com> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit Precedence: bulk
Series	Make iscsid-kernel communications namespace-aware \| expand [RFC,0/9] Make iscsid-kernel communications namespace-aware [RFC,1/9] iscsi: create per-net iscsi netlink kernel sockets [RFC,2/9] iscsi: associate endpoints with a host [RFC,3/9] iscsi: sysfs filtering by network namespace [RFC,4/9] iscsi: make all iSCSI netlink multicast namespace aware [RFC,5/9] iscsi: set netns for iscsi_tcp hosts [RFC,6/9] iscsi: check net namespace for all iscsi lookup [RFC,7/9] iscsi: convert flashnode devices from bus to class [RFC,8/9] iscsi: rename iscsi_bus_flash_* to iscsi_flash_* [RFC,9/9] iscsi: filter flashnode sysfs by net namespace

Message ID

566c527d12f6ed56eeb40952fef7431a0ccdc78f.1675876735.git.lduncan@suse.com (mailing list archive)

State

Deferred

Headers

From: Lee Duncan <leeman.duncan@gmail.com>
To: linux-scsi@vger.kernel.org, open-iscsi@googlegroups.com,
        netdev@vger.kernel.org
Cc: Lee Duncan <lduncan@suse.com>, Chris Leech <cleech@redhat.com>
Subject: [RFC PATCH 5/9] iscsi: set netns for iscsi_tcp hosts
Date: Wed,  8 Feb 2023 09:40:53 -0800
Message-Id: 
 <566c527d12f6ed56eeb40952fef7431a0ccdc78f.1675876735.git.lduncan@suse.com>
In-Reply-To: <cover.1675876731.git.lduncan@suse.com>
References: <cover.1675876731.git.lduncan@suse.com>
MIME-Version: 1.0
Content-Transfer-Encoding: 8bit
Precedence: bulk

Series

Make iscsid-kernel communications namespace-aware | expand

Commit Message

Lee Duncan Feb. 8, 2023, 5:40 p.m. UTC

From: Lee Duncan <lduncan@suse.com>

This lets iscsi_tcp operate in multiple namespaces.  It uses current
during session creation to find the net namespace, but it might be
better to manage to pass it along from the iscsi netlink socket.

Signed-off-by: Chris Leech <cleech@redhat.com>
Signed-off-by: Lee Duncan <lduncan@suse.com>
---
 drivers/scsi/iscsi_tcp.c            | 7 +++++++
 drivers/scsi/scsi_transport_iscsi.c | 7 ++++++-
 include/scsi/scsi_transport_iscsi.h | 1 +
 3 files changed, 14 insertions(+), 1 deletion(-)

Comments

Hannes Reinecke March 14, 2023, 4:29 p.m. UTC | #1

On 2/8/23 18:40, Lee Duncan wrote:
> From: Lee Duncan <lduncan@suse.com>
> 
> This lets iscsi_tcp operate in multiple namespaces.  It uses current
> during session creation to find the net namespace, but it might be
> better to manage to pass it along from the iscsi netlink socket.
> 
And indeed, I'd rather use the namespace from the iscsi netlink socket.
If you use the namespace from session creation you'd better hope that
this function is not called from a workqueue ...


> Signed-off-by: Chris Leech <cleech@redhat.com>
> Signed-off-by: Lee Duncan <lduncan@suse.com>
> ---
>   drivers/scsi/iscsi_tcp.c            | 7 +++++++
>   drivers/scsi/scsi_transport_iscsi.c | 7 ++++++-
>   include/scsi/scsi_transport_iscsi.h | 1 +
>   3 files changed, 14 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/scsi/iscsi_tcp.c b/drivers/scsi/iscsi_tcp.c
> index 0454d94e8cf0..22e7a5c93627 100644
> --- a/drivers/scsi/iscsi_tcp.c
> +++ b/drivers/scsi/iscsi_tcp.c
> @@ -1069,6 +1069,11 @@ static int iscsi_sw_tcp_slave_configure(struct scsi_device *sdev)
>   	return 0;
>   }
>   
> +static struct net *iscsi_sw_tcp_netns(struct Scsi_Host *shost)
> +{
> +	return current->nsproxy->net_ns;
> +}
> +

See above if you can't reference the namespace for the netlink socket here.

>   static struct scsi_host_template iscsi_sw_tcp_sht = {
>   	.module			= THIS_MODULE,
>   	.name			= "iSCSI Initiator over TCP/IP",
> @@ -1124,6 +1129,8 @@ static struct iscsi_transport iscsi_sw_tcp_transport = {
>   	.alloc_pdu		= iscsi_sw_tcp_pdu_alloc,
>   	/* recovery */
>   	.session_recovery_timedout = iscsi_session_recovery_timedout,
> +	/* net namespace */
> +	.get_netns		= iscsi_sw_tcp_netns,
>   };
>   
>   static int __init iscsi_sw_tcp_init(void)
> diff --git a/drivers/scsi/scsi_transport_iscsi.c b/drivers/scsi/scsi_transport_iscsi.c
> index 230b43d34c5f..996a9abfa1f5 100644
> --- a/drivers/scsi/scsi_transport_iscsi.c
> +++ b/drivers/scsi/scsi_transport_iscsi.c
> @@ -1600,10 +1600,15 @@ static int iscsi_setup_host(struct transport_container *tc, struct device *dev,
>   {
>   	struct Scsi_Host *shost = dev_to_shost(dev);
>   	struct iscsi_cls_host *ihost = shost->shost_data;
> +	struct iscsi_internal *priv = to_iscsi_internal(shost->transportt);
> +	struct iscsi_transport *transport = priv->iscsi_transport;
>   
>   	memset(ihost, 0, sizeof(*ihost));
>   	mutex_init(&ihost->mutex);
> -	ihost->netns = &init_net;
> +	if (transport->get_netns)
> +		ihost->netns = transport->get_netns(shost);
> +	else
> +		ihost->netns = &init_net;
>   
>   	iscsi_bsg_host_add(shost, ihost);
>   	/* ignore any bsg add error - we just can't do sgio */
> diff --git a/include/scsi/scsi_transport_iscsi.h b/include/scsi/scsi_transport_iscsi.h
> index af0c5a15f316..f8885d0c37d8 100644
> --- a/include/scsi/scsi_transport_iscsi.h
> +++ b/include/scsi/scsi_transport_iscsi.h
> @@ -156,6 +156,7 @@ struct iscsi_transport {
>   	int (*logout_flashnode_sid) (struct iscsi_cls_session *cls_sess);
>   	int (*get_host_stats) (struct Scsi_Host *shost, char *buf, int len);
>   	u8 (*check_protection)(struct iscsi_task *task, sector_t *sector);
> +	struct net *(*get_netns)(struct Scsi_Host *shost);
>   };
>   
>   /*

Cheers,

Hannes

Chris Leech April 11, 2023, 12:21 a.m. UTC | #2

On Tue, Mar 14, 2023 at 05:29:25PM +0100, Hannes Reinecke wrote:
> On 2/8/23 18:40, Lee Duncan wrote:
> > From: Lee Duncan <lduncan@suse.com>
> > 
> > This lets iscsi_tcp operate in multiple namespaces.  It uses current
> > during session creation to find the net namespace, but it might be
> > better to manage to pass it along from the iscsi netlink socket.
> > 
> And indeed, I'd rather use the namespace from the iscsi netlink socket.
> If you use the namespace from session creation you'd better hope that
> this function is not called from a workqueue ...

The cleanest way I see to do this is to split the transport
session_create function between bound and unbound, instead of checking
for a NULL ep.  That should cleanly serperate out the host-per-session
behavior of iscsi_tcp, so we can pass in the namespace without changing
the other drivers.

This is what that looks like on top of the existing patches, but we can
merge it in and rearrange if desired.

- Chris

---

Distinguish between bound and unbound session creation with different
transport functions, instead of just checking for a NULL endpoint.

This let's the transport code pass the network namespace into the
unbound session creation of iscsi_tcp, without changing the offloading
drivers which all expect an bound endpoint.

iSER has compatibility checks to work without a bound endpoint, so
expose both transport functions there.

Signed-off-by: Chris Leech <cleech@redhat.com>
---
 drivers/infiniband/ulp/iser/iscsi_iser.c | 41 +++++++++++++++++-------
 drivers/scsi/iscsi_tcp.c                 | 16 ++++-----
 drivers/scsi/iscsi_tcp.h                 |  1 +
 drivers/scsi/scsi_transport_iscsi.c      | 17 +++++++---
 include/scsi/scsi_transport_iscsi.h      |  3 ++
 5 files changed, 52 insertions(+), 26 deletions(-)

diff --git a/drivers/infiniband/ulp/iser/iscsi_iser.c b/drivers/infiniband/ulp/iser/iscsi_iser.c
index 6865f62eb831..ca8de612d585 100644
--- a/drivers/infiniband/ulp/iser/iscsi_iser.c
+++ b/drivers/infiniband/ulp/iser/iscsi_iser.c
@@ -593,20 +593,10 @@ static inline unsigned int iser_dif_prot_caps(int prot_caps)
 	return ret;
 }
 
-/**
- * iscsi_iser_session_create() - create an iscsi-iser session
- * @ep:             iscsi end-point handle
- * @cmds_max:       maximum commands in this session
- * @qdepth:         session command queue depth
- * @initial_cmdsn:  initiator command sequnce number
- *
- * Allocates and adds a scsi host, expose DIF supprot if
- * exists, and sets up an iscsi session.
- */
 static struct iscsi_cls_session *
-iscsi_iser_session_create(struct iscsi_endpoint *ep,
+__iscsi_iser_session_create(struct iscsi_endpoint *ep,
 			  uint16_t cmds_max, uint16_t qdepth,
-			  uint32_t initial_cmdsn)
+			  uint32_t initial_cmdsn, struct net *net)
 {
 	struct iscsi_cls_session *cls_session;
 	struct Scsi_Host *shost;
@@ -694,6 +684,32 @@ iscsi_iser_session_create(struct iscsi_endpoint *ep,
 	return NULL;
 }
 
+/**
+ * iscsi_iser_session_create() - create an iscsi-iser session
+ * @ep:             iscsi end-point handle
+ * @cmds_max:       maximum commands in this session
+ * @qdepth:         session command queue depth
+ * @initial_cmdsn:  initiator command sequnce number
+ *
+ * Allocates and adds a scsi host, expose DIF supprot if
+ * exists, and sets up an iscsi session.
+ */
+static struct iscsi_cls_session *
+iscsi_iser_session_create(struct iscsi_endpoint *ep,
+			  uint16_t cmds_max, uint16_t qdepth,
+			  uint32_t initial_cmdsn) {
+	return __iscsi_iser_session_create(ep, cmds_max, qdepth,
+					   initial_cmdsn, NULL);
+}
+
+static struct iscsi_cls_session *
+iscsi_iser_unbound_session_create(struct net *net,
+				  uint16_t cmds_max, uint16_t qdepth,
+				  uint32_t initial_cmdsn) {
+	return __iscsi_iser_session_create(NULL, cmds_max, qdepth,
+					   initial_cmdsn, net);
+}
+
 static int iscsi_iser_set_param(struct iscsi_cls_conn *cls_conn,
 				enum iscsi_param param, char *buf, int buflen)
 {
@@ -983,6 +999,7 @@ static struct iscsi_transport iscsi_iser_transport = {
 	.caps                   = CAP_RECOVERY_L0 | CAP_MULTI_R2T | CAP_TEXT_NEGO,
 	/* session management */
 	.create_session         = iscsi_iser_session_create,
+	.create_unbound_session = iscsi_iser_unbound_session_create,
 	.destroy_session        = iscsi_iser_session_destroy,
 	/* connection management */
 	.create_conn            = iscsi_iser_conn_create,
diff --git a/drivers/scsi/iscsi_tcp.c b/drivers/scsi/iscsi_tcp.c
index 171685011ad9..b78239f25073 100644
--- a/drivers/scsi/iscsi_tcp.c
+++ b/drivers/scsi/iscsi_tcp.c
@@ -922,7 +922,7 @@ iscsi_sw_tcp_conn_get_stats(struct iscsi_cls_conn *cls_conn,
 }
 
 static struct iscsi_cls_session *
-iscsi_sw_tcp_session_create(struct iscsi_endpoint *ep, uint16_t cmds_max,
+iscsi_sw_tcp_session_create(struct net *net, uint16_t cmds_max,
 			    uint16_t qdepth, uint32_t initial_cmdsn)
 {
 	struct iscsi_cls_session *cls_session;
@@ -931,11 +931,6 @@ iscsi_sw_tcp_session_create(struct iscsi_endpoint *ep, uint16_t cmds_max,
 	struct Scsi_Host *shost;
 	int rc;
 
-	if (ep) {
-		printk(KERN_ERR "iscsi_tcp: invalid ep %p.\n", ep);
-		return NULL;
-	}
-
 	shost = iscsi_host_alloc(&iscsi_sw_tcp_sht,
 				 sizeof(struct iscsi_sw_tcp_host), 1);
 	if (!shost)
@@ -952,6 +947,9 @@ iscsi_sw_tcp_session_create(struct iscsi_endpoint *ep, uint16_t cmds_max,
 		goto free_host;
 	shost->can_queue = rc;
 
+	tcp_sw_host = iscsi_host_priv(shost);
+	tcp_sw_host->net_ns = net;
+
 	if (iscsi_host_add(shost, NULL))
 		goto free_host;
 
@@ -968,7 +966,6 @@ iscsi_sw_tcp_session_create(struct iscsi_endpoint *ep, uint16_t cmds_max,
 		goto remove_session;
 
 	/* We are now fully setup so expose the session to sysfs. */
-	tcp_sw_host = iscsi_host_priv(shost);
 	tcp_sw_host->session = session;
 	return cls_session;
 
@@ -1074,7 +1071,8 @@ static int iscsi_sw_tcp_slave_configure(struct scsi_device *sdev)
 
 static struct net *iscsi_sw_tcp_netns(struct Scsi_Host *shost)
 {
-	return current->nsproxy->net_ns;
+	struct iscsi_sw_tcp_host *tcp_sw_host = iscsi_host_priv(shost);
+	return tcp_sw_host->net_ns;
 }
 
 static struct scsi_host_template iscsi_sw_tcp_sht = {
@@ -1104,7 +1102,7 @@ static struct iscsi_transport iscsi_sw_tcp_transport = {
 	.caps			= CAP_RECOVERY_L0 | CAP_MULTI_R2T | CAP_HDRDGST
 				  | CAP_DATADGST,
 	/* session management */
-	.create_session		= iscsi_sw_tcp_session_create,
+	.create_unbound_session	= iscsi_sw_tcp_session_create,
 	.destroy_session	= iscsi_sw_tcp_session_destroy,
 	/* connection management */
 	.create_conn		= iscsi_sw_tcp_conn_create,
diff --git a/drivers/scsi/iscsi_tcp.h b/drivers/scsi/iscsi_tcp.h
index 68e14a344904..f0020cb22f59 100644
--- a/drivers/scsi/iscsi_tcp.h
+++ b/drivers/scsi/iscsi_tcp.h
@@ -53,6 +53,7 @@ struct iscsi_sw_tcp_conn {
 
 struct iscsi_sw_tcp_host {
 	struct iscsi_session	*session;
+	struct net *net_ns;
 };
 
 struct iscsi_sw_tcp_hdrbuf {
diff --git a/drivers/scsi/scsi_transport_iscsi.c b/drivers/scsi/scsi_transport_iscsi.c
index 8fafa8f0e0df..4d346e79468e 100644
--- a/drivers/scsi/scsi_transport_iscsi.c
+++ b/drivers/scsi/scsi_transport_iscsi.c
@@ -3144,14 +3144,21 @@ static int
 iscsi_if_create_session(struct iscsi_internal *priv, struct iscsi_endpoint *ep,
 			struct iscsi_uevent *ev, pid_t pid,
 			uint32_t initial_cmdsn,	uint16_t cmds_max,
-			uint16_t queue_depth)
+			uint16_t queue_depth, struct net *net)
 {
 	struct iscsi_transport *transport = priv->iscsi_transport;
 	struct iscsi_cls_session *session;
 	struct Scsi_Host *shost;
 
-	session = transport->create_session(ep, cmds_max, queue_depth,
-					    initial_cmdsn);
+	if (ep) {
+		session = transport->create_session(ep, cmds_max, queue_depth,
+						    initial_cmdsn);
+	} else {
+		session = transport->create_unbound_session(net, cmds_max,
+							    queue_depth,
+							    initial_cmdsn);
+	}
+
 	if (!session)
 		return -ENOMEM;
 
@@ -4145,7 +4152,7 @@ iscsi_if_recv_msg(struct net *net, struct sk_buff *skb,
 					      portid,
 					      ev->u.c_session.initial_cmdsn,
 					      ev->u.c_session.cmds_max,
-					      ev->u.c_session.queue_depth);
+					      ev->u.c_session.queue_depth, net);
 		break;
 	/* MARK */
 	case ISCSI_UEVENT_CREATE_BOUND_SESSION:
@@ -4160,7 +4167,7 @@ iscsi_if_recv_msg(struct net *net, struct sk_buff *skb,
 					portid,
 					ev->u.c_bound_session.initial_cmdsn,
 					ev->u.c_bound_session.cmds_max,
-					ev->u.c_bound_session.queue_depth);
+					ev->u.c_bound_session.queue_depth, net);
 		iscsi_put_endpoint(ep);
 		break;
 	case ISCSI_UEVENT_DESTROY_SESSION:
diff --git a/include/scsi/scsi_transport_iscsi.h b/include/scsi/scsi_transport_iscsi.h
index 0c3fd690ecf8..4d8a3d770bed 100644
--- a/include/scsi/scsi_transport_iscsi.h
+++ b/include/scsi/scsi_transport_iscsi.h
@@ -79,6 +79,9 @@ struct iscsi_transport {
 	struct iscsi_cls_session *(*create_session) (struct iscsi_endpoint *ep,
 					uint16_t cmds_max, uint16_t qdepth,
 					uint32_t sn);
+	struct iscsi_cls_session *(*create_unbound_session) (struct net *net,
+					uint16_t cmds_max, uint16_t qdepth,
+					uint32_t sn);
 	void (*destroy_session) (struct iscsi_cls_session *session);
 	struct iscsi_cls_conn *(*create_conn) (struct iscsi_cls_session *sess,
 				uint32_t cid);

Hannes Reinecke April 11, 2023, 6:58 a.m. UTC | #3

On 4/11/23 02:21, Chris Leech wrote:
> On Tue, Mar 14, 2023 at 05:29:25PM +0100, Hannes Reinecke wrote:
>> On 2/8/23 18:40, Lee Duncan wrote:
>>> From: Lee Duncan <lduncan@suse.com>
>>>
>>> This lets iscsi_tcp operate in multiple namespaces.  It uses current
>>> during session creation to find the net namespace, but it might be
>>> better to manage to pass it along from the iscsi netlink socket.
>>>
>> And indeed, I'd rather use the namespace from the iscsi netlink socket.
>> If you use the namespace from session creation you'd better hope that
>> this function is not called from a workqueue ...
> 
> The cleanest way I see to do this is to split the transport
> session_create function between bound and unbound, instead of checking
> for a NULL ep.  That should cleanly serperate out the host-per-session
> behavior of iscsi_tcp, so we can pass in the namespace without changing
> the other drivers.
> 
> This is what that looks like on top of the existing patches, but we can
> merge it in and rearrange if desired.
> 
> - Chris
> 
> ---
> 
> Distinguish between bound and unbound session creation with different
> transport functions, instead of just checking for a NULL endpoint.
> 
> This let's the transport code pass the network namespace into the
> unbound session creation of iscsi_tcp, without changing the offloading
> drivers which all expect an bound endpoint.
> 
> iSER has compatibility checks to work without a bound endpoint, so
> expose both transport functions there.
> 
> Signed-off-by: Chris Leech <cleech@redhat.com>
> ---
>   drivers/infiniband/ulp/iser/iscsi_iser.c | 41 +++++++++++++++++-------
>   drivers/scsi/iscsi_tcp.c                 | 16 ++++-----
>   drivers/scsi/iscsi_tcp.h                 |  1 +
>   drivers/scsi/scsi_transport_iscsi.c      | 17 +++++++---
>   include/scsi/scsi_transport_iscsi.h      |  3 ++
>   5 files changed, 52 insertions(+), 26 deletions(-)
> 
> diff --git a/drivers/infiniband/ulp/iser/iscsi_iser.c b/drivers/infiniband/ulp/iser/iscsi_iser.c
> index 6865f62eb831..ca8de612d585 100644
> --- a/drivers/infiniband/ulp/iser/iscsi_iser.c
> +++ b/drivers/infiniband/ulp/iser/iscsi_iser.c
> @@ -593,20 +593,10 @@ static inline unsigned int iser_dif_prot_caps(int prot_caps)
>   	return ret;
>   }
>   
> -/**
> - * iscsi_iser_session_create() - create an iscsi-iser session
> - * @ep:             iscsi end-point handle
> - * @cmds_max:       maximum commands in this session
> - * @qdepth:         session command queue depth
> - * @initial_cmdsn:  initiator command sequnce number
> - *
> - * Allocates and adds a scsi host, expose DIF supprot if
> - * exists, and sets up an iscsi session.
> - */
>   static struct iscsi_cls_session *
> -iscsi_iser_session_create(struct iscsi_endpoint *ep,
> +__iscsi_iser_session_create(struct iscsi_endpoint *ep,
>   			  uint16_t cmds_max, uint16_t qdepth,
> -			  uint32_t initial_cmdsn)
> +			  uint32_t initial_cmdsn, struct net *net)
>   {
>   	struct iscsi_cls_session *cls_session;
>   	struct Scsi_Host *shost;
> @@ -694,6 +684,32 @@ iscsi_iser_session_create(struct iscsi_endpoint *ep,
>   	return NULL;
>   }
>   
> +/**
> + * iscsi_iser_session_create() - create an iscsi-iser session
> + * @ep:             iscsi end-point handle
> + * @cmds_max:       maximum commands in this session
> + * @qdepth:         session command queue depth
> + * @initial_cmdsn:  initiator command sequnce number
> + *
> + * Allocates and adds a scsi host, expose DIF supprot if
> + * exists, and sets up an iscsi session.
> + */
> +static struct iscsi_cls_session *
> +iscsi_iser_session_create(struct iscsi_endpoint *ep,
> +			  uint16_t cmds_max, uint16_t qdepth,
> +			  uint32_t initial_cmdsn) {
> +	return __iscsi_iser_session_create(ep, cmds_max, qdepth,
> +					   initial_cmdsn, NULL);
> +}
> +
> +static struct iscsi_cls_session *
> +iscsi_iser_unbound_session_create(struct net *net,
> +				  uint16_t cmds_max, uint16_t qdepth,
> +				  uint32_t initial_cmdsn) {
> +	return __iscsi_iser_session_create(NULL, cmds_max, qdepth,
> +					   initial_cmdsn, net);
> +}
> +
>   static int iscsi_iser_set_param(struct iscsi_cls_conn *cls_conn,
>   				enum iscsi_param param, char *buf, int buflen)
>   {
> @@ -983,6 +999,7 @@ static struct iscsi_transport iscsi_iser_transport = {
>   	.caps                   = CAP_RECOVERY_L0 | CAP_MULTI_R2T | CAP_TEXT_NEGO,
>   	/* session management */
>   	.create_session         = iscsi_iser_session_create,
> +	.create_unbound_session = iscsi_iser_unbound_session_create,
>   	.destroy_session        = iscsi_iser_session_destroy,
>   	/* connection management */
>   	.create_conn            = iscsi_iser_conn_create,
> diff --git a/drivers/scsi/iscsi_tcp.c b/drivers/scsi/iscsi_tcp.c
> index 171685011ad9..b78239f25073 100644
> --- a/drivers/scsi/iscsi_tcp.c
> +++ b/drivers/scsi/iscsi_tcp.c
> @@ -922,7 +922,7 @@ iscsi_sw_tcp_conn_get_stats(struct iscsi_cls_conn *cls_conn,
>   }
>   
>   static struct iscsi_cls_session *
> -iscsi_sw_tcp_session_create(struct iscsi_endpoint *ep, uint16_t cmds_max,
> +iscsi_sw_tcp_session_create(struct net *net, uint16_t cmds_max,
>   			    uint16_t qdepth, uint32_t initial_cmdsn)
>   {
>   	struct iscsi_cls_session *cls_session;
> @@ -931,11 +931,6 @@ iscsi_sw_tcp_session_create(struct iscsi_endpoint *ep, uint16_t cmds_max,
>   	struct Scsi_Host *shost;
>   	int rc;
>   
> -	if (ep) {
> -		printk(KERN_ERR "iscsi_tcp: invalid ep %p.\n", ep);
> -		return NULL;
> -	}
> -
>   	shost = iscsi_host_alloc(&iscsi_sw_tcp_sht,
>   				 sizeof(struct iscsi_sw_tcp_host), 1);
>   	if (!shost)
> @@ -952,6 +947,9 @@ iscsi_sw_tcp_session_create(struct iscsi_endpoint *ep, uint16_t cmds_max,
>   		goto free_host;
>   	shost->can_queue = rc;
>   
> +	tcp_sw_host = iscsi_host_priv(shost);
> +	tcp_sw_host->net_ns = net;
> +
>   	if (iscsi_host_add(shost, NULL))
>   		goto free_host;
>   
> @@ -968,7 +966,6 @@ iscsi_sw_tcp_session_create(struct iscsi_endpoint *ep, uint16_t cmds_max,
>   		goto remove_session;
>   
>   	/* We are now fully setup so expose the session to sysfs. */
> -	tcp_sw_host = iscsi_host_priv(shost);
>   	tcp_sw_host->session = session;
>   	return cls_session;
>   
> @@ -1074,7 +1071,8 @@ static int iscsi_sw_tcp_slave_configure(struct scsi_device *sdev)
>   
>   static struct net *iscsi_sw_tcp_netns(struct Scsi_Host *shost)
>   {
> -	return current->nsproxy->net_ns;
> +	struct iscsi_sw_tcp_host *tcp_sw_host = iscsi_host_priv(shost);
> +	return tcp_sw_host->net_ns;
>   }
>   
>   static struct scsi_host_template iscsi_sw_tcp_sht = {
> @@ -1104,7 +1102,7 @@ static struct iscsi_transport iscsi_sw_tcp_transport = {
>   	.caps			= CAP_RECOVERY_L0 | CAP_MULTI_R2T | CAP_HDRDGST
>   				  | CAP_DATADGST,
>   	/* session management */
> -	.create_session		= iscsi_sw_tcp_session_create,
> +	.create_unbound_session	= iscsi_sw_tcp_session_create,
>   	.destroy_session	= iscsi_sw_tcp_session_destroy,
>   	/* connection management */
>   	.create_conn		= iscsi_sw_tcp_conn_create,
> diff --git a/drivers/scsi/iscsi_tcp.h b/drivers/scsi/iscsi_tcp.h
> index 68e14a344904..f0020cb22f59 100644
> --- a/drivers/scsi/iscsi_tcp.h
> +++ b/drivers/scsi/iscsi_tcp.h
> @@ -53,6 +53,7 @@ struct iscsi_sw_tcp_conn {
>   
>   struct iscsi_sw_tcp_host {
>   	struct iscsi_session	*session;
> +	struct net *net_ns;
>   };
>   
>   struct iscsi_sw_tcp_hdrbuf {
> diff --git a/drivers/scsi/scsi_transport_iscsi.c b/drivers/scsi/scsi_transport_iscsi.c
> index 8fafa8f0e0df..4d346e79468e 100644
> --- a/drivers/scsi/scsi_transport_iscsi.c
> +++ b/drivers/scsi/scsi_transport_iscsi.c
> @@ -3144,14 +3144,21 @@ static int
>   iscsi_if_create_session(struct iscsi_internal *priv, struct iscsi_endpoint *ep,
>   			struct iscsi_uevent *ev, pid_t pid,
>   			uint32_t initial_cmdsn,	uint16_t cmds_max,
> -			uint16_t queue_depth)
> +			uint16_t queue_depth, struct net *net)
>   {
>   	struct iscsi_transport *transport = priv->iscsi_transport;
>   	struct iscsi_cls_session *session;
>   	struct Scsi_Host *shost;
>   
> -	session = transport->create_session(ep, cmds_max, queue_depth,
> -					    initial_cmdsn);
> +	if (ep) {
> +		session = transport->create_session(ep, cmds_max, queue_depth,
> +						    initial_cmdsn);
> +	} else {
> +		session = transport->create_unbound_session(net, cmds_max,
> +							    queue_depth,
> +							    initial_cmdsn);
> +	}
> +
>   	if (!session)
>   		return -ENOMEM;
>   
> @@ -4145,7 +4152,7 @@ iscsi_if_recv_msg(struct net *net, struct sk_buff *skb,
>   					      portid,
>   					      ev->u.c_session.initial_cmdsn,
>   					      ev->u.c_session.cmds_max,
> -					      ev->u.c_session.queue_depth);
> +					      ev->u.c_session.queue_depth, net);
>   		break;
>   	/* MARK */
>   	case ISCSI_UEVENT_CREATE_BOUND_SESSION:
> @@ -4160,7 +4167,7 @@ iscsi_if_recv_msg(struct net *net, struct sk_buff *skb,
>   					portid,
>   					ev->u.c_bound_session.initial_cmdsn,
>   					ev->u.c_bound_session.cmds_max,
> -					ev->u.c_bound_session.queue_depth);
> +					ev->u.c_bound_session.queue_depth, net);
>   		iscsi_put_endpoint(ep);
>   		break;
>   	case ISCSI_UEVENT_DESTROY_SESSION:
> diff --git a/include/scsi/scsi_transport_iscsi.h b/include/scsi/scsi_transport_iscsi.h
> index 0c3fd690ecf8..4d8a3d770bed 100644
> --- a/include/scsi/scsi_transport_iscsi.h
> +++ b/include/scsi/scsi_transport_iscsi.h
> @@ -79,6 +79,9 @@ struct iscsi_transport {
>   	struct iscsi_cls_session *(*create_session) (struct iscsi_endpoint *ep,
>   					uint16_t cmds_max, uint16_t qdepth,
>   					uint32_t sn);
> +	struct iscsi_cls_session *(*create_unbound_session) (struct net *net,
> +					uint16_t cmds_max, uint16_t qdepth,
> +					uint32_t sn);
>   	void (*destroy_session) (struct iscsi_cls_session *session);
>   	struct iscsi_cls_conn *(*create_conn) (struct iscsi_cls_session *sess,
>   				uint32_t cid);

I'm not _that_ happy with these two functions; but can't really see a 
way around it.
Can't we rename the 'unbound' version to
'create_session_ns' or something?

Cheers,

Hannes

Chris Leech April 11, 2023, 6:03 p.m. UTC | #4

On Tue, Apr 11, 2023 at 08:58:54AM +0200, Hannes Reinecke wrote:
> On 4/11/23 02:21, Chris Leech wrote:
> > diff --git a/include/scsi/scsi_transport_iscsi.h b/include/scsi/scsi_transport_iscsi.h
> > index 0c3fd690ecf8..4d8a3d770bed 100644
> > --- a/include/scsi/scsi_transport_iscsi.h
> > +++ b/include/scsi/scsi_transport_iscsi.h
> > @@ -79,6 +79,9 @@ struct iscsi_transport {
> >   	struct iscsi_cls_session *(*create_session) (struct iscsi_endpoint *ep,
> >   					uint16_t cmds_max, uint16_t qdepth,
> >   					uint32_t sn);
> > +	struct iscsi_cls_session *(*create_unbound_session) (struct net *net,
> > +					uint16_t cmds_max, uint16_t qdepth,
> > +					uint32_t sn);
> >   	void (*destroy_session) (struct iscsi_cls_session *session);
> >   	struct iscsi_cls_conn *(*create_conn) (struct iscsi_cls_session *sess,
> >   				uint32_t cid);
> 
> I'm not _that_ happy with these two functions; but can't really see a way
> around it.
> Can't we rename the 'unbound' version to
> 'create_session_ns' or something?

Yes, in my mind I was matching the netlink commands, but those are
create_session and create_bound_session. I got it exactly backwards
with which one had the additional text.

I'm OK with changing to a shorter name, like the one you suggested.

Thanks,
- Chris

diff --git a/drivers/scsi/iscsi_tcp.c b/drivers/scsi/iscsi_tcp.c
index 0454d94e8cf0..22e7a5c93627 100644
--- a/drivers/scsi/iscsi_tcp.c
+++ b/drivers/scsi/iscsi_tcp.c
@@ -1069,6 +1069,11 @@  static int iscsi_sw_tcp_slave_configure(struct scsi_device *sdev)
 	return 0;
 }
 
+static struct net *iscsi_sw_tcp_netns(struct Scsi_Host *shost)
+{
+	return current->nsproxy->net_ns;
+}
+
 static struct scsi_host_template iscsi_sw_tcp_sht = {
 	.module			= THIS_MODULE,
 	.name			= "iSCSI Initiator over TCP/IP",
@@ -1124,6 +1129,8 @@  static struct iscsi_transport iscsi_sw_tcp_transport = {
 	.alloc_pdu		= iscsi_sw_tcp_pdu_alloc,
 	/* recovery */
 	.session_recovery_timedout = iscsi_session_recovery_timedout,
+	/* net namespace */
+	.get_netns		= iscsi_sw_tcp_netns,
 };
 
 static int __init iscsi_sw_tcp_init(void)
diff --git a/drivers/scsi/scsi_transport_iscsi.c b/drivers/scsi/scsi_transport_iscsi.c
index 230b43d34c5f..996a9abfa1f5 100644
--- a/drivers/scsi/scsi_transport_iscsi.c
+++ b/drivers/scsi/scsi_transport_iscsi.c
@@ -1600,10 +1600,15 @@  static int iscsi_setup_host(struct transport_container *tc, struct device *dev,
 {
 	struct Scsi_Host *shost = dev_to_shost(dev);
 	struct iscsi_cls_host *ihost = shost->shost_data;
+	struct iscsi_internal *priv = to_iscsi_internal(shost->transportt);
+	struct iscsi_transport *transport = priv->iscsi_transport;
 
 	memset(ihost, 0, sizeof(*ihost));
 	mutex_init(&ihost->mutex);
-	ihost->netns = &init_net;
+	if (transport->get_netns)
+		ihost->netns = transport->get_netns(shost);
+	else
+		ihost->netns = &init_net;
 
 	iscsi_bsg_host_add(shost, ihost);
 	/* ignore any bsg add error - we just can't do sgio */
diff --git a/include/scsi/scsi_transport_iscsi.h b/include/scsi/scsi_transport_iscsi.h
index af0c5a15f316..f8885d0c37d8 100644
--- a/include/scsi/scsi_transport_iscsi.h
+++ b/include/scsi/scsi_transport_iscsi.h
@@ -156,6 +156,7 @@  struct iscsi_transport {
 	int (*logout_flashnode_sid) (struct iscsi_cls_session *cls_sess);
 	int (*get_host_stats) (struct Scsi_Host *shost, char *buf, int len);
 	u8 (*check_protection)(struct iscsi_task *task, sector_t *sector);
+	struct net *(*get_netns)(struct Scsi_Host *shost);
 };
 
 /*

[RFC,5/9] iscsi: set netns for iscsi_tcp hosts

Commit Message

Comments

Patch