On Thu, 2012-08-16 at 09:24 -0700, Roland Dreier wrote:
> Actually I'm not sure removing cmd_spdtl is the right answer.
>
> If /dev/sda is a device on an iSCSI initiator exported by an LIO
> target, try doing:
>
> # sg_raw -r512 /dev/sda 28 0 0 0 0 0 0 0 2 0
>
> This issues a READ (10) for 2 sectors, but only sends a length of 512
> at the transfer level.
>
> The target responds by setting the residual to 512 but transmits all
> 1024 bytes, and the Linux initiator at least rejects it because it
> hits:
>
> if (tcp_task->data_offset + tcp_conn->in.datalen > total_in_length) {
> ISCSI_DBG_TCP(conn, "data_offset(%d) + data_len(%d) > "
> "total_length_in(%d)\n", tcp_task->data_offset,
> tcp_conn->in.datalen, total_in_length);
> return ISCSI_ERR_DATA_OFFSET;
> }
>
Ok, this is the 'overflow' case when the fabric ->data_length (expected
data transfer length of the initiator's buffer) is smaller than the SCSI
CDB allocation length or sectors*block_size (attempted transfer length)
the target has been asked to process.
The following patch which appears to do the right thing from the
perspective of the target for overflow, but AFAICT open-iscsi still
returns GOOD status w/ no data-in payload (at least via sg_raw) when the
iscsi-target is signaling overflow bit in iSCSI Data IN PDU. Not sure
yet why this is the case, but drivers/scsi/libiscsi.c:iscsi_data_in_rsp
code:
if (rhdr->flags & (ISCSI_FLAG_DATA_UNDERFLOW |
ISCSI_FLAG_DATA_OVERFLOW)) {
int res_count = be32_to_cpu(rhdr->residual_count);
if (res_count > 0 &&
(rhdr->flags & ISCSI_FLAG_CMD_OVERFLOW ||
res_count <= scsi_in(sc)->length))
scsi_in(sc)->resid = res_count;
else
sc->result = (DID_BAD_TARGET << 16) | rhdr->cmd_status;
}
appears to be setting resid for non bidi cases correctly, right..? (mnc
+ boaz CC'ed)
How about the following to ensure we restrict overflow to keep the
existing (smaller) cmd->data_length assignment, and only re-assign this
value for the underflow case..? (hch CC'ed)
WDYT..?
diff --git a/drivers/target/target_core_transport.c b/drivers/target/target_core_transport.c
index 0eaae23..63b3594 100644
--- a/drivers/target/target_core_transport.c
+++ b/drivers/target/target_core_transport.c
@@ -1183,15 +1183,20 @@ int target_cmd_size_check(struct se_cmd *cmd, unsigned int size)
/* Returns CHECK_CONDITION + INVALID_CDB_FIELD */
goto out_invalid_cdb_field;
}
-
+ /*
+ * For the overflow case keep the existing fabric provided
+ * ->data_length. Otherwise for the underflow case, reset
+ * ->data_length to the smaller SCSI expected data transfer
+ * length.
+ */
if (size > cmd->data_length) {
cmd->se_cmd_flags |= SCF_OVERFLOW_BIT;
cmd->residual_count = (size - cmd->data_length);
} else {
cmd->se_cmd_flags |= SCF_UNDERFLOW_BIT;
cmd->residual_count = (cmd->data_length - size);
+ cmd->data_length = size;
}
- cmd->data_length = size;
}
return 0;
--
To unsubscribe from this list: send the line "unsubscribe target-devel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
