Subject: [OSPF] tsv-dir review of draft-ietf-ospf-encapsulation-cap-06

From: Joe Touch <[email-masked]>
Date: Wed, 23 Aug 2017 21:39:04 -0700

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 [email-masked] 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.

----

---

From: "Acee Lindem (acee)" <[email-masked]>
Date: Thu, 24 Aug 2017 20:47:28 +0000

Xiaohu and other Co-authors,

Please respond to Joe’s comments. You don’t necessarily have to address
them all as he as suggested but please respond.

Joe, 

For your comment on the introduction, can you provide text?

Thanks, 
Acee 

On 8/24/17, 12:39 AM, "OSPF on behalf of Joe Touch" <[email-masked]
on behalf of [email-masked]> wrote:

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

---

From: Xuxiaohu <[email-masked]>
Date: Mon, 28 Aug 2017 01:01:45 +0000

Hi Acee,

Sure, we will respond to all reviews soon.

Best regards,
Xiaohu

> -----邮件原件-----
> 发件人: Acee Lindem (acee) [mailto:[email-masked]]
> 发送时间: 2017年8月25日 4:47
> 收件人: Joe Touch; IETF discussion list; [email-masked];
> [email-masked]
> 抄送: TSV Dir
> 主题: Re: [OSPF] tsv-dir review of draft-ietf-ospf-encapsulation-cap-06
> 
> >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 [email-masked] if you reply
> >to or forward this review.
> >
> >"This draft has serious issues, described in the review, and needs to
> >be rethought."
> >
> >drafts). There are many better and more established examples (going
> >back to the M-Bone, the 6-Bone, etc., to VPNs, network virtualization, etc.).
> >should expand and explain what is meant (e.g., by "egress tunneling"
> >and the reason for wanting to encode tunnels as LSAs.
> >
> >terminology is relevant from that RFC. Other terms need to be defined
> >in this doc or used from others.
> >
> >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
> >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).
> >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
> >them!) or using an additional few bits (e.g., carved out of the
> >excesses noted in Sec 5).
> >
> >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.
> >
> >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.
> >
> >----
> >
> >