Telechat Review of draft-ietf-babel-rfc6126bis-11
review-ietf-babel-rfc6126bis-11-rtgdir-telechat-qu-2019-08-01-00

Request Review of draft-ietf-babel-rfc6126bis
Requested rev. no specific revision (document currently at 17)
Type Telechat Review
Team Routing Area Directorate (rtgdir)
Deadline 2019-08-06
Requested 2019-07-12
Requested by Martin Vigoureux
Authors Juliusz Chroboczek, David Schinazi
Draft last updated 2019-08-01
Completed reviews Rtgdir Early review of -04 by Susan Hares (diff)
Opsdir Early review of -04 by Mehmet Ersue (diff)
Rtgdir Early review of -04 by Nicolai Leymann (diff)
Genart Last Call review of -10 by Russ Housley (diff)
Secdir Last Call review of -10 by Charlie Kaufman (diff)
Rtgdir Telechat review of -11 by Yingzhen Qu (diff)
Assignment Reviewer Yingzhen Qu
State Completed
Review review-ietf-babel-rfc6126bis-11-rtgdir-telechat-qu-2019-08-01
Posted at https://mailarchive.ietf.org/arch/msg/rtg-dir/pjxR2t9eOjMM5JC6k8yujmSgBUE
Reviewed rev. 11 (document currently at 17)
Review result Has Issues
Review completed: 2019-08-01

Review
review-ietf-babel-rfc6126bis-11-rtgdir-telechat-qu-2019-08-01

I have been selected as the 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-babel-rfc6126bis
Reviewer: Yingzhen Qu
Review Date: 1 August 2019
Intended Status: Standards Track

Summary:
This document describes the Babel routing protocol, and obsoletes RFC 6126 and 7557.
Note: both RFC 6126 and 7557 are experimental, the current bis version is standards track. I don’t know the history behind, but just want to point it out.

Overall comments:
The document is mostly well written.
I’m not a native speaker, so I’d leave all language nits to RFC editors.

Issues: (the line number is used is from idnits)

There are security concerns raised by Secdir review: https://datatracker.ietf.org/doc/review-ietf-babel-rfc6126bis-10-secdir-lc-kaufman-2019-06-28/

The description	about packet trailer is in RFC 7557 section 2.5, but not included in the bis version. Considering this document will obsolete RFC 7557, I suppose it should be self-complete, so readers don’t need to go back to RFC 7557 for more information.

There are multiple places in the document about TLV/Sub-TLV being “self-terminating”, but I didn’t find what it means. Maybe I’m missing something here?

Section 1.1
“unmanaged and wireless environment”, what does “unmanaged” mean here?

Section 3.2.3
The interface Hello seqno is changed to outing multicast hello seqno, however there is no description about unicast hello at all. While I was reading it, I kept thinking what if it’s unicast hello? Is there a unicast hello timer needed? From later sections, I realized there are unicast hellos. So I’d suggest add some descriptions to avoid the confusion.

Section 3.2.4
There is “the expected incoming Unicast Hello sequence number” and “the outgoing Unicast Hello sequence number”, but it was not mentioned in section 3.2.3 (related with previous comment).

Section 3.8.1.1
After a node receives a route request, if the given prefix doesn’t exist in its route table, it MUST send a retraction for that prefix. So my question is whether a node is allowed to send multiple explicit requests for a given prefix? If so, what happens if both retractions and updates are received?

Section 4.4
1616	4.4.  Sub-TLV Format

1618	   Every TLV carries an explicit length in its header; however, most
1619	   TLVs are self-terminating, in the sense that it is possible to
1620	   determine the length of the body without reference to the explicit
1621	   Length field.  If a TLV has a self-terminating format, then it MAY
1622	   allow a sequence of sub-TLVs to follow the body.
This section is about Sub-TLV, however the description language in this paragraph is mainly about TLV.  

Section 4.5
it says that “an implementation may choose to use a dedicated stateless parser to parse the packet trailer”. Will the packet trailer be able to use the state parser if there are state there useful or just for implementation purpose? Although there is no packet trailer defined at this moment.

Section 4.5
1674	   parsing a TLV MUST update the parser state even if the TLV is
1675	   otherwise ignored due to an unknown mandatory sub-TLV.


Section 4.6.5
1805	             send a new scheduled Hello TLV with the same setting of the
1806	             Unicast flag.  If this is 0, then this Hello represents an
I’d suggest changing the text to “the same setting of flags” instead of “the same setting of the Unicast flag”, considering the flags will be extended later.

Nits: 

1049	   metric) from a neighbour neigh with a link cost value equal to cost,
1050	   it checks whether it already has a route table entry indexed by
[neighbour neigh] => [neighbour]


Thanks,
Yingzhen