d/numam-spdk
1
0
Fork 0
Code Issues Pull Requests Projects Releases Wiki Activity

iscsi: Bug fix for residual count when length of data transfer from SCSI was 0

Browse Source 
This bug was caused by the following two changes:

iscsi: Remove duplication of the variable for write completion to bdev
https://review.gerrithub.io/#/c/393582/

iscsi: restore data_transferred accumulation for read
https://review.gerrithub.io/#/c/393713/

For write, bytes_completed is always equal to data_transferred. However,
for read, bytes_completed is based on expected data transfer length and
data_transferred is actual read size. Hence bytes_completed cannot be
used to calculate residual counts.

One of the reason why this bug cannot be found was that there was not any
test case when task->scsi.data_transferred is 0 and task->scsi.length is
not 0. Hence UT code is also added.

Same bug will occur for read sense data. Hence UT code for read sense data
is added in the next patch.

Change-Id: Ib1a283b769e5af0c2d05acb69f90948c5d658087
Signed-off-by: Shuhei Matsumoto <shuhei.matsumoto.xt@hitachi.com>
Reviewed-on: https://review.gerrithub.io/417960
Tested-by: SPDK Automated Test System <sys_sgsw@intel.com>
Reviewed-by: Jim Harris <james.r.harris@intel.com>
Reviewed-by: Daniel Verkamp <daniel.verkamp@intel.com>
Reviewed-by: Ben Walker <benjamin.walker@intel.com>
This commit is contained in:
Shuhei Matsumoto 2018-07-05 15:09:55 +09:00 committed by Jim Harris
parent 9d258c75af
commit a02ab95ebb
2 changed files with 134 additions and 1 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
lib/iscsi/iscsi.c
View File

@ -3135,7 +3135,7 @@ void spdk_iscsi_task_response(struct spdk_iscsi_conn *conn,
o_bit = u_bit = O_bit = U_bit = 0;
bidi_residual_len = residual_len = 0;
data_len = primary->bytes_completed;
data_len = primary->scsi.data_transferred;
if ((transfer_len != 0) &&
(task->scsi.status == SPDK_SCSI_STATUS_GOOD)) {

133
test/unit/lib/iscsi/iscsi.c/iscsi_ut.c
View File

@ -238,6 +238,135 @@ maxburstlength_test(void)
spdk_put_pdu(req_pdu);
}
static void
underflow_for_read_transfer_test(void)
{
struct spdk_iscsi_sess sess;
struct spdk_iscsi_conn conn;
struct spdk_iscsi_task task;
struct spdk_iscsi_pdu *pdu;
struct iscsi_bhs_scsi_req *scsi_req;
struct iscsi_bhs_data_in *datah;
uint32_t residual_count = 0;
TAILQ_INIT(&g_write_pdu_list);
memset(&sess, 0, sizeof(sess));
memset(&conn, 0, sizeof(conn));
memset(&task, 0, sizeof(task));
sess.MaxBurstLength = SPDK_ISCSI_MAX_BURST_LENGTH;
conn.sess = &sess;
conn.MaxRecvDataSegmentLength = 8192;
pdu = spdk_get_pdu();
SPDK_CU_ASSERT_FATAL(pdu != NULL);
scsi_req = (struct iscsi_bhs_scsi_req *)&pdu->bhs;
scsi_req->read_bit = 1;
spdk_iscsi_task_set_pdu(&task, pdu);
task.parent = NULL;
task.scsi.iovs = &task.scsi.iov;
task.scsi.iovcnt = 1;
task.scsi.length = 512;
task.scsi.transfer_len = 512;
task.bytes_completed = 512;
task.scsi.data_transferred = 256;
task.scsi.status = SPDK_SCSI_STATUS_GOOD;
spdk_iscsi_task_response(&conn, &task);
spdk_put_pdu(pdu);
/*
* In this case, a SCSI Data-In PDU should contain the Status
* for the data transfer.
*/
to_be32(&residual_count, 256);
pdu = TAILQ_FIRST(&g_write_pdu_list);
SPDK_CU_ASSERT_FATAL(pdu != NULL);
CU_ASSERT(pdu->bhs.opcode == ISCSI_OP_SCSI_DATAIN);
datah = (struct iscsi_bhs_data_in *)&pdu->bhs;
CU_ASSERT(datah->flags == (ISCSI_DATAIN_UNDERFLOW | ISCSI_FLAG_FINAL | ISCSI_DATAIN_STATUS));
CU_ASSERT(datah->res_cnt == residual_count);
TAILQ_REMOVE(&g_write_pdu_list, pdu, tailq);
spdk_put_pdu(pdu);
CU_ASSERT(TAILQ_EMPTY(&g_write_pdu_list));
}
static void
underflow_for_zero_read_transfer_test(void)
{
struct spdk_iscsi_sess sess;
struct spdk_iscsi_conn conn;
struct spdk_iscsi_task task;
struct spdk_iscsi_pdu *pdu;
struct iscsi_bhs_scsi_req *scsi_req;
struct iscsi_bhs_scsi_resp *resph;
uint32_t residual_count = 0, data_segment_len;
TAILQ_INIT(&g_write_pdu_list);
memset(&sess, 0, sizeof(sess));
memset(&conn, 0, sizeof(conn));
memset(&task, 0, sizeof(task));
sess.MaxBurstLength = SPDK_ISCSI_MAX_BURST_LENGTH;
conn.sess = &sess;
conn.MaxRecvDataSegmentLength = 8192;
pdu = spdk_get_pdu();
SPDK_CU_ASSERT_FATAL(pdu != NULL);
scsi_req = (struct iscsi_bhs_scsi_req *)&pdu->bhs;
scsi_req->read_bit = 1;
spdk_iscsi_task_set_pdu(&task, pdu);
task.parent = NULL;
task.scsi.length = 512;
task.scsi.transfer_len = 512;
task.bytes_completed = 512;
task.scsi.data_transferred = 0;
task.scsi.status = SPDK_SCSI_STATUS_GOOD;
spdk_iscsi_task_response(&conn, &task);
spdk_put_pdu(pdu);
/*
* In this case, only a SCSI Response PDU is expected and
* underflow must be set in it.
* */
to_be32(&residual_count, 512);
pdu = TAILQ_FIRST(&g_write_pdu_list);
SPDK_CU_ASSERT_FATAL(pdu != NULL);
CU_ASSERT(pdu->bhs.opcode == ISCSI_OP_SCSI_RSP);
resph = (struct iscsi_bhs_scsi_resp *)&pdu->bhs;
CU_ASSERT(resph->flags == (ISCSI_SCSI_UNDERFLOW | 0x80));
data_segment_len = DGET24(resph->data_segment_len);
CU_ASSERT(data_segment_len == 0);
CU_ASSERT(resph->res_cnt == residual_count);
TAILQ_REMOVE(&g_write_pdu_list, pdu, tailq);
spdk_put_pdu(pdu);
CU_ASSERT(TAILQ_EMPTY(&g_write_pdu_list));
}
int
main(int argc, char **argv)
{
@ -257,6 +386,10 @@ main(int argc, char **argv)
if (
CU_add_test(suite, "login check target test", op_login_check_target_test) == NULL
|| CU_add_test(suite, "maxburstlength test", maxburstlength_test) == NULL
|| CU_add_test(suite, "underflow for read transfer test",
underflow_for_read_transfer_test) == NULL
|| CU_add_test(suite, "underflow for zero read transfer test",
underflow_for_zero_read_transfer_test) == NULL
) {
CU_cleanup_registry();
return CU_get_error();