linux-scsi.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Douglas Gilbert <dgilbert@interlog.com>
To: Bart Van Assche <Bart.VanAssche@wdc.com>,
	"linux-scsi@vger.kernel.org" <linux-scsi@vger.kernel.org>
Cc: "hare@suse.de" <hare@suse.de>,
	"martin.petersen@oracle.com" <martin.petersen@oracle.com>
Subject: Re: [PATCH v2 5/5] scsi_debug: add resp_write_scat function
Date: Thu, 14 Dec 2017 21:44:53 -0500	[thread overview]
Message-ID: <b52cfa8c-6fc8-dae2-bdbc-b2d55e6dd9d0@interlog.com> (raw)
In-Reply-To: <1513292262.2475.100.camel@sandisk.com>

[-- Attachment #1: Type: text/plain, Size: 2585 bytes --]

On 2017-12-14 05:57 PM, Bart Van Assche wrote:
> On Wed, 2017-12-13 at 20:40 -0500, Douglas Gilbert wrote:
>> +	{0, 0x7f, 0x11, F_SA_HIGH | F_D_OUT | FF_MEDIA_IO, resp_write_scat,
>> +	    NULL, {32,  0xc7, 0, 0, 0, 0, 0x3f, 0x18, 0x0, 0x11, 0xf8,
>> +		   0, 0xff, 0xff, 0x0, 0x0} },	/* WRITE SCATTERED(32) */
> 
> I know this is consistent with other command declarations in the scsi_debug
> driver, but it seems weird to me to report to the initiator that only some
> bits of the ADDITIONAL CDB LENGTH fields are supported (0x18)? From SPC-5:
> 
> The CDB USAGE DATA field contains information about the CDB for the command
> being queried. The first byte of the CDB USAGE DATA field shall contain the
> operation code for the command being queried. If the command being queried
> contains a service action, then that service action code shall be placed in
> the CDB USAGE DATA field in the same location as the SERVICE ACTION field of
> the command CDB. All other bytes of the CDB USAGE DATA field shall contain a
> usage map for bits in the CDB for the command being queried.

Given the opcode and service action for WRITE SCATTERED(32), if the
ADDITIONAL CDB LENGTH is other than 0x18, then the command is in error.
The mask with the best coverage on that is 0x18. It will catch any
bits set that are clear in the corresponding mask position but not catch
bits that are clear where the mask is set. So 0x19 will get caught as
invalid but 0x10 won't.

Attached is some output from a Seagate SAS SSD using the sg_opcodes
utility. Note that the 5 variable length cdb_s are all 32 bytes long
and have a mask of 0x18 for the ADDITIONAL CDB LENGTH.

>> +	lrdp = kzalloc(lbdof_blen, GFP_ATOMIC);
>> +	if (lrdp == NULL) {
>> +		mk_sense_buffer(scp, ILLEGAL_REQUEST, INSUFF_RES_ASC,
>> +				INSUFF_RES_ASCQ);
>> +		return illegal_condition_result;
>> +	}
> 
> It seems weird to me to report a fatal error to the initiator if a memory
> allocation fails? I think other SCSI LLDs return SCSI_MLQUEUE_HOST_BUSY if a
> memory allocation fails.

The scsi_debug driver plays the role of SCSI initiator and target
connected via a virtual transport. If the underlying OS reports a
resource error than I believe it is the implementer's choice whether
to treat it as a initiator error or a target (logical unit) error.
I chose the latter because the failure is in code that is roughly
equivalent to the device server (inside a LU inside a target).

The scsi_debug driver reports SCSI_MLQUEUE_HOST_BUSY in other contexts,
both real out-of-resources situations and via error injection.

Doug Gilbert

[-- Attachment #2: opcodes_m.txt --]
[-- Type: text/plain, Size: 10159 bytes --]

  SEAGATE   ST200FM0073       CB02
  Peripheral device type: disk

Opcode  Service    CDB    Name
(hex)   action(h)  size       
-----------------------------------------------
 00                  6    Test Unit Ready
        cdb usage: 00 00 00 00 00 00 
 01                  6    Rezero Unit
        cdb usage: 01 00 01 00 ff 00 
 03                  6    Request Sense
        cdb usage: 03 e1 00 00 ff 00 
 04                  6    Format Unit
        cdb usage: 04 ff 00 ff ff 00 
 07                  6    Reassign Blocks
        cdb usage: 07 03 00 00 00 00 
 08                  6    Read(6)
        cdb usage: 08 1f ff ff ff 00 
 0a                  6    Write(6)
        cdb usage: 0a 1f ff ff ff 00 
 0b                  6    Seek(6)
        cdb usage: 0b 1f ff ff 00 00 
 12                  6    Inquiry
        cdb usage: 12 e1 ff ff ff 00 
 15                  6    Mode select(6)
        cdb usage: 15 11 00 00 ff 00 
 16                  6    Reserve(6)
        cdb usage: 16 1e ff ff ff 00 
 17                  6    Release(6)
        cdb usage: 17 1e ff 00 00 00 
 1a                  6    Mode sense(6)
        cdb usage: 1a 08 ff ff ff 00 
 1b                  6    Start stop unit
        cdb usage: 1b 01 00 0f f5 00 
 1c                  6    Receive diagnostic results
        cdb usage: 1c 01 ff ff ff 00 
 1d                  6    Send diagnostic
        cdb usage: 1d f7 00 ff ff 00 
 25                 10    Read capacity(10)
        cdb usage: 25 00 7f ff ff ff 00 00 01 00 
 28                 10    Read(10)
        cdb usage: 28 fa ff ff ff ff 00 ff ff 00 
 2a                 10    Write(10)
        cdb usage: 2a fa ff ff ff ff 00 ff ff 00 
 2b                 10    Seek(10)
        cdb usage: 2b 00 ff ff ff ff 00 00 00 00 
 2e                 10    Write and verify(10)
        cdb usage: 2e f2 ff ff ff ff 00 ff ff 00 
 2f                 10    Verify(10)
        cdb usage: 2f f2 ff ff ff ff 00 ff ff 00 
 35                 10    Synchronize cache(10)
        cdb usage: 35 06 ff ff ff ff 00 ff ff 00 
 37                 10    Read defect data(10)
        cdb usage: 37 00 1f 00 00 00 00 ff ff 00 
 3b        0        10    Write buffer, combined header and data [or multiple modes]
        cdb usage: 3b 00 ff ff ff ff ff ff ff 00 
 3b        2        10    Write buffer, data
        cdb usage: 3b 02 ff ff ff ff ff ff ff 00 
 3b        4        10    Write buffer, download microcode and activate
        cdb usage: 3b 04 ff ff ff ff ff ff ff 00 
 3b        5        10    Write buffer, download microcode, save, and activate
        cdb usage: 3b 05 ff ff ff ff ff ff ff 00 
 3b        6        10    Write buffer, download microcode with offsets and activate
        cdb usage: 3b 06 ff ff ff ff ff ff ff 00 
 3b        7        10    Write buffer, download microcode with offsets, save, and activate
        cdb usage: 3b 07 ff ff ff ff ff ff ff 00 
 3b        a        10    Write buffer, write data to echo buffer
        cdb usage: 3b 0a ff ff ff ff ff ff ff 00 
 3b        d        10    Write buffer, download microcode with offsets, select activation events, save and defer activate
        cdb usage: 3b 0d ff ff ff ff ff ff ff 00 
 3b        e        10    Write buffer, download microcode with offsets, save and defer activate
        cdb usage: 3b 0e ff ff ff ff ff ff ff 00 
 3b        f        10    Write buffer, activate deferred microcode
        cdb usage: 3b 0f ff ff ff ff ff ff ff 00 
 3b       1a        10    Write buffer, enable expander comms protocol and echo buffer
        cdb usage: 3b 1a ff ff ff ff ff ff ff 00 
 3b       1c        10    Write buffer, download application client error history
        cdb usage: 3b 1c ff ff ff ff ff ff ff 00 
 3c        0        10    Read buffer, combined header and data [or multiple modes]
        cdb usage: 3c 00 ff ff ff ff ff ff ff 00 
 3c        2        10    Read buffer, data
        cdb usage: 3c 02 ff ff ff ff ff ff ff 00 
 3c        3        10    Read buffer, descriptor
        cdb usage: 3c 03 ff ff ff ff ff ff ff 00 
 3c        a        10    Read buffer, read data from echo buffer
        cdb usage: 3c 0a ff ff ff ff ff ff ff 00 
 3c        b        10    Read buffer, echo buffer descriptor
        cdb usage: 3c 0b ff ff ff ff ff ff ff 00 
 3c       1c        10    Read buffer, error history
        cdb usage: 3c 1c ff ff ff ff ff ff ff 00 
 3e                 10    Read long(10)
        cdb usage: 3e 06 ff ff ff ff 00 ff ff 00 
 3f                 10    Write long(10)
        cdb usage: 3f e0 ff ff ff ff 00 ff ff 00 
 41                 10    Write same(10)
        cdb usage: 41 e2 ff ff ff ff 00 ff ff 00 
 42                 10    Unmap
        cdb usage: 42 00 00 00 00 00 1f ff ff 00 
 48        2        10    Sanitize, block erase
        cdb usage: 48 02 00 00 00 00 00 ff ff 00 
 48        3        10    Sanitize, cryptographic erase
        cdb usage: 48 83 00 00 00 00 00 ff ff 00 
 48       1f        10    Sanitize, exit failure mode
        cdb usage: 48 9f 00 00 00 00 00 ff ff 00 
 4c                 10    Log select
        cdb usage: 4c 03 c0 00 00 00 00 ff ff 00 
 4d                 10    Log sense
        cdb usage: 4d 03 ff 00 00 ff ff ff ff 00 
 55                 10    Mode select(10)
        cdb usage: 55 11 00 00 00 00 00 ff ff 00 
 56                 10    Reserve(10)
        cdb usage: 56 12 ff 00 00 00 00 ff ff 00 
 57                 10    Release(10)
        cdb usage: 57 12 00 00 00 00 00 ff ff 00 
 5a                 10    Mode sense(10)
        cdb usage: 5a 18 ff ff 00 00 00 ff ff 00 
 5e        0        10    Persistent reserve in, read keys
        cdb usage: 5e 00 00 00 00 00 00 ff ff 00 
 5e        1        10    Persistent reserve in, read reservation
        cdb usage: 5e 01 00 00 00 00 00 ff ff 00 
 5e        2        10    Persistent reserve in, report capabilities
        cdb usage: 5e 02 00 00 00 00 00 ff ff 00 
 5e        3        10    Persistent reserve in, read full status
        cdb usage: 5e 03 00 00 00 00 00 ff ff 00 
 5f        0        10    Persistent reserve out, register
        cdb usage: 5f 00 ff 00 00 00 00 ff ff 00 
 5f        1        10    Persistent reserve out, reserve
        cdb usage: 5f 01 ff 00 00 00 00 ff ff 00 
 5f        2        10    Persistent reserve out, release
        cdb usage: 5f 02 ff 00 00 00 00 ff ff 00 
 5f        3        10    Persistent reserve out, clear
        cdb usage: 5f 03 ff 00 00 00 00 ff ff 00 
 5f        4        10    Persistent reserve out, preempt
        cdb usage: 5f 04 ff 00 00 00 00 ff ff 00 
 5f        5        10    Persistent reserve out, preempt and abort
        cdb usage: 5f 05 ff 00 00 00 00 ff ff 00 
 5f        6        10    Persistent reserve out, register and ignore existing key
        cdb usage: 5f 06 ff 00 00 00 00 ff ff 00 
 5f        7        10    Persistent reserve out, register and move
        cdb usage: 5f 07 ff 00 00 00 00 ff ff 00 
 7f        9        32    Read(32)
        cdb usage: 7f 09 00 00 00 00 1f 18 00 ff fa 00 ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff 
 7f        a        32    Verify(32)
        cdb usage: 7f 0a 00 00 00 00 1f 18 00 ff f2 00 ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff 
 7f        b        32    Write(32)
        cdb usage: 7f 0b 00 00 00 00 1f 18 00 ff fa 00 ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff 
 7f        c        32    Write an verify(32)
        cdb usage: 7f 0c 00 00 00 00 1f 18 00 ff f2 00 ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff 
 7f        d        32    Write same(32)
        cdb usage: 7f 0d 00 00 00 00 1f 18 00 ff fe 00 ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff 
 88                 16    Read(16)
        cdb usage: 88 fa ff ff ff ff ff ff ff ff 00 00 ff ff 00 00 
 8a                 16    Write(16)
        cdb usage: 8a fa ff ff ff ff ff ff ff ff 00 00 ff ff 00 00 
 8e                 16    Write and verify(16)
        cdb usage: 8e f2 ff ff ff ff ff ff ff ff 00 00 ff ff 00 00 
 8f                 16    Verify(16)
        cdb usage: 8f f2 ff ff ff ff ff ff ff ff 00 00 ff ff 00 00 
 91                 16    Synchronize cache(16)
        cdb usage: 91 06 ff ff ff ff ff ff ff ff 00 00 ff ff 00 00 
 93                 16    Write same(16)
        cdb usage: 93 ea ff ff ff ff ff ff ff ff ff ff ff ff 00 00 
 9e       10        16    Read capacity(16)
        cdb usage: 9e 10 ff ff ff ff ff ff ff ff 00 00 ff ff 01 00 
 9e       11        16    Read long(16)
        cdb usage: 9e 11 ff ff ff ff ff ff ff ff 00 00 ff ff 03 00 
 9f       11        16    Write long(16)
        cdb usage: 9f f1 ff ff ff ff ff ff ff ff 00 00 ff ff 00 00 
 a0                 12    Report luns
        cdb usage: a0 e0 00 00 00 00 ff ff ff ff 00 00 
 a2                 12    Security protocol in
        cdb usage: a2 ff ff ff ff ff ff ff ff ff 00 00 
 a3        5        12    Report identifying information
        cdb usage: a3 05 00 00 00 00 ff ff ff ff 00 00 
 a3        c        12    Report supported operation codes
        cdb usage: a3 0c 07 ff ff ff ff ff ff ff 00 00 
 a3        d        12    Report supported task management functions
        cdb usage: a3 0d 07 ff ff ff ff ff ff ff 00 00 
 a4        6        12    Set identifying information
        cdb usage: a4 06 00 00 00 00 ff ff ff ff 00 00 
 a4        f        12    Set timestamp
        cdb usage: a4 0f 00 00 00 00 ff ff ff ff 00 00 
 b5                 12    Security protocol out
        cdb usage: b5 ff ff ff ff ff ff ff ff ff 00 00 
 b7                 12    Read defect data(12)
        cdb usage: b7 1f 00 00 00 00 ff ff ff ff 00 00 
 e0                 10    Vendor specific [0xe0]
        cdb usage: e0 ff ff ff ff 00 ff ff 00 00 
 e1                 10    Vendor specific [0xe1]
        cdb usage: e1 ff ff ff ff 03 ff ff 00 00 
 e2                 10    Vendor specific [0xe2]
        cdb usage: e2 ff ff ff ff 00 ff ff 00 00 
 e6                 10    Vendor specific [0xe6]
        cdb usage: e6 ff 1f ff ff ff ff ff ff 00 
 f7                 10    Vendor specific [0xf7]
        cdb usage: f7 fe ff ff ff ff fe ff ff 00 

      reply	other threads:[~2017-12-15  2:44 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-12-14  1:40 [PATCH v2 0/5] scsi_debug: add write scattered support Douglas Gilbert
2017-12-14  1:40 ` [PATCH v2 1/5] scsi_debug: tab, kstrto changes Douglas Gilbert
2017-12-14 22:15   ` Bart Van Assche
2017-12-14  1:40 ` [PATCH v2 2/5] scsi_debug: fix group_number mask Douglas Gilbert
2017-12-14 22:26   ` Bart Van Assche
2017-12-14  1:40 ` [PATCH v2 3/5] scsi_debug: do_device_access, add sg offset argument Douglas Gilbert
2017-12-14 22:27   ` Bart Van Assche
2017-12-14  1:40 ` [PATCH v2 4/5] scsi_debug: ARRAY_SIZE() and FF_MEDIA_IO Douglas Gilbert
2017-12-14 22:30   ` Bart Van Assche
2017-12-14  1:40 ` [PATCH v2 5/5] scsi_debug: add resp_write_scat function Douglas Gilbert
2017-12-14 22:57   ` Bart Van Assche
2017-12-15  2:44     ` Douglas Gilbert [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=b52cfa8c-6fc8-dae2-bdbc-b2d55e6dd9d0@interlog.com \
    --to=dgilbert@interlog.com \
    --cc=Bart.VanAssche@wdc.com \
    --cc=hare@suse.de \
    --cc=linux-scsi@vger.kernel.org \
    --cc=martin.petersen@oracle.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).