Skip to main content

Last Call Review of draft-ietf-mpls-bfd-directed-27
review-ietf-mpls-bfd-directed-27-tsvart-lc-eggert-2024-04-15-00

Request Review of draft-ietf-mpls-bfd-directed
Requested revision No specific revision (document currently at 31)
Type Last Call Review
Team Transport Area Review Team (tsvart)
Deadline 2024-04-15
Requested 2024-04-01
Authors Greg Mirsky , Jeff Tantsura , Ilya Varlashkin , Mach Chen
I-D last updated 2024-04-15
Completed reviews Rtgdir Last Call review of -26 by Andrew Alston (diff)
Opsdir Last Call review of -27 by Joe Clarke (diff)
Genart Last Call review of -27 by Linda Dunbar (diff)
Tsvart Last Call review of -27 by Lars Eggert (diff)
Rtgdir Early review of -07 by Carlos Pignataro (diff)
Assignment Reviewer Lars Eggert
State Completed
Request Last Call review on draft-ietf-mpls-bfd-directed by Transport Area Review Team Assigned
Posted at https://mailarchive.ietf.org/arch/msg/tsv-art/VSUn-JJJ1JbitWG6cufdSHTFo6c
Reviewed revision 27 (document currently at 31)
Result On the Right Track
Completed 2024-04-15
review-ietf-mpls-bfd-directed-27-tsvart-lc-eggert-2024-04-15-00
# tsvart review of draft-ietf-mpls-bfd-directed-27

CC @larseggert

This document has been reviewed as part of the transport area review
team's ongoing effort to review key IETF documents. These comments were
written primarily for the transport area directors, but are copied to the
document's authors and WG to allow them to address any issues raised and
also to the IETF discussion list for information.

When done at the time of IETF Last Call, the authors should consider this
review as part of the last-call comments they receive. Please always CC
tsv-art@ietf.org if you reply to or forward this review.

## Comments

### Section 3.1, paragraph 7
```
     Reverse Path field contains none, one or more sub-TLVs.  Any non-
     multicast Target FEC Stack sub-TLV (already defined, or to be defined
     in the future) for TLV Types 1, 16, and 21 of MPLS LSP Ping
     Parameters registry MAY be used in this field.  Multicast Target FEC
```
I think you mean "no other sub-TLV than X, Y, Z MUST be used"?(The MAY
makes anything allowed.)

### Section 3.1, paragraph 6
```
     MAY be included in the BFD Reverse Path TLV.  However, the number of
     sub-TLVs in the Reverse Path field MUST be limited.  The default
     limit is 128, but an implementation MAY be able to control that
```
Why must it be limited? And what unit is the default of 128 expressed
in, bytes (for the "length" field)? Or number of entries?

### Section 4, paragraph 0
```
  4.  Use Case Scenario
```
It would be more helpful if this example came before Section 3.

### Inclusive language

Found terminology that should be reviewed for inclusivity; see
https://www.rfc-editor.org/part2/#inclusive_language for background and more
guidance:

 * Term `traditional`; alternatives might be `classic`, `classical`, `common`,
   `conventional`, `customary`, `fixed`, `habitual`, `historic`,
   `long-established`, `popular`, `prescribed`, `regular`, `rooted`,
   `time-honored`, `universal`, `widely used`, `widespread`

## 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
```
-    there may be a need to direct egress BFD peer to use a specific path
+    there may be a need to direct the egress BFD peer to use a specific path
+                                 ++++
-    request that allows a BFD system requests that the remote BFD peer
-                                            -
-    transmits BFD control packets over the specified LSP.
-            -
+    request that allows a BFD system to request that the remote BFD peer
+                                     +++
```
The rest of the doc has a bunch more grammar nits like this which make
following the content unnecessarily difficult. Please proofread.

### Section 2, paragraph 2
```
     *  detection by an ingress node of a failure on the reverse path may
        not be unambiguously interpreted as the failure of the path in the
        forward direction.
```
A bulleted list with a single bullet is a bit pointless?

### Section 3.1, paragraph 6
```
     If the egress LSR cannot find the path specified in the Reverse Path
     TLV it MUST send Echo Reply with the received BFD Discriminator TLV,
     Reverse Path TLV and set the Return Code to "Failed to establish the
     BFD session.  The specified reverse path was not found" Section 3.2.
```
What about Section 3.2?

### Section 3.1, paragraph 6
```
     The BFD Reverse Path TLV MAY be used in the bootstrapping of a BFD
     session process described in Section 6 [RFC5884].  A system that
```
Section 6 *of* RFC5884?

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