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 protected]> 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 <[email protected]> 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 < > [email protected]> 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 > >> [email protected] > >> https://www.ietf.org/mailman/listinfo/gen-art > > > >
_______________________________________________ Gen-art mailing list [email protected] https://www.ietf.org/mailman/listinfo/gen-art
