Hi Lars Thank you for your careful review and valuable comments. We have addressed them in version 10 of the draft.
Pls also see inline. Thanks Parag From: Lars Eggert via Datatracker <[email protected]> Date: Monday, April 24, 2023 at 9:29 AM To: The IESG <[email protected]> Cc: [email protected] <[email protected]>, [email protected] <[email protected]>, [email protected] <[email protected]>, Matthew Bocci <[email protected]>, [email protected] <[email protected]> Subject: Lars Eggert's Discuss on draft-ietf-bess-evpn-lsp-ping-09: (with DISCUSS and COMMENT) Lars Eggert has entered the following ballot position for draft-ietf-bess-evpn-lsp-ping-09: Discuss When responding, please keep the subject line intact and reply to all email addresses included in the To and CC lines. (Feel free to cut this introductory paragraph, however.) Please refer to https://www.ietf.org/about/groups/iesg/statements/handling-ballot-positions/ for more information about how to handle DISCUSS and COMMENT positions. The document, along with other ballot positions, can be found here: https://datatracker.ietf.org/doc/draft-ietf-bess-evpn-lsp-ping/ ---------------------------------------------------------------------- DISCUSS: ---------------------------------------------------------------------- # GEN AD review of draft-ietf-bess-evpn-lsp-ping-09 CC @larseggert Thanks to Joel Halpern for the General Area Review Team (Gen-ART) review (https://mailarchive.ietf.org/arch/msg/gen-art/o65AsTa-IlE5tq5kZy_2kZ80bfE). ## Discuss ### Section 4.1, paragraph 3 ``` The EVPN MAC/IP Sub-TLV fields are derived from the MAC/IP Advertisement route defined in [RFC7432] Section 7.2 and have the format as shown in Figure 1. This Sub-TLV is included in the Echo Request sent by a PBB-EVPN/EVPN PE to a Peer PE. IP Address field in this Sub-TLV is optional. ``` The field may be derived from the one in RFC7432 in terms of what it contains, but this specification still needs to define it precisely, e.g., say what unit the Mac Addr Len is in, what should be done with the MBZ field on receive, etc. paragj> Added unit of mac addresses, prefix len and other fields in the sub-TLVs. Added description of the behavior for MBZ fields as follows: The Must Be Zero fields are set to 0. The receiving PE should ignore the Must Be Zero fields. ### Section 4.2, paragraph 1 ``` The EVPN Inclusive Multicast Sub-TLV fields are based on the EVPN Inclusive Multicast route defined in [RFC7432] Section 7.3. ``` As above, the field may be derived from the one in RFC7432, but this specification still needs to define it precisely, e.g., say what unit the IP Addr Len is in, etc. paragj> described all the fields of the sub-TLVs. ### Section 4.4, paragraph 2 ``` The EVPN IP Prefix Sub-TLV fields are derived from the IP Prefix Route (RT-5) advertisement defined in [RFC9136] and applies to only EVPN. This Sub-TLV has the format as shown in Figure 4. ``` As above, the field may be derived from the one in RFC9136, but this specification still needs to define it precisely, e.g., say what unit the IP Prefix Len is in, paragj> > described all the fields of the sub-TLVs. what should be done with the MBZ field on receive, etc. paragj> added description of the behavior for MBZ fields as follows: The Must Be Zero fields are set to 0. The receiving PE should ignore the Must Be Zero fields. Also, any reason why the order of Ethernet Tag ID and Ethernet Segment Identifier is swapped compared to RFC9136 Section 3.1? paragj> this was done for 32 bit alignment and also wanted to keep IP Prefix Len and IP Prefix fields next to each other. ---------------------------------------------------------------------- COMMENT: ---------------------------------------------------------------------- ## Comments ### Section 4.1, paragraph 4 ``` Figure 1: EVPN MAC Sub-TLV format ``` Why are there two "Must Be Zero" bytes? RFC7432 doesn't seem to have these. Paragj> this is for 32 bit alignment of the EVPN MAC/IP Sub-TLV defined in this draft. ### Section 4.3, paragraph 5 ``` Figure 3: EVPN Ethernet Auto-Discovery Sub-TLV format ``` Why is there a "Must Be Zero" field? Paragj> this is for 32 bit alignment of the EVPN Ethernet Auto-Discovery Sub-TLV defined in this draft. ### Section 4.4, paragraph 3 ``` Figure 4: EVPN IP Prefix Sub-TLV format ``` Why is there a "Must Be Zero" field? Paragj> this is for 32 bit alignment of the EVPN IP Prefix Sub-TLV defined in this draft. ### Missing references No reference entries found for these items, which were mentioned in the text: `[RFC8174]` and `[RFC2119]`. paragj> added the references. ## Nits All comments below are about very minor potential issues that you may choose to address in some way - or ignore - as you see fit. Some were flagged by automated tools (via https://github.com/larseggert/ietf-reviewtool), so there will likely be some false positives. There is no need to let me know what you did with these suggestions. ### Typos #### "Abstract", paragraph 1 ``` - mechanisms for detecting data plane failures using LSP Ping in MPLS + mechanisms for detecting data plane failures using LSP Ping in MPLS- + + ``` #### Section 1, paragraph 1 ``` - [RFC7432] describes MPLS based Ethernet VPN (EVPN) technology. An - ^ + [RFC7432] describes MPLS-based Ethernet VPN (EVPN) technology. An + ^ ``` #### Section 1, paragraph 3 ``` - belonging to the same Forwarding Equivalent Classs (FEC). The Echo - - ``` #### Section 1, paragraph 3 ``` - packet contains sufficient infiormation to verify correctness of data - - ``` paragj> fixed. #### Section 3, paragraph 10 ``` - FEC: Forwarding Equivalent Classs - - ``` #### Section 4, paragraph 1 ``` - an indentifier for a given EVPN is programmed at the target node. - - ``` paragj> fixed. #### Section 4.3, paragraph 5 ``` - EVPN Ethernet AD Sub-TLV can be sent in the context of per-Ethernet - Segememnt (ES) or per-EVI. When an operator performs a connectivity - - - + EVPN Ethernet AD Sub-TLV can be sent in the context of per-Ethernet- + + ``` paragj> fixed. ### Grammar/style #### "Table of Contents", paragraph 1 ``` E(s) in the control plane using multi-protocol BGP. EVPN enables multihoming ^^^^^^^^^^^^^^ ``` This word is normally spelled as one. paragj> fixed. #### Section 1, paragraph 1 ``` ing LSP Ping in MPLS network are well defined in [RFC8029] and [RFC6425]. The ^^^^^^^^^^^^ ``` This word is normally spelled with a hyphen. ## Notes This review is in the ["IETF Comments" Markdown format][ICMF], You can use the [`ietf-comments` tool][ICT] to automatically convert this review into individual GitHub issues. Review generated by the [`ietf-reviewtool`][IRT]. [ICMF]: https://github.com/mnot/ietf-comments/blob/main/format.md [ICT]: https://github.com/mnot/ietf-comments [IRT]: https://github.com/larseggert/ietf-reviewtool
_______________________________________________ BESS mailing list [email protected] https://www.ietf.org/mailman/listinfo/bess
