Re: [Idr] Opsdir early review of draft-ietf-idr-eag-distribution-13

[Jeff Tantsura <[e-mail]>]

Hi Tim,

Many thanks for your  review.
Please see inline

Cheers,
Jeff
On Feb 15, 2021, 8:31 AM -0800, Tim Chown via Datatracker <[e-mail]>, wrote:
> Reviewer: Tim Chown
> Review result: Has Nits
>
> Hi,
>
> I have reviewed this document as part of the Operational directorate's ongoing
> effort to review all IETF documents being processed by the IESG. These
> comments were written with the intent of improving the operational aspects of
> the IETF drafts. Comments that are not addressed in last call may be included
> in AD reviews during the IESG review. Document editors and WG chairs should
> treat these comments just like any other last call comments.
>
> This document defines extensions to BGP-LS for advertisement of extended
> administrative groups (EAGs) to allow it to support administrative groups of
> size greater than 32 bits (apparently multiples of 32 bits).
>
> The document is easy to follow, well-written, and Ready for publication with
> Nits.
>
> Nits:
>
> Abstract:
>
> The abstract doesn’t actually say what this draft defines.
> Add at the end of the current abstract paragraph something like “This document
> defines extensions to BGP-LS for advertisement of extended administrative
> groups (EAGs) to allow it to support administrative groups of size greater than
[jeff] in this context - “extended” (RFC7308) means - larger that 32 bits, I’ll happily clarify this
> 32 bits.”
>
> Section 1:
>
> The BGP-LS advertisement is encoded -> The BGP-LS advertisement for the
> originally defined (non-extended) administrative groups is encoded (Adds
> clarity)
[jeff] you’d like me to compare extended encoding to the original (non-extended) one? Please note - the draft doesn’t define new attributes, EAG has been defined in the RFC7308, but provides BGP-LS transport for it
>
> Section 2:
>
> Extensions -> an extension (?)
[jeff] ack
>
> EAG of a -> The EAG of a
[jeff] ack
>
> must MUST be -> MUST be a
[jeff] ack
>
> Given the stipulation of the length being a multiple of 4, perhaps make it
> clear elsewhere that (if I understand correctly) the EAGs have masks that are
> multiples of 32 bits (or are they implemented as multiple 32 bit masks?)
>
> AG TLV 108 -> EAG LV 1108 (I assume?)
[jeff]this should be 1108, thanks for catching this, EAG TLV is indeed 1173
https://www.iana.org/assignments/bgp-ls-parameters/bgp-ls-parameters.xhtml
>
> existing AG -> originally defined (again, clarity?)
[jeff] ack
>
> When referring to backward compatibility in RFC 7308 perhaps add that Section
> 2.3.1 says how to handle that if both an AG and EAG are advertised, the first
> 32 bits of the EAG MUST be identical to the advertised 32 bit AG. (Just a
> suggestion, but probably an important point to reinforce?)
[jeff]ack, good point
>
> Section 3:
>
> Assigning code-point -> assigning a code point
[jeff] ack
>
> --
> Tim
>
>