Subject: Re: [nfsv4] DLB review of: draft-ietf-nfsv4-scsi-layout-02.txt

From: "Black, David" <[email-masked]>
Date: Mon, 5 Oct 2015 19:38:18 +0000

Hi Christoph,

David> A few comments inline ...

> -----Original Message-----
> From: Christoph Hellwig [mailto:[email-masked]]
> Sent: Monday, October 05, 2015 3:20 AM
> To: Black, David
> Cc: [email-masked]
> Subject: Re: [nfsv4] DLB review of: draft-ietf-nfsv4-scsi-layout-02.txt
> 
> Hi Dave,
> 
> thanks for the feedback.  I've pushed out the updated to
> http://git.infradead.org/users/hch/scsi-layout.git but I won't push out a new
> draft before processing at least some of the updates from Dave Noveck.
> 
> I'm taken almost all of your suggestions, but a few comments below:

David> That sounds great, thanks.

> > I'd expand this to explain what sorts of names the three types of
> > designators allow, capitalize must (-> MUST) and observe that this
> > requirement forbids a number of other types of SCSI names, including
> > those based on T10 vendor IDs (designator type 1).
> 
> I've also added T10 vendor IDs to the list.  I'm not sure why I excluded them,
> the intention was to not support all the relative identifiers which are useful
> for a NFS client to identify the device.

David> Did you also add some "heads-up" text to warn that more than one logical
David> unit name descriptor may come back in VPD page 83h, and all of them have
David> to be checked for a match, not just the first one?

> > I'd also add text to suggest a single key per client is a simpler
> > approach rather than key per [client, LU] pair, in part because it
> > simplifies identifying which clients have reservations when looking
> > at reservation state on the storage systems.
> 
> While I agree with this comment I didn't bother adding that text as that's
> simply an implementation choice.

David> Ok, but ... the current text struck me as favoring the larger number
David> of keys, so I wanted the alternative mentioned to avoid the possibility
David> of implementers viewing the "key per [client, LU] pair" text as implicitly
David> recommending that implementation approach.
 
> > Section 2.8 should also discuss client interaction with volatile write
> > caches.  If the storage system contains a volatile cache (e.g., V_SUP
> > is set to one in the Extended Inquiry VPD page [page 0x86]), setting
> > the FUA bit to one in write commands is a very good idea.
> 
> The idea is that the client never uses FUA or SYNCHRONIZE_CACHE, but as
> the server must do a LAYOUTCOMMIT for every write it will take care of
> sending SYNCHRONIZE_CACHE if needed.

David> Please say that explicitly, as T10 has gotten wrapped around the FUA
David> "axle" twice in the past year or so.  If the server has to figure
David> out what's going on here, the following text on the V_SUP bit in the
David> Extended INQUIRY data VPD page should be mentioned, although that bit
David> has limited utility:

A volatile cache supported (V_SUP) bit set to one indicates that the
device server supports a volatile cache and that the applicable command
standard defines features using this cache (e.g., the FUA bit in SBC-3).
An V_SUP bit set to zero indicates that the device server may or may not
support a volatile cache.

David> In particular, that bit is one-way; if set to one, a volatile cache is
David> present, but if set to zero, no information is reported ("may or may
David> not support").
David> The WCE bit in the Caching mode page is considerably more useful
David> (as it's two-way), if that mode page is supported, and should definitely
David> be mentioned.

> > Section 3 - Security considerations
> >
> >    At one extreme, pNFS with SCSI
> >    layouts can be used with storage access protocols (e.g., parallel
> >    SCSI) that provide essentially no security functionality.  At the
> >    other extreme, pNFS may be used with storage protocols such as iSCSI
> >    that can provide significant security functionality.
> >
> > References should be cited for both parallel SCSI and iSCSI.
> 
> I've added the referenceş, but replaced parallel SCSI with SAS as a more
> uptodate reference for a protocol without security functionality.

David> That's fine.

---

From: Christoph Hellwig <[email-masked]>
Date: Tue, 6 Oct 2015 11:37:01 +0200

Hi David,

> David> to be checked for a match, not just the first one?

I incorporated your suggestion directly:

+       NFS clients thus MUST check all the descriptors for a possible match
+       to "sbv_code_set", "sbv_designator_type" and "sbv_designator".

> David> recommending that implementation approach.

Ok, I'll see if I can add a little more text favouring less keys in this
section.

> David> (as it's two-way), if that mode page is supported, and should definitely

Ok, I will update section 2.8 to be more detailed and tied to SCSI.  That
text is actually something I wrote as an errate for RFC5663, which explains
the complete lack of SCSI terminology.