Last Call Review of draft-ietf-idr-tunnel-encaps-19
review-ietf-idr-tunnel-encaps-19-rtgdir-lc-sitaraman-2020-10-07-00

Request Review of draft-ietf-idr-tunnel-encaps
Requested rev. no specific revision (document currently at 20)
Type Last Call Review
Team Routing Area Directorate (rtgdir)
Deadline 2020-10-01
Requested 2020-09-17
Requested by Alvaro Retana
Authors Keyur Patel, Gunter Van de Velde, Srihari Sangli, John Scudder
Draft last updated 2020-10-07
Completed reviews Rtgdir Last Call review of -19 by Harish Sitaraman (diff)
Genart Last Call review of -19 by Gyan Mishra (diff)
Tsvart Last Call review of -19 by Brian Trammell (diff)
Opsdir Last Call review of -19 by Jouni Korhonen (diff)
Secdir Last Call review of -20 by Scott Kelly
Assignment Reviewer Harish Sitaraman 
State Completed
Review review-ietf-idr-tunnel-encaps-19-rtgdir-lc-sitaraman-2020-10-07
Posted at https://mailarchive.ietf.org/arch/msg/rtg-dir/mJ3U1_VZraPl8qTCt5qZ94Ipv78
Reviewed rev. 19 (document currently at 20)
Review result Has Issues
Review completed: 2020-10-05

Review
review-ietf-idr-tunnel-encaps-19-rtgdir-lc-sitaraman-2020-10-07

Hello,

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: https://tools.ietf.org/html/draft-ietf-idr-tunnel-encaps-19
Reviewer: Harish Sitaraman
Review Date: 10/3/2020
IETF LC End Date:
Intended Status: Standards Track

Summary:
I have some minor concerns about this document that I think should be
resolved before publication.

Comments:
The draft is detailed and section 6 is helpful to better understand
the usage. The comments below are around clarity of the text.

Major Issues:
No major issues found.

Minor Issues:

Section 1.2, last bullet: “In some cases, a two-octet length field may
be needed.” - For readability, can additional clarity be provided for
the specific cases needing more than one-octet?
Section 1.4: For readability, “MPLS-in-UDP [RFC7510] is also
supported, but an Encapsulation sub-TLV for it is not needed”, maybe
adding a note why it is not needed.
Section 3.1: I understand reserved fields are added in headers for
padding/future bits, but is there a reason in this case starting the
header with reserved?
Section 3.1: “The Tunnel Egress Endpoint sub-TLV, whose value is 6” =>
Is this referring to the “Sub-TLV Type of 6” vs. value? If yes, that
can be made clear.
Section 3.1: “The length of the sub-TLV's Value field is other than 6
plus the defined length…” - the use of plus isn't clear  - it seems
that only 6,10 and 22 are valid sub-TLV lengths. It might be easier to
mention “For address family behaviors defined in this document, if the
length of the sub-TLV's Value field is other than the following
permitted values:”
Section 3.1: Validating the Address field is mentioned as an optional
procedure and is expanded in 3.1.1: Is it possible to mention a best
practice on when this validation might be worth doing? I understand
there are security considerations that might have a limitation.
Section 3.2.3: VN-ID definition: It might be useful to clarify that
the VN-ID is the VSID for NVGRE and that it is further explained in
section 9. The last bullet of this section attempts to mention this
but for a reader not as familiar with NVGRE, it would be easier to
read.
Section 3.6: “If a packet is to be sent through the tunnel identified
in a particular TLV,…” - would it better to add a cross reference to
section 6 on how the tunnel is chosen
Section 3.6: Is there any impact to how entropy label values are
computed when the MPLS Label Stack Sub-TLV and the tunnel labels are
present?
Section 3.7: “This document defines a Prefix-SID sub-TLV” - I couldn’t
find a diagram of the format of this sub-TLV. From the reading, it
appears Label-Index and Originator SRGB formats from RFC8669 can be
used inside this sub-TLV.
Section 3.7: “The Prefix-SID sub-TLV can occur in a TLV identifying
any type of tunnel” - but the subsequent text mentions “The
corresponding MPLS label is pushed on after the processing of the MPLS
Label Stack sub-TLV”. I suppose the first sentence is only true if the
tunnel type imposes MPLS labels (e.g. MPLS, SR, etc. in
https://www.iana.org/assignments/bgp-parameters/bgp-parameters.xhtml#tunnel-types)
Section 4.3: Is there any guidance on how Color value is used or is it
same as in RFC 5512 “The color value is user defined and configured
locally on the routers.” and is opaque.

--
Harish