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

[<[e-mail]>]

Hi Joe,

Thanks for your review and comments.
Please see inline [Bruno]

> From: OSPF [mailto:[e-mail]] On Behalf Of Joe Touch
 > Sent: Thursday, August 24, 2017 6:39 AM
> 
 > 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 [e-mail] 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.

[Bruno]
- The two emerging examples have been removed in -07
- Current order to paragraph seems fine to me: first explaining that tunnel are used, then explaining the problematics that this document is addressing. I don't feel that swapping both would help the reader.
- "Ingress" meant "tunnel encapsulator". Would this term be better for you? I found it in RFC 7510 which did not define it in its terminology, so I would assume that this is a known term If not, would you know which RFC define it? Or would you prefer a different term? 

 
 > 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.
 
[Bruno] We can use the terms "tunnel encapsulator", "tunnel" and "tunnel decapsulator" which are used in RFC 7510. Would this be ok for you?
RFC 7510 did not redefine those terms in its terminology section Do you think that there is a need in this document? If so, can we point to RFC 7510 or would you have a better reference?

 
 > 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".
 
[Bruno] I agree that the names are confusing. We have changed some.
Proposed NEW text:

3. Tunnels Encapsulations TLV
      Routers advertise their supported tunnel encapsulation type(s) by
      advertising a new TLV of the OSPF Router Information (RI) Opaque LSA
      <xref target="RFC7770"/>, referred to as the Tunnel Encapsulations TLV.
      This TLV is applicable to both OSPFv2 and OSPFv3.
[...]
   The Type code of the Tunnels Encapsulations is TBD1, the
      Length value is variable, and the Value field contains one or more
      Tunnel Sub-TLVs as defined in <xref target="TunnelType"/>.
 

--
Although I see your point, I'm not in favor of showing the top-level OSPF RI Opaque LSA as I don't think that this would simplify this document. In addition, the encoding of this LSA is different between OSPFv2 and OSPFv3.


 > 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).
 
[Bruno]
If the length field is longer than addition of sub-TLVs, I guess that the sender implementation has a serious OSPF encoding issue. I'm more familiar with BGP error handling than the OSPF one, but I guess that the receiver can only try to interpret the additional octets as the presence of an additional sub-TLV. An unknown sub-TLVs is not an issue from a syntax standpoint, but the length of this sub-TLV may overshoot the size of the Tunnel Type TLV (IOW, the length field is now shorter, which is this opposite of your case), which makes this TLV in error.
Sub-TLV errors are already covered  ('"If a Sub-TLV is invalid, its
      Tunnel Encapsulation Type TLV MUST be ignored and skipped. However,
      other Tunnel Encapsulation Type TLVs MUST be considered.") but Tunnel Sub-TLV errors were not. I'm proposing to add a similar text in section "Tunnel Sub-TLV"
 
 > 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?)
 
[Bruno] Proposed NEW:

When a reserved  value (See <xref target="ParametersRegistry"/>) is seen in an LSA, it
      MUST be treated as an invalid Tunnel Parameter Sub-TLV. When a Tunnel Parameter Value has an incorrect syntax of semantic, it MUST be treated as an invalid Tunnel Parameter Sub-TLV. If a Tunnel Parameter Sub-TLV is invalid, its
      Tunnel Sub-TLV MUST be ignored and skipped. However,
      other Tunnel Sub-TLVs MUST be considered.

 > 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).
 
[Bruno] Texts have been changed to have theses sections defines the sub-Type. However, the syntax of the value field is indeed defined in the BGP document. The use of a reference ensures consistent specification encoding, while a copy/paste would allow both OSPF and BGP document be further modified during their process (e.g. IESG review, RFC editor) which would lead to differences.

NEW:
This Sub-TLV of type 1. The syntax, semantic and usage of its value field is defined in Section 3.2 "Encapsulation
        Sub-TLVs for Particular Tunnel Types" of <xref target="I-D.ietf-idr-tunnel-encaps".

 
 > 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).
 
[Bruno] Agreed that this could have been encoded differently.
This encoding is borrowed from RFC 5512, but indeed draft-ietf-idr-tunnel-encaps now adds an "Address Family" field.
I'm personally ok to add this "Address Family" field, but this is a non backward-compatible encoding change, so I would welcome more support from the OSPF WG.
 
 > 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.
 
[Bruno] IMO, this section does define this Color Sub-TLV:
"The color value is user-defined and configured locally on the
        advertising routers. It may be used by service providers to define
        policies on the tunnel encapsulator routers, for example, to control the
        selection of the tunnel to use."

Could you please be more specific?
	
In addition, this document refers to the Color Extended community defined in "I-D.ietf-idr-tunnel-encaps" to define the relationship between both. Arguably, this could alternatively be done in the IDR document, but in both cases, as we define a relationship between the OSPF Color Sub-TLV, used to color "underlay tunnels", and the BGP Extended Communities attached to "Overlay service routes", any document would need to refer to the other document. So IMO, it makes sense to define in the OSPF document.
 
 > 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).
 
[Bruno] I can agree with this as I've made a similar comment on the IDR document. I got the following answer "It's impossible for a single document to specify a sub-TLV for every 
encapsulation header field that might possibly be useful.  I tried to include in the document enough of a selection of sub-TLVs to indicate how more might be added.  If someone wants to signal a value for the IPv6 flow label field, it should be fairly straightforward to write a draft specifying a suitable sub-TLV."
I'm afraid that this is a valid answer. There is no requirement to specify the use of all outer header fields. Actually, it's probably better to leave this specification to the ones having the need for this.
 
 > Sec 6 - I would encourage moving this section earlier, before the
 > details of how the LSA is composed.
 
[Bruno] Noted. This is an editorial choice. On my side, I prefer separating the specification of the encoding, from the specification of the usage. I found this easier to read.
 
 > 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.

[Bruno] Experimental values is a well-Known Registration Status defined in RFC 8126 (Guidelines for Writing an IANA Considerations Section in RFCs). This RFC refers to rfc 3692 to answer your valid questions. cf end of https://tools.ietf.org/html/rfc3692#section-1.1 If this is not good enough, I'd suggest fixing the root document by writing a rfc3692bis.

As for reserved value, we have added the following text
"When a reserved value (See Section 6.2) is seen in an LSA, it MUST be treated as an invalid Sub-TLV. "

Thanks again for your careful review.

Regards,
--Bruno
 > 
 > ----
 > 
 > 
 > _______________________________________________
 > OSPF mailing list
 > [e-mail]
 > https://www.ietf.org/mailman/listinfo/ospf

_________________________________________________________________________________________________________________________

Ce message et ses pieces jointes peuvent contenir des informations confidentielles ou privilegiees et ne doivent donc
pas etre diffuses, exploites ou copies sans autorisation. Si vous avez recu ce message par erreur, veuillez le signaler
a l'expediteur et le detruire ainsi que les pieces jointes. Les messages electroniques etant susceptibles d'alteration,
Orange decline toute responsabilite si ce message a ete altere, deforme ou falsifie. Merci.

This message and its attachments may contain confidential or privileged information that may be protected by law;
they should not be distributed, used or copied without authorisation.
If you have received this email in error, please notify the sender and delete this message and its attachments.
As emails may be altered, Orange is not liable for messages that have been modified, changed or falsified.
Thank you.