[nfsv4] Re: AD review : draft-ietf-nfsv4-layoutwcc-02

[David Noveck <[e-mail]>]

On Wed, Aug 14, 2024, 11:56 AM Zaheduzzaman Sarker <
[e-mail]> wrote:

> Hi,
>
>
>
> I have now reviewed the draft-ietf-nfsv4-layoutwcc and I think this
> document need to improve the writing. Hence please respond to the comments
> and resolve them.
>
>
>
> I have done this review as If I don't really understand NFSv4 and NFSv3
> (which might be anyway true) and tried to reflect on things that was
> unclear in the description.
>
>
>
> I hope this review helps and feel free to take the opportunity to educate
> me.
>
>
>
> //Zahed
>
>
>
>
>
>
>
> My section by section comments are below -
>
> ====================================
>
>
>
> # Abstract
>
>     s/This document presents a refinement to RFC8435 to allow the client
> to update the metadata server to changes on the data server/ This document
> updates RFC8435 to allow the client to update the metadata server to
> changes on the data server.
>
>
>
>     This makes it clear that the current specification will update RFC
> 8435.
>
>
>
>     However, to me it is still not super clear if we are updating RFC 8435
> or extending RFC 7862.
>

I think both are done.

What is the rationale here for the update tag?
>

I believe it is necessary since, anyone implementing RFC8435 needs to know
about the additions in this document.


>
> # Section 1
>
>
>
>     My initial assumption was that we have a useful feature with WCC for
> NFv3 that we would like to allow for NFv4 with LAYOUT-WCC. However, the
> more I read Section 1 and Section 2, I am getting more confused as the
> description of NFSv4 and NFv3 is very intertwined. Either my assumption is
> wrong
>

It isn't.

or the text should be improved, either case I think the following would
> help.
>
>         - let’s assume the reader will read this as a starting document (I
> think this will be true for the most of the reviewers in the process)
>

True but I think that the people Tom was writing for are very different
from the reviewers.  I believe he was writing for those who had or might
implement RFC8435.

> hence, let's describe the scenario better for them and not assume they
> understand and know the difference between NFv4 and NFSv3 by heart. Hence,
> providing right reference is the right tools here. Also make sure we write
>
>             - the version of NFS the metadata server is running or does it
> even matter
>

It matters. It has to be NFSv4.2, at least.

           - short description of the WCC and reference the RFC for the
details

>         - Lets keep all the NFv3 related details in the Introduction
> section. This means describe some of the basic concepts those are described
> in different sub-sections of section 2 in Section 1, and focus on NFSv4
> specifics in section 2 and 3. This means already in introduction we write
> the relation of NFSv3 GETATTR to NFv4 GETATRR and who sends them , etc. as
> a base to understand the extension are doing in the rest of the sections.
>
>         - Make sure we are specific about the extension we are doing in
> this specification - are we writing this specification only for NFSv4
> client or the client's NFV version does not matter? If the client learns
> gid and uid from via NFSv3 how that is done and why that is important here.
> This pops up if I read the following sentence -
>
>
>
>                        "In this document, we introduce a new operation
> called LAYOUT_WCC which allows the client to periodically report the
> attributes of the data files to the metadata server."
>

My impression was that the attribute were the ones changed by doing WRITEs,
not including uid and gid. Not sure what to make of later references to
these.


>
>
>
> # Section 2.1
>
>
>
>     Please provide the reference to the data types of the structs
>
>
>
> # Section 2.3
>
>
>
>     It not clear if the extension is for NFSv4 only or it can also replace
> WCC for NFSv3.
>

It can't.


>
>     Why are first 3 paragraph relevant at all? what is the relation of
> this description to the extension?
>
>
>
>     Overall, sometimes it is not clear in the text on who does what. I
> think we can do a better job of stating what is client's responsibility and
> what is metadata server's. This might be get cleared automatically when we
> have a better split between the section 1 and section 2
>
>
>
> # Section 2.4.1
>
>
>
>     - Please provide reference to mtime, atime and all the attribute
> mentioned (size, space_used, change, time_access, time_metadata, and
> time_modify.).
>
>     - it says -
>
>
>
>             Whenever it sends an NFS4ERR_ACCESS error via LAYOUTRETURN or
> LAYOUTERROR - it could have already gotten the NFSv3 uid and gid values
> back in the WCC of the WRITE, READ, or COMMIT operation which got the error.
>
>
>
>         I am a bit confused here, just reading the text here, are you
> saying the client had got some attributes via NFSv3 but when error occurs
> it uses NFv4 LAYOUT-WCC to report?
>
>
>
> # Section 2.5
>
>
>
>     We should be very clear that we are not defining any new ERROR for
> LAYOUT_WCC if that is the case and reference to where the errors are
> defined. Otherwise, we need to explain each and every errors here in this
> document, that would be cumbersome or repeating.
>
>
>
> # Section 5 IANA considerations :
>
>
>
>     Please provide more details on which IANA registry to be
> updated/amended for the new entries.
> _______________________________________________
> nfsv4 mailing list -- [e-mail]
> To unsubscribe send an email to [e-mail]
>