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

["Carlos Pignataro (cpignata)" <[e-mail]>]

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 <[e-mail]> wrote:
> 
> 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 <[e-mail]> 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.”
>> 

Sure.

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

Indeed. Great catch and many thanks.

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

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.

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

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.

>> 
>> +-----------------------------+-------------------+
>> |            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
>> [e-mail]
>> https://www.ietf.org/mailman/listinfo/gen-art
>