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@xxxxxxxx >> https://www.ietf.org/mailman/listinfo/gen-art >
