- Delivery error response (instead of dropping connection) fixed

- Race when target deleted before the session(s) removal event(s) processed fixed
 - Docs updated



git-svn-id: http://svn.code.sf.net/p/scst/svn/trunk@1733 d57e44dd-8a1f-0410-8b47-8ef2f437770f

iscsi-scst/kernel/conn.c

@@ -269,6 +269,13 @@ void iscsi_make_conn_rd_active(struct iscsi_conn *conn)
 	TRACE_DBG("conn %p, rd_state %x, rd_data_ready %d", conn,
 		conn->rd_state, conn->rd_data_ready);
 
+	/*
+	 * Let's start processing ASAP not waiting for all the being waited
+	 * data be received, even if we need several wakup iteration to receive
+	 * them all, because starting ASAP, i.e. in parallel, is better for
+	 * performance, especially on multi-CPU/core systems.
+	 */
+
 	conn->rd_data_ready = 1;
 
 	if (conn->rd_state == ISCSI_CONN_RD_STATE_IDLE) {
@@ -292,6 +299,13 @@ void iscsi_make_conn_wr_active(struct iscsi_conn *conn)
 	TRACE_DBG("conn %p, wr_state %x, wr_space_ready %d", conn,
 		conn->wr_state, conn->wr_space_ready);
 
+	/*
+	 * Let's start sending waiting to be sent data ASAP, even if there's
+	 * still not all the needed buffers ready and we need several wakup
+	 * iteration to send them all, because starting ASAP, i.e. in parallel,
+	 * is better for performance, especially on multi-CPU/core systems.
+	 */
+
 	if (conn->wr_state == ISCSI_CONN_WR_STATE_IDLE) {
 		list_add_tail(&conn->wr_list_entry, &iscsi_wr_list);
 		conn->wr_state = ISCSI_CONN_WR_STATE_IN_LIST;

iscsi-scst/usr/event.c

@@ -63,7 +63,7 @@ static int nl_write(int fd, void *data, int len)
 	return sendmsg(fd, &msg, 0);
 }
 
-static int nl_read(int fd, void *data, int len)
+static int nl_read(int fd, void *data, int len, bool wait)
 {
 	struct iovec iov[2];
 	struct msghdr msg;
@@ -81,7 +81,7 @@ static int nl_read(int fd, void *data, int len)
 	msg.msg_iov = iov;
 	msg.msg_iovlen = 2;
 
-	res = recvmsg(fd, &msg, iscsi_enabled ? MSG_DONTWAIT : 0);
+	res = recvmsg(fd, &msg, wait ? 0 : MSG_DONTWAIT);
 	if (res > 0) {
 		res -= sizeof(nlh);
 		if (res < 0)
@@ -165,7 +165,7 @@ static int handle_e_add_target(int fd, const struct iscsi_kern_event *event)
 	offs = sprintf(buf, "Target ");
 
 	while (1) {
-		if ((rc = nl_read(fd, &buf[offs], event->param1_size)) < 0) {
+		if ((rc = nl_read(fd, &buf[offs], event->param1_size, true)) < 0) {
 			if ((errno == EINTR) || (errno == EAGAIN))
 				continue;
 			log_error("read netlink fd (%d) failed: %s", fd,
@@ -181,7 +181,7 @@ static int handle_e_add_target(int fd, const struct iscsi_kern_event *event)
 
 	if (event->param2_size > 0) {
 		while (1) {
-			if ((rc = nl_read(fd, &buf[offs], event->param2_size)) < 0) {
+			if ((rc = nl_read(fd, &buf[offs], event->param2_size, true)) < 0) {
 				if ((errno == EINTR) || (errno == EAGAIN))
 					continue;
 				log_error("read netlink fd (%d) failed: %s", fd,
@@ -402,7 +402,7 @@ static int handle_e_mgmt_cmd(int fd, const struct iscsi_kern_event *event)
 	}
 
 	while (1) {
-		if ((rc = nl_read(fd, buf, event->param1_size)) < 0) {
+		if ((rc = nl_read(fd, buf, event->param1_size, true)) < 0) {
 			if ((errno == EINTR) || (errno == EAGAIN))
 				continue;
 			log_error("read netlink fd (%d) failed: %s", fd,
@@ -491,7 +491,7 @@ static int handle_e_get_attr_value(int fd, const struct iscsi_kern_event *event)
 	}
 
 	while (1) {
-		if ((rc = nl_read(fd, buf, event->param1_size)) < 0) {
+		if ((rc = nl_read(fd, buf, event->param1_size, true)) < 0) {
 			if ((errno == EINTR) || (errno == EAGAIN))
 				continue;
 			log_error("read netlink fd (%d) failed: %s", fd,
@@ -642,7 +642,7 @@ static int handle_e_set_attr_value(int fd, const struct iscsi_kern_event *event)
 	}
 
 	while (1) {
-		if ((rc = nl_read(fd, buf, event->param1_size)) < 0) {
+		if ((rc = nl_read(fd, buf, event->param1_size, true)) < 0) {
 			if ((errno == EINTR) || (errno == EAGAIN))
 				continue;
 			log_error("read netlink fd (%d) failed: %s", fd,
@@ -657,7 +657,7 @@ static int handle_e_set_attr_value(int fd, const struct iscsi_kern_event *event)
 	offs += sprintf(&buf[offs], " ");
 
 	while (1) {
-		if ((rc = nl_read(fd, &buf[offs], event->param2_size)) < 0) {
+		if ((rc = nl_read(fd, &buf[offs], event->param2_size, true)) < 0) {
 			if ((errno == EINTR) || (errno == EAGAIN))
 				continue;
 			log_error("read netlink fd (%d) failed: %s", fd,
@@ -894,7 +894,7 @@ out_free_server:
 
 #endif /* CONFIG_SCST_PROC */
 
-void handle_iscsi_events(int fd)
+int handle_iscsi_events(int fd, bool wait)
 {
 	struct session *session;
 	struct connection *conn;
@@ -910,9 +910,9 @@ void handle_iscsi_events(int fd)
 	 */
 
 retry:
-	if ((rc = nl_read(fd, &event, sizeof(event))) < 0) {
+	if ((rc = nl_read(fd, &event, sizeof(event), wait)) < 0) {
 		if (errno == EAGAIN)
-			return;
+			return EAGAIN;
 		if (errno == EINTR)
 			goto retry;
 		log_error("read netlink fd (%d) failed: %s", fd, strerror(errno));
@@ -1014,7 +1014,7 @@ retry:
 		break;
 	}
 
-	return;
+	return 0;
 }
 
 int nl_open(void)

iscsi-scst/usr/iscsi_scstd.c

@@ -332,15 +332,17 @@ static void event_conn(struct connection *conn, struct pollfd *pollfd)
 {
 	int res, opt;
 
+again:
 	switch (conn->iostate) {
 	case IOSTATE_READ_BHS:
 	case IOSTATE_READ_AHS_DATA:
 	      read_again:
 		res = read(pollfd->fd, conn->buffer, conn->rwsize);
 		if (res <= 0) {
-			if (res == 0 || (errno != EINTR && errno != EAGAIN))
-				conn->state = STATE_EXIT;
-			else if (errno == EINTR)
+			if (res == 0 || (errno != EINTR && errno != EAGAIN)) {
+				conn->state = STATE_DROP;
+				goto out;
+			} else if (errno == EINTR)
 				goto read_again;
 			break;
 		}
@@ -361,7 +363,7 @@ static void event_conn(struct connection *conn, struct pollfd *pollfd)
 				log_warning("Recv PDU with invalid size %d "
 					"(max: %d)", conn->rwsize,
 					INCOMING_BUFSIZE);
-				conn->state = STATE_EXIT;
+				conn->state = STATE_DROP;
 				goto out;
 			}
 			if (conn->rwsize) {
@@ -369,7 +371,7 @@ static void event_conn(struct connection *conn, struct pollfd *pollfd)
 					conn->req_buffer = malloc(INCOMING_BUFSIZE);
 					if (!conn->req_buffer) {
 						log_error("Failed to alloc recv buffer");
-						conn->state = STATE_EXIT;
+						conn->state = STATE_DROP;
 						goto out;
 					}
 				}
@@ -386,6 +388,11 @@ static void event_conn(struct connection *conn, struct pollfd *pollfd)
 			log_pdu(2, &conn->req);
 			if (!cmnd_execute(conn))
 				conn->state = STATE_EXIT;
+
+			if (conn->state == STATE_EXIT) {
+				/* We need to send response */
+				goto again;
+			}
 			break;
 		}
 		break;
@@ -398,9 +405,10 @@ static void event_conn(struct connection *conn, struct pollfd *pollfd)
 		setsockopt(pollfd->fd, SOL_TCP, TCP_CORK, &opt, sizeof(opt));
 		res = write(pollfd->fd, conn->buffer, conn->rwsize);
 		if (res < 0) {
-			if (errno != EINTR && errno != EAGAIN)
-				conn->state = STATE_EXIT;
-			else if (errno == EINTR)
+			if (errno != EINTR && errno != EAGAIN) {
+				conn->state = STATE_DROP;
+				goto out;
+			} else if (errno == EINTR)
 				goto write_again;
 			break;
 		}
@@ -445,6 +453,7 @@ static void event_conn(struct connection *conn, struct pollfd *pollfd)
 				break;
 			case STATE_EXIT:
 			case STATE_CLOSE:
+			case STATE_DROP:
 				break;
 			default:
 				conn_read_pdu(conn);
@@ -491,7 +500,7 @@ static void event_loop(void)
 
 	while (1) {
 		if (!iscsi_enabled) {
-			handle_iscsi_events(nl_fd);
+			handle_iscsi_events(nl_fd, true);
 			continue;
 		}
 		res = poll(poll_array, POLL_MAX, isns_timeout);
@@ -528,7 +537,7 @@ static void event_loop(void)
 		}
 
 		if (poll_array[POLL_NL].revents)
-			handle_iscsi_events(nl_fd);
+			handle_iscsi_events(nl_fd, false);
 
 		if (poll_array[POLL_IPC].revents)
 			iscsi_adm_request_handle(ipc_fd);
@@ -554,7 +563,8 @@ static void event_loop(void)
 			event_conn(conn, pollfd);
 
 			if ((conn->state == STATE_CLOSE) ||
-			    (conn->state == STATE_EXIT)) {
+			    (conn->state == STATE_EXIT) ||
+			    (conn->state == STATE_DROP)) {
 				struct session *sess = conn->sess;
 				log_debug(1, "closing conn %p", conn);
 				conn_free_pdu(conn);
@@ -562,7 +572,7 @@ static void event_loop(void)
 				pollfd->fd = -1;
 				incoming[i] = NULL;
 				incoming_cnt--;
-				if (conn->state == STATE_EXIT) {
+				if (conn->state != STATE_CLOSE) {
 					if (conn->passed_to_kern) {
 						kernel_conn_destroy(conn->tid,
 							conn->sess->sid.id64,

iscsi-scst/usr/iscsid.c

@@ -541,11 +541,20 @@ static void login_start(struct connection *conn)
 			return;
 		}
 
+		/* We may "leak" here if we have an iSCSI event on the wrong time */
+		if (!iscsi_enabled) {
+			log_info("Connect from %s to disabled iSCSI-SCST refused",
+				name);
+			login_rsp_tgt_err(conn, 0);
+			conn->state = STATE_DROP;
+			return;
+		}
+
 		if (!target->tgt_enabled) {
 			log_info("Connect from %s to disabled target %s refused",
 				name, target_name);
 			login_rsp_tgt_err(conn, 0);
-			conn->state = STATE_EXIT;
+			conn->state = STATE_DROP;
 			return;
 		}
 
@@ -1037,6 +1046,7 @@ void cmnd_finish(struct connection *conn)
 
 	switch (conn->state) {
 	case STATE_EXIT:
+	case STATE_DROP:
 		break;
 	case STATE_SECURITY_LOGIN:
 		conn->state = STATE_LOGIN;

iscsi-scst/usr/iscsid.h

@@ -28,6 +28,10 @@
 #include "param.h"
 #include "misc.h"
 
+#ifndef bool
+typedef enum {false = 0, true} bool;
+#endif
+
 #define sBUG() assert(0)
 #define sBUG_ON(p) assert(!(p))
 
@@ -135,6 +139,7 @@ struct connection {
 #define STATE_KERNEL		9
 #define STATE_CLOSE		10
 #define STATE_EXIT		11
+#define STATE_DROP		12
 
 #define AUTH_STATE_START	0
 #define AUTH_STATE_CHALLENGE	1
@@ -212,6 +217,8 @@ enum {
 
 extern struct pollfd poll_array[POLL_MAX];
 
+extern int nl_fd;
+
 /* chap.c */
 extern int cmnd_exec_auth_chap(struct connection *conn);
 
@@ -310,7 +317,7 @@ extern int kernel_conn_create(u32 tid, u64 sid, u32 cid, u32 stat_sn, u32 exp_st
 extern int kernel_conn_destroy(u32 tid, u64 sid, u32 cid);
 
 /* event.c */
-extern void handle_iscsi_events(int fd);
+extern int handle_iscsi_events(int fd, bool wait);
 extern int nl_open(void);
 
 /* config.c */

iscsi-scst/usr/target.c

@@ -339,6 +339,9 @@ int target_del(u32 tid, u32 cookie)
 
 	list_del(&target->tlist);
 
+	/* We might need to handle session(s) removal event(s) from the kernel */
+	while (handle_iscsi_events(nl_fd, false) == 0);
+
 	if (!list_empty(&target->sessions_list)) {
 		log_error("%s: target %u still has sessions\n", __FUNCTION__,
 			  tid);

scst/README

@@ -1305,6 +1305,11 @@ with suffix ".1". Those backup files are used in case of power or other
 failure to prevent Persistent Reservation information from corruption
 during update.
 
+The "Persistence Through Power Loss" feature is not available in the
+procfs build, because the SCST proc interface doesn't allow to keep
+persistent Relative Target IDs of each target between reboots/reloads
+(they are load and initialization order dependent).
+
 The Persistent Reservations available on all transports implementing
 get_initiator_port_transport_id() callback. Transports not implementing
 this callback will act in one of 2 possible scenarios ("all or

scst/README_in-tree

@@ -888,6 +888,11 @@ with suffix ".1". Those backup files are used in case of power or other
 failure to prevent Persistent Reservation information from corruption
 during update.
 
+The "Persistence Through Power Loss" feature is not available in the
+procfs build, because the SCST proc interface doesn't allow to keep
+persistent Relative Target IDs of each target between reboots/reloads
+(they are load and initialization order dependent).
+
 The Persistent Reservations available on all transports implementing
 get_initiator_port_transport_id() callback. Transports not implementing
 this callback will act in one of 2 possible scenarios ("all or