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

["Rabadan, Jorge (Nokia - US/Mountain View)" <[e-mail]>]

Hi Pascal,

Thank you for the thorough review. All the changes are incorporated in version 10.

Please see in-line.
Thanks.
Jorge

From: Pascal Thubert via Datatracker <[e-mail]>
Date: Friday, October 15, 2021 at 6:44 AM
To: [e-mail] <[e-mail]>
Cc: [e-mail] <[e-mail]>, [e-mail] <[e-mail]>, [e-mail] <[e-mail]>
Subject: Intdir telechat review of draft-ietf-bess-evpn-optimized-ir-09
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) ?
[jorge] yes, I completed the relevant sentence to include them:
“AR-LEAF nodes SHALL send service-level BM control plane packets following regular IR procedures. An example would be IGMP, MLD or PIM multicast packets, and in general any packets using link-local scope multicast IPv4 or IPv6 packets. The AR-REPLICATORs MUST NOT replicate these control plane packets to other overlay tunnels since they will use the regular IR-IP Address.”


 [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.
[jorge] I added a figure and explained the difference between IR and PIM based trees in NVO networks.


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


 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.
[jorge] clarified as follows:
   The Inclusive Multicast Ethernet Tag route (RT-3) as in [RFC7432] is
   shown in Figure 2 and it is used in this document without any
   modifications to its format.  The PMSI Tunnel Attribute's (PTA)
   general format as in [RFC7432] is used in this document, only a new
   Tunnel Type and new flags are specified, as shown in Figure 3:




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

                                          0  1  2  3  4  5  6  7

   +---------------------------------+    +--+--+--+--+--+--+--+--+

   |  Flags (1 octet)                | -> |x |E |x |  T  |BM|U |L |

   +---------------------------------+    +--+--+--+--+--+--+--+--+

   |  Tunnel Type (1 octets)         |    T = AR Type

   +---------------------------------+    BM = Broadcast and Multicast

   |  MPLS Label (3 octets)          |    U = Unknown unicast

   +---------------------------------+    x = unassigned

   |  Tunnel Identifier (variable)   |

   +---------------------------------+



                   Figure 3: PMSI Tunnel Attribute (PTA)



   The Flags field in Figure 3 is 8 bits long as per [RFC7902], where

   the Extension flag (E) and the Leaf Information Required (L) Flag are

   already allocated.  This document defines the use of 4 bits of this

   Flags field, and suggests the following allocation to IANA:




Also, a figref would be nice as opposed to "below:"
[jorge] figure references added, thanks.


"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.
[jorge] I added the bit numbers in Figure 3 – see above.


"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".
[jorge] true - I changed ‘should be’ to ‘is’..


"                  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.
[jorge] I modified the text for the Replicator-AR route as follows, let me know if it helps (note that AR and IR forwarding modes are defined in the terminology section):

   -  Replicator-AR route: this route is used by the AR-REPLICATOR to

      advertise its AR capabilities, with the fields set as follows:



      o  Originating Router's IP Address MUST be set to an IP address of

         the PE that is common to all the EVIs on the PE (usually this

         is a loopback address of the PE).



         +  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.  Irrespective of the values in

            the Tunnel Identifier and Originating Router's IP Address

            fields, the ingress NVE/PE will process the received

            Replicator-AR route and will use the IP Address in the Next-

            Hop field to create IP tunnels to the AR-REPLICATOR.



         +  The Next-Hop address is referred to as the AR-IP and MUST be

            different from the IR-IP for a given PE/NVE, unless the

            procedures in Section 8 are followed.



      o  Tunnel Type = Assisted-Replication Tunnel.  Section 11 provides

         the allocated type value.



      o  T (AR role type) = 01 (AR-REPLICATOR).



      o  L (Leaf Information Required) = 0 (for non-selective AR) or 1

         (for selective AR).



   An NVE/PE configured as AR-REPLICATOR for a BD MUST advertise a

   Replicator-AR route for the BD and MAY advertise a Regular-IR route.

   The advertisement of the Replicator-AR route will indicate the AR-

   LEAFs what outer IP DA, i.e., the AR-IP, they need to use for IP

   encapsulated BM frames that use AR forwarding mode.  The AR-

   REPLICATOR will forward an IP encapsulated BM frame in AR forwarding

   mode if the outer IP DA matches its AR-IP, but will forward in IR

   forwarding mode if the outer IP DA matches its IR-IP.



Fig 1: please indicate the ACs
[jorge] added the following above the figure:
“The PEs and NVEs in the diagram have TSes or VMs connected to their ACs.”


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


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"?
[jorge] I changed to a SHOULD, thx


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.
[jorge] it refers to the one selected for a given flow or BD (if load balancing per flow or per BD is carried out). I changed the sentence to:

       o  When an AR-REPLICATOR is selected for a given flow or BD, the

          AR-LEAF MUST send all the BM packets targeted to that AR-

          REPLICATOR



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.
[jorge] added this (note that the AR-LEAF can still revert to IR in case of AR-Rep failure in the selective method):

   The Selective AR procedures create multiple AR-LEAF-sets in the EVPN

   BD, and builds single-hop trees among AR-LEAFs of the same set (AR-

   LEAF->AR-REPLICATOR->AR-LEAF), and two-hop trees among AR-LEAFs of

   different sets (AR-LEAF->AR-REPLICATOR->AR-REPLICATOR->AR-LEAF).

   Compared to the Selective solution, the Non-Selective AR method

   assumes that all the AR-LEAFs of the BD are in the same set and

   always create two-hop trees among AR-LEAFs.  While the Selective

   solution is more efficient than the Non-Selective solution in multi-

   stage IP fabrics, the trade-off is additional signaling and an

   additional outer source IP address lookup.



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
[jorge] sure, figure added.


page 17: "A Selective AR-REPLICATOR data path implementation will be compatible
   with the following rules: " MUST again? reword maybe?
[jorge] yes, I changed it to “MUST”