Subject: Re: [mpls] [Gen-art] Genart last call review of draft-ietf-mpls-rfc8287-len-clarification-02

From: Alissa Cooper <[email-masked]>
Date: Tue, 6 Aug 2019 15:11:50 -0400

Ines, thanks for your review. I entered a DISCUSS ballot to get the figure fixed in Section 4.2.

Alissa

> On Jul 30, 2019, at 8:11 AM, Ines Robles via Datatracker <[email-masked]> wrote:
> 
> Reviewer: Ines Robles
> Review result: Ready with Issues
> 
> 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
> 
> _______________________________________________
> Gen-art mailing list
> [email-masked]
> https://www.ietf.org/mailman/listinfo/gen-art

---

From: "Carlos Pignataro (cpignata)" <[email-masked]>
Date: Tue, 6 Aug 2019 19:25:34 +0000

Hi, Ines, Many thanks for your very useful review!

Thanks Alissa for flagging this.

Please find some follow-up comments inline.

> On Aug 6, 2019, at 3:11 PM, Alissa Cooper <[email-masked]> wrote:
> 
>> [RFC8174] when, and only when, they appear in all capitals, as shown here.”
>> 

Sure.

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

Indeed. Great catch and many thanks.

>> the table of Section 4.3.

We had thought about this, and received this comment before also, but since this is largely a fix and update to RFC 8287, we want to remain consistent to the format used there.

>> combinations of the table of Section 4.3, e.g. with tables as detailed below:

Thanks for this suggestion. To me, it seems a bit overkill to explain the the size of an IPv4 is 4 octets, and the size of an IPv6 is 16 octets, etc. The fact that we are including the table seems the right middle-ground for readability, thoroughness, and detailed-orientedness.

But thanks for the suggestion.

Best,

Carlos.

---

From: Ines Robles <[email-masked]>
Date: Wed, 7 Aug 2019 11:46:59 +0300

Hi Carlos,

Ok, thank you very much for addressing my comments.

Best,

Ines.

On Tue, Aug 6, 2019 at 10:25 PM Carlos Pignataro (cpignata) <
[email-masked]> wrote:

> We had thought about this, and received this comment before also, but
> since this is largely a fix and update to RFC 8287, we want to remain
> consistent to the format used there.
>
> Thanks for this suggestion. To me, it seems a bit overkill to explain the
> the size of an IPv4 is 4 octets, and the size of an IPv6 is 16 octets, etc.
> The fact that we are including the table seems the right middle-ground for
> readability, thoroughness, and detailed-orientedness.
>
> >> |                                                       | Any | OSPF |
> ISIS |
> >> |           Reserved                                 |  2  |   2  |
>  2  |
> >> |                                                        | Any | OSPF |
> ISIS |
> >> |           Reserved                                  |  2  |   2  |
>  2  |
> >> |            Field                                |   IPv6 (octets)
>       |
> >> |                                                       | Any | OSPF |
> ISIS |
> >> |           Reserved                                  |  2  |   2  |
>  2  |