Last Call Review of draft-ietf-ospf-encapsulation-cap-06
review-ietf-ospf-encapsulation-cap-06-tsvart-lc-touch-2017-08-31-00

Request Review of draft-ietf-ospf-encapsulation-cap
Requested rev. no specific revision (document currently at 09)
Type Last Call Review
Team Transport Area Review Team (tsvart)
Deadline 2017-08-28
Requested 2017-08-14
Authors Xiaohu Xu, Bruno Decraene, Robert Raszuk, Luis Contreras, Luay Jalil
Draft last updated 2017-08-31
Completed reviews Opsdir Last Call review of -06 by Tim Wicinski (diff)
Tsvart Last Call review of -06 by Joseph Touch (diff)
Secdir Last Call review of -06 by David Mandelberg (diff)
Genart Last Call review of -06 by Pete Resnick (diff)
Opsdir Last Call review of -06 by Susan Hares (diff)
Assignment Reviewer Joseph Touch
State Completed
Review review-ietf-ospf-encapsulation-cap-06-tsvart-lc-touch-2017-08-31
Reviewed rev. 06 (document currently at 09)
Review result Not Ready
Review completed: 2017-08-31

Review
review-ietf-ospf-encapsulation-cap-06-tsvart-lc-touch-2017-08-31

Hi, all,

I am the assigned TSV-ART reviewer for draft-ietf-ospf-encapsulation-cap

For background on TSV-ART, please see the FAQ at
<https://trac.ietf.org/trac/tsv/wiki/TSV-Directorate>.

Please resolve these comments along with any other Last Call comments
you may receive.

Joe

------

I've reviewed this document as part of the transport area directorate's
ongoing effort to review key IETF documents. These comments were written
primarily for the transport area directors, but are copied to the
document's authors for their information and to allow them to address
any issues raised. When done at the time of IETF Last Call, the authors
should consider this review together with any other last-call comments
they receive. Please always CC tsv-dir at ietf.org if you reply to or
forward this review.

"This draft has serious issues, described in the review, and needs to be
rethought."

Although I found no transport issues, there are many fundamental
problems that need to be corrected before this document should proceed.
These are summarized below.

Sec1: The introduction jumps too quickly into a list of situations in
which tunnels are used, two of which are emerging (currently only
drafts). There are many better and more established examples (going back
to the M-Bone, the 6-Bone, etc., to VPNs, network virtualization, etc.).
Terms are used before defined (what is an ingress? are you referring to
the router or host at which a tunnel is attached, or the input to the
tunnel itself?). The last two sentences should begin the intro, which
should expand and explain what is meant (e.g., by "egress tunneling" and
the reason for wanting to encode tunnels as LSAs.

Sec2: The terminology section is incomplete. The terms tunnel, ingress,
egress, egress tunneling, etc. are not defined in RFC7770. Only the LSA
terminology is relevant from that RFC. Other terms need to be defined in
this doc or used from others.

Sec3: This section is written as if the "Encapsulation Capability TLV"
is already defined (in RFC7770), rather than being defined herein. The
section title doesn't match this name, which is confusing. The term TLV
is itself confusing, as this is really a single TLV of the RI Opaque LSA
set of types, which itself is represented as a (sub)TLV list. I
appreciate that this terminology was made confusing in previous,
established documents, but a bit of explanation would go a long way - as
would showing the top-level OSPF RI Opaque LSA and where the new TBD1
value would appear - before diving into the structure inside this new "TLV".

Sec 4: the discussion should clarify what happens if the length field is
longer than the fields indicated by the sub-TLVs (is this an error or
padding to be ignored; does the latter present a covert channel).
additionally, this section refers to RFC5512 - which is being obsoleted
by ietf-idr-tunnel-encaps - and this pending ID is used as the primary
reference in Secs 5.1 and 5.2. References to RFC5512 should be replaced
with that ID (which I'll assume will be published concurrently).

Sec 5: this section refers to the concept of an "invalid" sub-TLV; given
the previous sentence explains that unknown sub-TLVs are silently
ignored, then what exactly is an invalid sub-TLV? It further isn't clear
why 2 octets of type and 2 octets of length are needed (granted it
provides alignment, but is that really important for this sort of
application-layer protocol?)

Sec 5.1 and 5.2: it is unclear why the two fundamental sub-TLVs of this
TLV are defined elsewhere; in specific, the example of the sub-TLV
should appear here (actually for each of the sub-TLVs).

Sec 5.3 infers IP address version from length, when the version could
(should) easily encoded either as different types (you have 65K of
them!) or using an additional few bits (e.g., carved out of the excesses
noted in Sec 5).

Sec 5.4 - I had to thread through several different sections of the IDR
ID to figure out what Color meant and how it is used. Granted that is a
different problem in a different doc, IMO this doc should include enough
of a summary that this sort of 'traceback' should not be needed to
understand what is being described. Further, the other doc doesn't even
explain Color sufficiently, IMO.

Secs 5.6 and 5.7 - There are many rules for how the outer encapsulation
header is composed, and many fields it may affect beyond QoS and UDP
destination port. This section is incomplete in describing values for
the outer header(s) or relationships to the inner header(s).

Sec 6 - I would encourage moving this section earlier, before the
details of how the LSA is composed.

Sec 7 - the document should describe where experimental values
can/should be safely used and how collisions are handled; it should also
explain what happens when a reserved value is seen in an LSA.

----