[Int-area] AD review of draft-ietf-intarea-gue

["Eric Vyncke (evyncke)" <[e-mail]>]

Tom, Lucy, Osama,

As you probably know by now, I have become the responsible AD for this document for a couple of months (since I replaced Suresh who is in cc).

As usual, I do an AD review before launching the IETF Last Call step. So, here are my comments.

The first one is really about what is the added value of GUE wrt to so many already specified encapsulations. Of course, UDP-encapsulation guarantees a better middle-box traversal (being NAT or firewall or ...) but L2TPv2, Geneve/NVO3, IPsec, VxLAN, GRE over UDP, ... already apply the same UDP envelope (albeit for very specific tunneling techniques and to encapsulate a layer-2 frame). As the intarea WG has adopted this document and the WGLC was positive, I will go forward with the publication process anyway; but, I fear that the IETF Last Call and IESG evaluation will bring this discussion again. Section 6 should perhaps be moved forward as well.

I failed to see any reply to David Blake's early review for tsvart: https://mailarchive.ietf.org/arch/msg/int-area/Z67X78mJyzijDmt4GlCktndInzI/

In Tom's reply to Gory's review in https://mailarchive.ietf.org/arch/msg/int-area/66jAlVmJ-JqO_yy9utivM6bGhHg/ there is "I will reply to comments in time", and I have not seen a reply (possibly a bad search of mine).

In one email from Tom, it seems that GUE is implemented in Linux for years. This could become part of an implementation status section that is always useful for an intended PS status.

Where is described the process of receiving an ICMP by the encapsulator about a GUE packet?

Abstract: I wonder what is "specialized capabilities in networking hardware for efficient handling of UDP packets" except perhaps for ECMP ? Probably worth being specific even in the abstract.

Introduction: the text is about 'optional data' and 'extensions' and is not really clear whether they are identical, especially when the text is split in two paragraphs. Suggest to merge the last 2 paragraphs.

Section 1.2: why is this 'implicitly' for 'control' rather than 'explicitly' and nothing is said for 'data'. C-bit is an 'explicit' way of declaring this packet as 'control', or did I miss something?

Section 1.2: "control the state or behavior of a tunnel", humm I had in mind that GUE is not only for tunnels

Section 1.2: suggest to also define primary and extension fields (even if those terms can be inferred/guessed)

Section 1.2: "Proto/ctype" there is no protocol number in IPv6

Section 1.3: please use BCP 14 and RFC 8174

Section 3.1: about the UDP source port, is a 'RECOMMENDED' recommended in the choice of source port ?

Section 3.1: please add "and ignored at reception" when describing the 'Flags' field.

Section 3.2.1 in the last paragraph: 'nodes' is rather vague... is it the decapsulating node ? And beside 'MUST NOT parse', should the packet be dropped silently ? or dropped and ICMP generated ? or ?

Section 3.2.2: "Control messages will be defined in an IANA" should rather be "Control messages are defined in an IANA". Also, "and may be defined in standards" is not the usual wording for extensions.

Section 3.3: draft-ietf-intarea-gue-extensions seems to be expired... and MUST be normative, IMHO, as " [GUEEXTEN] defines an initial set of GUE extensions." indicates it.

Section 3.3.1: while the exact meaning of "Extension fields are placed in order of the flags" can be guessed, strongly suggest to be clear about this 'order of the flags'.

Section 3.2.2: unsure whether a copy & paste of another not-really-active document is wise. Suggest to remove this section completely.

Section 3.4: is "optional integrity check" defined in this document ? Else, I suggest to remove this text

Section 3.5.1: "implicitly" or "explicitly" because C-flag is set and the dest address is obviously the decapsulator

Section 3.5.2: we are well deep in the document and it is still unclear how the encapsulation can be done with variant 0. Is it a 'virtual' removing of UDP & GUE header ? I.e., leaving the outer IP header intact ?

Section 4: it is really smart to use the IP version field to indicate variant 1 (assuming that Internet Stream Protocol is no more used)

Section 5.2: why "should be co-resident with the hosts" ? Isn't it obvious then, rather than using "should be", let's use "are"

Section 5.2: "and the transport packet." how would ICMP qualify as it is rather layer 3 and not really 'transport' ?

Section 5.2: "Note that the transport layer ports " => "Note that the transport layer ports (if any) "

Section 5.2: I am afraid that some text about IPv6 extension headers is required here (and possibly for IPv4 options)

Section 5.3: "intended target (decapsulator)" why not simply writing "decapsulator" ?

Section 5.3: suggest s/it SHOULD follow standard conventions /it MUST follow standard conventions /

Section 5.3: must include the normative references to 'standard conventions'

Section 5.3: what about MTU considerations as packet size is increased ?

Section 5.4: should an ICMP message be generated when packet is not validated and should it be dropped ? rate limited of course ;-)

Section 5.4: s/GUE packet is received by a decapsulator with unknown flags, the packet MUST be dropped./GUE packet is received by a decapsulator with unknown flags set, the packet MUST be dropped/

Section 5.4: should an ICMP message be generated when dropping GUE packets ?

Section 5.4.1: must be explicit about the processing of any IPv6 extension headers

Section 5.4.1: suggest to also indicate the inner/outer src and dst IPv4 addresses

Section 5.4.2: should/may an ICMP be generated ?
"
Section 5.5: unsure whether a section about middle box is required/useful because those will do whatever they want (typically evil). Also, "A middlebox MUST NOT drop a GUE packet merely because there are flags unknown to it." is wishful thinking as firewalls tend to block what they do not recognize.

Section 5.6.2: would a section on NAT64 be useful ?

Section 5.7: RFC 4459 is informational and will cause a down ref (that is OK) as the reference should be normative. As mentioned by David Blake in his review, RFC 4459 is about to be updated by draft-ietf-intarea-tunnels, the later should probably be mentioned as well.

Section 5.8.2: replace RFC 2460 by RFC 8200 ;-)

Section 5.8.2: while I am unsure whether the considerations about when using no UDP checksum are relevant/useful, it is probably shared by IPv4 and IPv6, so, move this part outside of 5.8.2 ?

Section 5.9.1: up to the authors, but I would use a "SHOULD NOT" in "a GUE tunnel MUST NOT be deployed to carry non-congestion-controlled traffic over the Internet"

Section 5.10: even after reading carefully the document, I am unclear about what are the "GUE parameters and properties" required to decapsulate

Section 5.10: is it useful to specify whether anycast traffic is supported?

Section 5.11.1: is "connection semantics"  already defined? Not the first time it is mentioned but its definition should either occur before or a forward internal reference pointer added

Section 5.11.1: "corresponds to the flow of the inner packet" seems to indicate a 1:1 mapping between outer source port and internal 5-tuple. Suggest to use a different word than "corresponds"

Section 5.11.1: "is set" or "SHOULD be set" ?

Section 5.11.1: please add references to ESP & AH RFC

Section 5.11.2: would it be useful to specify that the "flow entropy" should be constant for the same inner flow (= 5-tuple) ?

Section 6: this is really a useful section, IMHO, it should appear earlier in the document.

Section 6.1: "Standardized optional data ... are defined" ... where ?

Section 6.2: I would try to prioritize the list, i.e., imho flow entropy is important, the multiplexing also exists notably in GRE so is less important.

IANA: please use RFC 8126 rather than RFC 5226 ;-)

Possibly a couple of nits (but please bear in mind that English is not my native language):

  *   s/UDP based protocol/UDP-based protocol/
  *   s/four byte header/four-octet header/ (at least use 'octet' rather than 'byte' as some ADs are very strict on this use)
  *   Section 2: about OAM, typically we use the expanded version and put the acronym later, not the other way round.


Hope this helps

-éric