Re: [Roll] Review of draft-ietf-roll-enrollment-priority-04

[Rahul Jadhav <[e-mail]>]

Excellent review. Many thanks Konrad.

Please find my comments inline.

Thanks,
Rahul

On Thu, 5 Aug 2021 at 02:02, Konrad Iwanicki <[e-mail]> wrote:

> Dear authors,
>
> This is the first review I am doing for IETF's draft, so any suggestions
> on how I could improve the quality of my future reviews are welcome.
>

[RJ] This is an excellent in-depth review. Appreciate.

>
> I have the following major issues with the draft. Perhaps some of them
> have been covered during some meetings (sorry for missing those); my
> comments are based just on what I got from the draft (and the related
> drafts).
>
>
> 1. I was confused by two seemingly contradictory statements.
>
> Par. 2 of sect. 2 (starting with "With this specification") states that
> the introduced option "MUST be propagated down the DODAG without
> modification" (compared to what is issued by the DODAG root).
>
> However, par. 2 of sect. 2.1 (starting with "6LRs that support this
> option") states that "nodes adjust this base value based upon their
> observed congestion, emitting their adjusted DIO value to their children."
>

[RJ] This is a good point. Will raise this as an issue on GH. The way I see
it, the value adjustment might have to be done going downstream and I would
like to see what others think of it. Nonetheless, the text will need
modifications.

>
> What I understand is that the adjustment applies only in a particular
> case. More specifically, if a node receives the option, it copies it to
> its DIOs without any modification. In contrast, if the node does not
> receive the option but is capable of processing it, then it synthesizes
> the option for its DIOs by taking as the min priority the base value
> (0x40) adjusted with the node's local observations. This seems
> inconsistent, though. In particular, two children with the same parent
> can emit DIOs with different values of min priority. Why not simply omit
> the option in such a case?
>

[RJ] Two children with the same parent are permitted to emit DIOs with
different values of min priority. Consider the case where child1 has fewer
resources than child2 and wants to limit downstream nodes joining through
it. This case requires different min-priority to be emitted. There are
several other reasons why the priorities could be different.

>
>
> 2. The draft implicitly assumes a DIO processing policy that differs
> from RPL's original one.
>
> More specifically, throughout the draft, it is implied or assumed that a
> node processes the option received only from its parent(s). For example:
> - par. 8 of sect. 1 (starting with "A network operator might"): "... it
> would like to adjust the enrollment priority (the proxy priority field
> of [...]) among all nodes in the subtree of a congested link.";

- par. 1 of sect. 2.1 (starting with "A 6LR which did not support"):
> "Children and grandchildren nodes would therefore not receive any
> telemetry via that path and need to assume a default value.";
> - par. 2 of sect. 2.1: "6LRs that support this option, but whose parent
> does not send it [...]" and the rest of the paragraph as well.


> In contrast, RFC 6550, sect. 8.2.3 states that DIOs not only from
> parents but also other neighbors must be processed by a node. This
> guarantees correct route formation and maintenance.
>
> Therefore, if the draft changes this policy with respect to the
> introduced option, I would expect it to state this change explicitly.


[RJ] I don't think the draft changes this policy.
RFC 6550 says that the DIO will be received by a node from all its
neighbors and that is because of the broadcast nature of the DIOs. When
DIOs are received from the neighbors, these neighbors could be promoted to
the parent set. The draft does not change this processing behavior. Par 2
of sect 2.1 "6LRs that support this option, but whose parent does not send
it...." this statement does not change the 6550 behavior. All it says is
for some reason, if the selected parent does not support the option then
the draft dictates a particular behavior.


> Such a change would also require answering several questions, notably:
> - From which parent do we process the option?
> - If only from the preferred parent, what happens if a node has none?
> - If from any, then what happens if a node gets different values of min
> priority or DODAG_Size from different parents?
>

[RJ] If we agree that we are not changing 6550 DIO processing behavior as
mentioned above, then these questions do not apply.

>
> However, I believe that a meta question worth answering first is: Do we
> want to have different min priority values in different subtrees or just
> one global value for the entire DODAG?
>

[RJ] This is the point to be discussed. IMO, the min priority is not a
constant global value. However, the draft text contradicts this and will
raise the issue in GH before we fix it to garner some discussion.

>
> Either of the answers in my view requires explicit formulation and
> preferably also an explanation in the draft.
>
>
> 3. Even if the draft identified a single parent as the only source of
> min priority values for a node, there still would be a problem of value
> consistency.
>
> Since the DODAG root can change its values of min priority and
> DODAG_Size and since a node can change its parents, there may still be a
> case when the node must decide whether to adopt the newly received
> values or not. Without a proper conflict resolution policy, it may be
> the case that when the root, for example, disables enrollment, then
> enables it again, then the DODAG may exhibit strange behaviors: the
> root's decision may not be propagated to all nodes, a decision may be
> revoked by nodes that have already accepted it, and the like.
>

[RJ] The exact conflict resolution policy is not within the scope of this
document. However, for the scenario you quoted, "What happens if the
enrollment is enabled/disabled by the root?" We do not expect the
enrollment enable/disable to be precisely known by all others i.e., there
could be nodes who do not have the latest status of the enrollment option.
This could cause some additional signaling i.e., a node might try to join
even though the (latest) priority did not allow it. This could lead to a
few more messaging but it should not result in any untoward behavior.



>
> There are many ways in which such conflict resolution can be provided.
> If I may suggest anything, I would recommend adding to the option a
> lollipop sequence counter (like those described in RFC 6550, sect. 7.2),
> let's call it osn. It would effectively order different versions of the
> option values produced by the DODAG root. In effect, whenever a node
> received an option (be it from a parent or another neighbor, depending
> on the way the previous issues are addressed), it would adopt the min
> priority and DODAG_Size values the option carries if and only if the osn
> value in the received option is greater than the node's locally stored
> osn. This would guarantee eventual consistency and avoid strange
> behaviors such as those mentioned above.
>

[RJ] With LLNs there is no way to have guaranteed consistency. Consider the
case where a node goes off the network and comes back later and did not see
part of the traffic at all.
Also, note that there is other non-static information such as
metrics/constraints that are carried by the DIO. Worst case if the
information changes too quickly and the nodes might not have consistent
information there could be some additionally messaging but over time the
network should converge to the right configuration. I am not sure if
introducing something like OSN is the way to tackle this problem.


>
>
> 4. Why put DODAG_Size in the option?
>

[RJ] This was a recent proposition. Please check this discussion for
context, "[Roll] About measure DODAG size and priority
<https://mailarchive.ietf.org/arch/msg/roll/-MqqxvTW-3bM5F4k4qH7n5FaxYM/>"
https://mailarchive.ietf.org/arch/msg/roll/-MqqxvTW-3bM5F4k4qH7n5FaxYM/


> I can imagine that other services/protocols would like to use it as
> well, and extracting this information from an option related solely to
> enrollment does not seem the best idea. I have been thinking for some
> time about an option for RPL that would provide some aggregate
> information about the DODAG to the nodes, such as its size, max depth,
> and the like. The option could also be used in the enrollment process.
> Do you think this makes sense?
>

[RJ] size is already there. For max-depth, what could be the use-case? Is
there anything else you have in mind? Would be nice to discuss it now.



>
>
> 5. It should be made clearer in the draft what is actually being
> configured.
>
> The only place where field "proxy priority" of EB appears in the entire
> draft is par. 8 of sect. 1. Reading the draft for the first time I was
> confused what min priority for the option is used for and what is
> actually adjusted at various places.
>

[RJ] The aim of the draft was to be generic enough but also serve/fulfill
the use-case for the 6tisch scenario. Hence the EB point appears only in
the introduction.
In my previous experience where we used PANA
<https://en.wikipedia.org/wiki/Protocol_for_Carrying_Authentication_for_Network_Access>
for onboarding nodes in an AMI network, we needed a similar feature and we
ended up using a proprietary (but similar) signaling.



>
>
> Apart from these major issues, I have also a few editorial suggestions.
>
> A. After the par. 1 of sect. 1, I would introduce a subsection heading
> titled, for instance, "Terminology Disambiguation" to prepare the
> readers for what they are going to see next.
>

[RJ] I think we should have defined the terms used in the document in the
Terminology section as done in all other drafts. Will do that? Do you see
any ambiguous terms used?

>
> B. Similarly, before par. 5 of sect. 1 (starting with "It has become
> clear that not every routing member"), I would introduce a subsection
> heading titled "Motivation and Overview", just "Overview", or something
> along these lines.
>

[RJ] Let me check if we could improve this.

>
> C. I would promote the "Terminology" subsection to a section.
>

[RJ] We'll add the terms used in the document in the Terminology section
(Section 1.1). The Terminology section defined today doesn't serve the
draft well. Needs update.

>
> D. I would move par. 1 of sect. 2 (starting with "The size of the DODAG
> is measured by the Root") toward the end of sect. 1 as it has little to
> do with the option definition.
>

[RJ] Makes sense to me.

>
> E. I would fix the bullet list in sect. 2.1 so that it renders correctly
> in the text version of the draft.
>

[RJ] The text rendering seems alright to me in the browser. Should we add a
colon ':' after the bullet tag?

>
>
> Finally, I have numerous small editorial suggestions, but they seem
> immaterial at this point.


> Hope the review helps.
>

[RJ] Certainly helps. Thank You.

>
> Best,
> --
> - Konrad Iwanicki.
>
> _______________________________________________
> Roll mailing list
> [e-mail]
> https://www.ietf.org/mailman/listinfo/roll
>