Subject: [homenet] DNCP questions (and minor nits)

From: Juliusz Chroboczek <[email-masked]>
Date: Fri, 03 Jul 2015 23:28:51 +0200

Markus, Steven,

Section 4.4 of DNCP says that the NODE-STATE TLVs sent in reply to
a REQ-NODE-STATE MUST NOT contain the optional part.  Why is that?  If
I've recently republished my own data (e.g. because I gained a neighbour),
it makes sense to me to send my own NODE-STATE in order to avoid a round-trip.

Still in the same section, when a duplicate node-id is detected, the new
seqno chosen myst be significantly higher than the one received.  Why is
that?  Is +1 not enough?  (I'm using +42 right now, of course.)

The DNCP profile MUST provide guidance about how to deal with node-id
collisions.  Where is that guidance provided in HNCP?

The expression defining the circular ordering is written in C syntax, but
I think it assumes different priorities than C.  Please put brackets
around the bitwise conjunction.

Section 4.5 suggests sending a unicast REQ-NETWORK-STATE when receiving
ENDPOINT.  If adding the peer fails (say, out of memory), won't this cause
a livelock?

Shouldn't the first part of 4.5 be moved into 4.4?

4.6, MUST be traversed Either immediately...  Is it not better to say that
it must be traversed before the next message is scheduled to be sent?

6.1.2 a new t MUST be chosen, but is the interval not reset?  Same in 6.1.3.

---

From: Markus Stenberg <[email-masked]>
Date: Sat, 4 Jul 2015 05:40:24 +0300

> it makes sense to me to send my own NODE-STATE in order to avoid a round-trip.

I assume you are talking about REQ-NET-STATE. The underlying problem 
here is that (generally speaking) there is no way to know if they have 
received the state already from someone else even if you did change it 
recently.

Especially given large node datas, the difference is relatively large.

Now that we discuss it, I think SHOULD NOT might be correct, though; if 
you haven't shared it with _anyone_, obviously first one is at least 
unknown to the rest of the network.

> that?  Is +1 not enough?  (I'm using +42 right now, of course.)

There may be more than one update from previous reboot cycle in the 
network still; it is not even bounded, given infinitely large network, 
but 'few' is enough in real world, and so should 42.

Funnily enough I thought +1 was enough at first too.

> collisions.  Where is that guidance provided in HNCP?

    o  HNCP uses opaque 32-bit node identifiers
       (DNCP_NODE_IDENTIFIER_LENGTH = 32).  A node implementing HNCP
       SHOULD generate and use a random node identifier.  If using a
       random node identifier and there is a node identifier collision,
       the node MUST immediately generate and use a new random node
       identifier which is not used by any other node.

> around the bitwise conjunction.

Actually I do not even think it is C; at least, for it to be valid C, 
some bonus parentheses and 1<<X should replace 2^ portions.

eg what we use is ((((uint32_t)(a) - (uint32_t)(b)) & ((uint32_t)1<<31)) 
!= 0)

Hmm. Wonder if removing the guidance altogether is the preferable choice 
here.

> ENDPOINT.  If adding the peer fails (say, out of memory), won't this cause

Given stream case, yes, as there is no guarantee of second one ever 
showing up.

For datagrams, next ENDPOINT should do it, no?

> Shouldn't the first part of 4.5 be moved into 4.4?

Hm, it might improve the readability a bit :)

> it must be traversed before the next message is scheduled to be sent?

Not really, as in some cases, you may have relatively long next 
keepalive or Trickle timer, and waiting until then to discover that you 
should have gotten rid of the stuff sooner and updated others about the 
change is not good.

> 6.1.2 a new t MUST be chosen, but is the interval not reset?  Same in 6.1.3.

Hm, true. Any suggestions on better wording? Even the current one is bit 
cumbersome, and repeating it twice with even more stuff in it is not 
very appealing either.

Cheers,

-Markus

---

From: Juliusz Chroboczek <[email-masked]>
Date: Sat, 04 Jul 2015 10:21:57 +0200

>> a REQ-NODE-STATE MUST NOT contain the optional part.

> I assume you are talking about REQ-NET-STATE. The underlying problem here

[...]

> unknown to the rest of the network.

This explanation is enlightening and should be in the draft.

>> that?  Is +1 not enough?  (I'm using +42 right now, of course.)

[...]

> Funnily enough I thought +1 was enough at first too.

This explanation is enlightening and should be in the draft.

Markus, DNCP is a beautiful protocol.  There are a number of things that
I've asked before that are subtle enough to warrant an explanation.  In
addition to the two points above, the following come to mind:

 - the fact that you use a single-line change to Trickle that has huge
   consequences on its behaviour in case of persistent desynchronisation;
 - the fact that flooding link-local data, while wasteful of storage,
   avoids the need for keeping reliable state beyond what is in the
   Merckle tree.

These things need to be clearly explained to the naive implementor.

I've been deliberately copying the list with all of my questions (sorry
for the spam) so that we can have a record.  Now somebody (somebody with
strong nerves) needs to go through my recent flurry of epistolar activity
and decide, for each of my questions, whether I'm just being stupid or
whether this deserves a clarification.  In case of doubt, clarify.

I realise I'm being a pain, especially since it contradicts our motto "it
was difficult to write, there's no reason it should be easy to read".

> Hmm. Wonder if removing the guidance altogether is the preferable choice

No, please leave it, just add the missing pair of brackets, and add "where
(a % b) represents the remainder of a modulo b and (a & b) represents
bitwise conjunction of a and b".

[Aside: I think the official IETF definition of seqno comparisons is in
 RFC 1982, which uses a slightly different definition than both HNCP and
 Babel: comparision between s and s + 2^(n-1) is undefined, "implementations
 are free to return either result, or flag an error".  Yeah, right.  They
 use a definition by cases.

 RFC 793 Section 3.3 says "There are some subtleties to computer modulo
 arithmetic, so great care should be taken in programming the comparison of
 such values.", and then they define the "in-window" relation by cases.

 RFC 6126, simply says "seqno > seqno' ... where sequence numbers are
 compared modulo 2^16", with no further ado.  End of aside.]

> For datagrams, next ENDPOINT should do it, no?

Not if allocation fails again.  Am I being even more paranoid than you?
I suggest putting a word of caution in there.

> cumbersome, and repeating it twice with even more stuff in it is not very
> appealing either.

This really needs to be stated accurately.  I think you have two choices:
you either repeat the definition of Trickle in this document, or you put
precise references to RFC 6206 whenever you mention a Trickle action.
I think the latter is a better choice from the implementors point of view,
the former is the better choice for the casual reader.  Screw the casual
reader.

In this particular instance, I think what you want to say is "and starts
a new trickle interval, as in step 2 of Section 4.2 of RFC 6206".

-- Juliusz

---

From: Mikael Abrahamsson <[email-masked]>
Date: Sat, 4 Jul 2015 10:30:51 +0200 (CEST)

On Sat, 4 Jul 2015, Juliusz Chroboczek wrote:

> I realise I'm being a pain, especially since it contradicts our motto 
> "it was difficult to write, there's no reason it should be easy to 
> read".

For what it's worth, I fully agree with you. This was what I meant by my 
earlier comment that there is a lot of valuable information in the heads 
of the implementors/designers that isn't written down in the drafts as 
they stand right now.

Remember also that this is intended to be a protocol that's being 
implemented and run by a lot of different vendors who do not have a great 
track record of writing and maintaining excellent software (to put it 
mildly). Any help given to them to get their implementations right is very 
valuable.

-- 
Mikael Abrahamsson    email: [email-masked]