Subject: [Gen-art] Gen-ART LC Review of draft-ietf-nfsv4-pnfs-block-09

From: Ben Campbell <[email-masked]>
Date: Tue, 4 Nov 2008 16:51:07 -0600

I have been selected as the General Area Review Team (Gen-ART)
reviewer for this draft (for background on Gen-ART, please see
http://www.alvestrand.no/ietf/gen/art/gen-art-FAQ.html).

Please resolve these comments along with any other Last Call comments
you may receive.

Document: draft-ietf-nfsv4-pnfs-block-09
Reviewer: Ben Campbell
Review Date:  2008-11-04
IETF LC End Date: 2008-11-27

Summary:

This draft is almost ready for publication as a proposed standard. I  
have a couple of questions that I think should be considered first,  
and a couple of nits.

Comments:

[Disclaimer]

While it is normal for a Gen-ART reviewer to be inexpert in the  
subject matter being reviewed, I found myself feeling much more  
inexpert than usual while reviewing this draft. This is due to the  
subject matter, not the presentation, which seems to be clear in  
general.

[substantive]

-- Section 2.3.4, second paragraph, last sentence:

> This policy SHOULD be implemented when
>    storage devices do not provide atomicity for concurrent read/write
>    and write/write operations to the same data.

I wonder why that's not a MUST. It seems like the result of neither  
providing atomic operations, or implementing this policy, would result  
in corruption, right? Isn't it safe to say that the client MUST NOT  
create corruption?

-- Section 3, paragraph 2:

> In environments where the
>    security requirements are such that client-side protection from
>    access to storage outside of the layout is not sufficient pNFS
>    block/volume storage layouts for pNFS SHOULD NOT be used, unless  
> the
>    storage device is able to implement the appropriate access checks,
>    via use of virtualized block addresses, or other means.

Maybe this is my ignorance of the subject talking, but I have to  
wonder what sort of environment might exist where it is sufficient to  
allow client-side protection only for this sort of thing. It seems  
like a malicious client could cause all sorts of mayhem.

[Nits and Editorial]

-- IDNITs reports that Page 1 is too long (59 lines)

-- Section 2.2.2, last paragraph:

> In the case of the server
>    returning NFS4ERR_TOOSMALL the client SHOULD allocate a buffer of  
> at
>    least gdir_mincount_bytes to contain the expected result and retry
>    the GETDEVICEINFO request.
>

Should this be GETDEVICELIST? I don't see a prior reference to an  
initial GETDEVICEINFO request.

Thanks!

Ben.

_______________________________________________
Gen-art mailing list
[email-masked]
https://www.ietf.org/mailman/listinfo/gen-art

---

From: [email-masked]
Date: Tue, 18 Nov 2008 17:52:28 -0500

Ben,

Thanks for doing this review.

> in corruption, right? Isn't it safe to say that the client MUST NOT  

That's actually from 2.3.5, and yes, we'll do something like that -
a MUST or MUST NOT is clearly called for here.

the

> like a malicious client could cause all sorts of mayhem.

The motivation is a physically isolated SAN where all the servers are
trusted.  FWIW, early versions of this technology worked over parallel
SCSI cables, and the block layout is designed to be used this way if
desired.

at
> initial GETDEVICEINFO request.

I believe you've found a think-o.  We'll double-check this and correct
it.

Thanks,
--David
----------------------------------------------------
David L. Black, Distinguished Engineer
EMC Corporation, 176 South St., Hopkinton, MA  01748
+1 (508) 293-7953             FAX: +1 (508) 293-7786
[email-masked]        Mobile: +1 (978) 394-7754
----------------------------------------------------

> -----Original Message-----
> From: Ben Campbell [mailto:[email-masked]] 
> Sent: Tuesday, November 04, 2008 5:51 PM
> To: General Area Review Team; Black, David; fridella, 
> stephen; [email-masked]
> Cc: Lars Eggert; [email-masked]; [email-masked]; 
> [email-masked]
> Subject: Gen-ART LC Review of draft-ietf-nfsv4-pnfs-block-09
>

---

From: Ben Campbell <[email-masked]>
Date: Tue, 18 Nov 2008 17:14:58 -0600

> a MUST or MUST NOT is clearly called for here.

Good, thanks!

> SCSI cables, and the block layout is designed to be used this way if

Ok, understood.

> I believe you've found a think-o.  We'll double-check this and correct

Great, thanks!

Thanks,

Ben.

---

From: Jason Glasgow <[email-masked]>
Date: Tue, 18 Nov 2008 23:19:36 -0500

David,

Thank you to Ben for the review.  His comment on "GETDEVICEINFO" points out
a problem in the draft.  The problem is that the preceding paragraph
references GETDEVICELIST, where it should really reference GETDEVICEINFO.

So,  in section "2.2.2 Volume Topology", the text:

   The "pnfs_block_device_addr4" data structure is returned by the
   server as the storage-protocol-specific opaque field da_addr_body in
   the "device_addr4" structure by a successful GETDEVICELIST operation.
   [NFSV4.1].

should read as:

   The "pnfs_block_device_addr4" data structure is returned by the
   server as the storage-protocol-specific opaque field da_addr_body in
   the "device_addr4" structure by a successful GETDEVICEINFO operation.
   [NFSV4.1].

-Jason

On Tue, Nov 18, 2008 at 6:14 PM, Ben Campbell <[email-masked]> wrote:

>>>>  storage devices do not provide atomicity for concurrent
>>>>
>>> read/write
>>
>>>  storage device is able to implement the appropriate access
>>>>
>>> checks,
>>
>>>  least gdir_mincount_bytes to contain the expected result and
>>>>
>>> retry
>>
>>>>  storage devices do not provide atomicity for concurrent
>>>>
>>> read/write
>>>
>>>>  block/volume storage layouts for pNFS SHOULD NOT be
>>>>
>>> used, unless
>>>
>>>> the
>>>>  storage device is able to implement the appropriate
>>>>
>>> access checks,
>>>
>>>>  returning NFS4ERR_TOOSMALL the client SHOULD allocate a
>>>>
>>> buffer of
>>>
>>>> at
>>>>  least gdir_mincount_bytes to contain the expected result
>>>>
>>> and retry
>>>

---

From: <[email-masked]>
Date: Wed, 19 Nov 2008 14:59:58 +0000

Hi,

do you want me to stick this change into the tracker as an RFC Editor Note,
or do you prefer to rev the draft?

Lars

On 08-11-18 22:19, "ext Jason Glasgow" <[email-masked]> wrote:

> David,
> 
> Thank you to Ben for the review.  His comment on "GETDEVICEINFO" points out a
> problem in the draft.  The problem is that the preceding paragraph references
> GETDEVICELIST, where it should really reference GETDEVICEINFO.
>

---

From: [email-masked]
Date: Wed, 19 Nov 2008 10:42:15 -0500

I think I want to rev the draft as Ben's first issue regarding
a SHOULD may require some careful wordsmithing - it's not
immediately clear to me that just substituting a MUST for
the SHOULD will suffice.

Thanks,
--David

> From: [email-masked] [mailto:[email-masked]] 
> Sent: Wednesday, November 19, 2008 10:00 AM
> To: [email-masked]; Black, David
> Cc: [email-masked]; [email-masked]; fridella, stephen; 
> [email-masked]; [email-masked]
> Subject: Re: Gen-ART LC Review of draft-ietf-nfsv4-pnfs-block-09
> 
> Hi,
>