[nfsv4] AD review of draft-ietf-nfsv4-rfc5667bis-11

Spencer Dawkins at IETF <spencerdawkins.ietf@gmail.com> Mon, 07 August 2017 21:45 UTC

Return-Path: <spencerdawkins.ietf@gmail.com>
X-Original-To: nfsv4@ietfa.amsl.com
Delivered-To: nfsv4@ietfa.amsl.com
Received: from localhost (localhost [127.0.0.1]) by ietfa.amsl.com (Postfix) with ESMTP id 89735124BE8; Mon, 7 Aug 2017 14:45:52 -0700 (PDT)
X-Virus-Scanned: amavisd-new at amsl.com
X-Spam-Flag: NO
X-Spam-Score: -2.699
X-Spam-Level:
X-Spam-Status: No, score=-2.699 tagged_above=-999 required=5 tests=[BAYES_00=-1.9, DKIM_SIGNED=0.1, DKIM_VALID=-0.1, DKIM_VALID_AU=-0.1, FREEMAIL_FROM=0.001, HTML_MESSAGE=0.001, RCVD_IN_DNSWL_LOW=-0.7, SPF_PASS=-0.001] autolearn=ham autolearn_force=no
Authentication-Results: ietfa.amsl.com (amavisd-new); dkim=pass (2048-bit key) header.d=gmail.com
Received: from mail.ietf.org ([4.31.198.44]) by localhost (ietfa.amsl.com [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id 4zQB82aKbPyf; Mon, 7 Aug 2017 14:45:50 -0700 (PDT)
Received: from mail-yw0-x22b.google.com (mail-yw0-x22b.google.com [IPv6:2607:f8b0:4002:c05::22b]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by ietfa.amsl.com (Postfix) with ESMTPS id ED0751201F2; Mon, 7 Aug 2017 14:45:46 -0700 (PDT)
Received: by mail-yw0-x22b.google.com with SMTP id p68so10785378ywg.0; Mon, 07 Aug 2017 14:45:46 -0700 (PDT)
DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=mime-version:from:date:message-id:subject:to:cc; bh=j1PBurzKnxcXq33/4N6wekvEEp6PIWgtFnuGIFsZA8Y=; b=gacLmSPGTP6a2Fi9Ds1ERu0lnN1VN7uPZI1DVOBmzNuL7ykploObglVfasjigf9DwK 7uyTXeh7k9lfA00ucUgHZHqRDkDWL6/4rNhPw7F1I+leEe18guO1Ou4cTWSbtaugSP05 NK+LLEYjVAOdH7JR6CZo5VVxAjs/hMUtUBGgXBI8T3iO7ygDTJNTRGLDFEMz24RiCl63 vHf+Vj6bJ4J9GG4n6suSsZ1caksqt4B4OcnGyAcPw+YZxXnBQ1rCJeq0dxGwNDAtYwBm 7PM9tJtet5sY0JvuwSfutPGwYpUUn8UPHiIm5QLOqqrZOR2zlIhF5visfdE+xEUv+EBM 5dbA==
X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:from:date:message-id:subject:to:cc; bh=j1PBurzKnxcXq33/4N6wekvEEp6PIWgtFnuGIFsZA8Y=; b=h+Y9J+qVDZ+a5oGh9P1GcvAusfVdtDUkTNjWd/31Is0c7iF5aPRPuL5mYDsc+4rZB1 pqTN5kTafH1YRtJT4hGEMni+PhY98yY4q6tWe1jW0RMS8SLy1ltgK4jGWTr2tDjbHNUW q96YFDsSlAJv6i38y/UrndRGlPJvXDOGAZzFnGhtULLr9kB6faa04DdTHQEr9sQkCJJ7 i3EnZXmkcoeCYOQq1oVo3W6p5KT2eIlrTSBt2a4e52b5gnOExNJVTCiCVSrVLh4r4s4g ValPEEGlbjxiUw5hptsLB/7JABiL7cLiDCy6b23HLQjzlV1G4sdvEI9qmlkQyx5iF4J0 RoVA==
X-Gm-Message-State: AHYfb5jFtKptfiRVE1vL8D7/b2VhS+P5VLu/hGuoOOnd1DgIQi6UpepW E3mP9WWaJ5j9DwV0QHSiaxeCIQlrNtOz
X-Received: by 10.13.210.67 with SMTP id u64mr1714495ywd.218.1502142345800; Mon, 07 Aug 2017 14:45:45 -0700 (PDT)
MIME-Version: 1.0
Received: by 10.37.52.79 with HTTP; Mon, 7 Aug 2017 14:45:45 -0700 (PDT)
From: Spencer Dawkins at IETF <spencerdawkins.ietf@gmail.com>
Date: Mon, 7 Aug 2017 16:45:45 -0500
Message-ID: <CAKKJt-fFFyxV=VhJxhe=3P283xZQG_yvdP6AHJkzU6iB=74JCw@mail.gmail.com>
To: draft-ietf-nfsv4-rfc5667bis@ietf.org
Cc: nfsv4-chairs@ietf.org, NFSv4 <nfsv4@ietf.org>
Content-Type: multipart/alternative; boundary="001a114e7e30ec727b055630c4da"
Archived-At: <https://mailarchive.ietf.org/arch/msg/nfsv4/I0OJvQSdfviMhy1t1bgwM4T-w1c>
Subject: [nfsv4] AD review of draft-ietf-nfsv4-rfc5667bis-11
X-BeenThere: nfsv4@ietf.org
X-Mailman-Version: 2.1.22
Precedence: list
List-Id: NFSv4 Working Group <nfsv4.ietf.org>
List-Unsubscribe: <https://www.ietf.org/mailman/options/nfsv4>, <mailto:nfsv4-request@ietf.org?subject=unsubscribe>
List-Archive: <https://mailarchive.ietf.org/arch/browse/nfsv4/>
List-Post: <mailto:nfsv4@ietf.org>
List-Help: <mailto:nfsv4-request@ietf.org?subject=help>
List-Subscribe: <https://www.ietf.org/mailman/listinfo/nfsv4>, <mailto:nfsv4-request@ietf.org?subject=subscribe>
X-List-Received-Date: Mon, 07 Aug 2017 21:45:53 -0000

Hi, Chuck,

This is another one of the NFSv4 docs that is easy to review. Thanks for
that.

I have some comments, but most of them are about clarity, rather than about
technical concerns.

Please let me know what you think, after you've had an opportunity to look
these over.

Thanks,

Spencer (D)

--

Pretty much every reviewer is going to ask you to update the reference to
draft-ietf-nfsv4-rfc5666bis to point to RFC 8166. Might as well do that
sooner, rather than later, because when reviewers start typing, they rarely
stop ...

When I saw this text

   This document contains material required of Upper Layer Bindings, as
   specified in [I-D.ietf-nfsv4-rfc5666bis], for the following NFS
   protocol versions:

   o  NFS Version 2 [RFC1094]

   o  NFS Version 3 [RFC1813]

   o  NFS Version 4.0 [RFC7530]

   o  NFS Version 4.1 [RFC5661]

   o  NFS Version 4.2 [RFC7862]

   Upper Layer Bindings are also provided for auxiliary protocols used
   with NFS versions 2 and 3.

I found myself wondering "what auxiliary protocols?" When you define the
Upper Layer Bindings for them in Section 4, that's very clear. Could you
consider either moving some of the Section 4 text here or just adding a
forward reference here, pointing to Section 4?

I found this text

   (to avoid hitting in a Duplicate Reply Cache)

a trifle unclear - is this saying

   (to avoid receiving the same error again from a Duplicate Reply Cache)

or something else? But even s/hitting in/hitting/ would be clearer for me.

In this text,

   Unless
   explicitly mentioned as applicable, short Reply chunk retry should
   not be used.

I wondered if you're saying "this is less efficient", or "this will
probably fail". Or some other possibility. But perhaps being clear about
likely consequences of doing something you're recommending against, would
help people talk themselves into believing you.

This text

   A transport error does not give an indication of whether the server
   has processed the arguments of the RPC Call, or whether the server
   has accessed or modified client memory associated with that RPC.

appears in Section 3, which covers NFS 2 and NFS 3/legacy NFS. Is this also
true for more recent versions? Or is that not a meaningful concept in
non-legacy NFS?

Pardon my unfamiliarity, but in this text,

   Historically, NFS/RDMA implementations have chosen to convey the
   MOUNT, NLM, and NSM protocols via TCP.  To enable interoperation of
   these protocols when NFS/RDMA is in use, a legacy NFS server MUST
   provide TCP-based MOUNT, NLM, and NSM services.

are these auxiliary protocols a set (so, they would all be present), or are
they independent (so some might be present while others might not be)? If
they're a set, "MUST provide x, y, and z" is the right answer, but if
they're independent, "MUST provide support for these protocols via TCP"
would be more appropriate. (I only ask about this because this text is
inside a MUST)

I see that NFSACL isn't mentioned in the previous paragraph I asked about.
I see

   Legacy clients and servers that support the NFSACL RPC Program
   typically convey NFSACL procedures on the same connection as NFS RPC
   Programs.  This obviates the need for separate rpcbind queries to
   discover server support for this RPC Program.

in the following section, and I'm assuming that the question of transport
protocol support for NFSACL has already been answered because it's using
the same connection as some other protocol that you've already answered the
question about. Did I get that right?

In this text,

   Specifically, the requirements of
   [I-D.ietf-nfsv4-rfc5666bis] ensure that this choice does not
   introduce new vulnerabilities. ensure that this choice does not
   introduce new vulnerabilities.

I believe you, but the people who write security reviews might appreciate a
pointer to a specific section in [I-D.ietf-nfsv4-rfc5666bis] to bound their
search, if there's one you can point to.