Re: [spring] Benjamin Kaduk's Discuss on draft-ietf-spring-segment-routing-policy-17: (with DISCUSS and COMMENT)

Benjamin Kaduk <kaduk@mit.edu> Sat, 19 March 2022 19:24 UTC

Return-Path: <kaduk@mit.edu>
X-Original-To: spring@ietfa.amsl.com
Delivered-To: spring@ietfa.amsl.com
Received: from localhost (localhost [127.0.0.1]) by ietfa.amsl.com (Postfix) with ESMTP id 40EEA3A0EAD; Sat, 19 Mar 2022 12:24:37 -0700 (PDT)
X-Virus-Scanned: amavisd-new at amsl.com
X-Spam-Flag: NO
X-Spam-Score: -1.907
X-Spam-Level:
X-Spam-Status: No, score=-1.907 tagged_above=-999 required=5 tests=[BAYES_00=-1.9, GB_SUMOF=5, RCVD_IN_DNSWL_HI=-5, SPF_HELO_NONE=0.001, SPF_NONE=0.001, T_SCC_BODY_TEXT_LINE=-0.01, URIBL_BLOCKED=0.001] autolearn=ham autolearn_force=no
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 rnjKJpXVJCmR; Sat, 19 Mar 2022 12:24:31 -0700 (PDT)
Received: from outgoing.mit.edu (outgoing-auth-1.mit.edu [18.9.28.11]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ietfa.amsl.com (Postfix) with ESMTPS id 57BED3A0E71; Sat, 19 Mar 2022 12:24:28 -0700 (PDT)
Received: from mit.edu (c-73-169-244-254.hsd1.wa.comcast.net [73.169.244.254]) (authenticated bits=56) (User authenticated as kaduk@ATHENA.MIT.EDU) by outgoing.mit.edu (8.14.7/8.12.4) with ESMTP id 22JJOGrq015236 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT); Sat, 19 Mar 2022 15:24:22 -0400
Date: Sat, 19 Mar 2022 12:24:16 -0700
From: Benjamin Kaduk <kaduk@mit.edu>
To: Ketan Talaulikar <ketant.ietf@gmail.com>
Cc: The IESG <iesg@ietf.org>, draft-ietf-spring-segment-routing-policy@ietf.org, spring-chairs@ietf.org, SPRING WG <spring@ietf.org>, james.n.guichard@futurewei.com
Message-ID: <20220319192416.GH13021@mit.edu>
References: <164507858486.11948.7818447548279924153@ietfa.amsl.com> <CAH6gdPzPVhzg81okeQxFapR-ckrzQPEU64603O9rCPg=RnLMFw@mail.gmail.com> <20220225020227.GM12881@kduck.mit.edu> <CAH6gdPxTbZGZ0weFL17VyMAaHZFAvAF=LxvHLyCODvQKHDEM1Q@mail.gmail.com>
MIME-Version: 1.0
Content-Type: text/plain; charset="iso-8859-1"
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: <CAH6gdPxTbZGZ0weFL17VyMAaHZFAvAF=LxvHLyCODvQKHDEM1Q@mail.gmail.com>
Archived-At: <https://mailarchive.ietf.org/arch/msg/spring/DCcjd5AJzOom759XoQsbuPvdR7w>
Subject: Re: [spring] Benjamin Kaduk's Discuss on draft-ietf-spring-segment-routing-policy-17: (with DISCUSS and COMMENT)
X-BeenThere: spring@ietf.org
X-Mailman-Version: 2.1.29
Precedence: list
List-Id: "Source Packet Routing in NetworkinG \(SPRING\)" <spring.ietf.org>
List-Unsubscribe: <https://www.ietf.org/mailman/options/spring>, <mailto:spring-request@ietf.org?subject=unsubscribe>
List-Archive: <https://mailarchive.ietf.org/arch/browse/spring/>
List-Post: <mailto:spring@ietf.org>
List-Help: <mailto:spring-request@ietf.org?subject=help>
List-Subscribe: <https://www.ietf.org/mailman/listinfo/spring>, <mailto:spring-request@ietf.org?subject=subscribe>
X-List-Received-Date: Sat, 19 Mar 2022 19:24:38 -0000

Hi Ketan,

My apologies for the slow reply, there are quite a few things for me to
wrap up before my term as AD ends.  As you put it in your graceful note
off-list, we are very close.

More inline...

On Sat, Mar 05, 2022 at 04:06:53PM +0530, Ketan Talaulikar wrote:
> Hi Ben,
> 
> Thanks for your response and please check inline below with KT2
> 
> We've also just posted another update to address some of your comments and
> those from other ADs.
> 
> https://datatracker.ietf.org/doc/html/draft-ietf-spring-segment-routing-policy-19
> 
> On Fri, Feb 25, 2022 at 7:32 AM Benjamin Kaduk <kaduk@mit.edu> wrote:
> 
> > Hi Ketan,
> >
> > Thanks for the replies here and the updates in the -18.
> > I think there are still some open topics, though; more inline.
> >
> > On Thu, Feb 17, 2022 at 09:21:04PM +0530, Ketan Talaulikar wrote:
> > > Hi Ben,
> > >
> > > Thanks for your detailed review and your comments/inputs. Please check
> > > inline for responses.
> > >
> > >
> > > On Thu, Feb 17, 2022 at 11:46 AM Benjamin Kaduk via Datatracker <
> > > noreply@ietf.org> wrote:
> > >
> > > > Benjamin Kaduk has entered the following ballot position for
> > > > draft-ietf-spring-segment-routing-policy-17: 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/blog/handling-iesg-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-spring-segment-routing-policy/
> > > >
> > > >
> > > >
> > > > ----------------------------------------------------------------------
> > > > DISCUSS:
> > > > ----------------------------------------------------------------------
> > > >
> > > > (1) I may just be misunderstanding things, but I'd like to pull on a
> > thread
> > > > in §8.4 a bit more.  We say that the headend H learns a BGP route that
> > has
> > > > a
> > > > VPN label V, but then the following procedures seem to say that we
> > install
> > > > a
> > > > route on the appropriate SR Policy P and that when we receive a packet
> > that
> > > > matches the route in question, push a label stack including the VPN
> > label,
> > > > and send the resulting packet out.
> > >
> > >
> > > KT> Note that we are sending the packet to the selected BGP NH (i.e.
> > egress
> > > PE) that advertised the route. The SR Policy is enabling the packets to
> > > traverse a path that is different from (perhaps) the best-effort IGP
> > > routing.
> > >
> > >
> > > > Nowhere do we say to check the VPN
> > > > status of the incoming packet,
> > >
> > >
> > > KT> That is the ingress part of the forwarding entry which maps the
> > > incoming traffic over a customer interface to their specific VPN context
> > > and then performs a lookup in their VPN specific table. This all is
> > > unchanged.
> > >
> > >
> > > > so this seems like it would open a hole in
> > > > the VPN by allowing "arbitrary" incoming traffic (not marked as
> > specific to
> > > > V) to enter that VPN.  Is the label V filling some other role than
> > > > identifying a specific VPN of many VPNs that could run along the route
> > R/r?
> > > > (This is the only instance of the phrase "VPN label" in the document,
> > and
> > > > no
> > > > reference is given, so I'm relying heavily on instinct to ascertain the
> > > > intent here.)
> > > >
> > >
> > > KT> I hope my responses clarified that the only thing that is changed
> > here
> > > by the steering over the SR Policy is the path taken through the network
> > to
> > > get to the egress PE. Rest is as today. And yes, of course, we have the
> > > ability to indicate the need for such steering via the matching Color
> > > Extended community to the BGP route.
> >
> > Yes, these help clarify that the part we're focusing on in this document is
> > conceptually "after" the determination of the incoming VPN, and so there
> > isn't any new processing needed for it.  Thanks for the explanation.
> 
> 
> > >
> > > >
> > > > (2) The security considerations says that this document does not
> > define any
> > > > new protocol extensions and (accordingly) does not introduce any
> > further
> > > > security considerations.  The first part of this seems false, not least
> > > > since we define the meaning of the "CO" bits in the Color Extended
> > > > Community.  I'm pretty sure that makes the second part also false, and
> > we
> > > > need to discuss the security considerations relating to imposing SR
> > > > Policies
> > > > based only on color and not next-hop.  Alvaro has also noted additional
> > > > aspects where security considerations are missing.
> > > >
> > >
> > > KT> Ack. We will add text in the security consideration sections for the
> > CO
> > > steering modes.
> >
> > What about the SR-DB concept and other new concepts that Alvaro identified?
> > Aren't there security and manageability considerations there as well?
> >
> 
> KT2> We've just added text for the endpoint uniqueness and steering
> aspects. The SRDB is internal to the computation node and the information
> within it is derived from existing routing protocols and their extensions -
> we are not adding anything new to it. Do let me know, however, if you feel
> we are missing something.

I see that Alvaro has cleared his Discuss, so I am willing to consider this
topic closed.  I did not think very hard about whether there is anything
missing, so accordingly I don't have anything to propose adding.

> 
> > >
> > > >
> > > > (3) The Discriminator as defined in §2.5 does not seem wide enough to
> > be
> > > > able to provide the needed properties.  Some later clarification in
> > §2.6
> > > > implies that the definition in §2.5 is incomplete and the width is
> > actually
> > > > appropriate, but in either case §2.5 seems inadequate in its current
> > form.
> > > > (Details in the COMMENT.)
> > > >
> > >
> > > KT> 32 bit is wide enough and please see further response below.
> >
> > I think I'm still failing to understand exactly why; more below.
> >
> > > >
> > > > (4) Section 2.11 contains the statement, "A valid SR Policy is
> > instantiated
> > > > in the forwarding plane."
> > > >
> > > > Is this a statement of fact (i.e., a consequence of the definition of
> > > > "valid") or a mandate for something (e.g., the headend) to take action
> > to
> > > > make it so?  Given that the point of SR is to be stateless on nodes
> > other
> > > > than the headend, I suspect the former, but if we are relying on the
> > > > headend
> > > > (or some other entity) to take action to ensure this is the case, that
> > > > needs
> > > > to be a clearly stated normative requirement.
> > > >
> > >
> > > KT> The validity of a candidate path and as an extension, the SR Policy
> > is
> > > discussed in Sec 5. Sec 2.11 describes how a valid SR Policy and its
> > > constructs are instantiated in the forwarding plane.
> >
> > Would it make sense to say something like "In order to be considered valid,
> > an SR policy needs to be instantiated in the forwarding plane (Section 5)"?
> >
> 
> KT2> The SR Policy may be valid, but could not be instantiated into the
> forwarding due to a resource issue. We are simply stating here that
> generally only a valid SR Policy is instantiated in the forwarding plane.
> We don't have the "only" here since we have use-cases as in section 8.2
> where we do have to instantiate a "drop" entry in the forwarding for an
> invalid SR Policy.

Thanks for helping me understand the scenarios here.  A few more
alternative proposals:
"A valid SR policy is instantiated in the forwarding plane by the headend."
"A valid SR policy is generally instantiated in the forwarding plane."
"Generally, only valid SR policies are instantiated in the forwarding plane."

Feel free to use any of those, a modified version thereof, or leave it
unchanged (though I would prefer to make some form of clarification, I do
not plan to ask my successor to take up this as a Discuss point when my
term ends in a few days).

> 
> >
> > >
> > > >
> > > > (5) Section 8.4 uses the phrase "any AFI/SAFI of LISP [RFC6830]."
> > > > There's nothing in the IANA registry for SAFI
> > > > (https://www.iana.org/assignments/safi-namespace/safi-namespace.xhtml)
> > > > about
> > > > LISP, and RFC 6830 doesn't talk about SAFI.  What is this referring to?
> > > >
> > >
> > > KT> Ack - there is no SAFI in LISP and the reference was meant to be for
> > > routing of both IPv4 and IPv6 packets with LISP. Will fix this.
> >
> > The new text is still a bit terse/opaque for me to be confident that I
> > understand properly, but I will drop the discuss point as I think I see how
> > it works.
> 
> 
> > >
> > > >
> > > >
> > > > ----------------------------------------------------------------------
> > > > COMMENT:
> > > > ----------------------------------------------------------------------
> > > >
> > > > There's a lot of this document that feels like just some informational
> > > > discussion of "here are some things that many people do", "here are
> > some
> > > > possible things you can do with SR", etc..  There are also a small
> > handful
> > > > of places in the document that look to actually be specifying parts of
> > > > protocol behavior (I suspect that John has already identified them in
> > his
> > > > enumeration), and the overall impression ends up being a bit jumbled,
> > > > like there are a bunch of topics stuck together without an overarching
> > > > theme.
> > > > I think the overall content would be more valuable if divided into a
> > tight
> > > > "protocol specification" portion that could stay at proposed standard,
> > plus
> > > > an informational "architecture details" document that contains the
> > > > in-depth exposition that didn't make it into 8402.
> > > >
> > > > This draft would benefit greatly from a terminology section.  I note
> > in the
> > > > section-by-section comments several places where a term is first used
> > > > without sufficient background/definition, leaving the matter at hand
> > > > underspecified for the reader.
> > > >
> > > > Section 2
> > > >
> > > >    An SR Policy is a framework that enables the instantiation of an
> > > >    ordered list of segments on a node for implementing a source routing
> > > >
> > > > Really, an SR policy is a *framework*?  I thought an SR policy was a
> > > > specific instantiation of a list of segments, or at least that's what
> > I'm
> > > > getting from RFC 8402.  Perhaps we should say that the general concept
> > of
> > > > SR
> > > > Policy provides a framework?
> > > >
> > >
> > > KT> Ack. Will rephrase.
> > >
> > >
> > > >
> > > > Section 2.1
> > > >
> > > >    An SR Policy MUST be identified through the tuple <headend, color,
> > > >    endpoint>.  In the context of a specific headend, an SR policy MUST
> > > >    be identified by the <color, endpoint> tuple.
> > > >
> > > > These two MUSTs appear to be in (nominal) conflict.  Maybe start the
> > first
> > > > one with "absent further context" or "absent the context of a known
> > headend
> > > > node"?
> > > >
> > >
> > > KT> It is not "absence of context" but "within the context of a specific
> > > headend".
> > >
> > >
> > > >
> > > >    The headend is the node where the policy is
> > instantiated/implemented.
> > > >    The headend is specified as an IPv4 or IPv6 address and is expected
> > > >    to be unique in the domain.
> > > >
> > > > This is the first instance of the word "domain" in this document.  I
> > > > suggest
> > > > using the introduction to introduce what is meant by the word, even if
> > just
> > > > by reference to RFC 8402.
> > > >
> > >
> > > KT> Ack. It should be SR Domain.
> > >
> > >
> > > >
> > > >    An implementation MAY allow the assignment of a symbolic name
> > > >    comprising printable ASCII [RFC0020] characters (i.e.  0x20 to 0x7E)
> > > >    to an SR Policy to serve as a user-friendly attribute for debugging
> > > >    and troubleshooting purposes.  [...]
> > > >
> > > > I agree with the other ADs that limiting to US-ASCII is not actually
> > > > user-friendly for many users, and that the likelihood of some
> > > > implementations not properly enforcing such a limitation to be high.
> > > > (Likewise for the other places where symbolic names are admitted.)
> > > >
> > >
> > > KT> Please see the response to the other reviews on this point.
> >
> > I don't find them compelling, but this is just the COMMENT section so
> > you're not obligated to persuade me.
> >
> > >
> > > >
> > > > Section 2.2
> > > >
> > > >    A dynamic candidate path expresses an optimization objective and a
> > > >    set of constraints.  [...]
> > > >
> > > > Down in §5.2 when we discuss validation procedures for dynamic
> > candidate
> > > > paths, we say that the optimization problem is solved "for either the
> > > > SR-MPLS or the SRv6 data-plane as specified".  Does the data plane
> > need to
> > > > be specified as part of the dynamic candidate path itself?
> > > >
> > >
> > > KT> Yes.
> >
> > Should we say that here, e.g., "a dynamic candidate path expresses an
> > optimization objective and a set of constraints within a specified data
> > plane"?
> >
> 
> KT2> Ack. Have clarified this in the text.
> 
> 
> >
> > >
> > > >
> > > > Section 2.3
> > > >
> > > >    in Section 2.9.  The table below specifies the RECOMMENDED default
> > > >    values of Protocol-Origin:
> > > >
> > > > I feel like it would be useful to provide some justification for why
> > the
> > > > recommended default behavior prefers BGP SR configuration over PCEP,
> > even
> > > > if
> > > > that justification is just "we need to have a clear ordering and this
> > one
> > > > is
> > > > arbitrary".
> > > >
> > >
> > > KT> Ack. Will clarify that.
> > >
> > >
> > > >
> > > > Section 2.4
> > > >
> > > >    o  Node Address : represented as a 128-bit value.  IPv4 addresses
> > > >       MUST be encoded in the lowest 32 bits, and the high-order bits
> > > >       MUST be set to zero.
> > > >
> > > >    Its application in the candidate path selection is described in
> > > >    Section 2.9.
> > > >
> > > > The tie-breaker procedure for path selection described in §2.9 seems to
> > > > always prefer IPv4 originators over IPv6 ones (by virtue of preferring
> > the
> > > > smaller value).  I guess if we wanted to change that to prefer IPv6 we
> > have
> > > > the option of fc00::/7 (unique-local) or fe80::/10 (link-scoped
> > unicast)
> > > > from BCP 153, but it's a bit hard to justify either of those as
> > appropriate
> > > > on technical grounds, and since this is just a tie-breaker and the
> > > > Preference is explicitly preferred, it seems like this is probably
> > "good
> > > > enough" as-is.
> > > >
> > >
> > > KT> Ack. As clarified in a recent text update in v17, preference is the
> > key
> > > parameter really.
> > >
> > >
> > > >
> > > > Section 2.5
> > > >
> > > >    The Discriminator is a 32-bit value associated with a candidate path
> > > >    that uniquely identifies it within the context of an SR Policy from
> > a
> > > >    specific Protocol-Origin as specified below:
> > > >
> > > > What are the constraints that underlie the 32-bit requirement here?
> > > > It looks like some of the scenarios are going to involve uncoordinated
> > > > (random) assignment of these discriminator values (e.g., with the BGP
> > > > distribution mechanism, when coming from different BGP peers), and the
> > > > birthday-bound collision probability is not negligible for this few
> > bits.
> > > > That, in turn, calls into question the "uniquely identifies" property
> > being
> > > > claimed.  Or is there some other property that means that only
> > > > discriminators from a single issuer will ever need to be compared with
> > each
> > > > other (making the allocation "coordinated"), such as being additionally
> > > > associated with the originator?
> > > > If my initial analysis was incorrect and these are indeed allocated in
> > a
> > > > "coordinated" fashion, would it be typical/expected for the allocation
> > to
> > > > occur by incrementing a local counter on the originator?  In some
> > > > situations
> > > > such allocation by counter can have security considerations, which
> > > > draft-gont-numeric-ids-sec-considerations attempts to cover.
> > > >
> > >
> > > KT> The discriminator is scoped to a particular originating node for the
> > > candidate path and as such, there is no requirement for coordination
> > across
> > > sources/nodes. Therefore, 32-bit is more than sufficient.
> >
> > When you say "originating node", does that refer to the SR headend, or the
> > (BGP) originator of the BGP route containing the SR Policy NLRI?
> >
> 
> KT2> The BGP originator as you've correctly understood below.
> 
> 
> >
> > I assume the latter, and agree that *within the context of BGP*, the
> > discriminator is scoped to the originating BGP node.  But the description
> > we give in §2.5 of this document does not say anything about making use of
> > such information.  As far as I know, the BGP originator information is lost
> > when the BGP distinguisher is converted into the SR Policy candidate path
> > discriminator data model.
> 
> 
> KT2> The BGP originator information is not lost. We have clarified this in
> the text below in sec 2.5 itself:
> 
>    o  When signaling is via BGP SR Policy, the BGP process receiving the
>       route provides the distinguisher (refer to Section 2.1 of
>       [I-D.ietf-idr-segment-routing-te-policy
> <https://datatracker.ietf.org/doc/html/draft-ietf-spring-segment-routing-policy-18#ref-I-D.ietf-idr-segment-routing-te-policy>])
> as the discriminator.

I think this is trying to cover too much subtlety to be accessible to all
readers.  Thanks for the pointer to
draft-filsfils-spring-sr-policy-considerations below, that seems to help
clarify that the intent is that collisions on BGP distinguisher are
expected to be resolved within the BGP layer using BGP best-path selection,
so that only a single candidate path is received by the SR layer from BGP
for a given distinguisher/discriminator, even if the BGP agent on the SR
node receives multiple routes that could be candidate paths prior to the
BGP bestpath selection.

It seems that there are a number of ways that we could modify this document
to clarify; let me list a couple (but these are not intended to be
exhaustive):

- in this chunk of text, add another sentence like "Note that BGP best path
  selection is applied before the route is supplied as a candidate path, so
  only a single candidate path will be seen for a given discriminator."

- In the preface at the top of §2.5 before the list of per-Protocol-Origin
  discussions, add a note that "each Protocol-Origin is expected to apply
  any processing needed to ensure that only a single candidate path is
  supplied with a given Discriminiator value"

> 
> 
> > And we don't say anything about how candidate
> > paths for a given SR Policy can only be originated from a single BGP node,
> > so I have to account somehow for the possibility that two BGP nodes are
> > independently announcing candidate paths for the same SR Policy, and thus
> > might collide in their assignment of distinguisher.
> 
> 
> KT2> You are correct. This can and does indeed happen today in deployments
> for redundancy and other reasons.
> 
> 
> > Whereas BGP can
> > resolve that collision via the BP origination information, I don't see how
> > that would be done in the SR data model.  Does that help you understand
> > what part I am missing?
> >
> 
> KT2> We did have some text in this document in the early days to explain
> these scenarios but it was moved out to an individual draft. Sec 2.9 has a
> pointer to this informative draft. Please check
> https://datatracker.ietf.org/doc/html/draft-filsfils-spring-sr-policy-considerations-08#section-4
> and if they clarify.

(As indicated above, they do clarify, but that draft is not referenced here
and is not a normative reference, so I must insist on further changes to
this document.)

> 
> >
> >
> > >
> > > >
> > > > Section 2.8
> > > >
> > > >    A candidate path is usable when it is valid.  A common path validity
> > > >    criterion is the validity of any of its constituent Segment-Lists.
> > > >    The validation rules are specified in Section 5.
> > > >
> > > > This document claims to target Proposed Standard status; are we really
> > > > content to say only that this is "a common" criterion?  Even when we
> > also
> > > > go
> > > > on to flat-out state "the validation rules are specified [below]"?
> > > >
> > >
> > > KT> We will change this to introduce the word RECOMMENDED. There are
> > > deployments where an operator might need a local policy to declare the
> > > candidate path invalid when the number of valid SLs drops below a certain
> > > threshold (for b/w or load-balancing considerations).
> > >
> > >
> > > >
> > > > Section 2.9
> > > >
> > > >    The candidate path selection process operates primarily on the
> > > >    candidate path Preference.  A candidate path is selected when it is
> > > >    valid and it has the highest preference value among all the
> > candidate
> > > >    paths of the SR Policy.
> > > >
> > > > Should this be "among all the valid candidate paths"?  A path that's
> > > > invalid
> > > > is still invalid, even if it has the highest preference value.
> > > >
> > >
> > > KT> Ack - will clarify this.
> > >
> > >
> > > >
> > > >    2.  If specified by configuration, prefer the existing installed
> > > >        path.
> > > >
> > > > Does "if specified by configuration" refer to the act of applying this
> > rule
> > > > at all, or that the existing installed path was one specified by
> > > > configuration?
> > > >
> > >
> > > KT> The existing installed path. The rationale was that in some
> > deployment
> > > designs an operator may not want to disturb/churn an active and
> > > valid/working path that has been installed in the forwarding.
> > >
> > >
> > > >
> > > > Section 2.11
> > > >
> > > >    The fraction of the flows associated with a given Segment-List is w/
> > > >    Sw, where w is the weight of the Segment-List and Sw is the sum of
> > > >    the weights of the Segment-Lists of the selected path of the SR
> > > >    Policy.
> > > >
> > > > Thank you for stating this clearly!
> > > >
> > > > Section 3
> > > >
> > > >    o  TE Link Attributes (such as TE metric, Shared Risk Link Groups,
> > > >       attribute-flag, extended admin group) [RFC5305] [RFC3630].
> > > >
> > > > Is RFC 5329 applicable here as well?
> > > >
> > >
> > > KT> Yes, will add that. Thanks.
> >
> > (It looks like a second 3630 reference got added in the -18, not 5329; I'll
> > mention that in my updated ballot remarks as well.)
> >
> 
> KT2> Ooops :-( .. now fixed for real
> 
> 
> >
> > >
> > > >
> > > > Section 4
> > > >
> > > >    Type E: IPv4 Prefix with Local Interface ID:
> > > >          This type allows identification of Adjacency SID or BGP Peer
> > > >          Adjacency SID (as defined in [RFC8402]) SR-MPLS label for
> > > >          point-to-point links including IP unnumbered links.  The
> > > >          headend is required to resolve the specified IPv4 Prefix
> > > >          Address to the Node originating it and then use the Local
> > > >          Interface ID to identify the point-to-point link whose
> > > >          adjacency is being referred to.  The Local Interface ID link
> > > >          descriptor follows semantics as specified in [RFC7752].  This
> > > >
> > > > The phrase "local interface ID" does not appear in RFC 7752 (and even
> > > > "local
> > > > interface" appears just once"; please use terminology actually present
> > in
> > > > the referred-to document to clarify what is being referenced.
> > > >
> > >
> > > KT> This one is a bit complicated since RFC7752 (sec 3.2.2) in turn
> > > references RFC5307 which in turn RFC4202. We use RFC7752 since it covers
> > > and explains the use of the various link descriptors that we use for
> > > various segment types.
> > >
> > >
> > > >
> > > > Section 4.1
> > > >
> > > >    When steering unlabeled IPv6 BGP destination traffic using an SR
> > > >    policy composed of Segment-List(s) based on IPv4 SIDs, the Explicit
> > > >    Null Label Policy is processed as specified in
> > > >    [I-D.ietf-idr-segment-routing-te-policy]) Section 2.4.4.  When an
> > > >
> > > > It looks like this is §2.4.5, not 2.4.4, in the referenced document.
> > > >
> > >
> > > KT> Ack. Will fix.
> > >
> > >
> > > >
> > > > Section 5.1
> > > >
> > > >    The computation/logic that leads to the choice of the Segment-List
> > is
> > > >    external to the SR Policy headend.  The SR Policy headend does not
> > > >    compute the Segment-List.  The SR Policy headend only confirms its
> > > >    validity.
> > > >
> > > > Does the headend actually have to confirm validity?  Is it okay to just
> > > > trust the controller and blindly use what is provided?
> > > >
> > >
> > > KT> At least the first segment needs to be validated from a resolvability
> > > perspective. The subsequent segments depend on how (i.e., using what
> > > segment types) the controller has signalled the path to the headend. If
> > it
> > > is indicated by just referring to a Prefix (e.g. loopback) of a node,
> > then
> > > the headend will need to resolve and as such validate. While if specified
> > > as a label, then no resolution is required.
> >
> > Hmm.  So maybe we could say "the SR Policy headend only confirms the
> > validity of any segments that it needs to resolve as part of packet
> > processing"?  Or does that not actually convey the needed information?
> >
> 
> KT2> IMHO, the term used in the draft "path resolution to the SID" is more
> informative and cleared (at least to someone working on the programming of
> forwarding entries) than "packet processing".

Okay, I am happy to defer to your domain expertise.

> 
> >
> > >
> > > >
> > > > Section 6.2
> > > >
> > > >    When the active candidate path has a specified BSID, the SR Policy
> > > >    uses that BSID if this value (label in MPLS, IPv6 address in SRv6)
> > is
> > > >    available (i.e., not associated with any other usage: e.g. to
> > another
> > > >    MPLS client, to another SRv6 client, to another SID, to another SR
> > > >    Policy, outside the range of SRv6 Locators).
> > > >
> > > > I don't think I understand what is meant by "client" here (for "another
> > > > client").  This sentence is the only place where the word "client"
> > appears
> > > > in this document...
> > > >
> > >
> > > KT> The term is "MPLS client" or "SRv6 client". MPLS clients can be IS-IS
> > > enabled with SR-MPLS, LDP, RSVP-TE or BGP-LU that allocate label from a
> > > "label manager" within the router.
> >
> > I think this explanation actually makes me more concerned about the way
> > this is written than I previously was.  It seems to imply that we are
> > trying to describe both that there is an MPLS vs SRv6 data-plane in use and
> > that the node in question is a client of some unspecified protocol that can
> > allocate SIDs/MPLS labels, which could be as varied as an IGP or a
> > dedicated path computation protocol.  Furthermore, not all of these
> > possible protocols would intrinsically provide for arbitrary headend nodes
> > to even know if the label/SID in question has already been allocated to
> > another "client"!
> 
> 
> > Is the determination of availability to be made by the controller or by the
> > headend?
> 
> 
> KT2> This is local on the headend.
> 
> 
> > I think we should state that clearly, since (given the above
> > discussion) it seems like it is the controller that is best place to
> > actually make the determination, but the phrasing "the SR Policy uses"
> > implies (at least to me) that the determination is made on the headend,
> > since it is the headend that actually instantiates the policy.
> >
> 
> KT2> The controller can (and does in some of the deployments that I am
> aware of) keep track of the usage of the Labels in the SRGB and SRLB (refer
> RFC8402). The same goes for SRv6 SIDs from under the SRv6 Locator. In
> deployments where controllers do drive the whole SR Policy provisioning,
> they do keep track. However, this is not mandatory in general. The headend
> is taking care of the actual programming into the forwarding and doesn't
> have any option but to handle these conditions.

Okay.  I might consider coalescing "to another MPLS client, to another SRv6
client" down to "to another node in the SR domain" since it's not worth
adding enough detail to fully clarify "MPLS client" and "SRv6 client", but
it's up to you.

> 
> >
> > >
> > > >
> > > >    Optionally, instead of only checking that the BSID of the active
> > path
> > > >    is available, a headend MAY check that it is available within a
> > given
> > > >    SID range i.e., Segment Routing Local Block (SRLB) as specified in
> > > >    [RFC8402].
> > > >
> > > > Is the only allowed range to check the SRLB?  If not, I think we need
> > to
> > > > s/i.e./e.g./.
> > > >
> > >
> > > KT> Yes, SRLB is the one to check/allocate from for such usage.
> >
> > Okay.  I suspect we want s/a given/the given/ then, but am not 100% sure.
> >
> 
> KT2> Ack. Fixed.
> 
> 
> > >
> > > >
> > > >    When the specified BSID is not available (optionally is not in the
> > > >    SRLB), an alert message MUST be generated.
> > > >
> > > > This is the first time (of only two) the word "alert" appears in this
> > > > document, and there is no prior expalanation of what entity might be
> > > > receiving alerts generated by a headend.  Please clarify.
> > > >
> > >
> > > KT> Alert mechanism could be one or more of syslog, Netconf
> > notification, a
> > > telemetry mechanism, etc..
> >
> > I think the clarification is best placed in the document itself, e.g., in a
> > glossary/terminology section as I suggested in my high-level comments.
> >
> 
> KT2> We have clarified inline.
> 
> 
> >
> > >
> > > >
> > > >    Assuming that at time t the BSID of the SR Policy is B1, if at time
> > > >    t+dt a different candidate path becomes active and this new active
> > > >    path does not have a specified BSID or its BSID is specified but is
> > > >    not available (e.g. it is in use by something else), then the SR
> > > >    Policy MAY keep the previous BSID B1.
> > > >
> > > > Is there a strict bound on or other guidance for what values of dt are
> > > > allowable for this purpose?
> > >
> > >
> > > KT> None
> > >
> > >
> > > >   Is the intent that there be an atomic
> > > > transition from BSID=B1;active-path=P1 to BSID=B1;active-path=P2?
> > > >
> > >
> > > KT> There is no atomicity requirement. A switch from one active CP to
> > > another will vary depending on the cause of the switch - e.g. if it is
> > due
> > > to a failure or because a more preferred path came up.
> > >
> > >
> > > >
> > > >    The association of an SR Policy with a BSID thus MAY change over the
> > > >    life of the SR Policy (e.g., upon active path change).  Hence, the
> > > >    BSID SHOULD NOT be used as an identification of an SR Policy.
> > > >
> > > > Is there any guidance available on how long to wait with a given BSID
> > value
> > > > unused before binding it to a new SR Policy?
> > > >
> > >
> > > KT> None. These depend on the implementation and scenarios like resource
> > > availability e.g., a BSID might get re-used sooner if the system is
> > running
> > > short of labels.
> > >
> > >
> > > >
> > > > Section 6.2.3
> > > >
> > > >    An implementation MAY support the configuration of the Specified-
> > > >    BSID-only restrictive behavior on the headend for all SR Policies or
> > > >    individual SR Policies.  Further, this restrictive behavior MAY also
> > > >    be signaled on a per SR Policy basis to the headend.
> > > >
> > > > Elsewhere in the document we discuss specific potential signaling
> > > > mechanisms/protocols, but here we say nothing.  Is that vagueness
> > > > intentional?
> > > >
> > >
> > > KT> Since this isn't a protocol specification the mechanism is not
> > > described here. However, you can look at
> > >
> > https://datatracker.ietf.org/doc/html/draft-ietf-idr-segment-routing-te-policy-14#section-2.4.2
> > > and that document refers back to this specification.
> >
> > Okay, but if this document isn't a protocol specification and that's
> > grounds to not describe mechanism in it, then there are quite a few other
> > places in the document that should also not describe mechanism, if we want
> > to be applying the rule consistently.
> >
> 
> KT2> There may be very high-level references to some mechanisms in addition
> to the informative pointer to the specific protocol spec. This was done to
> improve readability.
> 
> 
> >
> > >
> > > >
> > > > Section 6.3
> > > >
> > > >    A valid SR Policy installs a BSID-keyed entry in the forwarding
> > plane
> > > >    with the action of steering the packets matching this entry to the
> > > >    selected path of the SR Policy.
> > > >
> > > > I don't think this is stated properly.  An SR Policy is the list of
> > > > segments; it isn't the entity that's installing entries in the
> > forwarding
> > > > plane.  Some other entity is installing an entry in the forwarding
> > plane to
> > > > realize the SR Policy in question, and we should make our writing
> > reflect
> > > > that.
> > > >
> > >
> > > KT> Ack. s/installs/results in the installation of
> > >
> > >
> > > >
> > > > Section 6.4
> > > >
> > > >    An implementation MAY choose to associate a Binding SID with any
> > type
> > > >    of interface (e.g. a layer 3 termination of an Optical Circuit) or a
> > > >    tunnel (e.g.  IP tunnel, GRE tunnel, IP/UDP tunnel, MPLS RSVP-TE
> > > >    tunnel, etc).  This enables the use of other non-SR enabled
> > > >
> > > > Should we have some discussion that contrasts this scenario against the
> > > > End.X behavior from RFC 8986 (for the "interface" case)?
> > > >
> > >
> > > KT> What this document says is that a BSID may be also associated to
> > direct
> > > over these other types of interfaces/tunnels. We can look at them as
> > having
> > > no other segment being imposed but just redirecting to an interface. In
> > > that sense, it is somewhat similar to End.X in SRv6 (or Adjacency SID in
> > > SR-MPLS). However, those tend to be associated with protocol
> > adjacencies. I
> > > am not sure that I've followed your point though and please do let me
> > know
> > > if I've not.
> >
> > What you write here indicates to me that you got my point.  My proposal was
> > for a sentence something like "this behavior is analogous to the End.X
> > behavior defined in [RFC8986] in that the SID is removed, no new SIDs
> > applied, and the packet is directed across a particular interface, but it
> > conceptually fits better as a Binding SID since the SID is bound to a
> > specific (logical or physical) interface and End.X is typically used with
> > protocol adjacencies rather than interfaces".  But that's just a
> > suggestion, and if you think it doesn't make sense or doesn't add much
> > value, please ignore it.
> >
> > >
> > > >
> > > > Section 7
> > > >
> > > >    The SR Policy State is maintained on the headend to represent the
> > > >    state of the policy and its candidate paths.  [...]
> > > >
> > > > I confess I don't really understand why we need to have the current,
> > > > minimal, description of SR Policy State in this document.  What would
> > be
> > > > lost if we deferred its discussion entirely until there is a more
> > > > comprehensive discussion available?
> > > >
> > >
> > > KT> We will add an informational pointer to the SR Policy YANG model.
> > What
> > > this document calls out is the requirement for reporting the operational
> > > state and provides pointers to other specs where this is being worked
> > out.
> > >
> > >
> > > >
> > > >    The SR Policy state can be reported by the headend node via BGP-LS
> > > >    [I-D.ietf-idr-te-lsp-distribution] or PCEP [RFC8231] and
> > > >    [I-D.ietf-pce-binding-label-sid].
> > > >
> > > > The functionality of draft-ietf-pce-binding-label-sid seems much more
> > > > limited than that of draft-ietf-idr-te-lsp-distribution; in
> > particular, the
> > > > former does not seem to actually report SR Policy state to the headed
> > at
> > > > all; rather, it only concerns itself with BSID association to path,
> > with no
> > > > information about "active", "not preferred", etc.
> > > >
> > >
> > > KT> The PCEP work might be spread out over different documents but we do
> > > need to cover these requirements. That is one of the objectives of having
> > > this document coordinate work across protocol WGs.
> >
> > It still feels like we're claiming something here that isn't true -- "can
> > be reported via ... PCEP and [I-D.ietf-pce-binding-label-sid]".  It seems
> > like we are really trying to say "... via extensions to PCEP [some of which
> > don't exist yet in a form that we're willing to reference]".
> >
> 
> KT2> This document, like some others in SPRING, does provide informative
> references (perhaps still in the WG doc stage) for better readability and
> clarity.
> 
> 
> > >
> > > >
> > > > Section 8.3
> > > >
> > > >    If the SR Policy P is invalid, the BSID B is not in the forwarding
> > > >    plane and hence the packet K is dropped by H.
> > > >
> > > > We literally just in the previous section talked about a scenario
> > where the
> > > > BSDI is kept in the forwarding plane (but with the action to drop, so
> > the
> > > > overall outcome is not changed from what this text describes).
> > > > Nonetheless,
> > > > it's inaccurate to state that "the BSID B is not in the forwarding
> > plane"
> > > > here.
> > > >
> > >
> > > KT> There is a difference between the drop being referred to in 8.2 and
> > > 8.3. What we have in 8.2 is like a route pointing to null where we may
> > > advertise and get packets but will drop it with normal counters
> > associated
> > > with that specific entry. While 8.3 is like we are dropping because we
> > have
> > > no route (ideally we shouldn't have got the packet) and we increment a
> > > generic lookup failed counter. The use-cases for both are different.
> >
> > I (think I) understand that the mechanisms are different and both have use
> > cases.  I just don't think that the specific combination of words used here
> > makes a true statement.  There are probably a number of ways to make a
> > slight
> > adjustment and come up with a true statement, such as starting it with a
> > caveat as in "When Drop-Upon-Invalid behavior is not in use, for an invalid
> > SR Policy P, its BSID B is not in the forwarding plane and hence the packet
> > K is dropped by H".
> >
> 
> KT2> Thanks for that suggestion. We've incorporated it.
> 
> 
> >
> > >
> > > >
> > > > Section 8.4.1
> > > >
> > > >    When a BGP route has multiple Color Extended communities each with a
> > > >    valid SR Policy, the BGP process installs the route on the SR Policy
> > > >    giving preference to the color with the highest numerical value.
> > > >
> > > > Do we want to say anything about this being an arbitrary tiebreaker
> > (rather
> > > > than an intentional preference), or is that thought to be implicitly
> > clear?
> > > >
> > >
> > > KT> It is an intentional preference.
> >
> > Peeking forward to §8.8.2, is this also referring to the BGP color rather
> > than the SR Policy color?  If so, please specifically qualify the word
> > "color" as being the BGP one.
> >
> > If not, then I strongly suggest that the introduction of color in §2.1
> > state that the numerical value of the color is used as a preference
> > mechanism in addition to indicating intent or objective (with the
> > implication or explicit statement that assignment of color values must be
> > performed in a manner that is compatible with the operator's
> > preference/policy).
> >
> 
> KT2> This document needs to refer to the "BGP color" as the "Color Extended
> Community". Thanks for catching that. This is not done in a few places and
> we've fixed that.

Many thanks for those fixes, it is a big help.


> 
> >
> > >
> > > >
> > > > Section 8.5
> > > >
> > > >    In this section, independent of the a-priori existence of any
> > > >    explicit candidate path of the SR policy (C, N), it is to be noted
> > > >    that the BGP process at headend node H triggers the instantiation of
> > > >    a dynamic candidate path for the SR policy (C, N) as soon as:
> > > >
> > > > I strongly suggest providing a more explicit framework of what the
> > > > assumptions and preconditions are for the mechanism described in this
> > > > section.  My intuition says that it's a fairly optional thing that
> > would
> > > > need to be specifically configured, but trying to wrap that sentiment
> > into
> > > > the long bullet point involving "a local policy" seems like a very
> > > > confusing
> > > > way to express the desired behavior.
> > > >
> > >
> > > KT> It is actually a matter of local policy with perhaps some template
> > and
> > > configuration to drive this. I believe this should get covered in the SR
> > > Policy YANG model at some point in time.
> > >
> > >
> > > >
> > > > Section 8.6
> > > >
> > > >    o  is configured to instantiate an array of paths to N where the
> > > >       entry 0 is the IGP path to N, color C1 is the first entry and
> > > >       Color C2 is the second entry.  The index into the array is called
> > > >       a Forwarding Class (FC).  The index can have values 0 to 7.
> > > >
> > > > Why are the only allowed values 0 to 7?  Where does this restriction
> > arise
> > > > from?  It is because of some protocol element?
> > > >
> > >
> > > KT> There is text further in the section to indicated that these
> > > ranges/values are implementation-specific. Historically, the 8 values
> > have
> > > come from the MPLS EXP bits.
> >
> > If the ranges/values are implementation-specific, then don't give a
> > specific range here stated as if it is a universal limitation!  I would
> > either drop the sentence entirely or say something like "when the index is
> > conveyed using the MPLS EXP bits, only indices 0 to 7 are usable".
> >
> 
> KT2> Ack. Have clarified.
> 
> 
> >
> > >
> > > >
> > > >    If the local configuration does not specify any explicit forwarding
> > > >    information for an entry of the array, then this entry is filled
> > with
> > > >    the same information as entry 0 (i.e. the IGP shortest path).
> > > >
> > > >    If the SR Policy mapped to an entry of the array becomes invalid,
> > > >    then this entry is filled with the same information as entry 0.
> > When
> > > >    all the array entries have the same information as entry0, the
> > > >    forwarding entry for N is updated to bypass the array and point
> > > >    directly to its outgoing interface and next-hop.
> > > >
> > > > I can't tell how much of this is supposed to be protocol specification
> > and
> > > > how much an illustrative example.  Is A(0) always the IGP shortest
> > path?
> > > > Are these protocol requirements to fall back to the IGP shortest path
> > when
> > > > an entry is otherwise unpopulated or the associated SR Policy becomes
> > > > invalid?
> > > >
> > >
> > > KT> The specifics are for illustration purposes. Most of these are policy
> > > knobs/options.
> >
> > Please add a note at the top of the section that "this section provides an
> > example of how a headend might apply per-flow steering in practice".
> >
> 
> KT2> Ack. Added.
> 
> 
> >
> > >
> > > >
> > > > Section 8.8.2
> > > >
> > > >    The steering preference is first based on the highest color value
> > and
> > > >    then CO-dependent for the color.  [...]
> > > >
> > > > This seems to contradict what I assumed earlier about the "highest
> > color"
> > > > rule being a tiebreaker, e.g., the word "preference" is used here.  Is
> > it
> > > > actually intended to be a deliberately configured priority/preference
> > > > scheme?  If so, that would seem to require some wide-ranging reworking
> > > > throughout the document.
> > > >
> > >
> > > KT> There is the color that is in the identification of an SR Policy -
> > > (color, endpoint). This does not get into any tiebreaker or selection
> > > logic. All that (sec 2.9) is about the selection of a candidate path
> > within
> > > an SR Policy. Then there is the color value signaled via the Color
> > Extended
> > > community on the BGP routes and here, we have the preference for higher
> > > color when the route is advertised with multiple colors tagged to it.
> >
> > I think you should clarify that this section refers to the BGP Color, then
> > -- previously in the document it has been the SR Policy color value and an
> > unqualified reference to "color value" seems like it refers to the concept
> > defined in this document, absent other qualifiers.
> >
> 
> KT2> Ack. Fixed references as mentioned in a previous comment.
> 
> 
> >
> > >
> > > >
> > > > Section 9.3
> > > >
> > > >    the most appropriate alternative for the active candidate path.  A
> > > >    fast re-route mechanism MAY then be used to trigger sub 50msec
> > > >    switchover from the active to the backup candidate path in the
> > > >    forwarding plane.  Mechanisms like Bidirectional Forwarding
> > Detection
> > > >    (BFD) MAY be used for fast detection of such failures.
> > > >
> > > > Why is the specific 50msec value important here?  Is there some other
> > > > requirement that imposes it?
> > > >
> > >
> > > KT> This comes from "typical" expectations from fast-reroute mechanisms.
> >
> > I'd consider '''to trigger "fast" (sub 50msec) switchover''', then, but
> > it's not very important.
> >
> > >
> > > >
> > > > Section 10
> > > >
> > > > I think we also want to mention the security considerations of several
> > more
> > > > documents, including (but not limited to)
> > > > draft-ietf-idr-segment-routing-te-policy and RFCs 8660, 8754, and 8986.
> > > >
> > >
> > > KT> Ack on the three RFCs, but convinced about the
> > > draft-ietf-idr-segment-routing-te-policy since that depends on this and
> > not
> > > the other way around.
> >
> > I think this relates to John's Discuss point and whether this document
> > specifies the protocol behavior of the two bits in the context of the
> > extension defined in draft-ietf-idr-segment-routing-te-policy.  You need to
> > understand draft-ietf-idr-segment-routing-te-policy in order to understand
> > the security considerations relating to the protocol behavior controlled by
> > those two bits, and IMO the protocol behavior specified by those two bits
> > is solely the responsibility of this document, so this document must
> > incorporate the security considerations of
> > draft-ietf-idr-segment-routing-te-policy in order to fully document the
> > security considerations of the concepts and protocol elements that this
> > document defines.
> >
> 
> KT2> I am working with John to address his comments.

Okay, the changes from -18 to -20 are looking promising, but I will let
John decide when it's done.

Thanks for all these replies and updates; I didn't respond to them
individally but they are all appreciated.

-Ben

> 
> 
> >
> > Thanks for this, and all the other parts I didn't specifically reply to.
> >
> > I will attempt to update my ballot position in the datatracker to remove
> > the parts that are fully addressed (though I will probably inadvertently
> > leave in something I shouldn't have).
> >
> > -Ben
> >
> > >
> > > >
> > > > Section 15.2
> > > >
> > > > I agree with John that draft-ietf-idr-segment-routing-te-policy must be
> > > > classified as a normative reference.
> > > >
> > > > It also seems that RFC 7752 should be classified as normative, as we
> > > > incorporate its definition for the semantics of several of the segment
> > type
> > > > descriptions.
> > > >
> > >
> > > KT> Ack
> > >
> > >
> > > >
> > > >
> > > > NITS
> > > >
> > > > Section 1
> > > >
> > > >    Segment Routing Policy (SR Policy) [RFC8402] is an ordered list of
> > > >    segments (i.e. instructions) that represent a source-routed policy.
> > > >
> > > > /Segment/A Segment/
> > > >
> > >
> > > KT> Ack
> > >
> > >
> > > >
> > > >    The headend node is said to steer a flow into a SR Policy.  The
> > > >    packets steered into an SR Policy carry an ordered list of segments
> > > >    associated with that SR Policy.  [...]
> > > >
> > > > In a certain sense this can be read as saying that the packets that
> > "carry
> > > > an ordered list of segments" are the ones prior to being steered into
> > an SR
> > > > policy, which would make this statement not true.  Perhaps we want to
> > say
> > > > "after being steered into an SR Policy, packets carry an ordered list
> > ..."?
> > > > (I also went back and forth with myself about whether "packets ...
> > carry"
> > > > implies in the payload or not.  I settled on "not" but make this note
> > just
> > > > in case I am missing an aspect of that question.)
> > > >
> > >
> > > KT> Ack. Will rephrase.
> > >
> > >
> > > >
> > > > Section 2
> > > >
> > > >    An SR Policy is a framework that enables the instantiation of an
> > > >    ordered list of segments on a node for implementing a source routing
> > > >
> > > > It's easy to read this as saying that all of the segments in the list
> > > > instantiated on the single node in question, which I assume is not the
> > > > intent.  Probably the easiest way to aid readability here is to split
> > the
> > > > sentence up into multiple smaller sentences that are easier to parse.
> > > >
> > >
> > > KT> Will rephrase.
> > >
> > >
> > > >
> > > > Section 2.2
> > > >
> > > >    A dynamic candidate path expresses an optimization objective and a
> > > >    set of constraints.  The headend (potentially with the help of a
> > PCE)
> > > >    computes the solution Segment-List (or set of Segment-Lists) that
> > > >    solves the optimization problem.
> > > >
> > > > I'd suggest computes the solution/computes a solution/ for genericity.
> > > >
> > >
> > > KT> Ack.
> > >
> > >
> > > > A stateful PCE might end up computing a path that is not the optimal
> > one
> > > > for
> > > > this specific optimization problem, due to a desire to cooperate with
> > other
> > > > paths in the network, and the "Min-Metric with margin and maximum
> > number of
> > > > SIDs" objective in draft-filsfils-spring-sr-policy-considerations
> > doesn't
> > > > even have a guaranteed unique best solution.
> > > >
> > > > Section 2.5
> > > >
> > > >    When provisioning is via configuration, this is an implementation's
> > > >    configuration model-specific unique identifier for a candidate path.
> > > >    The default value is 0.
> > > >
> > > > I'm having a lot of trouble parsing this.  Did we perhaps mean to
> > hyphenate
> > > > as "configuration-model-specific"?
> > > >
> > >
> > > KT> Ack
> > >
> > >
> > > >
> > > > Section 2.13
> > > >
> > > >    The SR Policy POL1 is identified by the tuple <headend, color,
> > > >    endpoint>.  It has two candidate paths CP1 and CP2.  Each is
> > > >    identified by a tuple <protocol-origin, originator, discriminator>.
> > > >
> > > > I suggest (for the last sentence) "identified within the scope of POL1"
> > > >
> > >
> > > KT> Ack
> > >
> > >
> > > >
> > > >    forwarding instantiation of SR policy POL1.  Traffic steered on POL1
> > > >    is flow-based hashed on Segment-List <SID11...SID1i> with a ratio
> > > >    W1/(W1+W2).
> > > >
> > > > If I read "ratio" I would instinctively think of the ratio of (traffic
> > on
> > > > segment list 1)/(traffic on segment list 2), as opposed to the
> > proportion
> > > > of
> > > > all traffic, that would be measured as the indicated W1/(W1+W2).
> > > >
> > >
> > > KT> Ack. s/ratio/proportion
> > >
> > >
> > > >
> > > > Section 3
> > > >
> > > >    The attached domain topology may be learned via IGP, BGP-LS or
> > > >    NETCONF.
> > > >
> > > >    A non-attached (remote) domain topology may be learned via BGP-LS or
> > > >    NETCONF.
> > > >
> > > > I think these are both probably not exhaustive lists, so "e.g." or
> > similar
> > > > may be appropriate.
> > > >
> > >
> > > KT> Ack.
> > >
> > >
> > > >
> > > > Section 4
> > > >
> > > >    Type C: IPv4 Prefix with optional SR Algorithm:
> > > >          The headend is required to resolve the specified IPv4 Prefix
> > > >          Address to the SR-MPLS label corresponding to a Prefix SID
> > > >          segment (as defined in [RFC8402]).  The SR algorithm (refer to
> > > >          Section 3.1.1 of [RFC8402]) to be used MAY also be provided.
> > > >
> > > >    Type D: IPv6 Global Prefix with optional SR Algorithm for SR-MPLS:
> > > >          In this case, the headend is required to resolve the specified
> > > >          IPv6 Global Prefix Address to the SR-MPLS label corresponding
> > > >          to its Prefix SID segment (as defined in [RFC8402]).  The SR
> > > >          Algorithm (refer to Section 3.1.1 of [RFC8402]) to be used MAY
> > > >
> > > > These are effectively just the IPv4 and IPv6 incarnations of the same
> > > > underlying procedure, right?  Can't we minimize the diff between the
> > > > paragraphs further?
> > > >
> > >
> > > KT> Ack
> > >
> > >
> > > >
> > > > Section 5.1
> > > >
> > > >    Additionally, a Segment-List MAY be declared invalid when:
> > > >
> > > > We probably want another word here ("both"?), to specify how the two
> > > > conditions are combined.
> > > >
> > >
> > > KT> Ack - will rephrase.
> > >
> > >
> > > >
> > > > Section 5.2
> > > >
> > > >    When the local computation is not possible (e.g., a policy's
> > tail-end
> > > >    is outside the topology known to the headend) or not desired, the
> > > >    headend MAY send path computation request to a PCE supporting PCEP
> > > >    extension specified in [RFC8664].
> > > >
> > > > missing article ("the PCEP extension").  I forget if it should be
> > > > "extensions" plural.
> > > >
> > >
> > > KT> Ack
> > >
> > >
> > > >
> > > > Section 8.7
> > > >
> > > >    Finally, headend H MAY be configured with a local routing policy
> > > >    which overrides any BGP/IGP path and steer a specified packet on an
> > > >
> > > > singular/plural mismatch -- s/steer/steers/
> > > >
> > >
> > > KT> Ack.
> > >
> > > Thanks,
> > > Ketan
> >
> >