Subject: [Softwires] Genart last call review of draft-ietf-softwire-iftunnel-04

From: Dale Worley via Datatracker <[email-masked]>
Date: Sun, 05 May 2019 19:22:06 -0700

Reviewer: Dale Worley
Review result: Ready with Nits

I am the assigned Gen-ART reviewer for this draft. The General Area
Review Team (Gen-ART) reviews all IETF documents being processed
by the IESG for the IETF Chair.  Please treat these comments just
like any other last call comments.

For more information, please see the FAQ at

<https://trac.ietf.org/trac/gen/wiki/GenArtfaq>.

Document:  draft-ietf-softwire-iftunnel-04
Reviewer:  Dale R. Worley
Review Date:  2019-05-05
IETF LC End Date:  2019-05-07
IESG Telechat date:  [not known]

Summary:

       This draft is ready for publication as a Standards Track RFC.

Nits/editorial comments: 

   Editorial Note (To be removed by RFC Editor)

This section is a great idea.  I haven't seen this usage before.

   Please update the "revision" date of the YANG modules.

This sentence doesn't say what to update the revision date to.

   1.  Introduction

   This document specifies the initial version of the iana-tunnel-type
   YANG module identifying tunnel interface types.

This could be made more specific, e.g.,

   This document specifies the initial version of the iana-tunnel-type
   YANG module containing a collection of IANA maintained YANG
   identities identifying tunnel interface types.

--

   2.  IANA Tunnel Type YANG Module

   The initial version of the module includes tunnels types defined in
   [RFC4087], [RFC7856], [RFC7870], and [RFC6346].

s/tunnels types/tunnel types/

Should this mention the provenance of IP-HTTPS, which is in none of
these RFCs?

     identity iphttps {
       base ift:tunnel;
       description
         "IP over HTTPS (IP-HTTPS) Tunneling Protocol.";
       reference
         "Microsoft Corporation, IP over HTTPS (IP-HTTPS) Tunneling
          Protocol Specification,
          https://msdn.microsoft.com/en-us/library/dd358571.aspx";
     }

This type's reference doesn't appear in the list of references.

   3.  Security Considerations

   These identies are intended to be
   referenced by other YANG modules, and by themselves do not expose any
   nodes which are writable, contain read-only state, or RPCs.

Logically, this is correct, but I think it would read better if
s/contain read-only state/contain readable state/.

   4.1.  YANG Module

   The name of the "identity" is the same as the corresponding
   enumeration in the IANAifType-MIB (i.e., IANAtunnelType).

This should be "... is the lower-case of the corresponding enumeration
...".

   "base":        Contains the name assigned to the tunnel type, in
                  lowercase.

The description of this item should be "Contains 'ift:tunnel'.".

   6.2.  Informative References

   [TUNNELTYPE-IANA-REGISTRY]
              Internet Assigned Numbers Authority, "tunnelType
              Definitions", <https://www.iana.org/assignments/smi-
              numbers/smi-numbers.xhtml#smi-numbers-6>.

Given that this document specifies substantial interaction with this
registry, this reference should be normative.

The title of this reference cannot be "tunnelType Definitions",
because that text does not appear in either the referenced URL or the
IANA assigned numbers index (https://www.iana.org/protocols).  The
title of the entire web page is "Structure of Management Information
(SMI) Numbers (MIB Module Registrations)"; the title of the section
that is referenced is "Internet-standard MIB -
mib-2.interface.ifTable.ifEntry.ifType.tunnelType", which is a
subsection of the section titled "ifType definitions".  There is no
reference to the section from the IANA assigned numbers index.

Comparing with this passage from section 4.1

      They must instead be respectively added to the
      "tunnelType" sub-registry (under the "ifType definitions"
      registry).

suggests the title "ifType definitions:  tunnelType" may be in
informal use.

[END]

---

From: <[email-masked]>
Date: Mon, 6 May 2019 07:49:43 +0000

Dear Dale, 

Thank for the review. 

Please see inline.

Cheers,
Med

> -----Message d'origine-----
> De : Dale Worley via Datatracker [mailto:[email-masked]]
> Envoyé : lundi 6 mai 2019 04:22
> À : [email-masked]
> Cc : [email-masked]; [email-masked]; draft-ietf-softwire-
> [email-masked]
> Objet : Genart last call review of draft-ietf-softwire-iftunnel-04
> 
> Reviewer: Dale Worley

[Med] The revision date will be updated with the publication date of the (future) RFC. The RFC editor is used with that. 

>    identities identifying tunnel interface types.

[Med] OK.

[Med] Fixed.

> Should this mention the provenance of IP-HTTPS, which is in none of

[Med] This is already covered by the "reference" clause. 

> This type's reference doesn't appear in the list of references.

[Med] That is on purpose because otherwise that reference will need to be called out in the core text. 

> s/contain read-only state/contain readable state/.

[Med] I will leave the initial wording. 

> The description of this item should be "Contains 'ift:tunnel'.".

[Med] Good catch. Thanks. 

> registry, this reference should be normative.

[Med] We used to have this one as normative but we received comments asking to move it informative. I will leave this one to the AD. 

> suggests the title "ifType definitions:  tunnelType" may be in

[Med] I went for: s/tunnelType Definitions/ifType definitions: tunnelType

---

From: "Eric Vyncke (evyncke)" <[email-masked]>
Date: Fri, 17 May 2019 15:00:27 +0000

Dale, thank you very much for the detailed review of this I-D.

Med, thank you for the reply (and the -05), see below my INT AD input to some remarks/questions 

-éric

EVY> I agree with Med on this one

 
EVY> I agree with Dale on this one. The IESG statement on normative/informative references states: "Normative references specify documents that must be read to understand or implement the technology in the new RFC, or whose technology must be present for the technology in the new RFC to work" and the specific IANA registry is indeed required. Thank Med for having move it to the normative section.