Subject: [RTG-DIR] RtgDir review: draft-ietf-roll-rpl-11.txt

From: Abhay Roy <[email-masked]>
Date: Wed, 01 Sep 2010 23:38:07 -0700

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. The purpose 
of the review is to provide assistance to the Routing ADs. For more 
information about the Routing Directorate, please see 
http://www.ietf.org/iesg/directorate/routing.html

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-rpl-11.txt
Reviewer: Abhay Roy
Review Date: 1st September 2010
IETF LC End Date: 1st September 2010
Intended Status: Proposed Standard

*Summary:*
I have some major issues with this document that needs be resolved 
before publication.

*Comments:*
This is a very thoroughly written document and attempts to solve a large 
number of scenario's in ROLL problem statement. I only had two weeks to 
go over this 130+ pages long draft. So my review (especially on finer 
protocol state machines) is not sufficiently in-depth.

*Major Issues:*

This is a pretty complex protocol with a lot of intricate rules to hold 
it all together. It's vital that the correctness of many of these state 
machines be proven by requiring 2 (or more) interoperable 
implementations as a prerequisite for it becoming a Proposed Standard.

There is a normative reference to draft-ietf-roll-trickle-02, which is 
an Informational track document. This is generally not allowed. Any 
reason why this is being done here?

This document requires use of an Objective Function, but none have been 
defined in the draft. There is an Informative reference to  
draft-ietf-roll-of0-03 (why not Normative?). Ideally, at least one OF 
should have been defined in this document with an option to define 
additional ones in time.

Section 6.2.1"
"Format of the DIS Base Object

         0                   1                   2
         0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1 2 3
        +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
        |     Flags     |   Reserved    |   Option(s)...
        +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+

                       Figure 13: The DIS Base Object

    Flags:  8-bit unused field reserved for flags.  The field MUST be
          initialized to zero by the sender and MUST be ignored by the
          receiver.

    Reserved:  8-bit unused field.  The field MUST be initialized to zero
          by the sender and MUST be ignored by the receiver."

#### This document doesn't allocate any bit from the Flags and Reserved 
fields. I chose this message just to cite an example. There are similar 
fields in other messages which are unused. I can understand leaving a 
few bits aside for future proofing. But having two 8 bit fields next to 
each other, and both completely unassigned, doesn't make much sense. Any 
rationale here?

Section 6.6:
"A CC message (request or response) MUST NOT set the 'C' bit of the 
security section: CC messages always have full counters."

#### I can't seem to find the C bit in security section.

*Minor Issues:*

It's not clear on what should happen if state being stored in 
storing-mode "overflows", i.e. a node can't quite keep all the state.I 
may have missed it. In routing problems, it's important that the routing 
behavior is described in cases where an intermediate node doesn't have 
full state.

Section 3.3.2:
"A DODAG Root institutes a global repair operation by incrementing the 
DODAG Version Number."

### What are the event based trigger to cause this?

Section 3.3.7:
"For example, if a node receives a packet flagged as moving in the  
upward direction, and if that packet records that the transmitter is of 
a lower (lesser) Rank than the receiving node, then the receiving node 
is able to conclude that the packet has not progressed in the upward 
direction and that the DODAG is inconsistent. "

### Doing such checks in hardware forwarding paths isn't exactly 
trivial. Is the assumption here that the packet forwarding rate is low, 
and hence software forwarding path is good enough?

Section 3.6.2:
"DAGRank(M) equals DAGRank(N):  In this case the positions of M and N 
within the DODAG and with respect to the DODAG root are similar 
(identical).  Routing through a node with equal Rank may cause a routing 
loop (i.e., if that node chooses to route through a node with equal Rank 
as well).

### Does this mean, one can't route "upwards" via another node of the 
same rank? Wouldn't think cause black holes in some scenarios where that 
might be the only viable path left?

Section 3.8.1.1:

### Nice example of nodes B and C both trying to get 2 parents. But I am 
not sure, if the spec is crisp in terms of what are the rules to avoid it.

Section 3.8.3:
"This loop happens when a No-Path (a DAO message that invalidates a 
previously announced prefix) was missed and persists until all state has 
been cleaned up."

### What is the trigger to cleanup old stale state?

Section 3.8.3:
RPL includes loop detection mechanisms that may mitigate the impact of  
DAO loops and trigger their repair.

### May? Shouldn't this be a more stronger statement? Loops are bad ;)

There are a lot of usage of variable size values using TLV encoding. The 
authors have chosen an 8 bit field for length.  It ends up limiting to 
256 bytes of payload. In many other protocols, this has become a 
"limitation" in the long run. Just wanted to bring to your attention, if 
you see a need arising for bigger payloads.

*Nits:*
none

---

From: Tim Winter <[email-masked]>
Date: Mon, 13 Sep 2010 16:53:25 -0400

Hi Abhay,

 > discussion or by updating the draft.

Thank you for your review -- we will strive to resolve them.

 > implementations as a prerequisite for it becoming a Proposed Standard.

There are 10+ known implementations.  Interoperability events coordinated
by the IP for Smart Objects Alliance (IPSO) and the Zigbee Alliance have
taken place and both were successful.  Although the results were not
public, we did have positive feedback, and both the storing and non-storing
modes have been tested.  We cannot guarantee that ALL features have been
tested, but certainly a good number of them thus far.

 > reason why this is being done here?

The WG has moved draft-ietf-roll-trickle to Standards Track to resolve
this issue, and a publication request has been made.

 > should have been defined in this document with an option to define

We explicitly did not intend to bind the implementer to use a particular
objective function, or to include objective functions in the RPL base
specification.  It was thought that this would enforce that the RPL base
specification could be flexible with regard to supporting a range of
objectives.  In fact, the WG did decide that not even a 'default' OF should
be mandated.  It is further expected that a given SDO such as Zigbee or
ISA100 will define its own OF as part of the package.

 > "Format of the DIS Base Object

We do anticipate future follow-on work to this RPL base specification that
will make use of those flag fields, and we did desire to open those IANA
registries at this time to administer those flags. In this particular case,
as a direct result of Ticket 71, the WG is considering some explicit
follow-up work to RPL that will make use/allocate from the DIS Flags field.

 > #### I can’t seem to find the C bit in security section.

This text is an editorial mistake and will be removed.  The 'C' bit refers
to a mode of operation where the Counter field could be compressed, and was
removed in WG discussion.

 >
 > *Minor Issues:*
 >
 > It’s not clear on what should happen if state being stored in
 > storing-mode “overflows”, i.e. a node can’t quite keep all the state.I

This was discussed in the WG -- the intention of the RPL base spec is to
suggest that nodes in storing-mode deployments should be properly scaled to
handle the size of the network segments that they will encounter.  This is
consistent with proprietary practices deployed today.  Future
specifications may elaborate on more refined actions/behaviors to manage
this case.

As it stands today, if an intermediate node cannot accommodate full state
then the routing state is not complete and messages may be dropped, similar
to what other protocols do today.

We can add a sentence to clarify this intent, and to point to the
possibility of future work.

 > ### What are the event based trigger to cause this?

This is declared to be application/implementation specific and out of
scope.  RPL defines how to trigger the global repair, but leaves it to the
implementation to figure out when it is necessary.  This is in part because
global repair in an LLN may be very expensive and we did not want to
mandate how and when it is invoked in the RPL base.  Section 17.2.5 and
17.5 also elaborate as follows:

     17.2.5.  Parameters to be configured on the DODAG root

        ...

        DAG Version Number Increment: a RPL implementation may allow by
        configuration at the DODAG root to refresh the DODAG states by
        updating the DODAGVersionNumber.  A RPL implementation SHOULD allow
        configuring whether or not periodic or event triggered mechanisms
        are used by the DODAG root to control DODAGVersionNumber change
        (which triggers a global repair as specified in Section 3.3.2.

        ...

     17.5.  Fault Management

        Fault management is a critical component used for troubleshooting,
        verification of the correct mode of operation of the protocol, network
        design, and is also a key component of network performance
        monitoring.  A RPL implementation SHOULD allow providing the
        following information related to fault managements:

        o  Memory overflow along with the cause (e.g. routing tables
           overflow, ...)

        o  Number of times a packet could not be sent to a DODAG parent
           flagged as valid

        o  Number of times a packet has been received for which the router
           did not have a corresponding RPLInstanceID

        o  Number of times a local repair procedure was triggered

        o  Number of times a global repair was triggered by the DODAG root

 > ### Doing such checks in hardware forwarding paths isn’t exactly

In general, today, it is the case that most LLN technologies are limited
more so by communication bandwidth than by processing bandwidth, i.e.  that
software in general can easily keep up with the link layer.  But we do not
intend to assume this in the design of RPL, and that RPL should be
applicable for other future technologies.

More specifically, it is known in the art and practice that
'forwarding-time' inconsistency detection is a useful technique for LLNs.
Such a detection technique does require this type of checking to be done by
the receiving node.  (Note that in our response to GenART comments we do
intend to add a few more informative references to give additional background
to this rationale)

We would then expect that any future LLN implementation that did include HW
for the forwarding logic would have to accommodate this non-trivial check.

 > ### Does this mean, one can’t route “upwards” via another node of the
 > same rank? Wouldn’t think cause black holes in some scenarios where that

That is true.  This concept was denoted 'sibling' forwarding in the WG
(note the broad interpretation of 'sibling' as 'nodes of the same Rank'
does not necessarily imply common parents), and was  explicitly ruled out
by the WG in favor of a more simplified/streamlined base specification
(Ticket 43).  It is expected that this concept will be elaborated/explored
further in future work.

 > not sure, if the spec is crisp in terms of what are the rules to avoid it.

In this example it is our attempt to give a general illustration of how
greedy behavior can lead to loops/instabilities.  At the end of the
example we do attempt to summarize the mechanisms RPL takes as follows:

       o  This cycle can be averted through mechanisms in RPL:

           *  Nodes (B) and (C) stay at a rank sufficient to attach to their
              most preferred parent (A) and don't go for any deeper (worse)
              alternate parents (Nodes are not greedy)

           *  Nodes (B) and (C) do not process DIO messages from nodes
              deeper than themselves (because such nodes are possibly in
              their own sub-DODAGs)

We could further clarify by adding a reference from this example to the
rules in Section 8.2.2.4, which serve to bound the impact greedy behavior
while allowing limited movement for local repair:

     8.2.2.4.  Rank and Movement within a DODAG Version

        ...

        2.  A node MAY advertise a Rank lower than its prior advertisement
            within the DODAG Version.

        3.  Let L be the lowest rank within a DODAG Version that a given
            node has advertised.  Within the same DODAG Version, that node
            MUST NOT advertise an effective rank higher than L +
            DAGMaxRankIncrease.  INFINITE_RANK is an exception to this rule:
            a node MAY advertise an INFINITE_RANK within a DODAG version
            without restriction.  If a node's Rank were to be higher than
            allowed by L + DAGMaxRankIncrease, when it advertises Rank it
            MUST advertise its Rank as INFINITE_RANK.

        ...

        Conceptually, an implementation is maintaining a DODAG parent set
        within the DODAG Version.  Movement entails changes to the DODAG
        parent set.  Moving Up does not present the risk to create a loop
        but moving Down might, so that operation is subject to additional
        constraints.

        ...

        A node is allowed to join any DODAG Version that it has never been a
        prior member of without any restrictions, but if the node has been a
        prior member of the DODAG Version then it must continue to observe
        the rule that it may not advertise an effective rank higher than
        L+DAGMaxRankIncrease at any point in the life of the DODAG Version.
        This rule must be observed so as not to create a loophole that would
        allow the node to effectively increment its rank all the way to
        INFINITE_RANK, which may have impact on other nodes and create a
        resource-wasting count-to-infinity scenario.

 > ### What is the trigger to cleanup old stale state?

Generally this will be triggered by inconsistency detection as that
forwarding path is attempted and fails.  This is covered in Section 11.2,
more specifically in Section 11.2.2.3:

     11.2.2.3.  DAO Inconsistency Loop Detection and Recovery

        A DAO inconsistency happens when a router has a downward route that
        was previously learned from a DAO message via a child, but that
        downward route is not longer valid in the child, e.g. because that
        related state in the child has been cleaned up.  With DAO
        inconsistency loop recovery, a packet can be used to recursively
        explore and cleanup the obsolete DAO states along a sub-DODAG.

        In a general manner, a packet that goes Down should never go Up
        again.  If DAO inconsistency loop recovery is applied, then the
        router SHOULD send the packet back to the parent that passed it with
        the Forwarding-Error 'F' bit set and the 'O' bit left untouched.
        Otherwise the router MUST silently discard the packet.

We can add a reference to Section 11.2 from Section 3.8.3 to help make this
clear for the reader.

 > ### May? Shouldn’t this be a more stronger statement? Loops are bad ;)

The problem with a stronger statement is that RPL does not guarantee loop
freedom.  In practice strict loop avoidance is costly and difficult to
achieve in LLNs.  RPL's approach tries to avoid the construction of
topologies that contain loops, further allows to detect a loop by the
detection of inconsistencies in a packet flow, and then to initiate a
repair.  We could strike the 'may', since loop detection does certainly
mitigate the impact of DAO loops (even though that mitigation may not be
100% effective).

Prior to the text that you noted, in Section 3.8, we do say:

     3.8.  Loop Avoidance

        RPL avoids creating loops when undergoing topology changes and
        includes rank-based datapath validation mechanisms for detecting
        loops when they do occur (see Section 11 for more details).  In
        practice, this means that RPL guarantees neither loop free path
        selection nor tight delay convergence times, but can detect and
        repair a loop as soon as it is used.  RPL uses this loop detection
        to ensure that packets make forward progress within the DODAG
        Version and trigger repairs when necessary.

 > “limitation” in the long run. Just wanted to bring to your attention, if

Noted.  8-bit length fields are thought to be sufficient for the majority
of the RPL options.  In the case of the Metric Container, which may exceed
256 bytes, we did adopt the following convention in Section 6.7.4, which
could be carried forward to other options if needed:

     6.7.4.  Metric Container

        ...

        The Metric Container MAY appear more than once in the same RPL
        control message, for example to accommodate a use case where the
        Metric Data is longer than 256 bytes.  More information is in
        [I-D.ietf-roll-routing-metrics].

 > none

Did we address your concerns?

Regards,
-RPL Authors

---

From: Abhay Roy <[email-masked]>
Date: Wed, 15 Sep 2010 23:31:56 -0700

Hi Tim (and other RPL Authors),

Thanks for taking the time for the elaborate explanations for the issues 
raised. With the suggested edits/clarifications below, I have no further 
concerns.

Once again, it was a joy to review such a comprehensive draft!

Regards,
-Abhay