Last Call Review of draft-ietf-mpls-rfc8287-len-clarification-02
review-ietf-mpls-rfc8287-len-clarification-02-genart-lc-robles-2019-07-30-00

Request Review of draft-ietf-mpls-rfc8287-len-clarification
Requested rev. no specific revision (document currently at 04)
Type Last Call Review
Team General Area Review Team (Gen-ART) (genart)
Deadline 2019-07-31
Requested 2019-07-10
Draft last updated 2019-07-30
Completed reviews Rtgdir Last Call review of -02 by Susan Hares (diff)
Genart Last Call review of -02 by Ines Robles (diff)
Opsdir Last Call review of -02 by Joel Jaeggli (diff)
Secdir Last Call review of -02 by Kyle Rose (diff)
Genart Telechat review of -04 by Ines Robles
Assignment Reviewer Ines Robles
State Completed
Review review-ietf-mpls-rfc8287-len-clarification-02-genart-lc-robles-2019-07-30
Posted at https://mailarchive.ietf.org/arch/msg/gen-art/ifFzugb3iK6YfgH3-_2wYx6QYeE
Reviewed rev. 02 (document currently at 04)
Review result Ready with Issues
Review completed: 2019-07-30

Review
review-ietf-mpls-rfc8287-len-clarification-02-genart-lc-robles-2019-07-30

I am the assigned Gen-ART reviewer for this draft. The General Area
Review Team (Gen-ART) reviews all IETF documents being processed
by the IESG for the IETF Chair.  Please treat these comments just
like any other last call comments.

For more information, please see the FAQ at

<https://trac.ietf.org/trac/gen/wiki/GenArtfaq>.

Document: draft-ietf-mpls-rfc8287-len-clarification-02
Reviewer: Ines Robles
Review Date: 2019-07-30
IETF LC End Date: 2019-07-31
IESG Telechat date: Not scheduled for a telechat

Summary:

I believe the draft is technically good. This document is well written.

The document updates RFC8287 by clarifying the length for the following Segment ID Sub-TLVs: IPv4 IGP-Prefix Segment ID Sub-TLV, IPv6 IGP-Prefix Segment ID Sub-TLV and IGP-Adjacency Segment ID Sub-TLV.

There are some minor issues detailed below that should be addressed.

Major issues: Not found

Minor issues:

1- Section 3 - Requirements notation is not complete, it should be added:  "NOT RECOMMENDED" and "...are to be interpreted as described in BCP 14 [RFC2119] [RFC8174] when, and only when, they appear in all capitals, as shown here."

2- Figure of Section 4.2: Type = 35 (IPv4 IGP-Prefix SID) ---> Type = 35 (IPv6 IGP-Prefix SID)

2.1- It would be nice if the figures have a caption where we can point to the figure number, and the figure number is referenced in the text.
The same for the table of Section 4.3.


3- Question: What do you think?

I think it would be nice to explain a bit more the length for the different combinations of the table of Section 4.3, e.g. with tables as detailed below:

+-----------------------------+-------------------+
|            Field                               | Parallel (octets) |
|     rfc8287#section-5.3            +-----+------+------+
|                                                       | Any | OSPF | ISIS |
+-----------------------------+-----+------+------+
|      Local Interface ID                    |  4  |   4  |   4  |
+-----------------------------+-----+------+------+
|     Remote Interface ID                 |  4  |   4  |   4  |
+-----------------------------+-----+------+------+
| Advertising Node Identifier          |  4  |   4  |   6  |
+-----------------------------+-----+------+------+
|  Receiving Node Identifier             |  4  |   4  |   6  |
+-----------------------------+-----+------+------+
|           Reserved                                 |  2  |   2  |   2  |
+-----------------------------+-----+------+------+
|     Adj. Type + Protocol                     |  2  |   2  |   2  |
+-----------------------------+-----+------+------+
|       Sum Total octets =                      |  20 |  20  |  24  |
+-----------------------------+-----+------+------+




+-----------------------------+-------------------+
|            Field                                |   IPv4 (octets)   |
|     rfc8287#section-5.3             +-----+------+------+
|                                                        | Any | OSPF | ISIS |
+-----------------------------+-----+------+------+
|      Local Interface ID                     |  4  |   4  |   4  |
+-----------------------------+-----+------+------+
|     Remote Interface ID                 |  4  |   4  |   4  |
+-----------------------------+-----+------+------+
| Advertising Node Identifier           |  4  |   4  |   6  |
+-----------------------------+-----+------+------+
|  Receiving Node Identifier             |  4  |   4  |   6  |
+-----------------------------+-----+------+------+
|           Reserved                                  |  2  |   2  |   2  |
+-----------------------------+-----+------+------+
|     Adj. Type + Protocol                     |  2  |   2  |   2  |
+-----------------------------+-----+------+------+
|       Sum Total octets =                     |  20 |  20  |  24  |
+-----------------------------+-----+------+------+





+-----------------------------+-------------------+
|            Field                                |   IPv6 (octets)          |
|     rfc8287#section-5.3           +-----+------+------+
|                                                       | Any | OSPF | ISIS |
+-----------------------------+-----+------+------+
|      Local Interface ID                    |  16 |  16  |  16  |
+-----------------------------+-----+------+------+
|     Remote Interface ID                  |  16 |  16  |  16  |
+-----------------------------+-----+------+------+
| Advertising Node IdentifieR           |  4  |   4  |   6  |
+-----------------------------+-----+------+------+
|  Receiving Node Identifier              |  4  |   4  |   6  |
+-----------------------------+-----+------+------+
|           Reserved                                  |  2  |   2  |   2  |
+-----------------------------+-----+------+------+
|     Adj. Type + Protocol                     |  2  |   2  |   2  |
+-----------------------------+-----+------+------+
|     sum  Total octets =                       |  44 |  44  |  48  |
+-----------------------------+-----+------+------+


Nits/editorial comments: Issue tool: Summary: 0 errors (**), 0 flaws (~~), 0 warnings (==), 1 comment (--).

Thanks for this document,

Ines