From 34a632a04ed67872c680d456640003bbeb51cdef Mon Sep 17 00:00:00 2001 From: David Disseldorp Date: Mon, 19 Oct 2020 14:23:12 +0200 Subject: [PATCH 1/5] lib: drop generic sense data descriptor VALID check According to SPC-5 (r17), the sense data descriptor format follows: byte field ---- ----- 0: DESCRIPTOR TYPE 1: ADDITIONAL LENGTH 2-n: Sense data descriptor specific The VALID bit is a sense data descriptor specific flag, and is not present in the only sense data descriptor supported by libiscsi - Sense key specific sense data descriptors. Drop the generic VALID bit check, in preparation for handling it on a sense data descriptor specific basis. Suggested-by: Bart Van Assche Signed-off-by: David Disseldorp --- lib/iscsi-command.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/lib/iscsi-command.c b/lib/iscsi-command.c index ef6337e..ba4bc44 100644 --- a/lib/iscsi-command.c +++ b/lib/iscsi-command.c @@ -328,8 +328,6 @@ static void parse_sense_descriptors(struct scsi_sense *sense, const uint8_t *sb, for (p = sb; p < end; p += p[1]) { if (p[1] < 4) /* length */ break; - if (!(p[2] & 0x80)) /* VALID bit */ - break; switch (p[0]) { case 2: /* Sense key specific sense data descriptor */ From 02b9b01fa118dbca3d0053ed5a8fba785d4f2974 Mon Sep 17 00:00:00 2001 From: David Disseldorp Date: Mon, 19 Oct 2020 14:37:13 +0200 Subject: [PATCH 2/5] lib: check length for sense key specific sense data descriptors Explicitly check that the sense data descriptor ADDITIONAL LENGTH field matches the expected value for sense key specific sense data descriptors. Signed-off-by: David Disseldorp --- lib/iscsi-command.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/lib/iscsi-command.c b/lib/iscsi-command.c index ba4bc44..a45b8dd 100644 --- a/lib/iscsi-command.c +++ b/lib/iscsi-command.c @@ -326,12 +326,14 @@ static void parse_sense_descriptors(struct scsi_sense *sense, const uint8_t *sb, const unsigned char *p, *const end = sb + sb_len; for (p = sb; p < end; p += p[1]) { - if (p[1] < 4) /* length */ + uint8_t addl_len = p[1]; + if (addl_len < 4) break; switch (p[0]) { case 2: /* Sense key specific sense data descriptor */ - parse_sense_spec(sense, p + 4); + if (addl_len == 0x06) + parse_sense_spec(sense, p + 4); break; } } From 6eb4b7b6e592d358dd613b9da887b9e823895a57 Mon Sep 17 00:00:00 2001 From: David Disseldorp Date: Wed, 14 Oct 2020 20:43:11 +0200 Subject: [PATCH 3/5] lib: parse Information sense descriptor type The Information descriptor type is defined in SPC-5 (r17 4.4.2.2) and may be used to provide the data offset on COMPARE_AND_WRITE miscompare. Signed-off-by: David Disseldorp --- include/scsi-lowlevel.h | 2 ++ lib/iscsi-command.c | 7 +++++++ 2 files changed, 9 insertions(+) diff --git a/include/scsi-lowlevel.h b/include/scsi-lowlevel.h index a70dcf7..2561b01 100644 --- a/include/scsi-lowlevel.h +++ b/include/scsi-lowlevel.h @@ -281,8 +281,10 @@ struct scsi_sense { unsigned sense_specific:1; unsigned ill_param_in_cdb:1; unsigned bit_pointer_valid:1; + unsigned info_valid:1; unsigned char bit_pointer; uint16_t field_pointer; + uint64_t information; }; struct scsi_data { diff --git a/lib/iscsi-command.c b/lib/iscsi-command.c index a45b8dd..582517b 100644 --- a/lib/iscsi-command.c +++ b/lib/iscsi-command.c @@ -330,6 +330,13 @@ static void parse_sense_descriptors(struct scsi_sense *sense, const uint8_t *sb, if (addl_len < 4) break; switch (p[0]) { + case 0: + /* Information descriptor with VALID flag */ + if (addl_len == 0x0a && p[2] & 0x80) { + sense->info_valid = 1; + sense->information = scsi_get_uint64(p + 4); + } + break; case 2: /* Sense key specific sense data descriptor */ if (addl_len == 0x06) From 53c58f8aea983baf2f6b9b1155109221c689681b Mon Sep 17 00:00:00 2001 From: David Disseldorp Date: Wed, 14 Oct 2020 20:43:14 +0200 Subject: [PATCH 4/5] lib: parse Information fixed sense field This field is documented in SPC-5 (r17 4.4.3). Unlike the descriptor type, the fixed Information field is four bytes wide. Signed-off-by: David Disseldorp --- lib/iscsi-command.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/lib/iscsi-command.c b/lib/iscsi-command.c index 582517b..3eb8e8f 100644 --- a/lib/iscsi-command.c +++ b/lib/iscsi-command.c @@ -354,6 +354,10 @@ void scsi_parse_sense_data(struct scsi_sense *sense, const uint8_t *sb) case 0x71: /* Fixed format */ sense->key = sb[2] & 0x0f; + if (sb[0] & 0x80) { /* VALID */ + sense->info_valid = 1; + sense->information = scsi_get_uint32(sb + 3); + } sense->ascq = scsi_get_uint16(&sb[12]); parse_sense_spec(sense, sb + 15); break; From 957cc9623223dcbf9dcf7ee27455dabfb7bd29eb Mon Sep 17 00:00:00 2001 From: David Disseldorp Date: Thu, 15 Oct 2020 01:03:10 +0200 Subject: [PATCH 5/5] test-tool: add COMPARE AND WRITE MiscompareSense test This test checks regular sense/ascq values on miscompare as well as the Information sense field, which should be set as follows: sense key set to MISCOMPARE and the additional sense code set to MISCOMPARE DURING VERIFY OPERATION. In the sense data (see 4.18 and SPC-5) the offset from the start of the Data-Out Buffer to the first byte of data that was not equal shall be reported in the INFORMATION field. Signed-off-by: David Disseldorp --- test-tool/iscsi-test-cu.c | 1 + test-tool/iscsi-test-cu.h | 1 + test-tool/test_compareandwrite_miscompare.c | 76 +++++++++++++++++++++ 3 files changed, 78 insertions(+) diff --git a/test-tool/iscsi-test-cu.c b/test-tool/iscsi-test-cu.c index 76a41d5..b0c4fc1 100644 --- a/test-tool/iscsi-test-cu.c +++ b/test-tool/iscsi-test-cu.c @@ -68,6 +68,7 @@ static CU_TestInfo tests_compareandwrite[] = { { "Simple", test_compareandwrite_simple }, { "DpoFua", test_compareandwrite_dpofua }, { "Miscompare", test_compareandwrite_miscompare }, + { "MiscompareSense", test_compareandwrite_miscompare_sense }, { "Unwritten", test_compareandwrite_unwritten }, { "InvalidDataOutSize", test_compareandwrite_invalid_dataout_size }, diff --git a/test-tool/iscsi-test-cu.h b/test-tool/iscsi-test-cu.h index af5cced..49b7819 100644 --- a/test-tool/iscsi-test-cu.h +++ b/test-tool/iscsi-test-cu.h @@ -54,6 +54,7 @@ void test_teardown(void); void test_compareandwrite_simple(void); void test_compareandwrite_dpofua(void); void test_compareandwrite_miscompare(void); +void test_compareandwrite_miscompare_sense(void); void test_compareandwrite_unwritten(void); void test_compareandwrite_invalid_dataout_size(void); diff --git a/test-tool/test_compareandwrite_miscompare.c b/test-tool/test_compareandwrite_miscompare.c index 3fb6374..04a9fb5 100644 --- a/test-tool/test_compareandwrite_miscompare.c +++ b/test-tool/test_compareandwrite_miscompare.c @@ -158,3 +158,79 @@ test_compareandwrite_miscompare(void) } } } + +void +test_compareandwrite_miscompare_sense(void) +{ + unsigned i; + + CHECK_FOR_DATALOSS; + CHECK_FOR_SBC; + CHECK_FOR_ISCSI(sd); + + if (inq_bl->max_cmp < 1) { + logging(LOG_NORMAL, "[SKIPPED] COMPAREANDWRITE " + "max_cmp less than 1."); + CU_PASS("[SKIPPED] single block COMPAREANDWRITE not supported " + "Skipping test"); + return; + } + + logging(LOG_VERBOSE, LOG_BLANK_LINE); + logging(LOG_VERBOSE, "Test COMPARE_AND_WRITE of 1 block at the " + "start of the LUN"); + + logging(LOG_VERBOSE, "Write 1 block of 'A' at LBA:0"); + memset(scratch, 'A', 2 * block_size); + + WRITE16(sd, 0, block_size, + block_size, 0, 0, 0, 0, 0, scratch, + EXPECT_STATUS_GOOD); + + memset(scratch + block_size, 'B', block_size); + + logging(LOG_VERBOSE, "Overwrite blocks with 'B' " + "at LBA:0 (if they all contain 'A')"); + COMPAREANDWRITE(sd, 0, + scratch, 2 * block_size, block_size, + 0, 0, 0, 0, + EXPECT_STATUS_GOOD); + /* we've confirmed that c&w is supported, time for the proper test... */ + + logging(LOG_VERBOSE, "Vary location of miscompare in %zd bytes and check" + "sense", block_size); + memset(scratch + block_size, 'C', block_size); + + for (i = 0; i < block_size; i++) { + struct scsi_task *tsk; + struct scsi_iovec iov; + + logging(LOG_VERBOSE, "Fill buffer with 'B' except for %d " + "offset", i); + memset(scratch, 'B', block_size); + scratch[i] = 'Z'; + + tsk = scsi_cdb_compareandwrite(0, 2 * block_size, block_size, + 0, 0, 0, 0, 0); + CU_ASSERT(tsk != NULL); + + iov.iov_base = scratch; + iov.iov_len = 2 * block_size; + scsi_task_set_iov_out(tsk, &iov, 1); + + tsk = iscsi_scsi_command_sync(sd->iscsi_ctx, sd->iscsi_lun, + tsk, NULL); + CU_ASSERT_FATAL(tsk != NULL); + CU_ASSERT(tsk->status == SCSI_STATUS_CHECK_CONDITION); + CU_ASSERT(tsk->sense.key == SCSI_SENSE_MISCOMPARE); + CU_ASSERT(tsk->sense.ascq + == SCSI_SENSE_ASCQ_MISCOMPARE_DURING_VERIFY); + if (tsk->sense.info_valid) { + logging(LOG_VERBOSE, "Check Information field provided" + " with miscompare sense response"); + CU_ASSERT_EQUAL(tsk->sense.information, i); + } + + scsi_free_scsi_task(tsk); + } +}