Re: [Roll] Review of draft-ietf-roll-rnfd-02

[Konrad Iwanicki <[e-mail]>]

Dear Carles,

Thank you a lot for your feedback! Please, find my replies inline. 
Whenever I talk about changes made in the draft, I mean its version 03, 
which I submitted today.

On 27/11/2023 11:14, Carles Gomez Montenegro wrote:
> - Abstract: probably, the RFC Editor will request to expand the RFND 
> acronym in the abstract (and perhaps even in the title). Maybe this can 
> be done already in the current stage.

I would leave this to the editor, because an expansion of the acronym in 
the abstract may not be informative at all, not to mention the title. I 
definitely will keep the comment in mind, though.

> - Section 1, 1st paragraph: "the main"? Perhaps a "significant challenge"?

Done.

> - Section 1, 2nd paragraph: "more prone to failures"...? Compared to 
> what? Perhaps remove "more".

Done.

> - Section 1.2, 3: what is "the critical path"?

Replaced with “the overall latency”.

> - Section 2. s/is the DODAG/is a DODAG

Done.

> - Section 2. At this stage, a reader might wonder whether the Acceptor 
> role has any relationship with being a root's neighbor or not. Feel free 
> to ignore this comment if the current definition of Acceptor is preferred.

I get the point but do not see how to modify the definition to be both 
accurate and succinct. I would leave it as is for a while.

> - Section 3. s/is the DODAG root’s/is a DODAG root’s

I would say this refers to the root from the first sentence of the 
paragraph. Does it make sense?

> - Section 3.1. "The same is true for a Sentinel node in states 
> “SUSPECTED DOWN” and “LOCALLY DOWN”." Perhaps it might be good to add 
> next an explicit sentence regarding Acceptors?

Done -- reformulated the sentence so that hopefully it now covers both 
roles more explicitly.

> - Section 3.2. The meaning of "merges" is not clear at this stage. 
> Perhaps it might be good to add a forward reference to the section where 
> the "merge" operation is defined?

I would leave this sentence as is. The word gives some intuition, which 
I believe is all that is necessary at this point. Moreover, the next 
sentence explicitly refers to the merge operation by mentioning its 
properties, which suggests that the operation must be defined later more 
extensively.

> - Section 3.2.  s/acros/across

Done.

> - Section 4.2. "Returns TRUE, if more than 63% of the bits in c are 
> equal to 1, or FALSE, otherwise."  Regarding the 63% ratio, is there any 
> motivation behind it? For example, any scenario, network topology, etc., 
> from which this ratio has been derived? Could it be a configurable 
> parameter?

The threshold is much as the maximal load factor value in a hash table. 
The higher it is, the higher the probability of collisions when Sentinel 
nodes add themselves to CFRCs. The selected value of 63% bits set to one 
corresponds to the maximum likelihood estimate equal to the total number 
of available bits, that is, as if the hash table was fully loaded. In 
the reviewed draft, this single value was selected for simplicity. 
However, we did test the algorithm with lower values, and they make 
sense if one requires a lower probability of collisions. Therefore, in 
the revised version, the value is made a configuration parameter with 
the default value of 63%. Good suggestion!

> - Section 5.1. "RNFD—if active" This is the first, but there are various 
> instances in the document where some explanatory phrase is inserted 
> between clauses. In my opinion, a blank space should be inserted before 
> the first hyphen and after the last hyphen to avoid understanding that 
> e.g. "RNFD-if" is a term, and so on.

The writing guidelines that I usually follow suggest not putting spaces 
but also permit doing so. Since I agree that in the case of a draft the 
clarity should be preferred, I put spaces around all em dashes in the 
new version. It indeed looks better when rendered.

> - Section 5.1. "a switch from Sentinel to Acceptor has no 
> preconditions". Perhaps it might be good to describe any reasons why a 
> node might want to switch from Sentinel to Acceptor.

This information would require a longer explanation, which is there in 
further sections of the document. This section in turn only lists 
preconditions, so that they are explicitly stated in one place. 
Therefore, I would leave it as is.

> - Section 5.2. Is there any maximum amount of time during which LORS can 
> be in "SUSPECTED DOWN"?

No. This may depend on the verification mechanisms the implementation uses.

> - Section 5.2. I would suggest adding "(see Section 5.1)" after "do this 
> before the previous conditions 2–4"

Done.

> - Section 5.3. "It is RECOMMENDED to use for RNFD a dedicated Trickle 
> timer". What would be the default settings for this dedicated Trickle timer?

I assumed that the settings should be the same as for RPL’s timer, and 
hence did not put this information in the original text. Of course, they 
need not be, and one may want to trade off latency for traffic. But this 
would be done in precisely the same ways as in RPL.

> - Section 5.5. "The node MUST NOT activate the protocol". What does "the 
> protocol" refer to?

RNFD, which is mentioned in the previous statement as inactive.

> - Section 5.5. "the protocol SHOULD be active from the moment of the 
> joining." It may be good to explain reasons or use cases that motivate 
> that the SHOULD is not a MUST.

I see no such reasons. MUST is the right word then. Good catch.

> - Section 5.6. "MAY reset its Trickle timer". Does this refer to the 
> normal Trickle timer, or to the RNFD-dedicated one?

Whichever is adopted in the implementation. I clarified this in Section 
5.3 of the new version.

> - Section 5.8. "The default value of the threshold is 0.12". "The 
> default value of the threshold is 0.51". Any reason, or hint on 
> underlying scenarios these numbers have been derived from?

Value 0.51 means that a majority of Sentinels must locally consider the 
DODAG root as down in order for the network to globally consent that it 
is indeed down. I added this explanation. Similarly, value 0.12 simply 
indicates a growth that we believe is significant enough. The values 
were selected empirically and should work well in medium and high 
density networks. For really sparse networks, they may be configured 
differently.

> - Section 5.8. s/increased traffic due suspicion/increased traffic due 
> to suspicion.

Done.

> - Just a suggestion: perhaps it might be helpful to add examples 
> regarding some of the functionality defined in Section 5 (e.g., sections 
> 5.1 and 5.3).

Do you have any specific examples in mind for the two sections, because 
it is hard for me to come up with any?

In any case, thank you once again! The comments certainly helped improve 
the clarity of the draft.

Best,
-- 
- Konrad Iwanicki.