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.
