Re: [nfsv4] I-D Action: draft-ietf-nfsv4-flex-files-11.txt

[David Noveck <[e-mail]>]

> Although I'll let the IETF guys debate the correct use of SHOULD, it seems
> rather strong here, given that it is "at best a hint" as described here:


> Looking at Section 12.5 of RFC 7862, IO_ADVISE, it uses MAY.

> I can’t shoehorn MAY and MIGHT with the negation, so “should” it is…

Your choices are not limited to "should" and "SHOULD".  You have the entire
English language available.  If this is a hint, it might be best to say
so.  If it desirable to
not do the IO through the MDS, you might explain why that is.

Note that RFC 8174 says that text that is not all-UPPERCASE can still be
normative, although
it is not as clear about what "should" means as RFC2119 is about what
"SHOULD" means.

BTW, although "SHOULD not" seems like a red flag, RFC8174 seems to say it
means the
same as "should not", although it leaves that meaning unclear :-(.


On Wed, Jul 19, 2017 at 4:11 AM, Thomas Haynes <[e-mail]>
wrote:

> Hi Rick,
>
> Thanks for the comments - in particular you helped me in the “SHOULD” wars.
>
> Tom
>
> On Jul 19, 2017, at 1:16 AM, Rick Macklem <[e-mail]> wrote:
>
> Here's some comments w.r.t. draft11, rick
>
> Generically, I think the only effects on the client of "tight" vs "loose"
> coupling is:
> Loose coupling (when doing I/O on storage server):
> - client uses synthetic uid/gid and anonymous stateid
> - *** when fenced off, client sees NFS4ERR_ACCESS reply to I/O
> Tight coupling (when doing I/O on storage server):
> - client uses same uid/gid as for MDS for the file and ffds_stateid
> - *** when fenced off, client sees NFS4ERR_BAD_STATEID reply to I/O
>
>
> As I tried to point out in my earlier reply, this may or may not be true.
>
> Look at https://tools.ietf.org/html/draft-ietf-nfsv4-layout-types-04 : on
> page 5:
>
>    Note that there is no requirement on how these are implemented.
>    While the File Layout Type does use the stateid to fence off the
>    client, there is no requirement that other Layout Types use this
>    stateid approach.  But the other Layout Types MUST document how the
>    client, metadata server, and storage devices interact to meet these
>    requirements.
>
>
> Since the flex file draft does not spell out how the control protocol will
> work, it is up to the
> implementation to do so.
>
> I’ve added a new paragraph to the end of section 2.2:
>
>       The client need not know the model used between the metadata
>       server and the storage device. It need only react consistently
>       to any errors in interacting with the storage device. It should
>       both return the layout and error to the metadata server and ask for
>       a new layout. At that point, the metadata server can either
>       hand out a new layout, hand out no layout (forcing the I/O through
>       it), or deny the client further access to the file.
>
>
>
> The "***" cases aren't explicitly spelled out in the draft. It might be
> nice
> to have something like this near the beginning of the document, so that
> client
> implementors aren't confused by the various discussions of "loose" vs
> "tight"
> coupling.
>
> Now, here's some things I spotted in draft-11.
>
> 2.1.  LAYOUTCOMMIT
>
>   The metadata server has the responsibility, upon receiving a
>   LAYOUTCOMMIT (see Section 18.42 of [RFC5661]), of ensuring that the
>   semantics of pNFS are respected (see Section 12.5.4 of [RFC5661]).
>   These do include a requirement that data written to data storage
>   device be stable before the occurance of the LAYOUTCOMMIT.
>
> Spelling of occurance as noted before, plus..
> I think that this needs to be changed to apply to both loose and tight
> couplings too? (line 527-530 in the .txt file)
>
>
> I’m torn between I think it is obvious from the context, yet the context of
> entire section is the difference in models.
>
> The first sentence is now:
>
> Regardless of the coupling model, the metadata server ...
>
>
>  case that there is loose coupling is in effect.  If
>  ffdv_tightly_coupled is not set, then the client MUST commit writes
>  to the storage devices for the file before sending a LAYOUTCOMMIT to
>  the metadata server.  I.e., the writes MUST be committed by the
>  client to stable storage via issuing WRITEs with stable_how ==
>
> ...
>
> 2.2.  Fencing Clients from the Storage Device
>
>  With loosely coupled storage devices, the metadata server uses
>  synthetic uids and gids for the data file, where the uid owner of the
>  data file is allowed read/write access and the gid owner is allowed
>  read only access.  As part of the layout (see ffds_user and
>  ffds_group in Section 5.1), the client is provided with the user and
>  group to be used in the Remote Procedure Call (RPC) [RFC5531]
>  credentials needed to access the data file.  Fencing off of clients
>  is achieved by the metadata server changing the synthetic uid and/or
>  gid owners of the data file on the storage device to implicitly
>  revoke the outstanding RPC credentials.
>
> It might be nice to add that the client will receive a NFS4ERR_ACCESS reply
> to I/O operations done against the storage server for the loosely coupled
> case?
>
>
> I’ve added wording as such...
>
> ...
>
>  With tightly coupled storage devices, the metadata server sets the
>  user and group owners, mode bits, and ACL of the data file to be the
>  same as the metadata file.  And the client must authenticate with the
>  storage device and go through the same authorization process it would
>  go through via the metadata server.  In the case of tight coupling,
>  fencing is the responsibility of the control protocol and is not
>  described in detail here.  However, implementations of the tight
>  coupling locking model (see Section 2.3), will need a way to prevent
>  access by certain clients to specific files by invalidating the
>  corresponding stateids on the storage device.
>
> As above, it might be nice to clarify that the client will receive a
> NFS4ERR_BAD_STATEID for the tight coupled case?
>
>
> Added such wording, but kept the new paragraph.
>
>
> ...
>
>     strongly coupled.  They would use a back-end control protocol to
>
> I think David N. mentioned this before, but I think "strongly" should be
> "tightly"?
> …
>
>
> Thanks, I must have skipped it in his review.
>
>
>  F_FLAGS_NO_IO_THRU_MDS:  can be set to indicate that the client
>     SHOULD not send I/O operations to the metadata server.  I.e., even
>     if the client could determine that there was a network diconnect
>     to a storage device, the client SHOULD not try to proxy the I/O
>     through the metadata server.
>
> Although I'll let the IETF guys debate the correct use of SHOULD, it seems
> rather strong here, given that it is "at best a hint" as described here:
>
>
> Looking at Section 12.5 of RFC 7862, IO_ADVISE, it uses MAY.
>
> I can’t shoehorn MAY and MIGHT with the negation, so “should” it is…
>
> And at the very least, “SHOULD not” should be a red flag, in that I MUST
> use “SHOULD NOT” or “should not”, but not “should NOT” or “SHOULD not”.
>
>
>
>
> 5.1.2.  Client Interactions with FF_FLAGS_NO_IO_THRU_MDS
>
>  Even if the metadata server provides the FF_FLAGS_NO_IO_THRU_MDS,
>  flag, the client can still perform I/O to the metadata server.  The
>  flag is at best a hint.  The flag is indicating to the client that
>
> ...
>
>  data strucures are built on concepts introduced in NFSv4.2, the
>
> Spelling of structures.
>
>
> Ack
>
> ...
>
> 13.  Recalling a Layout
>
>  While Section 12.5.5 of [RFC5661] discusses layout type independent
>  reasons for recalling a layout, the flexible file layout type
>  metadata server should recall outstanding layouts in the following
>  cases:
>
>  o  When the file's security policy changes, i.e., Access Control
>     Lists (ACLs) or permission mode bits are set.
>
> I don't think this applies to the "tightly coupled" case, if the server
> propagates the ACL, perm changes to the storage server?
> Same comment applies to parts of:
>
>
> Hah, I’m saved since I used “should’ and not “SHOULD” or “MUST”.
>
>
>
>
> 15.  Security Considerations
>
>  third para.
>
>  devices.  When the metadata server receives a request to change a
>  file's permissions or ACL, it SHOULD recall all layouts for that file
>
>
> Using the same logic, this should be a “should”. :-)
>
> No, I think it has to stay a “SHOULD” and we do have to call out this.
>
> I’ve replaced the second paragraph with:
>
>    The metadata server enforces the file access-control policy at
>    LAYOUTGET time.  The client should use RPC authorization credentials
>    (uid/gid for AUTH_SYS or tickets for Kerberos) for getting the layout
>    for the requested iomode (READ or RW) and the server verifies the
>    permissions and ACL for these credentials, possibly returning
>    NFS4ERR_ACCESS if the client is not allowed the requested iomode.
>    For the tightly coupled model, the LAYOUTGET does not return
>    credentials for accessing the storage devices.  Instead, the existing
>    credential will be used to enforce subsequent access to the storage
>    device.
>
>    For the loosely coupled model, if the LAYOUTGET operation succeeds
>    then the client receives, as part of the layout, a set of credentials
>    allowing it I/O access to the specified data files corresponding to
>    the requested iomode.  When the client acts on I/O operations on
>    behalf of its local users, it MUST authenticate and authorize the
>    user by issuing respective OPEN and ACCESS calls to the metadata
>    server, similar to having NFSv4 data delegations.
>
>
> (And fwiw, I had the hardest time with this point you brought up.)
>
>
>  and then MUST fence off any clients still holding outstanding layouts
>  for the respective files by implicitly invalidating the previously
>  distributed credential on all data file comprising the file in
>
>
> ________________________________________
> From: nfsv4 <[e-mail]> on behalf of Thomas Haynes <
> [e-mail]>
> Sent: Tuesday, July 18, 2017 6:06:16 AM
> To: [e-mail]
> Subject: Re: [nfsv4] I-D Action: draft-ietf-nfsv4-flex-files-11.txt
>
> This has the FF_FLAGS_WRITE_ONE_MIRROR and the fixed IANA considerations.
>
>
> On Jul 18, 2017, at 12:04 PM, [e-mail] wrote:
>
>
> A New Internet-Draft is available from the on-line Internet-Drafts
> directories.
> This draft is a work item of the Network File System Version 4 of the IETF.
>
>       Title           : Parallel NFS (pNFS) Flexible File Layout
>       Authors         : Benny Halevy
>                         Thomas Haynes
>      Filename        : draft-ietf-nfsv4-flex-files-11.txt
>      Pages           : 40
>      Date            : 2017-07-18
>
> 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 flexible file layout type is defined in this
>  document as an extension to pNFS which allows the use of storage
>  devices in a fashion such that they require only a quite limited
>  degree of interaction with the metadata server, using already
>  existing protocols.  Client side mirroring is also added to provide
>  replication of files.
>
>
> The IETF datatracker status page for this draft is:
> https://datatracker.ietf.org/doc/draft-ietf-nfsv4-flex-files/
>
> There are also htmlized versions available at:
> https://tools.ietf.org/html/draft-ietf-nfsv4-flex-files-11
> https://datatracker.ietf.org/doc/html/draft-ietf-nfsv4-flex-files-11
>
> A diff from the previous version is available at:
> https://www.ietf.org/rfcdiff?url2=draft-ietf-nfsv4-flex-files-11
>
>
> Please note that it may take a couple of minutes from the time of
> submission
> until the htmlized version and diff are available at tools.ietf.org.
>
> Internet-Drafts are also available by anonymous FTP at:
> ftp://ftp.ietf.org/internet-drafts/
>
> _______________________________________________
> nfsv4 mailing list
> [e-mail]
> https://www.ietf.org/mailman/listinfo/nfsv4
>
>
> _______________________________________________
> nfsv4 mailing list
> [e-mail]
> https://www.ietf.org/mailman/listinfo/nfsv4
>
>
>
> _______________________________________________
> nfsv4 mailing list
> [e-mail]
> https://www.ietf.org/mailman/listinfo/nfsv4
>
>