Hi Benoit, Thanks for you review and comments. Please see inline [Bruno]
> From: Benoit Claise [mailto:bcla...@cisco.com] > Sent: Wednesday, August 30, 2017 9:57 AM > > Benoit Claise has entered the following ballot position for > draft-ietf-ospf-encapsulation-cap-06: Discuss > > When responding, please keep the subject line intact and reply to all > email addresses included in the To and CC lines. (Feel free to cut this > introductory paragraph, however.) > > > Please refer to https://www.ietf.org/iesg/statement/discuss-criteria.html > for more information about IESG DISCUSS and COMMENT positions. > > > The document, along with other ballot positions, can be found here: > https://datatracker.ietf.org/doc/draft-ietf-ospf-encapsulation-cap/ > > > > ---------------------------------------------------------------------- > DISCUSS: > ---------------------------------------------------------------------- > > 1. I agree with Tim Wicinski's OPS DIR point about IANA. > > The content appears to be fine, but there are some outdated (the biggest > one is 5226 replaced by 8126), [Bruno] Thanks. Fixed. > but its the IANA section which appears the > most confusing. > > 7.1 OSPF Router Information (RI) Registry - appears fine > > 7.2 OSPF Tunnel Encapsulation Attribute Sub-TLV Registry > > This one defines the values being defined/allocated from "This Document" > but in Section 5, each Sub-TLV is defined in other documents, so it's > totally confusing. [Bruno] Your comment is in line with other comments. We are proposing the two below changes. Would this address your point? 1) In sections 5.x, clarifies that the Type values are defined in this document. Only the Value field is defined in the BGP document. More specifically, the propose change is OLD: This Sub-TLV of type 2 is defined in Section 3.4.1 "Protocol Type sub-TLV" of <xref target="I-D.ietf-idr-tunnel-encaps"/> from a syntactic, semantic, and usage standpoint. NEW: This Sub-TLV type is 2. The syntax, semantic, and usage of its value field is defined in Section 3.4.1 "Protocol Type sub-TLV" of <xref target="I-D.ietf-idr-tunnel-encaps"/>. 2) As per other comments, draft-07 has changed to IANA section OLD: 2 Protocol Type This document NEW: 2 Protocol Type This document & [I-D.ietf-idr-tunnel-encaps] Note that I'm personally not so found of the above change as draft-ietf-ospf-encapsulation-cap does create this new registry and allocate those initial values. But I can live with those changes if people see value. > 2. It's not clear which of the following sub-TLVs are > required/relevant/interconnected in the Encapsulation Capability TLV > > 0 Reserved This document > 1 Encapsulation This document > 2 Protocol Type This document > 3 Endpoint This document > 4 Color This document > 5 Load-Balancing Block This document > 6 IP QoS This document > 7 UDP Destination Port This document > > The only hint is: > > Value (variable): Zero or more Tunnel Encapsulation Attribute Sub- > TLVs as defined in Section 5. > > Zero? really, what's the point? [Bruno] There is no point in sending zero Sub-TLVs, as per draft-ietf-ospf-encapsulation-cap. But I'm not sure to see a reason to: - forbid this usage in a future specification - create an error case with specific error handling associated. Also this text is taken from RFC 5512 section 4, which originally defined the BGP Tunnel Encapsulation Attribute https://tools.ietf.org/html/rfc5512#section-4 > Now, from an operational point of view, which sub-TLVs are required/make > sense? > Are some sub-TLVs irrelevant without others? Ex: Color without Encapsulation [Bruno] RFC5512 did not cover this point, nor does draft-ietf-idr-tunnel-encaps. In theory, as of today, Sub-TLV type 3 (Endpoint) seems required to minimally define the IP address of the tunnel decapsulator. However, I'm reluctant to define sub-TLV as REQUIRED, as future Sub-TLV/usage could possibly obsolete the initially required Sub-TLV. So we would create error condition which would possibly limit backward compatible improvement. Also the Encapsulation Sub-TLV (type 1) is required for some type of tunnels. Being a network operator, I'm all in favor of interoperability and operational aspects. However, I don't see this applicable to a generic protocol extension. I'd rather see this in applicability statements. > Could we have multiple identical sub-TLVs? Ex: Color [Bruno] Excellent question... RFC5512 did not cover this point, nor draft-ietf-idr-tunnel-encaps... None of the Sub-TLV defined in this document need more than one instance. Limitation to a single instance could be specified during the definition of the "Tunnel Encapsulation Capabilities" in section 4, which would impose such restriction for future Sub-TLV. Or for each Sub-TLV in sections 5. The former seems an easier path, while the latter more extensible. Given the example of RFC 7770 / 4970, I think I would propose the latter: i.e. allowing the advertisement of multiple instance of a Sub-TLV, but disallowing it for all Sub-TLV defined in this draft. However, I'd welcome any other opinion. I'm definitely open to follow the easier path. > > ---------------------------------------------------------------------- > COMMENT: > ---------------------------------------------------------------------- > > - Sometimes you use "Encapsulation Capability TLV" (section 3), sometimes > "The > Tunnel Encapsulation Type Sub-TLV" I guess that: OLD: > > The Tunnel Encapsulation Type Sub-TLV is structured as follows: > > 0 1 2 3 > 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1 > +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+ > | Tunnel Type (2 Octets) | Length (2 Octets) | > +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+ > | | > | Sub-TLVs | > | | > +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+ > > NEW: > The Encapsulation Capability TLV is structured as follows: > > 0 1 2 3 > 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1 > +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+ > | Tunnel Type (2 Octets) | Length (2 Octets) | > +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+ > | | > | Sub-TLVs | > | | > +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+ [Bruno] You are right that names are not consistently used. We had try to have names be aligned both with the BGP encapsulation draft and OSPF RI/TLV names, but finally, that seem difficult. Given that the IDR document is still a draft and its names could change, I think we will favor more independent names. Text changed to NEW: The Tunnel Sub-TLV is structured as follows: 0 1 2 3 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1 +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+ | Tunnel Type (2 Octets) | Length (2 Octets) | +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+ | | | Tunnel Parameter Sub-TLVs | | | +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+ > In section 7.1, should it be? > OLD: > Value TLV Name Reference > ----- ------------------------------------ ------------- > TBD1 Tunnel Capabilities This document > > NEW: > Value TLV Name Reference > ----- ------------------------------------ ------------- > TBD1 Encapsulation Capabilities This document > > OR: > Value TLV Name Reference > ----- ------------------------------------ ------------- > TBD1 Tunnel Encapsulation Capabilities This document [Bruno] Changed to NEW: TBD1 Tunnel Encapsulations This document > > - Then there is a discrepancy between Sub-TLVs and Value in the related text [Bruno] Agreed. > 0 1 2 3 > 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1 > +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+ > | Tunnel Type (2 Octets) | Length (2 Octets) | > +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+ > | | > | Sub-TLVs | > | | > +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+ > > Proposal: Sub-TLVs should be replaced by "Tunnel Encapsulation Attribute > Sub-TLVs", and the following text updated: > > Value (variable): Zero or more Tunnel Encapsulation Attribute Sub- > TLVs as defined in Section 5. [Bruno] As the names change, changed to 0 1 2 3 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1 +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+ | Tunnel Type (2 Octets) | Length (2 Octets) | +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+ | | | Tunnel Parameter Sub-TLVs | | | +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+ [...] Value (variable): Zero or more Tunnel Parameter Sub-TLVs as defined in <xref target="ParameterTLVs"/>. > - Then, reading section 5, I see yet another name: "OSPF Tunnel Encapsulation > Attribute Sub-TLVs" Section 7.2. [Bruno] That one was the name of the _IANA registry_ , not the name of the Sub-TLVs. I believe adding "OSPF" is useful as IDR and IS-IS would have a different registry. However, I'm open to different instructions. > You should re-read the document to be consistent with your naming convention, > in the text and in the IANA sections. [Bruno] Yes. Sorry for this. Names had changed. I've re-read the document. Many thanks for the review. Regards, --Bruno _________________________________________________________________________________________________________________________ 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. _______________________________________________ OSPF mailing list OSPF@ietf.org https://www.ietf.org/mailman/listinfo/ospf