[Stgt-devel] your recent sense checkin

Pete Wyckoff pw
Wed Feb 28 21:13:23 CET 2007


I took a look at your SVN and rebased my patches on top of it.  One
thing to point out:  it should be possible for a target to send back
both data-in bytes as the result of a command, and status bytes in
the response.  Your use of task->addr to get the status bytes
appears to make that impossible.  I had added extra task->sense and
task->senselen to accommodate this usage.

Regarding the "Need to clean up this mess" comment in
iscsi_scsi_cmd_tx_start:  it gets worse once we mix in the
bidirectional.  There you cannot do a phase collapse on the final
data packet, so it becomes even more of a mess.

I have cleaned most of that mess up, perhaps, as part of getting
bidirectional transfers to work, but the patches are not ready for
submission to stgt because they depend on AHS structures defined
only in non-mainline kernel patches.  If you are willing to remove
the dependency on kernel headers by copying the relevant bits into
stgt, we can avoid that problem.

Note the duplication in iscsi_cmd_rsp_build and
iscsi_sense_rsp_build; wouldn't it be better to have a single
function with "if (task->result != 0) {...}" to return status?

Here they are, fyi.  Let me know if you have suggestions on how I
can help you get more of this merged.

		-- Pete
-------------- next part --------------
return sense data in iscsi response

From: Pete Wyckoff <pw at osc.edu>

It is legal to return both data and sense data.  Modify iscsi to do this.
Let target and scsi generate sense information to return through the task.
---

 usr/iscsi/iscsid.c |   27 +++++++++++++++------------
 usr/iscsi/iscsid.h |    3 +++
 usr/target.c       |    5 +++--
 usr/tgtd.h         |    2 ++
 4 files changed, 23 insertions(+), 14 deletions(-)

diff --git a/usr/iscsi/iscsid.c b/usr/iscsi/iscsid.c
index 1d0cd9a..920a07a 100644
--- a/usr/iscsi/iscsid.c
+++ b/usr/iscsi/iscsid.c
@@ -766,7 +766,7 @@ static int iscsi_sense_rsp_build(struct iscsi_task *task)
 	struct iscsi_connection *conn = task->conn;
 	struct iscsi_cmd_rsp *rsp = (struct iscsi_cmd_rsp *) &conn->rsp.bhs;
 	struct iscsi_sense_data *sense;
-	uint16_t sense_len = task->len;
+	uint16_t len = task->senselen;
 
 	memset(rsp, 0, sizeof(*rsp));
 	rsp->opcode = ISCSI_OP_SCSI_CMD_RSP;
@@ -775,18 +775,19 @@ static int iscsi_sense_rsp_build(struct iscsi_task *task)
 	rsp->response = ISCSI_STATUS_CMD_COMPLETED;
 	rsp->cmd_status = SAM_STAT_CHECK_CONDITION;
 
-	sense = (void *) (unsigned long) task->addr;
+	dprintf("len %d senselen %d\n", task->len, task->senselen);
 
-	/* FIXME: we assume that sense_buffer is large enough
-	 * (sense_len + 2 bytes). It's true now, but... */
+	sense = (void *)(uintptr_t) conn->sense_buffer;
 
-	memmove(sense->data, sense, sense_len);
-	sense->length = cpu_to_be16(sense_len);
+	if (len > sizeof(conn->sense_buffer)-2)
+		len = sizeof(conn->sense_buffer) - 2;
 
-	conn->rsp.datasize = sense_len + sizeof(*sense);
-	hton24(rsp->dlength, sense_len + sizeof(*sense));
-	conn->rsp.data = (void *) (unsigned long) task->addr;
-	task->offset += sense_len + sizeof(*sense);
+	memcpy(sense->data, task->sense, len);
+	sense->length = cpu_to_be16(len);
+
+	conn->rsp.datasize = len + sizeof(*sense);
+	hton24(rsp->dlength, len + sizeof(*sense));
+	conn->rsp.data = conn->sense_buffer;
 
 	return 0;
 }
@@ -916,8 +917,8 @@ int iscsi_scsi_cmd_done(uint64_t nid, int result, struct scsi_cmd *cmd)
 	struct iscsi_session *session;
 	struct iscsi_task *task;
 
-	dprintf("%" PRIu64 " %d %d %d %" PRIx64 " %" PRIx64 "\n", nid,
-		cmd->len, result, cmd->rw, cmd->uaddr, cmd->tag);
+	dprintf("%" PRIu64 " %d %d %d %" PRIx64 " %" PRIx64 "senselen %d\n", nid,
+		cmd->len, result, cmd->rw, cmd->uaddr, cmd->tag, cmd->senselen);
 	session = session_lookup(nid);
 	if (!session)
 		return -EINVAL;
@@ -947,6 +948,8 @@ found:
 	task->result = result;
 	task->len = cmd->len;
 	task->rw = cmd->rw;
+	task->sense = cmd->sense;
+	task->senselen = cmd->senselen;
 
 	list_add_tail(&task->c_list, &task->conn->tx_clist);
 	tgt_event_modify(task->conn->fd, EPOLLIN | EPOLLOUT);
diff --git a/usr/iscsi/iscsid.h b/usr/iscsi/iscsid.h
index c4b23d9..bc6ec39 100644
--- a/usr/iscsi/iscsid.h
+++ b/usr/iscsi/iscsid.h
@@ -103,6 +103,8 @@ struct iscsi_task {
 	int result;
 	int len;
 	int rw;
+	uint8_t *sense;
+	uint8_t senselen;
 
 	int offset;
 	int data_sn;
@@ -146,6 +148,7 @@ struct iscsi_connection {
 	struct iscsi_pdu req;
 	void *req_buffer;
 	struct iscsi_pdu rsp;
+	uint8_t sense_buffer[252];  /* architectural limit */
 	void *rsp_buffer;
 	unsigned char *rx_buffer;
 	unsigned char *tx_buffer;
diff --git a/usr/target.c b/usr/target.c
index f50089b..f2b16d7 100644
--- a/usr/target.c
+++ b/usr/target.c
@@ -463,8 +463,9 @@ int target_cmd_queue(uint64_t nid, uint8_t *scb, int scblen, uint8_t rw,
 
 		cmd_post_perform(q, cmd);
 
-		dprintf("%" PRIx64 " %x %" PRIx64 " %" PRIu64 " %u %d %d\n",
-			tag, scb[0], cmd->uaddr, cmd->offset, cmd->len, result, cmd->async);
+		dprintf("%" PRIx64 " %x %lx %" PRIu64 " %d %d %d senselen %d\n",
+			tag, scb[0], cmd->uaddr, cmd->offset, cmd->len, result,
+			cmd->async, cmd->senselen);
 
 		set_cmd_processed(cmd);
 		if (!cmd->async)
diff --git a/usr/tgtd.h b/usr/tgtd.h
index 9cb1b41..18ccd11 100644
--- a/usr/tgtd.h
+++ b/usr/tgtd.h
@@ -94,6 +94,8 @@ struct scsi_cmd {
 
 #define SCSI_SENSE_BUFFERSIZE	96
 	unsigned char sense_buffer[SCSI_SENSE_BUFFERSIZE];
+	uint8_t *sense;    /* output values from target */
+	uint8_t senselen;
 };
 
 #ifdef USE_KERNEL
-------------- next part --------------
Make bidirectional transfers work.  Also updates iscsi code to use

From: Pete Wyckoff <pw at osc.edu>

new kernel data structures for extended CDBs in Panasas patch.
---

 usr/iscsi/iscsid.c |  241 ++++++++++++++++++++++++++++++++++++++++------------
 usr/iscsi/iscsid.h |   23 +++--
 usr/osd.c          |    6 +
 usr/target.c       |    4 +
 4 files changed, 207 insertions(+), 67 deletions(-)

diff --git a/usr/iscsi/iscsid.c b/usr/iscsi/iscsid.c
index 920a07a..e8d03aa 100644
--- a/usr/iscsi/iscsid.c
+++ b/usr/iscsi/iscsid.c
@@ -738,10 +738,15 @@ static void cmnd_finish(struct iscsi_connection *conn)
 	}
 }
 
+/*
+ * Send the final command response.  For successful (and non-bidirectional)
+ * tasks that return data, this packet is not required.
+ */
 static int iscsi_cmd_rsp_build(struct iscsi_task *task)
 {
 	struct iscsi_connection *conn = task->conn;
 	struct iscsi_cmd_rsp *rsp = (struct iscsi_cmd_rsp *) &conn->rsp.bhs;
+	uint32_t residual;
 
 	memset(rsp, 0, sizeof(*rsp));
 	rsp->opcode = ISCSI_OP_SCSI_CMD_RSP;
@@ -753,6 +758,31 @@ static int iscsi_cmd_rsp_build(struct iscsi_task *task)
 	rsp->exp_cmdsn = cpu_to_be32(conn->session->exp_cmd_sn);
 	rsp->max_cmdsn = cpu_to_be32(conn->session->exp_cmd_sn + MAX_QUEUE_CMD);
 
+	/* we never have write under/over flow, no way to signal that
+	 * back from the target currently. */
+
+	residual = 0;
+	if (task->dir == BIDIRECTIONAL) {
+		if (task->len < task->read_len) {
+			rsp->flags |= ISCSI_FLAG_CMD_BIDI_UNDERFLOW;
+			residual = task->read_len - task->len;
+		} else if (task->len > task->read_len) {
+			rsp->flags |= ISCSI_FLAG_CMD_BIDI_OVERFLOW;
+			residual = task->len - task->read_len;
+		}
+		rsp->bi_residual_count = cpu_to_be32(residual);
+		rsp->residual_count = 0;
+	} else {
+		if (task->len < task->read_len) {
+			rsp->flags |= ISCSI_FLAG_CMD_UNDERFLOW;
+			residual = task->read_len - task->len;
+		} else if (task->len > task->read_len) {
+			rsp->flags |= ISCSI_FLAG_CMD_OVERFLOW;
+			residual = task->len - task->read_len;
+		}
+		rsp->residual_count = cpu_to_be32(residual);
+	}
+
 	return 0;
 }
 
@@ -766,6 +796,7 @@ static int iscsi_sense_rsp_build(struct iscsi_task *task)
 	struct iscsi_connection *conn = task->conn;
 	struct iscsi_cmd_rsp *rsp = (struct iscsi_cmd_rsp *) &conn->rsp.bhs;
 	struct iscsi_sense_data *sense;
+	uint32_t residual;
 	uint16_t len = task->senselen;
 
 	memset(rsp, 0, sizeof(*rsp));
@@ -774,8 +805,37 @@ static int iscsi_sense_rsp_build(struct iscsi_task *task)
 	rsp->flags = ISCSI_FLAG_CMD_FINAL;
 	rsp->response = ISCSI_STATUS_CMD_COMPLETED;
 	rsp->cmd_status = SAM_STAT_CHECK_CONDITION;
+	rsp->statsn = cpu_to_be32(conn->stat_sn++);
+	rsp->exp_cmdsn = cpu_to_be32(conn->session->exp_cmd_sn);
+	rsp->max_cmdsn = cpu_to_be32(conn->session->exp_cmd_sn + MAX_QUEUE_CMD);
 
-	dprintf("len %d senselen %d\n", task->len, task->senselen);
+	/* we never have write under/over flow, no way to signal that
+	 * back from the target currently. */
+
+	residual = 0;
+	if (task->dir == BIDIRECTIONAL) {
+		if (task->len < task->read_len) {
+			rsp->flags |= ISCSI_FLAG_CMD_BIDI_UNDERFLOW;
+			residual = task->read_len - task->len;
+		} else if (task->len > task->read_len) {
+			rsp->flags |= ISCSI_FLAG_CMD_BIDI_OVERFLOW;
+			residual = task->len - task->read_len;
+		}
+		rsp->bi_residual_count = cpu_to_be32(residual);
+		rsp->residual_count = 0;
+	} else {
+		if (task->len < task->read_len) {
+			rsp->flags |= ISCSI_FLAG_CMD_UNDERFLOW;
+			residual = task->read_len - task->len;
+		} else if (task->len > task->read_len) {
+			rsp->flags |= ISCSI_FLAG_CMD_OVERFLOW;
+			residual = task->len - task->read_len;
+		}
+		rsp->residual_count = cpu_to_be32(residual);
+	}
+
+	dprintf("status %d len %d senselen %d\n", task->result, task->len,
+	        task->senselen);
 
 	sense = (void *)(uintptr_t) conn->sense_buffer;
 
@@ -792,23 +852,24 @@ static int iscsi_sense_rsp_build(struct iscsi_task *task)
 	return 0;
 }
 
+/*
+ * Send a data-in PDU.  If status was 0, collapse the response message
+ * into the last data-in PDU.
+ */
 static int iscsi_data_rsp_build(struct iscsi_task *task)
 {
 	struct iscsi_connection *conn = task->conn;
 	struct iscsi_data_rsp *rsp = (struct iscsi_data_rsp *) &conn->rsp.bhs;
-	struct iscsi_cmd *req = (struct iscsi_cmd *) &task->req;
-	int residual, datalen, exp_datalen = ntohl(req->data_length);
+	int residual, datalen, exp_datalen = task->read_len;
 	int max_burst = conn->session_param[ISCSI_PARAM_MAX_XMIT_DLENGTH].val;
 
 	memset(rsp, 0, sizeof(*rsp));
 	rsp->opcode = ISCSI_OP_SCSI_DATA_IN;
 	rsp->itt = task->tag;
 	rsp->ttt = cpu_to_be32(ISCSI_RESERVED_TAG);
-	rsp->cmd_status = ISCSI_STATUS_CMD_COMPLETED;
 
 	rsp->offset = cpu_to_be32(task->offset);
-	rsp->datasn = cpu_to_be32(task->data_sn++);
-	rsp->cmd_status = task->result;
+	rsp->datasn = cpu_to_be32(task->exp_r2tsn++);
 
 	datalen = min(exp_datalen, task->len);
 	datalen -= task->offset;
@@ -816,21 +877,26 @@ static int iscsi_data_rsp_build(struct iscsi_task *task)
 	dprintf("%d %d %d %d %x\n", datalen, exp_datalen, task->len, max_burst, rsp->itt);
 
 	if (datalen <= max_burst) {
-		rsp->flags = ISCSI_FLAG_CMD_FINAL | ISCSI_FLAG_DATA_STATUS;
-		if (task->len < exp_datalen) {
-			rsp->flags |= ISCSI_FLAG_CMD_UNDERFLOW;
-			residual = exp_datalen - task->len;
-		} else if (task->len > exp_datalen) {
-			rsp->flags |= ISCSI_FLAG_CMD_OVERFLOW;
-			residual = task->len - exp_datalen;
-		} else
-			residual = 0;
-		rsp->residual_count = cpu_to_be32(residual);
+		rsp->flags = ISCSI_FLAG_CMD_FINAL;
+
+		/* collapse status into final packet if successful */
+		if (task->result == 0 && task->dir != BIDIRECTIONAL) {
+			rsp->flags |= ISCSI_FLAG_DATA_STATUS;
+			if (task->len < exp_datalen) {
+				rsp->flags |= ISCSI_FLAG_CMD_UNDERFLOW;
+				residual = exp_datalen - task->len;
+			} else if (task->len > exp_datalen) {
+				rsp->flags |= ISCSI_FLAG_CMD_OVERFLOW;
+				residual = task->len - exp_datalen;
+			} else
+				residual = 0;
+			rsp->cmd_status = task->result;
+			rsp->statsn = cpu_to_be32(conn->stat_sn++);
+			rsp->residual_count = cpu_to_be32(residual);
+		}
 	} else
 		datalen = max_burst;
 
-	if (rsp->flags & ISCSI_FLAG_CMD_FINAL)
-		rsp->statsn = cpu_to_be32(conn->stat_sn++);
 	rsp->exp_cmdsn = cpu_to_be32(conn->session->exp_cmd_sn);
 	rsp->max_cmdsn = cpu_to_be32(conn->session->exp_cmd_sn + MAX_QUEUE_CMD);
 
@@ -912,12 +978,17 @@ static void iscsi_free_cmd_task(struct iscsi_task *task)
 	iscsi_free_task(task);
 }
 
+/*
+ * Called from target layer when the command completes.  The addr and
+ * len are what should be sent back.
+ */
 int iscsi_scsi_cmd_done(uint64_t nid, int result, struct scsi_cmd *cmd)
 {
 	struct iscsi_session *session;
 	struct iscsi_task *task;
 
-	dprintf("%" PRIu64 " %d %d %d %" PRIx64 " %" PRIx64 "senselen %d\n", nid,
+	dprintf("len %" PRIu64 " result %d rw %d addr %d %" PRIx64
+	        " tag %" PRIx64 " senselen %d\n", nid,
 		cmd->len, result, cmd->rw, cmd->uaddr, cmd->tag, cmd->senselen);
 	session = session_lookup(nid);
 	if (!session)
@@ -977,30 +1048,46 @@ static int cmd_attr(struct iscsi_task *task)
 	return attr;
 }
 
+/*
+ * Try to execute the command.  Called when the initial scsi command PDU
+ * arrives and after each finished data burst.  If there is still data-out
+ * unreceived, put back on the queue to wait until that is done.
+ */
 static int iscsi_scsi_cmd_execute(struct iscsi_task *task)
 {
 	struct iscsi_connection *conn = task->conn;
 	struct iscsi_cmd *req = (struct iscsi_cmd *) &task->req;
 	unsigned long uaddr = (unsigned long) task->data;
 	uint8_t rw = req->flags & ISCSI_FLAG_CMD_WRITE;
-	uint8_t *cdb, cdbbuf[260];
+	uint32_t data_len;
+	uint8_t *cdb, cdbbuf[260], *ahs, ahslen;
 	int cdblen;
 	int err = 0;
 
-	if (rw && task->r2t_count) {
-		if (!task->unsol_count)
+	/* wait for more data; if unsol, initiator will send without r2t */
+	if (task->r2t_count) {
+		if (!task->waiting_unsol)
 			list_add_tail(&task->c_list, &task->conn->tx_clist);
 		goto noqueue;
 	}
 
+	task->offset = 0;  /* for use as transmit pointer for data-ins */
+
+	/* build full cdb for target, possibly using ahs */
 	cdb = req->cdb;
 	cdblen = 16;
-	if (req->hlength) {
+	ahs = task->ahs;
+	ahslen = req->hlength * 4;
+	if (ahslen >= 4) {
 		/* concatenate extended cdb */
-		struct iscsi_ecdb_ahdr *ahs_extcdb = task->ahs;
+		struct iscsi_ecdb_ahdr *ahs_extcdb = (void *) ahs;
 		if (ahs_extcdb->ahstype == ISCSI_AHSTYPE_CDB) {
 			int extlen = ntohs(ahs_extcdb->ahslength) - 1;
 			dprintf("extcdb len %d\n", extlen);
+			if (4 + extlen > ahslen) {
+				err = -EINVAL;
+				goto noqueue;
+			}
 			if (extlen + 16 > sizeof(cdbbuf)) {
 				err = -ENOMEM;
 				goto noqueue;
@@ -1009,13 +1096,40 @@ static int iscsi_scsi_cmd_execute(struct iscsi_task *task)
 			memcpy(cdbbuf + 16, ahs_extcdb->ecdb, extlen);
 			cdb = cdbbuf;
 			cdblen = 16 + extlen;
+
+			/* advance pointers for possible bidi */
+			ahs += 4 + extlen;
+			ahslen -= 4 + extlen;
 		}
 	}
 
+	/* figure out incoming (write) and outgoing (read) sizes */
+	data_len = 0;
+	task->write_len = 0;
+	if (task->dir == WRITE || task->dir == BIDIRECTIONAL) {
+		task->write_len = ntohl(req->data_length);
+		data_len = task->write_len;
+	}
+	task->read_len = 0;
+	if (task->dir == BIDIRECTIONAL && ahslen >= 8) {
+		struct iscsi_rlength_ahdr *ahs_bidi = (void *) ahs;
+		if (ahs_bidi->ahstype == ISCSI_AHSTYPE_RLENGTH) {
+			task->read_len = ntohl(ahs_bidi->read_length);
+			dprintf("bidi read len %u\n", task->read_len);
+		}
+	}
+	if (task->dir == READ) {
+		task->read_len = ntohl(req->data_length);
+		data_len = task->read_len;
+	}
+
+	/*
+	 * When done, target will call iscsi_scsi_cmd_done with
+	 * addr and len of data-in, as well as sense.
+	 */
 	err = target_cmd_queue(conn->session->iscsi_nexus_id,
 			       cdb, cdblen, rw, uaddr, req->lun,
-			       ntohl(req->data_length),
-			       cmd_attr(task), req->itt);
+			       data_len, cmd_attr(task), req->itt);
 
 noqueue:
 	tgt_event_modify(conn->fd, EPOLLIN|EPOLLOUT);
@@ -1105,6 +1219,17 @@ static int iscsi_task_execute(struct iscsi_task *task)
 		tgt_event_modify(task->conn->fd, EPOLLIN | EPOLLOUT);
 		break;
 	case ISCSI_OP_SCSI_CMD:
+		/* convenient directionality for our internal use */
+		if (hdr->flags & ISCSI_FLAG_CMD_READ) {
+			if (hdr->flags & ISCSI_FLAG_CMD_WRITE)
+				task->dir = BIDIRECTIONAL;
+			else
+				task->dir = READ;
+		} else if (hdr->flags & ISCSI_FLAG_CMD_WRITE) {
+			task->dir = WRITE;
+		} else
+			task->dir = NONE;
+
 		err = iscsi_scsi_cmd_execute(task);
 		break;
 	case ISCSI_OP_SCSI_TMFUNC:
@@ -1130,12 +1255,14 @@ static int iscsi_data_out_rx_done(struct iscsi_task *task)
 	int err = 0;
 
 	if (hdr->ttt == cpu_to_be32(ISCSI_RESERVED_TAG)) {
+		/* unsolicited data, accumulate until final */
 		if (hdr->flags & ISCSI_FLAG_CMD_FINAL) {
-			task->unsol_count = 0;
+			task->waiting_unsol = 0;
 			if (!task_pending(task))
 				err = iscsi_scsi_cmd_execute(task);
 		}
 	} else {
+		/* response to a r2t we sent */
 		if (!(hdr->flags & ISCSI_FLAG_CMD_FINAL))
 			return err;
 
@@ -1162,7 +1289,7 @@ found:
 		task->r2t_count,
 		ntoh24(req->dlength), be32_to_cpu(req->offset));
 
-	conn->rx_buffer = (void *) (unsigned long) task->c_buffer;
+	conn->rx_buffer = task->c_buffer;
 	conn->rx_buffer += be32_to_cpu(req->offset);
 	conn->rx_size = ntoh24(req->dlength);
 
@@ -1185,6 +1312,7 @@ static int iscsi_task_queue(struct iscsi_task *task)
 	dprintf("%x %x %x\n", be32_to_cpu(req->statsn), session->exp_cmd_sn,
 		req->opcode);
 
+	/* immediate live outside of the CmdSN space */
 	if (req->opcode & ISCSI_OP_IMMEDIATE)
 		return iscsi_task_execute(task);
 
@@ -1253,9 +1381,9 @@ static int iscsi_scsi_cmd_rx_start(struct iscsi_connection *conn)
 	dlen = ntoh24(req->dlength);  /* just part in this PDU */
 	tot_data_length = ntohl(req->data_length);  /* all data */
 
-	dprintf("%u %x %d %d %x task %p lens %d %d %d\n", conn->session->tsih,
+	dprintf("%u %x %d %d task %p lens %d %d %d\n", conn->session->tsih,
 		req->cdb[0], ntohl(req->data_length),
-		req->flags & ISCSI_FLAG_CMD_ATTR_MASK, req->itt,
+		req->flags & ISCSI_FLAG_CMD_ATTR_MASK,
 		task, ahslen, dlen, tot_data_length);
 
 	/*
@@ -1286,11 +1414,11 @@ static int iscsi_scsi_cmd_rx_start(struct iscsi_connection *conn)
 
 	if (req->flags & ISCSI_FLAG_CMD_WRITE) {
 		task->r2t_count = tot_data_length - dlen;  /* bytes left */
-		task->unsol_count = !(req->flags & ISCSI_FLAG_CMD_FINAL);
+		task->waiting_unsol = !(req->flags & ISCSI_FLAG_CMD_FINAL);
 		task->offset = dlen;
 
 		dprintf("%d %d %d %d\n", conn->rx_size, task->r2t_count,
-			task->unsol_count, task->offset);
+			task->waiting_unsol, task->offset);
 	}
 
 	list_add(&task->c_hlist, &conn->session->cmd_list);
@@ -1413,30 +1541,23 @@ static int iscsi_task_rx_start(struct iscsi_connection *conn)
 	return 0;
 }
 
+/*
+ * Send something.  Could be a data-in PDU or a response.  In theory
+ * this can happen while we are still waiting data-out, but in this
+ * implementation, all data-out is received first.
+ */
 static int iscsi_scsi_cmd_tx_start(struct iscsi_task *task)
 {
 	int err = 0;
-	struct iscsi_cmd *req = (struct iscsi_cmd *) &task->req;
 
 	if (task->r2t_count)
-		err = iscsi_r2t_build(task);
-	else {
-		/* Needs to clean up this mess. */
-
-		if (req->flags & ISCSI_FLAG_CMD_WRITE)
-			if (task->result)
-				err = iscsi_sense_rsp_build(task);
-			else
-				err = iscsi_cmd_rsp_build(task);
-		else {
-			if (task->result)
-				err = iscsi_sense_rsp_build(task);
-			else if (task->len)
-				err = iscsi_data_rsp_build(task);
-			else
-				err = iscsi_cmd_rsp_build(task);
-		}
-	}
+		err = iscsi_r2t_build(task);  /* still receiving data-out */
+	else if (task->offset < task->len)
+		err = iscsi_data_rsp_build(task);  /* sending data-in */
+	else if (task->result)
+		err = iscsi_sense_rsp_build(task);  /* final, status nonzero */
+	else
+		err = iscsi_cmd_rsp_build(task);  /* final response */
 
 	return err;
 }
@@ -1504,6 +1625,10 @@ static int iscsi_tm_tx_start(struct iscsi_task *task)
 	return 0;
 }
 
+/*
+ * Look at the response we just sent and figure out if there is anything
+ * more to do.
+ */
 static int iscsi_scsi_cmd_tx_done(struct iscsi_connection *conn)
 {
 	struct iscsi_hdr *hdr = &conn->rsp.bhs;
@@ -1513,8 +1638,9 @@ static int iscsi_scsi_cmd_tx_done(struct iscsi_connection *conn)
 	case ISCSI_OP_R2T:
 		break;
 	case ISCSI_OP_SCSI_DATA_IN:
-		if (!(hdr->flags & ISCSI_FLAG_CMD_FINAL)) {
-			dprintf("more data %x\n", hdr->itt);
+		if (task->offset < task->len || task->result != 0
+		   || task->dir == BIDIRECTIONAL) {
+			dprintf("more data or sense or bidir %x\n", hdr->itt);
 			list_add_tail(&task->c_list, &task->conn->tx_clist);
 			return 0;
 		}
@@ -1547,6 +1673,10 @@ static int iscsi_task_tx_done(struct iscsi_connection *conn)
 	return 0;
 }
 
+/*
+ * Pick a task that wants to transmit and switch the connection to
+ * begin the transmit state machine.
+ */
 static int iscsi_task_tx_start(struct iscsi_connection *conn)
 {
 	struct iscsi_task *task;
@@ -1558,10 +1688,9 @@ static int iscsi_task_tx_start(struct iscsi_connection *conn)
 	conn_write_pdu(conn);
 
 	task = list_entry(conn->tx_clist.next, struct iscsi_task, c_list);
-	dprintf("found a task %" PRIx64 " %u %u %u\n", task->tag,
+	dprintf("found a task %" PRIx64 " %u %u\n", task->tag,
 		ntohl(((struct iscsi_cmd *) (&task->req))->data_length),
-		task->offset,
-		task->r2t_count);
+		task->offset);
 
 	list_del(&task->c_list);
 
diff --git a/usr/iscsi/iscsid.h b/usr/iscsi/iscsid.h
index bc6ec39..8458187 100644
--- a/usr/iscsi/iscsid.h
+++ b/usr/iscsi/iscsid.h
@@ -31,12 +31,12 @@
 #define DIGEST_CRC32C           (1 << 1)
 
 #define sid64(isid, tsih)					\
-({								\
+(								\
 	(uint64_t) isid[0] <<  0 | (uint64_t) isid[1] <<  8 |	\
 	(uint64_t) isid[2] << 16 | (uint64_t) isid[3] << 24 |	\
 	(uint64_t) isid[4] << 32 | (uint64_t) isid[5] << 40 |	\
-	(uint64_t) tsih << 48;					\
-})
+	(uint64_t) tsih << 48					\
+)
 
 #define sid_to_tsih(sid) ((sid) >> 48)
 
@@ -98,20 +98,24 @@ struct iscsi_task {
 	struct list_head c_list;
 
 	unsigned long flags;
+	enum { NONE, WRITE, READ, BIDIRECTIONAL } dir;
+	uint32_t write_len;  /* from command pdu, write and read lengths */
+	uint32_t read_len;
 
 	uint64_t addr;
 	int result;
-	int len;
+	int len;  /* before execute, data-out len; after, data-in len */
 	int rw;
 	uint8_t *sense;
 	uint8_t senselen;
 
-	int offset;
-	int data_sn;
+	int offset;  /* progress in data buffer for rx or tx */
+
+	/* data out */
+	int r2t_count;  /* bytes to arrive in unsol data and to solicit */
+	int waiting_unsol;  /* bool, waiting for unsolicited data */
 
-	int r2t_count;
-	int unsol_count;
-	int exp_r2tsn;
+	int exp_r2tsn;  /* next R2T or Data SN target should generate */
 
 	void *c_buffer;
 	void *ahs;  /* these point into c_buffer, parts after bhs */
@@ -141,6 +145,7 @@ struct iscsi_connection {
 	uint32_t stat_sn;
 	uint32_t exp_stat_sn;
 
+	/* these should be session-wide, not per-connection */
 	uint32_t cmd_sn;
 	uint32_t exp_cmd_sn;
 	uint32_t max_cmd_sn;
diff --git a/usr/osd.c b/usr/osd.c
index 9417171..6fb9f0a 100644
--- a/usr/osd.c
+++ b/usr/osd.c
@@ -213,6 +213,9 @@ int osd_cmd_perform(int host_no, struct scsi_cmd *cmd, void *key)
 	static const int group_lens[8] = { 6, 10, 10, 0, 16, 12, 0, 0 };
 	uint8_t *sense, senselen = 0;
 
+	dprintf(" indata %p len %u cdb[0] %02x cdblen %d\n", (void *) *uaddr,
+	        datalen, cdb[0], cdblen);
+
 	/* check cdb length, maybe this goes up a level; but now
 	 * the well-known commands need not verify the cdblen */
 	if (cdblen < 6 ||
@@ -281,7 +284,6 @@ int osd_cmd_perform(int host_no, struct scsi_cmd *cmd, void *key)
 		*len = 0;
 		break;
 	case VARLEN_CDB:
-		dprintf("cdb[0] %x datalen %u\n", cdb[0], datalen);
 		if (cdb[7] != 200-8)
 			eprintf("request size %d wrong, should be 200\n",
 			        cdb[7]+8);
@@ -326,6 +328,8 @@ out:
 		cmd->senselen = senselen;
 	}
 
+	dprintf("outdata %p len %u\n", (void *) *uaddr, *len);
+
 	return ret;
 }
 
diff --git a/usr/target.c b/usr/target.c
index f2b16d7..9c2f4b0 100644
--- a/usr/target.c
+++ b/usr/target.c
@@ -459,11 +459,13 @@ int target_cmd_queue(uint64_t nid, uint8_t *scb, int scblen, uint8_t rw,
 		enabled = cmd_enabled(q, cmd);
 
 	if (enabled) {
+		/* This must update cmd->uaddr and cmd->len even if there
+		 * is no data-in. */
 		result = scsi_cmd_perform(nexus->host_no, cmd, (void *)cmd);
 
 		cmd_post_perform(q, cmd);
 
-		dprintf("%" PRIx64 " %x %lx %" PRIu64 " %d %d %d senselen %d\n",
+		dprintf("%" PRIx64 " %x %lx %" PRIu64 " len %d res %d %d senselen %d\n",
 			tag, scb[0], cmd->uaddr, cmd->offset, cmd->len, result,
 			cmd->async, cmd->senselen);
 



More information about the stgt mailing list