[bess] Intdir telechat review of draft-ietf-bess-evpn-optimized-ir-09

[Pascal Thubert via Datatracker <[e-mail]>]

Reviewer: Pascal Thubert
Review result: Ready with Issues

https://datatracker.ietf.org/doc/draft-ietf-bess-evpn-optimized-ir/reviewrequest/15116/

Intdir Review draft-ietf-bess-evpn-optimized-ir-09

I am an assigned INT directorate reviewer for
draft-ietf-bess-evpn-optimized-ir-09.  These comments were written primarily
for the benefit of the Internet Area Directors.  Document editors and
shepherd(s) should treat these comments just like they would treat comments
from any other IETF contributors and resolve them along with any other Last
Call comments that have been received.  For more details on the INT
Directorate, see https://datatracker.ietf.org/group/intdir/about/.

I find the document to be well written but would benefit for clarification of
what IR is (pro/con vs multicast tree, which node does what) at the very
beginning. Overall I think the draft is ready with nits for publication.

High level: I'm curious about link scope IPv6 multicast packets. I understand
that MLDv2 is forwarded following regular IR procedures. Isn't that the case
for all link scope IPv6 multicast (FF02::/16) ?

 [Page 2] The introduction uses IR as if the reader new what it is before hand.
 Maybe a bit more explanantion could help. Also, a simple fig illustrating NVE
 PE etc would help figure what is and does what, in particular what to expect
 from an IR function.

 Page 5 : define PMSI, e.g., copy the terminology from
 draft-ietf-bess-evpn-bum-procedure-updates

 Page 3 : "The Inclusive Multicast Ethernet Tag route (RT-3) and its PMSI
 Tunnel Attribute’s (PTA) general format used in [RFC7432] are shown below:"

Unclear whether the below is the original format in RFC 7432 or the variation
instantiated for this document, which flags are already defined and which are
added by this spec.

For flags added for this spec to an existing field, it would make sense to add
that the flag positions are "suggested to IANA"?

Also, a figref would be nice as opposed to "below:"

"This document defines the use of 4 bits of this Flags field:"
It would be helpful to confirm the intuition that the bits are counted 0 to 7
left to right.

"MUST be set to an IP address of the PE that should be": strange construct.
The effect of the "MUST" appears destroyed by the "should".

"                  The Tunnel Identifier and
Next-Hop SHOULD be set to the same IP address as the
Originating Router’s IP address when the NVE/PE originates the
route. The Next-Hop address is referred to as the AR-IP and
SHOULD be different than the IR-IP for a given PE/NVE."
What are the consequences of not obeying those SHOULD?
Please explain there when/why the node uses both IR-IP and AR-IP. Some text here
would prepare for the reasons which can be inferred from behavior in page 11/12
but are not explicitly given.

Fig 1: please indicate the ACs

Page 11: " An AR-REPLICATOR will follow a data path implementation compatible
   with the following rules:" Should that be a MUST?

Page 13:"In case of a failure on the selected AR-REPLICATOR, another
          AR-REPLICATOR will be selected" is that a SHOULD or a "MUST if
          available"?

Page 13: "When an AR-REPLICATOR is selected, the AR-LEAF MUST send all
          the BM packets to that AR-REPLICATOR " contradicts
          "An AR-LEAF MAY select more than one AR-REPLICATOR and do
          either per-flow or per-BD load balancing."
          I guess it should say that the multicast packets are load-balanced
          across of the selected ARs using unicast underlay encapsulation.

Section 6:

Maybe indicate what the selective method does (build 2 hops trees) and the
consequence (failure if an AR prevents the AR Leaves attached to it to send and
receive multicast traffic till it's attached to a new AR.

Page 15  "Figure 1 is also used to describe the selective AR solution, however
   in this section we consider NVE2 as one more AR-LEAF for BD-1. ":
   please make that a new figure 2

page 17: "A Selective AR-REPLICATOR data path implementation will be compatible
   with the following rules: " MUST again? reword maybe?