Subject: [Roll] Benjamin Kaduk's No Objection on draft-ietf-roll-efficient-npdao-12: (with COMMENT)

From: Benjamin Kaduk via Datatracker <[email-masked]>
Date: Tue, 25 Jun 2019 17:26:50 -0700

Benjamin Kaduk has entered the following ballot position for
draft-ietf-roll-efficient-npdao-12: No Objection

When responding, please keep the subject line intact and reply to all
email addresses included in the To and CC lines. (Feel free to cut this
introductory paragraph, however.)

Please refer to https://www.ietf.org/iesg/statement/discuss-criteria.html
for more information about IESG DISCUSS and COMMENT positions.

The document, along with other ballot positions, can be found here:
https://datatracker.ietf.org/doc/draft-ietf-roll-efficient-npdao/

----------------------------------------------------------------------
COMMENT:
----------------------------------------------------------------------

I think that we need greater clarity about whether the DCOSequence
number is just a series of monotonic (i.e., time-ordered) nonces (to be
echoed back for matching request/response) or a full-on sequence counter
that allows for loss detection as well as providing in-order delivery.
It sounds like we just need the time-ordering and single-use properties,
but I'm not entirely sure.  I wavered about making this a Discuss point
but ended up not doing so since I'm not sure how much harm is being
risked.  (I also mention this topic a couple times in the
section-by-section comments below.)

I agree with Barry that the Abstract is really hard to parse.

Section 1.2

   RPL uses NPDAO messaging in the storing mode so that the node
   changing it routing adjacencies can invalidate the previous route.

nit: "its routing adjacencies"

   This is needed so that nodes along the previous path can release any
   resources (such as the routing entry) it maintains on behalf of
   target node.

nit: singular/plural mismatch "nodes"/"it maintains"

Section 4.1

                                                            When node A
   receives the regular DAO, it finds that it already has a routing
   table entry on behalf of the target address of node D.  It finds
   however that the next hop information for reaching node D has changed
   i.e., node D has decided to change the paths.  In this case, Node A
   which is the common ancestor node for node D along the two paths
   (previous and new), should generate a DCO which traverses downwards
   in the network.

I can't decide whether or not it helps readability to reiterate that in
addition to creating the DCO, node A also does normal DAO processing
(e.g., forwarding to the 6LBR).  I guess the example in A.1 does show
this normal processing, so maybe it's overkill to also do so here.

Section 4.2

               Transit Information Option should be carried in the DAO
   message with I-flag set in case route invalidation is sought for the
   corresponding target(s).

nit: this text as written implies thatthe I-flag is set in the DAO
itself, not the TIO therein.

I'd also suggest to s/in case/when/ for clarity.

   The common ancestor node SHOULD generate a DCO message in response to
   this I-flag when it sees that the routing adjacencies have changed
   for the target.  I-flag governs the ownership of the DCO message in a
   way that the target node is still in control of its own route
   invalidation.

nit: "The I-flag" (start of last sentence).

I'd further suggest rewording to something like "The I-flag is intended
to give the target node control over its own route invalidation, serving
as a signal to request DCO generation; in normal operation a DCO would
not otherwise be generated"; the current text about "ownership" has some
weird connotations/implications and this text also implicitly assumes
that DAO/TIO/I-flag will never be maliciously generated.  It is also a
little weaker about unsolicited DCO, per Section 4.5

Section 4.3

   A new ICMPv6 RPL control message type is defined by this
   specification called as "Destination Cleanup Object" (DCO), which is

nit: either "called" or "known as" or "referred to as" would be fine;
"called as" is a grammatical mismatch.

   DCOSequence: Incremented at each unique DCO message from a node and
   echoed in the DCO-ACK message.  The initial DCOSequence can be chosen
   randomly by the node.

What's the behavior if a sequence number is skipped?  (Why do we have a
sequence number if we aren't going to detect and act on this condition?)
Ah, I see Section 4.3.3, but perhaps a forward-reference is in order.

Section 4.3.4

It seems that the "Reserved" field should be called "Flags", since a
registry is being created for it.

(I trust that the language about the D flag and DODAGID optionality from
Barry's ballot thread is consistent between DCO and DCO-ACK.)

Section 4.4

   1.  If a node sends a DCO message with newer or different information
       than the prior DCO message transmission, it MUST increment the
       DCOSequence field by at least one.  A DCO message transmission
       that is identical to the prior DCO message transmission MAY
       increment the DCOSequence field.

While reading up to this point I managed to confuse myself about Path
Sequence (which must be consistent from DAO to DCO) and the separate
DAOSequence and DCOSequence fields.  To check my (less confused)
understanding, I guess if I could over-summarize, Path Sequence is like
a generation counter for a given node's position in the routing
topology, and the other two are for managing retransmission/ack of the
respective update messages.  So if that mental model is correct, then
there's not any value from trying to introduce a shared sequence number
space for DCO and DAO, even though they are frequently going to be
generated at the same time, especially since they have different
recipients.  Right?

I do agree with the other discussion that we need clarity about whether
the increment is exactly one or larger values are allowed (plus,
presumably, whether the recipient should infer anything from a sequence
number gap).  I do note that these are expected to be "lollipop
sequence counters" per RFC 6550.

   4.  A node receiving a unicast DCO message with the 'K' flag set
       SHOULD respond with a DCO-ACK.  A node receiving a DCO message
       without the 'K' flag set MAY respond with a DCO-ACK, especially
       to report an error condition.

This seems redundant with Section 4.3's "A node receiving a DCO message
without the 'K' flag set MAY respond with a DCO-ACK, especially to
report an error condition."

Section 4.4

   The scope of DCOSequence values is unique to each node.

recipient or originator?

Section 4.5

   path on behalf of the target entry.  The 6LR has all the state
   information namely, the Target address and the Path Sequence,

nit: comma before "namely".

Section 4.6.2

   Even with the changed semantics, the current NPDAO mechanism in
   [RFC6550] can still be used, for example, when the route lifetime
   expiry of the target happens or when the node simply decides to
   gracefully terminate the RPL session on graceful node shutdown.

Er, what changed semantics?  This document does not have an Updates:
relationship to any other document.

Section 4.6.3

   Note that there is no requirement of synchronization between DCO and
   DAOs.  The DelayDCO timer simply ensures that the DCO control
   overhead can be reduced and is only needed when the network contains
   nodes using multiple preferred parent.

This ("no requirement of synchronization") is because the benefit of DCO
is in expiring routes faster than their normal expiration time to save
local storage, rather than to provide synchronous route migration?  (It
might be worth reiterating, if you want.)

Section 7

   This document introduces the ability for a common ancestor node to
   invalidate a route on behalf of the target node.  The common ancestor
   node is directed to do so by the target node using the 'I' flag in
   DCO's Transit Information Option.  However, the common ancestor node

nit(?): there's perhaps some wordsmithing possible about "is directed to
do so", given the next sentence and Section 4.5.

   is also met.  Having said that a malicious 6LR may spoof a DAO on
   behalf of the (sub) child with the I-flag set and can cause route
   invalidation on behalf of the (sub) child node.

IIUC, such a malicious 6LR might also spoof a DAO even without this
mechanism (to invalidate the "proper" Path Sequence) or otherwise cause
denial of service by dropping traffic entirely, so perhaps we want to
add another clause ", so this new mechanism does not present a
substantially increased risk of disruption".

   This document assumes that the security mechanisms as defined in
   [RFC6550] are followed, which means that the common ancestor node and
   all the 6LRs are part of the RPL network because they have the
   required credentials.  A non-secure RPL network needs to take into
   consideration the risks highlighted in this section.

I'd consider adding "as well as those highlighted in [RFC6550]" to the
end.

Appendix A.1

   6.  Node G receives the DCO(tgt=D,pathseq=x+1).  It checks if the
       received path sequence is latest as compared to the stored path
       sequence.  If it is latest, Node G invalidates routing entry of
       target D and forwards the (un)reachability information downstream
       to B in DCO(tgt=D,pathseq=x+1).

This wording of "latest as compared to" feels unusual to me; I would
have expected "is later than the stored path sequence" and "If it is
later", but perhaps there is a convention here that I'm missing.

nit: "invalidates the routing entry"

   9.  The propagation of the DCO will stop at any node where the node
       does not have an routing information associated with the target.
       If the routing information is present and its Path Sequence is
       higher, then still the DCO is dropped.

nit: maybye reword to "If cached routing information is present and the
cached Path Sequence is higher than the value in the DCO, then the DCO
is dropped".

Appendix A.2

I feel like we should probably mention the DelayDAO timer as well as the
DelayDCO one.

I think this is a side note, but it seems like the timer mechanism for
DelayDAO (and by extension, DelayDCO) are a bit fragile, as one party
has to wait for the full timeout before sending the message (e.g., N22
in this example) that the other party is waiting the timeout to receive
(e.g., N11).  So it seems like we are still susceptible to transport
delay/jitter and race conditions at some point in the network, even if
it's not the next-hop of the target node.  But if that's a property of
DelayDAO from RFC 6550, it doesn't really make sense to try to address
it in this document (and it's also possible I misunderstand the
situation).

---

From: Rahul Arvind Jadhav <[email-masked]>
Date: Mon, 1 Jul 2019 00:51:22 +0000

Thank you Benjamin for the detailed review and the comments.
Please find my responses inline.

Thanks,
Rahul

I think that we need greater clarity about whether the DCOSequence number is just a series of monotonic (i.e., time-ordered) nonces (to be echoed back for matching request/response) or a full-on sequence counter that allows for loss detection as well as providing in-order delivery.
It sounds like we just need the time-ordering and single-use properties, but I'm not entirely sure.  I wavered about making this a Discuss point but ended up not doing so since I'm not sure how much harm is being risked.  (I also mention this topic a couple times in the section-by-section comments below.)

[RJ] The DCOSequence is not expected to provide in-order delivery. The receiver
is simply expected to echo back the DCOSequence whatever is received which
informs the sender that the info is received. Even if received un-ordered the receiver is expected to just ack the rcvd DCO.

[RJ] Yes. We had updated the abstract as per Barry's suggestion.

Section 1.2

[RJ] Fixed

nit: singular/plural mismatch "nodes"/"it maintains"
[RJ] changed "it maintains" to "they maintain"

Section 4.1

                                                            When node A

I can't decide whether or not it helps readability to reiterate that in addition to creating the DCO, node A also does normal DAO processing (e.g., forwarding to the 6LBR).  I guess the example in A.1 does show this normal processing, so maybe it's overkill to also do so here.

[RJ] As you hinted, It would be better for the sake of completeness to add DAO processing point here as well. I have added a statement saying, "Node A handles normal DAO forwarding to
6LBR as required by the RFC6550."

Section 4.2

nit: this text as written implies thatthe I-flag is set in the DAO itself, not the TIO therein.

[RJ] Updated to say, "Transit Information Option with I-flag set should be carried in the DAO message when route invalidation is sought ..."

[RJ] Fixed as mentioned above.

[RJ] Fixed

I'd further suggest rewording to something like "The I-flag is intended to give the target node control over its own route invalidation, serving as a signal to request DCO generation; in normal operation a DCO would not otherwise be generated"; the current text about "ownership" has some weird connotations/implications and this text also implicitly assumes that DAO/TIO/I-flag will never be maliciously generated.  It is also a little weaker about unsolicited DCO, per Section 4.5

[RJ] Have updated the text, " The I-flag is intended to give the target node control over its own route invalidation, serving as a signal to request DCO generation."
I have avoided to say "in normal operation a DCO would not otherwise be generated" especially given the case of unsolicited DCO.

Section 4.3

nit: either "called" or "known as" or "referred to as" would be fine; "called as" is a grammatical mismatch.

[RJ] fixed. using "referred to as"

What's the behavior if a sequence number is skipped?  (Why do we have a sequence number if we aren't going to detect and act on this condition?) Ah, I see Section 4.3.3, but perhaps a forward-reference is in order.

[RJ] Added a forward-reference.

Section 4.3.4

It seems that the "Reserved" field should be called "Flags", since a registry is being created for it.

[RJ] Yes, changed the "Reserved" field name to "Flags" in both the figure as well as the description below.

(I trust that the language about the D flag and DODAGID optionality from Barry's ballot thread is consistent between DCO and DCO-ACK.)

[RJ] Yes the DODAGID optionality statement is fixed in both the places as per Barry's suggestion. Thanks for reminding about the two places to fix.

Section 4.4

While reading up to this point I managed to confuse myself about Path Sequence (which must be consistent from DAO to DCO) and the separate DAOSequence and DCOSequence fields.  To check my (less confused) understanding, I guess if I could over-summarize, Path Sequence is like a generation counter for a given node's position in the routing topology, and the other two are for managing retransmission/ack of the respective update messages.  So if that mental model is correct, then there's not any value from trying to introduce a shared sequence number space for DCO and DAO, even though they are frequently going to be generated at the same time, especially since they have different recipients.  Right?

[RJ] The DAOSequence and DCOSequence are not dependent on each other at all.
Yes, the DAO could be generated and DCO could be generated at the same time for
different recipients.

I do agree with the other discussion that we need clarity about whether the increment is exactly one or larger values are allowed (plus, presumably, whether the recipient should infer anything from a sequence number gap).  I do note that these are expected to be "lollipop sequence counters" per RFC 6550.

[RJ] This will be fixed i.e., we will make it explicit about the sequence
counter operation and add a ref to the corresponding section in 6550.

This seems redundant with Section 4.3's "A node receiving a DCO message without the 'K' flag set MAY respond with a DCO-ACK, especially to report an error condition."

[RJ] Yes it is redundant. But i feel it is better to have it in both places.
The statements are consistent with each other.

Section 4.4

recipient or originator?

[RJ] Changed this to, "The scope of DCOSequence values is unique to the node
which generates it."

Section 4.5

[RJ] Fixed

Section 4.6.2

[RJ] I have reworded the section removing the statement, "Even with the changed
semantics". Also (I believe) the section is more readable now.

Section 4.6.3

This ("no requirement of synchronization") is because the benefit of DCO is in expiring routes faster than their normal expiration time to save local storage, rather than to provide synchronous route migration?  (It might be worth reiterating, if you want.)

[RJ] Sorry, but i didn't understand this. I didn't understand why the statement
"the benefits of DCO is in expiring routes faster than their normal expiration
time ...." should be mentioned in this context? NPDAO and DCO both are
proactive route invalidation messaging techniques and they both help in expiring
routes faster than their normal expiration times. DCO simply is more reliable
due to its use on more reliable links.

Section 7

nit(?): there's perhaps some wordsmithing possible about "is directed to do so", given the next sentence and Section 4.5.

[RJ] I changed the statement, "The common ancestor node is directed to do so by
the target node ..." to "The common ancestor node could be directed to do so by
the target node ..."

IIUC, such a malicious 6LR might also spoof a DAO even without this mechanism (to invalidate the "proper" Path Sequence) or otherwise cause denial of service by dropping traffic entirely, so perhaps we want to add another clause ", so this new mechanism does not present a substantially increased risk of disruption".

[RJ] Reworded and added new statements based on your suggestions to this para.

I'd consider adding "as well as those highlighted in [RFC6550]" to the end.

[RJ] Added the mentioned words to the end of the para.

Appendix A.1

This wording of "latest as compared to" feels unusual to me; I would have expected "is later than the stored path sequence" and "If it is later", but perhaps there is a convention here that I'm missing.

[RJ] I could not find a general convention here (after checking in 6550), so I
changed it per your suggestion to use "later" in both the statements.

[RJ] Fixed.

nit: maybye reword to "If cached routing information is present and the cached Path Sequence is higher than the value in the DCO, then the DCO is dropped".

[RJ] This sounds lot better. Thanks for the text.

Appendix A.2

I feel like we should probably mention the DelayDAO timer as well as the DelayDCO one.

I think this is a side note, but it seems like the timer mechanism for DelayDAO (and by extension, DelayDCO) are a bit fragile, as one party has to wait for the full timeout before sending the message (e.g., N22 in this example) that the other party is waiting the timeout to receive (e.g., N11).  So it seems like we are still susceptible to transport delay/jitter and race conditions at some point in the network, even if it's not the next-hop of the target node.  But if that's a property of DelayDAO from RFC 6550, it doesn't really make sense to try to address it in this document (and it's also possible I misunderstand the situation).

[RJ] DelayDAO and DelayDCO are mechanisms to reduce the control overhead in the
network. If an implementation chooses not to use it, it will still work ok,
albeit with more control overhead. Yes the DelayDCO is modeled similar to
DelayDAO and it is expected that a node simply waits for the timer to expire
before it could start sending the corresponding message (DCO is this case, DAO
in case of 6550). I am not fully sure if I completely understand what you mean
by "susceptibility to transport delay/jitter" but i understand that there could
be race conditions. If the race conditions happen the worst case problem would
be slightly more control overhead. Race condition would not result in invalid
states on any nodes.