Subject: [nfsv4] AD review: draft-ietf-nfsv4-rpcrdma-05

From: Lars Eggert <[email-masked]>
Date: Mon, 28 May 2007 18:22:24 +0300

I'm trying to do the AD review phase early (i.e., in parallel WG last  
call rather than afterwards), and for this document it has actually  
worked.

Summary: Mostly ready, but some issues need to be addressed before it  
con move forward.

Detailed comments:

Section 1., paragraph 2:
 >      ONC RPC [RFC1831] is a remote procedure call protocol that  
has been
 >      run over a variety of transports.

   Should this document refer to 1831bis instead?

Section 3.3., paragraph 3:
 >      The RPC over RDMA header for the RPC reply messages provides the
 >      granted result, which MAY have any value except it MAY NOT be  
zero

   s/MAY NOT/MUST NOT/ (MAY NOT is not an RFC2119 term)

Section 3.3., paragraph 5:
 >      connection.  Even if an error does not occur, it is NOT REQUIRED

   s/NOT REQUIRED/OPTIONAL/ (NOT REQUIRED is not an RFC2119 term)

Section 3.3., paragraph 6:
 >      to each connected RPC client.  It does however NOT REQUIRE  
that the
 >      server always be prepared to receive a request from each client,

   s/It does however NOT REQUIRE that the server/The server MAY/ (NOT
   REQUIRE is not an RFC2119 term)

Section 3.4., paragraph 8:
 >      Additionally, when making an RPC call that can result in bulk  
data
 >      transferred in the reply, it is desirable to provide chunks to

   "is is desirable" - enough to make this a "SHOULD" recommendation?

Section 3.4., paragraph 24:
 >      upper layer as part of RPC completion.  However, the RPC layer
 >      SHOULD NOT assume that the buffers have not been modified.

   s/SHOULD NOT/MUST NOT/ (or is there a reason why they sometimes may?)

Section 4.1., paragraph 2:
 >      1. Transaction ID (XID).
 >           The XID generated for the RPC call and reply.  Having  
the XID
 >           at the beginning of the message makes it easy to  
establish the
 >           message context.  This XID mirrors the XID in the RPC  
header,
 >           and takes precedence.  The receiver MAY ignore the XID  
in the
 >           RPC header, if it so chooses.

   "Mirrors" as in "is a duplicate of"? What does "take precedence" mean
   - is this concerned with when the two XIDs aren't the same? Why would
   that happen? Finally, wouldn't it be safer if the client would always
   check both XIDs?

Section 4.2., paragraph 2:
 >      support.  The version number in this reply MAY be any value
 >      otherwise valid at the receiver.  When other decoding errors are

   s/MAY be any/MUST be a/ (MAY means it could be something else)

Section 4.3., paragraph 12:
 >         union rpc_rdma_error switch (rpc_rdma_errcode) {

   rpcgen complains about "expected '*' or 'identifier'" here (note that
   I have no clue if this error is valid or not - I haven't done RPC
   programming since I was an undergrad)

Section 6.1., paragraph 1:
 >      connection parameters.  To provide a basis for connection
 >      negotiation, the connection is assumed to provide a basic  
level of
 >      interoperability: the ability to exchange at least one RPC  
message
 >      at a time that is at least 1 KB in size.  The server MAY exceed
 >      this basic level of configuration, but the client MUST NOT  
assume
 >      it.

   At what rate must the server be able to receive these 1KB RPC
   messages? The text limits the size but note the rate. (Is this  
related
   to maxrdmaread?)

Section 6.2., paragraph 2:
 >      The maxcall_sendsize argument is the maximum size of an RPC call
 >      message that the client MUST send inline in an RDMA Send  
message to
 >      the server.

   "MUST send inline", meaning it MUST NOT sent it in another way?

Section 11., paragraph 2:
 >      For efficiency, more appropriate security mechanism for RDMA  
links
 >      may be link-level protection, such as IPSec, which may be co-

   IPsec is not a link-level protection mechanism, but can be
   *configured* to provide such. Also, nit: s/IPSec,/IPsec,/

Section 12., paragraph 2:
 >      The following string is to be added to the "nc_proto"  
registry on
 >      page 5 of [RFC1833]:
 >           NC_RDMA "rdma"

   This won't work - IANA cannot modify RFCs. If that registry from
   RFC1833 has been created through an IANA action in another document,
   identify the resulting IANA registry here and request the entry.
   Otherwise, this document needs to create the registry and populate it
   with the entries from RFC1833 (see draft
   draft-narten-iana-considerations-rfc2434bis) before registering the
   new entry. In that case, this document also needs to "Update"  
RFC1833.

Section 12., paragraph 6:
 >      Currently, these numbers are not assigned by IANA, they are  
merely
 >      republished [IANA-RPC].

   If IANA republishes them, who is originally publishing them? We may
   need to register that entry there.

Section 14., paragraph 2:
 >      [RFC1094]
 >           Sun Microsystems, "NFS: Network File System Protocol
 >           Specification", (NFS version 2) Informational RFC,
 >           http://www.ietf.org/rfc/rfc1094.txt

   Please call out this DOWNREF in the document shepherd writeup - we
   need to confirm that this is OK during the IETF last call.

Section 14., paragraph 4:
 >      [RFC1813]
 >           B. Callaghan, B. Pawlowski, P. Staubach, "NFS Version 3
 >           Protocol Specification", Informational RFC,
 >           http://www.ietf.org/rfc/rfc1813.txt

   Please call out this DOWNREF in the document shepherd writeup - we
   need to confirm that this is OK during the IETF last call.

The rest are all nits (typos, etc.)

Section 3.4., paragraph 11:
 >      RECOMMENDED that upper layer RPC client protocol  
specifcations omit

   Nit: s/specifcations/specifications/

Section 3.4., paragraph 13:
 >      stream element.  In particular, for counted fields (e.g. a "<>"

    Nit: s/e.g./e.g.,/ (also elsewhere)

Section 3.7., paragraph 5:
 >      avoid encoding indivudual RDMA Read Chunks for roundup whenever

   Nit: s/indivudual/individual/

Section 5.1., paragraph 1:
 >      it separately as a chunk, i.e. reference it as the first  
entry in

   Nit: s/i.e./i.e.,/ (also elsewhere)

Section 15., paragraph 1:
 >           R. Recio et. al., "A Remote Direct Memory Access Protocol

   Nit: s/et./et/ (also elsewhere)

_______________________________________________
nfsv4 mailing list
[email-masked]
https://www1.ietf.org/mailman/listinfo/nfsv4