Re: [Idr] Martin Vigoureux's No Objection on draft-ietf-idr-tunnel-encaps-20: (with COMMENT)

[John Scudder <[e-mail]>]

Hi Martin,

Thanks for your review. My replies are in line below.

> On Nov 29, 2020, at 5:03 PM, Martin Vigoureux via Datatracker <[e-mail]> wrote:
> 
> 
> Martin Vigoureux has entered the following ballot position for
> draft-ietf-idr-tunnel-encaps-20: No Objection
> 
> 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://urldefense.com/v3/__https://www.ietf.org/iesg/statement/discuss-criteria.html__;!!NEt6yMaO-gk!RiSoO41msSz025RVdo8Vn6O5fTwV53eLsh7OxS6kCTFGqHJia79Japjfr2qQRg$
> for more information about IESG DISCUSS and COMMENT positions.
> 
> 
> The document, along with other ballot positions, can be found here:
> https://urldefense.com/v3/__https://datatracker.ietf.org/doc/draft-ietf-idr-tunnel-encaps/__;!!NEt6yMaO-gk!RiSoO41msSz025RVdo8Vn6O5fTwV53eLsh7OxS6kCTFGqHJia79JapgpMIT-3g$
> 
> 
> 
> ----------------------------------------------------------------------
> COMMENT:
> ----------------------------------------------------------------------
> 
> Hi
> 
> thank you for the work you put in this document.
> I only have minor comments/questions (though a couple of ones might be a bit
> more important than the rest).
> 
>   Because RFC 8365 depends on RFC 5640, it is similarly obsoleted.
>   This is further discussed in Appendix A.
> But neither the abstract nor the metadata mention this, and in fact Appendix A
> is not clear about whether RFC 8365 is effectively obsoleted.

Alvaro brought this up also; I have a revision in the works that corrects the error. 

> Not critical but IANA registration of the Tunnel Encapsulation Attribute is
> with a upper-case 'A', while through out the document lower-case 'a' is used.

It’s worse than that, a quick search finds 23 instances of upper-case, and 65 of lower-case. Some of the upper-case is because of use in titles, but some isn’t. Noted to think about which to settle on, thanks. (It would come up at the RFCEd phase anyway.)

>   A Tunnel Encapsulation TLV, also known as Tunnel TLV
> but in the document you most often omit using Tunnel, thus reducing it to TLV.

I see your point but I think the usage of “TLV” is clear from context. If you think it’s a problem, let me know.

>   The Reserved subfield SHOULD be originated as zero.  It MUST be
>   disregarded on receipt, and it MUST be propagated unchanged.
> In the rest of the document, for unused bits or reserved fields you require
> MUST be zero. Any reason for different levels of requirements?

As I recall it was because we only changed this field to reserved late in the document's evolution, and if anyone had implemented it as previously specified (it used to have an AS number in it), we didn’t want to make them nonconformant.

>   The Address Family subfield contains a value from IANA's "Address
>   Family Numbers" registry.  This document assumes that the Address
>   Family is either IPv4 or IPv6; use of other address families is
>   outside the scope of this document.
> 
>   If the Address Family subfield has any value other than IPv4 or IPv6,
>   the Tunnel Egress Endpoint sub-TLV is considered "unrecognized"
> Not critical but I have the impression that these two paragraphs slightly
> contradict each other. First one allows for other AFIs to be used in the future
> in the Address Family subfield, while the second kind of rules that out. Maybe
> simply add at the beginning of the second paragraph: "In the context of this
> specification,".

Done.

>   Note that in order to send an IP packet or an MPLS packet through a
>   VXLAN tunnel, the packet must first be encapsulated in an Ethernet
>   header, which becomes the "inner Ethernet header" described in
>   [RFC7348].  The VXLAN Encapsulation sub-TLV may contain information
>   (e.g.,the MAC address) that is used to form this Ethernet header.
> This seems redundant with the last paragraph of the second bullet above (in the
> document)

I think you’re right, but the quoted paragraph seems (to my eye) to add some useful additional context, so I’m inclined to leave it in.

>   sub-TLVs must be used
> MUST?

I think the lower-case must is correct in this context.

>      0 1 2 3 4 5 6 7
>     +-+-+-+-+-+-+-+-+
>     |     0 or 1                |
>     +-+-+-+-+-+-+-+-+
> Should the values be 1 or 2 instead? At least the text says the only two
> allowed values are 1 and 2.

Oopsie. Thanks, fixed.

>   If a Label-Index is present in the Prefix-SID sub-TLV, then when a
>   packet is sent through the tunnel identified by the TLV, if that
>   tunnel is from a labeled address family
> do you mean from "a BGP UPDATE of a labeled address family" ?

Doesn’t quite scan right with the proposed change, at least not to me. Maybe my coauthors or others in the WG would like to chime in. I’ve left it as written for now.

>   document specifying such joint use should provide details
> SHOULD?

I think lower-case is OK — it’s a common (and I think fair) criticism that 2119 language should (or SHOULD :-) really only be used for something actionable by an implementor. Using it in instructions to authors of future documents feels a little futile. 

That said, if there are strong feelings on this subject, I think the change would do no harm, let me know.

>    +--------------+--------------------------------+-----------------+
>    |      0       | V (Virtual Network Identifier) | (this document) |
> 
>           +--------------+-----------------+-----------------+
>           |      0       | V (VN-ID)       | (this document) |
> 
> Purely cosmetic, but may be choose VN-ID or Virtual Network Identifier as the
> same name for the two allocations.

Made it VN-ID throughout.

> Nits
> "Value Field" in the Fig1 and Fig2 legends seems superfluous.

Why so? The diagrams don’t show the preceding type and length fields.

> s/In this case, the length field/In this case, the Length field/

Done.

> s/address subfield/Address subfield/ (6 occurrences)

Fixed five. Couldn’t find a sixth.

> s/depicted in the Address Field/depicted in the Address subfield/ (3
> occurrences, including Section 3.1.1 title)

Fixed.

> s/Tunnel Type itself/tunnel type
> itself/?

Fixed.

> s/value field/Value field/ (17 occurrences + 6 occurrences with line
> break between the two words)

I think I got ‘em all.

> s/Reserved (two fields)/Reserved (two octets)/ (twice) 

That’s just weird, I remember that notation getting added because there were two fields marked “reserved”. But there clearly aren’t now. I’m going to chalk it up to an incomplete edit, and just remove the notation. (No other field descriptions say how many octets they are.) I only found one instance, though.

> s/The flags field of the VXLAN header/The flags field of the NVGRE
> header/ (in the NVGRE section)

Fixed.

> s/Inner Destination MAC address/Inner
> Destination MAC Address/

Fixed in two places.

> s/of the cookie/of the Cookie/

I think this is correct as written since the sentence talks about “the cookie” and not “the Cookie field”.

> s/procedures of Section
> 9/procedures of Section 9)/

Fixed. While I was at it, I did s/e.g./for example/g

> s/VNI (Virtual Network Identifier)/VNI (VXLAN
> Network Identifier)/

Fixed.

Regards,

—John