Subject: [nfsv4] Re: Gunter Van de Velde's Discuss on draft-ietf-nfsv4-layoutwcc-05: (with DISCUSS and COMMENT)
From: Thomas Haynes <[email-masked]>
To: Gunter Van de Velde <[email-masked]>
Cc: The IESG <[email-masked]>, [email-masked], nfsv4-chairs <[email-masked]>, [email-masked]
Date: Thu, 6 Feb 2025 12:58:52 -0800
List-Id: NFSv4 Working Group <nfsv4.ietf.org>

Hi Gunter,

My day job has gotten in the way of replying to your original comments.

Thanks for the review and my replies are below:

Tom

PS: I will send out a version 6 very shortly with the changes made below.

> On Jan 6, 2025, at 4:57 AM, Gunter Van de Velde via Datatracker <[email-masked]> wrote:
> 
> Gunter Van de Velde has entered the following ballot position for
> draft-ietf-nfsv4-layoutwcc-05: Discuss
> 
> When responding, please keep the subject line intact and reply to all
> email addresses included in the To and CC lines. (Feel free to cut this
> introductory paragraph, however.)
> 
> 
> Please refer to https://www.ietf.org/about/groups/iesg/statements/handling-ballot-positions/ 
> for more information about how to handle DISCUSS and COMMENT positions.
> 
> 
> The document, along with other ballot positions, can be found here:
> https://datatracker.ietf.org/doc/draft-ietf-nfsv4-layoutwcc/
> 
> 
> 
> ----------------------------------------------------------------------
> DISCUSS:
> ----------------------------------------------------------------------
> 
> # Gunter Van de Velde, RTG AD, comments for draft-ietf-nfsv4-layoutwcc-05
> 
> # the referenced line numbers are derived from the idnits tool:
> https://author-tools.ietf.org/api/idnits?url=https://www.ietf.org/archive/id/draft-ietf-nfsv4-layoutwcc-05.txt
> 
> # Many thanks for this document. I am not very NFS aware, so my review is only
> high level and editorial. I do however have a single easy to resolve DISCUSS.
> 
> # I found the text at times not trivial to read due to strange language
> constructs. In some instances i proposed an editorial rewrite you could
> consider.
> 
> #DISCUSS
> #=======
> 
> # In the IANA section is written that there is no action required, but line 244
> and 245 seem to indicate that there is a code point allocated for lowa_type.
> Maybe i got confused and misunderstood the text, or maybe the needs to describe
> the intent more accurate for NFS unware people as myself?
> 
> 244        The lowa_type is defined to be a value from the IANA registry for
> 245        'pNFS Layout Types Registry'.
> 
> 
> ----------------------------------------------------------------------
> COMMENT:
> ----------------------------------------------------------------------
> 
> #DETAILED COMMENTS
> #=================
> 
> 10      Abstract
> 11
> 12         The Parallel Network File System (pNFS) Flexible File Layout allows
> 13         for a file's metadata (MDS) and data (DS) to be on different servers.
> 14         It does not provide a mechanism for the data server to update the
> 15         metadata server to changes to the data part of the file.  The client
> 16         has knowledge of such updates, but lacks the ability to update the
> 17         metadata server.  This document presents a refinement to RFC8435 to
> 18         allow the client to update the metadata server to changes on the data
> 19         server.
> 
> GV> What about the following alternative abstract:
> 
> "
> This document specifies extensions to the parallel Network File System (NFS)
> version 4 (pNFS) for improving write cache consistency. These extensions
> introduce mechanisms that ensure partial writes performed under a pNFS layout
> remain coherent and correctly tracked. The solution addresses concurrency and
> data integrity concerns that may arise when multiple clients write to the same
> file through separate data servers. By defining additional interactions among
> clients, metadata servers, and data servers, this specification enhances the
> reliability of NFSv4 in parallel-access environments and ensures consistency
> across diverse deployment scenarios. "

Thanks, I’ll take the rewording.

> 
> 106        See Section 1.1 of [RFC8435] for a fuller set of definitions.
> 
> GV> Not sure that using the word "fuller" is appropriate. What about:
> 
> "
> For a more comprehensive set of definitions, see Section 1.1 of [RFC 8435].
> "

I took the rewording.

> 
> 143        A layout type for pNFS enables the metadata server to tell the client
> 144        both the storage protocol and location of data to be used to
> 145        communicate with the storage devices.  The Flex Files Layout Type (in
> 146        [RFC8435]) details how NFSv3 data servers can be accessed.  The
> 147        client is only allowed to perform NFSv3 READ (see Section 3.3.6 of
> 148        [RFC1813]), WRITE (see Section 3.3.6 of [RFC1813]), and COMMIT (see
> 149        Section 3.3.21 of [RFC1813]) operations on the file handles provided
> 150        in the layout.  I.e., the client is only allowed to use NFSv3
> 151        operations which directly act on the data portion of the data file.
> 
> GV> I find the use of "to tell" odd in IETF text. What about the following
> editorial suggestion:
> 
> "
> A pNFS layout type enables the metadata server to inform the client of both the
> storage protocol and the locations of the data that the client should use when
> communicating with the storage devices. The Flex Files Layout Type, as
> specified in [RFC 8435], describes how data servers using NFSv3 can be
> accessed. The client is restricted to performing NFSv3 READ (Section 3.3.6 of
> [RFC 1813]), WRITE (Section 3.3.6 of [RFC 1813]), and COMMIT (Section 3.3.21 of
> [RFC 1813]) operations on the file handles provided in the layout. In other
> words, the client may only use NFSv3 operations that act directly on the data
> portion of the file. "

I took the rewording.

> 
> 170        For example, the metadata server would issue a NFSv3 GETATTR to the
> 171        data server.  This query is most likely triggered in response to a
> 172        NFSv4 GETATTR issued by a client to the metadata server.  Not only
> 173        are these NFSv3 GETATTRs to the data server individually expensive,
> 174        the data server can become inundated by a storm of such requests.
> 175        NFSv3 solved a similar issue by having the READ and WRITE operations
> 176        employ a post-operation attribute to report the weak cache
> 177        consistency (WCC) data (See Section 2.6 of [RFC1813]).
> 
> GV> This reads odd to me. Are you trying to suggest the following
> 
> "
> For example, the metadata server might issue an NFSv3 GETATTR operation to the
> data server, which is typically triggered by a client’s NFSv4 GETATTR request
> to the metadata server. In addition to the cost of each individual GETATTR
> operation, the data server can be overwhelmed by a large volume of such
> requests. NFSv3 addressed a similar challenge by including a post-operation
> attribute in the READ and WRITE operations to report weak cache consistency
> (WCC) data (see Section 2.6 of [RFC 1813]). "
> 

I tend to not use commas so much…

I took the rewording.

> 179        Each NFSv3 operation corresponds to one round trip between the client
> 180        and server.  So a WRITE followed by a GETATTR would require two round
> 181        trips.  In that scenario, the attribute information retrieved is
> 182        considered to be strict server-client consistency.  For NFSv4, the
> 183        WRITE and GETATTR can be issued together inside a compound, which
> 184        only requires one round trip between the client and server.  And this
> 185        is also considered to be a strict server-client consistency.  In
> 186        essence, the NFSv4 READ and WRITE operations drop the post-operation
> 187        attributes, allowing the client to decide if it needs that
> 188        information.
> 
> GV> Proposal text:
> 
> "
> Each NFSv3 operation entails a single round trip between the client and server.
> Consequently, issuing a WRITE followed by a GETATTR would require two round
> trips. In that situation, the retrieved attribute information is regarded as
> strict server-client consistency. By contrast, NFSv4 enables a WRITE and
> GETATTR to be combined within a compound operation, which requires only one
> round trip. This combined approach is likewise considered strict server-client
> consistency. Essentially, NFSv4 READ and WRITE operations omit post-operation
> attributes, allowing the client to determine whether it requires that
> information. "
> 

I took the changes...

> 205        In this document, we present a new NFSv4.2 operation called
> 206        LAYOUT_WCC, which allows the client to update the metadata server
> 207        with information from the data server.  The client is responsible for
> 208        taking the NFSv3 WCC information (which is returned by the 3
> 209        operations it is allowed to use) and pass that back to the metadata
> 210        server in the NFSv4.2 attributes.  The metadata server MAY then avoid
> 211        costly NFSv3 GETATTR calls to the data servers.  As this is a weak
> 212        model, the metadata server MAY make such calls anyway in order to
> 213        strengthen the model.
> 
> GV> proposal text:
> 
> "
> This document introduces a new NFSv4.2 operation, LAYOUT_WCC, which enables the
> client to provide the metadata server with information obtained from the data
> server. The client is responsible for gathering the NFSv3 WCC data, returned by
> the three permissible NFSv3 operations, and conveying it back to the metadata
> server as part of NFSv4.2 attributes. The metadata server MAY therefore avoid
> issuing costly NFSv3 GETATTR calls to the data servers. Because this approach
> relies on a weak model, the metadata server MAY still perform these calls if it
> chooses to strengthen the model. "

I took the changes….

> 
> 239     3.3.  DESCRIPTION
> 240
> 241        The current filehandle and the lowa_stateid identifies the particular
> 242        layout for the LAYOUT_WCC operation.  The lowa_type indicates how to
> 243        unpack the layout type specific payload inside the lowa_body field.
> 244        The lowa_type is defined to be a value from the IANA registry for
> 245        'pNFS Layout Types Registry'.
> 246
> 247        The lowa_body will contain the data file attributes.  The client will
> 248        be responsible for mapping the NFSv3 post-operation attributes to
> 249        those in a fattr4.  Just as the post-operation attributes may be
> 250        ignored by the client, the server may ignore the attributes inside
> 251        the LAYOUT_WCC.  But the server can also use those attributes to
> 252        avoid querying the data server for the data file attributes.  Note
> 253        that as these attributes are optional and there is nothing the client
> 254        can do if the server ignores one, there is no need to return a
> 255        bitmap4 of which attributes were accepted in the result of the
> 256        LAYOUT_WCC.
> 
> GV> Alternative proposal text:
> 
> "
> The current filehandle and the lowa_stateid identify the specific layout for
> the LAYOUT_WCC operation. The lowa_type indicates how to interpret the
> layout-type-specific payload contained in the lowa_body field. This lowa_type
> is defined as a value (TBD#1) from the IANA registry for 'pNFS Layout Types'.

Here I went with:

       The current filehandle and the lowa_stateid identify the specific 
       layout for the LAYOUT_WCC operation. The lowa_type indicates how
       to interpret the layout-type-specific payload contained in the
       lowa_body field. The lowa_type is the corresponding value
       from the IANA registry for 'pNFS Layout Types' for the layout
       type being used.

I’m hoping that last sentence clarified that the value is to convey which existing Layout Type is being used.

> 
> The lowa_body contains the data file attributes. The client is responsible for
> mapping NFSv3 post-operation attributes to the fattr4 representation. Similar
> to the behavior of post-operation attributes, the client may ignore these
> attributes, and the server may also choose to ignore any attributes included in
> LAYOUT_WCC. However, the server can use these attributes to avoid querying the
> data server for data file attributes. Because these attributes are optional and
> the client has no recourse if the server opts to disregard them, there is no
> requirement to return a bitmap4 indicating which attributes have been accepted
> in the LAYOUT_WCC result. "

I took this change.

> 
> 458     6.  IANA Considerations
> 459
> 460        This section is to be removed before publishing as an RFC.
> 461
> 462        There are no IANA considerations for this document.
> 
> GV> Is this correct? Section 3 at line 244 & 245 i read:
> 
>   The lowa_type is defined to be a value from the IANA registry for
>   'pNFS Layout Types Registry'.
> 
> Does this not indicate that there is a value assigned by IANA?
> 

This was already addressed in another thread.

> Many thanks again for this document,
> 
> Kind Regards,
> Gunter Van de Velde,
> RTG AD
> 
> 
>