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

From: Tim Chown via Datatracker <[email-masked]>
Date: Mon, 15 Feb 2021 08:31:48 -0800

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
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)

Section 2:

Extensions -> an extension (?)

EAG of a -> The EAG of a

must MUST be -> MUST be a

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?)

existing AG -> originally defined  (again, clarity?)

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?)

Section 3:

Assigning code-point -> assigning a code point

--
Tim

---

From: Jeff Tantsura <[email-masked]>
Date: Mon, 15 Feb 2021 19:39:31 -0800

Hi Tim,

Many thanks for your  review.
Please see inline

Cheers,
Jeff
> 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
> originally defined (non-extended) administrative groups is encoded (Adds
[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
> Extensions -> an extension (?)
[jeff] ack
> EAG of a -> The EAG of a
[jeff] ack
> must MUST be -> MUST be a
[jeff] ack
> 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
> suggestion, but probably an important point to reinforce?)
[jeff]ack, good point
> Assigning code-point -> assigning a code point
[jeff] ack

---

From: Tim Chown <[email-masked]>
Date: Tue, 16 Feb 2021 13:20:11 +0000

Hi Jeff,

Thanks, a couple of points responded to below, rest trimmed…

> On 16 Feb 2021, at 03:39, Jeff Tantsura <[email-masked]> wrote:
> 

Thanks. The abstract only states the problem, not what the document provides as the solution.  

> [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…

No need, my point is that reading it as is it’s not clear which administrative groups that text refers to.

Best wishes,
Tim

---

From: Jeff Tantsura <[email-masked]>
Date: Wed, 17 Feb 2021 21:40:39 -0800

Dear Tim,

Many thanks for your comments, I have published v14 that should address it.

Cheers,
Jeff