Subject: [RTG-DIR] Rtg Dir review draft-ietf-6lo-rfc6775-update-13.txt

From: "Adrian Farrel" <[email-masked]>
Date: Sat, 24 Feb 2018 18:15:24 -0000

Hello, 

I have been selected as the Routing Directorate reviewer for this draft. The
Routing Directorate seeks to review all routing or routing-related drafts as
they pass through IETF last call and IESG review, and sometimes on special
request. The purpose of the review is to provide assistance to the Routing ADs.
For more information about the Routing Directorate, please see
http://trac.tools.ietf.org/area/rtg/trac/wiki/RtgDir 

Although these comments are primarily for the use of the Routing ADs, it would
be helpful if you could consider them along with any other IETF Last Call
comments that you receive, and strive to resolve them through discussion or by
updating the draft. 

Document: draft-ietf-6lo-rfc6775-update-13.txt
Note: I reviewed -13 (-14 came out half way through my review, but I kept right
on anyway).
Reviewer: Adrian Farrel
Review Date: 24th Feb 2018
IETF LC End Date: 6th Mar 2018
Intended Status: Standards Track 

Summary: 

I have concerns about this document and recommend that the Routing ADs discuss
these issues further with the authors. 
I also a long list of minor concerns and nits that I think should be resolved
before publication. 

Comments: 

The document is quite hard work. More cross-references would help, and bringing
the abbreviations to the top would also make things easier. But the main problem
was the trickle feed of how everything hangs together - it's all there, but you
have to read the entire document to get the whole picture.

==Major==

4.3

   With this specification, the Registration Unique ID is allowed to be
   extended to different types of identifier, as long as the type is
   clearly indicated.  For instance, the type can be a cryptographic
   string and used to prove the ownership of the registration as
   discussed in "Address Protected Neighbor Discovery for Low-power and
   Lossy Networks" [I-D.ietf-6lo-ap-nd].  In order to support the flows
   related to the proof of ownership, this specification introduces new
   status codes "Validation Requested" and "Validation Failed" in the
   EARO.

This seems fine, but I don't see how "the type is clearly indicated".

I think you also have to restate the uniqueness assumption in this new
context. Probably that uniqueness is required across {type, value}
(unless, of course, your answer to my first question is that type is 
embedded in value).  This possibly cuts into backward compatibility as
well?

See also 6.1/6.2

Furthermore, 7.1.2 says...

   A node that supports this specification MUST always use an EARO as a
   replacement to an ARO in its registration to a router.  This is
   harmless since the "T" flag and TID field are reserved in [RFC6775],
   and are ignored by a RFC6775-only router.  A router that supports
   this specification answers an ARO with an ARO and answers an EARO
   with an EARO.

...but this doesn't mention the "variation" of the RUID that might also
arise with the EARO. How will the RFC6775-only router handle these new
RUIDs and their "clearly indicated" types?

---

7.1.2

   One alternate way for a 6LN to discover the router's capabilities is
   to start by registering...

You went to a lot of trouble to define the E-flag. You then made the use
of the 6CIO (and hence the E-flag) only a SHOULD, and you defined an
alternate mechanism. (Note: you say "one alternate" implying there are
more!)

Choice is not good. It complicates the specification and the 
implementation. Why go there? Can't you settle on one mechanism and 
make it necessary and sufficient?

---

You create new registries in 10.1 and 10.2. You must tell the IANA what
allocation policies to use

==Minor==       

You have a number of cases of "SHOULD" and "RECOMMENDED" without
corresponding "MAY" clauses to describe variations. I think that I
could deduce the reasons (implementation decided to not do it) and
the results (function is not as rich), but you really should spell
these things out.

---

Would you consider folding Appendix C into the top of Section 2 so 
that it is possible to find the expansion of abbreviations more
easily?

---

RFC 6775 updates RFC 4944. Does this document also update RFC 4944?

---

I missed a discussion of Manageability and especially diagnosing faults.
It's not mandatory, but it would have been good. (There is a trickle of
info about policy configuration in 7.3).

---

The 2119 boilerplate should be updated to the most recent, viz.

   The key words "MUST", "MUST NOT", "REQUIRED", "SHALL", "SHALL NOT",
   "SHOULD", "SHOULD NOT", "RECOMMENDED", "NOT RECOMMENDED", "MAY", and
   "OPTIONAL" in this document are to be interpreted as described in BCP
   14 [RFC2119] [RFC8174] when, and only when, they appear in all
   capitals, as shown here.

...which will require you add 8174 as a reference

---

Sect 2 expects the reader to be familiar with terms and concepts in a
long list of other documents. Doesn't that make them normative refs?

[RFC7102]
[RFC7228]
[RFC4861]
[RFC4862]
[RFC6606]
[RFC4919]
[RFC6775]
[I-D.ietf-ipv6-multilink-subnets]

---

Sect 2 has

   Extended LLN:  The aggregation of multiple LLNs as defined in
         [RFC4919], interconnected by a Backbone Link via Backbone
         Routers, and forming a single IPv6 MultiLink Subnet.

I'm not super-familiar with 4919, but a quick scan did not make it
obvious what you are referencing in that document.  "LLN" is not
mentioned. Nor is "aggregation" or "interconnect" (in this context).
There is some mention in 4.2 of "seamless integration": is that what
you are referencing?

---

Sect 2 has

   Registration:  The process during which a 6LN registers its
         address(es) with the Border Router so the 6BBR can serve as
         proxy for ND operations over the Backbone.

Do you mean s/Border/Backbone/ ?
Otherwise it seems strange that the 6LN registers with the 6LBR so that
the 6BBR can do something.

---

Sect 2

   Binding:  The association between an IP address with a MAC address, a
         port and/or other information about the node that owns the IP
         address.

This was hard to parse. Possibly you mean...

   Binding:  The association between an IP address, a MAC address, a
         port, and other information about the node that owns the IP
         address.

---

Sect 2

   Registering Node:  The node that performs the registration to the
         6BBR, which may proxy for the registered node.

Confusing!
The 6BBR is defined as the "proxy for the registration".
Now we appear to have:
- a node that needs to be registered
- a node that does the registration to the 6BBR
- the 6BBR (the proxy)
The final subclause of your text ("which may proxy for the registered
node") could be applied to the node that performs the registration or
to the 6BBR.

---

4.1

   The Extended ARO (EARO) deprecates the ARO and is backward compatible
   with it.  More details on backward compatibility can be found in
   Section 7.

   The semantics of the ARO are modified as follows:

Given the "deprecates" statement, you probably want...

   The semantics of the EARO are identical to the ARO with the following
   modifications:

---

4.2

   The Transaction ID (TID) is a sequence number that is incremented
   with each re-registration.

Who increments?

---

4.2

   The TID may also be used by the
   network to track the sequence of movements of a node in order to
   route to the current (freshest known) location of a moving node.

You don't need to track the sequence of movements in order to identify
the freshest known location. You only have to spot the most recent TID.
This makes a big difference to implementations: is there a need to track
the sequence or can an implementation just look for the most recent TID?

---

4.7

   If the registry in the 6LBR is saturated, then the LBR cannot decide
   whether a new address is a duplicate.  In that case, the 6LBR replies
   to a EDAR message with a EDAC message that carries a new Status Code
   indicating "6LBR Registry saturated" Table 1.  Note: this code is
   used by 6LBRs instead of Status 2 when responding to a Duplicate
   Address message exchange and passed on to the Registering Node by the
   6LR.  There is no point for the node to retry this registration
   immediately via another 6LR, since the problem is global to the
   network.  The node may either abandon that address, de-register other
   addresses first to make room, or keep the address in TENTATIVE state
   and retry later.

Three points:

"cannot" seems strong since the first occurrence of the duplicate might
already be in the registry.

de-registering an address to make room seems a risky business since some
other 6LR might grab the space.

Shouldn't the actions be at the 6LBR
- to notify the operator
- to clear out "old" entries from the registry (although that may require
  magic beyond housekeeping after Registration Lifetime expiration: 
  perhaps it could curtail the Delay state?)

---

5.

   The 6CIO is typically sent in a Router Solicitation (RS) message.
   When used to signal capabilities per this specification, the 6CIO is
   typically present in Router Advertisement (RA) messages but can also
   be present in RS, Neighbor Solicitation (NS) and Neighbor
   Advertisement (NA) messages.

I didn't find the two uses of "typically" gave me confidence to know
what to code.

---

6.1

   |   3   | Moved: The registration failed because it is not the      |
   |       | freshest.  This Status indicates that the registration is |
   |       | rejected because another more recent registration was     |
   |       | done, as indicated by a same RUID and a more recent TID.  |
   |       | One possible cause is a stale registration that has       |
   |       | progressed slowly in the network and was passed by a more |
   |       | recent one.  It could also indicate a RUID collision.     |

Do you think "Moved" is the best name to cover all of these 
possibilities?

But, anyway, how can you have a RUID collision by definition of a RUID?

---

6.1

                   The node SHOULD maintain the TID in a persistent
                   storage.

Unfortunate to push persistent storage requirements. But, since this is
a SHOULD, all processes are in place to support not storing across 
restarts. So why make anyone do it? Surely you could fall back to the
default handling without persistent storage.

---

6.2 

   Code:           The ICMP Code as defined in [RFC4443].  The ICMP Code
                   MUST be set to 1 with this specification.  An odd
                   value of the ICMP Code indicates that the TID field  
                   is present and obeys this specification.             

You're overloading the ICMP Code in an ugly way. But you knew that :-)
It would probably be more normal to steal the top bit so that allocation
of new codes can continue more normally. But failing that, I think you 
would be wise to make it so the IANA registry clearly shows that the odd
values must not be assigned in future (section 10.2).

---

It is not clear whether anything in App A and B is intended to be normative.
A clear statement would be helpful.

==Nits==

idnits shows three problems

 ** There is 1 instance of too long lines in the document, the longest one
     being 3 characters in excess of 72.

  == Missing Reference: 'Perlman83' is mentioned on line 1392, but not defined

  == Missing Reference: 'IEEEstd802154' is mentioned on line 1615, but not
     defined

The references are caused by you having a third references section. I've not
seen that before. Why not merge the sections as normal?

---

Document header

I suspect "cisco" should have a capital "C"

---

I think the document title should spell out "Neighbor Discovery" and
"IPv6 over Low-Power Wireless Personal Area Networks"

---

Sect 1

s/an IPv6 Low Power Networks/any IPv6 Low Power Network/

---

Sect 1

   to enable additional capabilities and enhancements such as:

s/such as/including/

---

You use "NS" in section 4, but don't expand it until 4.1

---

4.4 could very usefully point at 6.2 to help the reader parse the text.

---

Shouldn't the figure in 6.2 show the optional ND options as described in
4.4?

---

4.7 has "LBR" which should be "6LBR"

---

6.1

s/SLLAO option/SLLA option/ (twice)
s/The EARO option/The EARO/  (three times)

---

6.3

   This specification defines new capability bits for use in the 6CIO,
   which was introduced by [RFC7400] for use in IPv6 ND RA messages.

Say "...defines four new..." to save me having to work out which bits
are new.

---

6.3
   Routers that support this specification MUST set the "E" flag and 6LN
   SHOULD favor 6LR routers that support this specification over those
   that do not. 

s/6LR routers that support/a 6LR that supports/ 

---

8.

   This specification extends [RFC6775], and the security section of
   that standard also applies to this as well.

Don't think 6775 is an Internet Standard.
Suggest s/standard/document/.

---

From: "Pascal Thubert (pthubert)" <[email-masked]>
Date: Mon, 26 Feb 2018 17:52:59 +0000

Hello Adrian:

Thanks a  bunch for your review!

Let us address the major concerns first. Please see below:
> 
> Comments:
> 
> The document is quite hard work. More cross-references would help, and
> bringing the abbreviations to the top would also make things easier. But the
> main problem was the trickle feed of how everything hangs together - it's all
> there, but you have to read the entire document to get the whole picture.
[PT>] 
[PT>] will do.

> This seems fine, but I don't see how "the type is clearly indicated".

[PT>] AP -ND  https://tools.ietf.org/html/draft-ietf-6lo-ap-nd-05 introduce a new flag to indicate its form of RUID. Other may come in the future. It is important that it does signal the type because that triggers the validation flow. 

> context. Probably that uniqueness is required across {type, value} (unless, of
> course, your answer to my first question is that type is embedded in value).
> This possibly cuts into backward compatibility as well?
> 
[PT>] A pure RFC 6775 6LR/6LBR will not need to know the difference, since it cannot validate a thing anyway. It will figure that the field is an EUI 64. As you point out, the additional risk vs. an implementation of the update is that the name spaces are not differentiated. So there is a (very^64 remote) risk that a crypto token ends up with the same value as an EUI 64 in the network; for that to have an impact, it would take that 2 nodes register a same address. Note that the Ap ND token is not expected to be used a IID. In other words, for the use that we make of the RUID, there is no real issue that an older implementation confuses the name spaces. Note that AP-ND may have 2 devices form the same RUID from 2 different keys. Then again, very bad luck. Still, not real damage even if that happens unless it is done willingly. DFrom the AP-ND perspective, that is address protection, one node could control the other's address but it does not even know they exist.

> ...but this doesn't mention the "variation" of the RUID that might also arise
> with the EARO. How will the RFC6775-only router handle these new RUIDs
> and their "clearly indicated" types?
[PT>] 
[PT>] This is defined in the specs that provide those variations; in the case of AP ND there is a proof of ownership challenge of the RUID. You own the RUID if you have the key, and then you are entitled to register and use the address... A router that supports the RFC 6775 update and not any of the coming specs cannot process the RUID differently than a RFC6775-only router. Which is OK as long as the extensions are backwards compatible, IOW the RUID can be used as a unique token.

> You went to a lot of trouble to define the E-flag. You then made the use of the
> 6CIO (and hence the E-flag) only a SHOULD, and you defined an alternate
> mechanism. (Note: you say "one alternate" implying there are
> more!)
> 
> Choice is not good. It complicates the specification and the implementation.
> Why go there? Can't you settle on one mechanism and make it necessary and
> sufficient?
> 
[PT>] We could live without the 6CIO. The draft does not seem to be very much used and for backward compatibility we cannot depend on it.
 Still I think it is a good idea to expose capabilities and then start.  This draft encourages to start using it more now.
Unsure what to do here, I need to take the pulse of the WG.

> allocation policies to use
[PT>] Yes, it is missing in 10.2 though present in 10.1. I do not see a reason to make them different, so that would be:
' The policy is "IETF Review" or "IESG Approval" [RFC8126]'. 

Take care,

Pascal