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) <cpignata@xxxxxxxxx> wrote:
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 <alissa@xxxxxxxxxx> 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 <noreply@xxxxxxxx> 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
>> Gen-art@xxxxxxxxx
>> https://www.ietf.org/mailman/listinfo/gen-art
>
