Re: [Lsr] AD Review of draft-ietf-isis-segment-routing-extensions-22

"Les Ginsberg (ginsberg)" <ginsberg@cisco.com> Wed, 20 March 2019 23:37 UTC

Return-Path: <ginsberg@cisco.com>
X-Original-To: lsr@ietfa.amsl.com
Delivered-To: lsr@ietfa.amsl.com
Received: from localhost (localhost [127.0.0.1]) by ietfa.amsl.com (Postfix) with ESMTP id E55B312F1AB; Wed, 20 Mar 2019 16:37:47 -0700 (PDT)
X-Virus-Scanned: amavisd-new at amsl.com
X-Spam-Flag: NO
X-Spam-Score: -14.502
X-Spam-Level:
X-Spam-Status: No, score=-14.502 tagged_above=-999 required=5 tests=[BAYES_00=-1.9, DKIMWL_WL_MED=-0.001, DKIM_SIGNED=0.1, DKIM_VALID=-0.1, DKIM_VALID_AU=-0.1, RCVD_IN_DNSWL_HI=-5, SPF_PASS=-0.001, USER_IN_DEF_DKIM_WL=-7.5] autolearn=ham autolearn_force=no
Authentication-Results: ietfa.amsl.com (amavisd-new); dkim=pass (1024-bit key) header.d=cisco.com header.b=UCJK2Gfh; dkim=pass (1024-bit key) header.d=cisco.onmicrosoft.com header.b=DeaYoG3K
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 vl79zvC-_wl6; Wed, 20 Mar 2019 16:37:42 -0700 (PDT)
Received: from alln-iport-3.cisco.com (alln-iport-3.cisco.com [173.37.142.90]) (using TLSv1.2 with cipher DHE-RSA-SEED-SHA (128/128 bits)) (No client certificate requested) by ietfa.amsl.com (Postfix) with ESMTPS id 3C67612788F; Wed, 20 Mar 2019 16:37:41 -0700 (PDT)
DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=cisco.com; i=@cisco.com; l=58066; q=dns/txt; s=iport; t=1553125062; x=1554334662; h=from:to:cc:subject:date:message-id:references: in-reply-to:content-transfer-encoding:mime-version; bh=ocGKu958zNwIK290EVYkH7TOnHreqhpO1lAlBVUhXHg=; b=UCJK2GfheqnuGTBn1tyyKLPPPw8y30If/QGAMxQgwJrJdJRpBXsduWpN W/mQUrdebaavD0yGRZFrx4/pidHhxuvq6tkpTWJeQ2A/5IZyGYVkJgVDL H3vw4o7rwa77IwvQPy2hh594tKwE5WaNQIg8z2CQcUVeHIZyR7Z2y5zPi s=;
IronPort-PHdr: 9a23:Z7GGPB3+dEYtO8JHsmDT+zVfbzU7u7jyIg8e44YmjLQLaKm44pD+JxKGt+51ggrPWoPWo7JfhuzavrqoeFRI4I3J8RVgOIdJSwdDjMwXmwI6B8vQBkz9N/TndSMSF8VZX1gj9Ha+YgBY
X-IronPort-Anti-Spam-Filtered: true
X-IronPort-Anti-Spam-Result: A0AoAABlzZJc/5xdJa1kGwEBAQEDAQEBBwMBAQGBUQYBAQELAYE9JCwDgVwECyeEDYNHA4RSilSCV3yIN41UgS4UgRADVAsBASyBKgGDFQIXhFAWDDQJDQEBAwEBCQEDAm0cDIVKAQEBAQIBGgEIEQwBASULBwEEBwQCAQgOAwEDAQEDAhEVAgICHxEVAgYIAgQBDQUIE4RlAw0IAQKgBwKKFHGBL4J4AQEFhQcNC4IMCIELJAGKEoEfF4FAP4EQAUaBTi4bNYJXgWUBDCICFhUPBhOCSzGCJooRIII4lzYqNgkCiyRBhAqDWIF9hXOLdYsSgReGHIgygykCBAIEBQIOAQEFgUw4QoEUcBWDJ4IKDBeDS4pTcoEoiRmCTAEB
X-IronPort-AV: E=Sophos;i="5.60,250,1549929600"; d="scan'208";a="250168867"
Received: from rcdn-core-5.cisco.com ([173.37.93.156]) by alln-iport-3.cisco.com with ESMTP/TLS/DHE-RSA-SEED-SHA; 20 Mar 2019 23:37:39 +0000
Received: from XCH-ALN-008.cisco.com (xch-aln-008.cisco.com [173.36.7.18]) by rcdn-core-5.cisco.com (8.15.2/8.15.2) with ESMTPS id x2KNbd3e016101 (version=TLSv1.2 cipher=AES256-SHA bits=256 verify=FAIL); Wed, 20 Mar 2019 23:37:39 GMT
Received: from xhs-aln-002.cisco.com (173.37.135.119) by XCH-ALN-008.cisco.com (173.36.7.18) with Microsoft SMTP Server (TLS) id 15.0.1473.3; Wed, 20 Mar 2019 18:37:38 -0500
Received: from xhs-rcd-002.cisco.com (173.37.227.247) by xhs-aln-002.cisco.com (173.37.135.119) with Microsoft SMTP Server (TLS) id 15.0.1473.3; Wed, 20 Mar 2019 18:37:37 -0500
Received: from NAM04-SN1-obe.outbound.protection.outlook.com (72.163.14.9) by xhs-rcd-002.cisco.com (173.37.227.247) with Microsoft SMTP Server (TLS) id 15.0.1473.3 via Frontend Transport; Wed, 20 Mar 2019 18:37:37 -0500
DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=cisco.onmicrosoft.com; s=selector1-cisco-com; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=ocGKu958zNwIK290EVYkH7TOnHreqhpO1lAlBVUhXHg=; b=DeaYoG3KCBIK6XC8XxGNRztjxdsD2GAD+c5sQTDv9kgnT7r2twTHdF5WQpjOwWpO3TIvcGm8m5yAPSpbRw9sP0xMluhJSXQjEng+tMMgsTzdPPRUPuQvvv0wXCaPDymwOn2ToyfopHr9RbDHtroy1tMfCC6uengsu3FhqlcyCzg=
Received: from BYAPR11MB3638.namprd11.prod.outlook.com (20.178.237.19) by BYAPR11MB2839.namprd11.prod.outlook.com (52.135.228.33) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.1709.14; Wed, 20 Mar 2019 23:37:35 +0000
Received: from BYAPR11MB3638.namprd11.prod.outlook.com ([fe80::b414:f534:f20a:a26d]) by BYAPR11MB3638.namprd11.prod.outlook.com ([fe80::b414:f534:f20a:a26d%3]) with mapi id 15.20.1709.015; Wed, 20 Mar 2019 23:37:35 +0000
From: "Les Ginsberg (ginsberg)" <ginsberg@cisco.com>
To: Alvaro Retana <aretana.ietf@gmail.com>, "draft-ietf-isis-segment-routing-extensions@ietf.org" <draft-ietf-isis-segment-routing-extensions@ietf.org>
CC: Uma Chunduri <uma.chunduri@huawei.com>, "lsr-chairs@ietf.org" <lsr-chairs@ietf.org>, "lsr@ietf.org" <lsr@ietf.org>
Thread-Topic: AD Review of draft-ietf-isis-segment-routing-extensions-22
Thread-Index: AQHU32s2LXimbEtSIUKKc+aYWZUjdqYVKWSw
Date: Wed, 20 Mar 2019 23:37:35 +0000
Message-ID: <BYAPR11MB36381DC64FB61562505987D7C1410@BYAPR11MB3638.namprd11.prod.outlook.com>
References: <CAMMESsz7nvqpSHY78j_3Gkksbf=SiyMfv8pV83iRN9GUfJJ_YA@mail.gmail.com>
In-Reply-To: <CAMMESsz7nvqpSHY78j_3Gkksbf=SiyMfv8pV83iRN9GUfJJ_YA@mail.gmail.com>
Accept-Language: en-US
Content-Language: en-US
X-MS-Has-Attach:
X-MS-TNEF-Correlator:
authentication-results: spf=none (sender IP is ) smtp.mailfrom=ginsberg@cisco.com;
x-originating-ip: [2001:420:c0c8:1003::3f]
x-ms-publictraffictype: Email
x-ms-office365-filtering-correlation-id: 3c004c8c-c55d-43a9-eade-08d6ad8d0838
x-microsoft-antispam: BCL:0; PCL:0; RULEID:(2390118)(7020095)(4652040)(8989299)(4534185)(4627221)(201703031133081)(201702281549075)(8990200)(5600127)(711020)(4605104)(2017052603328)(7153060)(7193020); SRVR:BYAPR11MB2839;
x-ms-traffictypediagnostic: BYAPR11MB2839:
x-microsoft-antispam-prvs: <BYAPR11MB283957709A0125F18747ACDDC1410@BYAPR11MB2839.namprd11.prod.outlook.com>
x-forefront-prvs: 098291215C
x-forefront-antispam-report: SFV:NSPM; SFS:(10009020)(39860400002)(376002)(346002)(396003)(366004)(136003)(13464003)(37854004)(199004)(189003)(51444003)(14454004)(2906002)(14444005)(8936002)(446003)(74316002)(486006)(7736002)(229853002)(30864003)(9686003)(11346002)(305945005)(476003)(316002)(106356001)(46003)(6436002)(53936002)(110136005)(53946003)(4326008)(25786009)(54906003)(105586002)(99286004)(68736007)(102836004)(186003)(76176011)(53546011)(8676002)(256004)(71200400001)(7696005)(52536014)(86362001)(6506007)(55016002)(6116002)(5660300002)(33656002)(97736004)(66574012)(2501003)(478600001)(6246003)(81156014)(81166006)(71190400001)(5024004)(569006); DIR:OUT; SFP:1101; SCL:1; SRVR:BYAPR11MB2839; H:BYAPR11MB3638.namprd11.prod.outlook.com; FPR:; SPF:None; LANG:en; PTR:InfoNoRecords; A:1; MX:1;
received-spf: None (protection.outlook.com: cisco.com does not designate permitted sender hosts)
x-ms-exchange-senderadcheck: 1
x-microsoft-antispam-message-info: miwL9SXvSnSc4Caxt/UHIeMlhjm9MW7phq734duO4Lqmo3DFgck7lg40zFji097PYkJMnnfCAt4Ki+iax5PX6Udyujh7fJ9a4hASqyai7lGl9ui74boPDj3VqVGitBk0lnUb38UiVeAQPYBXmtpFjAUQZjWi1DMkl3pv/SCAsIQ4b5jDyN6d+7TLpm7/dYdNSdaFHCSqa/ozjH/Jo+gDpV1QEFEgBqcU465j1OsP/C5edDoFZFGDb4TRwLEHIhqheWHHq0ctF6w1TcI26m+OFq3Xpu6dMKd9h5Gycqxd41M3gBCUJ332g/n5u2LzJWhzBnj1r83ZVb6zmmX6YCMmVxzRTPMZSQ/uXnguptIP7/Y1F/FlHFD3QSECDojlRxM5OcaWKD8owqSccVRDs/lbbYeE6DBfDOjY/ZeUpCecNPg=
Content-Type: text/plain; charset="utf-8"
Content-Transfer-Encoding: base64
MIME-Version: 1.0
X-MS-Exchange-CrossTenant-Network-Message-Id: 3c004c8c-c55d-43a9-eade-08d6ad8d0838
X-MS-Exchange-CrossTenant-originalarrivaltime: 20 Mar 2019 23:37:35.7174 (UTC)
X-MS-Exchange-CrossTenant-fromentityheader: Hosted
X-MS-Exchange-CrossTenant-id: 5ae1af62-9505-4097-a69a-c1553ef7840e
X-MS-Exchange-CrossTenant-mailboxtype: HOSTED
X-MS-Exchange-Transport-CrossTenantHeadersStamped: BYAPR11MB2839
X-OriginatorOrg: cisco.com
X-Outbound-SMTP-Client: 173.36.7.18, xch-aln-008.cisco.com
X-Outbound-Node: rcdn-core-5.cisco.com
Archived-At: <https://mailarchive.ietf.org/arch/msg/lsr/Aa9vFlLCoQuJoCHOnorgggLkH_U>
Subject: Re: [Lsr] AD Review of draft-ietf-isis-segment-routing-extensions-22
X-BeenThere: lsr@ietf.org
X-Mailman-Version: 2.1.29
Precedence: list
List-Id: Link State Routing Working Group <lsr.ietf.org>
List-Unsubscribe: <https://www.ietf.org/mailman/options/lsr>, <mailto:lsr-request@ietf.org?subject=unsubscribe>
List-Archive: <https://mailarchive.ietf.org/arch/browse/lsr/>
List-Post: <mailto:lsr@ietf.org>
List-Help: <mailto:lsr-request@ietf.org?subject=help>
List-Subscribe: <https://www.ietf.org/mailman/listinfo/lsr>, <mailto:lsr-request@ietf.org?subject=subscribe>
X-List-Received-Date: Wed, 20 Mar 2019 23:37:48 -0000

Alvaro -

Thanx for the review.
We will work on addressing the comments.

In regards to the relationship with RFC 7794, there is a history here.
The N and R flags were originally defined as part of the prefix-sid sub-TLV in early versions of this draft. However, it was realized that there were use cases for these flags that had nothing to do with SR. 
This prompted writing what became RFC 7794. But because there are early deployments in the field which were already using the N/R flags in prefix-SID sub-TLV we could not immediately deprecate the flags in prefix-SID sub-TLV.

The intent is for RFC 7794 advertisement to become the sole place where N/R flags are advertised - but in the interim implementations have to support logic which is roughly:

If (prefix attributes sub-TLV is present)
  Use prefix attributes sub-TLV settings and ignore N/R flag in prefix-SID sub-TLV if present
Else
  Use N/R flag in prefix-SID sub-TLV if present

"Someday" all implementations will support prefix attributes sub-TLV and there will be no use case for the flags in prefix-SID sub-TLV. At that point the flags can be repurposed.

When will "someday" happen??
I think you know as well as I that such transitions take many years.

The timing of this is not critical unless we run out of flag bits in the prefix-SID sub-TLV and need to reuse the N/R bits. At this point we do not foresee this happening any time soon.

   Les



> -----Original Message-----
> From: Alvaro Retana <aretana.ietf@gmail.com>
> Sent: Wednesday, March 20, 2019 3:21 PM
> To: draft-ietf-isis-segment-routing-extensions@ietf.org
> Cc: Uma Chunduri <uma.chunduri@huawei.com>; lsr-chairs@ietf.org;
> lsr@ietf.org
> Subject: AD Review of draft-ietf-isis-segment-routing-extensions-22
> 
> Dear authors:
> 
> I just finished reviewing this document.  Please take a look at the in-line
> comments below.  In general, I think that some more work is needed.  I will
> wait for that before starting the IETF Last Call.
> 
> There is a significant issue that I want to highlight here (there are also
> related comments in-line):
> 
> - §2.1.1.2 talks about interaction with rfc7794 and a transition period
> which apparently ends in this specification not being used.  Am I reading
> that correctly?
> 
> Thanks!
> 
> Alvaro.
> 
> [Line numbers come from idnits.]
> 
> ...
> 107 1.  Introduction
> 
> 109   Segment Routing (SR) allows for a flexible definition of end-to-end
> 110   paths within IGP topologies by encoding paths as sequences of
> 111   topological sub-paths, called "segments".  These segments are
> 112   advertised by the link-state routing protocols (IS-IS and OSPF).
> 113   Prefix segments represent an ecmp-aware shortest-path to a prefix (or
> 114   a node), as per the state of the IGP topology.  Adjacency segments
> 115   represent a hop over a specific adjacency between two nodes in the
> 116   IGP.  A prefix segment is typically a multi-hop path while an
> 117   adjacency segment, in most of the cases, is a one-hop path.  SR's
> 118   control-plane can be applied to both IPv6 and MPLS data-planes, and
> 119   do not require any additional signaling (other than the regular IGP).
> 120   For example, when used in MPLS networks, SR paths do not require any
> 121   LDP or RSVP-TE signaling.  Still, SR can interoperate in the presence
> 122   of LSPs established with RSVP or LDP.
> 
> [nit] s/ecmp-aware/ECMP-aware
> 
> 
> 124   There are additional segment types, e.g., Binding SID defined in
> 125   [RFC8402].  This draft also defines an advertisement for one type of
> 126   BindingSID: the Mirror Context segment.
> 
> [nit] s/BindingSID/Binding SID
> 
> 
> 128   This draft describes the necessary IS-IS extensions that need to be
> 129   introduced for Segment Routing operating on an MPLS data-plane.
> 
> 131   Segment Routing architecture is described in [RFC8402].
> 
> [nit] s/Segment Routing architecture/The Segment Routing architecture
> 
> 
> 133   Segment Routing use cases are described in [RFC7855].
> 
> 135 2.  Segment Routing Identifiers
> 
> 137   Segment Routing architecture ([RFC8402]) defines different types of
> 138   Segment Identifiers (SID).  This document defines the IS-IS encodings
> 139   for the IGP-Prefix-SID, the IGP-Adjacency-SID, the IGP-LAN-Adjacency-
> 140   SID and the Binding-SID.
> 
> [nit] s/([RFC8402])/[RFC8402]
> 
> [nit] s/Segment Routing architecture/The Segment Routing architecture
> 
> [major] rfc8402 doesn't use the names above.  Please be consistent.
> 
> 
> 142 2.1.  Prefix Segment Identifier (Prefix-SID Sub-TLV)
> 
> 144   A new IS-IS sub-TLV is defined: the Prefix Segment Identifier sub-TLV
> 145   (Prefix-SID sub-TLV).
> 
> 147   The Prefix-SID sub-TLV carries the Segment Routing IGP-Prefix-SID as
> 148   defined in [RFC8402].  The 'Prefix SID' MUST be unique within a given
> 149   IGP domain (when the L-flag is not set).  The 'Prefix SID' MUST carry
> 150   an index (when the V-flag is not set) that determines the actual SID/
> 151   label value inside the set of all advertised SID/label ranges of a
> 152   given router.  A receiving router uses the index to determine the
> 153   actual SID/label value in order to construct forwarding state to a
> 154   particular destination router.
> 
> [major] What if the Prefix SID is not unique?  What should the receiver do?
> 
> [major] "...MUST carry an index...that determines the actual SID/label
> value..."  What are you trying to specify with this "MUST"?  The
> description of the V-flag (below) already points at the value being an
> index, but the qualification ("that determines...") makes the enforcement
> of this "MUST" more complex: the receiver has to verify that the index
> actually results in a SID/label inside a range for it to be valid.   What
> should the receiver do if that is not the case?
> 
> Suggestion: Rearrange the text so that the TLV and its fields are described
> first.  I think that then some of the text above won't be needed.
> 
> 
> 156   In many use-cases a 'stable transport' IP Address is overloaded as an
> 157   identifier of a given node.  Because the IP Prefixes may be re-
> 158   advertised into other levels there may be some ambiguity (e.g.
> 159   Originating router vs. L1L2 router) for which node a particular IP
> 160   prefix serves as identifier.  The Prefix-SID sub-TLV contains the
> 161   necessary flags to disambiguate IP Prefix to node mappings.
> 162   Furthermore if a given node has several 'stable transport' IP
> 163   addresses there are flags to differentiate those among other IP
> 164   Prefixes advertised from a given node.
> 
> [minor] Again, this text may be clearer if the TLV description was
> presented first.  If you decide not to rearrange the text, at least put
> forward references so readers can look forward to find out how the
> statement above is achieved.
> 
> 
> ...
> 180   When the IP Reachability TLV is propagated across level boundaries,
> 181   the Prefix-SID sub-TLV SHOULD be kept.
> 
> [major] When would this sub-TLV not be kept?  IOW, why is that not a
> "MUST"?
> 
> §2.1.2: "The Prefix-SID sub-TLV MUST be preserved when the IP Reachability
> TLV
> gets propagated across level boundaries."
> 
> I'm not saying that one way is correct...either one may be.  Just be
> consistent.
> 
> [minor] I'm assuming that "IP Reachability TLV" means one of the TLVs
> above, true?  It may be worth clarifying that.
> 
> 
> 183   The Prefix-SID sub-TLV has the following format:
> 
> 185    0                   1                   2                   3
> 186    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
> 187   +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
> 188   |   Type        |     Length    |     Flags     |   Algorithm   |
> 189   +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
> 190   |                        SID/Index/Label (variable)             |
> 191   +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
> 
> 193   where:
> 
> 195      Type: TBD, suggested value 3
> 
> [major] This value (and all the others in the document) are not "suggested
> values", they have been already allocated by IANA.  Please update the
> individual descriptions and the IANA Considerations section accordingly.
> 
> 
> 197      Length: variable.
> 
> [minor] Yes, variable, but it can only have two values: 5 or 6.
> 
> 
> 199      Flags: 1 octet field of following flags:
> 
> 201    0 1 2 3 4 5 6 7
> 202   +-+-+-+-+-+-+-+-+
> 203   |R|N|P|E|V|L|   |
> 204   +-+-+-+-+-+-+-+-+
> 
> 206      where:
> 
> 208         R-Flag: Re-advertisement flag.  If set, then the prefix to
> 209         which this Prefix-SID is attached, has been propagated by the
> 210         router either from another level (i.e., from level-1 to level-2
> 211         or the opposite) or from redistribution (e.g.: from another
> 212         protocol).
> 
> [major] Should it be used to avoid looping as well?  Or is there no risk of
> that in this case?
> 
> 
> ...
> 236         Other bits: MUST be zero when originated and ignored when
> 237         received.
> 
> [major] Do you expect IANA to handle the assignment of those other bits?
> 
> 
> 239      Algorithm: the router may use various algorithms when calculating
> 240      reachability to other nodes or to prefixes attached to these
> 241      nodes.  Algorithms identifiers are defined in Section 3.2.
> 242      Examples of these algorithms are metric based Shortest Path First
> 243      (SPF), various sorts of Constrained SPF, etc.  The algorithm field
> 244      of the Prefix-SID contains the identifier of the algorithm the
> 245      router has used in order to compute the reachability of the prefix
> 246      to which the Prefix-SID is associated.
> 
> 248      At origination, the Prefix-SID algorithm field MUST be set to 0 or
> 249      to any value advertised in the SR-Algorithm sub-TLV (Section 3.2).
> 
> [major] What should a receiver do if the Algorithm value doesn't match what
> has been advertised in the SR-Algorithm Sub-TLV?
> 
> 
> ...
> 282 2.1.1.2.  R and N Flags
> 
> 284   The R-Flag MUST be set for prefixes that are not local to the router
> 285   and either:
> 
> 287      advertised because of propagation (Level-1 into Level-2);
> 
> 289      advertised because of leaking (Level-2 into Level-1);
> 
> 291      advertised because of redistribution (e.g.: from another
> 292      protocol).
> 
> 294   In the case where a Level-1-2 router has local interface addresses
> 295   configured in one level, it may also propagate these addresses into
> 296   the other level.  In such case, the Level-1-2 router MUST NOT set the
> 297   R bit.  The R-bit MUST be set only for prefixes that are not local to
> 298   the router and advertised by the router because of propagation and/or
> 299   leaking.
> 
> [minor] This last sentence is redundant with the start of this section.
> Please specify the behavior just once.
> 
> 
> 301   The N-Flag is used in order to define a Node-SID.  A router MAY set
> 302   the N-Flag only if all of the following conditions are met:
> 
> 304      The prefix to which the Prefix-SID is attached is local to the
> 305      router (i.e., the prefix is configured on one of the local
> 306      interfaces, e.g., a 'stable transport' loopback).
> 
> 308      The prefix to which the Prefix-SID is attached MUST have a Prefix
> 309      length of either /32 (IPv4) or /128 (IPv6).
> 
> [major] "The prefix...MUST have a Prefix length..."  This condition is not
> worded as one; perhaps: "The prefix...has a Prefix length..."
> 
> [minor] Why is the N-Flag needed?  If the setting is optional ("MAY"), then
> not setting it doesn't imply that the conditions are not met...
> 
> [major] It is clearly an error if both the R-Flag and N-Flag are set,
> right?  What should the receiver do in that case?
> 
> 
> 311   The router MUST ignore the N-Flag on a received Prefix-SID if the
> 312   prefix has a Prefix length different than /32 (IPv4) or /128 (IPv6).
> 
> 314   [RFC7794] also defines the N and R flags and with the same semantics
> 315   of the equivalent flags defined in this document.  There will be a
> 316   transition period where both sets of flags will be used and
> 317   eventually only the flags of the Prefix Attributes will remain.
> 318   During the transition period implementations supporting the N and R
> 319   flags defined in this document and the N and R flags defined in
> 320   [RFC7794] MUST advertise and parse all flags.  In case the received
> 321   flags have different values, the value of the flags defined in
> 322   [RFC7794] prevails.
> 
> [major] "...transition period where both sets of flags will be used and
> eventually only the flags of the Prefix Attributes will remain"  To be
> honest, you lost me here?  Please explain what will the transition period
> be between?
> 
> [major] If "both sets of flags will be used and eventually only the
> [already defined] flags of the Prefix Attributes will remain", why do we
> need the new ones?
> 
> [major] "MUST advertise and parse all flags"  What does this mean?  By "all
> flags" I guess you mean the N and R flags...but if both this document and
> rfc7794 are implemented, it seems to me that the specification above is
> really meaningless.
> 
> [major] "In case the received flags have different values..."  For a second
> it seemed to me that what you really wanted was for the flags in this
> document to behave the same way as the flags defined in rfc7794...the easy
> solution to that (and to avoid my questions listed before rfc7794 was
> mentioned) would have been to just point at rfc7794 as the place where the
> Flags are defined.  However, it looks like the definitions (at least for
> the R-Flag) are not the same.  Is that on purpose or is it just an
> oversight?
> 
> One piece of information that might make this discussion relevant is
> understanding the significance of these Flags...but neither document offers
> any specific use, beyond simply "it may be useful for other routers to
> know" (rfc7794, §2.2).
> 
> 
> 324 2.1.1.3.  E and P Flags
> 
> 326   When calculating the outgoing label for the prefix, the router MUST
> 327   take into account E and P flags advertised by the next-hop router, if
> 328   next-hop router advertised the SID for the prefix.  This MUST be done
> 329   regardless of next-hop router contributing to the best path to the
> 330   prefix or not.
> 
> [major] "the router MUST take into account"  What normative action is
> standardized with that phrase?  It would be clearer/better if the behavior
> is explicitly specified in function of the flags (as sone below).
>  s/MUST/must
> 
> [major] "This MUST be done regardless of next-hop router contributing to
> the best path to the prefix or not."  This what?  I'm assuming the text
> refers to "take into account"...  Same comment as before: s/MUST/must
> 
> 
> ...
> 366 2.1.2.  Prefix-SID Propagation
> 
> 368   The Prefix-SID sub-TLV MUST be preserved when the IP Reachability
> TLV
> 369   gets propagated across level boundaries.
> 
> [major] §2.1 says that it SHOULD.  Please be consistent...and, specify
> behavior only once.
> 
> 371   The level-1-2 router that propagates the Prefix-SID sub-TLV between
> 372   levels MUST set the R-flag.
> 
> 374   If the Prefix-SID contains a global index (L and V flags unset) and
> 375   it is propagated as such (with L and V flags unset), the value of the
> 376   index MUST be preserved when propagated between levels.
> 
> 378   The level-1-2 router that propagates the Prefix-SID sub-TLV between
> 379   levels MAY change the setting of the L and V flags in case a local
> 380   label value is encoded in the Prefix-SID instead of the received
> 381   value.
> 
> [major]  Up to now, I had been assuming that propagating and preserving
> meant not changing the sub-TLV (except for the R-Flag).  But these last
> paragraphs indicate that not only can some flags be changed, but the
> contents can completely change.  It would be ideal if the valid operations
> on the sub-TLV were discussed (or at least referenced) when the preserving
> behavior was first introduced (I think in §2.1).
> 
> 
> ...
> 409 2.2.1.  Adjacency Segment Identifier (Adj-SID) Sub-TLV
> 
> 411   The following format is defined for the Adj-SID sub-TLV:
> 
> 413    0                   1                   2                   3
> 414    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
> 415   +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
> 416   |   Type        |     Length    |     Flags     |     Weight    |
> 417   +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
> 418   |                         SID/Label/Index (variable)            |
> 419   +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
> 
> 421   where:
> 
> 423      Type: TBD, suggested value 31
> 
> 425      Length: variable.
> 
> [minor] Yes, variable, but it can only have two values...
> 
> 427      Flags: 1 octet field of following flags:
> 
> 429          0 1 2 3 4 5 6 7
> 430         +-+-+-+-+-+-+-+-+
> 431         |F|B|V|L|S|P|   |
> 432         +-+-+-+-+-+-+-+-+
> 
> 434      where:
> 
> 436         F-Flag: Address-Family flag.  If unset, then the Adj-SID refers
> 437         to an adjacency with outgoing IPv4 encapsulation.  If set then
> 438         the Adj-SID refers to an adjacency with outgoing IPv6
> 439         encapsulation.
> 
> [major] This document describes the operation on the MPLS data plane,
> right?  The F-Flag talks about the outgoing encapsulation...but that could
> simply be the next label in the MPLS stack, right?  What am I missing?  I
> get the impression that there's a relationship between what this Flag
> points to and something else...but I just don't know what.
> 
> 
> 441         B-Flag: Backup flag.  If set, the Adj-SID is eligible for
> 442         protection (e.g.: using IPFRR or MPLS-FRR) as described in
> 443         [RFC8355].
> 
> [major] Because the specification of the B-Flag depends directly on
> rfc8355, it should be a Normative reference.  Looking at rfc8355, I
> couldn't find a place where it talks about eligibility for
> protection...just text about potential strategies, but (honestly) nothing
> that I would call Normative.  Please clarify.  Perhaps rfc8402 might be a
> better reference.
> 
> [major] I'm assuming that this description (pointing at rfc8355), plus the
> text below about allocation of Adj-SIDs should result in some action
> related to the protection...what is that?  IOW, presented with the B-Flag
> set, what action should a node take?
> 
> 
> 445         V-Flag: Value flag.  If set, then the Adj-SID carries a value.
> 446         By default the flag is SET.
> 
> [minor] Is the description the same at the V-Flag in §2.1?  If so, then
> indicating that, or at least also pointing out here that the value is
> carried instead of an index would be helpful.
> 
> 
> 448         L-Flag: Local Flag.  If set, then the value/index carried by
> 449         the Adj-SID has local significance.  By default the flag is
> 450         SET.
> 
> [major] Looking back at §2.1.1.1, what are the conditions for which the
> combination of the V and L-Flags are valid/invalid?  Do the same conditions
> apply?
> 
> 
> ...
> 460         Other bits: MUST be zero when originated and ignored when
> 461         received.
> 
> [major] Do you expect IANA to handle the assignment of those other bits?
> 
> 
> ...
> 469      An SR capable router MAY allocate an Adj-SID for each of its
> 470      adjacencies and SHOULD set the B-Flag when the adjacency is
> 471      eligible for protection (IP or MPLS).
> 
> [major] If eligible for protection, when would the router not set the
> B-Flag?  IOW, why is that not a MUST?
> 
> Beyond that, I think that the use of SHOULD is in conflict with the
> definition of the B-Flag (above): "If set, the Adj-SID is eligible for
> protection...", which implies that it isn't eligible if it's not set...but
> the SHOULD opens the door for not setting the Flag...
> 
> 
> ...
> 479      When the P-flag is not set, the Adj-SID MAY be persistent.  When
> 480      the P-flag is set, the Adj-SID MUST be persistent.
> 
> [major] If the adjacency is persistent, why/when would the P-Flag not be
> set?
> 
> 
> ...
> 488 2.2.2.  Adjacency Segment Identifiers in LANs
> ...
> 537      Other bits: MUST be zero when originated and ignored when
> 538      received.
> 
> [major] Do you expect IANA to handle the assignment of those other bits?
> 
> 
> ...
> 544      System-ID: 6 octets of IS-IS System-ID of length "ID Length" as
> 545      defined in [ISO10589].
> 
> [major] Which System-ID?
> 
> [major] "6 octets of...length "ID Length""  ??
> 
> 
> 547      SID/Index/Label as defined in Section 2.1.1.1.
> 
> 549   Multiple LAN-Adj-SID sub-TLVs MAY be encoded.  Note that this sub-
> TLV
> 550   MUST NOT appear in TLV 141.
> 
> 552   When the P-flag is not set, the LAN-Adj-SID MAY be persistent.  When
> 553   the P-flag is set, the LAN-Adj-SID MUST be persistent.
> 
> 555   In case one TLV-22/23/222/223 (reporting the adjacency to the DIS)
> 556   can't contain the whole set of LAN-Adj-SID sub-TLVs, multiple
> 557   advertisements of the adjacency to the DIS MUST be used and all
> 558   advertisements MUST have the same metric.
> 
> 560   Each router within the level, by receiving the DIS PN LSP as well as
> 561   the non-PN LSP of each router in the LAN, is capable of
> 562   reconstructing the LAN topology as well as the set of Adj-SID each
> 563   router uses for each of its neighbors.
> 
> 565   A label is encoded in 3 octets (in the 20 rightmost bits).
> 
> 567   An index is encoded in 4 octets.
> 
> [minor] Some of the information above (for example, the paragraph about
> the
> P-Flag and these last 2 sentences) seems unnecessary given that the
> description of these fields is already done somewhere else.
> 
> 
> 569 2.3.  SID/Label Sub-TLV
> 
> 571   The SID/Label sub-TLV may be present in the following TLVs/sub-TLVs
> 572   defined in this document:
> 
> 574   SR-Capabilities Sub-TLV (Section 3.1)
> 
> 576   SR Local Block Sub-TLV (Section 3.3)
> 
> 578   SID/Label Binding TLV (Section 2.4)
> 
> 580   Multi-Topology SID/Label Binding TLV (Section 2.5)
> 
> [nit] It might have been nice, in line with the OSPF work, to flip Sections
> 2 and 3.  Just a nit...
> 
> 582   Note that the code point used in all of the above cases is the SID/
> 583   Label Sub-TLV code point assigned by IANA in the "sub-TLVs for TLV
> 584   149 and 150" registry.
> 
> [major] Because this document is setting up that new registry, we don't
> have to wait for IANA to assign anything: the document can specify it.
> IOW, the paragraph above is not needed.
> 
> 
> 586   The SID/Label sub-TLV contains a SID or a MPLS Label.  The SID/Label
> 587   sub-TLV has the following format:
> 
> 589    0                   1                   2                   3
> 590    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
> 591   +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
> 592   |   Type        |     Length    |
> 593   +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
> 594   |                          SID/Label (variable)                 |
> 595   +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
> 
> 597   where:
> 
> 599      Type: TBD, suggested value 1
> 
> 601      Length: variable
> 
> [major] Yes, variable...but right now only one value is defined.  There's a
> finite number of possibilities...
> 
> 
> 602      SID/Label: if length is set to 3 then the 20 rightmost bits
> 603      represent a MPLS label.
> 
> [major] Is that it..?  What about the SID?
> 
> 
> 605 2.4.  SID/Label Binding TLV
> ...
> 619   The SID/Label Binding TLV has Type TBD (suggested value 149), and has
> 620   the following format:
> 
> [nit] No need to talk about the Type twice.
> 
> 622      0                   1                   2                   3
> 623      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
> 624     +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
> 625     |      Type     |     Length    |     Flags     |     RESERVED  |
> 626     +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
> 627     |            Range              | Prefix Length |     Prefix    |
> 628     +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
> 629     //               Prefix (continued, variable)                  //
> 630     +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
> 631     |                    SubTLVs (variable)                         |
> 632     +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
> 
> 634                  Figure 1: SID/Label Binding TLV format
> 
> 636   o  Type: TBD, suggested value 149
> 
> 
> ...
> 657 2.4.1.  Flags
> ...
> 671      M-Flag: Mirror Context flag.  Set if the advertised SID
> 672      corresponds to a mirrored context.  The use of the M flag is
> 673      described in [RFC8402].
> 
> [major] Where?  Note that rfc8402 says that "The Mirror SID is advertised
> using the Binding segment defined in SR IGP protocol extensions
> [ISIS-SR-EXT].", pointing back at this document.
> 
> [minor] Some places use *-Flag and others "* flag", please be consistent.
> 
> 
> 675      S-Flag: If set, the SID/Label Binding TLV SHOULD be flooded across
> 676      the entire routing domain.  If the S flag is not set, the SID/
> 677      Label Binding TLV MUST NOT be leaked between levels.  This bit
> 678      MUST NOT be altered during the TLV leaking.
> 
> [major] If the S-Flag is set, when would the TLV not be flooded across the
> entire domain?  IOW, why is the SHOULD not a MUST?
> 
> 
> 680      D-Flag: when the SID/Label Binding TLV is leaked from level-2 to
> 681      level-1, the D bit MUST be set.  Otherwise, this bit MUST be
> 682      clear.  SID/Label Binding TLVs with the D bit set MUST NOT be
> 683      leaked from level-1 to level-2.  This is to prevent TLV looping
> 684      across levels.
> 
> [minor] What about leaking from L2 to L1 with the Flag set?
> 
> [minor] The text uses Flag, but in some cases bit is used as well.  Please
> be consistent.
> 
> 
> ...
> 695      An implementation MAY decide not to honor the S-flag in order not
> 696      to leak Binding TLV's between levels (for policy reasons).  In all
> 697      cases, the D flag MUST always be set by any router leaking the
> 698      Binding TLV from level-2 into level-1 and MUST be checked when
> 699      propagating the Binding TLV from level-1 into level-2.  If the D
> 700      flag is set, the Binding TLV MUST NOT be propagated into level-2.
> 
> [major] s/implementation MAY decide/implementation may decide   There's
> no
> Normative action there.
> 
> [major] Regardless of whether MAY/may is used, the statement in the first
> sentence contradicts the "MUST NOT be leaked" Normative statement.
> Given
> that there are exceptions, maybe "SHOULD NOT" would be more
> appropriate.
> 
> [major] This last paragraph seems to contain the same information as the
> text describing the S-Flag.  Please don't duplicate specifications.
> 
> 702      Other bits: MUST be zero when originated and ignored when
> 703      received.
> 
> [major] Do you expect IANA to handle the assignment of those other bits?
> 
> 705 2.4.2.  Range
> 
> 707   The 'Range' field provides the ability to specify a range of
> 708   addresses and their associated Prefix SIDs.  This advertisement
> 709   supports the SRMS functionality.  It is essentially a compression
> 710   scheme to distribute a continuous Prefix and their continuous,
> 711   corresponding SID/Label Block.  If a single SID is advertised then
> 712   the range field MUST be set to one.  For range advertisements > 1,
> 713   the number of addresses that need to be mapped into a Prefix-SID and
> 714   the starting value of the Prefix-SID range.
> 
> [nit] The last sentence sounds incomplete...
> 
> 716   Example 1: if the following router addresses (loopback addresses)
> 717   need to be mapped into the corresponding Prefix SID indexes.
> 
> [minor] Suggestion: move the examples to the end of §2.4...after all the
> fields have been discussed.
> 
> 719   Router-A: 192.0.2.1/32, Prefix-SID: Index 1
> 720   Router-B: 192.0.2.2/32, Prefix-SID: Index 2
> 721   Router-C: 192.0.2.3/32, Prefix-SID: Index 3
> 722   Router-D: 192.0.2.4/32, Prefix-SID: Index 4
> 
> 724      0                   1                   2                   3
> 725      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
> 726     +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
> 727     |      Type     |     Length    |0|0|           |     RESERVED  |
> 728     +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
> 729     |            Range = 4          |       /32     |      192      |
> 730     +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
> 731     |       .0      |        .2     |       .1      |Prefix-SID Type|
> 732     +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
> 733     | sub-TLV Length|     Flags     |   Algorithm   |               |
> 734     +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
> 735     |                                             1 |
> 736     +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
> 
> [nit] The Prefix Length should be "32", not "/32".
> 
> [nit] The Prefix itself is one field (in the description), but it looks
> like 4 separate fields above...and the numbers should not have "." in them.
> 
> [minor] It looks like the fields starting with "Prefix-SID Type" belong to
> the Prefix-SID Sub-TLV, but that is not explained anywhere (yet).  The
> Algorithm field is not set...nor are the Flags.
> 
> 738   Example-2: If the following prefixes need to be mapped into the
> 739   corresponding Prefix-SID indexes:
> 
> 741   10.1.1/24, Prefix-SID: Index 51
> 742   10.1.2/24, Prefix-SID: Index 52
> 743   10.1.3/24, Prefix-SID: Index 53
> 744   10.1.4/24, Prefix-SID: Index 54
> 745   10.1.5/24, Prefix-SID: Index 55
> 746   10.1.6/24, Prefix-SID: Index 56
> 747   10.1.7/24, Prefix-SID: Index 57
> 
> [major] Please use addresses in the rfc5737 reserved ranges.
> 
> [minor] It would have been nice to see an IPv6 example (see rfc3849).
> 
> 749      0                   1                   2                   3
> 750      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
> 751     +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
> 752     |      Type     |     Length    |0|0|           |     RESERVED  |
> 753     +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
> 754     |            Range = 7          |       /24     |      10       |
> 755     +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
> 756     |       .1      |        .1     |Prefix-SID Type| sub-TLV Length|
> 757     +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
> 758     |    Flags      | Algorithm     |                               |
> 759     +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
> 760     |                           51  |
> 761     +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
> 
> [] Same comments as above.
> 
> 
> ...
> 778 2.4.3.  Prefix Length, Prefix
> ...
> 784   The 'Prefix Length' field contains the length of the prefix in bits.
> 785   Only the most significant octets of the Prefix are encoded.  (i.e., 1
> 786   octet for prefix length 1 up to 8, 2 octets for prefix length 9 to
> 787   16, 3 octets for prefix length 17 up to 24 and 4 octets for prefix
> 788   length 25 up to 32, ...., 16 octets for prefix length 113 up to 128).
> 
> [nit] s/encoded.  (i.e.,/encoded (i.e.,
> 
> 
> 790 2.4.4.  Mapping Server Prefix-SID
> 
> 792   The Prefix-SID sub-TLV (suggested value 3) is defined in Section 2.1
> 793   and contains the SID/index/label value associated with the prefix and
> 794   range.  The Prefix-SID SubTLV MUST be present in the SID/Label
> 795   Binding TLV unless the M-flag is set in the Flags field of the parent
> 796   TLV.
> 
> [major] Does it then mean that the Prefix-SID SubTLV must not be present if
> the M-Flag is not set?
> 
> [nit] s/SubTLV/Sub-TLV (as it is used almost everywhere else)
> 
> 
> 798   A node receiving an SRMS entry for a prefix MUST check the existence
> 799   of such prefix in its link-state database prior to consider and use
> 800   the associated SID.
> 
> [major] What does it mean for the prefix to exist in the link-state
> database?  Does it mean that it must have been advertised in a "normal"
> routing TLV, or something else?  I ask not only to clarify, but because
> §2.4.3 says that the "'Prefix' does not need to correspond to a routable
> prefix of the originating node".
> 
> [Ah...I think I understand.  Because "the mapping server does not specify
> the originator of a prefix" (§2.4.4.2), then the prefix is obviously not
> routable in the context of the originating node (the server)...but it
> should be routable in general.  Is that it?]
> 
> 
> 802 2.4.4.1.  Prefix-SID Flags
> 
> 804   The Prefix-SID flags are defined in Section 2.1.  The Mapping Server
> 805   MAY advertise a mapping with the N flag set when the prefix being
> 806   mapped is known in the link-state topology with a mask length of 32
> 807   (IPv4) or 128 (IPv6) and when the prefix represents a node.  The
> 808   mechanisms through which the operator defines that a prefix
> 809   represents a node are outside the scope of this document (typically
> 810   it will be through configuration).
> 
> [major] §2.1.1.2 says that the "router MAY set the N-Flag only if... the
> prefix to which the Prefix-SID is attached MUST have a Prefix length of
> either /32 (IPv4) or /128 (IPv6)."  That is similar to what the text above
> says (but the text talks about "the prefix...known in the link-state
> topology"), and it is related to the last question in the previous
> section.  Does the prefix being in the link-state database/topology mean
> that the match is exact (to the prefix advertised), or is a supernet ok?
> 
> 
> 812   The other flags defined in Section 2.1 are not used by the Mapping
> 813   Server and MUST be ignored at reception.
> 
> [major] How does the receiver know that the sub-TLV was originated by a
> Mapping Server?  Is it the case that it would originate a Binding TLV?
> IOW, would the other flags always be ignored when the sub-TLV is included
> in the Binding TLV (but not other TLVs)?   Does this apply also to the
> Multi-Topology SID/Label Binding TLV (§2.5)?
> 
> 
> 815 2.4.4.2.  PHP Behavior when using Mapping Server Advertisements
> ...
> 823   o  A prefix reachability advertisement for the prefix has been
> 824      received which includes the Extended Reachability Attribute Flags
> 825      sub-TLV ([RFC7794]).
> 
> [nit] s/([RFC7794])/[RFC7794]
> 
> 827   o  X and R flags are both set to 0 in the Extended Reachability
> 828      Attribute Flags sub-TLV.
> 
> 830   In the absence of an Extended Reachability Attribute Flags sub-TLV
> 831   ([RFC7794]) the A flag in the binding TLV indicates that the
> 832   originator of a prefix reachability advertisement is directly
> 833   connected to the prefix and thus PHP MUST be done by the neighbors
> of
> 834   the router originating the prefix reachability advertisement.  Note
> 835   that A-flag is only valid in the original area in which the Binding
> 836   TLV is advertised.
> 
> [nit] s/([RFC7794])/[RFC7794]
> 
> [major] §2.4.1 says that "if the Binding TLV is leaked...the A-flag MUST be
> cleared", which is not the same as not considering it valid (ignoring, as
> described above).  In any case, please be specific about the functionality
> in the description.
> 
> 
> 838 2.4.4.3.  Prefix-SID Algorithm
> 
> 840   The algorithm field contains the identifier of the algorithm the
> 841   router MUST use in order to compute reachability to the range of
> 842   prefixes.  Use of the algorithm field is described in Section 2.1.
> 
> [major nit] At first glance, it looks like this section is not needed
> because it repeats what §2.1 already says about the algorithm.  But this
> paragraph refers to "the algorithm the router MUST use in order to compute
> reachability", in contrast to the definition of the field in §2.1: "The
> algorithm field...contains the identifier of the algorithm the router has
> used in order to compute the reachability..."
> 
> 
> ...
> 901 3.1.  SR-Capabilities Sub-TLV
> ...
> 909   The Router Capability TLV specifies flags that control its
> 910   advertisement.  The SR Capabilities sub-TLV MUST be propagated
> 911   throughout the level and MUST NOT be advertised across level
> 912   boundaries.  Therefore Router Capability TLV distribution flags are
> 913   set accordingly, i.e., the S flag in the Router Capability TLV
> 914   ([RFC7981]) MUST be unset.
> 
> [nit] s/([RFC7981])/[RFC7981]
> 
> 
> ...
> 943         I-Flag: MPLS IPv4 flag.  If set, then the router is capable of
> 944         processing SR MPLS encapsulated IPv4 packets on all interfaces.
> 
> 946         V-Flag: MPLS IPv6 flag.  If set, then the router is capable of
> 947         processing SR MPLS encapsulated IPv6 packets on all interfaces.
> 
> [minor] Just curious: How are these flags used?  Given that the data plane
> we're dealing with is MPLS...
> 
> 
> ...
> 981   The originating router MUST ensure the order is same after a graceful
> 982   restart (using checkpointing, non-volatile storage or any other
> 983   mechanism) in order to guarantee the same order before and after GR.
> 
> [nit] s/order is same/order is the same
> 
> 
> ...
> 1015 3.2.  SR-Algorithm Sub-TLV
> ...
> 1046   The SR-Algorithm sub-TLV is optional, it MAY only appear a single
> 1047   time inside the Router Capability TLV.
> 
> [major] "MAY only appear a single time" means that it can appear more than
> once.  If it does show up more than once, what should the the receiver do?
> 
> 
> 1049   When the originating router does not advertise the SR-Algorithm sub-
> 1050   TLV, then all the Prefix-SIDs advertised by the router MUST have
> 1051   algorithm field set to 0.  Any receiving router MUST assume SPF
> 1052   algorithm (i.e., Shortest Path First).
> 
> [minor] It seems to me that this part of the specification would fit better
> in §2.1.
> 
> [major] Besides maybe being redundant, what is specified in the last
> sentence?  If the indication of the algorithm is explicit, then I don't
> understand what "MUST assume" is trying to mandate.
> 
> 
> 1054   When the originating router does advertise the SR-Algorithm sub-TLV,
> 1055   then algorithm 0 MUST be present while algorithm 1 MAY be present.
> 
> [nit] I think that it is clear that algorithm 1 is optional.  I would take
> that last part out just because other algorithms may be defined in the
> future...and the important part at this point is the inclusion of algorithm
> 0.
> 
> 
> 1057   The SR-Algorithm sub-TLV has following format:
> 
> [nit] s/has following/has the following
> 
> 
> ...
> 1073      Algorithm: 1 octet of algorithm Section 2.1
> 
> [minor] The Algorithm field is described in this section, not in 2.1.
> 
> 
> 1075 3.3.  SR Local Block Sub-TLV
> 
> 1077   The SR Local Block (SRLB) Sub-TLV contains the range of labels the
> 1078   node has reserved for local SIDs.  Local SIDs are used, e.g., for
> 1079   Adjacency-SIDs, and may also be allocated by other components than
> 1080   IS-IS protocol.  As an example, an application or a controller may
> 1081   instruct the router to allocate a specific local SID.  Therefore, in
> 1082   order for such applications or controllers to know what are the local
> 1083   SIDs available in the router, it is required that the router
> 1084   advertises its SRLB.
> 
> [nit] s/than IS-IS protocol/than the IS-IS protocol
> 
> 
> ...
> 1122   The originating router MUST NOT advertise overlapping ranges.
> 
> [major] What should the receiver do if the ranges overlap?  I'm assuming
> the same thing as what was specified before...please be specific.
> 
> 
> 1124   It is important to note that each time a SID from the SRLB is
> 1125   allocated, it SHOULD also be reported to all components (e.g.:
> 1126   controller or applications) in order for these components to have an
> 1127   up-to-date view of the current SRLB allocation and in order to avoid
> 1128   collision between allocation instructions.
> 
> 1130   Within the context of IS-IS, the reporting of local SIDs is done
> 1131   through IS-IS Sub-TLVs such as the Adjacency-SID.  However, the
> 1132   reporting of allocated local SIDs may also be done through other
> 1133   means and protocols which mechanisms are outside the scope of this
> 1134   document.
> 
> [major] "SHOULD also be reported to all components"  Because that is
> outside the scope, then it shouldn't be a Normative statement.
>  s/SHOULD/should
> 
> 
> ...
> 1170 4.  Non backward compatible changes with prior versions of this
> document
> 
> [major] I find this section unnecessary.  If you want to keep it, please
> move it to an Appendix and don't use Normative language.
> 
> 
> ...
> 1190 5.  IANA Considerations
> 
> 1192   This documents request allocation for the following TLVs and subTLVs