[bess] Re: Rtgdir early review of draft-ietf-bess-evpn-geneve-07

["Matthew Bocci (Nokia)" <[e-mail]>]

Jonathan

Thanks for your review.

Authors: I intend to WG LC the draft soon, but please can you address the comments below before I start the WG LC.

Regards

Matthew




From: Jonathan Hardwick via Datatracker <[e-mail]>
Date: Wednesday, 5 June 2024 at 14:02
To: [e-mail] <[e-mail]>
Cc: [e-mail] <[e-mail]>, [e-mail] <[e-mail]>
Subject: Rtgdir early review of draft-ietf-bess-evpn-geneve-07

CAUTION: This is an external email. Please be very careful when clicking links or opening attachments. See the URL nok.it/ext for additional information.



Reviewer: Jonathan Hardwick
Review result: Has Issues

Document: draft-ietf-bess-evpn-geneve-07
Reviewer: Jon Hardwick
Review Date: 5 June 2024

Summary:
I have some minor concerns about this document that I think should be resolved
as part of the normal WGLC process.

Comments:
---

Section 1 - please could you also mention that you are adding a new Ethernet
Option TLV to Geneve (i.e. the thing in section 4) to account for BUM traffic
and split-horizon control?

Section 1 final paragraph - Introduction section is not the place for normative
statements. Suggest removing this sentence ("a transmitting NVE MUST NOT...")
as you say the same thing in 5.1.

Section 4.1 - since Length field is given in 4-byte multiples, please could you
change "length=4" to either "length=4 octets" or "length=0x1"? Similar for
"length=8".

Section 4.1 - The B,L,R bits are being defined where these three bits were
previously reserved, is that right?  I assume you are doing this just for
option-class=Ethernet and that they continue to be reserved for other option
classes?  Given that these reserved bits are part of the common Option TLV
header I'm not sure it's correct to give them a meaning just for one Option TLV
type and not others.  This is likely to lead to confusion, at least.  I suggest
that they should be part of the Option TLV payload.  That is, make the Ethernet
Option TLV always like Figure 2 (8 octets long), with the new flags in the Rsvd
field, and an extra flag to indicate whether the ESI label is included or not.

Section 4.1 - I wonder if you should use a different letter for the
Root-Indication flag, as R is commonly used to mean "Reserved" (particularly by
RFC 8926).

Section 4.1 - "Type is set to EVPN-OPTION with value = 0" - do you just mean "
Type is set to 0"?

Section 5 para 2 - typo "Typei"

Section 5.1 diagram - I believe that length is always 2 octets and never 1
octet, since the type is in the range 192-252 (any type >128 has a 2-octet
length per RFC 9012).

Section 5.1 - Regarding the following sentence:
BEGIN QUOTE
   An NVE receiving the above sub-TLV, MUST send Geneve packets to the
   originator NVE with only the option TLVs the receiver NVE is capable
   of receiving, and following the same order.
END QUOTE

Does the "receiver NVE" in this sentence (2nd line) refer to the NVE that
receives the Geneve Tunnel Option Types sub-TLV or to the NVE that receives the
subsequent Geneve packets (that is, the one referred to in the same sentence as
the "originator NVE")?  I think it must be the latter - "receiver NVE" and
"originator NVE" in this sentence are the same NVE.  If so, that's confusing!
Here is a suggested rewording.

BEGIN NEW:
   An NVE informs its peers which Geneve option TLVs it can receive
   by including the first 4 bytes of each option TLV in the Geneve Tunnel
   Option Types sub-TLV.  The peers MUST send Geneve packets to this
   NVE with only the option TLVs that it has specified here, following the
   same order.
END NEW:

Section 5.1 - the final sentence needs rewording because it makes two different
normative statements. OLD:
  "The above sub-TLV(s) MAY be included with only Ethernet A-D per-ES routes"
NEW:
  "The above sub-TLV(s) MAY be included with Ethernet A-D per-ES routes and
  MUST NOT be included with other routes."