Subject: Re: [Idr] AD Review of draft-ietf-idr-eag-distribution-15

From: Jeff Tantsura <[email-masked]>
Date: Sun, 18 Apr 2021 16:47:11 -0700

HI Alvaro,

Many thanks for your thoughtful review, please see inline (updated version has been published)

Cheers,
Jeff
On Mar 30, 2021, 1:25 PM -0700, Alvaro Retana <[email-masked]>, wrote:
> Dear authors:
>
> Here is my review of this document.  I have some major comments (see
> inline below) that should be easy to address and that I would like to
> see reflected in the text before starting the IETF Last Call.
>
> Thank you for your work!
>
> Alvaro.
>
>
> [Line numbers from idnits.]
>
>
> 11 Distribution of Traffic Engineering Extended Admin Groups using BGP-LS
>
> [minor] s/Admin/Administrative
[jeff] ack
>
>
> 12                   draft-ietf-idr-eag-distribution-15
>
> 14 Abstract
>
> 16   Administrative groups are link attributes (commonly referred to as
> 17   "colors" or "link colors") advertised by link state protocols (e.g.
> 18   ISIS or OSPF) and used for traffic engineering.  These administrative
> 19   groups were initially defined as 32 bit masks.  As network usage
> 20   grew, these 32 bit masks were found to constrain traffic engineering.
> 21   Therefore, link state protocols (ISIS, OSPF) were expanded to
> 22   advertise a variable length administrative group.This document
> 23   defines an extension to BGP-LS for advertisement of extended
> 24   administrative groups (EAGs) to allow to support a number of
> 25   administrative groups greater than 32, as defined in [RFC7308].
>
> [minor] A couple of changes to simplify.  The details can be found in
> the Introduction.
[jeff] ack
>
> Suggestion>
>    Administrative groups are link attributes advertised used for traffic
>    engineering.  This document defines an extension to BGP-LS for
>    advertisement of extended administrative groups (EAGs).
>
>
> ...
> 70 1.  Introduction
>
> 72   Administrative groups (commonly referred to as "colors" or "link
> 73   colors") are link attributes that are advertised by link state
> 74   protocols like IS-IS [RFC5305], OSPFv2 [RFC3630] and OSPFv3 [RFC5329]
> 75   for traffic engineering use-cases.  The BGP-LS advertisement of the
> 76   originally defined (non-extended) administrative groups is encoded
> 77   using the Administrative Group (color) TLV 1088 as defined in
> 78   [RFC7752].
[jeff] ack
>
> [minor] "IS-IS [RFC5305], OSPFv2 [RFC3630] and OSPFv3 [RFC5329]"
> These references should point at the protocols themselves, not the TE
> extensions.  Also, these references can be Informative.
[jeff] ack, changed
[jeff] Alvaro - this is the text from 7308, that specifically calls  out TE documents -
"This document defines the Extended Administrative Group (EAG) sub-TLVfor both OSPF [RFC3630] and IS-IS [RFC5305].” ?
>
>
> 80   These administrative groups are defined as a fixed-length 32-bit
> 81   bitmask.  As networks grew and more use-cases were introduced, the
> 82   32-bit length was found to be constraining and hence extended
> 83   administrative groups (EAG) were introduced in the IS-IS and OSPFv2
> 84   link state routing protocols [RFC7308].
>
> [minor] rfc7308 also includes OSPFv3.
>
> s/were introduced in the IS-IS and OSPFv2 link state routing protocols
> [RFC7308]./were introduced in [RFC7308].
[jeff] ack
>
>
> ...
> 97 2.  Advertising Extended Administrative Groups in BGP-LS
>
> 99   This document defines an extension that enable BGP-LS speakers to
> 100   signal the EAG of links in a network to a BGP-LS consumer of network
> 101   topology such as a centralized controller.  The centralized
> 102   controller can leverage this information in traffic engineering
> 103   computations and other use-cases.  When a BGP-LS speaker is
> 104   originating the topology learnt via link-state routing protocols like
> 105   OSPF or IS-IS, the EAG information of the links is sourced from the
> 106   underlying extensions as defined in [RFC7308].  The BGP-LS speaker
> 107   may also advertise the EAG information for the local links of a node
> 108   when not running any link-state IGP protocol e.g. when running BGP as
> 109   the only routing protocol.
>
> [minor] There is no defined mechanism to originate EAG information
> when not running a link-state protocol.  Let's take the last sentence
> out.
>
[jeff] ack
>
> 111   The EAG of a link is encoded in a new Link Attribute TLV [RFC7752]
> 112   using the following format:
>
> 114      0                   1                   2                   3
> 115      0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1
> 116     +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
> 117     |              Type             |             Length            |
> 118     +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
> 119     |    Extended Administrative Groups (variable)                 //
> 120     +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
>
> [minor] Maybe use the same representation as rfc7308 for the format.
[jeff] this is the standard way to represent encodings in BGP-LS documents, I think readers are more comfortable with this representation, please let me know what you think.
>
>
> ...
> 128   o  Length: variable length which represents the total length of the
> 129      value field.  The length value MUST be multiple of 4.  If the
> 130      length is not a multiple of 4, the TLV MUST be considered
> 131      malformed.
>
> [major] "total length of the value field"  What are the units?
[jeff ack]
>
>
> 133   o  Value: one or more sets of 32-bit bitmasks that indicate the
> 134      administrative groups (colors) that are enabled on the link when
> 135      those specific bits are set.
>
> [major] rfc7308 talks about an "Extended Admin Group" (singular!), and
> not a set of groups.  This TLV is then not in line with rfc7308.
>
> The whole document should be checked for (editorial) compliance with
> the concept of a single EAG.
>
[jeff] ack, i believe now it reads correct
>
> 137   The EAG TLV is an optional TLV.  The originally defined AG TLV 1108
> 138   and the EAG TLV 1173 defined in this document MAY be advertised
> 139   together.  The semantics of the EAG and the backward compatibility
> 140   aspects of EAG with respect to the AG are handled as described in the
> 141   Backward Compatibility section of [RFC7308], namely - If a node
> 142   advertises both AG and EAG, then the first 32 bits of the EAG MUST be
> 143   identical to the advertised AG.
>
> [minor] We don't really need this paragraph because any compatibility
> or checking is outside of what BGP-LS does (just transport).  Please
> remove it.
>
[jeff] ack
>
> ...
> 158 4.  Security Considerations
>
> 160   The extensions in this document advertise same administrative group
> 161   information specified via [RFC7752] but as a larger/extended value
> 162   and hence does not introduce security issues beyond those discussed
> 163   in [RFC7752] and [I-D.ietf-idr-rfc7752bis].
>
> [major] No need to mention rfc7752bis -- at all!
>
> [major] Please take a look at the Security Considerations in rfc8814
> and copy them here -- with appropriate modifications to talk about EAG
> instead of MSD, of course.
>
[jeff] ack
> [End of Review]

---

From: Alvaro Retana <[email-masked]>
Date: Mon, 19 Apr 2021 07:57:49 -0700

On April 18, 2021 at 7:47:22 PM, Jeff Tantsura wrote:

Jeff:

Hi!

Just a couple of points -- one major.  I'm starting the IETF Last Call.

Thanks!

Alvaro.

...
> "This document defines the Extended Administrative Group (EAG) sub-TLVfor
> both OSPF [RFC3630] and IS-IS [RFC5305].” ?

The text refers to the protocols ("protocols like...").  It's ok if
you want to change the text slightly to the TE extensions.

...
> [jeff] ack, i believe now it reads correct

I may be wrong, but rfc7308 and the encoding on this document give the
impression that you only carry one EAG, not "one or more sets".  Did I
miss something?  If multiple EAGs can be carried I think there's an
extra length missing: the TLV only has the total length, not the
length of each EAG.

---

From: "Ketan Talaulikar (ketant)" <[email-masked]>
Date: Mon, 19 Apr 2021 15:12:37 +0000

Hi Alvaro,

Regarding your 2nd point (that I picked from the IDR archive):

There is a single AG/EAG value for a link and hence a single TLV. What EAG (RFC7308) allows is a larger size value (i.e. more bits in multiples of 32) to be used when the requirement is for more than 32 bits (i.e. link colors) in a given deployment.

Thanks,
Ketan

-----Original Message-----
From: Alvaro Retana <[email-masked]> 
Sent: 19 April 2021 20:28
To: [email-masked]; Jeff Tantsura <[email-masked]>
Cc: Susan Hares <[email-masked]>; [email-masked]; IDR List <[email-masked]>
Subject: Re: AD Review of draft-ietf-idr-eag-distribution-15

Jeff:

Hi!

Just a couple of points -- one major.  I'm starting the IETF Last Call

---

From: "Les Ginsberg (ginsberg)" <[email-masked]>
Date: Mon, 19 Apr 2021 15:36:04 +0000

Alvaro -

To add to what Ketan has said...

https://www.rfc-editor.org/rfc/rfc7308.html#section-2.1 defines the format as one or more 32 bit " Extended Admin Groups".
One could argue that it might have better to define EAG as one or more 32 bit "Admin Groups", but that isn’t what RFC 7308 says.

I therefore think that draft-ietf-idr-eag-distribution is consistent with RFC 7308 and should go forward as is.

   Les

> From: Idr <[email-masked]> On Behalf Of Ketan Talaulikar (ketant)
> Sent: Monday, April 19, 2021 8:13 AM
> To: Alvaro Retana <[email-masked]>; draft-ietf-idr-eag-
> [email-masked]; Jeff Tantsura <[email-masked]>
> Cc: [email-masked]; Susan Hares <[email-masked]>; IDR List
> <[email-masked]>
> Subject: Re: [Idr] AD Review of draft-ietf-idr-eag-distribution-15
> 
> Hi Alvaro,
> 
> There is a single AG/EAG value for a link and hence a single TLV. What EAG
> (RFC7308) allows is a larger size value (i.e. more bits in multiples of 32) to be
> used when the requirement is for more than 32 bits (i.e. link colors) in a given
> deployment.
> 
> Thanks,
> Ketan
> 
> _______________________________________________
> Idr mailing list
> [email-masked]
> https://www.ietf.org/mailman/listinfo/idr

---

From: Jeff Tantsura <[email-masked]>
Date: Mon, 19 Apr 2021 08:56:41 -0700

Alvaro,

I’m with Ketan/Les here, I cleaned up the text though

Cheers,
Jeff

---

From: Alvaro Retana <[email-masked]>
Date: Mon, 19 Apr 2021 10:57:20 -0700

Yeah….  I’m good.  :-)

Thanks!

Alvaro.

On April 19, 2021 at 11:36:08 AM, Les Ginsberg (ginsberg) (
[email-masked]) wrote:

Alvaro -

https://www.rfc-editor.org/rfc/rfc7308.html#section-2.1 defines the format
as one or more 32 bit " Extended Admin Groups".
One could argue that it might have better to define EAG as one or more 32
bit "Admin Groups", but that isn’t what RFC 7308 says.

I therefore think that draft-ietf-idr-eag-distribution is consistent with
RFC 7308 and should go forward as is.

Les