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

[Alvaro Retana <[e-mail]>]

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


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.

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].

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


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].


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


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.


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


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.


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.


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


[End of Review]