|
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----- 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
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;
- 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 |
-- last-call mailing list last-call@xxxxxxxx https://www.ietf.org/mailman/listinfo/last-call
