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

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