Subject: [Last-Call] RtgDir Last Call review: draft-ietf-bess-evpn-optimized-ir-09

From: <[email-masked]>
Date: Thu, 14 Oct 2021 18:39:33 +0200

Hello,

I have been selected as the Routing Directorate reviewer for this draft. 
The Routing Directorate seeks to review all routing or routing-related 
drafts as they pass through IETF last call and IESG review. The purpose 
of the review is to provide assistance to the Routing ADs. For more 
information about the Routing Directorate, please see 
​http://trac.tools.ietf.org/area/rtg/trac/wiki/RtgDir

Although these comments are primarily for the use of the Routing ADs, it 
would be helpful if you could consider them along with any other IETF 
Last Call comments that you receive, and strive to resolve them through 
discussion or by updating the draft.

Document: draft-ietf-bess-evpn-optimized-ir-09
Reviewer: Julien Meuric
Review Date: 2021-10-12
IETF LC End Date: 2021-09-07
Intended Status: Standards Track

*Summary:*
I think the document is almost ready for publication, but a few 
clarification improvements should be considered before.

*Comments:*
First of all, I must say that I'm not an expert on NVO3 nor data 
centers. As a result, reading the document implied an extensive use of 
the terminology section at the beginning. Once through with this step, 
the document appears to be well written and specifications are clear and 
well organized. I suggest a few clarifications below.

*Major Issues:*
No major issues found.

*Minor Issues:*
- The terminology section doesn't gather all the terms/acronyms used 
along the I-D. Some are expanded on use (not necessarily the 1st one), 
some aren't. Among the missing ones I spotted: RD, TS, PMSI, Leaf A-D, 
VSID, ES, ESI, DF, NDF...
- Section 4 includes 2 naked figures. Each of them deserves a title 
(beware of the multiple references to the former Figure 1 when renumbering).
- The PTA's description mentions the T, BM, U and L flags: it would be 
valuable to include them on the figure, all the more as their position 
is already defined or even pre-existing (L).
- Section 8 tackles a particular case which may arise with "some 
software-based or low-end AR-REPLICATOR nodes" which do "not support 
more than one IP address". One could wonder if that kind of nodes would 
be appropriate to be used as AR-REPLICATORs. Anyway, assuming that 
situation can be faced, what is the expected behavior in case of common 
Ethernet Segments, i.e. if a BD falls within the scope of both section 8 
and section 9? (If nonsense, a sentence might be welcome.)
- The document describes several procedures in several steps, including 
a kind of functional description and a node-centric behavior, which are 
complementary and provide some useful rephrasing. However, they 
sometimes feel like overlapping a bit (e.g., in section 5.1, item d. and 
a few paragraphs further about "BM packet on an overlay tunnel"), or 
don't necessarily use the same level of RFC 2119 terminology. Careful 
alignment/limited duplication would be welcome there.
- Section 5.2 states that "the AR-LEAF can locally select which 
AR-REPLICATOR it sends the BM traffic to". It seems that nothing 
precludes an on-the-fly change, e.g. for administrative or policy 
reasons, but an explicit sentence could make it clearer. A similar 
comment applies to Section 6.2.
- In section 7, "a node MAY decide to honor the PFL flag". Two 
subsequent comments:
   * "a node MAY honor" is enough to describe the expectation,
   * what happens if the node doesn't honor? Is it "free to silently 
ignore" the flag?

*Nits:*

Abstract:
- EVPN isn't expanded at 1st use and isn't marked as well-know by the 
RFC Editor (though I agree it could; 
https://www.rfc-editor.org/materials/abbrev.expansion.txt).
- s/PIM (Protocol Independent Multicast) based trees/PIM (Protocol 
Independent Multicast)-based trees/

Section 1:
- s/Tenant Systems (TS) reducing/Tenant Systems (TS), reducing/

Section 2:
- BD's expansion should come earlier since it's forward referred to by 
some previous items in the list.
- The phrases "ingress traffic" and "ingress packets" are used: wouldn't 
"incoming traffic" and "incoming packets" be more appropriate?

Section 3:
- s/3. Solution requirements/3. Solution Requirements/

Section 4:
- s/Attributes for optimized-IR/Attributes for Optimized-IR/
- s/the PE's loopback address/a loopback address of the PE/
- s/different than the IR-IP/different from the IR-IP/
- s/its use SHOULD/their use SHOULD/

Section 5:
- OLD
    Unknown unicast SHALL follow
    the same path as known unicast traffic in order to avoid packet
    reordering for unicast applications and simplify the control and data
    plane procedures.
    Note that known unicast forwarding is not impacted by this solution.
   NEW
    Note that known unicast forwarding is not impacted by this solution,
    i.e. unknown unicast SHALL follow the same path as known unicast
    traffic.

- s/5.1. Non-selective AR-REPLICATOR procedures/5.1. Non-selective 
AR-REPLICATOR Procedures/
- s/ingress BM/incoming BM/
- s/to either AR-REPLICATOR or AR-LEAF/to AR-REPLICATOR or AR-LEAF/
- s/it is RECOMMENDED to use this flag for/this flag is useful for/ 
["SHOULD set" in the previous sentence is fine, RFC 2119 language to 
suggest a use case isn't appropriate]
- s/send all the BM packets to that AR-REPLICATOR/send all the BM 
packets targeted to that AR-REPLICATOR/
- s/overlay tunnel, will forward/overlay tunnel, it will forward/
- s/in section Section 5.1./in Section 5.1./
- s/5.3. RNVE procedures/5.3. RNVE Procedures/

Section 6:
- s/the AR-LEAF that requested/the AR-LEAFs that requested/
- s/6.1. Selective AR-REPLICATOR procedures/6.1. Selective AR-REPLICATOR 
Procedures/
- s/The Replicator-AR route MUST include/The AR-REPLICATOR MUST include/
- s/AR-LEAF-set (excluding the overlay tunnel to the source 
AR-LEAF)./AR-LEAF-set, excluding the overlay tunnel to the source AR-LEAF./
- s/speed up the convergence/speed up the traffic recovery/  [or 
"mitigate the traffic impact"]

Section 8:
- s/8. AR Procedures for single-IP AR-REPLICATORS/8. AR Procedures for 
Single-IP AR-REPLICATORS/

Section 10:
- s/existance/existence/
- s/this document provides a fall back/this document specifies a fall back/

Regards,

Julien