Source-Changes-HG archive

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]

[src/trunk]: src/sys/dev/iscsi kill_session now uses the session id to avoid ...



details:   https://anonhg.NetBSD.org/src/rev/2e7e4a1bf7fa
branches:  trunk
changeset: 370058:2e7e4a1bf7fa
user:      mlelstv <mlelstv%NetBSD.org@localhost>
date:      Tue Sep 13 13:09:16 2022 +0000

description:
kill_session now uses the session id to avoid stale session pointers.
protect network socket with rwlock to handle recconnects.
always take over socket from iscsid to prevent leaks.
keep a good connection alive.
don't forget child device when config_detach fails.
fix locking when reassigning CCBs.
pducount is protected by lock, no need for atomic.
some code rework, refined debug messages.

diffstat:

 sys/dev/iscsi/iscsi_globals.h |   14 ++-
 sys/dev/iscsi/iscsi_ioctl.c   |  133 ++++++++++++++++++++++++++++-------------
 sys/dev/iscsi/iscsi_main.c    |    5 +-
 sys/dev/iscsi/iscsi_rcv.c     |   73 ++++++++++++++--------
 sys/dev/iscsi/iscsi_send.c    |   71 ++++++++++++++++------
 sys/dev/iscsi/iscsi_utils.c   |   59 +++++++++++-------
 6 files changed, 237 insertions(+), 118 deletions(-)

diffs (truncated from 815 to 300 lines):

diff -r 5e8622044ad5 -r 2e7e4a1bf7fa sys/dev/iscsi/iscsi_globals.h
--- a/sys/dev/iscsi/iscsi_globals.h     Tue Sep 13 11:47:54 2022 +0000
+++ b/sys/dev/iscsi/iscsi_globals.h     Tue Sep 13 13:09:16 2022 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: iscsi_globals.h,v 1.26 2020/06/21 23:08:16 chs Exp $   */
+/*     $NetBSD: iscsi_globals.h,v 1.27 2022/09/13 13:09:16 mlelstv Exp $       */
 
 /*-
  * Copyright (c) 2004,2005,2006,2011 The NetBSD Foundation, Inc.
@@ -37,6 +37,8 @@
 /* Includes we need in all files */
 
 #include <sys/param.h>
+#include <sys/mutex.h>
+#include <sys/rwlock.h>
 #include <sys/proc.h>
 #include <sys/conf.h>
 #include <sys/errno.h>
@@ -354,6 +356,8 @@
 
        struct lwp                      *c_threadobj;
                /* proc/thread pointer of socket owner */
+
+       krwlock_t                       c_sock_rw;
        struct file                     *c_sock;        /* the connection's socket */
        session_t                       *c_session;
                                        /* back pointer to the owning session */
@@ -366,7 +370,7 @@
        int                             c_recover; /* recovery count */
                /* (reset on first successful data transfer) */
        volatile unsigned               c_usecount; /* number of active CCBs */
-       volatile unsigned               c_pducount; /* number of active PDUs */
+       unsigned                        c_pducount; /* number of active PDUs */
 
        bool                            c_destroy; /* conn will be destroyed */
        bool                            c_in_session;
@@ -537,8 +541,8 @@
 #define DEBOUT(x) printf x
 #define DEB(lev,x) { if (iscsi_debug_level >= lev) printf x ;}
 #define DEBC(conn,lev,x) { if (iscsi_debug_level >= lev) { printf("S%dC%d: ", \
-                               conn ? conn->c_session->s_id : -1, \
-                               conn ? conn->c_id : -1); printf x ;}}
+                       conn && conn->c_session ? conn->c_session->s_id : -1, \
+                       conn ? conn->c_id : -1); printf x ;}}
 
 #define STATIC static
 
@@ -646,7 +650,7 @@
 void add_event(iscsi_event_t, uint32_t, uint32_t, uint32_t);
 
 void kill_connection(connection_t *, uint32_t, int, bool);
-void kill_session(session_t *, uint32_t, int, bool);
+void kill_session(uint32_t, uint32_t, int, bool);
 int kill_all_sessions(void);
 void handle_connection_error(connection_t *, uint32_t, int);
 void add_connection_cleanup(connection_t *);
diff -r 5e8622044ad5 -r 2e7e4a1bf7fa sys/dev/iscsi/iscsi_ioctl.c
--- a/sys/dev/iscsi/iscsi_ioctl.c       Tue Sep 13 11:47:54 2022 +0000
+++ b/sys/dev/iscsi/iscsi_ioctl.c       Tue Sep 13 13:09:16 2022 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: iscsi_ioctl.c,v 1.32 2020/06/21 23:08:16 chs Exp $     */
+/*     $NetBSD: iscsi_ioctl.c,v 1.33 2022/09/13 13:09:16 mlelstv Exp $ */
 
 /*-
  * Copyright (c) 2004,2005,2006,2011 The NetBSD Foundation, Inc.
@@ -494,8 +494,8 @@
        if (recover &&
            !conn->c_destroy &&
            conn->c_recover > MAX_RECOVERY_ATTEMPTS) {
-               DEBC(conn, 1,
-                         ("Kill_connection: Too many recovery attempts, destroying\n"));
+               DEBC(conn, 1, ("Kill_connection: Too many recovery attempts, "
+                              "destroying\n"));
                conn->c_destroy = TRUE;
        }
 
@@ -547,6 +547,8 @@
                        }
                        mutex_exit(&iscsi_cleanup_mtx);
 
+                       DEBC(conn, 1, ("Send_logout for reason %d\n", logout));
+
                        connection_timeout_start(conn, CONNECTION_TIMEOUT);
 
                        if (!send_logout(conn, conn, logout, FALSE)) {
@@ -591,14 +593,24 @@
  */
 
 void
-kill_session(session_t *sess, uint32_t status, int logout, bool recover)
+kill_session(uint32_t sid, uint32_t status, int logout, bool recover)
 {
+       session_t *sess;
        connection_t *conn;
 
        DEB(1, ("ISCSI: kill_session %d, status %d, logout %d, recover %d\n",
-                       sess->s_id, status, logout, recover));
+                       sid, status, logout, recover));
 
        mutex_enter(&iscsi_cleanup_mtx);
+
+       sess = find_session(sid);
+       if (sess == NULL) {
+               mutex_exit(&iscsi_cleanup_mtx);
+
+               DEB(5, ("Session %u already gone\n", sid));
+               return;
+       }
+
        if (sess->s_terminating) {
                mutex_exit(&iscsi_cleanup_mtx);
 
@@ -648,6 +660,13 @@
        sess->s_sessions.tqe_next = NULL;
        sess->s_sessions.tqe_prev = NULL;
 
+       /*
+        * If all connections are already gone, trigger cleanup
+        * otherwise, the last connection will do this
+        */
+       if (sess->s_active_connections == 0)
+               iscsi_notify_cleanup();
+
        mutex_exit(&iscsi_cleanup_mtx);
 
        /* kill all connections */
@@ -661,6 +680,7 @@
 /*
  * create_connection:
  *    Create and init the necessary framework for a connection:
+ *       Take over userland socket
  *       Alloc the connection structure itself
  *       Copy connection parameters
  *       Create the send and receive threads
@@ -689,6 +709,9 @@
            sess->s_active_connections >= sess->s_MaxConnections) {
                DEBOUT(("Too many connections (max = %d, curr = %d)\n",
                                sess->s_MaxConnections, sess->s_active_connections));
+               /* Always close descriptor */
+               fd_close(par->socket);
+
                par->status = ISCSI_STATUS_MAXED_CONNECTIONS;
                return EIO;
        }
@@ -696,10 +719,35 @@
        conn = malloc(sizeof(*conn), M_DEVBUF, M_WAITOK | M_ZERO);
        if (conn == NULL) {
                DEBOUT(("No mem for connection\n"));
+
+               /* Always close descriptor */
+               fd_close(par->socket);
+
                par->status = ISCSI_STATUS_NO_RESOURCES;
                return EIO;
        }
 
+       rc = get_socket(par->socket, &conn->c_sock);
+       fd_close(par->socket);
+
+       if (rc) {
+               DEBOUT(("Invalid socket %d\n", par->socket));
+
+               callout_destroy(&conn->c_timeout);
+               rw_destroy(&conn->c_sock_rw);
+               cv_destroy(&conn->c_idle_cv);
+               cv_destroy(&conn->c_ccb_cv);
+               cv_destroy(&conn->c_pdu_cv);
+               cv_destroy(&conn->c_conn_cv);
+               mutex_destroy(&conn->c_lock);
+               free(conn, M_DEVBUF);
+               par->status = ISCSI_STATUS_INVALID_SOCKET;
+               return rc;
+       }
+
+       DEBC(conn, 1, ("get_socket: par_sock=%d, fdesc=%p\n",
+                       par->socket, conn->c_sock));
+
        mutex_enter(&iscsi_cleanup_mtx);
        /* create a unique ID */
        do {
@@ -721,6 +769,7 @@
        cv_init(&conn->c_pdu_cv, "pdupool");
        cv_init(&conn->c_ccb_cv, "ccbwait");
        cv_init(&conn->c_idle_cv, "idle");
+       rw_init(&conn->c_sock_rw);
 
        callout_init(&conn->c_timeout, CALLOUT_MPSAFE);
        callout_setfunc(&conn->c_timeout, connection_timeout_co, conn);
@@ -729,25 +778,6 @@
        init_sernum(&conn->c_StatSN_buf);
        create_pdus(conn);
 
-       if ((rc = get_socket(par->socket, &conn->c_sock)) != 0) {
-               DEBOUT(("Invalid socket %d\n", par->socket));
-
-               callout_destroy(&conn->c_timeout);
-               cv_destroy(&conn->c_idle_cv);
-               cv_destroy(&conn->c_ccb_cv);
-               cv_destroy(&conn->c_pdu_cv);
-               cv_destroy(&conn->c_conn_cv);
-               mutex_destroy(&conn->c_lock);
-               free(conn, M_DEVBUF);
-               par->status = ISCSI_STATUS_INVALID_SOCKET;
-               return rc;
-       }
-       DEBC(conn, 1, ("get_socket: par_sock=%d, fdesc=%p\n",
-                       par->socket, conn->c_sock));
-
-       /* close the file descriptor */
-       fd_close(par->socket);
-
        conn->c_threadobj = l;
        conn->c_login_par = par;
 
@@ -758,6 +788,7 @@
                DEBOUT(("Can't create rcv thread (rc %d)\n", rc));
 
                release_socket(conn->c_sock);
+               rw_destroy(&conn->c_sock_rw);
                callout_destroy(&conn->c_timeout);
                cv_destroy(&conn->c_idle_cv);
                cv_destroy(&conn->c_ccb_cv);
@@ -791,6 +822,7 @@
 
                release_socket(conn->c_sock);
                callout_destroy(&conn->c_timeout);
+               rw_destroy(&conn->c_sock_rw);
                cv_destroy(&conn->c_idle_cv);
                cv_destroy(&conn->c_ccb_cv);
                cv_destroy(&conn->c_pdu_cv);
@@ -868,27 +900,32 @@
            sess->s_active_connections >= sess->s_MaxConnections) {
                DEBOUT(("Too many connections (max = %d, curr = %d)\n",
                        sess->s_MaxConnections, sess->s_active_connections));
+
+               /* Always close the desecriptor */
+               fd_close(par->socket);
+
                par->status = ISCSI_STATUS_MAXED_CONNECTIONS;
                return EIO;
        }
 
-       /* close old socket */
+       rw_enter(&conn->c_sock_rw, RW_WRITER);
        if (conn->c_sock != NULL) {
                closef(conn->c_sock);
                conn->c_sock = NULL;
        }
+       rc = get_socket(par->socket, &conn->c_sock);
+       rw_exit(&conn->c_sock_rw);
+       fd_close(par->socket);
 
-       if ((rc = get_socket(par->socket, &conn->c_sock)) != 0) {
+       if (rc) {
                DEBOUT(("Invalid socket %d\n", par->socket));
                par->status = ISCSI_STATUS_INVALID_SOCKET;
                return rc;
        }
+
        DEBC(conn, 1, ("get_socket: par_sock=%d, fdesc=%p\n",
                        par->socket, conn->c_sock));
 
-       /* close the file descriptor */
-       fd_close(par->socket);
-
        conn->c_threadobj = l;
        conn->c_login_par = par;
        conn->c_terminating = ISCSI_STATUS_SUCCESS;
@@ -912,7 +949,7 @@
        mutex_exit(&conn->c_lock);
 
        if ((rc = send_login(conn)) != 0) {
-               DEBOUT(("Login failed (rc %d)\n", rc));
+               DEBC(conn, 0, ("Re-Login failed (rc %d)\n", rc));
                while ((ccb = TAILQ_FIRST(&old_waiting)) != NULL) {
                        TAILQ_REMOVE(&old_waiting, ccb, ccb_chain);
                        wake_ccb(ccb, rc);
@@ -1121,7 +1158,7 @@
                DEB(1, ("Login: map session %d\n", sess->s_id));
                if (!map_session(sess, dev)) {
                        DEB(1, ("Login: map session %d failed\n", sess->s_id));
-                       kill_session(sess, ISCSI_STATUS_MAP_FAILED,
+                       kill_session(par->session_id, ISCSI_STATUS_MAP_FAILED,
                                        LOGOUT_SESSION, FALSE);
                        par->status = ISCSI_STATUS_MAP_FAILED;
                        return;
@@ -1156,7 +1193,9 @@
        /* If the session exists, this always succeeds */
        par->status = ISCSI_STATUS_SUCCESS;
 
-       kill_session(session, ISCSI_STATUS_LOGOUT, LOGOUT_SESSION, FALSE);
+       kill_session(par->session_id,
+           ISCSI_STATUS_LOGOUT, LOGOUT_SESSION,
+           FALSE);
 }
 
 
@@ -1169,7 +1208,7 @@
  *          l        IN: The lwp pointer of the caller
  */



Home | Main Index | Thread Index | Old Index