Subject: Re: [Lsr] AD Review of draft-ietf-lsr-isis-srv6-extensions-11

From: Peter Psenak <[email-masked]>
Date: Thu, 11 Mar 2021 11:46:47 +0100

Hi Alvaro,

thanks for the review, please see inline (##PP):

On 26/02/2021 19:19, Alvaro Retana wrote:
> Dear authors:
> 
> Please find below my review of this document.  I appreciate you and
> the WG discussing the details, but there is more work needed to be
> done before starting the IETF LC (details inline).
> 
> Just one high-level comment: It is not clear to me why all the
> behaviors from rfc8986 are not covered in this document.  If some are
> not applicable, or are covered elsewhere, please explain in the text.

##PP
not all behaviors from rfc8986 are applicable to IGPs. Section 10 
("Advertising Endpoint Behaviors") lists the ones that are applicable to 
ISIS.

> 
> 
> Thanks!
> 
> Alvaro.
> 
> [Line numbers from idnits.]
> 
> 
> ...
> 16	Abstract
> 
> 18	   Segment Routing (SR) allows for a flexible definition of end-to-end
> 19	   paths by encoding paths as sequences of topological sub-paths, called
> 20	   "segments".  Segment routing architecture can be implemented over an
> 21	   MPLS data plane as well as an IPv6 data plane.  This draft describes
> 22	   the IS-IS extensions required to support Segment Routing over an IPv6
> 23	   data plane.
> 
> [nit] s/Segment routing architecture/The Segment routing architecture

##PP
fixed
> 
> 
> [nit] s/This draft/This document

##PP
fixed

> 
> 
> ...
> 108	1.  Introduction
> ...
> 119	   The network programming paradigm
> 120	   [I-D.ietf-spring-srv6-network-programming] is central to SRv6.  It
> 121	   describes how any behavior can be bound to a SID and how any network
> 122	   program can be expressed as a combination of SIDs.
> 
> [major] s/[I-D.ietf-spring-srv6-network-programming]/[RFC8986]/g

##PP
replaced all references to RFC8986
> 
> 
> ...
> 131	   This document defines one new top level IS-IS TLV and several new IS-
> 132	   IS sub-TLVs.
> 
> 134	   The SRv6 Capabilities sub-TLV announces the ability to support SRv6.
> 
> 136	   Several new sub-TLVs are defined to advertise various SRv6 Maximum
> 137	   SID Depths.
> 
> 139	   The new SRv6 Locator top level TLV announces SRv6 locators - a form
> 140	   of summary address for the set of topology/algorithm specific SIDs
> 141	   instantiated at the node.
> 
> 143	   The SRv6 End SID sub-TLV, the SRv6 End.X SID sub-TLV, and the SRv6
> 144	   LAN End.X SID sub-TLV are used to advertise which SIDs are
> 145	   instantiated at a node and what Endpoint behavior is bound to each
> 146	   instantiated SID.
> 
> [nit] There is some repetition in the last few paragraphs.  You talk
> about the new work, the the TLV, sub-TLVs, TLV again, and then the
> sub-TLVs.  A little consolidation would make this part read better.

##PP
Reordered and got rid of one of the paragraphs.

> 
> 
> 148	2.  SRv6 Capabilities sub-TLV
> 
> 150	   A node indicates that it supports the SR Segment Endpoint Node
> 151	   functionality as specified in [RFC8754] by advertising a new SRv6
> 152	   Capabilities sub-TLV of the router capabilities TLV [RFC7981].
> 
> [minor] What about the functionality in rfc8986?  I'm assuming that
> you want the nodes advertising this new sub-TLV to support both.  It
> may be implicit, but please make it explicit.

##PP
there are many behaviors specified in rfc8986, and we are not making 
that part of the capability. So we can not say the ISIS SRv6 capability 
means the support of entire  rfc8986.

> 
> 
> [minor] "router capabilities TLV [RFC7981]"  What should be the
> flooding scope of the TLV that includes this new sub-TLV?

##PP
It's up to the originator to set the S bit or not. We are not limiting 
it here.

> 
> 
> ...
> 159	      0                   1                   2                   3
> 160	       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
> 
> [nit] Please align the numbering.  There's one other occurrence in this section.

##PP
done

> 
> 
> ...
> 180	           O-flag: If set, the router supports use of the O-bit
> 181	           in the Segment Routing Header(SRH) as defined in
> 182	           [I-D.ietf-6man-spring-srv6-oam].
> 
> [nit] s/Segment Routing Header(SRH)/Segment Routing Header (SRH)

##PP
fixed

> 
> 
> [major] Please ask IANA to set up a registry for the Flags.

##PP
Les has started a separate thread with you on this point. I will wait 
till that discussion is closed.

> 
> 
> 184	3.  Advertising Supported Algorithms
> 
> 186	   SRv6 capable router indicates supported algorithm(s) by advertising
> 187	   the SR Algorithm TLV as defined in [RFC8667].
> 
> [nit] s/SRv6 capable router/An SRv6 capable router

##PP
fixed
> 
> 
> [minor] s/SR Algorithm TLV/SR Algorithm sub-TLV

##PP
fixed

> 
> 
> ...
> 200	4.1.  Maximum Segments Left MSD Type
> 
> 202	   The Maximum Segments Left MSD Type specifies the maximum value of the
> 203	   "SL" field [RFC8754] in the SRH of a received packet before applying
> 204	   the Endpoint behavior associated with a SID.
> 
> [minor] s/specifies/signals/g

#PP
fixed
> 
> 
> [minor] Please expand SL.

##PP
done

> 
> 
> 206	      SRH Max SL Type: 41
> 
> 208	      If no value is advertised the supported value is assumed to be 0.
> 
> [major] What exactly does this MSD type signal?  Is this the
> expectation that the SL will be <= the value when received at the
> advertiser?  Is there an example of its use?  I'm having a hard time
> picturing when (for non PSP behaviors) the SL would be more than 0.
> 

##PP

Yes, you are correct. As described in the paragraph right above it, this 
MSD type specifies what is the maximum value of the "Segments Left" 
field in the received SRH that this node is capable of processing.
A simple example: an ingress PE encapsulates a packet with a new IPv6 
and SRH. The SRH contains three segments. The Segments Left value of 
that SRH is set as per RFC8754 4.1, which in this case is equal to 2.
The router processing the first segment in the segment list, must 
support as a minimum an SRH Max SL MSD value of 2 in order to be able to 
process such packet.

> 
> 210	4.2.  Maximum End Pop MSD Type
> 
> 212	   The Maximum End Pop MSD Type specifies the maximum number of SIDs in
> 213	   the SRH to which the router can apply "PSP" or USP" behavior, as
> 214	   defined in [I-D.ietf-spring-srv6-network-programming] flavors.
> 
> [minor] Please expand PSP and USP.

##PP
done

> 
> 
> ...
> 221	4.3.  Maximum H.Encaps MSD Type
> 
> 223	   The Maximum H.Encaps MSD Type specifies the maximum number of SIDs
> 224	   that can be included as part of the "H.Encaps" behavior as defined in
> 225	   [I-D.ietf-spring-srv6-network-programming].
> 
> [nit] s/included/pushed   That is the terminology used in rfc8986.

##PP
fixed.

> 
> 
> ...
> 229	      If the advertised value is zero or no value is advertised
> 230	      then the router can apply H.Encaps only by encapsulating
> 231	      the incoming packet in another IPv6 header without SRH
> 232	      the same way IPinIP encapsulation is performed.
> 
> 234	      If the advertised value is non-zero then the router supports both
> 235	      IPinIP and SRH encapsulation subject to the SID limitation
> 236	      specified by the advertised value.
> 
> [major] rfc8986 doesn't talk about IPinIP encapsulation, but is does say this:
> 
>     The push of the SRH MAY be omitted when the SRv6 Policy only contains
>     one segment and there is no need to use any flag, tag or TLV.
> 
> Suggestion (to replace the last two paragraphs)>
>      If the advertised value is zero or no value is advertised then the
>      headend can apply an SR Policy that only contains one segment, by
>      omitting the SRH push.
> 
>      A non-zero SRH Max H.encaps MSD indicates that the headend can push
>      an SRH up to the advertised value.

##PP
done, but used "insert" instead of "push".

> 
> 
> 238	4.4.  Maximum End D MSD Type
> 
> 240	   The Maximum End D MSD Type specifies the maximum number of SIDs in an
> 241	   SRH when performing decapsulation associated with "End.Dx" behaviors
> 242	   (e.g., "End.DX6" and "End.DT6") as defined in
> 243	   [I-D.ietf-spring-srv6-network-programming].
> 
> [minor] "(e.g., "End.DX6" and "End.DT6")"  This MSD-Type only applies
> to *.DX6, DT6 and DT46, right?  If so, please be specific about it.

##PP

This MSD applies to any SID Behaviors that result in the outer IPv6 
header being removed and inner packet forwarded (e.g., End.DX4, End.DX6, 
End.DT4, End.DT6, End.DT46, End with USD, …).

I have updated the text to:

"The Maximum End D MSD Type specifies the maximum number of SIDs present 
   in an SRH when performing decapsulation. These includes, but not 
limited to, End.DX6, End.DT4, End.DT46, End with USD, End.X with USD as 
defined in [RFC8986].

> 
> 
> 245	      SRH Max End D Type: 45
> 
> 247	      If the advertised value is zero or no value is advertised
> 248	      then it is assumed that the router cannot apply
> 249	      "End.DX6" or "End.DT6" behaviors if the outer IPv6 header
> 250	      contains an SRH.
> 
> [minor] What about End.DT46?

##PP
This also applies to End.DT46 as clarified in the previous answer.

I have updated the text to:
   “If the advertised value is zero or no value is advertised
    then it is assumed that the router cannot apply
    any behavior that results in decapsulation and forwarding
    of the inner packet if the other IPv6 header contains an SRH.”

> 
> [major] What about other behaviors?  Are there no limitations to speak
> of related to them?  Why?  rfc8986 says this:
> 
>     The IGP should also advertise the Maximum SID Depth (MSD) capability of
>     the node for each type of SRv6 operation -- in particular, the SR source
>     (e.g., H.Encaps), intermediate endpoint (e.g., End and End.X), and final
>     endpoint (e.g., End.DX4 and End.DT6) behaviors.

##PP
the MSD types we defined do cover all SRv6 operations

> 
> 
> 252	5.  SRv6 SIDs and Reachability
> ...
> 269	   Locators are routable and MAY also be advertised in Prefix
> 270	   Reachability TLVs (236 or 237).
> 
> 272	   Locators associated with Flexible Algorithms [I-D.ietf-lsr-flex-algo]
> 273	   SHOULD NOT be advertised in Prefix Reachability TLVs (236 or 237).
> 
> 275	   Locators associated with algorithm 0 and 1 (for all supported
> 276	   topologies) SHOULD be advertised in a Prefix Reachability TLV (236 or
> 277	   237) so that legacy routers (i.e., routers which do NOT support SRv6)
> 278	   will install a forwarding entry for algorithm 0 and 1 SRv6 traffic.
> 
> 280	   In cases where a locator advertisement is received in both a Prefix
> 281	   Reachability TLV and an SRv6 Locator TLV, the Prefix Reachability
> 282	   advertisement MUST be preferred when installing entries in the
> 283	   forwarding plane.  This is to prevent inconsistent forwarding entries
> 284	   between SRv6 capable and SRv6 incapable routers.
> 
> [] There is a confusing combination of normative behavior for the
> locators in the last few paragraphs: "Locators...MAY also be
> advertised...SHOULD NOT be advertised...SHOULD be advertised in a
> Prefix Reachability TLV".  I realize that there are conditions
> applied...perhaps reorder some of them.
> 
> Suggestion (to replace the last 4 paragraphs)>
> 
>     Locators associated with algorithm 0 and 1 (for all supported
>     topologies) SHOULD be advertised in a Prefix Reachability TLV (236 or
>     237) so that legacy routers (i.e., routers which do not support SRv6)
>     will install a forwarding entry for algorithm 0 and 1 SRv6 traffic.
> 
>     In cases where a locator advertisement is received in both a Prefix
>     Reachability TLV and an SRv6 Locator TLV, the Prefix Reachability
>     advertisement MUST be preferred when installing entries in the
>     forwarding plane.  This is to prevent inconsistent forwarding entries
>     between SRv6 capable and SRv6 incapable routers.
> 
>     Locators associated with Flexible Algorithms (see Section 4,
>     [I-D.ietf-lsr-flex-algo]) SHOULD NOT be advertised in Prefix Reachability
>     TLVs (236 or 237).

##PP
done

> 
> 
> [major] "locator advertisement is received in both a Prefix
> Reachability TLV and an SRv6 Locator TLV"  What information results in
> these being an advertisement for the same locator?  Is only the
> locator (prefix length, etc.) considered, or should the algorithm,
> metric, etc. also match?

##PP
it's just prefix/mask. I added some text to clarify that.

> 
> 
> [major] "the Prefix Reachability advertisement MUST be preferred when
> installing entries in the forwarding plane"   Ok, but what about the
> rest of the information in the SRv6 Locator TLV?  How should that be
> considered?  For example, I'm assuming that the information in the
> sub-TLVs is still needed.

##PP
yes, there is no impact to the rest of the data in the Locator TLV.
Added sentence to clarify.

> 
> 
> [major] When is it ok to advertise locators in Prefix Reachability
> TLVs when associated with Flexible Algorithms?  IOW, why it it only
> recommended to do so and not required?  I assume the answer has to do
> with the possibility of routers implementing flex-algo and not SRv6 in
> the network -- please mention that.

##PP
Advertising the FA locator in regular Prefix Reachability would make the 
forwarding for it to follow algo 0 path. I don't know what this could be 
good for, but we did not want to preclude that. I added the text about it.

> 
> 
> [major] The SRv6 Locator TLV is a new TLV.  If no Prefix Reachability
> TLVs are present, how should the new TLV be used for route
> calculation/installation?  The text above suggests its use, but there
> is no specification.

#PP
The locator TLV advertises the prefix, same way as the Prefix 
Reachability. The calculation and installation of the prefix 
reachability is equal to what is done for regular Prefix Reachability.
I added the following text to one of the above paragraphs:

"The processing of the prefix advertised in the SRv6 Locator TLV,
the calculation of its reachability and  the installation in the 
forwarding plane is similar to one used for Prefix Reachability TLV (236 
or 237)."

> 
> 
> ...
> 291	   SRv6 SIDs are not directly routable and MUST NOT be installed in the
> 292	   forwarding plane.  Reachability to SRv6 SIDs depends upon the
> 293	   existence of a covering locator.
> 
> [major] "MUST NOT be installed"  This action depends on how the SIDs
> are advertised.  For example, if they are included in a Prefix
> Reachability TLV then the receiver will install them.  IOW, this
> action should be specified from the point of view of the sender; 

##PP
This section talks about received  SRv6 SIDs, not locators. SIDs are not 
advertised in Prefix Reachability TLV. Remote SIDs are never installed 
in the forwarding.

I updated the text to say "SRv6 SIDs received from other nodes..."

for
> example, "SIDs MUST be advertised in the sub-TLVs...[or maybe]...MUST
> NOT be advertised in another TLV...".

##PP
the previous section talks about how SIDs are advertised, which is the 
only way.

> 
> 
> [major] If some SIDs are advertised as "sub-TLVs in TLVs 22, 23, 222,
> 223, and 141", how can the "MUST NOT be installed" be satisfied?

##PP
above is the only way how SIDs are advertised.

> 
> 
> ...
> 302	   In order for forwarding to work correctly, the locator associated
> 303	   with SRv6 SID advertisements MUST be the longest match prefix
> 304	   installed in the forwarding plane for those SIDs.  There are a number
> 305	   of ways in which this requirement could be compromised.  In order to
> 306	   ensure correct forwarding, network operators should take steps to
> 307	   make sure that this requirement is not compromised.
> 
> 309	   o  Another locator associated with a different topology/algorithm is
> 310	      the longest match
> 
> 312	   o  A prefix advertisement (i.e., from TLV 236 or 237) is the longest
> 313	      match
> 
> [major] "MUST be the longest match prefix installed"  s/MUST/must
> This is not a normative statement.

##PP
ok, changed to must.

> 
> 
> [minor] The last two sentences in the paragraph sound redundant to me.
> 
> Suggestion>
>     In order to ensure correct forwarding, network operators should take
>     steps to make sure that this requirement is not compromised.  For
>     example, the following situations should be avoided:

##PP
done

> 
> 
> 315	6.  Advertising Anycast Property
> ...
> 322	   A new flag in "Bit Values for Prefix Attribute Flags Sub-TLV"
> 323	   registry [RFC7794] is defined to advertise the anycast property:
> 
> [major nit] The flag is defined in the sub-TLV, not the registry.
> 
> NEW>
>     A new flag in Prefix Attribute Flags Sub-TLV [RFC7794] is defined to
>     advertise the anycast property:

##PP
done

thanks,
Peter
> 
> 
> ...
> 328	       When the prefix/SRv6 locator is configured as anycast, the A-flag
> 329	       SHOULD be set. Otherwise, this flag MUST be clear
>

---

From: <[email-masked]>
Date: Fri, 12 Mar 2021 10:39:57 +0000

Peter, Alvaro

> From: Peter Psenak [mailto:[email-masked]]
> Sent: Thursday, March 11, 2021 11:47 AM

[...]

> done, but used "insert" instead of "push".

In SRv6, "Insert" has been used with a different meaning (SRH insertion without IP encapsulation) and hence is very connoted. So I would prefer if we could avoid the term "insert", to avoid both misunderstanding and ambiguities. 

I'm not sure how many/which  :s/push/insert  you are referring to as I'm seen 3  "push". I'll assume you meant the 3 of them. I would suggest the following change, but any other formulation would probably work for me.

OLD: The push of the SRH MAY be omitted
NEW: The SRH MAY be omitted

OLD: by omitting the SRH push.
NEW by omitting the SRH.

OLD: the headend can push an SRH up to the advertised value.
NEW: the headend can perform IP encapsulation with an SRH containing up to MSD SIDs.
(or may be: up to this number of SIDs)

[...]

> 249	      "End.DX6" or "End.DT6" behaviors if the outer IPv6 header

Since I've started, I'll continue to nick pick.

"assume" does not seem like the right term when talking about an explicit signalling.
I would suggest
OLD: then it is assumed that the router cannot apply
NEW: then the router cannot apply

*3 (in §4.1, §4.2, §4.4)

Thank you,
--Bruno

_________________________________________________________________________________________________________________________

Ce message et ses pieces jointes peuvent contenir des informations confidentielles ou privilegiees et ne doivent donc
pas etre diffuses, exploites ou copies sans autorisation. Si vous avez recu ce message par erreur, veuillez le signaler
a l'expediteur et le detruire ainsi que les pieces jointes. Les messages electroniques etant susceptibles d'alteration,
Orange decline toute responsabilite si ce message a ete altere, deforme ou falsifie. Merci.

This message and its attachments may contain confidential or privileged information that may be protected by law;
they should not be distributed, used or copied without authorisation.
If you have received this email in error, please notify the sender and delete this message and its attachments.
As emails may be altered, Orange is not liable for messages that have been modified, changed or falsified.
Thank you.

---

From: Peter Psenak <[email-masked]>
Date: Fri, 12 Mar 2021 15:12:43 +0100

Hi Bruno,

please see inline:

> (or may be: up to this number of SIDs)

not sure which document/version do you look at, but I don't see any 
occurrence of "push" or "insert" in the latest published version (11).

The single "push" I added as a response to Alvaro's comment was at:

The Maximum H.Encaps MSD Type signals the maximum number of SIDs
that can be pushed as part of the "H.Encaps" behavior as defined in
[RFC8986]"

Please let me know how do you prefer that to be modified.

> NEW: then the router cannot apply

fixed them all.

Peter

---

From: <[email-masked]>
Date: Fri, 12 Mar 2021 14:20:40 +0000

Hi Peter,

> Sent: Friday, March 12, 2021 3:13 PM
> 
> Hi Bruno,
> 
> please see inline:
> 
> >>> 223	   The Maximum H.Encaps MSD Type specifies the maximum
> number
> >> of SIDs
> >>> 224	   that can be included as part of the "H.Encaps" behavior as
> defined
> >> in
> > In SRv6, "Insert" has been used with a different meaning (SRH insertion
> without IP encapsulation) and hence is very connoted. So I would prefer if
> we could avoid the term "insert", to avoid both misunderstanding and
> ambiguities.
> >
> > I'm not sure how many/which  :s/push/insert  you are referring to as I'm
> seen 3  "push". I'll assume you meant the 3 of them. I would suggest the
> following change, but any other formulation would probably work for me.
> >

I'm referring to the email (above) that you sent yesterday on the LSR mailing list in which you said " ##PP done, but used "insert" instead of "push"."
It's not reflected in the latest published version which date from October 8, 2020

> > NEW: then the router cannot apply

Thanks,
--Bruno

> Peter
> >
> >
> > *3 (in §4.1, §4.2, §4.4)
> >
> >
> > Thank you,
> > --Bruno
> >
> >
> __________________________________________________________
> __________________________________________________________
> _____
> >
> > Ce message et ses pieces jointes peuvent contenir des informations
> confidentielles ou privilegiees et ne doivent donc
> > a l'expediteur et le detruire ainsi que les pieces jointes. Les messages
> electroniques etant susceptibles d'alteration,
> > This message and its attachments may contain confidential or privileged
> information that may be protected by law;

---

From: "Acee Lindem (acee)" <[email-masked]>
Date: Fri, 12 Mar 2021 14:38:16 +0000

Thanks for your review and comments Bruno and Peter for quick resolution. 
Acee

On 3/12/21, 9:20 AM, "[email-masked]" <[email-masked]> wrote:

    Hi Peter,

    Thanks,
    --Bruno

---

From: Alvaro Retana <[email-masked]>
Date: Tue, 16 Mar 2021 13:33:54 -0700

On March 11, 2021 at 5:46:51 AM, Peter Psenak wrote:

Peter:

Hi!

> thanks for the review, please see inline (##PP):

It looks like you didn't get the whole review (Outlook bug).  Take a
look at it here:
https://mailarchive.ietf.org/arch/msg/lsr/a4a4I4fP73DyfKsdKnRw_tRuStQ/

...
> ("Advertising Endpoint Behaviors") lists the ones that are applicable to

I understand that -- other readers may not.

Also, §8.1/rfc8986 talks about what is expected to be carried in the
IGP.  End.T is not included in this document.

...
...
> It's up to the originator to set the S bit or not. We are not limiting

But if the originator doesn't set the correct flooding scope then the
information will not make it to where is has to.  I would like to see
some guidance on the setting of the bit -- I'm assuming that most of
the cases require the S bit to be set, right?

...
...
> support as a minimum an SRH Max SL MSD value of 2 in order to be able to

I see.  It would be very nice if the text said somewhere that the MSD
applies not just to the final endpoint but also to intermediate nodes.

...
...
...
> it's just prefix/mask. I added some text to clarify that.

So...even if the MT-ID and algorithms are different, the
advertisements are considered the same?   I don't have a specific case
right now, but it sounds to me that this could result in some type of
incongruency.   I'll think more about this later -- no further action
needed.

...
> forwarding plane is similar to one used for Prefix Reachability TLV (236

s/is similar to one used for Prefix Reachability TLV (236 or
237)./follows the process defined for the Prefix Reachability TLV (236
or 237) [rfc5308].

Would that be ok?

> the previous section talks about how SIDs are advertised, which is the

Just to make sure we're talking about the same thing.  The SIDs are
covered by the LOC, which is routable, installed in the RIB/FIB, etc.

There's nothing in this document or rfc8986 that prevents a SID to
have the same value as the IP address on a link (see below).  Then
that link address could be advertised using normal (non-SR-related)
mechanisms.

In this case, the receiver would know the address is also a SID.
Should that result in "MUST NOT be installed"?   Not even the MUSTs
that I suggested could prevent this situation.

> above is the only way how SIDs are advertised.

This is the text from -11:

   SRv6 SIDs are advertised as sub-TLVs in the SRv6 Locator TLV except
   for SRv6 End.X SIDs/LAN End.X SIDs which are associated with a
   specific Neighbor/Link and are therefore advertised as sub-TLVs in
   TLVs 22, 23, 222, 223, and 141.

   SRv6 SIDs are not directly routable and MUST NOT be installed in the
   forwarding plane.  Reachability to SRv6 SIDs depends upon the
   existence of a covering locator.

The text says that there is more than one way to advertise SIDs, *and*
that they "MUST NOT be installed".

Thanks!

Alvaro.

---

From: Peter Psenak <[email-masked]>
Date: Thu, 25 Mar 2021 11:03:49 +0100

Hi Alvaro,

please see inline (##PP2):

> https://mailarchive.ietf.org/arch/msg/lsr/a4a4I4fP73DyfKsdKnRw_tRuStQ/

##PP2

sigh...

I added the rest of the comments to the end of this email and responded 
inline to them.

> I understand that -- other readers may not.

##PP2
we defined all behaviors that rfc8986 mentions should be advertised by 
IGP, except the END.T. The END.T was originally defined, but during the 
WGLC it was removed based on WG discussion:

Mailing list discussion:
Thread1: 
https://mailarchive.ietf.org/arch/msg/lsr/0Bp5DJrRJPvRyzZMZS0P_OE8Y7Q/
Thread2: 
https://mailarchive.ietf.org/arch/msg/lsr/nKJbY5f6SHEwVCqqfoXSPidGKXQ/

> the cases require the S bit to be set, right?

##PP2
not really. There is no need for the SRv6 Capabilities sub-TLV to be 
propagated across the area for now. If it will become required later, 
the S-bit is there. We don't really want to specify what is required 
because that may change over time. We are defining it in a way that both 
area and domain wide scope is possible. It's up to the user to pick 
which one does he need.

> applies not just to the final endpoint but also to intermediate nodes.

##PP2
well, I find that a bit misleading. The "router processing the first 
segment" is the one that applies the Endpoint behavior
associated with such SID.

> incongruency.   I'll think more about this later -- no further action

##PP2
Prefix Reachability TLV does not have the algo field, so it is 
implicitly associated with Algo 0. Yes, MT-ID needs to be considered, I 
have clarified that in the text.

> 237)./follows the process defined for the Prefix Reachability TLV (236

##PP2
done

> covered by the LOC, which is routable, installed in the RIB/FIB, etc.

##PP2
yes

> that link address could be advertised using normal (non-SR-related)

##PP2
I'm not sure how one can use the same 128 bit IPv6 address as a local 
address and as a SID at the same time.

> that I suggested could prevent this situation.

##PP2
regardless of the validity of the case, this section only talks about 
the remote SID. We are not prohibiting the same address advertised 
otherwise to be installed in forwarding in any way.

> that they "MUST NOT be installed".

##PP2
there are two ways to advertise SRv6 SIDs in ISIS depending of the SID 
type, but only one way to advertise any particular SID type. So there is 
no duplication possible. I thought the text above is clear on that.

---

...
328	       When the prefix/SRv6 locator is configured as anycast, the A-flag
329	       SHOULD be set. Otherwise, this flag MUST be clear.

[major] When is it ok to not set the flag if advertising an anycast
prefix?  IOW, why is setting the flag recommended and not required?

##PP2
The A-flag is an optional flag. It's hard to mandate the setting of it 
given that anycast has been used for years and we are introducing the 
flag just now.

331	   The A-flag MUST be preserved when leaked between levels.

[minor] s/preserved when leaked/preserved when the advertisement is leaked

##PP2
done

333	   The A-flag and the N-flag MUST NOT both be set.

335	   If both N-flag and A-flag are set in the prefix/SRv6 Locator
336	   advertisement, the receiving routers MUST ignore the N-flag.

[nit] Join the last two paragraphs.

##PP2
done

338	   The same prefix/SRv6 Locator can be advertised by multiple routers.
339	   If at least one of them sets the A-Flag in its advertisement, the
340	   prefix/SRv6 Locator SHOULD be considered as anycast.

[major] "SHOULD be considered as anycast"  Just to make sure I
understand, the A-flag is informational -- there is no action taken by
the receiving router, right?   If so, then there's no interoperability
requirement reflected here.  s/SHOULD/should

##PP2
there can be action on A-flag. For example, you do not want to use the 
SID associated with the anycast Prefix/Locator for TI-LFA or uloop 
prevention. We do not specify the usage here, because the usage is 
optional, but I would keep the SHOULD here.

342	   A prefix/SRv6 Locator that is advertised by a single node and without
343	   an A-Flag SHOULD be interpreted as a node specific locator.

[major] "advertised by a single node and without an A-Flag"  This is
equivalent to the current behavior of a prefix being "advertised by a
single node and without an A-Flag".  IOW, you seem to be specifying
behavior that a node that doesn't implement (or even know about) this
document is expected to follow.

##PP2
if I remove the "prefix" and only keep the SRv6 Locator, would you be 
fine with it? We are defining SRv6 Locator in this document.

[major] Related... What is a "node specific locator"?  The A-flag
functionality could be used in a network that otherwise doesn't
implement SRv6, so calling it a "locator" doesn't seem right.

##PP2
please see my previous response.

[major] "SHOULD be interpreted"  Interpreting is not really an
interoperability-requiring action.  Is there anything here resulting
from the interpretation that requires normative language?

##PP2
what about:

    "An SRv6 Locator that is advertised by a single node and without
     an A-Flag is considered as a node specific locator."

...
349	   The Prefix Attribute Flags Sub-TLV can be carried in the SRv6 Locator
350	   TLV as well as the Prefix Reachability TLVs.  When a router
351	   originates both the Prefix Reachability TLV and the SRv6 Locator TLV
352	   for a given prefix, and the router is originating the Prefix
353	   Attribute Flags Sub-TLV in one of the TLVs, the router SHOULD
354	   advertise identical versions of the Prefix Attribute Flags Sub-TLV in
355	   both TLVs.

[minor] This paragraph doesn't seem necessary given this text in §5:

    forwarding plane.

##PP2
above mentioned paragraph does not say anything about the Prefix 
Attribute Flags Sub-TLV present in the advertisement of the same prefix 
in the Locator TLV and Prefix Reachability TLV. So we need to keep it.

[major] If you decide to keep it...  "SHOULD advertise
identical...Prefix Attribute Flags Sub-TLV"   When is it ok to not do
so?  Again, given that the Prefix Reachability TLVs are preferred,
this statement doesn't seem to matter, or carry interoperability
weight.  s/SHOULD/should

##PP
well, not really. The Prefix Attribute Flags Sub-TLV should be the same 
to guarantee the same treatment of both locator and legacy prefix 
advertisements. The fact that the legacy prefix advertisement is 
preferred when installing reachability of the prefix to forwarding does 
not mean the Prefix Attribute Flags Sub-TLV advertised with Locator TLV 
is useless - it can still be used when using Locator for other things - 
e.g. derive SID for TILFA protection, etc.

[] This point is related to Gunter's recent e-mail [1].

##PP2
the text has been updated to address Gunter's comment as follows:

"The Prefix Attribute Flags Sub-TLV can be carried in the SRv6
  Locator TLV as well as the Prefix Reachability TLVs. When a router 
originates both the Prefix Reachability TLV and the SRv6 Locator TLV for 
a given prefix, and the router is originating the Prefix Attribute Flags 
Sub-TLV in one of the TLVs, the router SHOULD advertise same flags in 
the Prefix Attribute Flags Sub-TLV in both TLVs. However, unlike TLVs 
236/237 the X-flag in the Prefix Attributes Flags sub-TLV is valid when 
sent in the SRv6 Locator TLV. The state of the X-flag in the Prefix 
Attributes Flags sub-TLV when included in the Locator TLV MUST match the 
setting of the embedded X-flag in any advertisement of the same prefix 
in TLVs 236/237."

[1] https://mailarchive.ietf.org/arch/msg/lsr/AlglGZ8UsZSaPGwwTeA1qyDWQL0/

...
365	7.1.  SRv6 Locator TLV Format
...
369	    0                   1                   2                   3
370	    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
371	   +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
372	   |   Type        |     Length    |R|R|R|R|    MTID               |
373	   +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+

[minor] Please add Figure numbers/names for all packet formats.

##PP2
s that really required? I have never done it in any of RFCs I was editor 
for.

...
382	     MTID: Multitopology Identifier as defined in [RFC5120].
383	     Note that the value 0 is legal.

[major] s/MTID/MT ID/g

[] Is the note necessary?

##PP2
I would keep it to, as RFC5120 prevents 0 to be used for TLVs that are 
defined in RFC5120 itself.

[major] What should the receiver do if the MT ID is outside the valid range?

##PP2
MT-ID is defined as 12 bits, so there is no outside range here.

...
403	       0
404	       0 1 2 3 4 5 6 7
405	      +-+-+-+-+-+-+-+-+
406	      |D|    Reserved |
407	      +-+-+-+-+-+-+-+-+

[major] How should these reserved flags be managed?  Please create a 
registry.

##PP2
I'm following what the WG has been doing so far - no registry for flags 
field. If the WG decides to change its policy on this matter, I will 
update the document with the registries.

409	      where:
410	        D bit: When the Locator is leaked from level-2 to level-1, the D
411	        bit MUST be set.  Otherwise, this bit MUST be clear.  Locators
412	        with the D bit set MUST NOT be leaked from level-1 to level-2.
413	        This is to prevent looping.

[major] Is there any difference between this bit and the up/down bit
defined in rfc5305?   To reuse functionality, generalize (to multiple
levels), and avoid redefining please point to the definition in
rfc5305.

##PP2
done

...
418	     Algorithm: 1 octet. Associated algorithm. Algorithm values
419	      are defined in the IGP Algorithm Type registry.

[major] Use this text for all the Algorithm fields.

NEW>
    Algorithm: 1 octet. As defined in rfc8665.

Add a Normative reference.

##PP2
done

421	     Loc-Size: 1 octet. Number of bits in the SRv6 Locator field.
422	     (1 - 128)

[major] What should the receiver do if the Loc-Size is not in that range?

##PP2
Added:

"MUST be from the range (1 - 128). The TLV MUST be ignored if the 
Loc-Size is outside of this range."

...
429	     Sub-TLV-length: 1 octet. Number of octets used by sub-TLVs

[nit] Note that the name of the field in the packet format shown above
is written as "Sub-tlv-len".  Please be consistent!

##PP2
fixed

[major] Please indicate which sub-TLVs are applicable/can be included
with this new TLV.  I know that §12.1.2 updates the registry, but that
is not normative with respect to the operation of the protocol.  This
seems like a simple (one-sentence) addition since all the sub-TLVs
seem applicable.

##PP2:

Added:
Optional sub-TLVs: Sub-TLVs 1, 2, 4, 5, 11, 12 are allowed. Any other 
Sub-TLVs MUST be ignored.

431	     Optional sub-TLVs.

[] This seems to be leftover text.

##PP2
modified as above.

433	7.2.  SRv6 End SID sub-TLV

435	   The SRv6 End SID sub-TLV is introduced to advertise SRv6 Segment
436	   Identifiers (SID) with Endpoint behaviors which do not require a
437	   particular neighbor in order to be correctly applied
438	   [I-D.ietf-spring-srv6-network-programming].  SRv6 SIDs associated
439	   with a neighbor are advertised using the sub-TLVs defined in
440	   Section 8.

[minor] "which do not require a particular neighbor in order to be
correctly applied"  Which behaviors are those?   I didn't find in
rfc8986 any similar phrasing.  You may want to put a reference to
Section 10 instead.

##PP2
This is ISIS specific categorization, not something specified in 
rfc8986. I removed the reference to rfc8986 and added reference to 
section 10.

...
470	      Flags: 1 octet.  No flags are currently defined.

[minor] How will these flags be managed?  Do you want to define a new 
registry?

##PP2
I'm following what the WG has been doing so far - no registry for flags 
field. If the WG decides to change its policy on this matter, I will 
update the document with the registries.

472	      Endpoint Behavior: 2 octets, as defined in [I-D.ietf-spring-srv6-
473	      network-programming].  Legal behavior values for this sub-TLV are
474	      defined in Section 10 of this document.

[major] What should the receiver do if an illegal value is received?

[nit] s/Legal/Allowed

##PP2
I have changed the text to:

"Supported behavior values for this sub-TLV are defined in Section 10
  of this document. Unsupported or unrecognized behavior values are 
ignored by the receiver."

...
478	      Sub-sub-TLV-length: 1 octet.  Number of octets used by sub-sub-
479	      TLVs.

[nit] Note that the name of the field in the packet format shown above
is written as "Sub-sub-tlv-len".  Please be consistent!

##PP2
fixed

481	      Optional sub-sub-TLVs.

[] Leftover text.

##PP2
I don't think it is leftover text, the list described the content of the 
SRv6 End SID sub-TLV, and "Optional sub-sub-TLVs" are part of it.

483	   The SRv6 End SID MUST be a subnet of the associated Locator.  SRv6
484	   End SIDs which are NOT a subnet of the associated locator MUST be
485	   ignored.

[minor] s/NOT/not/g   This is not an rfc2119 keyword.

##PP2
fixed

...
494	8.  Advertising SRv6 Adjacency SIDs
...
508	   All End.X SIDs MUST be a subnet of a Locator with matching topology
509	   and algorithm which is advertised by the same node in an SRv6 Locator
510	   TLV.  End.X SIDs which do not meet this requirement MUST be ignored.

512	   All End.X and LAN End.X SIDs MUST be subsumed by the subnet of a
513	   Locator with the matching algorithm which is advertised by the same
514	   node in an SRv6 Locator TLV.  End.X SIDs which do not meet this
515	   requirement MUST be ignored.  This ensures that the node advertising
516	   the End.X or LAN End.X SID is also advertising its corresponding
517	   Locator with the algorithm that will be used for computing paths
518	   destined to the SID.

[minor] There is repetition in the last two paragraphs.

[minor] "All End.X SIDs..."  It looks like you're talking about all
SIDs that can be included in the End.X or LAN End.X sub-TLVs, and not
just about End.X (the behavior) SIDs, is that correct?   Please be
specific.

##PP2
correct. Avoided duplication and updated to be specific.

520	8.1.  SRv6 End.X SID sub-TLV
...
561	         B-Flag: Backup flag.  If set, the End.X SID is eligible for
562	         protection (e.g., using IPFRR) as described in [RFC8355].

[minor] "End.X SID"   As mentioned above, the use of this terminology
creates confusion because you should be talking about the SID (in the
End.X SID sub-TLV) and not the End.X SID.

##PP2
I have fixed that across the whole document

564	         S-Flag.  Set flag.  When set, the S-Flag indicates that the
565	         End.X SID refers to a set of adjacencies (and therefore MAY be
566	         assigned to other adjacencies as well).

[major] What should a receiver do if a SID is advertised in multiple
sub-TLVs (associated to different adjacencies) without the S-Flag set?
   What about the case where the SID was first advertised without the
S-Flag set and a second sub-TLV (for a different adjacency) has it
set?  Is this Flag only informational?

##PP2
it is informational. It can be used to avoid picking the shared SIDs for 
things like TILFA or uloop avoidance. It has no impact on the way the 
SID behaves.

...
573	         Reserved bits: MUST be zero when originated and ignored when
574	         received.

[major] How should these bits be managed?  Please set up a new registry.

##PP2
I'm following what the WG has been doing so far - no registry for flags 
field. If the WG decides to change its policy on this matter, I will 
update the document with the registries.

...
579	      Weight: 1 octet.  The value represents the weight of the End.X SID
580	      for the purpose of load balancing.  The use of the weight is
581	      defined in [RFC8402].

[major] What is the relationship between the Weight and the S-Flag?
If the S-Flag is set (in multiple sub-TLVs for different adjacencies),
should a Weight value also be present?   What should the receiver do
if only one of the sub-TLVs includes a weight?

##PP2
The usage of weight is described in rfc8402. We do not define it here. 
We are only defining the way it is advertised. We did the same in rfc8667.

583	      Endpoint Behavior: 2 octets.  As defined in [I-D.ietf-spring-srv6-
584	      network-programming] Legal behavior values for this sub-TLV are
585	      defined in Section 10.

[major] What should the receiver do if an illegal value is received?

[nit] s/Legal/Allowed

##PP2
I have changed the text to:

"Supported behavior values for this sub-TLV are defined in Section 10
  of this document. Unsupported or unrecognized behavior values are 
ignored by the receiver."

...
596	8.2.  SRv6 LAN End.X SID sub-TLV

[] Some of the comments form the previous section apply here as well.

##PP2
fixed them similarly.

598	   This sub-TLV is used to advertise an SRv6 SID associated with a LAN
599	   adjacency.  Since the parent TLV is advertising an adjacency to the
600	   Designated Intermediate System(DIS) for the LAN, it is necessary to
601	   include the System ID of the physical neighbor on the LAN with which
602	   the SRv6 SID is associated.  Given that a large number of neighbors
603	   may exist on a given LAN a large number of SRv6 LAN END.X SID sub-
604	   TLVs may be associated with the same LAN.  Note that multiple TLVs
605	   for the same DIS neighbor may be required in order to advertise all
606	   of the SRv6 End.X SIDs associated with that neighbor.

[nit] s/System(DIS)/System (DIS)

##PP2
fixed

...
666	9.  SRv6 SID Structure Sub-Sub-TLV

[] I don't understand what this sub-sub-TV is used for.  Can you
please explain?  Is there a relationship between it and the SID that
is advertised in the sub-TLVs?  For example, I would assume that the
SID would have the bits that correspond to the argument set to 0 --
what if they're not?  What is the purpose of this information?   [Of
course, none of the supported behaviors take an ARG...]

##PP2
The SRv6 SID Structure Sub-Sub-TLV indicates the structure of the SID 
associated with it. It can be used by implementation for validation of 
the SID for consistency (e.g. if there is no ARG but there is something 
in the ARG bits, then it can be ignored). They can be signalled via 
BGP-LS to controller/apps that can verify the consistency in the block 
and SID addressing in the domain. Details are outside the scope of this 
draft.

...
707	   The sum of all four sizes advertised in ISIS SRv6 SID Structure Sub-
708	   Sub-TLV must be lower or equal to 128 bits.  If the sum of all four
709	   sizes advertised in the ISIS SRv6 SID Structure Sub-Sub-TLV is larger
710	   than 128 bits, the parent Sub-TLV MUST be ignored by the receiver.

[major] s/must be lower/MUST be lower

##PP2
fixed

712	10.  Advertising Endpoint Behaviors

714	   Endpoint behaviors are defined in
715	   [I-D.ietf-spring-srv6-network-programming].  The codepoints for the
716	   Endpoint behaviors are defined in the "SRv6 Endpoint Behaviors"
717	   registry defined in [I-D.ietf-spring-srv6-network-programming].  This
718	   section lists the Endpoint behaviors which MAY be advertised by ISIS,
719	   together with their codepoints.  If this behavior is advertised it
720	   MUST only be advertised in the TLV[s] as indicated by "Y" in the
721	   table below, and MUST NOT be advertised in the TLV[s] as indicated by
722	   "N" in the table below.

[minor] The second sentence is redundant (and inaccurate), please take 
it out.

##PP2
removed

[major] What about other behaviors from rfc8986?  If they are not
applicable, please explain why.

##PP2
section 8.4 of rfc8986 only list subset of behaviors which are to be 
advertised by IGPs. We defined them all except END.T. as I described 
earlier.

724	     Endpoint             |Endpoint          | End | End.X | Lan End.X |
725	     Behavior             |Behavior Codepoint| SID | SID   |   SID     |
726	    ----------------------|------------------|-----|-------|-----------|
727	     End   (PSP, USP, USD)| 1-4, 28-31       |  Y  |   N   |    N      |
728	    ----------------------|------------------|-----|-------|-----------|
729	     End.X (PSP, USP, USD)| 5-8, 32-35       |  N  |   Y   |    Y      |
730	    ----------------------|------------------|-----|-------|-----------|
731	     End.DX6              | 16               |  N  |   Y   |    Y      |
732	    ----------------------|------------------|-----|-------|-----------|
733	     End.DX4              | 17               |  N  |   Y   |    Y      |
734	    ----------------------|------------------|-----|-------|-----------|
735	     End.DT6              | 18               |  Y  |   N   |    N      |
736	    ----------------------|------------------|-----|-------|-----------|
737	     End.DT4              | 19               |  Y  |   N   |    N      |
738	    ----------------------|------------------|-----|-------|-----------|
739	     End.DT64             | 20               |  Y  |   N   |    N      |

[minor] s/End.DT64/End.DT46

##PP2
fixed

741	11.  Implementation Status
...
752	      Types of SID supported: End, End.X, LAN End.X, END.OP

[] "END.OP" is not defined.  Also, the others are not types of SIDs,
but sub-TLVs.

##PP2
removed END.OP. This section is going to be removed anyway.

...
808	12.1.  SRv6 Locator TLV

810	   This document makes the following registrations in the the IS-IS TLV
811	   Codepoints registry:

813	      Type: 27

815	      Description: SRv6 Locator TLV.

817	      Reference: This document (Section 7.1).

[major] Include all the information related to the message types (IIH,
LSP, SNP, Purge).

##PP2
done

819	   A Locator TLV shares sub-TLV space with existing "Sub-TLVs for TLVs
820	   135, 235, 236 and 237 registry".  The name of this registry needs to
821	   be changed to "Sub-TLVs for TLVs 27, 135, 235, 236 and 237 registry".

[major] Use the complete name of the registry.

##PP2
done

NEW>
    The SRv6 Locator TLV shares sub-TLV space with TLVs 135, 235, 236 
and 237.
    IANA is requested to update the name of the "Sub-TLVs for TLVs 135, 235,
    236, and 237 (Extended IP reachability, MT IP. Reach, IPv6 IP. 
Reach, and
    MT IPv6 IP. Reach TLVs)" registry to "Sub-TLVs for TLVs 27, 135, 
235, 236,
    and 237 (SRv6 Locator, Extended IP reachability, MT IP. Reach, IPv6 IP.
    Reach, and MT IPv6 IP. Reach TLVs)".

Also, this action (renaming) should be moved to a common section with
the new SRv6 End SID sub-TLV (§12.1.1) and the updated table
(§12.1.2).  The action of allocating the SRv6 Locator TLV is
independent (different registry, etc.).

##PP2
done

...
834	12.1.2.  Revised sub-TLV table
...
839	      Type  27 135 235 236 237

841	      1     y   y   y   y   y
842	      2     y   y   y   y   y
843	      3     n   y   y   y   y
844	      4     y   y   y   y   y
845	      5     y   n   n   n   n
846	      6     n   y   y   y   y
847	      11    y   y   y   y   y
848	      12    y   y   y   y   y
849	      32    n   y   y   y   y

[major] Because the structure of the registry is changed, this
document should formally Update rfc7370 (where the current registry
was defined).

##PP2
I added following text:

"This document updates the "Sub-TLVs for TLVs 135, 235,
236, and 237 (Extended IP reachability, MT IP. Reach, IPv6 IP. Reach, 
and MT IPv6 IP. Reach TLVs)" registry defined in [RFC7370] to section 
§12.1.1."

Would that be sufficient?

[minor] Please name and number all tables.

##PP2
is that really required? I have never done it in any of RFCs I was 
editor for.

851	12.2.  SRv6 Capabilities sub-TLV

853	   This document makes the following registrations in the "Sub- TLVs for
854	   TLV 242 registry":

[major] s/"Sub- TLVs for TLV 242 registry"/"Sub-TLVs for TLV 242
(IS-IS Router CAPABILITY TLV)" registry

##PP2
done

...
862	   This document requests the creation of a new IANA managed registry
863	   for sub-sub-TLVs of the SRv6 Capability sub-TLV.  The registration
864	   procedure is "Expert Review" as defined in [RFC8126].  Suggested
865	   registry name is "sub-sub-TLVs for SRv6 Capability sub-TLV".  No sub-
866	   sub-TLVs are defined by this document except for the reserved value.

[major] Please indicate tat it should be part of "IS-IS TLV Codepoints".

##PP2
done

[major] ""Expert Review" as defined in [RFC8126]"  Please add that the
guidance for the Designated Experts is provided in rfc7310.

##PP2
done

...
872	12.3.  SRv6 End.X SID and SRv6 LAN End.X SID sub-TLVs

874	   This document makes the following registrations in the "sub- TLVs for
875	   TLV 22, 23, 25, 141, 222 and 223 registry":

[major] s/"sub- TLVs for TLV 22, 23, 25, 141, 222 and 223
registry"/"Sub-TLVs for TLVs 22, 23, 25, 141, 222, and 223 (Extended
IS reachability, IS Neighbor Attribute, L2 Bundle Member Attributes,
inter-AS reachability information, MT-ISN, and MT IS Neighbor
Attribute TLVs)"

Yes, I know that's a long name, but that is the name.

##PP2
done

...
894	12.4.  MSD Types

896	   This document makes the following registrations in the IGP MSD Types
897	   registry:

[minor] s/MSD Types/MSD-Types

#PP2
done

899	   Type  Description
900	   ------------------
901	    41    SRH Max SL
902	    42    SRH Max End Pop
903	    44    SRH Max H.encaps
904	    45    SRH Max End D

[minor] This table should have 3 columns: Value, Name and Reference.

##PP2
done

906	12.5.  Sub-Sub-TLVs for SID Sub-TLVs

908	   This document requests a new IANA registry be created under the IS-IS
909	   TLV Codepoints Registry to control the assignment of sub-TLV types
910	   for the SID Sub-TLVs specified in this document - Section 7.2,
911	   Section 8.1, Section 8.2.  The suggested name of the new registry is
912	   "Sub-Sub-TLVs for SID Sub-TLVs".  The registration procedure is
913	   "Expert Review" as defined in [RFC8126].  The following assignments
914	   are made by this document:

[minor] In line with the name of other registries; suggestion:
"Sub-sub-TLVs for sub-TLVs 5, 43 and 44 (SRv6 End SID , SRv6 End.X SID
and SRv6 LAN End.X SID)".

##PP2
I find that confusing as the sub-TLVs 5 is a locator Sub-TLV, while 
Sub-TLVs 43 and 44 are IS reachability sub-TLVs.
What about:

"Sub-Sub-TLVs for SID Sub-TLVs (SRv6 End SID, SRv6 End.X SID
  and SRv6 LAN End.X SID)"

[major] ""Expert Review" as defined in [RFC8126]"  Please add that the
guidance for the Designated Experts is provided in rfc7310.

##PP2
done

916	      0: Reserved

918	      1: SRv6 SID Structure Sub-Sub-TLV (Section 9).

[minor] Please create a table and include the unassigned range.

##PP2
done

920	12.6.  Prefix Attribute Flags Sub-TLV
...
927	      Description: A bit

[major] Can we please have the registry show "Anycast Flag (A-flag)"?

##PP2
done

...
931	13.  Security Considerations

933	   Security concerns for IS-IS are addressed in [ISO10589], [RFC5304],
934	   and [RFC5310].

<sigh> Well, at least it is not explicitly claimed that no new
security considerations exist. <sigh>

[minor] Please copy the first paragraph of the Security Consideration
from rfc8919.

##PP2
done

[major] Let's add some more meat.

Suggestion>
    This document describes the IS-IS extensions required to support Segment
    Routing over an IPv6 data plane.  The security considerations for 
Segment
    Routing are discussed in [RFC8402].  [RFC8986] defines the SRv6 Network
    Programming concept and specifies the main Segment Routing behaviors to
    enable the creation of interoperable overlays; the security 
considerations
    from that document apply too.

##PP
added

[major] Are there new security issues/risks created by this document?
Sure!  Note that even if the risk existed before, adding new ways to
exploit it is a new attack vector.

(1) Suggestion>
    The advertisement of an incorrect MSD value may have negative
    consequences, see rfc8491 for additional considerations.
##PP2
added

(2) The interaction in §5 between the Prefix Reachability TLV and SRv6
Locator TLV is not completely clear.  Depending on the answers to my
questions, there may be cases where inconsistent information can be
present resulting in unexpected forwarding.

##PP2
I don't see the security risk there. Traffic may be forwarded to a node 
where the SID does not exist and be dropped there, but that is not a 
security risk.

(3) Not following the operational considerations at the end of §5
could also result in unexpected forwarding.

##PP2
yes, but that is not a security concern. Similar to providing 
overlapping static routes pointing to each other. Is that a security 
risk? No, it's a misconfiguration.

What else?

...
982	15.1.  Normative References
...
997	   [ISO10589]
998	              Standardization", I. ". O. F., "Intermediate system to
999	              Intermediate system intra-domain routeing information
1000	              exchange protocol for use in conjunction with the 
protocol
1001	              for providing the connectionless-mode Network Service 
(ISO
1002	              8473), ISO/IEC 10589:2002, Second Edition.", Nov 2002.

[minor] Please use this as the reference:

[ISO10589]

     ISO, "Information technology -- Telecommunications and information
     exchange between systems -- Intermediate System to Intermediate System
     intra-domain routeing information exchange protocol for use in
     conjunction with the protocol for providing the connectionless-mode
     network service (ISO 8473)", ISO/IEC 10589:2002, Second Edition,
     November 2002.

##PP2
done

...
1015	   [RFC5304]  Li, T. and R. Atkinson, "IS-IS Cryptographic
1016	              Authentication", RFC 5304, DOI 10.17487/RFC5304, October
1017	              2008, <https://www.rfc-editor.org/info/rfc5304>.

[minor] This reference can be Informative.

##PP2
done

...
1023	   [RFC5310]  Bhatia, M., Manral, V., Li, T., Atkinson, R., White, R.,
1024	              and M. Fanto, "IS-IS Generic Cryptographic
1025	              Authentication", RFC 5310, DOI 10.17487/RFC5310, February
1026	              2009, <https://www.rfc-editor.org/info/rfc5310>.

[minor] This reference can be Informative.

##PP2
done

...
1067	15.2.  Informative References

1069	   [I-D.ietf-lsr-flex-algo]
1070	              Psenak, P., Hegde, S., Filsfils, C., Talaulikar, K., and
1071	              A. Gulko, "IGP Flexible Algorithm", draft-ietf-lsr-flex-
1072	              algo-12 (work in progress), October 2020.

[major] This reference should be Normative.

##PP2
done

...
1080	   [RFC8402]  Filsfils, C., Ed., Previdi, S., Ed., Ginsberg, L.,
1081	              Decraene, B., Litkowski, S., and R. Shakir, "Segment
1082	              Routing Architecture", RFC 8402, DOI 10.17487/RFC8402,
1083	              July 2018, <https://www.rfc-editor.org/info/rfc8402>.

[major] This reference should be Normative.

##PP2
done

[End of Review]

---

From: Alvaro Retana <[email-masked]>
Date: Tue, 30 Mar 2021 12:44:23 -0500

On March 25, 2021 at 6:03:53 AM, Peter Psenak wrote:

Peter:

Hi!

I have some comments (see below) -- nothing major.  I look forward to -12.

Thanks!

Alvaro.

...
> https://mailarchive.ietf.org/arch/msg/lsr/nKJbY5f6SHEwVCqqfoXSPidGKXQ/

Please just say somewhere that END.T is not included -- no need to
justify in the document.

...
> fine with it? We are defining SRv6 Locator in this document.

No.  The description throughout this section talks about both prefixes
and locators.  It reads as a general specification for all prefixes
(including locators).

Suggestion (taking onto account the responses below)>

  A prefix/SRv6 Locator that is advertised by a single node and without
  an A-Flag is considered node specific.

> e.g. derive SID for TILFA protection, etc.

Does this mean that having the same sub-TLV is also a consideration
when comparing the locator and legacy advertisements?   If so, then
that should also be mentioned in §5.

> setting of the embedded X-flag in any advertisement of the same prefix

[nit] s/advertise same flags/advertise the same flags

[nit] s/ 236/237 / 236 and 237

[major] "MUST match the setting of the embedded X-flag"

What if the setting doesn't match?  What should the receiver do?

By "embedded", you are referring to the X bit inside 236/237, right?
To differentiate, please call it "X bit" and add references to
rfc5120/5308.

...
> s that really required? I have never done it in any of RFCs I was editor

Required, no...but it is good practice.

...
> and SID addressing in the domain. Details are outside the scope of this

Can you please add something like that to the draft?  I think that for
now (unless other people ask as well) simply saying that the use is
outside the scope should be enough.

...
> removed END.OP. This section is going to be removed anyway.

That fact doesn't mean that the information in it can be wrong.  If
inaccurate, I would prefer the section not be there at all.

...
> Would that be sufficient?

We need the header to include the Updates tag, and something in the
Abstract and Introduction.

In the Abstract>
   This documents updates rfc7370 by modifying an existing registry.

The text in Introduction can be the same, with a reference to §12.1.2.

This Section should also ask IANA to add this document as a reference
in the regsitry.

...
> "Sub-Sub-TLVs for SID Sub-TLVs (SRv6 End SID, SRv6 End.X SID

Most of the other registries include the number of the TLV.  So I
think it would be good to remain consistent.  Maybe we should ask the
current DEs: Chris, Hannes and Les.

[End]

---

From: "Les Ginsberg (ginsberg)" <[email-masked]>
Date: Wed, 31 Mar 2021 05:01:37 +0000

Alvaro/Peter -

> current DEs: Chris, Hannes and Les.

I think this is an odd situation.
You have three new sub-TLVs:

SRv6 End SID sub-TLV(5) which is a sub-TLV of TLVs 27, 135, 235, 236 and 237 (only allowed in TLV 27)

SRv6 End.X SID(43) and SRv6 LAN End.X SID(44) sub-TLVs which are sub-TLVs of TLVs 22, 23, 25, 141, 222 and 223 (allowed in all TLVs)

You propose to create a single registry common to all three of these sub-TLVs to share codepoints for sub-sub-TLVs. This is motivated by the one defined sub-sub-TLV SRv6 SID Structure Sub-Sub-TLV(1) which is applicable to all three new sub-TLVs.

Such a registry  (code points for sub-sub-TLVs associated with sub-TLVs in different parent TLVs) has not been defined before. Closest we have come is the "Sub-sub-TLV Codepoints for Application-Specific Link Attributes" which exists separately from " sub-TLVs of TLVs 22, 23, 25, 141, 222 and 223" but comes with the advisory:

"If a link attribute can be advertised both as a sub-TLV of TLVs
22, 23, 25, 141, 222, and 223 and as a sub-sub-TLV of the 
Application-Specific Link Attributes sub-TLV defined in 
[RFC8919], then the same numerical code
should be assigned to the link attribute whenever possible."

You could elect to do the same thing here i.e., create two new registries but include a Note saying codepoints in the two should be the same whenever it makes sense to do so.
Or, you could do something new and create a single registry even though the grandparent TLVs are different.

I don’t have a strong opinion here, but if it is anticipated that most of the sub-sub-TLVs would be shared then a single registry is easier to manage.

The name however becomes quite a mouthful - something like:

"sub-sub-TLVs for SRv6EndSID(5) (sub-TLV of TLVs 27, 135, 235, 236 and 237) and SRv6 End.X SID(43)/SRv6 LAN End.X SID(44) (sub-TLVs of TLVs 27, 135, 235, 236 and 237)"

And you would need to create columns (analogous to the columns in existing registries which have multiple parent TLVs) to show in which parents a given sub-sub-TLV is allowed.

   Les

---

From: Peter Psenak <[email-masked]>
Date: Wed, 31 Mar 2021 10:22:57 +0200

Hi Les,

> I don’t have a strong opinion here, but if it is anticipated that most of the sub-sub-TLVs would be shared then a single registry is easier to manage.

single one is preferred.

> "sub-sub-TLVs for SRv6EndSID(5) (sub-TLV of TLVs 27, 135, 235, 236 and 237) and SRv6 End.X SID(43)/SRv6 LAN End.X SID(44) (sub-TLVs of TLVs 27, 135, 235, 236 and 237)"

I'm fine with that.

> And you would need to create columns (analogous to the columns in existing registries which have multiple parent TLVs) to show in which parents a given sub-sub-TLV is allowed.

sure.

Peter