[RTG-DIR] RtgDir review: draft-ietf-idr-tunnel-encaps-19.txt

[Harish Sitaraman <[e-mail]>]

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