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

[Lee Duncan]

From: Lee Duncan <ldu...@suse.com>

Make use of the per-net netlink sockets. Responses are sent back on the
same socket/namespace the request was received on. Async events are
reported on the socket/namespace stored in the iscsi_cls_host associated
with the event.

Signed-off-by: Chris Leech <cle...@redhat.com>
Signed-off-by: Lee Duncan <ldu...@suse.com>
---
drivers/scsi/scsi_transport_iscsi.c | 92 +++++++++++++++++++----------
1 file changed, 60 insertions(+), 32 deletions(-)

diff --git a/drivers/scsi/scsi_transport_iscsi.c b/drivers/scsi/scsi_transport_iscsi.c
index 2e2b291bce41..230b43d34c5f 100644
--- a/drivers/scsi/scsi_transport_iscsi.c
+++ b/drivers/scsi/scsi_transport_iscsi.c
@@ -2653,8 +2653,8 @@ iscsi_if_transport_lookup(struct iscsi_transport *tt)
}

static int
-iscsi_multicast_netns(struct net *net, struct sk_buff *skb,
- uint32_t group, gfp_t gfp)
+iscsi_multicast_skb(struct net *net, struct sk_buff *skb,
+ uint32_t group, gfp_t gfp)
{
struct sock *nls;
struct iscsi_net *isn;
@@ -2665,17 +2665,10 @@ iscsi_multicast_netns(struct net *net, struct sk_buff *skb,
}

static int
-iscsi_multicast_skb(struct sk_buff *skb, uint32_t group, gfp_t gfp)
-{
- return iscsi_multicast_netns(&init_net, skb, group, gfp);
-}
-
-static int
-iscsi_unicast_skb(struct sk_buff *skb, u32 portid)
+iscsi_unicast_skb(struct net *net, struct sk_buff *skb, u32 portid)
{
struct sock *nls;
struct iscsi_net *isn;
- struct net *net = &init_net;

isn = net_generic(net, iscsi_net_id);
nls = isn->nls;
@@ -2690,6 +2683,7 @@ int iscsi_recv_pdu(struct iscsi_cls_conn *conn, struct iscsi_hdr *hdr,
struct iscsi_uevent *ev;
char *pdu;
struct iscsi_internal *priv;
+ struct net *net;
int len = nlmsg_total_size(sizeof(*ev) + sizeof(struct iscsi_hdr) +
data_size);

@@ -2716,7 +2710,8 @@ int iscsi_recv_pdu(struct iscsi_cls_conn *conn, struct iscsi_hdr *hdr,
memcpy(pdu, hdr, sizeof(struct iscsi_hdr));
memcpy(pdu + sizeof(struct iscsi_hdr), data, data_size);

- return iscsi_multicast_skb(skb, ISCSI_NL_GRP_ISCSID, GFP_ATOMIC);
+ net = iscsi_conn_net(conn);
+ return iscsi_multicast_skb(net, skb, ISCSI_NL_GRP_ISCSID, GFP_ATOMIC);
}
EXPORT_SYMBOL_GPL(iscsi_recv_pdu);

@@ -2727,6 +2722,7 @@ int iscsi_offload_mesg(struct Scsi_Host *shost,
struct nlmsghdr *nlh;
struct sk_buff *skb;
struct iscsi_uevent *ev;
+ struct net *net;
int len = nlmsg_total_size(sizeof(*ev) + data_size);

skb = alloc_skb(len, GFP_ATOMIC);
@@ -2751,7 +2747,8 @@ int iscsi_offload_mesg(struct Scsi_Host *shost,

memcpy((char *)ev + sizeof(*ev), data, data_size);

- return iscsi_multicast_skb(skb, ISCSI_NL_GRP_UIP, GFP_ATOMIC);
+ net = iscsi_host_net(shost->shost_data);
+ return iscsi_multicast_skb(net, skb, ISCSI_NL_GRP_UIP, GFP_ATOMIC);
}
EXPORT_SYMBOL_GPL(iscsi_offload_mesg);

@@ -2761,6 +2758,7 @@ void iscsi_conn_error_event(struct iscsi_cls_conn *conn, enum iscsi_err error)
struct sk_buff *skb;
struct iscsi_uevent *ev;
struct iscsi_internal *priv;
+ struct net *net;
int len = nlmsg_total_size(sizeof(*ev));
unsigned long flags;
int state;
@@ -2808,7 +2806,8 @@ void iscsi_conn_error_event(struct iscsi_cls_conn *conn, enum iscsi_err error)
ev->r.connerror.cid = conn->cid;
ev->r.connerror.sid = iscsi_conn_get_sid(conn);

- iscsi_multicast_skb(skb, ISCSI_NL_GRP_ISCSID, GFP_ATOMIC);
+ net = iscsi_conn_net(conn);
+ iscsi_multicast_skb(net, skb, ISCSI_NL_GRP_ISCSID, GFP_ATOMIC);

iscsi_cls_conn_printk(KERN_INFO, conn, "detected conn error (%d)\n",
error);
@@ -2822,6 +2821,7 @@ void iscsi_conn_login_event(struct iscsi_cls_conn *conn,
struct sk_buff *skb;
struct iscsi_uevent *ev;
struct iscsi_internal *priv;
+ struct net *net;
int len = nlmsg_total_size(sizeof(*ev));

priv = iscsi_if_transport_lookup(conn->transport);
@@ -2842,7 +2842,9 @@ void iscsi_conn_login_event(struct iscsi_cls_conn *conn,
ev->r.conn_login.state = state;
ev->r.conn_login.cid = conn->cid;
ev->r.conn_login.sid = iscsi_conn_get_sid(conn);
- iscsi_multicast_skb(skb, ISCSI_NL_GRP_ISCSID, GFP_ATOMIC);
+
+ net = iscsi_conn_net(conn);
+ iscsi_multicast_skb(net, skb, ISCSI_NL_GRP_ISCSID, GFP_ATOMIC);

iscsi_cls_conn_printk(KERN_INFO, conn, "detected conn login (%d)\n",
state);
@@ -2853,11 +2855,17 @@ void iscsi_post_host_event(uint32_t host_no, struct iscsi_transport *transport,
enum iscsi_host_event_code code, uint32_t data_size,
uint8_t *data)
{
+ struct Scsi_Host *shost;
+ struct net *net;
struct nlmsghdr *nlh;
struct sk_buff *skb;
struct iscsi_uevent *ev;
int len = nlmsg_total_size(sizeof(*ev) + data_size);

+ shost = scsi_host_lookup(host_no);
+ if (!shost)
+ return;
+
skb = alloc_skb(len, GFP_NOIO);
if (!skb) {
printk(KERN_ERR "gracefully ignored host event (%d):%d OOM\n",
@@ -2876,7 +2884,9 @@ void iscsi_post_host_event(uint32_t host_no, struct iscsi_transport *transport,
if (data_size)
memcpy((char *)ev + sizeof(*ev), data, data_size);

- iscsi_multicast_skb(skb, ISCSI_NL_GRP_ISCSID, GFP_NOIO);
+ net = iscsi_host_net(shost->shost_data);
+ scsi_host_put(shost);
+ iscsi_multicast_skb(net, skb, ISCSI_NL_GRP_ISCSID, GFP_NOIO);
}
EXPORT_SYMBOL_GPL(iscsi_post_host_event);

@@ -2884,11 +2894,17 @@ void iscsi_ping_comp_event(uint32_t host_no, struct iscsi_transport *transport,
uint32_t status, uint32_t pid, uint32_t data_size,
uint8_t *data)
{
+ struct Scsi_Host *shost;
+ struct net *net;
struct nlmsghdr *nlh;
struct sk_buff *skb;
struct iscsi_uevent *ev;
int len = nlmsg_total_size(sizeof(*ev) + data_size);

+ shost = scsi_host_lookup(host_no);
+ if (!shost)
+ return;
+
skb = alloc_skb(len, GFP_NOIO);
if (!skb) {
printk(KERN_ERR "gracefully ignored ping comp: OOM\n");
@@ -2905,12 +2921,15 @@ void iscsi_ping_comp_event(uint32_t host_no, struct iscsi_transport *transport,
ev->r.ping_comp.data_size = data_size;
memcpy((char *)ev + sizeof(*ev), data, data_size);

- iscsi_multicast_skb(skb, ISCSI_NL_GRP_ISCSID, GFP_NOIO);
+ net = iscsi_host_net(shost->shost_data);
+ scsi_host_put(shost);
+ iscsi_multicast_skb(net, skb, ISCSI_NL_GRP_ISCSID, GFP_NOIO);
}
EXPORT_SYMBOL_GPL(iscsi_ping_comp_event);

static int
-iscsi_if_send_reply(u32 portid, int type, void *payload, int size)
+iscsi_if_send_reply(struct net *net, u32 portid, int type,
+ void *payload, int size)
{
struct sk_buff *skb;
struct nlmsghdr *nlh;
@@ -2924,11 +2943,11 @@ iscsi_if_send_reply(u32 portid, int type, void *payload, int size)

nlh = __nlmsg_put(skb, 0, 0, type, (len - sizeof(*nlh)), 0);
memcpy(nlmsg_data(nlh), payload, size);
- return iscsi_unicast_skb(skb, portid);
+ return iscsi_unicast_skb(net, skb, portid);
}

static int
-iscsi_if_get_stats(struct iscsi_transport *transport, struct nlmsghdr *nlh)
+iscsi_if_get_stats(struct net *net, struct iscsi_transport *transport, struct nlmsghdr *nlh)
{
struct iscsi_uevent *ev = nlmsg_data(nlh);
struct iscsi_stats *stats;
@@ -2985,7 +3004,7 @@ iscsi_if_get_stats(struct iscsi_transport *transport, struct nlmsghdr *nlh)
skb_trim(skbstat, NLMSG_ALIGN(actual_size));
nlhstat->nlmsg_len = actual_size;

- err = iscsi_multicast_skb(skbstat, ISCSI_NL_GRP_ISCSID,
+ err = iscsi_multicast_skb(net, skbstat, ISCSI_NL_GRP_ISCSID,
GFP_ATOMIC);
} while (err < 0 && err != -ECONNREFUSED);

@@ -3005,6 +3024,7 @@ int iscsi_session_event(struct iscsi_cls_session *session,
struct iscsi_uevent *ev;
struct sk_buff *skb;
struct nlmsghdr *nlh;
+ struct net *net;
int rc, len = nlmsg_total_size(sizeof(*ev));

priv = iscsi_if_transport_lookup(session->transport);
@@ -3049,7 +3069,8 @@ int iscsi_session_event(struct iscsi_cls_session *session,
* this will occur if the daemon is not up, so we just warn
* the user and when the daemon is restarted it will handle it
*/
- rc = iscsi_multicast_skb(skb, ISCSI_NL_GRP_ISCSID, GFP_KERNEL);
+ net = iscsi_sess_net(session);
+ rc = iscsi_multicast_skb(net, skb, ISCSI_NL_GRP_ISCSID, GFP_KERNEL);
if (rc == -ESRCH)
iscsi_cls_session_printk(KERN_ERR, session,
"Cannot notify userspace of session "
@@ -3412,7 +3433,8 @@ iscsi_send_ping(struct iscsi_transport *transport, struct iscsi_uevent *ev)
}

static int
-iscsi_get_chap(struct iscsi_transport *transport, struct nlmsghdr *nlh)
+iscsi_get_chap(struct net *net, struct iscsi_transport *transport,
+ struct nlmsghdr *nlh)
{
struct iscsi_uevent *ev = nlmsg_data(nlh);
struct Scsi_Host *shost = NULL;
@@ -3471,7 +3493,7 @@ iscsi_get_chap(struct iscsi_transport *transport, struct nlmsghdr *nlh)
skb_trim(skbchap, NLMSG_ALIGN(actual_size));
nlhchap->nlmsg_len = actual_size;

- err = iscsi_multicast_skb(skbchap, ISCSI_NL_GRP_ISCSID,
+ err = iscsi_multicast_skb(net, skbchap, ISCSI_NL_GRP_ISCSID,
GFP_KERNEL);
} while (err < 0 && err != -ECONNREFUSED);

@@ -3818,7 +3840,8 @@ static int iscsi_logout_flashnode_sid(struct iscsi_transport *transport,
}

static int
-iscsi_get_host_stats(struct iscsi_transport *transport, struct nlmsghdr *nlh)
+iscsi_get_host_stats(struct net *net, struct iscsi_transport *transport,
+ struct nlmsghdr *nlh)
{
struct iscsi_uevent *ev = nlmsg_data(nlh);
struct Scsi_Host *shost = NULL;
@@ -3878,8 +3901,8 @@ iscsi_get_host_stats(struct iscsi_transport *transport, struct nlmsghdr *nlh)
skb_trim(skbhost_stats, NLMSG_ALIGN(actual_size));
nlhhost_stats->nlmsg_len = actual_size;

- err = iscsi_multicast_skb(skbhost_stats, ISCSI_NL_GRP_ISCSID,
- GFP_KERNEL);
+ err = iscsi_multicast_skb(net, skbhost_stats,
+ ISCSI_NL_GRP_ISCSID, GFP_KERNEL);
} while (err < 0 && err != -ECONNREFUSED);

exit_host_stats:
@@ -4001,7 +4024,8 @@ static int iscsi_if_transport_conn(struct iscsi_transport *transport,
}

static int
-iscsi_if_recv_msg(struct sk_buff *skb, struct nlmsghdr *nlh, uint32_t *group)
+iscsi_if_recv_msg(struct net *net, struct sk_buff *skb,
+ struct nlmsghdr *nlh, uint32_t *group)
{
int err = 0;
u32 portid;
@@ -4096,7 +4120,7 @@ iscsi_if_recv_msg(struct sk_buff *skb, struct nlmsghdr *nlh, uint32_t *group)
err = iscsi_if_transport_conn(transport, nlh);
break;
case ISCSI_UEVENT_GET_STATS:
- err = iscsi_if_get_stats(transport, nlh);
+ err = iscsi_if_get_stats(net, transport, nlh);
break;
case ISCSI_UEVENT_TRANSPORT_EP_CONNECT:
case ISCSI_UEVENT_TRANSPORT_EP_POLL:
@@ -4121,7 +4145,7 @@ iscsi_if_recv_msg(struct sk_buff *skb, struct nlmsghdr *nlh, uint32_t *group)
err = iscsi_send_ping(transport, ev);
break;
case ISCSI_UEVENT_GET_CHAP:
- err = iscsi_get_chap(transport, nlh);
+ err = iscsi_get_chap(net, transport, nlh);
break;
case ISCSI_UEVENT_DELETE_CHAP:
err = iscsi_delete_chap(transport, ev);
@@ -4152,7 +4176,7 @@ iscsi_if_recv_msg(struct sk_buff *skb, struct nlmsghdr *nlh, uint32_t *group)
nlmsg_attrlen(nlh, sizeof(*ev)));
break;
case ISCSI_UEVENT_GET_HOST_STATS:
- err = iscsi_get_host_stats(transport, nlh);
+ err = iscsi_get_host_stats(net, transport, nlh);
break;
default:
err = -ENOSYS;
@@ -4170,6 +4194,8 @@ iscsi_if_recv_msg(struct sk_buff *skb, struct nlmsghdr *nlh, uint32_t *group)
static void
iscsi_if_rx(struct sk_buff *skb)
{
+ struct sock *sk = skb->sk;
+ struct net *net = sock_net(sk);
u32 portid = NETLINK_CB(skb).portid;

mutex_lock(&rx_queue_mutex);
@@ -4192,7 +4218,7 @@ iscsi_if_rx(struct sk_buff *skb)
if (rlen > skb->len)
rlen = skb->len;

- err = iscsi_if_recv_msg(skb, nlh, &group);
+ err = iscsi_if_recv_msg(net, skb, nlh, &group);
if (err) {
ev->type = ISCSI_KEVENT_IF_ERROR;
ev->iferror = err;
@@ -4208,7 +4234,9 @@ iscsi_if_rx(struct sk_buff *skb)
break;
if (ev->type == ISCSI_UEVENT_GET_CHAP && !err)
break;
- err = iscsi_if_send_reply(portid, nlh->nlmsg_type,
+ if (ev->type == ISCSI_UEVENT_GET_HOST_STATS && !err)
+ break;
+ err = iscsi_if_send_reply(net, portid, nlh->nlmsg_type,
ev, sizeof(*ev));
if (err == -EAGAIN && --retries < 0) {
printk(KERN_WARNING "Send reply failed, error %d\n", err);
--
2.39.1

[Hannes Reinecke]

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.

Might be done in an additional patch, though.
If you do you can add:

Reviewed-by: Hannes Reinecke <ha...@suse.de>

to this patch.

Cheers,

Hannes

[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

[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 <cle...@redhat.com>
---
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(&sesslock, flags);
- list_for_each_entry(sess, &sesslist, sess_list) {
+ isn = net_generic(net, iscsi_net_id);
+
+ spin_lock_irqsave(&isn->sesslock, flags);
+ list_for_each_entry(sess, &isn->sesslist, sess_list) {
if (sess->sid == sid) {
- ns = iscsi_sess_net(sess);
- if (ns != net)
- continue;
- spin_unlock_irqrestore(&sesslock, flags);
+ spin_unlock_irqrestore(&isn->sesslock, flags);
return sess;
}
}
- spin_unlock_irqrestore(&sesslock, flags);
+ spin_unlock_irqrestore(&isn->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(&connlock, flags);
- list_for_each_entry(conn, &connlist, conn_list) {
+ isn = net_generic(net, iscsi_net_id);
+
+ spin_lock_irqsave(&isn->connlock, flags);
+ list_for_each_entry(conn, &isn->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(&connlock, flags);
+ spin_unlock_irqrestore(&isn->connlock, flags);
return conn;
}
}
- spin_unlock_irqrestore(&connlock, flags);
+ spin_unlock_irqrestore(&isn->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(&sesslock, flags);
- list_add(&session->sess_list, &sesslist);
- spin_unlock_irqrestore(&sesslock, flags);
+ spin_lock_irqsave(&isn->sesslock, flags);
+ list_add(&session->sess_list, &isn->sesslist);
+ spin_unlock_irqrestore(&isn->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(&sesslock, flags);
+ spin_lock_irqsave(&isn->sesslock, flags);
if (!list_empty(&session->sess_list))
list_del(&session->sess_list);
- spin_unlock_irqrestore(&sesslock, flags);
+ spin_unlock_irqrestore(&isn->sesslock, flags);

if (!cancel_work_sync(&session->block_work))
cancel_delayed_work_sync(&session->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 iscsi_cls_session *session)
{
struct iscsi_transport *transport = session->transport;
+ struct net *net = iscsi_sess_net(session);
+ struct iscsi_net *isn = net_generic(net, iscsi_net_id);
unsigned long flags;

WARN_ON_ONCE(system_state == SYSTEM_RUNNING);

- spin_lock_irqsave(&sesslock, flags);
+ spin_lock_irqsave(&isn->sesslock, flags);
if (list_empty(&session->sess_list)) {
- spin_unlock_irqrestore(&sesslock, flags);
+ spin_unlock_irqrestore(&isn->sesslock, flags);
/*
* Conn/ep is already freed. Session is being torn down via
* async path. For shutdown we don't care about it so return.
*/
return;
}
- spin_unlock_irqrestore(&sesslock, flags);
+ spin_unlock_irqrestore(&isn->sesslock, flags);

device_for_each_child(&session->dev, NULL,
iscsi_iter_force_destroy_conn_fn);
@@ -2625,6 +2629,8 @@ int iscsi_add_conn(struct iscsi_cls_conn *conn)
int err;
unsigned long flags;
struct iscsi_cls_session *session = iscsi_dev_to_session(conn->dev.parent);
+ struct net *net = iscsi_sess_net(session);
+ struct iscsi_net *isn = net_generic(net, iscsi_net_id);

err = device_add(&conn->dev);
if (err) {
@@ -2640,9 +2646,9 @@ int iscsi_add_conn(struct iscsi_cls_conn *conn)
return err;
}

- spin_lock_irqsave(&connlock, flags);
- list_add(&conn->conn_list, &connlist);
- spin_unlock_irqrestore(&connlock, flags);
+ spin_lock_irqsave(&isn->connlock, flags);
+ list_add(&conn->conn_list, &isn->connlist);
+ spin_unlock_irqrestore(&isn->connlock, flags);

return 0;
}
@@ -2657,11 +2663,14 @@ EXPORT_SYMBOL_GPL(iscsi_add_conn);
*/
void iscsi_remove_conn(struct iscsi_cls_conn *conn)
{
+ struct net *net = iscsi_conn_net(conn);
+ struct iscsi_net *isn = net_generic(net, iscsi_net_id);
+
unsigned long flags;

- spin_lock_irqsave(&connlock, flags);
+ spin_lock_irqsave(&isn->connlock, flags);
list_del(&conn->conn_list);
- spin_unlock_irqrestore(&connlock, flags);
+ spin_unlock_irqrestore(&isn->connlock, flags);

transport_unregister_device(&conn->dev);
device_del(&conn->dev);
@@ -3432,20 +3441,21 @@ iscsi_set_path(struct net *net, struct iscsi_transport *transport,
return err;
}

-static int iscsi_session_has_conns(int sid)
+static int iscsi_session_has_conns(struct net *net, int sid)
{
+ struct iscsi_net *isn = net_generic(net, iscsi_net_id);
struct iscsi_cls_conn *conn;
unsigned long flags;
int found = 0;

- spin_lock_irqsave(&connlock, flags);
- list_for_each_entry(conn, &connlist, conn_list) {
+ spin_lock_irqsave(&isn->connlock, flags);
+ list_for_each_entry(conn, &isn->connlist, conn_list) {
if (iscsi_conn_get_sid(conn) == sid) {
found = 1;
break;
}
}
- spin_unlock_irqrestore(&connlock, flags);
+ spin_unlock_irqrestore(&isn->connlock, flags);

return found;
}
@@ -4157,7 +4167,7 @@ iscsi_if_recv_msg(struct net *net, struct sk_buff *skb,
session = iscsi_session_lookup(net, ev->u.d_session.sid);
if (!session)
err = -EINVAL;
- else if (iscsi_session_has_conns(ev->u.d_session.sid))
+ else if (iscsi_session_has_conns(net, ev->u.d_session.sid))
err = -EBUSY;
else
transport->destroy_session(session);
@@ -4166,15 +4176,16 @@ iscsi_if_recv_msg(struct net *net, struct sk_buff *skb,
session = iscsi_session_lookup(net, ev->u.d_session.sid);
if (!session)
err = -EINVAL;
- else if (iscsi_session_has_conns(ev->u.d_session.sid))
+ else if (iscsi_session_has_conns(net, ev->u.d_session.sid))
err = -EBUSY;
else {
+ struct iscsi_net *isn = net_generic(net, iscsi_net_id);
unsigned long flags;

/* Prevent this session from being found again */
- spin_lock_irqsave(&sesslock, flags);
+ spin_lock_irqsave(&isn->sesslock, flags);
list_del_init(&session->sess_list);
- spin_unlock_irqrestore(&sesslock, flags);
+ spin_unlock_irqrestore(&isn->sesslock, flags);

queue_work(system_unbound_wq, &session->destroy_work);
}
@@ -5176,6 +5187,13 @@ static int __net_init iscsi_net_init(struct net *net)
if (!nls)
return -ENOMEM;
isn = net_generic(net, iscsi_net_id);
+
+ INIT_LIST_HEAD(&isn->sesslist);
+ spin_lock_init(&isn->sesslock);
+ INIT_LIST_HEAD(&isn->connlist);
+ INIT_LIST_HEAD(&isn->connlist_err);
+ spin_lock_init(&isn->connlock);
+
isn->nls = nls;
return 0;
}
--
2.39.2

[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 <cle...@redhat.com>
---

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(&isn->sesslock, flags);
+ list_for_each_entry_safe(session, tmp, &isn->sesslist, sess_list) {
+ list_move_tail(&session->sess_list, &sessions);
+ }
+ spin_unlock_irqrestore(&isn->sesslock, flags);
+ list_for_each_entry_safe(session, tmp, &sessions, sess_list) {
+ device_for_each_child(&session->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

[Hannes Reinecke]

On 4/10/23 21:10, Chris Leech wrote:
> 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 <cle...@redhat.com>
> ---
> drivers/scsi/scsi_transport_iscsi.c | 104 ++++++++++++++++------------
> 1 file changed, 61 insertions(+), 43 deletions(-)
>

Reviewed-by: Hannes Reinecke <ha...@suse.de>

Cheers,

Hannes
--
Dr. Hannes Reinecke Kernel Storage Architect
ha...@suse.de +49 911 74053 688
SUSE Software Solutions GmbH, Maxfeldstr. 5, 90409 Nürnberg
HRB 36809 (AG Nürnberg), Geschäftsführer: Ivo Totev, Andrew
Myers, Andrew McDonald, Martje Boudien Moerman

[Hannes Reinecke]

On 4/10/23 21:10, Chris Leech wrote:

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

Ah, you shouldn't have mentioned that.
(Not quite sure how being namespace-aware relates to migration, though.)
We should be checking/modifying the iSCSI offload drivers, too.
But maybe with a later patch.

> Signed-off-by: Chris Leech <cle...@redhat.com>
> ---
> drivers/scsi/scsi_transport_iscsi.c | 18 ++++++++++++++++++
> 1 file changed, 18 insertions(+)
>

[Hannes Reinecke]

On 4/10/23 21:10, Chris Leech wrote:

Thanks a lot!
That's precisely what I had been looking for.

But you really shouldn't have mentioned iSCSI offloads; that was too
large an opening to _not_ comment on :-)

[Chris Leech]

On Tue, Apr 11, 2023 at 08:21:22AM +0200, Hannes Reinecke wrote:
> On 4/10/23 21:10, Chris Leech wrote:
> > 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.
> >
> Ah, you shouldn't have mentioned that.
> (Not quite sure how being namespace-aware relates to migration, though.)
> We should be checking/modifying the iSCSI offload drivers, too.
> But maybe with a later patch.

I shouldn't have left that opening ;-)

The idea with this design is to keep everything rooted on the
iscsi_host, and for physical HBAs those stay assigned to init_net.
With this patch set, offload drivers remain unusable in a net namespace
other than init_net. They simply are not visible.

By migration, I was implying the possibilty of assigment of an HBA
iscsi_host into a namespace like you can do with a network interface.
Such an iscsi_host would then need to be migrated back to init_net on
namespace exit.

I don't think it works to try and share an iscsi_host across namespaces,
and manage different sessions. The iSCSI HBAs have a limited number of
network configurations, exposed as iscsi_iface objects, and I don't want
to go down the road of figuring out how to share those.

- Chris

[Hannes Reinecke]

Ah, yes, indeed.

Quite some iSCSI offloads create the network session internally (or
don't even have one), so making them namespace aware will be tricky.

But then I guess we should avoid creating offload sessions from other
namespaces; preferably by a patch for the kernel such that userspace can
run unmodified.