Skip to content

Commit

Permalink
RtgDir Review: draft-ietf-roll-unaware-leaves-23
Browse files Browse the repository at this point in the history
Salut Julien:

Many thanks for your review!

Let's 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
>
>
>
> _______________________________________________________________
> __________________________________________________________
>
> 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.
  • Loading branch information
pthubert committed Dec 18, 2020
1 parent b584673 commit d029d60
Show file tree
Hide file tree
Showing 2 changed files with 392 additions and 319 deletions.

0 comments on commit d029d60

Please sign in to comment.