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

From: Martin Thomson <[email-masked]>
Date: Sun, 28 Apr 2013 20:15:45 -0700

(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.