Re: [bess] Lars Eggert's Discuss on draft-ietf-bess-evpn-lsp-ping-09: (with DISCUSS and COMMENT)

["Parag Jain (paragj)" <[e-mail]>]

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 <[e-mail]>
Date: Monday, April 24, 2023 at 9:29 AM
To: The IESG <[e-mail]>
Cc: [e-mail] <[e-mail]>, [e-mail] <[e-mail]>, [e-mail] <[e-mail]>, Matthew Bocci <[e-mail]>, [e-mail] <[e-mail]>
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