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

Jeff Tantsura <jefftant.ietf@gmail.com> Sun, 18 April 2021 23:47 UTC

Return-Path: <jefftant.ietf@gmail.com>
X-Original-To: idr@ietfa.amsl.com
Delivered-To: idr@ietfa.amsl.com
Received: from localhost (localhost [127.0.0.1]) by ietfa.amsl.com (Postfix) with ESMTP id 038373A12A4; Sun, 18 Apr 2021 16:47:30 -0700 (PDT)
X-Virus-Scanned: amavisd-new at amsl.com
X-Spam-Flag: NO
X-Spam-Score: -0.197
X-Spam-Level:
X-Spam-Status: No, score=-0.197 tagged_above=-999 required=5 tests=[DKIM_SIGNED=0.1, DKIM_VALID=-0.1, DKIM_VALID_AU=-0.1, DKIM_VALID_EF=-0.1, FREEMAIL_FROM=0.001, HTML_MESSAGE=0.001, RCVD_IN_DNSWL_BLOCKED=0.001, SPF_HELO_NONE=0.001, SPF_PASS=-0.001] autolearn=ham autolearn_force=no
Authentication-Results: ietfa.amsl.com (amavisd-new); dkim=pass (2048-bit key) header.d=gmail.com
Received: from mail.ietf.org ([4.31.198.44]) by localhost (ietfa.amsl.com [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id 7mXxfNOafLR0; Sun, 18 Apr 2021 16:47:25 -0700 (PDT)
Received: from mail-pg1-x52e.google.com (mail-pg1-x52e.google.com [IPv6:2607:f8b0:4864:20::52e]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by ietfa.amsl.com (Postfix) with ESMTPS id B40FA3A12AF; Sun, 18 Apr 2021 16:47:24 -0700 (PDT)
Received: by mail-pg1-x52e.google.com with SMTP id y32so22983716pga.11; Sun, 18 Apr 2021 16:47:24 -0700 (PDT)
DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=date:from:to:cc:message-id:in-reply-to:references:subject :mime-version; bh=0ehaMkvvpSbzYHZAIXM7k4T1Z+DZFTNbYNCRbcX/gb8=; b=nY26WqXJHLwx6J2g7soEnj9pHYxQNUliXrj+jieCtlua5tkv2Mg7HiNPri2cN3j/Am SIgl1+4W2MpE/39uI7bRYzJchZKe7DE8zH8jNReGCOzd+JJPEN/EG+b+9S7TP9YkmJIk CRwgnangkJyM+Nly0P3kWG0rhWi6Uj+tRWENRoVtcx6wSHbPDAtD5pDgOaLj8aRveZ0v 1hUFBlkfos4RYKwpg8BFzstE+2x5ZB/t3SnoZAJ3WDWMR+BGF3Rj/EmIJctTq/Cwb21D ps2TxdvIMO4e/SM0v/lCP0HXTTeuvT35tiSA37TGxwFAo58UVYEuiLG7/d8AsIzE0d3l +wJw==
X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:message-id:in-reply-to :references:subject:mime-version; bh=0ehaMkvvpSbzYHZAIXM7k4T1Z+DZFTNbYNCRbcX/gb8=; b=sPaeT6PvJIi16Elz3FWmIhlvy/NJrx1t3SXheeMox3AVAwf7ukbfpNeSgAK4K8KWi+ tdibekJ658Ptj1JEZQ4g//6yTfyQvwQirlaV50ZHkE1KQWuhJ8bjYXPBDsY+0KsZ0Iaf VTZh5s8M643IqMCNJcjzjKkVrHTSCaWNLwX73YpCyC1ziGm3tEQ8TN1E3f0uYYpPkZlA qAI3cJRyl30fAcCw/SVZ484Fsc5gDlrs3ecvH5yhGr8vzTpcUUCLH0/eFEk7xfgvjuLp SNI4WB0DBxFo6inYxZGkCod3s0xMh86Jz5gDZfKDNFUqAdLhQMLhX0gJLK1dJX41FQ5k R5hA==
X-Gm-Message-State: AOAM531ABQlTObrbpCNdMD5SK2P7B/A2yBO1aR/xHewYlZqHW+6pfVHu ppTkcBORe9RV9awQDqNLdxZg/figDE8=
X-Google-Smtp-Source: ABdhPJwDYV0EhaU4SCi9EwQIgSgA1uJ1v1cT+OYo7N0QZTWFcNTuxOHonu8V7dBKBSGqqMnGODtWew==
X-Received: by 2002:aa7:868e:0:b029:25e:cf47:30d8 with SMTP id d14-20020aa7868e0000b029025ecf4730d8mr3802693pfo.69.1618789642383; Sun, 18 Apr 2021 16:47:22 -0700 (PDT)
Received: from [192.168.1.2] (c-73-63-232-212.hsd1.ca.comcast.net. [73.63.232.212]) by smtp.gmail.com with ESMTPSA id c23sm2937412pgj.50.2021.04.18.16.47.21 (version=TLS1_2 cipher=ECDHE-ECDSA-AES128-GCM-SHA256 bits=128/128); Sun, 18 Apr 2021 16:47:21 -0700 (PDT)
Date: Sun, 18 Apr 2021 16:47:11 -0700
From: Jeff Tantsura <jefftant.ietf@gmail.com>
To: draft-ietf-idr-eag-distribution@ietf.org, Alvaro Retana <aretana.ietf@gmail.com>
Cc: Susan Hares <shares@ndzh.com>, idr-chairs@ietf.org, IDR List <idr@ietf.org>
Message-ID: <7c725fa0-52ec-4713-98e3-9bd83d4cb8e3@Spark>
In-Reply-To: <CAMMESsy7QcDch_iPwou6sEHLWGoFUAir=TprfsoZ_T-yuipcBg@mail.gmail.com>
References: <CAMMESsy7QcDch_iPwou6sEHLWGoFUAir=TprfsoZ_T-yuipcBg@mail.gmail.com>
X-Readdle-Message-ID: 7c725fa0-52ec-4713-98e3-9bd83d4cb8e3@Spark
MIME-Version: 1.0
Content-Type: multipart/alternative; boundary="607cc508_436c6125_650b"
Archived-At: <https://mailarchive.ietf.org/arch/msg/idr/-anPzSZE52TjeUOc1piCEEL_5Kk>
Subject: Re: [Idr] AD Review of draft-ietf-idr-eag-distribution-15
X-BeenThere: idr@ietf.org
X-Mailman-Version: 2.1.29
Precedence: list
List-Id: Inter-Domain Routing <idr.ietf.org>
List-Unsubscribe: <https://www.ietf.org/mailman/options/idr>, <mailto:idr-request@ietf.org?subject=unsubscribe>
List-Archive: <https://mailarchive.ietf.org/arch/browse/idr/>
List-Post: <mailto:idr@ietf.org>
List-Help: <mailto:idr-request@ietf.org?subject=help>
List-Subscribe: <https://www.ietf.org/mailman/listinfo/idr>, <mailto:idr-request@ietf.org?subject=subscribe>
X-List-Received-Date: Sun, 18 Apr 2021 23:47:30 -0000

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 <aretana.ietf@gmail.com>om>, 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]