Hi Julien, Thanks very much for the review. Great points.
We took all your suggestions and incorporate them in version 10. Please see the resolutions in-line below with [jorge]. Thanks! Jorge -----Original Message----- From: julien.meu...@orange.com <julien.meu...@orange.com> Date: Thursday, October 14, 2021 at 12:39 PM To: rtg-...@ietf.org <rtg-...@ietf.org> Cc: rtg-...@ietf.org <rtg-...@ietf.org>, draft-ietf-bess-evpn-optimized-ir....@ietf.org <draft-ietf-bess-evpn-optimized-ir....@ietf.org>, bess@ietf.org <bess@ietf.org>, last-c...@ietf.org <last-c...@ietf.org> Subject: RtgDir Last Call review: draft-ietf-bess-evpn-optimized-ir-09 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... [jorge] added. Thanks. - Section 4 includes 2 naked figures. Each of them deserves a title (beware of the multiple references to the former Figure 1 when renumbering). [jorge] done. Thanks. - 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). [jorge] added, thanks. - 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.) [jorge] excellent point. I modified section 9.2 and added bullet ‘d’, as follows: a. For BUM traffic received on a local AR-REPLICATOR's AC, "Local- Bias" procedures as in [RFC8365] SHOULD be followed. b. For BUM traffic received on an AR-REPLICATOR overlay tunnel with AR-IP as the IP DA, "Local-Bias" SHOULD also be followed. That is, traffic received with AR-IP as IP DA will be treated as though it had been received on a local AC that is part of the ES and will be forwarded to all local ES, irrespective of their DF or NDF state. c. BUM traffic received on an AR-REPLICATOR overlay tunnel with IR- IP as the IP DA, will follow regular [RFC8365] "Local-Bias" rules and will not be forwarded to local ESes that are shared with the AR-LEAF or AR-REPLICATOR originating the traffic. d. In cases where the AR-REPLICATOR supports a single IP address, the IR-IP and the AR-IP are the same IP address, as discussed in Section 8. The received BUM traffic will be treated as in 'b' above if the received VNI is the AR-VNI, and as in 'c' if the VNI is the IR-VNI. - 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. [jorge] ok, I aligned the normative language in 5.1, 5.2, 6.1 and 6.2. Thanks. - 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. [jorge] fair point. Added this in both sections: “An AR-LEAF MAY change the AR-REPLICATOR(s) selection dynamically, due to an administrative or policy configuration change.” - 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? [jorge] ok, I changed it to: The ability to signal and process these PFL flags SHOULD be an administrative choice. If a node is configured to process the PFL flags, upon receiving a non-zero PFL flag the node will remove the sender from the corresponding flood-list. A node not following this document or not configured for this optimization will ignore any of the received PFL bits. *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). [jorge] added - s/PIM (Protocol Independent Multicast) based trees/PIM (Protocol Independent Multicast)-based trees/ [jorge] fixed Section 1: - s/Tenant Systems (TS) reducing/Tenant Systems (TS), reducing/ [jorge] fixed Section 2: - BD's expansion should come earlier since it's forward referred to by some previous items in the list. [jorge] fixed - The phrases "ingress traffic" and "ingress packets" are used: wouldn't "incoming traffic" and "incoming packets" be more appropriate? [jorge] fixed Section 3: - s/3. Solution requirements/3. Solution Requirements/ [jorge] fixed 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/ [jorge] fixed 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. [jorge] done - 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/ [jorge] fixed 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"] [jorge] fixed, except for the “Replicator-AR routes” phrase, which is correct. Section 8: - s/8. AR Procedures for single-IP AR-REPLICATORS/8. AR Procedures for Single-IP AR-REPLICATORS/ [jorge] fixed Section 10: - s/existance/existence/ - s/this document provides a fall back/this document specifies a fall back/ [jorge] fixed Regards, Julien
_______________________________________________ BESS mailing list BESS@ietf.org https://www.ietf.org/mailman/listinfo/bess
