On Tue, 2008-05-13 at 11:52 +0300, Boaz Harrosh wrote:
> James Bottomley wrote:
> > On Wed, 2008-04-30 at 11:30 +0300, Boaz Harrosh wrote:
> >> Let through upto the largest command of 260 defined by the scsi standard.
> >> iscsi core supports this already. Now that the scsi-ml supports it we can
> >> start using large commands.
> >>
> >> Signed-off-by: Boaz Harrosh <bharrosh@xxxxxxxxxxx>
> >> Signed-off-by: Mike Christie <michaelc@xxxxxxxxxxx>
> >> ---
> >> drivers/scsi/iscsi_tcp.c | 2 +-
> >> 1 files changed, 1 insertions(+), 1 deletions(-)
> >>
> >> diff --git a/drivers/scsi/iscsi_tcp.c b/drivers/scsi/iscsi_tcp.c
> >> index 72b9b2a..826c97c 100644
> >> --- a/drivers/scsi/iscsi_tcp.c
> >> +++ b/drivers/scsi/iscsi_tcp.c
> >> @@ -1978,7 +1978,7 @@ static struct iscsi_transport iscsi_tcp_transport = {
> >> .host_template = &iscsi_sht,
> >> .conndata_size = sizeof(struct iscsi_conn),
> >> .max_conn = 1,
> >> - .max_cmd_len = 16,
> >> + .max_cmd_len = SCSI_MAX_VARLEN_CDB_SIZE,
> >> /* session management */
> >> .create_session = iscsi_tcp_session_create,
> >> .destroy_session = iscsi_tcp_session_destroy,
> >
> > OK, this isn't quite right. The escb definition in iscsi.h is:
> > struct iscsi_ecdb_ahdr {
> > __be16 ahslength; /* CDB length - 15, including reserved byte */
> > uint8_t ahstype;
> > uint8_t reserved;
> > /* 4-byte aligned extended CDB spillover */
> > uint8_t ecdb[260 - ISCSI_CDB_SIZE];
> > };
> >
> > Either that 260 needs to become SCSI_MAX_VARLEN_CDB_SIZE or we need to
> > hard code 260 in the max_cmd_len.
> >
>
> Yes that 260 needs to become SCSI_MAX_VARLEN_CDB_SIZE. The reason it
> is not is because that code is much older than the definition of
> SCSI_MAX_VARLEN_CDB_SIZE.
>
> > Since SCSI_MAX_VARLEN_CDB_SIZE is really a useless constant (nothing
> > depends on it), and internal packets in iscsi depend on this, it
> > probably makes the most sense for this to be an iscsi local constant.
> >
>
> As you said below, this is not an iscsi limitation it is a scsi
> limitation. Logically it belongs to scsi.h near the varlen definitions.
> If you prefer hard coded constants I don't mind, just that from the school
> I came from they would fail me if I did that, even for a single user.
>
> > The value (260) also looks a bit bogus, isn't 262 the maximum possible
> > size for a 0x7f variable length command? The iSCSI maxiumum is far
> > higher than this (but no protocol sends anything above the 0x7f maximum
> > currently).
> >
>
> 260 comes from the scsi standard. The 8th byte of a scsi varlen header
> is a one byte length specifier. (see struct scsi_varlen_cdb_hdr in scsi.h)
> Now the standard says that the header must be 4 bytes aligned so the
> maximum that can be written in that byte is 252, plus the constant 8.
I don't think it can be alignment issues otherwise six byte commands
like READ_6/WRITE_6 would be illegal. I don't think there are any
alignment requirements per se. However, it does look like the
definition section of SAM-3:3.1.15 does say "... or a variable length of
between 12 and 260 bytes" with no reason given, so that will do.
James
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
