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

2023-04-10 Thread Chris Leech
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 
> > 
> > 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 
---
 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 *

[PATCH 10/11] iscsi: make session and connection lists per-net

2023-04-10 Thread Chris Leech
Eliminate the comparisions on list lookups, and it will make it easier
to shut down session on net namespace exit in the next patch.

Signed-off-by: Chris Leech 
---
 drivers/scsi/scsi_transport_iscsi.c | 104 
 1 file changed, 61 insertions(+), 43 deletions(-)

diff --git a/drivers/scsi/scsi_transport_iscsi.c 
b/drivers/scsi/scsi_transport_iscsi.c
index 9a176ea0d866..3a068d8eca2d 100644
--- a/drivers/scsi/scsi_transport_iscsi.c
+++ b/drivers/scsi/scsi_transport_iscsi.c
@@ -1734,17 +1734,16 @@ static 
DECLARE_TRANSPORT_CLASS_NS(iscsi_connection_class,
 
 struct iscsi_net {
struct sock *nls;
+   spinlock_t sesslock;
+   struct list_head sesslist;
+   spinlock_t connlock;
+   struct list_head connlist;
+   struct list_head connlist_err;
 };
 
 static int iscsi_net_id __read_mostly;
 static DEFINE_MUTEX(rx_queue_mutex);
 
-static LIST_HEAD(sesslist);
-static DEFINE_SPINLOCK(sesslock);
-static LIST_HEAD(connlist);
-static LIST_HEAD(connlist_err);
-static DEFINE_SPINLOCK(connlock);
-
 static uint32_t iscsi_conn_get_sid(struct iscsi_cls_conn *conn)
 {
struct iscsi_cls_session *sess = iscsi_dev_to_session(conn->dev.parent);
@@ -1759,19 +1758,18 @@ static struct iscsi_cls_session 
*iscsi_session_lookup(struct net *net,
 {
unsigned long flags;
struct iscsi_cls_session *sess;
-   struct net *ns;
+   struct iscsi_net *isn;
 
-   spin_lock_irqsave(, flags);
-   list_for_each_entry(sess, , sess_list) {
+   isn = net_generic(net, iscsi_net_id);
+
+   spin_lock_irqsave(>sesslock, flags);
+   list_for_each_entry(sess, >sesslist, sess_list) {
if (sess->sid == sid) {
-   ns = iscsi_sess_net(sess);
-   if (ns != net)
-   continue;
-   spin_unlock_irqrestore(, flags);
+   spin_unlock_irqrestore(>sesslock, flags);
return sess;
}
}
-   spin_unlock_irqrestore(, flags);
+   spin_unlock_irqrestore(>sesslock, flags);
return NULL;
 }
 
@@ -1783,19 +1781,18 @@ static struct iscsi_cls_conn *iscsi_conn_lookup(struct 
net *net, uint32_t sid,
 {
unsigned long flags;
struct iscsi_cls_conn *conn;
-   struct net *ns;
+   struct iscsi_net *isn;
 
-   spin_lock_irqsave(, flags);
-   list_for_each_entry(conn, , conn_list) {
+   isn = net_generic(net, iscsi_net_id);
+
+   spin_lock_irqsave(>connlock, flags);
+   list_for_each_entry(conn, >connlist, conn_list) {
if ((conn->cid == cid) && (iscsi_conn_get_sid(conn) == sid)) {
-   ns = iscsi_conn_net(conn);
-   if (ns != net)
-   continue;
-   spin_unlock_irqrestore(, flags);
+   spin_unlock_irqrestore(>connlock, flags);
return conn;
}
}
-   spin_unlock_irqrestore(, flags);
+   spin_unlock_irqrestore(>connlock, flags);
return NULL;
 }
 
@@ -2207,6 +2204,9 @@ EXPORT_SYMBOL_GPL(iscsi_alloc_session);
 int iscsi_add_session(struct iscsi_cls_session *session, unsigned int 
target_id)
 {
struct Scsi_Host *shost = iscsi_session_to_shost(session);
+   struct iscsi_cls_host *ihost = shost->shost_data;
+   struct net *net = iscsi_host_net(ihost);
+   struct iscsi_net *isn = net_generic(net, iscsi_net_id);
unsigned long flags;
int id = 0;
int err;
@@ -2250,9 +2250,9 @@ int iscsi_add_session(struct iscsi_cls_session *session, 
unsigned int target_id)
goto release_dev;
}
 
-   spin_lock_irqsave(, flags);
-   list_add(>sess_list, );
-   spin_unlock_irqrestore(, flags);
+   spin_lock_irqsave(>sesslock, flags);
+   list_add(>sess_list, >sesslist);
+   spin_unlock_irqrestore(>sesslock, flags);
 
iscsi_session_event(session, ISCSI_KEVENT_CREATE_SESSION);
ISCSI_DBG_TRANS_SESSION(session, "Completed session adding\n");
@@ -2322,15 +2322,17 @@ static int iscsi_iter_destroy_conn_fn(struct device 
*dev, void *data)
 
 void iscsi_remove_session(struct iscsi_cls_session *session)
 {
+   struct net *net = iscsi_sess_net(session);
+   struct iscsi_net *isn = net_generic(net, iscsi_net_id);
unsigned long flags;
int err;
 
ISCSI_DBG_TRANS_SESSION(session, "Removing session\n");
 
-   spin_lock_irqsave(, flags);
+   spin_lock_irqsave(>sesslock, flags);
if (!list_empty(>sess_list))
list_del(>sess_list);
-   spin_unlock_irqrestore(, flags);
+   spin_unlock_irqrestore(>sesslock, flags);
 
if (!cancel_work_sync(>block_work))
cancel_delayed_work_sync(>recovery_work);
@@ -2541,20 +2543,22 @@ static int iscsi_iter_force_destroy_conn_fn(struct 
device *dev, void *data)
 void iscsi_force_destroy_session(struct 

[PATCH 11/11] iscsi: force destroy sesions when a network namespace exits

2023-04-10 Thread Chris Leech
The namespace is gone, so there is no userspace to clean up.
Force close all the sessions.

This should be enough for software transports, there's no implementation
of migrating physical iSCSI hosts between network namespaces currently.

Signed-off-by: Chris Leech 
---
 drivers/scsi/scsi_transport_iscsi.c | 18 ++
 1 file changed, 18 insertions(+)

diff --git a/drivers/scsi/scsi_transport_iscsi.c 
b/drivers/scsi/scsi_transport_iscsi.c
index 3a068d8eca2d..8fafa8f0e0df 100644
--- a/drivers/scsi/scsi_transport_iscsi.c
+++ b/drivers/scsi/scsi_transport_iscsi.c
@@ -5200,9 +5200,27 @@ static int __net_init iscsi_net_init(struct net *net)
 
 static void __net_exit iscsi_net_exit(struct net *net)
 {
+   struct iscsi_cls_session *session, *tmp;
+   struct iscsi_transport *transport;
struct iscsi_net *isn;
+   unsigned long flags;
+   LIST_HEAD(sessions);
 
isn = net_generic(net, iscsi_net_id);
+
+   /* force session destruction, there is no userspace anymore */
+   spin_lock_irqsave(>sesslock, flags);
+   list_for_each_entry_safe(session, tmp, >sesslist, sess_list) {
+   list_move_tail(>sess_list, );
+   }
+   spin_unlock_irqrestore(>sesslock, flags);
+   list_for_each_entry_safe(session, tmp, , sess_list) {
+   device_for_each_child(>dev, NULL,
+ iscsi_iter_force_destroy_conn_fn);
+   transport = session->transport;
+   transport->destroy_session(session);
+   }
+
netlink_kernel_release(isn->nls);
isn->nls = NULL;
 }
-- 
2.39.2

-- 
You received this message because you are subscribed to the Google Groups 
"open-iscsi" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to open-iscsi+unsubscr...@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/open-iscsi/20230410191033.1069293-3-cleech%40redhat.com.


Re: [RFC PATCH 4/9] iscsi: make all iSCSI netlink multicast namespace aware

2023-04-10 Thread Chris Leech
> As discussed with Lee: you should tear down sessions related to this
> namespace from the pernet ->exit callback, otherwise you end up with
> session which can no longer been reached as the netlink socket is
> gone.

These two follow on changes handle removing active sesions when the
namespace exits. Tested with iscsi_tcp and seems to be working for me.

Chris Leech (2):
  iscsi: make session and connection lists per-net
  iscsi: force destroy sesions when a network namespace exits

 drivers/scsi/scsi_transport_iscsi.c | 122 ++--
 1 file changed, 79 insertions(+), 43 deletions(-)

-- 
2.39.2

-- 
You received this message because you are subscribed to the Google Groups 
"open-iscsi" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to open-iscsi+unsubscr...@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/open-iscsi/20230410191033.1069293-1-cleech%40redhat.com.