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

[Thomas Haynes <[e-mail]>]

> On Aug 14, 2024, at 8:55 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
>  


Zahed,

The review does help.

I am sending out a revised copy of the draft to see if my changes tackle the issues you have raised.

I think we will have another round after this one.


Thanks,
Tom




>  
>  
> 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. What is the rationale here for the update tag?


Good catch!

We removed it from delstid after discussion. I have removed it.

Also, we have this:

This document presents a
      refinement to RFC8435 to allow the client to update the metadata
      server to changes on the data server.

Would you prefer:

This document presents an
      extension to RFC8435 to allow the client to update the metadata
      server to changes on the data server.



>  
> # 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 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) 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
>             - 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."
>  
>  
> # Section 2.1
>  
>     Please provide the reference to the data types of the structs

Done

>  
> # Section 2.3
>  
>     It not clear if the extension is for NFSv4 only or it can also replace WCC for NFSv3.
>  
>     Why are first 3 paragraph relevant at all? what is the relation of this description to the extension?


It is the motivation for the changes. Pulled into a new section as per your other comments.



>    
>     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.).


Done

>     - 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?

Yes. 


But I will need to check to see if it can get those values on an error.



>  
> # 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.

Agreed and done.

>  
> # Section 5 IANA considerations :
>  
>     Please provide more details on which IANA registry to be updated/amended for the new entries. 


Marked the section as not to be included in the final draft.

I think historically we use this to show that RFC-TBD means the assigned RFC number.


> _______________________________________________
> nfsv4 mailing list -- [e-mail]
> To unsubscribe send an email to [e-mail]