Subject: Re: [Softwires] Last Call: <draft-ietf-softwire-yang-06.txt> (YANG Modules for IPv4-in-IPv6 Address plus Port Softwires) to Proposed Standard

From: tom petch <[email-masked]>
Date: Mon, 1 Oct 2018 11:26:59 +0000

Some more thoughts on this I-D

No mention of NMDA - I see the IESG asking for such a statement in
Abstract and in the body of an I-D

Abbreviations are expanded but on the nth use, not the first use e.g.
BR, PSID; they probably should be expanded on first use within the YANG
module as well.

'   Please update the "revision" date of the YANG module.'  There are
three of them:-)

Terminology is problematic especially as it seems inconsistent with the
Normatively Referenced RFC7596, RFC7597, RFC7599.

 Customer Premises Equipment (CEs ..
CE is a well known abbreviations for Customer Edge, as oppposed to
Provider Edge, and this is not meant here.   Indeed, RFC7599 uses CE for
Customer Edge.  Customer Premises Equipment is widely abbreviated to
CPE.  RFC7596, a  Normative Reference, has 'Customer Premises Equipment
(CPE)' which I should be used here.

In places, it is 'MAP-E, and MAP-T', elsewhere 'MAP-E or MAP-T'. Does
feature 'algorithm' mean both are supported or just one, and if one, how
can the user tell?

The description clause of 'module ietf-softwire-common' is misleading.
The introductory sentence of the section accurately describes the module
as common definitions but the description clause claims to configure
Lw4o6, MAP-E and MAP-T which it seems wrong.

'algorithm' is widely (mis?)used in this I-D.  The Normative Reference
RFC7597 is much easier to follow since it mostly talks of 'Mapping
Algorithm' or 'Mapping Rule'.  I think
      case algorithm {
        if-feature algorithm;
        container algo-instances {
          list algo-instance {
with
      grouping algorithm-instance {
in softwire-common and
      case algorithm {
        if-feature algorithm;
        container algorithm {
          if-feature algorithm;
need a different term or terms.  Likewise
      case binding {
        if-feature binding;
        container binding {
          if-feature binding;
          list bind-instance {
for binding.  A widely used, and helpful convention is to have a list
the plural - interfaces - and entries singular - interface; that would
help here.  And what does
          if-feature algorithm;
add that
        if-feature algorithm;
does not?

BR is a well known abbreviation for Border Router; here it used for MAP
Border Relay and while RFC7599 says 'A MAP BR may also be referred to as
simply a "BR" within the context of MAP.', I think that the context here
is wider - the modules are not just MAP - and the term should be 'MAP
BR' not just 'BR'.

After my previous message
ietfa.amsl.com.
gave me a bounce message for
[email-masked]>

Overall, I get a slight flavour that this is written by those intimately
acquainted with the technology (although not so much with the RFC!) for
similar readers.

Tom Petch

----- Original Message -----
From: "tom petch" <[email-masked]>
Sent: Friday, September 28, 2018 5:12 PM

> I believe that this I-D needs some admin-type changes to make it
usable.
>
> All three modules import some or all of
>
> ietf-inet-types
> ietf-interfaces
> iana-if-type
> ietf-softwire-common
>
> These imports should have YANG reference statements identifying the
> relevant RFC, probably
>   6991
>   8343
>   7244
>   XXXX
>
> and these need to be Normative References for the I-D; 8343 is, 6991
is
> not.
>
> The first two modules have a sentence mentioning the use of RFC6991;
> this should mention all the modules referenced, those above and
> RFC7596
> RFC7597
> RFC7599:
> these last are already Normative References.  A similar sentence is
> needed for the third module for the RFC that it references.
>
> The third module is a bit light on references - I cannot see any!
>
> There are three references to RFC XXX- I suspect that RFC XXXX is
> intended.
>
> IANA Considerations references RFC7950 - this is a poor reference
since
> all it says is 'Go look at RFC6020' which thus should be the reference
> here.
>
> Security Considerations starts "The YANG module defined in this
document
> ...
> Give us a clue - there are three of them:-)
>
> Appendix A.  Configutation Examples
>
> Tom Petch
>
> ----- Original Message -----
> From: "The IESG" <[email-masked]>
> To: "IETF-Announce" <[email-masked]>
> Cc: <[email-masked]>; <[email-masked]>;
> <[email-masked]>; <[email-masked]>;
> <[email-masked]>
> Sent: Thursday, September 27, 2018 1:06 PM
>
> >
> > The IESG has received a request from the Softwires WG (softwire) to
> consider
> > the following document: - 'YANG Modules for IPv4-in-IPv6 Address
plus
> Port
> > Softwires'
> >   <draft-ietf-softwire-yang-06.txt> as Proposed Standard
> >
> > The IESG plans to make a decision in the next few weeks, and
solicits
> final
> > comments on this action. Please send substantive comments to the
> > [email-masked] mailing lists by 2018-10-11. Exceptionally, comments
may
> be
> > sent to [email-masked] instead. In either case, please retain the
> beginning of
> > the Subject line to allow automated sorting.
> >
> > Abstract
> >
> >    This document defines YANG modules for the configuration and
> >    operation of IPv4-in-IPv6 softwire Border Relays and Customer
> >    Premises Equipment for the Lightweight 4over6, MAP-E, and MAP-T
> >    softwire mechanisms.
> >
> > Editorial Note (To be removed by RFC Editor)
> >
> >    Please update these statements within this document with the RFC
> >    number to be assigned to this document:
> >
> >    o  "This version of this YANG module is part of RFC XXXX;"
> >
> >    o  "RFC XXXX: YANG Modules for IPv4-in-IPv6 Address plus Port
> >       Softwires";
> >
> >    o  "reference: RFC XXXX"
> >
> >    Please update the "revision" date of the YANG module.
> >
> > The file can be obtained via
> > https://datatracker.ietf.org/doc/draft-ietf-softwire-yang/
> >
> > IESG discussion can be tracked via
> > https://datatracker.ietf.org/doc/draft-ietf-softwire-yang/ballot/
> >
> > No IPR declarations have been submitted directly on this I-D.

---

From: tom petch <[email-masked]>
Date: Wed, 3 Oct 2018 11:56:37 +0000

And some technical comments on this I-D as a result of which, I do not
think this is ready to progress; perhaps no show stoppers, just a lot of
changes are needed IMHO.  The more I look into this I-D, the less I
understand (which may be my ignorance of YANG).

This I-D is concerned with three tunnel types (Lw4o6, MAP-E, MAP-T).  In
several places, you have
  augment "/if:interfaces/if:interface" {
    when "if:type = 'ianaift:tunnel'";
This will augment for all tunnel types, not just those of this I-D.  I
think you should have your own three (?) derived identities for the
three specific tunnels described here, all in the common module.

The three YANG modules are for Customer Premises Equipment, Border Relay
and a common module. Both the first two define two features, binding and
algorithm, binding for Lw4o6 and algorithm for MAP-E/MAP-T - I do not
know if/how this duplication of features will work (and as I said
before, I am confused about support for 'MAP-E and MAP-T' versus 'MAP-E
or
MAP-T' both of which phrases appear in the I-D).  As with tunnel types,
should
there be three features, should they be in the common module?  (yes, and
yes)

2.2
'they should be imported here, as needed. '
import is a YANG technical term as used in this I-D so I think the use
of it here wrong

'   The CE must already have minimal IPv6 configuration'
Could that also be IPv4 which, by definition, is going to be widespread
in the deployment?

3.2
softwire-path-mru:
what is the point of this field?  I looked at RFC7596, RFC7597 and
RFC7599
and could find no reference thereto.  I went back to their references,
such as RFC4821, RFC2473, RFC1981, RFC6346 ... - no joy.  I had thought
to suggest a reference was called for but seeing the complete absence of
any connection, I think that a substantial explanation is called for.

Likewise, I note that references to MTU are qualified with IPv4 but
references to MRU are not; rather, this object
'set the maximum IPv6 softwire packet size that can be received ...'
when
'the implementation is unable to calculate the correct IPv4  size  '
Really?

'IPv6 prefix type' or ''IPv6 address type'.
Well, no It can be
type  ipv6-prefix or ipv6-address

This description of the  ietf-softwire-ce module describes objects that
are not in the  ietf-softwire-ce module, which confuses me.  Rather they
are in the ietf-softwire-common module.  I think you need a description
of the objects in the ietf-softwire-common module first and then moving
on to the two specific modules.  The sense I get is that while splitting
into three makes sense, the consequences have not been thought through.

The descriptions of objects here are (mostly) good, but do not appear in
the YANG module.  Those in the YANG module are shallow by comparison and
should be at least as comprehensive as those in the body of the I-D; the
YANG module has to be able to survive on its own.

'The version number is  incremented  '
Any constraints on the increment?  one, a hundred, a million...?

4.2
As with 3.2, the descriptions here of the objects look fine, those in
the description clauses of the YANG module do not; they are thin by
comparison.

5
leaf br-ipv6-addr {
     mandatory true;
        "The IPv6 address for of the binding BR.";
This is not quite English.
And since this object is MAP-E only, should it be, can it be, mandatory?

              leaf id {
              type uint32;
              mandatory true;
              description "Algorithm Instance ID";
Any constraints on how this 32-bit integer will be used?

6

leaf softwire-num-max {  type uint32;
             description
                "The maximum number of softwires that can be created on
                the binding BR.";
Any restriction on the range of this. Can it be zero?

/ "Enables the generation of ICMPv6 errors messages/
  "Enables the generation of ICMPv6 error messages/

                leaf icmpv4-rate {
                leaf icmpv6-rate {
Can these validly be zero?  Also, I think that there should be a
recommended value (the Transport Area are keen on such things)

            leaf id {
              type uint32;
              mandatory true;
              description "id";
Not the most helpful of descriptions

'This must be  initialized when the BR instance is configured'
Perhaps
"This must be reset to the current date-and-time when the BR instance is
configured"

'Notify the client that a specific binding entry has been
expired/invalid.
Perhaps
'Notify the client that a specific binding entry has expired or is
invalid.'

      leaf date {
        type yang:date-and-time;
        description "Timestamp to the algorithm";
Not a helpful description IMHO

    leaf name {
      type string;
      description "The name for the instance.";
ditto; what is the namespace, how is the name used?

"Embedded Address (EA) bits are the IPv4 EA-bits in the IPv6
        address identify an IPv4 prefix/address (or part thereof) or
        a shared IPv4 address (or part thereof) and a port-set
        identifier.
This is not quite English

8
The IESG like to see TLS1.3 RFC8446 here

 I have yet to review the examples, but if my suggestion above result in
changes, then the examples will change significantly.

Tom Petch

---

From: <[email-masked]>
Date: Mon, 22 Oct 2018 13:58:07 +0000

Re-,

Please see inline. 

Cheers,
Med

> -----Message d'origine-----
> De : tom petch [mailto:[email-masked]]
> Envoyé : lundi 1 octobre 2018 13:27
> À : [email-masked]
> Cc : [email-masked]; [email-masked]; draft-ietf-softwire-
> [email-masked]; [email-masked]
> Objet : Re: Last Call: <draft-ietf-softwire-yang-06.txt> (YANG Modules for
> IPv4-in-IPv6 Address plus Port Softwires) to Proposed Standard
> 

[Med] Added to the introduction:

   The adopts the Network Management Datastore Architecture (NMDA)
   [RFC8342].

> BR, PSID; they probably should be expanded on first use within the YANG

[Med] Will double check this. 

> '   Please update the "revision" date of the YANG module.'  There are

[Med] OK.

> (CPE)' which I should be used here.

[Med] Added: (a.k.a., CPE). 

Actually, we just went with CE used in RFC7599. 

> feature 'algorithm' mean both are supported or just one, and if one, how

[Med] Feature 'algorithm' means 'MAP-E and/or MAP-T' is supported. Nevertheless, when it comes to actual configuration, only MAP-E or MAP-T parameters can be conveyed. This is hinted by the data-plane clause. 

> Lw4o6, MAP-E and MAP-T which it seems wrong.

[Med] Fixed. 

> Algorithm' or 'Mapping Rule'.

[Med] Added: 

For simplicity, "algorithm" is used to refer to "mapping algorithm" [RFC7599].

  I think
> need a different term or terms.  

[Med] OK. Went for:

       case algo {
         if-feature algorithm-mode;
         container algo-instances {
           list algo-instances {

Likewise
> the plural - interfaces - and entries singular - interface; that would

[Med] Went for list bind-instances.

> does not?

[Med] Removed one if-feature statement. 

> is wider - the modules are not just MAP - and the term should be 'MAP

[Med] Added:

   The document uses BR to refer to MAP BR [RFC7599] or Lightweight
   4over6 BR [RFC7596].

---

From: <[email-masked]>
Date: Mon, 22 Oct 2018 13:57:06 +0000

Hi Tom,

Thank you for the review. 

Please see inline. 

Cheers,
Med

> Envoyé : vendredi 28 septembre 2018 18:13
> À : [email-masked]
> Cc : [email-masked]; [email-masked]; [email-masked];
> [email-masked]; [email-masked]
> I believe that this I-D needs some admin-type changes to make it usable.
> 

[Med] Added reference statements for those. 

> 
> and these need to be Normative References for the I-D; 8343 is, 6991 is
> not.
> 

[Med] Done. 

> There are three references to RFC XXX- I suspect that RFC XXXX is

[Med] Yes. 

> IANA Considerations references RFC7950 - this is a poor reference since

[Med] OK.

> 
> Security Considerations starts "The YANG module defined in this document
> ...

[Med] Fixed. 

> > the following document: - 'YANG Modules for IPv4-in-IPv6 Address plus
> Port
> > Softwires'
> > The IESG plans to make a decision in the next few weeks, and solicits
> final
> > [email-masked] mailing lists by 2018-10-11. Exceptionally, comments may
> be

---

From: <[email-masked]>
Date: Mon, 22 Oct 2018 14:49:11 +0000

Re-,

Please see inline. 

Cheers,
Med

> Envoyé : mercredi 3 octobre 2018 13:57
> À : tom petch; ietf
> [email-masked]

[Med] Good point. A new parameter called tunnel-type is now added to the interface YANG module (augments 8343). 

> there be three features, should they be in the common module?  (yes, and

[Med] The feature indicates the support of MAP-E and/or MAP-T. the data plane clause is supposed to hint whether this is about MAP-E (encapsulation) or MAP-T (transaltion). I do agree that defining features for each of these two may be more simple. 

[Med] Fixed. 

> Could that also be IPv4 which, by definition, is going to be widespread

[Med] A+P mechanisms assumes that no IPv4 configuration is provided because A+P is meant to provide IPv4 service continuity over IPv6. 

> any connection, I think that a substantial explanation is called for.

[Med] I completely lost the context why the document ended to have this in. I'm sure Ian has a record about this. 

> type  ipv6-prefix or ipv6-address

[Med] Yeah!

> into three makes sense, the consequences have not been thought through.

[Med] Not sure which part you are referring to. Can you please explicit your comment? Thank you.

> Any constraints on the increment?  one, a hundred, a million...?

[Med] This is implementation-specific. As indicated in this example, a timestamp would be sufficient. 

[Med] Fixed. 

> And since this object is MAP-E only, should it be, can it be, mandatory?

[Med] this is of lw4over6. Hence, the mandatory clause. 

> Any constraints on how this 32-bit integer will be used?

[Med] No, there is no constraint defined in existing RFCs. 

> Any restriction on the range of this. Can it be zero?

[Med] No restriction is set because we don't have such recommendation in softwire RFCs. Setting it to zero is equivalent to disabling the instance. 

>   "Enables the generation of ICMPv6 error messages/
[Med] Fixed. Thank you. 

> recommended value (the Transport Area are keen on such things)

[Med] We'd love to add a reco here, but absent any authoritative RFC, we are avoiding to add it. 

> Not the most helpful of descriptions

[Med] Yes, indeed. 

> "This must be reset to the current date-and-time when the BR instance is

[Med] This is better. 

[Med] Fixed. 

> Not a helpful description IMHO

[Med] Updated the text. 

> ditto; what is the namespace, how is the name used?

[Med] This is used to uniquely distinguish instances by their alias/name.  

> This is not quite English

[Med] s/identify/identifying. 

[Med] Done. 

> changes, then the examples will change significantly.

[Med] Many thanks for the detailed review .

---

From: tom petch <[email-masked]>
Date: Fri, 26 Oct 2018 11:40:41 +0000

Terry

Since the IETF Last Call on this I-D ended, there have been five new
versions of this I-D incorporating, IMHO, substantive changes to the
I-D, e.g. introducing a new YANG module for tunnel types, IETF-wide..

I think that the changes are good (although I have not had time to
digest them all yet) but since there are so many, I think that this I-D
can no longer be regarded as the one that the softwires WG forwarded for
publication.  Rather, I think that the I-D should be returned to the WG
to confirm that this is still what the WG wants.  It is not that I would
expect the WG to want to make further substantive changes, rather that
the IETF needs to know that this is what the softwire WG wants, and at
present, I do not think that we can be sure of that.

Tom Petch

----- Original Message -----
From: <[email-masked]>
To: "tom petch" <[email-masked]>; "ietf" <[email-masked]>
Cc: <[email-masked]>; <[email-masked]>;
<[email-masked]>; <[email-masked]>
Sent: Monday, October 22, 2018 3:49 PM
Subject: RE: Last Call: <draft-ietf-softwire-yang-06.txt> (YANG Modules
for IPv4-in-IPv6 Address plus Port Softwires) to Proposed Standard