5761 nfs4_prot.x should be updated for RFC 7531

Review Request #21 — Created April 5, 2015 and submitted


webrev: http://cr.illumos.org/~webrev/marcel/il-5761-rfc-7531/

This replaces the nfs4_prot.x file with the output of the following command:

curl http://tools.ietf.org/rfc/rfc7531.txt | grep "^  *///" | sed 's?^  */// ??' | sed 's?^  *///$??'

The changes in all other files were evoked by the nfs4_prot.x file update.

I tested the basic NFSv4 functionality on both NFS server and NFS client. I
also make sure the automounter works as expected. As a sanity check I tested
both NFSv2 and NFSv3 as well.

  • 0
  • 0
  • 1
  • 0
  • 1
Description From Last Updated
  1. From what I can see, the real semantic changes client4's cb_program member being uint_t instead of uint32_t, the new struct linktext4. The new ascii_REQUIRED4 typedef doesn't really count because it just uses the same validation as the original utf8string.

  2. These casts shouldn't be necessary, unless I'm missing something. Just for consistency with the existing code?

    1. Yes, this is for consistency only. Note: If I would generate this code using rpcgen(1) it would have the cast as well. Maybe we should change rpcgen(1) to do not add the cast where not needed...

  3. usr/src/uts/common/fs/nfs/nfs4_srv.c (Diff revision 1)

    This and similar casts from linktext4 * in nfs4_vnops.c seem ill-advised if linktext4 now explicitly has a separate definition from a utf8string. Maybe add a new set of functions taking linktext4 *?

    1. You mean a new set of functions doing basically the same thing as utf8_to_str() does, but accepts linktext4 instead of utf8string?

    2. Yep. The current definition was introduced in one of the drafts along with RFC 7530's
      "The contents of symbolic links (of type linktext4 in the XDR) MUST be treated as opaque data
      by NFSv4 servers. Although UTF-8 encoding is often used, it need not be."
      We may have to use separate conversion functions to avoid current or future UTF-8 assumptions in utf8_to_str() or its inverse. It should be treated similarly to component4.

    3. I fully understand what you mean and what is your point. The problem is that our implementation is even more complex than one would expect (or want): we do not treat the linktext4 as opaque data, nor UTF-8 string. The linktext4 in our NFS server implementation is subject of the character conversion; the nfscmd_convname() is called on it. So we cannot avoid the current UTF-8 assumptions, because the assumptions are a bit different. Having said that, it looks like the calls like utf8_to_str() on linktext4 are not correct (with my fix, and even without my fix), but we should deal with that separately (if we would do so). So I believe the typecasting, as I am suggesting in the fix, is the best way how to deal with that for now.

      OTOH, if you strongly feel that I should create new functions for linktext4, I'll do so.

      Note: The component4 is defined as UTF-8 string in the RFC.


    4. Thanks for the explanation of the additional problems, I agree with the suggestion to address them separately. This seems like a case where the goals of cleaning up the code and adherence to the standards align. I will create a related issue (or you can if you like) for our treatment of linktext4, and let's go ahead with this in the meantime.

    5. Thank you, Albert. I'll leave the related bug creation to you, if I can :-).

    6. Great. Here's #5860

  1. Ship It!
Review request changed

Status: Closed (submitted)

Change Summary:

commit bbe876c07ed632b8f85e195d41e7948382064a95
Author:     Marcel Telka <marcel.telka@nexenta.com>
AuthorDate: Thu Apr 2 23:33:50 2015 +0200
Commit:     Robert Mustacchi <rm@joyent.com>
CommitDate: Wed Apr 22 08:36:01 2015 -0700

    5761 nfs4_prot.x should be updated for RFC 7531
    Reviewed by: Gordon Ross <Gordon.W.Ross@gmail.com>
    Reviewed by: Albert Lee <trisk@omniti.com>
    Approved by: Robert Mustacchi <rm@joyent.com>

:100644 100644 4f3c232... 6a5dce8... M  exception_lists/copyright
:100644 100644 0ee7a6c... 88e2ff0... M  usr/src/cmd/cmd-inet/usr.sbin/snoop/nfs4_xdr.c
:100644 100644 bb1db5c... 107020c... M  usr/src/cmd/cmd-inet/usr.sbin/snoop/snoop_nfs4.c
:100644 100644 bf1c1c4... ebc6b7d... M  usr/src/cmd/fs.d/autofs/autod_nfs.c
:000000 100644 0000000... 04e670d... A  usr/src/head/rpcsvc/THIRDPARTYLICENSE.nfs4_prot
:000000 100644 0000000... 1e498b2... A  usr/src/head/rpcsvc/THIRDPARTYLICENSE.nfs4_prot.descrip
:100644 100644 0a218a6... 24460cc... M  usr/src/head/rpcsvc/nfs4_prot.x
:100644 100644 e33f2aa... 022fb45... M  usr/src/stand/lib/fs/nfs/nfs4_xdr.c
:100644 100644 fe1a10b... 2f8f776... M  usr/src/uts/common/fs/nfs/nfs4_srv.c
:100644 100644 1752a28... 151cb62... M  usr/src/uts/common/fs/nfs/nfs4_vfsops.c
:100644 100644 ef07aa3... d6bf384... M  usr/src/uts/common/fs/nfs/nfs4_vnops.c
:100644 100644 6d94aa0... 572e0aa... M  usr/src/uts/common/fs/nfs/nfs4_xdr.c
:100644 100644 c0d20ad... e27bd42... M  usr/src/uts/common/nfs/nfs4.h
:100644 100644 be30bed... 30d1e43... M  usr/src/uts/common/nfs/nfs4_kprot.h