Index | Thread | Search

From:
Claudio Jeker <cjeker@diehard.n-r-g.com>
Subject:
iscsid: cleanup initiator functions a bit
To:
tech@openbsd.org
Cc:
Jack Burton <jack@saosce.com.au>
Date:
Wed, 22 Jan 2025 13:26:36 +0100

Download raw body.

Thread
The initator object is a singleton and there is no need to pass a pointer
around for it. The result is a somewhat cleaner interaction between
sessions and initator.

Tested against netbsd-iscsi-target.
-- 
:wq Claudio

Index: initiator.c
===================================================================
RCS file: /cvs/src/usr.sbin/iscsid/initiator.c,v
diff -u -p -r1.19 initiator.c
--- initiator.c	22 Jan 2025 10:30:55 -0000	1.19
+++ initiator.c	22 Jan 2025 10:38:47 -0000
@@ -33,7 +33,7 @@
 #include "iscsid.h"
 #include "log.h"
 
-struct initiator *initiator;
+static struct initiator *initiator;
 
 struct task_login {
 	struct task		 task;
@@ -61,7 +61,7 @@ void	initiator_logout_cb(struct connecti
 struct session_params		initiator_sess_defaults;
 struct connection_params	initiator_conn_defaults;
 
-struct initiator *
+void
 initiator_init(void)
 {
 	if (!(initiator = calloc(1, sizeof(*initiator))))
@@ -77,24 +77,34 @@ initiator_init(void)
 	initiator_conn_defaults = iscsi_conn_defaults;
 	initiator_sess_defaults.MaxConnections = ISCSID_DEF_CONNS;
 	initiator_conn_defaults.MaxRecvDataSegmentLength = 65536;
-
-	return initiator;
 }
 
 void
-initiator_cleanup(struct initiator *i)
+initiator_cleanup(void)
 {
 	struct session *s;
 
-	while ((s = TAILQ_FIRST(&i->sessions)) != NULL) {
-		TAILQ_REMOVE(&i->sessions, s, entry);
+	while ((s = TAILQ_FIRST(&initiator->sessions)) != NULL) {
+		TAILQ_REMOVE(&initiator->sessions, s, entry);
 		session_cleanup(s);
 	}
 	free(initiator);
 }
 
 void
-initiator_shutdown(struct initiator *i)
+initiator_set_config(struct initiator_config *ic)
+{
+	initiator->config = *ic;
+}
+
+struct initiator_config *
+initiator_get_config(void)
+{
+	return &initiator->config;
+}
+
+void
+initiator_shutdown(void)
 {
 	struct session *s;
 
@@ -105,7 +115,7 @@ initiator_shutdown(struct initiator *i)
 }
 
 int
-initiator_isdown(struct initiator *i)
+initiator_isdown(void)
 {
 	struct session *s;
 	int inprogres = 0;
@@ -118,6 +128,49 @@ initiator_isdown(struct initiator *i)
 }
 
 struct session *
+initiator_new_session(u_int8_t st)
+{
+	struct session *s;
+
+	if (!(s = calloc(1, sizeof(*s))))
+		return NULL;
+
+	/* use the same qualifier unless there is a conflict */
+	s->isid_base = initiator->config.isid_base;
+	s->isid_qual = initiator->config.isid_qual;
+	s->cmdseqnum = arc4random();
+	s->itt = arc4random();
+	s->state = SESS_INIT;
+
+	s->sev.sess = s;
+	evtimer_set(&s->sev.ev, session_fsm_callback, &s->sev);
+
+	if (st == SESSION_TYPE_DISCOVERY)
+		s->target = 0;
+	else
+		s->target = initiator->target++;
+
+	TAILQ_INIT(&s->connections);
+	TAILQ_INIT(&s->tasks);
+
+	TAILQ_INSERT_HEAD(&initiator->sessions, s, entry);
+
+	return s;
+}
+
+struct session *
+initiator_find_session(char *name)
+{
+	struct session *s;
+
+	TAILQ_FOREACH(s, &initiator->sessions, entry) {
+		if (strcmp(s->config.SessionName, name) == 0)
+			return s;
+	}
+	return NULL;
+}
+
+struct session *
 initiator_t2s(u_int target)
 {
 	struct session *s;
@@ -127,6 +180,12 @@ initiator_t2s(u_int target)
 			return s;
 	}
 	return NULL;
+}
+
+struct session_head *
+initiator_get_sessions(void)
+{
+	return &initiator->sessions;
 }
 
 void
Index: iscsid.c
===================================================================
RCS file: /cvs/src/usr.sbin/iscsid/iscsid.c,v
diff -u -p -r1.24 iscsid.c
--- iscsid.c	22 Jan 2025 10:14:54 -0000	1.24
+++ iscsid.c	22 Jan 2025 10:37:30 -0000
@@ -38,7 +38,6 @@ void		main_sig_handler(int, short, void 
 __dead void	usage(void);
 void		shutdown_cb(int, short, void *);
 
-extern struct initiator *initiator;
 struct event exit_ev;
 int exit_rounds;
 #define ISCSI_EXIT_WAIT 5
@@ -148,13 +147,13 @@ main(int argc, char *argv[])
 	signal(SIGPIPE, SIG_IGN);
 
 	control_event_init();
-	initiator = initiator_init();
+	initiator_init();
 
 	event_dispatch();
 
 	/* do some cleanup on the way out */
 	control_cleanup(ctrlsock);
-	initiator_cleanup(initiator);
+	initiator_cleanup();
 	log_info("exiting.");
 	return 0;
 }
@@ -164,7 +163,7 @@ shutdown_cb(int fd, short event, void *a
 {
 	struct timeval tv;
 
-	if (exit_rounds++ >= ISCSI_EXIT_WAIT || initiator_isdown(initiator))
+	if (exit_rounds++ >= ISCSI_EXIT_WAIT || initiator_isdown())
 		event_loopexit(NULL);
 
 	timerclear(&tv);
@@ -184,7 +183,7 @@ main_sig_handler(int sig, short event, v
 	case SIGTERM:
 	case SIGINT:
 	case SIGHUP:
-		initiator_shutdown(initiator);
+		initiator_shutdown();
 		evtimer_set(&exit_ev, shutdown_cb, NULL);
 		timerclear(&tv);
 		if (evtimer_add(&exit_ev, &tv) == -1)
@@ -211,6 +210,7 @@ iscsid_ctrl_dispatch(void *ch, struct pd
 {
 	struct ctrlmsghdr *cmh;
 	struct initiator_config *ic;
+	struct session_head *sh;
 	struct session_config *sc;
 	struct session *s;
 	struct session_poll p = { 0 };
@@ -228,7 +228,7 @@ iscsid_ctrl_dispatch(void *ch, struct pd
 			break;
 		}
 		ic = pdu_getbuf(pdu, NULL, 1);
-		memcpy(&initiator->config, ic, sizeof(initiator->config));
+		initiator_set_config(ic);
 		control_compose(ch, CTRL_SUCCESS, NULL, 0);
 		break;
 	case CTRL_SESSION_CONFIG:
@@ -250,9 +250,9 @@ iscsid_ctrl_dispatch(void *ch, struct pd
 		else
 			sc->InitiatorName = NULL;
 
-		s = session_find(initiator, sc->SessionName);
+		s = initiator_find_session(sc->SessionName);
 		if (s == NULL) {
-			s = session_new(initiator, sc->SessionType);
+			s = initiator_new_session(sc->SessionType);
 			if (s == NULL) {
 				control_compose(ch, CTRL_FAILURE, NULL, 0);
 				goto done;
@@ -280,10 +280,11 @@ iscsid_ctrl_dispatch(void *ch, struct pd
 		    sizeof(struct vscsi_stats));
 		break;
 	case CTRL_SHOW_SUM:
-		control_compose(ch, CTRL_INITIATOR_CONFIG, &initiator->config,
-		    sizeof(initiator->config));
+		ic = initiator_get_config();
+		control_compose(ch, CTRL_INITIATOR_CONFIG, ic, sizeof(*ic));
 
-		TAILQ_FOREACH(s, &initiator->sessions, entry) {
+		sh = initiator_get_sessions();
+		TAILQ_FOREACH(s, sh, entry) {
 			struct ctrldata cdv[3];
 			bzero(cdv, sizeof(cdv));
 
@@ -308,7 +309,8 @@ iscsid_ctrl_dispatch(void *ch, struct pd
 		control_compose(ch, CTRL_SUCCESS, NULL, 0);
 		break;
 	case CTRL_SESS_POLL:
-		TAILQ_FOREACH(s, &initiator->sessions, entry)
+		sh = initiator_get_sessions();
+		TAILQ_FOREACH(s, sh, entry)
 			poll_session(&p, s);
 		poll_finalize(&p);
 		control_compose(ch, CTRL_SESS_POLL, &p, sizeof(p));
Index: iscsid.h
===================================================================
RCS file: /cvs/src/usr.sbin/iscsid/iscsid.h,v
diff -u -p -r1.22 iscsid.h
--- iscsid.h	22 Jan 2025 10:14:54 -0000	1.22
+++ iscsid.h	22 Jan 2025 10:37:30 -0000
@@ -329,11 +329,16 @@ void	iscsi_merge_sess_params(struct sess
 void	iscsi_merge_conn_params(struct connection_params *,
 	    struct connection_params *, struct connection_params *);
 
-struct initiator *initiator_init(void);
-void	initiator_cleanup(struct initiator *);
-void	initiator_shutdown(struct initiator *);
-int	initiator_isdown(struct initiator *);
-struct session *initiator_t2s(u_int);
+void	initiator_init(void);
+void	initiator_cleanup(void);
+void	initiator_set_config(struct initiator_config *);
+struct initiator_config *initiator_get_config(void);
+void	initiator_shutdown(void);
+int	initiator_isdown(void);
+struct session	*initiator_new_session(u_int8_t);
+struct session	*initiator_find_session(char *);
+struct session	*initiator_t2s(u_int);
+struct session_head	*initiator_get_sessions(void);
 void	initiator_login(struct connection *);
 void	initiator_discovery(struct session *);
 void	initiator_logout(struct session *, struct connection *, u_int8_t);
@@ -347,8 +352,6 @@ void	control_queue(void *, struct pdu *)
 int	control_compose(void *, u_int16_t, void *, size_t);
 int	control_build(void *, u_int16_t, int, struct ctrldata *);
 
-struct session	*session_find(struct initiator *, char *);
-struct session	*session_new(struct initiator *, u_int8_t);
 void	session_cleanup(struct session *);
 int	session_shutdown(struct session *);
 void	session_config(struct session *, struct session_config *);
Index: session.c
===================================================================
RCS file: /cvs/src/usr.sbin/iscsid/session.c,v
diff -u -p -r1.12 session.c
--- session.c	22 Jan 2025 10:14:54 -0000	1.12
+++ session.c	22 Jan 2025 10:38:59 -0000
@@ -46,49 +46,6 @@ int	sess_do_reinstatement(struct session
 const char *sess_state(int);
 const char *sess_event(enum s_event);
 
-struct session *
-session_find(struct initiator *i, char *name)
-{
-	struct session *s;
-
-	TAILQ_FOREACH(s, &i->sessions, entry) {
-		if (strcmp(s->config.SessionName, name) == 0)
-			return s;
-	}
-	return NULL;
-}
-
-struct session *
-session_new(struct initiator *i, u_int8_t st)
-{
-	struct session *s;
-
-	if (!(s = calloc(1, sizeof(*s))))
-		return NULL;
-
-	/* use the same qualifier unless there is a conflict */
-	s->isid_base = i->config.isid_base;
-	s->isid_qual = i->config.isid_qual;
-	s->cmdseqnum = arc4random();
-	s->itt = arc4random();
-	s->initiator = i;
-	s->state = SESS_INIT;
-
-	s->sev.sess = s;
-	evtimer_set(&s->sev.ev, session_fsm_callback, &s->sev);
-
-	if (st == SESSION_TYPE_DISCOVERY)
-		s->target = 0;
-	else
-		s->target = s->initiator->target++;
-
-	TAILQ_INSERT_HEAD(&i->sessions, s, entry);
-	TAILQ_INIT(&s->connections);
-	TAILQ_INIT(&s->tasks);
-
-	return s;
-}
-
 void
 session_cleanup(struct session *s)
 {