[apps-discuss] [appsdir] Fwd: APPSDIR review of draft-ietf-netmod-rfc6021-bis-01

Martin Thomson <martin.thomson@gmail.com> (by way of S Moonesamy <sm+ietf@elandsys.com>) Mon, 29 April 2013 06:36 UTC

Return-Path: <sm@elandsys.com>
X-Original-To: apps-discuss@ietfa.amsl.com
Delivered-To: apps-discuss@ietfa.amsl.com
Received: from localhost (localhost [127.0.0.1]) by ietfa.amsl.com (Postfix) with ESMTP id 8CE3921F97EE; Sun, 28 Apr 2013 23:36:35 -0700 (PDT)
X-Virus-Scanned: amavisd-new at amsl.com
X-Spam-Flag: NO
X-Spam-Score: -1.3
X-Spam-Level:
X-Spam-Status: No, score=-1.3 tagged_above=-999 required=5 tests=[AWL=1.299, BAYES_00=-2.599]
Received: from mail.ietf.org ([12.22.58.30]) by localhost (ietfa.amsl.com [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id fJhfTatUXZOE; Sun, 28 Apr 2013 23:36:34 -0700 (PDT)
Received: from mx.ipv6.elandsys.com (mx.ipv6.elandsys.com [IPv6:2001:470:f329:1::1]) by ietfa.amsl.com (Postfix) with ESMTP id B531521F97E0; Sun, 28 Apr 2013 23:36:34 -0700 (PDT)
Received: from SUBMAN.elandsys.com ([197.226.235.145]) (authenticated bits=0) by mx.elandsys.com (8.14.5/8.14.5) with ESMTP id r3T6aK0S017956 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=NO); Sun, 28 Apr 2013 23:36:31 -0700 (PDT)
DKIM-Signature: v=1; a=rsa-sha256; c=simple/simple; d=opendkim.org; s=mail2010; t=1367217393; bh=aWp8PCMxDoEoNtGBOpYa0lKKWEERyw0seaCmWBW/eHg=; h=Date:To:From:Subject:Cc; b=B1XzKBXRArZDFabUq4olawm5k2yUJuIElMxF12jQhw2zVYDUuh8utiSSxvj60fGdh FQlYhAXU2cjGR2g2LpUlUu6DUgGD2nH4uTbqz8v84KQlbYtJWYeSTFGKfy3gEBKbqW 6Q+4WEdg+5ac4Qaw5CuVe1eSGaqhEP/Ktb/7orNo=
Message-Id: <6.2.5.6.2.20130428233426.0b62fed0@elandnews.com>
X-Mailer: QUALCOMM Windows Eudora Version 6.2.5.6
Date: Sun, 28 Apr 2013 23:36:08 -0700
To: apps-discuss@ietf.org, draft-ietf-netmod-rfc6021-bis.all@tools.ietf.org
From: Martin Thomson <martin.thomson@gmail.com> (by way of S Moonesamy <sm+ietf@elandsys.com>)
Mime-Version: 1.0
Content-Type: text/plain; charset="us-ascii"; format=flowed
Cc: iesg@ietf.org
Subject: [apps-discuss] [appsdir] Fwd: APPSDIR review of draft-ietf-netmod-rfc6021-bis-01
X-BeenThere: apps-discuss@ietf.org
X-Mailman-Version: 2.1.12
Precedence: list
List-Id: General discussion of application-layer protocols <apps-discuss.ietf.org>
List-Unsubscribe: <https://www.ietf.org/mailman/options/apps-discuss>, <mailto:apps-discuss-request@ietf.org?subject=unsubscribe>
List-Archive: <http://www.ietf.org/mail-archive/web/apps-discuss>
List-Post: <mailto:apps-discuss@ietf.org>
List-Help: <mailto:apps-discuss-request@ietf.org?subject=help>
List-Subscribe: <https://www.ietf.org/mailman/listinfo/apps-discuss>, <mailto:apps-discuss-request@ietf.org?subject=subscribe>
X-List-Received-Date: Mon, 29 Apr 2013 06:36:35 -0000

(Apologies, I have this habit of not including directorates on
directorate reviews.  Correcting this oversight.)

I have been selected as the Applications Area Directorate reviewer for
this draft (for background on appsdir, please see
http://trac.tools.ietf.org/area/app/trac/wiki/ApplicationsAreaDirectorate
).

Please resolve these comments along with any other Last Call comments
you may receive. Please wait for direction from your document shepherd
or AD before posting a new version of the draft.

Document: draft-ietf-netmod-rfc6021-bis-01
Title: Common YANG Data Types
Reviewer: Martin Thomson
Review Date: 2013-04-29

Summary: This draft is ready for publication as a Proposed Standard
RFC.  I have some minor issues and questions.

This is some of the most readable code that I have seen.

Major Issues: none

Minor Issues:

Section 4:
There is an RFC that describes a canonical textual representation of
IPv6 addresses.  You should use that: RFC 5952.

domain-name doesn't mention RFC 5890 at all (5891 is cited, but not
used in the text).  I assume that these are LDH-labels:
http://tools.ietf.org/html/rfc5890#section-2.3.1 and that this
document should say as much.

Questions:

Section 3:
phys-address: why is this optionally empty?  Maybe explain why in the
document - value absent?
hex-string: why is this required to include at least one octet? The
text does not mention this at all, I tend to find lots of uses for
empty octet strings, so this seems odd.
xpath1.0: are we expected to infer that "context" includes document
and where the namespace prefixes are bound?

Section 4:
ipv6-address: that is a very short IPv6 pattern.  Is it provably
correct?  I only ask because I've had occasion to build a real
pattern, and it's very long (the following is based on the "official"
ABNF definition):

          <xs:pattern value="[0-9A-Fa-f]{1,4}(:[0-9A-Fa-f]{1,4}){7}"/>
          <!-- Double colon start -->
          <xs:pattern value=":(:[0-9A-Fa-f]{1,4}){1,7}"/>
          <!-- Double colon middle -->
          <xs:pattern value="([0-9A-Fa-f]{1,4}:){1,6}
                 (:[0-9A-Fa-f]{1,4}){1}"/>
          <xs:pattern value="([0-9A-Fa-f]{1,4}:){1,5}
                 (:[0-9A-Fa-f]{1,4}){1,2}"/>
          <xs:pattern value="([0-9A-Fa-f]{1,4}:){1,4}
                 (:[0-9A-Fa-f]{1,4}){1,3}"/>
          <xs:pattern value="([0-9A-Fa-f]{1,4}:){1,3}
                 (:[0-9A-Fa-f]{1,4}){1,4}"/>
          <xs:pattern value="([0-9A-Fa-f]{1,4}:){1,2}
                 (:[0-9A-Fa-f]{1,4}){1,5}"/>
          <xs:pattern value="([0-9A-Fa-f]{1,4}:){1}
                 (:[0-9A-Fa-f]{1,4}){1,6}"/>
          <!-- Double colon end -->
          <xs:pattern value="([0-9A-Fa-f]{1,4}:){1,7}:"/>
          <!-- IPv4-Compatible and IPv4-Mapped Addresses -->
          <xs:pattern value="((:(:0{1,4}){0,3}:[fF]{4})|(0{1,4}:
          (:0{1,4}){0,2}:[fF]{4})|((0{1,4}:){2}
          (:0{1,4})?:[fF]{4})|((0{1,4}:){3}:[fF]{4})
          |((0{1,4}:){4}[fF]{4})):(25[0-5]|2[0-4][0-9]|
          [0-1]?[0-9]?[0-9])\.(25[0-5]|2[0-4][0-9]|[0-1]
          ?[0-9]?[0-9])\.(25[0-5]|2[0-4][0-9]|[0-1]?
          [0-9]?[0-9])\.(25[0-5]|2[0-4][0-9]|[0-1]?
          [0-9]?[0-9])"/>

Nits: I didn't find anything at all, and the items that idnits found
were bogus, so it looks good.
_______________________________________________
appsdir mailing list
appsdir@ietf.org
https://www.ietf.org/mailman/listinfo/appsdir