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

[[e-mail]]

Salut Pascal.

Sorry for the late feedback. For the sake of following-up, your
responses and I-D updates address my concerns.

Please consider the following few nits.

A missing coma and a typo in section 4.2.2:
OLD
   This is the reason why the support
   of [RFC8505] by the RUL, as opposed to only [RFC6775] is a
   prerequisite for this specification)/; this requirement is fully
   explained in Section 5.1.
NEW
   This is the reason why the support
   of [RFC8505] by the RUL, as opposed to only [RFC6775], is a
   prerequisite for this specification; this requirement is fully
   explained in Section 5.1.

In section 11: s/the overusing that channel/overusing that channel/  [or
"the overuse of that channel"]

In Table 5 of section 12.6, the allocation for the [EFFICIENT-NPDAO]
work-in-progress should rather be referred to as "to be defined".

Thanks,

Julien


On 18/12/2020 15:11, Pascal Thubert (pthubert) wrote:
> 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
>                |
>             +-----+
>             |     | <------------- 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.
>
>> - 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.
> 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
>> pi..."]: please pick one and be consistent.
> 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.
>
>> ------
>> Section 1.
>> ---
>> - 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". 
>
>> -  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.
> 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.
>
>> ------
>> Section 2.
>> ---
>> - s/Neighbor solicitation/Neighbor Solicitation/
> Done
>
>> - s/Information solicitation (DIS)/Information Solicitation (DIS)/
> Removed
>
>> ------
>> Section 3.
>> ---
>> - s/The RPL Root tunnels the packets/The RPL Root tunnels data packets/
>> [unless we're talking about control packet]
> Done
>
>> - s/forwards the original (inner) packet/forwards original (inner) packets/
> Done
>
>> ------
>> Section 4.
>> ---
>> - 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. 
>
>
>> ------
>> Section 5.
>> ---
>> - 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
>
>> ------
>> Section 6.
>> ---
>> - 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
>
>> ------
>> 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.
> 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
>
>> ------
>> Section 8.
>> ---
>> - Spacing inconsistency on the section title.
> Fixed
>
>> ------
>> Section 9.
>> ---
>> - 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
>
>> ------
>> Section 11.
>> ---
>> - s/supporting th extension/supporting the extension/
> Fixed
>
>> ------
>> Section 12.
>> ---
>> - s/doesn't/does not/
> Fixed
>
>> ------
>>
>>
>> Regards,
>>
>> Julien
>>