[PATCH 1/1] SCSI residual reporting

[ronnie sahlberg <ronniesahlberg@xxxxxxxxx>]

Tomo, List

Please find attached a patch against the residual calculation for iscsi.
The residual calculation used to check task->len versus the size of
the in buffer,
which probably always would be the same so residuals were probably
never reported.

Instead I think more correct is to check the in->residual counter for
the actual buffer and use it as
the residual detection.

The patch also sets the expected and residual values from the sbc_rw()
function, which should make READ*/WRITE*
now setting and reproting residuals properly.
(Well,  the write functions will not do that yet since the lack of
out->residual checking, but that is just as before)


To test this I use a special test tool which contains tests for
different types of malformed iscsi/scsi commands.
In this case sedning iSCSI commands that have EDTL values that do not
match up to the READ10 TRANSFER LENGTH.


I have also attached a small capture of this particular test that
shows how tgtd responds with this patch.
Ignore packets 20/21 for now. I dont know what these should result in
but it probably should be a check condition, not just a simple
residual.



regards
ronnie sahlberg

Attachment: 0001-Implement-residual-correctly-when-creating-a-DATA-IN.patch.gz
Description: GNU Zip compressed data

From 7ce7da39cb978db5cdb29010ada420b752d725af Mon Sep 17 00:00:00 2001
From: Ronnie Sahlberg <ronniesahlberg@xxxxxxxxx>
Date: Mon, 18 Apr 2011 17:11:00 +1000
Subject: [PATCH] Implement residual correctly when creating a DATA-IN PDU.
 Look at the residual fields in the actual data buffer instead of
 comparing the task->len with the amount of data to send back.
 (which will always be the equal anyway)

Make SBC READ commands set the residual in case the read CDB specifies more data than was set in the iscsi layer for buffer size.

Signed-off-by: Ronnie Sahlberg <ronniesahlberg@xxxxxxxxx>
---
 usr/iscsi/iscsid.c |   18 +++++++++---------
 usr/sbc.c          |    2 ++
 2 files changed, 11 insertions(+), 9 deletions(-)

diff --git a/usr/iscsi/iscsid.c b/usr/iscsi/iscsid.c
index 357b64a..33f397c 100644
--- a/usr/iscsi/iscsid.c
+++ b/usr/iscsi/iscsid.c
@@ -996,27 +996,27 @@ static void calc_residual(struct iscsi_cmd_rsp *rsp, struct iscsi_task *task)
 {
 	uint32_t residual = 0;
 	struct scsi_cmd *scmd = &task->scmd;
-	uint32_t read_len = scsi_get_in_length(scmd);
+	int in_residual = scsi_get_in_resid(scmd);
 
 	/* we never have write under/over flow, no way to signal that
 	 * back from the target currently. */
 	if (scsi_get_data_dir(scmd) == DATA_BIDIRECTIONAL) {
-		if (task->len < read_len) {
+		if (in_residual > 0) {
 			rsp->flags |= ISCSI_FLAG_CMD_BIDI_UNDERFLOW;
-			residual = read_len - task->len;
-		} else if (task->len > read_len) {
+			residual = in_residual;
+		} else if (in_residual < 0) {
 			rsp->flags |= ISCSI_FLAG_CMD_BIDI_OVERFLOW;
-			residual = task->len - read_len;
+			residual = -in_residual;
 		}
 		rsp->bi_residual_count = cpu_to_be32(residual);
 		rsp->residual_count = 0;
 	} else {
-		if (task->len < read_len) {
+		if (in_residual > 0) {
 			rsp->flags |= ISCSI_FLAG_CMD_UNDERFLOW;
-			residual = read_len - task->len;
-		} else if (task->len > read_len) {
+			residual = in_residual;
+		} else if (in_residual < 0) {
 			rsp->flags |= ISCSI_FLAG_CMD_OVERFLOW;
-			residual = task->len - read_len;
+			residual = -in_residual;
 		}
 		rsp->residual_count = cpu_to_be32(residual);
 	}
diff --git a/usr/sbc.c b/usr/sbc.c
index cb07be6..d5d682a 100644
--- a/usr/sbc.c
+++ b/usr/sbc.c
@@ -140,6 +140,8 @@ static int sbc_rw(int host_no, struct scsi_cmd *cmd)
 	lba = scsi_rw_offset(cmd->scb) << cmd->dev->blk_shift;
 	tl  = scsi_rw_count(cmd->scb) << cmd->dev->blk_shift;
 
+	scsi_set_in_resid_by_actual(cmd, tl);
+
 	/* Verify that we are not doing i/o beyond
 	   the end-of-lun */
 	if (tl && (lba + tl > lu->size)) {
-- 
1.7.3.1

Attachment: 0105_read10_invalid.cap
Description: application/cap