Subject: [Gen-art] Gen-art LC review of draft-ietf-nfsv4-minorversion2-dot-x-39

From: Elwyn Davies <[email-masked]>
Date: Mon, 14 Dec 2015 00:44:08 +0000

I am the assigned Gen-ART reviewer for this draft. The General Area
Review Team (Gen-ART) reviews all IETF documents being processed
by the IESG for the IETF Chair.  Please treat these comments just
like any other last call comments.

For more information, please see the FAQ at

<http://wiki.tools.ietf.org/area/gen/trac/wiki/GenArtfaq>.

Document: draft-ietf-nfsv4-minorversion2-dot-x-39.txt
Reviewer: Elwyn Davies
Review Date: 2015-12-13
IETF LC End Date: 2015-12-09
IESG Telechat date: (if known) -

Summary: Ready with nits.  The XDR specification appears to be a 
superset of the v4.1 XDR specification and combines a correction of the 
five remaining discrepancies between v4.1 and v4.0bis ( definition and 
use of the NFS4_OTHER_SIZE constant, addition and use of the 
ascii_REQUIRED4 type, modification of the typedef of linktext4). The 
additions of the 4.2 interface appears to match the specification in 
draft-ietf-nfsv4-minorversion2-39 except for two attribute typedef  
items mentioned below.  One discrepancy would have no operational effect 
as the type used in the other draft  is an alias for the type used here 
but the clone_blksize error changes the size of the type.

I have checked that the extracted code is accepted by rpcgen and 
generates files as expected.

Major issues:
None

Minor issues:
None

Nits/editorial comments:
Observation:  It might be useful to note that this XDR specification is 
fully upwards compatible with the v4.0bis with the minor exception of 
the clientaddr4 structure which has been replaced by (strictly, aliased 
to) netaddr4 which has the same members with the same purposes but the 
names have changed (r_netid -> na_r_netid, r_addr ->na_r_addr).  This 
effectively fully reconverges the v4.0bis and v4.1 strands of the XDR.

Line 1145: In draft-ietf-nfsv4-minorversion2-39 we have typedef length4 
fattr4_space_freed
                     whereas in this draft we 
have                           typedef uint64_t fattr4_space_freed

Line 1149: In draft-ietf-nfsv4-minorversion2-39 we have typedef length4 
fattr4_clone_blksize
                     whereas in this draft we 
have                           typedef uint32_t fattr4_clone_blksize

---

From: Tom Haynes <[email-masked]>
Date: Wed, 16 Dec 2015 18:19:25 -0800

> On Dec 13, 2015, at 4:44 PM, Elwyn Davies <[email-masked]> wrote:
> 
> Summary: Ready with nits.  The XDR specification appears to be a superset of the v4.1 XDR specification and combines a correction of the five remaining discrepancies between v4.1 and v4.0bis ( definition and use of the NFS4_OTHER_SIZE constant, addition and use of the ascii_REQUIRED4 type, modification of the typedef of linktext4). The additions of the 4.2 interface appears to match the specification in draft-ietf-nfsv4-minorversion2-39 except for two attribute typedef  items mentioned below.  One discrepancy would have no operational effect as the type used in the other draft  is an alias for the type used here but the clone_blksize error changes the size of the type.
> 
> I have checked that the extracted code is accepted by rpcgen and generates files as expected.
> 
> Major issues:
> None
> 
> Minor issues:
> None

Hi Elwyn,

Thanks for the review - sorry for the delay in responding, I’m just now surfacing from my job. :-)

Responses inline.

> 
> Nits/editorial comments:
> Observation:  It might be useful to note that this XDR specification is fully upwards compatible with the v4.0bis with the minor exception of the clientaddr4 structure which has been replaced by (strictly, aliased to) netaddr4 which has the same members with the same purposes but the names have changed (r_netid -> na_r_netid, r_addr ->na_r_addr).  This effectively fully reconverges the v4.0bis and v4.1 strands of the XDR.
> 

Shamelessly stolen almost verbatim!

> Line 1145: In draft-ietf-nfsv4-minorversion2-39 we have typedef length4 fattr4_space_freed
>                    whereas in this draft we have                           typedef uint64_t fattr4_space_freed

Fixed this in the XDR document.

> 
> Line 1149: In draft-ietf-nfsv4-minorversion2-39 we have typedef length4 fattr4_clone_blksize
>                    whereas in this draft we have                           typedef uint32_t fattr4_clone_blksize

And for this one, I made the change to uint32_t in draft-ietf-nfsv4-minorversion2-39 .

Thanks again!
Tom

---

From: Elwyn Davies <[email-masked]>
Date: Thu, 17 Dec 2015 20:09:28 +0000

Hi, Tom.

Excellent... so I think we are done with dot-x.

One additional point that I was going to write into the main 
minorversion2 review relating to referencing  the requirements RFC 
7204.  rpcsec-gssv3 also references the requirements RFC but the amount 
of info that is needful to  support implementers seems to be mostly in 
minorversion2.  Would it be possible for you and Andy to work out if 
anything extra is really needed in minorversio2 (about guest mode 
primarily I think) so that the requirements reference is not needed and 
rpcsec-gssv3 can just reference minorversion2 for all info on the 
modes?  I wasn't sure what, if anything, extra was needed in minorversion2.

Cheers,
Elwyn

---

From: Tom Haynes <[email-masked]>
Date: Fri, 18 Dec 2015 12:32:38 -0800

> On Dec 17, 2015, at 12:09 PM, Elwyn Davies <[email-masked]> wrote:
> 
> Hi, Tom.
> 
> One additional point that I was going to write into the main minorversion2 review relating to referencing  the requirements RFC 7204.  rpcsec-gssv3 also references the requirements RFC but the amount of info that is needful to  support implementers seems to be mostly in minorversion2.  Would it be possible for you and Andy to work out if anything extra is really needed in minorversio2 (about guest mode primarily I think) so that the requirements reference is not needed and rpcsec-gssv3 can just reference minorversion2 for all info on the modes?  I wasn't sure what, if anything, extra was needed in minorversion2.
> 

Ack, will look into that - I’m just starting on the main document review now. :-)