Subject: RE: [nfsv4] Last Call: <draft-ietf-nfsv4-scsi-layout-06.txt> (Parallel NFS (pNFS) SCSI Layout) to Proposed Standard

From: "Black, David" <[email-masked]>
Date: Sun, 10 Jul 2016 18:51:40 +0000

This draft generally looks good.  Looking at this from a SCSI standards perspective,
I found a few small things.  The 512 vs. 4k block size (2.3.1) and reservation scope
(2.4.10.3) items are important - everything else is editorial, although clarity on
commonality with RFC 5663 would be of significant help to any implementer who
has exposure to that.

-- Introduction, last paragraph.

Add a sentence saying that there are no other significant differences from RFC 5663
(previous sentences indicate use of SCSI for fencing and LAYOUTCOMMIT improvements),
e.g., the volume topology (Section 2.3.2) and data structures that describe extents
(Section 2.3.3.) are common with RFC 5663.  Those two examples seem important.

-- Section 2.1, 1st paragraph
	" and the SCSI initiators used for the pNFS Metadata Server and clients MUST
	   support SCSI persistent reservations."
Add a citation of [SPC4] to support that MUST requirement.

-- Section 2.1, 2nd paragraph:

   Clients MUST be able to perform I/O to
   the block extents without affecting additional areas of storage
   (especially important for writes); therefore, extents MUST be aligned
   to 512-byte boundaries.

That assumes a 512 byte logical block size, which is generally ok for now, but 4k is coming.
At a minimum,  "extents MUST be aligned to logical block size boundaries of the logical
units, e.g., 512 bytes."  OTOH, would it be reasonable to just make 4k alignment a MUST
now, as there will be storage systems that need 4k alignment and 4k alignment generally
works better than 512-byte alignment with existing systems?

-- Section 2.3.1

   It is similar to the "Identification
   Descriptor Target Descriptor" specified in [SPC4], but limits the
   allowed values to those that uniquely identify a LU.

I suggest just deleting this sentence, as that is now called an "Identification CSCD Descriptor" - 
if this sentence is retained, the use of those descriptors in EXTENDED COPY would be
important to mention - that seems like a diversion.

   2.  The "DESIGNATOR TYPE" MUST be set to one of four values

T10 is now allowing UUIDs to also be used in the working draft of SPC-5, and those would be
appropriate here.  Nonetheless, I suggest no change until SPC-5 is completed at T10, as this
draft is (properly, IMHO) based on SPC-4.

-- Section 2.4.10.3

   To make sure all I_T nexuses are registered,
   the client SHOULD set the "All Target Ports" (ALL_TG_PT) bit when
   registering the key, or otherwise ensure the registration is
   performed for each initiator port.

It looks like initiator and target registration scopes were conflated here and
need to be separated.  Suggested text:

   To make sure all I_T nexuses are registered,
   the client SHOULD set the "All Target Ports" (ALL_TG_PT) bit when
   registering the key, or otherwise ensure the registration is
   performed for each target port, and MUST perform registration
   for each initiator port.

-- References

Current version of SAM is SAM-5, consider updating SAM-4 reference to SAM-5.

Thanks, --David

> -----Original Message-----
> From: nfsv4 [mailto:[email-masked]] On Behalf Of The IESG
> Sent: Tuesday, June 28, 2016 11:08 AM
> To: IETF-Announce
> Cc: [email-masked]; [email-masked]; [email-masked];
> [email-masked]
> Subject: [nfsv4] Last Call: <draft-ietf-nfsv4-scsi-layout-06.txt> (Parallel NFS
> (pNFS) SCSI Layout) to Proposed Standard
> 
> 
> The IESG has received a request from the Network File System Version 4 WG
> (nfsv4) to consider the following document:
> - 'Parallel NFS (pNFS) SCSI Layout'
>   <draft-ietf-nfsv4-scsi-layout-06.txt> as Proposed Standard
> 
> The IESG plans to make a decision in the next few weeks, and solicits
> final comments on this action. Please send substantive comments to the
> [email-masked] mailing lists by 2016-07-12. Exceptionally, comments may be
> sent to [email-masked] instead. In either case, please retain the
> beginning of the Subject line to allow automated sorting.
> 
> Abstract
> 
> 
>    The Parallel Network File System (pNFS) allows a separation between
>    the metadata (onto a metadata server) and data (onto a storage
>    device) for a file.  The SCSI Layout Type is defined in this document
>    as an extension to pNFS to allow the use SCSI based block storage
>    devices.
> 
> 
> 
> 
> The file can be obtained via
> https://datatracker.ietf.org/doc/draft-ietf-nfsv4-scsi-layout/
> 
> IESG discussion can be tracked via
> https://datatracker.ietf.org/doc/draft-ietf-nfsv4-scsi-layout/ballot/
> 
> 
> No IPR declarations have been submitted directly on this I-D.
> 
> 
> _______________________________________________
> nfsv4 mailing list
> [email-masked]
> https://www.ietf.org/mailman/listinfo/nfsv4

---

From: Christoph Hellwig <[email-masked]>
Date: Mon, 18 Jul 2016 06:35:12 +0200

Hi David,

thanks for the updates.  Let's talk about the detail tomorrow in Berlin,
but unless Spencer disagrees I'll prepare a new draft once the meeting
is over.

> (Section 2.3.3.) are common with RFC 5663.  Those two examples seem important.

Just scrapping the sentence might be best.

> Add a citation of [SPC4] to support that MUST requirement.

Ok.
> works better than 512-byte alignment with existing systems?

I think the right thing is to simply scrap the 512 byte example and just
require extents to be aligned to at least the logical block size.

All implementations of the block and scsi layout known to me align to 4k or
larger, but I see no fundamental reason to forbid 512 byte alignment.

> important to mention - that seems like a diversion.

Ok.

> draft is (properly, IMHO) based on SPC-4.

Yes, I would love to be able to support UUIDs, but I see no way to allow
for that until SPC-5 has been completed.

>    performed for each target port, and MUST perform registration

Thanks, this will need an update.

> Current version of SAM is SAM-5, consider updating SAM-4 reference to SAM-5.

Ok.

---

From: Spencer Shepler <[email-masked]>
Date: Mon, 18 Jul 2016 05:50:02 +0000

Updated draft is fine with me (if the question was for Spencer S. instead of Spencer D).

-----Original Message-----
From: nfsv4 [mailto:[email-masked]] On Behalf Of Christoph Hellwig
Sent: Sunday, July 17, 2016 9:35 PM
To: Black, David <[email-masked]>
Cc: [email-masked]; [email-masked]; [email-masked]; [email-masked]; [email-masked]
Subject: Re: [nfsv4] Last Call: <draft-ietf-nfsv4-scsi-layout-06.txt> (Parallel NFS (pNFS) SCSI Layout) to Proposed Standard

Hi David,

Ok.

Ok.

Ok.

_______________________________________________
nfsv4 mailing list
[email-masked]
https://na01.safelinks.protection.outlook.com/?url=https%3a%2f%2fwww.ietf.org%2fmailman%2flistinfo%2fnfsv4&data=01%7c01%7csshepler%40microsoft.com%7c507dc68413fa4645e0f808d3aec4ec9a%7c72f988bf86f141af91ab2d7cd011db47%7c1&sdata=1v23lbjmin6wj%2bPK%2fKkGaZNEAYpa3A9r50Nf4tunVrY%3d

---

From: Christoph Hellwig <[email-masked]>
Date: Mon, 18 Jul 2016 10:14:22 +0200

> Updated draft is fine with me (if the question was for Spencer S. instead of Spencer D).

The question was for you, but i guess Spencer D should also have a say
here now that you mention it.

---

From: Spencer Dawkins at IETF <[email-masked]>
Date: Mon, 18 Jul 2016 10:28:14 +0200

Hi, Christoph,

> here now that you mention it.

Spencer S. is your shepherd, so you can take your cue from him, but he's
right - Last Call has completed, so submitting an updated draft that
reflects the comments you've received is fine.

I'll watch for an updated draft to move this to IESG evaluation.

Thanks for your good work!

Spencer

---

From: "Black, David" <[email-masked]>
Date: Tue, 19 Jul 2016 08:26:38 +0000

Hi Christoph,

This generally looks good, two quick comments:
- I'd prefer that the comparison to RFC 5663 (existing pNFS Block/Volume layout) remain.
- Yes, making 4k alignment a MUST would be a fine thing to do now.

Thanks, --David

> From: Christoph Hellwig [mailto:[email-masked]]
> Sent: Monday, July 18, 2016 12:35 AM
> To: Black, David
> Cc: [email-masked]; [email-masked];
> [email-masked]; [email-masked]; [email-masked]

---

From: Christoph Hellwig <[email-masked]>
Date: Tue, 19 Jul 2016 10:37:24 +0200

> - Yes, making 4k alignment a MUST would be a fine thing to do now.

Can we converge on a MUST for the device logical block size, be that
512, 4k or any other value that might happen in the future?

---

From: "Black, David" <[email-masked]>
Date: Tue, 19 Jul 2016 10:22:00 +0000

Ok, although that results in client code having to deal with multiple logical block sizes. The transition from 512 to 4k has been sufficiently involved and lengthy that introduction of yet another logical block size in the foreseeable future seems rather unlikely. I'm ok with either approach.

Thanks, --David ++Sent from Android smartphone ...