Re: [RTG-DIR] Rtgdir last call review of draft-ietf-idr-rfc7752bis-11

[Ketan Talaulikar <[e-mail]>]

Hi Joel,

Thanks for your review and please check inline below for responses.

On Thu, Oct 27, 2022 at 7:32 PM Joel Halpern via Datatracker <
[e-mail]> wrote:

> Reviewer: Joel Halpern
> Review result: Has Nits
>
> Hello,
>
> I have been selected as the Routing Directorate reviewer for this draft.
> The
> Routing Directorate seeks to review all routing or routing-related drafts
> as
> they pass through IETF last call and IESG review, and sometimes on special
> request. The purpose of the review is to provide assistance to the Routing
> ADs.
> For more information about the Routing Directorate, please see
> http://trac.tools.ietf.org/area/rtg/trac/wiki/RtgDir
>
> Although these comments are primarily for the use of the Routing ADs, it
> would
> be helpful if you could consider them along with any other IETF Last Call
> comments that you receive, and strive to resolve them through discussion
> or by
> updating the draft.
>
> Document: draft-ietf-idr-rfc7752bis-11
> Reviewer: Joel Halpern
> Review Date: 26-Oct-2022
> IETF LC End Date: N/A
> Intended Status: Proposed Standard
>
> Summary:
>     This document is basically ready for publication but has nits that
> should
>     be considered prior to publication.
>
> Comments:
>
> Please supply an overview of the draft quality and readability.
> Include anything else that you think will be helpful toward understanding
> your
> review.
>
> Major Issues:
>     None
>
> Minor Issues:
>     None
>
> Nits:
>   At the end of the first paragraph of section 4, could we add a sentence
>   saying "the BGP-LS attributes appear within the corresponding new BGP
> NLRI"
>   or similar?  While that is explained later in section 4, the length of
> the
>   section means that a new reader is left wondering for quite some time.
>

KT> The BGP-LS Attribute TLVs do not appear within the NLRI. Sec 4.2
introduces BGP-LS NLRIs and indicates that they are carried within the
MP_REACH/UNREACH. Further Sec 4.3 introduces the BGP-LS Attribute and
indicates that they are carried as part of the BGP update along with the
Link State NLRIs (and other attributes). Perhaps we can clarify a bit
upfront that the new NLRI types are carried in the MP_REACH/UNREACH?


>
>  Section 4.1 has the paragraph:
>    All TLVs within the NLRI that are not specified as mandatory are
>    considered optional.  All TLVs within the BGP-LS Attribute are
>    considered optional unless specified otherwise.
>   As far as I can tell, those two sentences are saying, about two different
>   aspects of the encoding, the same thing.  But they say it in different
> ways.
>   If there is some subtle difference in meaning taht is intended, please
>   clarify.  If the meaning is indeed the same, could we use parallel
>   construction to avoid readers thinking there is a difference?
>

KT> They are talking about two different "containers" - the NLRI and the
BGP-LS Attribute. The "default" is different for them.

Thanks,
Ketan