6482030 stale comment in usr/src/cmd/iscsi/iscsitgtd/main.c

6482080 memory leak in iscsi_handle_login_pkt()
6521041 MODE_SENSE on errors attempts to send both STATUS_CHECK and STATUS_GOOD
6521425 When sending a NOP-In message the statsn value should not be incremented.
6522093 target returns wrong status when invalid LUN is used.
6523439 Just Say No to Nagle

6 files changed, 67 insertions, 62 deletions

diff --git a/usr/src/cmd/iscsi/iscsitgtd/iscsi_conn.c b/usr/src/cmd/iscsi/iscsitgtd/iscsi_conn.c
index 1d8df6e538..5e7430c49d 100644
--- a/usr/src/cmd/iscsi/iscsitgtd/iscsi_conn.c
+++ b/usr/src/cmd/iscsi/iscsitgtd/iscsi_conn.c
@@ -471,8 +471,27 @@ iscsi_conn_pkt(iscsi_conn_t *c, iscsi_rsp_hdr_t *in)
 		free(in);
 		return;
 	}
+
 	(void) pthread_mutex_lock(&c->c_mutex);
-	in->statsn	= htonl(c->c_statsn++);
+	/*
+	 * Make any final per command adjustments.
+	 */
+	switch (in->opcode & ISCSI_OPCODE_MASK) {
+	case ISCSI_OP_NOOP_IN:
+		in->statsn	= htonl(c->c_statsn);
+		/*
+		 * Only bump the STATSN value if this packet is in response to
+		 * an initiator's ping. Section 10.19.1 of RFC3720 specifies
+		 * that the value must be 0xffffffff when responding to an
+		 * initiator. So, if the value is the reserved ITT value then
+		 * this packet was generated in reponse to an initiator ping.
+		 * Otherwise, we timed out on the connection and are requesting
+		 * a ping.
+		 */
+		if (((iscsi_nop_in_hdr_t *)in)->ttt == ISCSI_RSVD_TASK_TAG)
+			c->c_statsn++;
+		break;
+	}
 	(void) pthread_mutex_lock(&c->c_sess->s_mutex);
 	in->expcmdsn	= htonl(c->c_sess->s_seencmdsn + 1);
 	in->maxcmdsn	= htonl(iscsi_cmd_window(c) + c->c_sess->s_seencmdsn);
@@ -611,6 +630,8 @@ iscsi_conn_data_in(t10_cmd_t *t)
 				 * initiator expected and we're sending.
 				 */
 
+				queue_prt(c->c_mgmtq, Q_CONN_ERRS,
+				    "CON%x  Underflow occurred\n", c->c_num);
 				send_datain_pdu(c, t, 0);
 				send_scsi_rsp(c, t);
 			}
@@ -762,13 +783,21 @@ send_scsi_rsp(iscsi_conn_t *c, t10_cmd_t *t)
 	(void) pthread_mutex_unlock(&c->c_sess->s_mutex);
 	rsp.cmd_status	= T10_CMD_STATUS(t);
 
+	if (cmd->c_writeop == True) {
+		if (cmd->c_offset_out != cmd->c_dlen_expected)
+			rsp.flags |= ISCSI_FLAG_CMD_OVERFLOW;
+		rsp.residual_count = htonl(cmd->c_dlen_expected -
+		    cmd->c_offset_out);
+	} else {
+		if (cmd->c_offset_in != cmd->c_dlen_expected)
+			rsp.flags |= ISCSI_FLAG_CMD_UNDERFLOW;
+		rsp.residual_count = htonl(cmd->c_dlen_expected -
+		    cmd->c_offset_in);
+	}
+
 	if (rsp.cmd_status) {
 		rsp.response		= ISCSI_STATUS_CMD_COMPLETED;
 		rsp.residual_count	= htonl(T10_CMD_RESID(t));
-		if (cmd->c_writeop == True)
-			rsp.flags |= ISCSI_FLAG_CMD_OVERFLOW;
-		else
-			rsp.flags |= ISCSI_FLAG_CMD_UNDERFLOW;
 
 		if (T10_SENSE_LEN(t) != 0) {
 			/*
@@ -784,16 +813,6 @@ send_scsi_rsp(iscsi_conn_t *c, t10_cmd_t *t)
 	} else {
 		rsp.response	= ISCSI_STATUS_CMD_COMPLETED;
 		rsp.expdatasn	= htonl(cmd->c_datasn);
-		if (cmd->c_writeop == True) {
-			if (cmd->c_offset_out != cmd->c_dlen_expected)
-				rsp.flags |= ISCSI_FLAG_CMD_OVERFLOW;
-			rsp.residual_count = htonl(cmd->c_dlen_expected -
-			    cmd->c_offset_out);
-		} else {
-			rsp.flags |= ISCSI_FLAG_CMD_UNDERFLOW;
-			rsp.residual_count = htonl(cmd->c_dlen_expected -
-			    cmd->c_offset_in);
-		}
 	}
 
 	send_iscsi_pkt(c, (iscsi_hdr_t *)&rsp, auto_sense);
@@ -1217,6 +1236,11 @@ send_iscsi_pkt(iscsi_conn_t *c, iscsi_hdr_t *h, char *opt_text)
 			return;
 		}
 		free(abuf);
+#ifdef FULL_DEBUG
+		queue_prt(c->c_mgmtq, Q_CONN_IO,
+		    "CON%x  Response(0x%x), Data: len=0x%x addr=0x%llx\n",
+		    c->c_num, h->opcode, dlen, opt_text);
+#endif
 		return;
 	}
 #endif
diff --git a/usr/src/cmd/iscsi/iscsitgtd/iscsi_login.c b/usr/src/cmd/iscsi/iscsitgtd/iscsi_login.c
index 27a736f500..9c015d1d90 100644
--- a/usr/src/cmd/iscsi/iscsitgtd/iscsi_login.c
+++ b/usr/src/cmd/iscsi/iscsitgtd/iscsi_login.c
@@ -208,6 +208,7 @@ iscsi_handle_login_pkt(iscsi_conn_t *c)
 		    ISCSI_LOGIN_STATUS_NO_VERSION);
 		queue_str(c->c_mgmtq, Q_CONN_ERRS, msg_log, debug);
 		conn_state(c, T7);
+		free(rsp);
 		return (True);
 	}
 
diff --git a/usr/src/cmd/iscsi/iscsitgtd/main.c b/usr/src/cmd/iscsi/iscsitgtd/main.c
index 02d9bd5e41..73b0a7a343 100644
--- a/usr/src/cmd/iscsi/iscsitgtd/main.c
+++ b/usr/src/cmd/iscsi/iscsitgtd/main.c
@@ -842,8 +842,8 @@ main(int argc, char **argv)
 	(void) sigignore(SIGPIPE);
 
 	/*
-	 * Look at the function lu_buserr_handler() above to see the details
-	 * of why we need to handle segmentation violations.
+	 * Look at the function lu_buserr_handler() in t10_sam.c to see the
+	 * details of why we need to handle segmentation violations.
 	 */
 	bzero(&act, sizeof (act));
 	act.sa_sigaction	= lu_buserr_handler;
diff --git a/usr/src/cmd/iscsi/iscsitgtd/t10_sam.c b/usr/src/cmd/iscsi/iscsitgtd/t10_sam.c
index 288caa4a50..2da22ac625 100644
--- a/usr/src/cmd/iscsi/iscsitgtd/t10_sam.c
+++ b/usr/src/cmd/iscsi/iscsitgtd/t10_sam.c
@@ -20,7 +20,7 @@
  */
 
 /*
- * Copyright 2006 Sun Microsystems, Inc.  All rights reserved.
+ * Copyright 2007 Sun Microsystems, Inc.  All rights reserved.
  * Use is subject to license terms.
  */
 
@@ -498,9 +498,6 @@ static t10_cmd_state_t
 t10_cmd_state_machine(t10_cmd_t *c, t10_cmd_event_t e)
 {
 	t10_lu_impl_t	*lu		= c->c_lu;
-#ifdef FULL_DEBUG
-	t10_cmd_state_t	oldstate	= c->c_state;
-#endif
 
 	/* ---- Callers must already hold the mutex ---- */
 	assert(pthread_mutex_trylock(&lu->l_cmd_mutex) != 0);
@@ -516,12 +513,6 @@ t10_cmd_state_machine(t10_cmd_t *c, t10_cmd_event_t e)
 
 		case T10_Cmd_T5:
 			c->c_state = T10_Cmd_S1_Free;
-#ifdef FULL_DEBUG
-			queue_prt(mgmtq, Q_STE_IO,
-			    "SAM:  %s(%s) -> %s -- %llx\n",
-			    state_to_str(oldstate), event_to_str(e),
-			    state_to_str(c->c_state), c->c_trans_id);
-#endif
 			cmd_common_free(c);
 			return (T10_Cmd_S1_Free);
 
@@ -548,12 +539,6 @@ t10_cmd_state_machine(t10_cmd_t *c, t10_cmd_event_t e)
 
 		case T10_Cmd_T5:
 			c->c_state = T10_Cmd_S1_Free;
-#ifdef FULL_DEBUG
-			queue_prt(mgmtq, Q_STE_IO,
-			    "SAM:  %s(%s) -> %s -- %llx\n",
-			    state_to_str(oldstate), event_to_str(e),
-			    state_to_str(c->c_state), c->c_trans_id);
-#endif
 			cmd_common_free(c);
 			return (T10_Cmd_S1_Free);
 
@@ -581,12 +566,6 @@ t10_cmd_state_machine(t10_cmd_t *c, t10_cmd_event_t e)
 			/*FALLTHRU*/
 		case T10_Cmd_T6:
 			c->c_state = T10_Cmd_S1_Free;
-#ifdef FULL_DEBUG
-			queue_prt(mgmtq, Q_STE_IO,
-			    "SAM:  %s(%s) -> %s -- %llx\n",
-			    state_to_str(oldstate), event_to_str(e),
-			    state_to_str(c->c_state), c->c_trans_id);
-#endif
 			cmd_common_free(c);
 			return (T10_Cmd_S1_Free);
 
@@ -628,12 +607,6 @@ t10_cmd_state_machine(t10_cmd_t *c, t10_cmd_event_t e)
 
 		case T10_Cmd_T6:
 			c->c_state = T10_Cmd_S1_Free;
-#ifdef FULL_DEBUG
-			queue_prt(mgmtq, Q_STE_IO,
-			    "SAM:  %s(%s) -> %s -- %llx\n",
-			    state_to_str(oldstate), event_to_str(e),
-			    state_to_str(c->c_state), c->c_trans_id);
-#endif
 			cmd_common_free(c);
 			return (T10_Cmd_S1_Free);
 
@@ -651,12 +624,6 @@ t10_cmd_state_machine(t10_cmd_t *c, t10_cmd_event_t e)
 		case T10_Cmd_T3:
 		case T10_Cmd_T4:
 			c->c_state = T10_Cmd_S1_Free;
-#ifdef FULL_DEBUG
-			queue_prt(mgmtq, Q_STE_IO,
-			    "SAM:  %s(%s) -> %s -- %llx\n",
-			    state_to_str(oldstate), event_to_str(e),
-			    state_to_str(c->c_state), c->c_trans_id);
-#endif
 			cmd_common_free(c);
 			return (T10_Cmd_S1_Free);
 
@@ -671,11 +638,6 @@ t10_cmd_state_machine(t10_cmd_t *c, t10_cmd_event_t e)
 	default:
 		assert(0);
 	}
-#ifdef FULL_DEBUG
-	queue_prt(mgmtq, Q_STE_IO, "SAM:  %s(%s) -> %s -- %llx\n",
-	    state_to_str(oldstate), event_to_str(e), state_to_str(c->c_state),
-	    c->c_trans_id);
-#endif
 	return (c->c_state);
 }
 
@@ -1424,8 +1386,18 @@ t10_find_lun(t10_targ_impl_t *t, int lun, t10_cmd_t *cmd)
 
 	if ((ll = tgt_node_next(targ, XML_ELEMENT_LUNLIST, NULL)) == NULL)
 		goto error;
-	if ((n = tgt_node_next(ll, XML_ELEMENT_LUN, NULL)) == NULL)
+	n = NULL;
+	while ((n = tgt_node_next(ll, XML_ELEMENT_LUN, n)) != NULL) {
+		if (strtol(n->x_value, NULL, 0) == lun)
+			break;
+	}
+	if (n == NULL) {
+		spc_sense_create(cmd, KEY_ILLEGAL_REQUEST, 0);
+		/* ---- ACCESS DENIED - INVALID LU IDENTIFIER ---- */
+		spc_sense_ascq(cmd, 0x20, 0x9);
 		goto error;
+	}
+
 	if (tgt_find_value_str(n, XML_ELEMENT_GUID, &guid) == False) {
 		/*
 		 * Set the targ variable back to NULL to indicate that
diff --git a/usr/src/cmd/iscsi/iscsitgtd/t10_spc.c b/usr/src/cmd/iscsi/iscsitgtd/t10_spc.c
index 28ff505fd4..2df2c5a69d 100644
--- a/usr/src/cmd/iscsi/iscsitgtd/t10_spc.c
+++ b/usr/src/cmd/iscsi/iscsitgtd/t10_spc.c
@@ -20,7 +20,7 @@
  */
 
 /*
- * Copyright 2006 Sun Microsystems, Inc.  All rights reserved.
+ * Copyright 2007 Sun Microsystems, Inc.  All rights reserved.
  * Use is subject to license terms.
  */
 
@@ -572,7 +572,7 @@ spc_mselect_data(t10_cmd_t *cmd, emul_handle_t id, size_t offset, char *data,
 		spc_sense_create(cmd, KEY_ILLEGAL_REQUEST, 0);
 		spc_sense_ascq(cmd, SPC_ASC_INVALID_CDB, 0x00);
 		trans_send_complete(cmd, STATUS_CHECK);
-		break;
+		return;
 	}
 	trans_send_complete(cmd, STATUS_GOOD);
 }
@@ -671,8 +671,9 @@ spc_report_luns(t10_cmd_t *cmd, uint8_t *cdb, size_t cdb_len)
 				continue;
 			lun_idx += SCSI_REPORTLUNS_ADDRESS_SIZE;
 		}
-		if (trans_send_datain(cmd, (char *)buf, expected_data, 0,
-		    spc_free, True, buf) == False) {
+		if (trans_send_datain(cmd, (char *)buf,
+		    len + SCSI_REPORTLUNS_ADDRESS_SIZE, 0, spc_free, True,
+		    buf) == False) {
 			trans_send_complete(cmd, STATUS_BUSY);
 		}
 	} else {
diff --git a/usr/src/cmd/iscsi/iscsitgtd/util_port.c b/usr/src/cmd/iscsi/iscsitgtd/util_port.c
index 0c62e1b456..46801a4fde 100644
--- a/usr/src/cmd/iscsi/iscsitgtd/util_port.c
+++ b/usr/src/cmd/iscsi/iscsitgtd/util_port.c
@@ -20,7 +20,7 @@
  */
 
 /*
- * Copyright 2006 Sun Microsystems, Inc.  All rights reserved.
+ * Copyright 2007 Sun Microsystems, Inc.  All rights reserved.
  * Use is subject to license terms.
  */
 
@@ -34,6 +34,7 @@
 #include <stdio.h>
 #include <errno.h>
 #include <netinet/in.h>
+#include <netinet/tcp.h>
 #include <assert.h>
 #include <syslog.h>
 #include <unistd.h>
@@ -71,6 +72,7 @@ port_watcher(void *v)
 	target_queue_t		*q = p->port_mgmtq;
 	int			l,
 				accept_err_sleep = 1;
+	const int		just_say_no = 1;
 	pthread_t		junk;
 	struct in_addr		addr;
 	struct in6_addr		addr6;
@@ -162,6 +164,11 @@ port_watcher(void *v)
 			queue_str(q, Q_GEN_ERRS, msg_status,
 			    "setsockopt keepalive failed");
 
+		if (setsockopt(fd, IPPROTO_TCP, TCP_NODELAY,
+		    (char *)&just_say_no, sizeof (just_say_no)) != 0)
+			queue_str(q, Q_GEN_ERRS, msg_status,
+			    "setsockopt NODELAY failed");
+
 		if ((conn = (iscsi_conn_t *)calloc(sizeof (iscsi_conn_t),
 		    1)) == NULL) {
 			/*