Re: [Idr] John Scudder's Discuss on draft-ietf-idr-bgpls-srv6-ext-12: (with DISCUSS and COMMENT)
Alvaro Retana <aretana.ietf@gmail.com> Wed, 01 February 2023 22:35 UTC
Return-Path: <aretana.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 6760DC1516F3; Wed, 1 Feb 2023 14:35:46 -0800 (PST)
X-Virus-Scanned: amavisd-new at amsl.com
X-Spam-Flag: NO
X-Spam-Score: -1.094
X-Spam-Level:
X-Spam-Status: No, score=-1.094 tagged_above=-999 required=5 tests=[BAYES_00=-1.9, DKIM_SIGNED=0.1, DKIM_VALID=-0.1, DKIM_VALID_AU=-0.1, DKIM_VALID_EF=-0.1, FREEMAIL_FROM=0.001, FREEMAIL_REPLY=1, GB_SUMOF=5, HTML_MESSAGE=0.001, RCVD_IN_DNSWL_HI=-5, RCVD_IN_ZEN_BLOCKED_OPENDNS=0.001, SPF_HELO_NONE=0.001, SPF_PASS=-0.001, URIBL_BLOCKED=0.001, URIBL_DBL_BLOCKED_OPENDNS=0.001, URIBL_ZEN_BLOCKED_OPENDNS=0.001] autolearn=no autolearn_force=no
Authentication-Results: ietfa.amsl.com (amavisd-new); dkim=pass (2048-bit key) header.d=gmail.com
Received: from mail.ietf.org ([50.223.129.194]) by localhost (ietfa.amsl.com [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id aj-9Rxf5wU1T; Wed, 1 Feb 2023 14:35:42 -0800 (PST)
Received: from mail-pf1-x429.google.com (mail-pf1-x429.google.com [IPv6:2607:f8b0:4864:20::429]) (using TLSv1.3 with cipher TLS_AES_128_GCM_SHA256 (128/128 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by ietfa.amsl.com (Postfix) with ESMTPS id 9BD7AC1516EA; Wed, 1 Feb 2023 14:35:42 -0800 (PST)
Received: by mail-pf1-x429.google.com with SMTP id z3so13659360pfb.2; Wed, 01 Feb 2023 14:35:42 -0800 (PST)
DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; h=cc:to:subject:message-id:date:mime-version:references:in-reply-to :from:from:to:cc:subject:date:message-id:reply-to; bh=+vYC2vJuagBjFjtasGzH4Be+kbIZzO7I7U/ydKZsXDU=; b=QN6TTVqvZDlcLx1tEms3BMCsXFwSSwMbModol9122Z+Kd2J6s96HKylWs4KBJ/qpqp powKHxkes47N5pV7WZpLRo7tAnr88FrTUYjH1AvZBt35MSUqTOkikYoSpVi6I91xyxWR 7CcO7n6uBTfr95vjKK4Q17EzBiOpVfT4/nHuqrHOY/Z//0kBM4wTEc95sBMNG1mAKnNm uHQdXCUAFT+2N6svqX6NIO1zXVd98H6ArLAjSXHTbOMO7qJE4VRCTsE+kRBdbggNo9sv QPlQxCKGd21dlHV5BCzQvz84pbK/2pHCMwpaCtzNZSybeJMhIXlhOHNuM+4azI697/J5 CmnQ==
X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=cc:to:subject:message-id:date:mime-version:references:in-reply-to :from:x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=+vYC2vJuagBjFjtasGzH4Be+kbIZzO7I7U/ydKZsXDU=; b=1R4bl4qpUESjwkSSUdhAkfpni50Bzzh/jAvhEhN7ISvtEgKT0f9sNi/a4HG/RuE/Lq zfopH/3U8RQKKZkMF24Ii1tJZnyNc6xchpYpUD8VjTRed++kPWYgWOOjtGv0FntaydiG n94yfAAPyR/yXJgwdSa6KAJ1P18hMvKEg/aR+83E5jk/RJ1l439Nu2MEJnjDbTQObrq6 plnp1ks7Wke6ksZSoMH6j3zJZeiyJRcj2nx8uosE696t2Dglq1mU6ZuWfpTjKHOjiMwF sI0gVXzaUx2pCXjf+x2eOJ/eHoyOV2KiZ3RonbL85i8jr6Qlr0d3LyR+5KYnFfLYpQOo 0CxA==
X-Gm-Message-State: AO0yUKW2MxzAVzJ7NVhec4u5XyXhtaNAXJThdds637mQQJumsJLWb+3N LHjyWaeeQLIQUzTcyfdCpBV+CfZMJcH8msyys0t6B9eP
X-Google-Smtp-Source: AK7set/PgvA+sz642SIT1u186+2pqY3HfHNvXpUFbvArsGhDC8S0o+na1ft28FPuaOdeMbcUlwph9UqS6uirs1jQGrU=
X-Received: by 2002:a05:6a00:851:b0:592:d6c:9729 with SMTP id q17-20020a056a00085100b005920d6c9729mr876261pfk.11.1675290941886; Wed, 01 Feb 2023 14:35:41 -0800 (PST)
Received: from 1058052472880 named unknown by gmailapi.google.com with HTTPREST; Wed, 1 Feb 2023 16:35:40 -0600
From: Alvaro Retana <aretana.ietf@gmail.com>
In-Reply-To: <CAH6gdPwZCT29otzH69n9PNQoyLpYB9hz3Q=fuhZMXQfWgxK8-Q@mail.gmail.com>
References: <167105275561.48205.3228688610682958007@ietfa.amsl.com> <CAH6gdPwZCT29otzH69n9PNQoyLpYB9hz3Q=fuhZMXQfWgxK8-Q@mail.gmail.com>
MIME-Version: 1.0
Date: Wed, 01 Feb 2023 16:35:40 -0600
Message-ID: <CAMMESszQ_OSTzzOLwYsPq0_YY2Qx8qRFJeaub4ZMw_i3RSCfWQ@mail.gmail.com>
To: John Scudder <jgs@juniper.net>, Ketan Talaulikar <ketant.ietf@gmail.com>
Cc: draft-ietf-idr-bgpls-srv6-ext@ietf.org, idr-chairs@ietf.org, shares@ndzh.com, The IESG <iesg@ietf.org>, idr@ietf.org
Content-Type: multipart/alternative; boundary="0000000000007c986705f3ab1132"
Archived-At: <https://mailarchive.ietf.org/arch/msg/idr/EdKGzswt8MLenmT_J4Pu2_gr2sM>
Subject: Re: [Idr] John Scudder's Discuss on draft-ietf-idr-bgpls-srv6-ext-12: (with DISCUSS and COMMENT)
X-BeenThere: idr@ietf.org
X-Mailman-Version: 2.1.39
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: Wed, 01 Feb 2023 22:35:46 -0000
John: Hi! Can you please take a look at this? [I think we may have talked about this earlier in the week, but I’m not sure. :-( Sorry for insisting.] Thanks! Alvaro. On January 14, 2023 at 8:20:18 AM, Ketan Talaulikar (ketant.ietf@gmail.com) wrote: Hi John, Thanks for your detailed review and your comments/suggestions. Please check inline below for responses. We've also posted an updated version with these changes included: https://datatracker.ietf.org/doc/html/draft-ietf-idr-bgpls-srv6-ext-13 On Thu, Dec 15, 2022 at 2:49 AM John Scudder via Datatracker < noreply@ietf.org> wrote: > John Scudder has entered the following ballot position for > draft-ietf-idr-bgpls-srv6-ext-12: Discuss > > When responding, please keep the subject line intact and reply to all > email addresses included in the To and CC lines. (Feel free to cut this > introductory paragraph, however.) > > > Please refer to > https://www.ietf.org/about/groups/iesg/statements/handling-ballot-positions/ > for more information about how to handle DISCUSS and COMMENT positions. > > > The document, along with other ballot positions, can be found here: > https://datatracker.ietf.org/doc/draft-ietf-idr-bgpls-srv6-ext/ > > > > ---------------------------------------------------------------------- > DISCUSS: > ---------------------------------------------------------------------- > > # John Scudder, RTG AD, comments for draft-ietf-idr-bgpls-srv6-ext-12 > CC @jgscudder > > ### Global, field definitions are unclear throughout > > I have a problem with how several different fields are defined. For > example, in > Section 3.1, the description of the flags field says, > > ``` > * Flags: 2 octet field. The flags are derived from the SRv6 > Capabilities sub-TLV/TLV of IS-IS (section 2 of > [I-D.ietf-lsr-isis-srv6-extensions]) and OSPFv3 (section 2 of > [I-D.ietf-lsr-ospfv3-srv6-extensions]). > ``` > There are two problems here: "derived", and "and". On consideration, I > think > "and" is the more serious, although it wasn't what initially caught my > attention. > Regarding "derived": Normally when I think of something being "derived > from" > something else, I think of some derivation function, e.g. pressure can be > derived from the volume of a confined gas according to Boyle's Law. Of > course, > the function can technically be the identity function, which appears to be > what > you mean here. There's a suggestion related to that in my second discuss > point, > below. > > Regarding "and": As written (here and elsewhere), you're saying that the > field > comes from both places at once. That doesn't make sense on the face of it, > and > it led me down a rabbit hole I'll spare you the description of. > > In the end, I guess how you arrived at your current text is probably by > trying > to capture that BGP-LS is a vessel to convey fields that originate in one > of > the IGPs, hence the fields aren't defined here and you don't want to write > rules or semantics for them here. I think there are two basic changes you > can > make to your pattern that would address my concern. > > First and less importantly, choose some verb other than "derive". "Copied" > seems like the most precise, but something else could be proposed. (But > again, > see my second discuss point.) > KT> Ack. How about using "copied" in the context of a specific field and "derived" when we are talking about something like a TLV/sub-TLV? > > Second, use "or", not "and", or a similar approach that accomplishes the > same > end. > KT> Ack - the way the text is structured, "or" is more appropriate and this has been fixed. > > Rewriting the example text according to this suggestion, we'd have > something > like, > > ``` > * Flags: 2 octet field. The flags are copied from the SRv6 > Capabilities sub-TLV/TLV of either IS-IS (section 2 of > [I-D.ietf-lsr-isis-srv6-extensions]) or OSPFv3 (section 2 of > [I-D.ietf-lsr-ospfv3-srv6-extensions]), depending on the > source protocol. > ``` > I don't insist that you use this exact pattern, it's just an example of a > minimal patch that addresses the issue, I don't claim it's the best fix. > And > TBH, if you made the other changes but left "copied" as "derived" I think > it > would have been clear enough in context. Indeed, I think just changing > "and" to > "or" could pass the smell test, especially if there's an introductory > paragraph > somewhere, setting up the pattern. (Again, see my second point.) > > For some examples of how we've solved this for past BGP-LS specs, we can > consider RFCs 8814 and 9085. In Section 2 of RFC 8814, it says, "When a > BGP-LS > speaker is originating the topology learnt via link-state routing protocols > such as OSPF or IS-IS, the MSD information for the nodes and their links is > sourced from the underlying extensions as defined in [RFC8476] and > [RFC8491], > respectively." Or, if we look at RFC 9085, you use "derived" but mostly in > ways > I find clear enough, for example, §2.3.5: > > ``` > The > information advertised in the Range TLV is derived from the SID/Label > Binding TLV in the case of IS-IS and the OSPFv2/OSPFv3 Extended > Prefix Range TLV in the case of OSPFv2/OSPFv3. > ``` > The use of "in the case of" makes it clear that there's only a single > source > for the field in any given instance. Or §2.1.1: > KT> I am leaning towards the use of "... in the case of ..." in addition to the "or" to further disambiguate. Does that work? > > ``` > The SID/Label TLV is used as a sub-TLV by the SR Capabilities > (Section 2.1.2) and Segment Routing Local Block (SRLB) > (Section 2.1.4) TLVs. This information is derived from the protocol- > specific advertisements. > > * IS-IS, as defined by the SID/Label Sub-TLV in Section 2.3 of > [RFC8667]. > > * OSPFv2/OSPFv3, as defined by the SID/Label Sub-TLV in Section 2.1 > of [RFC8665] and Section 3.1 of [RFC8666]. > ``` > The use of "from the protocol-specific advertisements" makes it clear in > context. > > I'm not going to flag each instance of this pattern in my review, but ask > that > you do a global search for "derived". I've noted some other issues that > won't > be revealed by a search for "derived" in my COMMENT section. > > (I notice that in Section 7.1 you have "The flags map to the IS-IS or > OSPFv3..." -- which works just fine as far as I'm concerned.) > > ### Section 2 > > In the same vein, it also seems worth tightening up this paragraph from > Section > 2: > > ``` > When a BGP-LS router advertises topology information that it sources > from the underlying link-state routing protocol (as described in > [RFC7752]), then it maps the corresponding SRv6 information from the > SRv6 extensions for IS-IS [I-D.ietf-lsr-isis-srv6-extensions] and > OSPFv3 [I-D.ietf-lsr-ospfv3-srv6-extensions] protocols to their BGP- > LS TLVs/sub-TLVs for all SRv6 capable nodes in that routing protocol > domain. When a BGP-LS router advertises topology information from > the BGP routing protocol (e.g., for EPE), then it advertises the SRv6 > information from the local node alone. > ``` > Perhaps this could be made more specific, for example: > > ``` > In most cases, a BGP-LS router will advertise topology information it > has > sourced from an underlying link-state routing protocol, as described in > [RFC7752]. In such cases, it will derive the corresponding SRv6 > information > from the SRv6 extensions for IS-IS [I-D.ietf-lsr-isis-srv6-extensions] > or > OSPFv3 [I-D.ietf-lsr-ospfv3-srv6-extensions], as applicable. In > practice, > this derivation comprises a simple copy of the relevant field into a > field > of the corresponding BGP-LS TLV/sub-TLV. This document defines an > exception case where a BGP-LS router can originate topology information > itself, for EPE. Such information is advertised only on behalf of the > local router, in contrast to the usual [RFC7752] behavior of advertising > information for an entire underlying IGP domain. > ``` > This rolls together a few of the windmills I've been tilting at, including > supplying a small definition of "derives" to perhaps minimize rewriting in > the > rest of the document. (See also my second COMMENT.) > KT> Thanks for that suggestion. I've incorporated a lot from this. One thing to clarify is that RFC7752 covered both sourcing from IGPs and Direct/Static. BGP EPE was in a different RFC added later and there is work to extend it for BGP-only topologies (please see further responses). > > > ---------------------------------------------------------------------- > COMMENT: > ---------------------------------------------------------------------- > > ## COMMENT > > ### Global, is RFC 7752 the right reference? > > Since we have rfc7752bis in the final stages of preparation (thanks for > your > work on it!), is RFC 7752 the best reference for this document to cite? > KT> We didn't want to put a dependency between this and RFC7752bis given that there are already many BGP-LS documents that point to RFC7752. Readers/implementers/operators would need to "move" to RFC7752bis in their minds when they think of BGP-LS. One more in that list should not be much of an issue? > > ### Section 1 isn't EPE the only case? > > Related to my DISCUSS, > > ``` > This document describes extensions to BGP-LS to advertise the SRv6 > SIDs and other SRv6 information from all the SRv6 capable nodes in > the IGP domain when sourced from link-state routing protocols and > directly from individual SRv6 capable nodes (e.g. when sourced from > BGP for EPE). > ``` > Are there any other concrete cases other than EPE, that the document > defines, > where the router sources data directly into BGP-LS? If not, consider > rewriting > to make the text exhaustive instead of exemplary, by removing the "e.g." or > turning it into an "i.e.". > KT> We have a WG document https://datatracker.ietf.org/doc/draft-ietf-idr-bgp-ls-bgp-only-fabric/ that extends this beyond BGP EPE. We don't want to preclude that. > > ### Section 4.1 IS-IS and OSPFv3 > > Related to my DISCUSS point, here we have > > ``` > The SRv6 End.X SID TLV is used to advertise the SRv6 SIDs associated > with an IGP Adjacency SID behavior that correspond to a point-to- > point or point-to-multipoint link or adjacency of the node running > the IS-IS and OSPFv3 protocols. > ``` > That should be "the IS-IS or OSPFv3 protocol", I think, since generally, a > node > won't be running both at the same time. (Noted here since searching for > "derived" won't find it.) > KT> Ack > > ### Section 4.2 > > Again related to DISCUSS point, > > ``` > * Neighbor ID : 6 octets of Neighbor System-ID in IS-IS SRv6 LAN > End.X SID TLV and 4 octets of Neighbor Router-id in the OSPFv3 > SRv6 LAN End.X SID TLV. > ``` > Should be "... or 4 octets," I think. Figure 3 gets this right, so > regardless > of the bigger picture it's worth changing just for consistency with the > figure. > KT> Fixed. > > ### Section 4.3 > > Again related. > > ``` > Each MSD-type is encoded in the BGP-LS Link MSD TLV as a one-octet > type followed by a one-octet value as derived from the IS-IS and > OSPFv3 Link MSD advertisements as specified in [RFC8814]. > ``` > Debatable since you refer back to a single underlying spec, but I think > "IS-IS > or OSPFv3". > KT> I know what you mean, and we have aligned for consistency. > > ### Section 6.1 > > This looks like "the exception that proves the rule": > > ``` > When advertising the SRv6 SIDs from the IGPs, the SID information is > derived from the SRv6 End SID sub-TLV of IS-IS (section 7.2 of > [I-D.ietf-lsr-isis-srv6-extensions]) and OSPFv3 (section 8 of > [I-D.ietf-lsr-ospfv3-srv6-extensions]). > ``` > That is to say, the "when advertising" part implies you might advertise > information from other sources too, I guess the Protocol-ID field has the > same > implication since that also can take values OSPFv2, Direct, and Static. So > then, if the source is NOT IS-IS or OSPFv3, where is the SID information > derived from? > KT> It is sourced by BGP-LS "direct"ly from the local node as covered by RFC7752(bis). Any locally instantiated SRv6 SID can be advertised by that node via BGP-LS - refer https://datatracker.ietf.org/doc/html/rfc8986#section-8.4. I realize that we are missing some details here and I've clarified as part of the cleanup for the "derived" usage. > > ## NITS > > ### Section 4.2 > > ``` > For a LAN interface, an IGP node announces normally only its > adjacency to the IS-IS pseudo-node (or the equivalent OSPF DR). > ``` > I think that should be "normally announces". As written it expresses the > implication that it announces other adjacencies abnormally. Maybe > "ordinarily" > would be better than "normally" since it doesn't carry the same risk of > ambiguity. > KT> Ack. > > ``` > Without this > TLV, multiple BGP-LS Link NLRIs would need to be originated for each > additional adjacency to advertise the SRv6 End.X SID TLVs for these > neighbor adjacencies. > ``` > Do you mean "... would need to be originated, one for each additional > adjacency, to advertise..."? > KT> Ack. Rephrased for clarity. > > ### Section 5.1 > > ``` > on that node. These Locators are advertised as BGP-LS Prefix NLRI > objects along with the SRv6 Locator TLV in its BGP-LS Attribute. > ``` > "Each Locator is advertised as a BGP-LS Prefix NLRI object along with the > SRv6 > Locator TLV in its BGP-LS Attribute." (Agreement in number.) > KT> Ack > > ### Section 7.2 > > ``` > - B-Flag: Backup Flag. If set, the SID is eligible for > protection using fast reroute (FRR). > ``` > I suggest "eligible to be protected" instead. > KT> Ack > > ``` > - Other bits are reserved for future use and MUST be set to 0 and > ignored on receipt. > ``` > ``` > * Reserved: 2 octet field. The value MUST be set to 0 and ignored > on receipt. > ``` > For both these cases, I suggest "MUST be set to 0 when originated, and..." > KT> Ack > > ### Section 8 > > ``` > The sum of the LB Length, LN Length, Func. Length, and Arg. Length > MUST be less than or equal to 128. > ``` > I was a little surprised these don't have to total exactly 128 but I guess > this > is a deliberate choice and consistent with other specs? (Presumably, any > leftover bits are don't-care.) > KT> Yes. Please refer to https://datatracker.ietf.org/doc/html/rfc8986#section-3.1 Thanks, Ketan > > ## Notes > > This review is in the ["IETF Comments" Markdown format][ICMF], You can use > the > [`ietf-comments` tool][ICT] to automatically convert this review into > individual GitHub issues. > > [ICMF]: https://github.com/mnot/ietf-comments/blob/main/format.md > [ICT]: https://github.com/mnot/ietf-comments > > > >
- [Idr] John Scudder's Discuss on draft-ietf-idr-bg… John Scudder via Datatracker
- Re: [Idr] John Scudder's Discuss on draft-ietf-id… Ketan Talaulikar
- Re: [Idr] John Scudder's Discuss on draft-ietf-id… Alvaro Retana
- Re: [Idr] John Scudder's Discuss on draft-ietf-id… John Scudder
- Re: [Idr] John Scudder's Discuss on draft-ietf-id… John Scudder