Subject: [manet] comments on draft-ietf-manet-dlep-19

From: Lou Berger <[email-masked]>
Date: Fri, 4 Mar 2016 10:03:07 -0500

Hello,
    I've taken a look at the latest spec and past comments and still
have some outstanding comments.  That said, I think the document could
be published as is -- but there is room for some real improvement that I
think will benefit implementations and interoperability down the road.

I have four "main" comments and then a bunch of minor and nit comments.

1)   section 10.16 and current hearbeat support only a multisecond
failure detection period.  Is this  sufficient for all current and
future DLEP applications?  If not, a 100ms resolution (at least) should
be defined.

2)  From my original review
>     - Multiple ways to implement the same function are allowed, e.g.,
>       optional presence of Status, Interval and TCP port.  Generally
>       allowing such complicates testing and leads to interoperability
>       issues.  The document should pick one way and require it.

I'm happy to provide specific edits, removing the options, if that will
help resolve this issue. 

3) - There remains cases where  the same function/requirement defined in
two places, at least one of which is inconsistent (MAY in 1st, SHOULD in
second.) Defining formatting rules separately from processing is just
fine, but the current split/duplicate approach for rules *will* lead to
interop issues -- (Perhaps make the earlier sections informative 'theory
of operation' and section 9/2 the place with normative conformance
language.)

I'm happy to provide specific edits if that will help resolve this issue. 

> Again, this more than just a style point.  It's really hard to tell if
> all cases are covered from a review (and implementation) perspective
> with the current redundant and distributed approach to processing
> conformance.
>     ... Basically 2119 language should be used to cover
> required processing that impacts externally visible behavior.  (It can't
> apply to people or informative text.  Also each behavior should be
> formally specified only once.  Certainly narrative/informative text
> related to the processing can occur in multiple places as makes sense to
> describe the solution.
>

4) As stated in
https://www.ietf.org/mail-archive/web/manet/current/msg18328.html
> DLEP-17 describes  Error handling in terms of a fixed "Failure Mode",
> See Table 3.  I like this formalization of something that was previously
> implicit.  I'd like to suggest going on step further by making
> Terminate|Continue explicit in the Status data item.  This will both
> ease initial implementation and facilitate forward compatibility.
I think this is a future interop issue and should be changed, so would
like to see some discussion on this.  That said, I don't think this is a
*required* change for publishing.   I'd say if there is no support of
this in 1 weeks time to drop this point.

The rest of my comments are more editorial in nature, or provide
specific examples of the above comments.  Line numbers from
https://tools.ietf.org/idnits?url=https://tools.ietf.org/id/draft-ietf-manet-dlep-19.txt

Minor items that should be fixed, or at least discussed:

 - Section 3 seems really out of place and is hard to follow.  Suggest
moving to after 9 or, better yet, integrate in 5.3 or 10.

- Lines  446 and 1475/76 seem to be referring to the same behavior but
have different requirements.  (suggest consolidating)

- Section 4 - seems really out of place and is hard to follow.  Suggest
moving to before 9, integrate in the appropriate section.  Either way
the duplicate requirements in the section should be removed (e.g., see
lines 483-495 and 1145-1158)

- section 5.1 drop "on the UDP socket"

- Section 5.2 doesn't cover timeout case -- and partially duplicates
text in 10.3.  Suggest all details of processing to section 10. or
bringing forward all message processing rules to this section.

- Section 5.3, section 5.2 says a unknown metric will also result in a
state change, is this also not true for in-session state?

- Section 10 please pick one place to describe processing rules.

- section 11.  No need to repeat which messages/signals may carry the
data items as is already covered in 10.

- heuristics remain in 4 places in the document, as previously stated I
think an optional (aka default) approach these should be spelled out for
each.

Nit level issues (feel free to ignore):

-Abstract   
     -- should state what DLEP is not its motivation
-1. Intro
    -- should be broken into
    1. Intro
        states what the document defines
    1.1 Background
        Lines 158->211
    1.2 DLEP (or move to the start of section 2)
        Lines 213->291

 -   Lines 300-309 really belong in section 2.1

 -   Lines 869-880 belong in section 2.1

- Line 567 add (Defined in Section 6).

- line 717 add '  (Section 11.5)' after ' interval'

- line 791.  I assume this restriction is per modem/router pair? If so
this should be added.

- lines 823/824 lower case MAY and MUST.

- lines 945-950, this clause violates the IETF design principle aka
Postel's Maxim, that said see draft-thomson-postel-was-wrong

- Section 10 -   as signals and messages don't share the same number
space, both can start with 1 (i.e., the first message could be 1 not 3).

- line 1140, BOTH should be in lower case/

- section 10.17/18 - I'm not clear on when link characteristics metrics
would be used over destination provided metrics, but perhaps I'm missing
something obvious.

- sections 11.8->11.11, routing protocols generally treat host addresses
as prefixes with max prefix length - is there really a reason to special
case hosts addresses?

-    Section 12 is pretty weak, but let's see what the security review
says...

That's it,
Lou

---

From: Rick Taylor <[email-masked]>
Date: Fri, 4 Mar 2016 15:58:57 +0000

Thanks Lou,

I am currently going back through Chris Dearlove's excellent comments, 
and will include yours in the deep pass I'm doing.  We do have updated 
text up our sleeve which does address some of your comments already.

Meanwhile, some inline comments to most of your points...

Rick

> future DLEP applications?  If not, a 100ms resolution (at least) should

I can see your point, but you are the first to raise this.  I'm happy to 
go with WG consensus on this one.  We have 16bits currently, but we can 
go to 32bits millisecs or whatever people fancy.  Opinions please people?

> I'm happy to provide specific edits, removing the options, if that will

Interval is mandatory, but may be 0, i.e. disabled.  This is intentional 
as their may be out of band techniques to be used instead, and so I 
intend to leave it as is.

TCP Port is optional, but I believe removing it makes experimentation 
with unusual setups or system integration much harder.  I'm very happy 
to make life as simple as possible for the end user, e.g. 
peer-discovery, but the implementer has to be reasonably capable, 
checking the length field isn't really that onerous.

> I'm happy to provide specific edits if that will help resolve this issue.

These need to be sorted.  Can you detail any specific cases you see 
using the line numbers from draft-19 as you have below please, and we'll 
fix them.

> this in 1 weeks time to drop this point.

I went for a half-way house approach here, in the style of HTTP and 
other ASCII error codes: <100 = Terminate, >=100 Continue.  I previously 
proposed a bit-flag approach a-la SCTP status codes, but got push-back 
from the list.

I will need to update the IANA considerations section to make sure this 
split is made obvious.

> moving to after 9 or, better yet, integrate in 5.3 or 10.

I will admit to struggling to find the right place to put this information.
> lines 483-495 and 1145-1158)

As with Section 5, these parts have evolved in a slightly unstructured 
way over the previous 17 drafts.

> state change, is this also not true for in-session state?

That is intentional to support the case of an extension data item used 
during Session Intialization.  It's ok in that state, but not in the In 
Session state.

> data items as is already covered in 10.

Noted.

> think an optional (aka default) approach these should be spelled out for

We have tried a 'variable declaration with defaults' style to cover the 
heuristics, but it ends up being more obtuse.  I agree it could be better.

> Nit level issues (feel free to ignore):

All comments are welcome!

> _______________________________________________
> manet mailing list
> [email-masked]
> https://www.ietf.org/mailman/listinfo/manet
>

---

From: Lou Berger <[email-masked]>
Date: Fri, 4 Mar 2016 11:43:38 -0500

Rick,
    Thanks for the response -- see below.

> go to 32bits millisecs or whatever people fancy.  Opinions please people?

I think a minimum of 's/in seconds/in tenths of seconds (100s of
milisecond)/

> with unusual setups or system integration much harder. 
I suggest making tcp port mandatory, *not* dropping it.

>  I'm very happy 

yes, but testing the number of combination implied in having options
*is* onerous and does lead to interop problems.

Status is the other thing that I think should be mandatory.
 
> using the line numbers from draft-19 as you have below please, and we'll 

If you send me your working source file (word, xml, nroff, whatever)
I'll send back a marked up version.

Line numbers is too painful for this level of comments.
 
> I will need to update the IANA considerations section to make sure this 

formally splitting the range has the same effect, so WFM.

Thanks,
Lou