On 30.6.2015 20.10, Juliusz Chroboczek wrote:
Concerns about DNCP
===================
DNCP is an elegant and small protocol that distributes HNCP data across
the Homenet. DNCP works by flooding a hash of the full network state over
link-local multicast, and synchronising the actual state piecewise by
using link-local unicast request/response pairs. This document outlines
a number of concerns with DNCP as described in draft-ietf-homenet-dncp-06.
I very strongly support the work that is being done on DNCP and HNCP.
I will be glad to see something very similar to the current drafts adopted
by the Homenet WG.
Packet format
-------------
### There is no header
A DNCP packet consists of just a sequence of TLVs. This means that there
is no way to version the DNCP protocol. Should a change in the TLV format
be required, a new UDP port will need to be allocated. (A new multicast
group is not sufficient, since DNCP also uses unicast).
I recommend adding a fixed-size header with a version number.
I disagree with it. The points have been enumerated in response to
Thomas C's review, but they boil down to:
- DNCP itself has no port, nor any assignments
- big TLV space, can reuse parts of it
- DNCP profiles can define versioning schemes (and HNCP does, and now
our implementation might actually follow it, cough)
### NODE-ENDPOINT is stateful
The DNCP/HNCP protocol suite uses the elegant technique of using
recursively embedded TLVs: hop-to-hop data is in the packet toplevel,
end-to-end data is in the NODE-STATE TLV, which may in turn contain TLVs
that themselves contain embedded TLVs. The TLVs are mostly stateless, in
the sense that they can be sent in any order or even in independent
packets. NODE-ENDPOINT is an exception.
NODE-ENDPOINT identifies the sender of this packet, and applies to all
TLVs in this packet. The current specification implies that the
NODE-ENDPOINT may appear anywhere in the packet, which would force the
receiver to make two passes over the packet.
Not really. In the current specified processing flow, the only
dependency on endpoint is with the Network State TLV (and it's handling
of consistent <> inconsistent per neighbor).
Assuming you keep that state as a flag during the processing, the
Endpoint TLV itself can appear after Network State TLV too.
(This in case of datagrams; in a stream, it has to show up once, early,
and then be remembered.)
Conceptually, NODE-ENDPOINT is a packet header, and it is best treated
that way. Ideally, the information it contains would be part of the
fixed-size packet header suggested above (but after the version number,
which should be parsable even when the NODE-ENDPOINT format changes).
Alternatively, specify that NODE-ENDPOINT MUST be the very first TLV in
a packet, or at least appear before all currently-defined TLVs, which
merely formalises what existing implementations already do.
See above. If there is call for it on the list, we can change it to:
- MUST be before Network State TLV (I am fine with this, it removes one
flag from my current implementation's one-pass parsing)
- MUST be first (I strongly disagree with this design due to it
potentially being problem later - what if there is second version with
MUST have first of something else?)
### NODE-ENDPOINT is underspecified
It is not clear whether NODE-ENDPOINT is required in all packets, and if
not which TLVs are allowed in a packet without a NODE-ENDPOINT. Existing
implementations appear to differ in that respect.
Current text says MUST all (if desire bidir peer relationship), while in
truth (for protocol to work) it is likely sufficient to include it only
within subset:
- Network State TLV MUST given cursory inspection of the spec about this
(those are used for keepalives and periodic Trickling -> should be
enough to maintain the peer state)
- requests SHOULD, as it makes initial neighbor detection faster
The current spec is simpler, so I prefer that, and rather just fix
implementations.
### Node data is underspecified
The NODE-STATE TLV carries the end-to-end hash of the "node data".
However, the exact "node data" is never specified exactly. For example,
there is padding applied between TLVs (look, Ma, I've saved 4ns parsing my
packet), and it is nowhere specified whether this padding participates in
the hashing (it does).
It turns out that in the absence of fragmentation the "node data" is just
the raw binary data in the NODE-STATE TLV. This is the reasonable thing
to do, and must be specified in this manner. It must also be made clear
that this binary data MUST NOT be modified in transit (parsing/reformatting
is not likely to work reliably), and that its hash SHOULD (or is that MAY?)
be validated on reception.
Agreed. Probably SHOULD, to avoid corrupt data <> hash combinations from
spreading. Updating the text.
### Normalisation is apparently useless
DNCP specifies that the TLVs within a node state be sorted. Since both
the raw binary data and the hash are end-to-end, it is not clear why this
partial normalisation is useful.
At the very least, the draft should make it clear that this normalisation
should not be relied upon, and that peers MUST forward the binary data
unchanged. I recommend simply dropping the normalisation, although this
will require changes to the fragmentation scheme.
Disagree. It is much more efficient to have guaranteed ordering done by
publisher, and always available as such for receivers (there are amusing
hacks using this, e.g. linear time delta processing, one-pass index
creation for all packets with only one pointer or two per indexed data
type, and others). I see the benefits of saying MUST as greater than
lessening the requirements. No planned change.
### FRAGMENT-COUNT is stateful in the reverse direction
FRAGMENT-COUNT is stateful, but in the reverse direction: it changes the
interpretation of the enclosing NODE-STATE TLV. Usually, the NODE-STATE
hash carries exactly data being hashed; with a FRAGMENT-COUNT, it carries
part of that data, and FRAGMENT-COUNT is not being hashed.
FRAGMENT-COUNT is not end-to-end data, and it doesn't belong within the
NODE-STATE TLV. It should either be a field in the NODE-STATE TLV itself
(not in an embedded TLV), or put in a TLV that contains the NODE-STATE
TLV.
### Fragmentation is specified at the TLV level
The section about fragmentation is not quite clear to me. It appears to
specify fragmentation in terms of TLVs (every fragment must consist of
a valid series of TLVs), and appears to assume that the receiver is doing
something smart in order to avoid reassembly timeouts.
Unfortunately, this encoding makes it very difficult to implement
a simpler scheme, where fragmentation and reassembly are TLV-agnostic and
act entirely at the byte-stream level. In particular, a fragment TLV does
not specify an initial byte offset, and the total length of the
reassembled packet is not known beforehand, which makes buffer management
in the receiver challenging.
I recommend a TLV-agnostic fragmentation scheme, with fragment offsets
counted in octets and a total reassembled size explicitly encoded.
### The fragment timeout is not specified
It is not clear when the receiver can discard an incomplete defragmentation
buffer. This might not be an issue if a TLV-based fragmentation scheme is
used.
This sounds much more complex fragmentation scheme than what was
intended; original intent was to essentially reuse Merkle tree as parts
of node data, and without changing protocol, easiest way was to just use
concatenated sets of node data.
I guess I have to see about other reviews, but I am essentially for
removing whole fragmentation scheme, than adding something overly
complex, especially as nobody has implemented it (nor seems to have
plans for it) any time soon.
### Keep-alive intervals are flooded
The KEEP-ALIVE-INTERVAL is within the node state, and hence flooded across
the network. This information is of no interest to remote nodes, this TLV
should be within at the top level of the packet in order to reduce the
amount of information being flooded.
It is assumed to be default, which is not published. The tradeoff is
essentially between global state and on-link spam (this value is
unlikely to change, but expecting yet more storage of other nodes is
also awkward).
We prototyped both, but essentially global state is cheaper in a small
network than constant payloads on the link. Non-constant payloads on the
link make it more session-oriented than I like.
Protocol dynamics
-----------------
### Need to check sender
Most of the packets in DNCP have exactly the same meaning whether they are
sent to a unicast or a multicast address. There is one exception: the
keepalive timer is only reset for inconsistent network state when they
are sent over unicast. This requires that the receiver check the
destination address of packets, which is non-portable and might be
impossible on some systems.
Not in IPv6 at least given RFC3542 (IPV6_RECVPKTINFO).
Unicast and multicast also have different security properties (given
security, which I recommend), so I rather not change all TLVs to say 'I
am good guy, really', for example.
I recommend removing the requirement to distinguish between unicast and
multicast. If such distinction is needed, make it explicit in the TLV
contents.
The distinction is needed (I think); otherwise multicasting 'something'
(=something completely braindead and-or topology graph-wise
unidirectionally reachable now) would still be kept danngling as long as
transmissions continue.
### Not clear when multicasts and unsolicited packets can be sent
As it is written, the draft requires that all TLVs other than
NETWORK-STATE be sent over unicast and does not allow unsolicited packets
other than NETWORK-STATE. However, the draft authors indicate that
unsolicited multicast packets are allowed.
The draft specifies mandatory processing flow for particular TLVs (X
happens, do Y). Those Y replies are by unicast. It (intentionally) does
not say anything about {uni,multi}cast of X, and therefore X can come
either way. It does not say you MUST NOT multicast additionally if you
feel like it, for example.
The draft should specify clearly when multicast and unsolicited packets
are allowed. In particular, it should mention whether it is legal to
reply to a request over multicast (which may make sense, at least on some
link layers), and which packets can be sent unsolicited over multicast.
I do not think it is unclear on MUST reply via unicast. How would you
prefer it written out?
X part I am reluctant to explicitly specify, as there are already
caveats about security, profile, etc constraining the valid X set for
multicast. Those are already in the text.
### Non standard port
The draft specifies that it is required to answer a request from a node
that does not publish a node state. What should happen when this request
comes from a non-standard port? Monitoring software may need to run on
the same node as a normal DNCP implementation.
Transport specific issue, DNCP itself does not really specify port-level
semantics one way or another (HNCP might need something there though).
### Non-publishing nodes
A node may want to participate in the full protocol without publishing
a node state in order to reduce the amount of data being flooded. Doing
this naively might cause persistent state desynchronisation, leading to
repeated resetting of the Trickle timers.
Ideally, the draft would specify exactly which behaviours are allowed for
non-publishing nodes. At any rate, a warning about the dangers of
non-publishing nodes should be included in the draft.
Agreed. Updating.
Cheers,
-Markus
_______________________________________________
homenet mailing list
[email protected]
https://www.ietf.org/mailman/listinfo/homenet
