fix(logout): client connection shouldn't be closed on logout conn (#248)

Signed-off-by: Vitta <[email protected]>

src/istgt_iscsi.c

@@ -279,11 +279,12 @@ istgt_iscsi_read_pdu(CONN_Ptr conn, ISCSI_PDU_Ptr pdu)
 	iovec[0].iov_len = 4 * pdu->total_ahs_len;
 
 	/* Header Digest */
-	iovec[1].iov_base = pdu->header_digest;
 	if (conn->header_digest) {
+		iovec[1].iov_base = pdu->header_digest;
 		iovec[1].iov_len = ISCSI_DIGEST_LEN;
 		total += ISCSI_DIGEST_LEN;
 	} else {
+		iovec[1].iov_base = NULL;
 		iovec[1].iov_len = 0;
 	}
 
@@ -313,11 +314,12 @@ istgt_iscsi_read_pdu(CONN_Ptr conn, ISCSI_PDU_Ptr pdu)
 	iovec[2].iov_len = ISCSI_ALIGN(pdu->data_segment_len);
 
 	/* Data Digest */
-	iovec[3].iov_base = pdu->data_digest;
 	if (conn->data_digest && data_len != 0) {
+		iovec[3].iov_base = pdu->data_digest;
 		iovec[3].iov_len = ISCSI_DIGEST_LEN;
 		total += ISCSI_DIGEST_LEN;
 	} else {
+		iovec[3].iov_base = NULL;
 		iovec[3].iov_len = 0;
 	}
 
@@ -349,7 +351,7 @@ istgt_iscsi_read_pdu(CONN_Ptr conn, ISCSI_PDU_Ptr pdu)
 		for (i = 0; i < 4; i++) {
 			if (iovec[i].iov_len != 0 && iovec[i].iov_len > (size_t)rc) {
 				iovec[i].iov_base
-					= (void *) (((uintptr_t)iovec[i].iov_base) + rc);
+					= (void *) (((char *)iovec[i].iov_base) + rc);
 				iovec[i].iov_len -= rc;
 				break;
 			} else {
@@ -680,8 +682,8 @@ istgt_iscsi_write_pdu_internal(CONN_Ptr conn, ISCSI_PDU_Ptr pdu, ISTGT_LU_CMD_Pt
 		rc = writev(conn->sock, &iovec[0], 5);
 		if (rc < 0) {
 			now = time(NULL);
-			ISTGT_ERRLOG("writev() failed (errno=%d,%s,time=%f) for opcode:0x%2.2x CSN:0x%x\n",
-				errno, conn->initiator_name, difftime(now, start), lu_cmd->cdb0, lu_cmd->CmdSN);
+			ISTGT_ERRLOG("writev() failed (errno=%d,%s,time=%f) for opcode:%d cdb:0x%2.2x CSN:0x%x\n",
+				errno, conn->initiator_name, difftime(now, start), opcode, lu_cmd->cdb0, lu_cmd->CmdSN);
 			return (-1);
 		}
 		nbytes -= rc;
@@ -5349,7 +5351,7 @@ wait_all_task(CONN_Ptr conn)
 		conn->id, conn->running_tasks);
 }
 
-
+#if 0
 static void
 snd_cleanup(void *arg)
 {
@@ -5434,6 +5436,7 @@ worker_cleanup(void *arg)
 	MTX_UNLOCK(&g_conns_mutex);
 	ISTGT_TRACELOG(ISTGT_TRACE_DEBUG, "cancel cleanup UNLOCK\n");
 }
+#endif
 
 const char lu_task_typ[4][12] = {
 	"RESPONSE",
@@ -5730,7 +5733,7 @@ sender(void *arg)
 	pthread_set_name_np(slf, tinfo);
 #endif
 
-	pthread_cleanup_push(snd_cleanup, (void *)conn);
+//	pthread_cleanup_push(snd_cleanup, (void *)conn);
 	memset(&abstime, 0, sizeof (abstime));
 	/* handle DATA-IN/SCSI status */
 	ISTGT_TRACELOG(ISTGT_TRACE_DEBUG, "sender loop start (%d)\n", conn->id);
@@ -5888,7 +5891,7 @@ sender(void *arg)
 //		MTX_UNLOCK(&conn->wpdu_mutex);
 	}
 	// MTX_UNLOCK(&conn->sender_mutex);
-	pthread_cleanup_pop(0);
+//	pthread_cleanup_pop(0);
 	ISTGT_NOTICELOG("sender loop ended (%d:%d:%d)\n", conn->id, conn->epfd, ntohs(conn->iport));
 	return (NULL);
 }
@@ -5935,22 +5938,21 @@ worker(void *arg)
 // #endif
 
 	events.data.fd = conn->sock;
-		events.events = EPOLLIN;
-		rc = epoll_ctl(epfd, EPOLL_CTL_ADD, conn->sock, &events);
-		if (rc == -1) {
+	events.events = EPOLLIN;
+	rc = epoll_ctl(epfd, EPOLL_CTL_ADD, conn->sock, &events);
+	if (rc == -1) {
 		ISTGT_ERRLOG("epoll_ctl() failed\n");
 		close(epfd);
 		return (NULL);
-		}
+	}
 	events.data.fd = conn->task_pipe[0];
-		events.events = EPOLLIN;
-		rc = epoll_ctl(epfd, EPOLL_CTL_ADD, conn->task_pipe[0], &events);
-		if (rc == -1) {
+	events.events = EPOLLIN;
+	rc = epoll_ctl(epfd, EPOLL_CTL_ADD, conn->task_pipe[0], &events);
+	if (rc == -1) {
 		ISTGT_ERRLOG("epoll_ctl() failed\n");
 		close(epfd);
 		return (NULL);
-		}
-
+	}
 
 	// TODO
 	// if (!conn->istgt->daemon) {
@@ -6025,7 +6027,7 @@ worker(void *arg)
 	conn->exec_lu_task = NULL;
 	lu_task = NULL;
 
-	pthread_cleanup_push(worker_cleanup, conn);
+//	pthread_cleanup_push(worker_cleanup, conn);
 	pthread_setcancelstate(PTHREAD_CANCEL_ENABLE, NULL);
 
 	/* create sender thread */
@@ -6100,25 +6102,27 @@ worker(void *arg)
 
 		/* on socket */
 		if (events.data.fd == conn->sock) {
+			/* considered the half-close case i.e., (EPOLLRDHUP | EPOLLIN) */
 			if ((events.events & EPOLLERR) ||
 					(events.events & EPOLLHUP) ||
 					(!(events.events & EPOLLIN))) {
-				ISTGT_ERRLOG("close conn %d\n", errno);
+				ISTGT_ERRLOG("close conn events %d\n", events.events);
 				break;
 			}
 
 			rc = istgt_iscsi_read_pdu(conn, &conn->pdu);
 			if (rc < 0) {
 				if (errno == EAGAIN) {
+					ISTGT_ERRLOG("close conn %d %d", errno, events.events);
 					break;
 				}
 				if (conn->state != CONN_STATE_EXITING) {
 					ISTGT_ERRLOG("conn->state = %d\n", conn->state);
 				}
 				if (conn->state != CONN_STATE_RUNNING) {
 					if (errno == EINPROGRESS) {
-						sleep(1);
-						continue;
+						ISTGT_ERRLOG("iscsi_read_pdu shouldn't get EINPROGRESS");
+						break;
 					}
 					if (errno == ECONNRESET
 						|| errno == ETIMEDOUT) {
@@ -6145,7 +6149,6 @@ worker(void *arg)
 				break;
 			} else if (rc == 1) { // means successful logout ISCSI_OP_LOGOUT
 				ISTGT_TRACELOG(ISTGT_TRACE_ISCSI, "logout received\n");
-				break;
 			}
 
 			if (conn->pdu.ahs != NULL) {
@@ -6279,7 +6282,7 @@ worker(void *arg)
 
 	cleanup_exit:
 ;
-	pthread_cleanup_pop(0);
+//	pthread_cleanup_pop(0);
 	conn->state = CONN_STATE_EXITING;
 	if (conn->sess != NULL) {
 		lu = conn->sess->lu;

test_istgt.sh

@@ -1018,7 +1018,7 @@ run_rebuild_time_test_in_single_replica()
 
 	while [ 1 ]; do
 		# With replica poll timeout as 10, volume should become
-		# healthy in less than 40 seconds.
+		# healthy in less than 20 seconds.
 		cmd="$ISTGTCONTROL -q REPLICA vol1 | jq '.\"volumeStatus\"[0].\"replicaStatus\"[0].\"upTime\"'"
 		rt=$(eval $cmd)
 		echo "replica start time $rt"