[nfsv4] Re: review of draft-ietf-nfsv4-internationalization-10.txt

[David Noveck <[e-mail]>]

Thanks for the review.   Am looking to provide a new draft in early August.

On Wed, Jul 24, 2024 at 1:23 PM Christoph Hellwig <[e-mail]> wrote:

> Hi Dave,
>
> this is a review of draft-ietf-nfsv4-internationalization-10.txt.
>
> First, please run the document through a spell checker, there are a lot
> of typos and annoying inconsistencies like using implementers and
> implementors right next to each other.
>
> Second, please refer to my recent mail to the WG list about covering 4.0
> or not.  I think that needs to be a WG conclusion an is not document
> specific, but highly relevant here.
>
> The document refers to the term 'physical file systems' without
> explaining it first.  A more common term would be 'local file system'
> but even that is not entirely correct as NFS servers can also
> export remote or distributed file systems, so 'underlying file system'
> might be a better choice.  Either way the term should be explained in
> the General Definitions section.
>
> Section 2.2. "Requirements Language Derivation" is very confusing.
> The BCP 14 terms designate requirements of the specification, and
> the reason why the specification has these requirements does not
> matter.  As far as I can tell simply removing this section would
> improve the document significantly.
>
> The forward reference from Section 6 to Section 7 harms the readability
> of the document.  Instead of referring to different modes of operation
> that are defined later, please define the high level operation modes
> earlier my moving section 7 before section 6. In general I also don't
> find references to "NFSv3" or "older" string handling very useful.
> To an unencumbered implementer, spelling out the actual modes with
> descriptive names will be a lot more useful and descriptive and useful.
> That does not preclude mentioning what NFSv3 did, but it should be part
> of the primary definition.  It would also really help to define the
> two operation modes and the terms used by them in a General Definitions
> section.
>
> The name caching described in Section 6.3 can work in a slightly modified
> form when file name normalization is used.  Linux implements it for
> local file systems using Unicode based normalization and case
> insensitivity.  In the NFS context that would require that the
> client knows the exact normalization behavior and Unicode version used
> on the server, and thus is relatively impractical.  Given how little
> NFS is used with case insensitive file systems is also doesn't
> really matter.
>
> Section 8 and especially Section 8.1 are very meandering and the
> wording feels like an essay and not suitable for a normative document.
>
> I would suggest to replace Section 8 with a short introduction of
> what changes compared to the definition in RFC8881 and drop section
> 8.1 entirely.  The details history of the text is of no importance to
> implementers.  If you feel it should be properly archive somewhere
> an informational document on the history of internationalization in
> NFS might be somewhat interesting.
>
> Section 8.2 should be worded as actual normative language.  My
> quick and dirty proposal is below:
>
> -------------------------------- snip --------------------------------
> This section replaces Section 14.4 of [RFC8881], taking into account the
> behavior of existing implementation of [RFC5661] [RFC8881] while
> providing best effort compatibility with the definition in [RFC5661] and
> [RFC8881].
>
>
>   const FSCHARSET_CAP4_CONTAINS_NON_UTF8  = 0x1;
>   const FSCHARSET_CAP4_ALLOWS_ONLY_UTF8   = 0x2;
>
>   typedef uint32_t        fs_charset_cap4;
>
> This attribute provides a simple way of determining whether a particular
> file system behaves as a UTF-8-only server and rejects file names which
> are not valid UTF8-encoded strings. When this attribute is supported and
> the value returned has the FSCHARSET_CAP4_ALLOWS_ONLY_UTF8 flag set, the
> error NFS4ERR_INVAL MUST be returned if any file name argument contains a
> string which is not a valid UTF8-encoded string.
>
> When this attribute is supported and the value returned has the
> FSCHARSET_CAP4_ALLOWS_ONLY_UTF8 flag clear, the error NFS4ERR_INVAL will
> not be returned based on adherence to the rules of UTF-8.
>
> The FSCHARSET_CAP4_CONTAINS_NON_UTF8 flags exist for historical reasons
> only and has no clear behavior associated with it.  Servers SHOULD set
> the FSCHARSET_CAP4_CONTAINS_NON_UTF8 when they set the
> FSCHARSET_CAP4_ALLOWS_ONLY_UTF8 flag, and clear the
> FSCHARSET_CAP4_CONTAINS_NON_UTF8 when they clear
> FSCHARSET_CAP4_ALLOWS_ONLY_UTF8 to provide the best compatibility to
> historic clients.
>
> Clients SHOULD ignore the FSCHARSET_CAP4_CONTAINS_NON_UTF8 flag.
>
> When the fs_charset_cap attribute is not supported, the client can
> perform a LOOKUP using a name not conforming to the rules of UTF-8
> and use the error returned to determine whether non-UTF-8 names are
> accepted.
> -------------------------------- snip --------------------------------
>
> Section 12 is closely related to Sections 6 and 7 and it would
> be very helpful if it was located close to them in the document.
>
> The section could use a bit of rewording, and the use of BCP 14
> terms to describe actual server behavior and not just protocol
> requirements seems wrong.  And possible replacement for the start
> of this section is provided below:
>
> -------------------------------- snip --------------------------------
> Servers MAY accept component names that are not valid UTF-8 strings on
> all or on some subset of file systems exported.
>
> A typical pattern is for a server to use UTF‑8-unaware underlying file
> systems that treat component names as uninterpreted strings of bytes,
> rather than having any awareness of the character set being used.
> Such servers do not change the stored representation of component names
> from those received on the wire and use an octet-by-octet comparison of
> component name strings to determine equivalence (as opposed to any
> broader notion of string comparison).
>
> This is because the server has no knowledge of the specific character
> encoding being used.
> -------------------------------- snip --------------------------------
>
> Section 13 feels like editorializing.  I don't think this belongs into
> a normative section.  If you feel attached to it maybe move it to
> an appendix or the above mentioned informational document?
>
> In general the document could use a bit less of term "As stated"
> {,above,previously}".
>
> This review didn't make it to the appendices yet, but I plan to
> look over them in the future.
>