Subject: [Roll] RtgDir Review: draft-ietf-roll-unaware-leaves-23

From: <[email-masked]>
Date: Thu, 17 Dec 2020 17:08:36 +0100

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-roll-unaware-leaves-23
Reviewer: Julien Meuric
Review Date: 2020-12-17
IETF LC End Date: 2020-12-09
Intended Status: Standards Track

*Summary:*

I have some minor concerns about this document that I think should be
resolved before publication.

*Comments:*

I am not an LLN expert, but I find the I-D convenient to read thanks to
the appropriate (but numerous) references. I just feel that a few
sections deserve some clarification to improve readability and that flag
number selection doesn't really follow the expected process.

*Major Issues:*

No major issues found.

*Minor Issues:*

- Sorry if ask questions on implicit contexts that are obvious to LLN
people, but I am confused in section 3 with that "6LR that acts as a
border Router for external routes": does it advertise towards the RPL
domain? Does it talk to a peer 6LR that is also a RPL Router? Is that
particular router necessarily the RPL Root?

- The text in section 6.2, as well as figures 3 and 4, use the F and P
flags as if the bit numbers were defined, though their number is only
"suggested" in the IANA section. If no early allocation process
happened, it is inappropriate to assume the to-be-allocated bit number
in the body of the document.

- Section 6.3 reduces an 8-bit field to a 6 bit value. It would be worth
mentioning that it sounds reasonable because the associated registry
relies on standards action for registration and only values up to 10 are
currently allocated.
Furthermore, is there an update of that 6-bit space split to map the
previous ranges? If the answer lies in the IANA section, then a few
words would be welcome in section 6.

*Nits:*
------
Overall
---
- RPL-[An]aware, RAL and RUL are preceded most of the times by "a" [OK
if pronounced "riple", "ral", "rul"], but sometimes by "an" ["are
pi..."]: please pick one and be consistent.
- Any reason why "Host", "Hop" and "Router" are often written with a
capital H/R?
- RFC 2119 keywords SHOULD be used more often for an IETF standard
track, it sometimes feels that the text is written to circumvent them.
E.g., section 4.2.1 uses a wording based on "requires... if and only
if..."; section 4.3 describes a behavior without any normative keyword;
section 5.1 says "needs to", "is expected not to", "is suggested to"; etc.

------
Section 1.
---
- The phrase "terminate packets" feels odd: is "terminate paths" intended?
- s/expectations/requirements/
- The term "change" is used several times in the section summary though
it is a bit loose: depending on the situation, "modify" or "extend"
would be more specific (the former may impact existing implementations,
the latter does not).
-  OLD
      Section 8 presents the changes made to [RFC6775] and [RFC8505];
      The range of the ND status codes is reduced down to 64 values, and
      the remaining bits in the original status field are now reserved.
  NEW
      Section 8 presents how [RFC6775] and [RFC8505] are used; the
      range of the ND status codes is narrowed down to 64 values, and
      the unused bits are reserved.

------
Section 2.
---
- s/Neighbor solicitation/Neighbor Solicitation/
- s/Information solicitation (DIS)/Information Solicitation (DIS)/

------
Section 3.
---
- s/The RPL Root tunnels the packets/The RPL Root tunnels data packets/ 
[unless we're talking about control packet]
- s/forwards the original (inner) packet/forwards original (inner) packets/

------
Section 4.
---
- s/Neighbor solicitation (NS)/Neighbor Solicitation (NS)/
- s/6LN functionality in [RFC8505]/6LN functionality from [RFC8505]/
- Section 4.2.3 summarizes the main use case of the ROVR though its use
here is a bit different: why not focus the paragraph on the latest
sentence and bring a correct topic balance into the section?

------
Section 5.
---
- s/with a CIO/with a 6CIO/
- s/the Root terminates the IP-in-IP tunnel at the parent 6LR/the
IP-in-IP tunnel from the Root terminates at the parent 6LR/

------
Section 6.
---
- s/between the 6LR and the RPL Root/between the RPL Root and the 6LR/
- s/encodes it in one of these reserved flags of the RPL DODAG
configuration option/allocates a new one in the RPL DODAG configuration
option/
- s/values zero (0) to six (6)/values from zero (0) to six (6)/

------
Section 7.
---
- The section title should point to the draft title ("Efficient Route
Invalidation") rather than the draft name which will be replaced by an
RFC number at publication time.
- s/hop by hop/hop-by-hop/

------
Section 8.
---
- Spacing inconsistency on the section title.

------
Section 9.
---
- In section 9.2.2, the capital T is missing on "the" for steps 4 and 5.
- s/Lifetime. e.g./Lifetime. E.g./
- s/6LoWPAN ND related reasons/6LoWPAN ND-related reasons/
- s/An error injecting the route/An error when injecting the route/
- In section 9.2.3, the capital T is missing on "the" for steps 2 and 3.

------
Section 11.
---
- s/supporting th extension/supporting the extension/

------
Section 12.
---
- s/doesn't/does not/
------

Regards,

Julien

_________________________________________________________________________________________________________________________

Ce message et ses pieces jointes peuvent contenir des informations confidentielles ou privilegiees et ne doivent donc
pas etre diffuses, exploites ou copies sans autorisation. Si vous avez recu ce message par erreur, veuillez le signaler
a l'expediteur et le detruire ainsi que les pieces jointes. Les messages electroniques etant susceptibles d'alteration,
Orange decline toute responsabilite si ce message a ete altere, deforme ou falsifie. Merci.

This message and its attachments may contain confidential or privileged information that may be protected by law;
they should not be distributed, used or copied without authorisation.
If you have received this email in error, please notify the sender and delete this message and its attachments.
As emails may be altered, Orange is not liable for messages that have been modified, changed or falsified.
Thank you.

---

From: "Pascal Thubert (pthubert)" <[email-masked]>
Date: Fri, 18 Dec 2020 14:11:46 +0000

Salut Julien:

Many thanks for your review!

I committed the changes that I understood in https://github.com/roll-wg/roll-unaware-leaves/commit/d029d605fe4dbfa2ff1c444210fa67f923b715e4

For your convenience, I also published an update for you to visualize the changes:
Htmlized:       https://tools.ietf.org/html/draft-ietf-roll-unaware-leaves-28
Diff:           https://www.ietf.org/rfcdiff?url2=draft-ietf-roll-unaware-leaves-28

Please let me know if there is more to be done.

For the deep review discussion, please see below.

> *Comments:*
> 
> I am not an LLN expert, but I find the I-D convenient to read thanks to the
> appropriate (but numerous) references. I just feel that a few sections deserve
> some clarification to improve readability and that flag number selection
> doesn't really follow the expected process.

Cool, let's see how we can make things (even 😉) better.

> 
> *Minor Issues:*
> 
> - Sorry if ask questions on implicit contexts that are obvious to LLN people,
> but I am confused in section 3 with that "6LR that acts as a border Router for
> external routes": does it advertise towards the RPL domain? Does it talk to a
> peer 6LR that is also a RPL Router? Is that particular router necessarily the RPL
> Root?

There were similar comments and I already made some changes:
1) added a picture that hopefully helps here

         ------+---------
               |          Internet
               |
            +-----+
            |     | &lt;------------- 6LBR / RPL Root
            +-----+                     ^
               |                        |
         o    o   o  o                  | RPL
     o o   o  o   o  o     o    o       |
    o  o o  o o    o   o  o   o  o      |  +
    o   o      o     o   o   o    o     |
   o  o   o  o   o  o    o    o  o      | 6LoWPAN ND
      o  o  o  o        o   o           |
     o       o            o    o        v
   o      o     o <------------- 6LR / RPL Border router
                                        ^
                                        | 6LoWPAN ND only
                                        v
                u <------------- 6LN / RPL-Unaware Leaf

To clarify more I changed section 3 as follows:
"
3.  RPL External Routes and Dataplane Artifacts

   RPL was initially designed to build stub networks whereby the only
   border router would be the RPL Root (typically collocated with the
   6LBR) and all the nodes in the stub would be RPL-Aware.  [RFC6550]
   has the provision of external routes with the External 'E' flag in
   the Transit Information Option (TIO).  External Routes enable to
   reach destinations that are outside the RPL domain and connected to
   the RPL domain via RPL border routers that are not the route.
   Section 4.1 of [USEofRPLinfo] provides a set of rules summarized
   below that must be followed for routing packets from and to a
   external destinations (targets in RPL parlance).  A RUL is a special
   case of external target that is also a host directly connected to the
   RPL domain.
"

Does that help?

> 
> - The text in section 6.2, as well as figures 3 and 4, use the F and P flags as if
> the bit numbers were defined, though their number is only "suggested" in the
> IANA section. If no early allocation process happened, it is inappropriate to
> assume the to-be-allocated bit number in the body of the document.

At this stage, we already made a pass with Amanda Baber (IANA). 
A change would be pure form to be undone by the RFC editor.

> mentioning that it sounds reasonable because the associated registry relies
> on standards action for registration and only values up to 10 are currently
> allocated.

Good point; done.

> Furthermore, is there an update of that 6-bit space split to map the previous
> ranges? If the answer lies in the IANA section, then a few words would be
> welcome in section 6.

Aïe, I do not understand the question. We used to map 1 to 1 the ND status to RPL status, but after a discussion with Alvaro we found that embedding was better.

> 
> *Nits:*
> ------
> Overall
> ---
> - RPL-[An]aware, RAL and RUL are preceded most of the times by "a" [OK if
> pronounced "riple", "ral", "rul"], but sometimes by "an" ["are

Done. We say a RPL, a RUL, a RAL, and an RPI.

> - Any reason why "Host", "Hop" and "Router" are often written with a capital
> H/R?

Hum I thought I cleaned that up for the most part. Doing another pass I do not find much. 
I use the uppercase on Hop-by-Hop because that is what RFC 8200 does. Same for 6LBR, 6LR, 6BBR, etc... else I used lowercase

> - RFC 2119 keywords SHOULD be used more often for an IETF standard track,
> it sometimes feels that the text is written to circumvent them.
> E.g., section 4.2.1 uses a wording based on "requires... if and only if...";
> section 4.3 describes a behavior without any normative keyword; section 5.1
> says "needs to", "is expected not to", "is suggested to"; etc.

There's more here than meet the eye. Remember that we extend existing work in RPL, ND and IPv6. The reason for the wording is that the MUST or SHOULD is already written in an RFC and we do not want to paraphrase std track text here.

> - The phrase "terminate packets" feels odd: is "terminate paths" intended?

This is gone already

> - s/expectations/requirements/

Done

> - The term "change" is used several times in the section summary though it is
> a bit loose: depending on the situation, "modify" or "extend"

There's both; we actually are more specific in the sections. I found one place where I did a replacement.

> would be more specific (the former may impact existing implementations, the
> latter does not).

Yes this is when we use "update". 

>       the unused bits are reserved.

This was the goal when I wrote RFC 8505. I missed tiny things and had to perform updates which are listed in " Enhancements to RFC 6775 and RFC8505". Because of that this document updates RFC 6775 and 8505. Sadly.

> - s/Neighbor solicitation/Neighbor Solicitation/
Done

> - s/Information solicitation (DIS)/Information Solicitation (DIS)/
Removed

> [unless we're talking about control packet]

Done

> - s/forwards the original (inner) packet/forwards original (inner) packets/

Done

> - s/Neighbor solicitation (NS)/Neighbor Solicitation (NS)/
Done

> - s/6LN functionality in [RFC8505]/6LN functionality from [RFC8505]/
Done though unconvinced. Let's see what the RFC editor ends up doing.

> - Section 4.2.3 summarizes the main use case of the ROVR though its use here
> is a bit different: why not focus the paragraph on the latest sentence and
> bring a correct topic balance into the section?

This section describes RFC 8505. It hints about how we use it in the rest of the spec but that's not the main goal. 

> - s/with a CIO/with a 6CIO/
Done

> - s/the Root terminates the IP-in-IP tunnel at the parent 6LR/the IP-in-IP
> tunnel from the Root terminates at the parent 6LR/
Done

> - s/between the 6LR and the RPL Root/between the RPL Root and the 6LR/
In both directions in fact

> - s/encodes it in one of these reserved flags of the RPL DODAG configuration
> option/allocates a new one in the RPL DODAG configuration option/
That sentence was reworded since -23. But the allocation game is for IANA section. Arguably  what matters to the implementer is the bit position.

> - s/values zero (0) to six (6)/values from zero (0) to six (6)/
Done

> Invalidation") rather than the draft name which will be replaced by an RFC
> number at publication time.
Point is, both drafts will publish together and this is to help the editor to the replacements.

> - s/hop by hop/hop-by-hop/
Done

> - Spacing inconsistency on the section title.
Fixed

> - In section 9.2.2, the capital T is missing on "the" for steps 4 and 5.
Fixed

> - s/Lifetime. e.g./Lifetime. E.g./
Fixed

> - s/6LoWPAN ND related reasons/6LoWPAN ND-related reasons/
Fixed

> - s/An error injecting the route/An error when injecting the route/
Not sure; since several native speakers reviewed it I'd rather leave this as is

> - In section 9.2.3, the capital T is missing on "the" for steps 2 and 3.
Fixed

> - s/supporting th extension/supporting the extension/
Fixed

> - s/doesn't/does not/
Fixed

> ------
> 
> 
> Regards,
> 
> Julien
> 
> 
> 
> _______________________________________________________________
> __________________________________________________________
> 
> Ce message et ses pieces jointes peuvent contenir des informations
> confidentielles ou privilegiees et ne doivent donc pas etre diffuses, exploites
> ou copies sans autorisation. Si vous avez recu ce message par erreur, veuillez
> le signaler a l'expediteur et le detruire ainsi que les pieces jointes. Les
> messages electroniques etant susceptibles d'alteration, Orange decline toute
> responsabilite si ce message a ete altere, deforme ou falsifie. Merci.
> 
> This message and its attachments may contain confidential or privileged
> information that may be protected by law; they should not be distributed,
> used or copied without authorisation.