Content-Type: text/plain;
	charset=utf-8
Mime-Version: 1.0 (Mac OS X Mail 16.0 \(3826.400.131.1.6\))
From: Thomas Haynes <loghyr@gmail.com>
In-Reply-To: 
 <173616823022.1843766.10743657654524313983@dt-datatracker-65f549669d-2xld9>
Date: Thu, 6 Feb 2025 12:58:52 -0800
Content-Transfer-Encoding: quoted-printable
Message-Id: <28A9D41B-3F17-4D25-B295-4D76E11493EB@gmail.com>
References: 
 <173616823022.1843766.10743657654524313983@dt-datatracker-65f549669d-2xld9>
To: Gunter Van de Velde <gunter.van_de_velde@nokia.com>
Message-ID-Hash: AR4PR6MEFSFKDI653ESS2AIP2YB4XWK5
CC: The IESG <iesg@ietf.org>, draft-ietf-nfsv4-layoutwcc@ietf.org,
 nfsv4-chairs <nfsv4-chairs@ietf.org>, nfsv4@ietf.org
Precedence: list
Subject: =?utf-8?q?=5Bnfsv4=5D_Re=3A_Gunter_Van_de_Velde=27s_Discuss_on_draft-ietf-nf?=
 =?utf-8?q?sv4-layoutwcc-05=3A_=28with_DISCUSS_and_COMMENT=29?=
Archived-At: 
 <https://mailarchive.ietf.org/arch/msg/nfsv4/ZjhLfi4Msewmc6yC5Entdaq_GDs>

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=E2=80=AFAM, Gunter Van de Velde via =
Datatracker <noreply@ietf.org> wrote:
>=20
> Gunter Van de Velde has entered the following ballot position for
> draft-ietf-nfsv4-layoutwcc-05: Discuss
>=20
> 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.)
>=20
>=20
> Please refer to =
https://www.ietf.org/about/groups/iesg/statements/handling-ballot-position=
s/=20
> for more information about how to handle DISCUSS and COMMENT =
positions.
>=20
>=20
> The document, along with other ballot positions, can be found here:
> https://datatracker.ietf.org/doc/draft-ietf-nfsv4-layoutwcc/
>=20
>=20
>=20
> ----------------------------------------------------------------------
> DISCUSS:
> ----------------------------------------------------------------------
>=20
> # Gunter Van de Velde, RTG AD, comments for =
draft-ietf-nfsv4-layoutwcc-05
>=20
> # the referenced line numbers are derived from the idnits tool:
> =
https://author-tools.ietf.org/api/idnits?url=3Dhttps://www.ietf.org/archiv=
e/id/draft-ietf-nfsv4-layoutwcc-05.txt
>=20
> # 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.
>=20
> # 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.
>=20
> #DISCUSS
> #=3D=3D=3D=3D=3D=3D=3D
>=20
> # 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?
>=20
> 244        The lowa_type is defined to be a value from the IANA =
registry for
> 245        'pNFS Layout Types Registry'.
>=20
>=20
> ----------------------------------------------------------------------
> COMMENT:
> ----------------------------------------------------------------------
>=20
> #DETAILED COMMENTS
> #=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D
>=20
> 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.
>=20
> GV> What about the following alternative abstract:
>=20
> "
> This document specifies extensions to the parallel Network File System =
(NFS)
> version=E2=80=AF4 (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=E2=80=99ll take the rewording.



>=20
> 106        See Section 1.1 of [RFC8435] for a fuller set of =
definitions.
>=20
> GV> Not sure that using the word "fuller" is appropriate. What about:
>=20
> "
> For a more comprehensive set of definitions, see Section=E2=80=AF1.1 =
of [RFC=E2=80=AF8435].
> "


I took the rewording.



>=20
> 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.
>=20
> GV> I find the use of "to tell" odd in IETF text. What about the =
following
> editorial suggestion:
>=20
> "
> 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=E2=80=AF8435], describes how data servers using =
NFSv3 can be
> accessed. The client is restricted to performing NFSv3 READ =
(Section=E2=80=AF3.3.6 of
> [RFC=E2=80=AF1813]), WRITE (Section=E2=80=AF3.3.6 of [RFC=E2=80=AF1813])=
, and COMMIT (Section=E2=80=AF3.3.21 of
> [RFC=E2=80=AF1813]) 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.


>=20
> 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]).
>=20
> GV> This reads odd to me. Are you trying to suggest the following
>=20
> "
> For example, the metadata server might issue an NFSv3 GETATTR =
operation to the
> data server, which is typically triggered by a client=E2=80=99s 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=E2=80=AF2.6 of [RFC=E2=80=AF1813]). "
>=20

I tend to not use commas so much=E2=80=A6

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.
>=20
> GV> Proposal text:
>=20
> "
> 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. "
>=20

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.
>=20
> GV> proposal text:
>=20
> "
> 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=E2=80=A6.


>=20
> 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.
>=20
> GV> Alternative proposal text:
>=20
> "
> 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=20=

       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=E2=80=99m hoping that last sentence clarified that the value is to =
convey which existing Layout Type is being used.
=20


>=20
> 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.


>=20
> 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.
>=20
> GV> Is this correct? Section 3 at line 244 & 245 i read:
>=20
>   The lowa_type is defined to be a value from the IANA registry for
>   'pNFS Layout Types Registry'.
>=20
> Does this not indicate that there is a value assigned by IANA?
>=20


This was already addressed in another thread.



> Many thanks again for this document,
>=20
> Kind Regards,
> Gunter Van de Velde,
> RTG AD
>=20
>=20
>=20