Subject: [homenet] RtgDir review: draft-ietf-homenet-dncp-07.txt

From: "Les Ginsberg (ginsberg)" <[email-masked]>
Date: Tue, 7 Jul 2015 05:25:18 +0000

Hello,

I have been selected as a 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, and sometimes on special

request. The purpose of the review is to provide assistance to the Routing ADs.

For more information about the Routing Directorate, please see

http://trac.tools.ietf.org/area/rtg/trac/wiki/RtgDir

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-homenet-dncp-07

Reviewer: Les Ginsberg

Review Date: July 6, 2015

IETF LC End Date: Seems to have already occurred??

Intended Status: Standard

Major Issues:

My biggest concern is that the document - and its companion HNCP - are not

yet mature enough to be doing last call. What is being defined here is a

"state synchronization protocol" which is used within the context of a

"parent protocol" (most interestingly a routing protocol for the homenet

context) and which depends upon another configuration protocol

(presumably HNCP) to fully define the behavior.

Judging from the review comments provided by others (notably Thomas Clausen's

detailed review) and the continued discussion on the mailing list it has not

yet been demonstrated that the specification is clear enough and robust enough

for implementations to meet all the requirements and interoperate.

This is not to suggest that you are on the wrong track - but given the

dependencies pushing this to last call seems - to put it politely -

very "ambitious". I would prefer to see more implementation experience before

the document moves to a state where it is presumed to be complete.

I still have some trouble calling this a protocol. This is more of a process -

or part of a process - which comprises a routing protocol. The process defined

here serves to support reliable distribution and synchronization of "state"

in an efficient manner under a limited set of conditions. I don't want to

quibble too much about the term "protocol" - but I would prefer something like:

"a generic set of procedures which - when supplemented by a specific profile -

define a means of maintaining state synchronization"

Some specific comments on points in the draft follow.

***************

Section 2 Terminology

The term "neighbor" is not defined - but used frequently in the document.

The term "peer" is defined as:

"another DNCP node with which a DNCP node communicates using a particular

local and remote endpoint pair."

What I am used to is that the definition above for "peer" is usually

associated with the term "neighbor", whereas the term "peer" is more generic -

it is associated with a node in the network which performs the same functions

in the protocol - but is not necessarily a neighbor.

Section 4.5 illustrates why I find this confusing as it says

"When receiving a Node Endpoint TLV... the remote node MUST be added as a peer

      on the endpoint and a Neighbor TLV (Section 7.3.2) MUST be created

      for it."

???

*******

Section 4.4 - final bullet on Page 11

   o  Any other TLV: TLVs not recognized by the receiver MUST be

      silently ignored.

Does "ignore mean "discard"? (This is one traditional meaning)

If so this seems inappropriate as it is part of the database sent by

the node and therefore needs to be retained in order to keep a consistent

database. Perhaps "store but do not process" is a more accurate behavioral

description?

********************

Section 6.1 Keep-alives

Here is another case where the confusion between "peer" and "neighbor" arises

for me. I would expect that keep-alives are only used between neighbors -

but the text here uses the term "peer".

Are keep alives sent in multicast-listener mode? From the text in 6.1.2 and

6.1.3 it seems "no" - but I am not certain.

*****************

Section 6.2 Support For Dense Broadcast Links

If a node is in Multicast-listen+unicast mode does it bear any responsibility

for publishing state data in the event the node with highest node identifier

does not have the latest information? I presume yes - but the text does not

discuss this point.

Also, does multicast-listener mode affect the way neighbors are advertised?

It seems not - so what you are preventing w multicast-listener mode is

redundant state updates - but there is no change to the set of neighbors

advertised (N*(N-1))?

********************

Section 6.3 Node Data Fragmentation

The significance of the MTU limitation is network-wide i.e. a too large

Node State TLV generated anywhere in the network could cause problems in

some other part of the network. This issue is usually not well understood

as experience w IS-IS has shown. More discussion of this point would be

helpful.

Second paragraph says

"The data within Node State TLVs of all fragments MUST be valid..."

And if it is not then all fragments are ignored/discarded??

When multiple fragments are used and the location of specific pieces of

information move from one fragment to another cases arise where some data

temporarily disappears and/or there are two copies. Are you addressing this

by expecting that a new set of fragments will not be used at all until all of

them have been received? If so, what happens if you receive some fragments

but not all of them? Is there a timeout to be applied here?

***************

Section 7 TLVs

It says "padding bytes with value zero"".

Is "0' a MUST - or is this the usual "SHOULD be transmitted as zero and

ignored on receipt"?

*******************

Section 7.2.1 Node Endpoint TLV

When is this sent and how often?

---

From: Alia Atlas <[email-masked]>
Date: Tue, 7 Jul 2015 17:09:46 -0400

Les,

Thanks very much for your  review.

Alia

On Tue, Jul 7, 2015 at 1:25 AM, Les Ginsberg (ginsberg) <[email-masked]>
wrote:

> Are keep alives sent in multicast-listener mode? >From the text in 6.1.2
> and
>
> _______________________________________________
> homenet mailing list
> [email-masked]
> https://www.ietf.org/mailman/listinfo/homenet
>
>

---

From: Steven Barth <[email-masked]>
Date: Thu, 09 Jul 2015 08:10:15 +0200

Hello Les,

thanks for your review. Please find detailed replies inline.

Cheers,

Steven & Markus

> yet mature enough to be doing last call.

Please note that DNCP does not depend on nor reference HNCP in any way. It
is a separate draft and it can and should be used not only by HNCP.
Furthermore while DNCP has been submitted to the IESG, HNCP has not and is
still in WGLC. Therefore I'd kindly ask you to keep issues separate from
one another.

> What is being defined here is a

I don't believe this to be an accurate summary. Again DNCP does not depend on HNCP,
rather the reverse is true. DNCP even defines some extensions that are not used or
intended to be used in HNCP.

Neither of the two protocols depend on any routing protocol
for correct operation. DNCP does not even mention routing anywhere in the draft.
HNCP in fact enables routing protocols to operate in typical home network scenarios.

> the document moves to a state where it is presumed to be complete.

We addressed Thomas' comments to the best of our knowledge. Please let us know,
if any of the issues still remain in the current revisions. Also, a second
independent and interoperable implementation of DNCP and (parts of) HNCP exists:
https://github.com/jech/shncpd

Comments of the author regarding clarity of the draft are already integrated
in the current revisions, some more are queued for the next.

> or part of a process - which comprises a routing protocol.
Again "comprises a routing protocol" is simply incorrect.

> The process defined

It is very hard to find a wording for this that everyone agrees with.
For now it was changed to “DNCP is an abstract protocol, that must be
combined with a specific profile to make a complete implementable
protocol.” based on a suggestion in Ole Troan’s review.

> in the protocol - but is not necessarily a neighbor.

The terms "peer" and "neighbor" are used relatively interchangeably in this
document. The terminology has been adapted to reflect that.

>       on the endpoint and a Neighbor TLV (Section 7.3.2) MUST be created

I think this particular case should actually not be confusing, since all terms
used here are defined in the Terminology or as TLVs. However, we may have a look at it again.

> database. Perhaps "store but do not process" is a more accurate behavioral

TLVs that are part of the database are sent within the Node State TLV
(Node Data field). Said Node State TLV is mentioned just before "Any other TLV"
in the same list and thus not meant to be part of "other TLV"s. The text now states this explicitly.

> Section 6.1 Keep-alives
>
> Are keep alives sent in multicast-listener mode? >From the text in 6.1.2 and

The are no multicast keep-alives in this case, please see next comment as well.

> does not have the latest information? I presume yes - but the text does not

Yes, it is implied by "SHOULD treat the endpoint as
   a unicast endpoint connected to the node that has the highest node
   identifier".

So effectively, the node with lower identifier (multicast-listen aside) treats
it as a unicast connection to the node with highest identifier, including
state synchronisation, keep-alives etc.

Maybe we can explicitly list some of those consequences, if that makes it more clear.

> redundant state updates - but there is no change to the set of neighbors

The initial paragraph states that that the main reason for this extension is
to avoid lots of redundant Neighbor TLVs. A later paragraph also states that
usage of this extension results in only a subset of the topology being advertised.
So I think this point should be pretty clear already.
As for the underlying logic, please see the comment before.

> Section 6.3 Node Data Fragmentation
The extension and thereby this section has been removed as requested by another reviewer.

> Is "0' a MUST - or is this the usual "SHOULD be transmitted as zero and

It actually says "Padding bytes with value zero MUST be added" so I guess
it should be pretty unambiguous already.

Making this a non-MUST would make some things like ordering and hashing of
TLV sets a bit awkward.

> When is this sent and how often?

This is described in detail in 4.2, a reference has been added.

---

From: Juliusz Chroboczek <[email-masked]>
Date: Thu, 09 Jul 2015 17:28:41 +0200

> It is very hard to find a wording for this that everyone agrees with.

Yes.

> protocol.”

I'm not sure that I know what an "abstract protocol" is.  I would suggest
something like the following:

  This document (DNCP) defines a distributed algorithm that can be used by
  protocols that need to flood static or slowly changing information in
  a timely manner across an IP network, as well as a suggested packet
  format to be used by such protocols.

I'd also eliminate the vague term "DNCP profile", and replace it by
"protocol using DNCP".

-- Juliusz

---

From: Ray Bellis <[email-masked]>
Date: Thu, 09 Jul 2015 16:35:27 +0100

> I'd also eliminate the vague term "DNCP profile", and replace it by

<no-hat>

The draft defines TLVs.  To me that means it's somewhat more than an
"algorithm".

In OO programming terms, it would be an "abstract" superclass of HNCP,
but it's not "pure abstract"

</no-hat>

Ray

---

From: Tim Chown <[email-masked]>
Date: Thu, 9 Jul 2015 16:40:16 +0100

Hi,

> On 9 Jul 2015, at 16:35, Ray Bellis <[email-masked]> wrote:
> 

Having reviewed the homenet prefix distribution draft for OPS-DIR, I would say that *that* is an algorithm. It mentions no protocol specifics (and is thus not something you could build an interoperable implementation from).

Tim

---

From: Juliusz Chroboczek <[email-masked]>
Date: Thu, 09 Jul 2015 17:55:32 +0200

> The draft defines TLVs.  To me that means it's somewhat more than an

Yes, which is why I wrote "as well as a suggested packet format to be used
by such protocols".

-- Juliusz

---

From: "Les Ginsberg (ginsberg)" <[email-masked]>
Date: Thu, 9 Jul 2015 19:10:58 +0000

Steven -

Thanx for the reply - inline.

> -----Original Message-----
> From: homenet [mailto:[email-masked]] On Behalf Of Steven
> Barth
> Sent: Wednesday, July 08, 2015 11:10 PM
> To: Alia Atlas; Les Ginsberg (ginsberg)
> Cc: [email-masked]; [email-masked] Group; draft-ietf-homenet-
> [email-masked]; [email-masked]
> Subject: Re: [homenet] RtgDir review: draft-ietf-homenet-dncp-07.txt
> 
> Hello Les,
> 
> > My biggest concern is that the document - and its companion HNCP - are
> > not yet mature enough to be doing last call.
> 
> Please note that DNCP does not depend on nor reference HNCP in any way.
> It is a separate draft and it can and should be used not only by HNCP.
> Furthermore while DNCP has been submitted to the IESG, HNCP has not and
> is still in WGLC. Therefore I'd kindly ask you to keep issues separate from one
> another.
> 
[Les:] We are not understanding each other. Let me try to explain more clearly than I did the first time.

DNCP has two dependencies:

1)The (routing) protocol which uses it
2)The profile "protocol" which completes the specification of what information needs to be sent by DNCP.

You are of course correct that HNCP is only one of many possible profile "protocols" - I mention it specifically because it is clearly the one being used in current prototyping/testing.

To have high confidence that the DNCP specification is complete we would like to have experience w multiple routing protocols/profile protocols. So...

In the best of all worlds we would have development experience w multiple routing protocols using multiple profile protocols in combination w DNCP.

Second best would be multiple implementations of a single routing protocol/single profile protocol using  DNCP.

A distant third is a single implementation of a routing protocol/single profile protocol  using DNCP.

It seems to me that we are somewhere between #2 and #3 today - and HNCP itself is still under construction. This makes me concerned that we don't really know what gaps/problems might exist in DNCP because we simply don't have enough experience with it yet.

HTH

> I don't believe this to be an accurate summary. Again DNCP does not depend
> on HNCP, rather the reverse is true. DNCP even defines some extensions
> that are not used or intended to be used in HNCP.
> 
> Neither of the two protocols depend on any routing protocol for correct
> operation. DNCP does not even mention routing anywhere in the draft.
> > Judging from the review comments provided by others (notably Thomas
> > Clausen's detailed review) and the continued discussion on the mailing
> > list it has not yet been demonstrated that the specification is clear
> > enough and robust enough for implementations to meet all the
> requirements and interoperate.
> >
> > very "ambitious". I would prefer to see more implementation experience
> > before the document moves to a state where it is presumed to be
> complete.
> 
> We addressed Thomas' comments to the best of our knowledge. Please let
> us know, if any of the issues still remain in the current revisions. Also, a
> second independent and interoperable implementation of DNCP and (parts
> of) HNCP exists:
> > I still have some trouble calling this a protocol. This is more of a
> > process - or part of a process - which comprises a routing protocol.
> > in an efficient manner under a limited set of conditions. I don't want
> > to quibble too much about the term "protocol" - but I would prefer
> something like:
> >
> > "a generic set of procedures which - when supplemented by a specific
> > profile - define a means of maintaining state synchronization"
> 
> For now it was changed to "DNCP is an abstract protocol, that must be
> protocol." based on a suggestion in Ole Troan's review.
> 
> 
> 
> > Section 2 Terminology
> >
> >
> > "another DNCP node with which a DNCP node communicates using a
> > particular local and remote endpoint pair."
> >
> > associated with the term "neighbor", whereas the term "peer" is more
> > generic - it is associated with a node in the network which performs
> > the same functions in the protocol - but is not necessarily a neighbor.
> 

[Les:] Well "neighbor" is not defined - though yours would not be the only specification to omit that definition. And I could intuit using "peer" to identify other nodes operating the protocol who are not necessarily neighbors - but that can't be done using your definition of "peer".

> > the node and therefore needs to be retained in order to keep a
> > consistent database. Perhaps "store but do not process" is a more
> > accurate behavioral description?
> 
> TLVs that are part of the database are sent within the Node State TLV (Node
> Data field). Said Node State TLV is mentioned just before "Any other TLV"
[Les:] But there can be TLVs within the Node State TLV - and these cannot be discarded - though they could be ignored. So there seem to be two classes of "unrecognized TLVs" - those within the Node State TLV and those not associated with the Node State TLV.
??

> > If a node is in Multicast-listen+unicast mode does it bear any
> > responsibility for publishing state data in the event the node with
> > highest node identifier does not have the latest information? I
> > presume yes - but the text does not discuss this point.
> 
> So effectively, the node with lower identifier (multicast-listen aside) treats it
> as a unicast connection to the node with highest identifier, including state
> synchronisation, keep-alives etc.
> 
> > redundant state updates - but there is no change to the set of
> > neighbors advertised (N*(N-1))?
> 
> The initial paragraph states that that the main reason for this extension is to
> avoid lots of redundant Neighbor TLVs. A later paragraph also states that
> > Is "0' a MUST - or is this the usual "SHOULD be transmitted as zero
> > and ignored on receipt"?
> 
> It actually says "Padding bytes with value zero MUST be added" so I guess it
> should be pretty unambiguous already.
> 
> Making this a non-MUST would make some things like ordering and hashing
> of TLV sets a bit awkward.
> 
> 
[Les:] Ahhh - thanx for the pointer. I searched for "Node Endpoint" - but Section 4.2 uses simply "Endpoint" - so I did not find a match. If you could use the full TLV name in Section 4.2 that would be helpful.

 Thanx.

     Les

---

From: Brian E Carpenter <[email-masked]>
Date: Fri, 10 Jul 2015 13:26:18 +1200

(Cc's restored)

> Yes, which is why I wrote "as well as a suggested packet format to be used

It isn't a "suggested" TLV format. It's fully specified with IANA
Considerations and all that. IMHO this is clearly a protocol, although
I share Les's doubts about its maturity.

On the other hand, it's only going for Proposed Standard at first, and
it isn't a routing protocol, although it walks and quacks* a bit like one.

https://en.wikipedia.org/wiki/Duck_test

   Brian

---

From: Steven Barth <[email-masked]>
Date: Fri, 10 Jul 2015 07:30:19 +0200

Hello Les,

> 1)The (routing) protocol which uses it
I'm sorry, but I can't seem to follow your logic.
Are you thinking of a DNCP-based routing protocol here?
But even then how can a thing using DNCP be a dependency of DNCP?

> 2)The profile "protocol" which completes the specification of what information needs to be sent by DNCP.
Let me tackle this another way: Trickle does not depend on DNCP, but DNCP depends on Trickle. That's why the DNCP specification has a Normative Reference on Trickle, but not the other way around. Similarly RPL uses Trickle, but that doesn't affect Trickle either and does not make it responsible if something was missing in RPL.

> To have high confidence that the DNCP specification is complete we would like to have experience w multiple routing protocols/profile protocols. So...
I don't see how building routing protocols out of DNCP proofs or disproofs anything. Furthermore, in general how could adding something proof or disproof the completeness of it, especially if the original thing is deliberately abstract?

Or do you think existing routing protocols have any impact on DNCP? That is not the case either, i.e. DNCP-based profiles can rely purely on link-local communication - like HNCP does - and are thus unaffected by presence or absence of routing protocols.

Or are you referring to mentions of "IGP" and "routing protocol" in the HNCP draft?
If so, then these are unrelated to DNCP since they are in a separate draft. However even for HNCP there is not much sense, since it merely tells the router whether to run or not run any routing protocol on a network interface. It can also optionally provide a pre-shared key to use for authentication, but that's about it.

> This makes me concerned that we don't really know what gaps/problems might exist in DNCP because we simply don't have enough experience with it yet.
You may want to scan through https://www.ietf.org/id/draft-jin-homenet-dncp-experience-00.txt for  some up-to-date information on the current state of DNCP and HNCP implementations.
This draft also includes simulation results using a relatively bare-metal DNCP-profile (e.g. just using DNCP-TLVs IIRC).

> [Les:] Well "neighbor" is not defined - though yours would not be the only specification to omit that definition. And I could intuit using "peer" to identify other nodes operating the protocol who…
The sentence you quoted didn't have the term "neighbor" in it but rather "Neighbor TLV" which is clearly defined as one of the TLVs in the document.

> [Les:] But there can be TLVs within the Node State TLV - and these cannot be discarded - though they could be ignored. So there seem to be two classes of "unrecognized TLVs" - those within the Node…
Yes, "Any other TLVs" as well as all the other elements of that enumeration are referring to top-level TLVs, not nested ones.

I currently propose "TLVs not recognized by the receiver MUST be
      silently ignored unless they are sent as part of the payload of
      one of the TLVs above (such as TLVs in the Node Data field of a
      Node State TLV)." to make that more clear.

Furthermore the definition of the Node State TLV already states that the Node Data content MUST not be modified, so it should be fine.

> [Les:] Ahhh - thanx for the pointer. I searched for "Node Endpoint" - but Section 4.2 uses simply "Endpoint" - so I did not find a match. If you could use the full TLV name in Section 4.2 that woul…
Yes, I noticed that as well when adding the reference and corrected it.

Cheers,

Steven