Label Switched Path (LSP) Ping Mechanisms for EVPN and Provider Backbone Bridging EVPN (PBB-EVPN)
draft-ietf-bess-evpn-lsp-ping-11

Thanks for this update, I've cleared my DISCUSS. 

I do think there is a little further improvement possible. The new text is adequate, so please use your own judgment, but my suggestion would be something like the following, using RFC 9136 Section 3.1 as a model. This adds a MUST, some parentheses, and a comma, and drops an "as". Other than the MUST the changes are just stylistic.

OLD:
   The EVPN IP Prefix Sub-TLV has the format as shown in Figure 4.  The
   total length of this sub-TLV can be either 32 bytes if IPv4 addresses
   are carried or 56 bytes if IPv6 addresses are carried. The IP prefix
   and gateway IP address MUST be from the same IP address family as
   described in Section 3.1 of [RFC9136].
   
NEW:
   The EVPN IP Prefix Sub-TLV has the format shown in Figure 4.  The
   total length (not shown) of this sub-TLV MUST be either 32 bytes (if 
   IPv4 addresses are carried) or 56 bytes (if IPv6 addresses are carried).
   The IP prefix and gateway IP address MUST be from the same IP address 
   family, as described in Section 3.1 of [RFC9136].
   
and,

OLD:
   *  The IP prefix field is set to a 4-octet IPv4 address (with
      trailing 0 bits to make 32 bits in all) or 16-octet IPv6 address
      (with trailing 0 bits to make 128 bits in all).

   *  The Gateway (GW) IP Address field is set to a 4-octet IPv4 address
      or 16-octet IPv6 address if it's used as an Overlay Index for the
      IP prefixes.  If the GW IP Address is not being used, it must be
      set to 0 as described in Section 3.1 of [RFC9136].

NEW:
   *  The IP prefix field is set to a 4-octet IPv4 address (with
      trailing 0 bits to make 32 bits in all) or 16-octet IPv6 address
      (with trailing 0 bits to make 128 bits in all). The address family
      of this field is inferred from the sub-TLV length field, as 
      discussed above.

   *  The Gateway (GW) IP Address field is set to a 4-octet IPv4 address
      or 16-octet IPv6 address if it's used as an Overlay Index for the
      IP prefixes.  If the GW IP Address is not being used, it must be
      set to 0 as described in Section 3.1 of [RFC9136]. The address 
      family of this field is inferred from the sub-TLV length field, as 
      discussed above.
      
I proposed the additional sentence in the two bulleted text items because, without that, there's no description *in the bullet list* of how to infer the address family, unlike RFC 9136 which does provide this context in the bullet list. In my experience, people referring to the document quickly can't necessarily be relied upon to carefully read the entire section. :-(

Thank you to Rifaat Shekh-Yusef for the SECDIR review.

I support the DISCUSS positions of Jim Guichard and Lars Eggert.

Like Roman, I support the DISCUSS positions of Jim Guichard and Lars Eggert.

I'd also like to thank Di Ma for the DNS-DIR review.

Thanks for working on this document. Special thanks to Wesley Eddy for his TSVART review. Based on TSVART review and my read of the specification, I have not found transport related issues.

and I am supporting Jim's Discuss.

Thanks for addressing my previous DISCUSS points. For archive: https://mailarchive.ietf.org/arch/msg/bess/sZ4wA52Tblso9dInQUpePkWZ5BM/

I sincerely believe that the changes have improved the document.

Regards

-éric

# 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).

## 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.

### Section 4.3, paragraph 5
```
                Figure 3: EVPN Ethernet Auto-Discovery Sub-TLV format
```
Why is there a "Must Be Zero" field?

### Section 4.4, paragraph 3
```
                       Figure 4: EVPN IP Prefix Sub-TLV format
```
Why is there a "Must Be Zero" field?

### Missing references

No reference entries found for these items, which were mentioned in the text:
`[RFC8174]` and `[RFC2119]`.

## 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
-                                  -
```

#### Section 3, paragraph 10
```
-    FEC: Forwarding Equivalent Classs
-                                    -
```

#### Section 4, paragraph 1
```
-    an indentifier for a given EVPN is programmed at the target node.
-        -
```

#### 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-
+                                                                       +
```

### 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.

#### 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

All my comments have already been covered in other ADs ballots.