Segment Routing over IPv6 (SRv6) Network Programming
draft-ietf-spring-srv6-network-programming-28

Note: This ballot was opened for revision 18 and is now closed.

Martin Vigoureux Yes

Deborah Brungard No Objection

Alissa Cooper No Objection

Roman Danyliw (was Discuss) No Objection

Comment (2020-09-28 for -22)
No email
send info
Thanks for responding to the SECDIR review (and thank you to Brian Weis for doing it)

Thanks for addressing my DISCUSS and COMMENTs.

===[ remaining COMMENTs
** Section 4.1.  Pseudo-code without formal definition might be open to interpretation.  My nits (which likely apply to other sections) are that:
-- it might not be clear that S03, S06 and S10 are effectively returns/exits from this “behavior”, and if they are run, S12-S15 should never be reached (otherwise Segments Left or Hop Limit = -1)

-- (for consistency) the IF … ELSEIF … END construct is used in Section 4.8, but not here.

Martin Duke No Objection

Comment (2020-09-14 for -18)
Please define RIR on first use.

Erik Kline (was Discuss) No Objection

Comment (2020-10-29 for -24)
I think my main comments were addressed between -20 and -24; thanks.

Barry Leiba No Objection

Comment (2020-09-21 for -19)
Throughout the document:
It should be “an SID”, “an FIB”, “an RIR”, and some others, not “a”, because one reads these as “ess-eye-dee” and “eff-eye-bee”, not as the expansions thereof.

— Section 3.1 —

   An SRv6 endpoint behavior MAY require additional information

I think this is a statement of fact, so “may” (or “might”), not “MAY”.

— Section 3.2 —
Nit: The plural of “SID” is “SIDs”, without an apostrophe.

   The provider historically deployed IPv6 and assigned
   infrastructure address from a portion of the fc00::/7 prefix

Nit: “addresses”.

   In another example, a large mobile and fixed line service provider

Nit: hyphenate “fixed-line”.

   IPv6 address consumption in both these examples is minimum,

Nit: “minimal” (also, put a comma before “respectively”).

   A remote node uses the IANA behavior codepoint to map the received
   SID (B:N:FUNCT) to a behavior.

I don’t know what “IANA behavior codepoint” means.  As the rest of the sentence makes it clear that this is mapping to “a behavior”, maybe it’s better to say “registered codepoint”, or some such?  Or, as you say a couple of paragraphs down, “Endpoint Behavior codepoint”?  In any case, please be consistent about “IANA behavior”, “IANA Behavior”, and “IANA Endpoint Behavior”.  My preference would be to avoid saying “IANA” in these, but use your judgment on what’s most understandable and clear.

   o  Assign an SRv6 Locator 2001:db8:bbbb:3::/64 to the Router 3 in
      their SR Domain

What is “the Router 3” (and router 4)?  There’s no introduction nor diagram that says.  Also, please be consistent in capitalization of these.

— Section 3.3 —

   routing protocol specific aspects that are outside the scope of this

Nit: “routing-protocol-specific” needs to be hyphenated.  As that’s awkward, you might want to reword to avoid it, “are specific to the routing protocol and are outside the scope....”

— Section 4 —

   The pseudocode describing these behaviors detail local processing

Nit: “details”, singular, to match “pseudocode”.

— Section 4.1 —

   The End behavior operates on the same FIB table (i.e.  VRF, L3 relay
   id) associated to the packet.

Is “i.e.” really what you want here?  How does “VRF, L3 relay” equate to “FIB table”?

— Section 4.16.1.3 —

   segments left to 0.  The SDN controller knows that no-other node

Nit: “no other” should not be hyphenated.  For that matter, the word “other” isn’t needed anyway, because you have “after” in there.

   -as part of the decapsulation process the egress PE is required to
    terminates less bytes from the packet.

I can’t figure this out.  It looks like it should be “required to terminate”, but I don’t know what it means to “terminate less bytes”.  Can you reword this?

    to the lookup engine in the forwarding ASIC.

“ASIC” needs to be expanded.  As this is the only place you use it, I suggest just using the expansion and not bothering with the abbreviation.

— Section 7 —

   This document introduces SRv6 Endpoint and SR Policy Headend
   behaviors for implementation on SRv6 capable nodes in the network.
   As such, this document does not introduce any new security
   considerations.

I’m not convinced of this.  It seems that misuse (such as injection or alteration) of some of these Behaviors could, for example, result in packets being forwarded to nodes they were not intended to go to.  Is it not important to discuss issues such as that: how these Behaviors might be attacked?  Is that really fully covered in 8754 and 8402?

— Section 8.1 —

   The presence of SIDs in the IGP do not imply any routing semantics

Nit: “does not”, to match the singular subject “the presence”.

   an IPv6 address is solely governed by the, non-SID-related, IGP

Nit: remove both commas.

   not governed neither influenced in any way by a SID advertisement

Nit: make it “neither governed nor influenced”

   build TI-LFA [I-D.ietf-rtgwg-segment-routing-ti-lfa] based FRR
   solutions

Nit: there needs to be a hyphen in there, but the citation gets in the way.  Make it, “build FRR solutions based on TI-LFA [I-D.ietf-rtgwg-segment-routing-ti-lfa]”.

— Section 8.4 —

   For example, a BGP-LS advertisement of H.Encaps behavior would
   describe the capability of node N to perform a H.Encaps behavior,
   specifically it would describe how many SIDs could be pushed by N
   without significant performance degradation.

This is a comma splice.  Split the sentence before “specifically” (and put a comma after “specifically”).

Alvaro Retana (was Discuss) No Objection

Comment (2020-10-07 for -24)
No email
send info
[Thanks for addressing my DISCUSS points.]

Éric Vyncke No Objection

Comment (2020-09-22 for -19)
Thank you for the work put into this document.

Thank you also for having edited the document based on the INT-DIR review by Brian Haberman (thank you Brian for the detailed review!) and replying to Brian's review.
https://datatracker.ietf.org/doc/review-ietf-spring-srv6-network-programming-18-intdir-telechat-haberman-2020-09-14/

Please find below a couple of non-blocking COMMENT points and nits.

I hope that this helps to improve the document,

Regards,

-éric

== COMMENTS ==
-- Section 3 --
There is some text copied from section 4.3 from RFC 8754. But, even if it seems obvious that the behaviors specified in this document _ONLY_ apply when "A FIB entry that represents a locally instantiated SRv6 SID", I suggest to spell it out to avoid any ambiguity.

-- Section 3.2 --
I am a little ambivalent with Brian's point about the location of the given examples. On one hand, those real case examples of SID allocation are really helpful to understand the impact of allocation; on the other hand, it is usual to move examples in appendix. Curious to see if other IESG members have other point of view.

In the last short example (this one could stay in the current location), should the value of F and A be stated? They are obvious from the used SID value of course, but, let's be complete ?
    
-- Section 3.3 --
Please follow RFC 5952 and write all IPv6 in lowercase.

-- Section 4.2 and 4.3 --
As "End.X" and "End.T" behaviors are variants of the "End" behavior, does the section 4.1.1 (Upper-Layer Header) also apply ?

-- Section 4.9 --
For consistency with previous behavior descriptions of End.DT[46], I suggest to mention that 143 has been reserved by IANA.

== NITS ==
-- Section 2 --
In the bullet list, there are a couple of missing '.' at the end of the list items.

-- Section 3 --
"longest-prefix-match lookup on the packets destination address" "packets" should probably be singular.

In "IPv6 subnet allocated for SRv6 SIDs by the operator", should it rather be "prefix" than "subnet" ?

-- Section 3.2 --
Strongly suggest to be consistent with the use of "IANA Endpoint Behavior codepoint" as sometimes it is shortened into "IANA Behavior" (also unsure whether the "IANA" qualification is really useful here)

-- Section 4.2 --
"one for the bundle(LAG)" Link Aggregation has not been expanded before and I am not sure whether the 'LAG' adds anything to the paragraph.

-- Section 4.16 --
Should PSP, USP, USD be expanded in the short introduction of this section ?

-- Section 8 --

The usual location of the security considerations is after all specification section, so, it should be after the 'control plane' section.

Robert Wilton No Objection

Comment (2020-09-22 for -19)
Hi,

Thank for this document.  Not surprisingly given the large number of contributors working on this document I only have a few minor comments:


    2.  Terminology

       NH: Next-header field of the IPv6 header [RFC8200].  NH=SRH means
       that the next-header of the IPv6 header is Routing Header for
       IPv6(43) with the Type field set to 4.
   
Suggest expanding "4" to SRH(4)


    10.1.  Ethernet Next Header Type

       This document requests IANA to allocate, in the "Protocol Numbers"
       registry (https://www.iana.org/assignments/protocol-numbers/protocol-
       numbers.xhtml), a new value for "Ethernet" with the following
       definition: The value 143 in the Next Header field of an IPv6 header
       or any extension header indicates that the payload is an Ethernet
       [IEEE.802.3_2012].

Should this point to the latest published version of 802.3 (2018), or does it specifically need to reference an older version?

   
    3.1.  SID Format

       This document defines an SRv6 SID as consisting of LOC:FUNCT:ARG,
       where a locator (LOC) is encoded in the L most significant bits of
       the SID, followed by F bits of function (FUNCT) and A bits of
       arguments (ARG).  L, the locator length, is flexible, and an operator
       is free to use the locator length of their choice.  F and A may be
       any value as long as L+F+A <= 128.  When L+F+A is less than 128 then
       the remainder of the SID MUST be zero.

This was reasonably clear to me, but since the rest of the description refers to bits, then I wonder whether the last sentence would be more precise as "the remaining bits of the SID must be set to zero"?


    4.1.  End: Endpoint
       S09.   If ((Last Entry > max_LE) or (Segments Left > Last Entry+1)) {

It wasn't entirely apparent to me where 'Last Entry' comes from, although that became more clear when I saw its usage later.  Possibly it would be helpful to list "Segments Left" and "Last Entry" in the terminology as taken from RFC 8754?


    4.1.  End: Endpoint
       S12.   Decrement Hop Limit by 1
       
Would it be more clear to expand this to say "Decrement IPv6 Hop Limit by 1", since it is referred to as the IPv6 Hop Limit previously.  If this is changed, then there are a couple of other places in the document that should be checked as well.

   
    4.2.  End.X: Layer-3 Cross-Connect

       Note that if N has an outgoing interface bundle I to a neighbor Q
       made of 10 member links, N may allocate up to 11 End.X local SIDs:
       one for the bundle(LAG) itself and then up to one for each Layer-2
       member link.

This behaviour seems slightly unusual to me.  I presume that the intent here is for the SR program to control which member link the traffic travels over rather than the normal behaviour of relying on the Bundle hashing.  Possibly adding a bit more text to clarify the expected behaviour might be helpful.


    4.4.  End.DX6: Decapsulation and IPv6 Cross-Connect

       S05.  If the End.DX6 SID is bound to an array of L3 adjacencies, then
       one entry of the array is selected based on the hash of the packet's
       header Section 7.

The end of this sentence doesn't scan for me, probably it should be something end something like: ", see Section 7"


    6.  Counters

       A node supporting this document SHOULD implement a combined traffic
       counter (packets and bytes) per local SID entry, for traffic that
       matched that SID and was processed correctly.  The retrieval of these
       counters via MIB, NETCONF/YANG or any other means is outside the
       scope of this document.
   
From my reading, a combined counter implies a single counter that is incremented by both the number of packets processed and number of bytes processed.  Normally in the management models you would normally expect this to be modeled as a pair of counters, and I presume that is what is desired here?  If so I would suggest clarifying this text to indicate that it is a pair of counters (one for packets, one for bytes) that are required here.


    Section 4, various places.

At various points the Upper-Layer Header type is compared to a number (e.g. 4, 41, 143).  It might be a bit more readable if the associated names were also included in the description.  E.g., 4(IPv4), 41(IPv6), etc.


    4.10.  End.DX2V: Decapsulation and VLAN L2 Table Lookup

Is this lookup just on the outermost VLAN Id, or all VLAN tags in the exposed Ethernet header?  Or is this implementation specific?

Regards,
Rob

Benjamin Kaduk (was Discuss) Abstain

Comment (2020-12-28 for -27)
No email
send info
The -27 resolved my Discuss point from the -26; thank you.

I retain my other comments from the -26 unchanged, below, for posterity.

%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%

[note: I originally prepared these comments when looking at the -24.  I
tried to remove comments about things fixed in the -25 or -26, but may
have missed a couple; please ignore any such already-addressed comments.
There are also a couple points that we have already been discussing in
the other thread but I retain as a "placeholder"; hopefully we can keep
the actual discussion about them in just one place.]

As mentioned in the Discuss section, I did a lot of background reading
while preparing this updated ballot position.  Another thing I noticed
while doing that reading is that the pseudocode in
https://tools.ietf.org/html/rfc8754#section-4.3.1.1 explicitly mentions
"perform TLV processing"; we might consider repeating that step in our
pseudocode procedures, and otherwise making our procedures as analogous
as possible to the RFC 8754 procedures, just from the perspective of
keeping the writing style as consistent as possible across the related
documents.  However, that's entirely an editorial matter and thus left
to the discretion of the authors/AD.

Section 2

   The following terms used within this document are defined in
   [RFC8754]: SRH, SR Source Node, Transit Node, SR Segment Endpoint
   Node, Reduced SRH, Segments Left and Last Entry.

It's slightly unfortunate that 8754 didn't have a dedicated terminology
section containing these, though it's too late to really do anything
about it now.

Section 3

   The term "function" refers to the bit-string in the SRv6 SID.  The
   term "behavior" identifies the behavior bound to the SID.  The
   behaviors are defined in Section 4 of this document.

(nit) using "the behaviors" to some extent implies that these are the
only ones allowed or defined, which is not true.  Perhaps "some
behaviors" would be more accurate (or some other phrasing would also
cover the expected evolution of the ecosystem)?

Section 3.3

   A packet could be steered to a non-routed SID 2001:db8:b:2:101:: by
   using a SID list <...,2001:db8:b:1:100::,2001:db8:b:2:101::,...>
   where the non-routed SID is preceded by a routed SID to the same
   node.  Routed and non-routed SRv6 SIDs are the SRv6 instantiation of
   global and local segments, respectively [RFC8402].

If it's (also) possible to steer a packet to a non-routed SID without a
preceding routed SID for the same node (e.g., via End.X), it seems like
that might be worth listing an example of as well.  Otherwise a reader
might assume that the global segment is a necessary part of using the
non-routed SID.

Section 4

  End.DT2U           Endpoint with decaps and unicast MAC L2table lookup
                     e.g. EVPN Bridging unicast use-case

(nit) we seem to put a space in "L2 table" the other times it appears.

   The list is not exhaustive.  In practice, any function can be
   attached to a local SID: e.g. a node N can bind a SID to a local VM
   or container which can apply any complex processing on the packet.

(nit) the preceding list was a list of well-known behaviors, not a list
of functions.  IIUC, it is more appropriate to use "behavior" here than
"function", since the "function" is just the opaque bitstring.

Section 4.1.1, ...

   When processing the Upper-layer Header of a packet matching a FIB
   entry locally instantiated as an SRv6 End SID do the following:

(editorial, I think) I find it interesting to compare the phrasing here
to what was used in §4.1 (when processing an SRH), where the text is
"receives a packet whose IPv6 DA is S and S is a local End SID".  Why
the distinction between "End SID" and 'SRv6 End SID"?  IIUC the
distinction between checking "IPv6 DA" and "matching a FIB entry locally
instantiated" is important and the language should not be harmonized
between occurrences.
The "..." in the Section number listing indicates that this (or similar)
phrasing appears throughout, whenever we talk about processing an
upper-layer header.

Section 4.2

   Note that if N has an outgoing interface bundle I to a neighbor Q
   made of 10 member links, N may allocate up to 11 End.X local SIDs:
   one for the bundle itself and then up to one for each Layer-2 member
   link.  The flows steered using the End.X SID corresponding to the

(nit) I think that while "up to 11" might be the situation that makes
the most sense (in that having many distinct subgroups with 1 < n < 10
member links doesn't make sense), it is not strictly speaking a physical
or protocol requirement.  Perhaps "might allocate 11" is better than
"may allocate up to 11" for that reason.

Section 4.4, ...

   When N receives a packet destined to S and S is a local End.DX6 SID,
   N does the following processing:

(nit) we have a mismatch of "N does the following processing" (like
appears here) and "N does" or similar; it is probably worth normalizing
on one phrasing.

   When processing the Upper-layer header of a packet matching a FIB
   entry locally instantiated as an SRv6 End.DX6 SID, the following is
   done:

Similarly here, we use "the following is done" but the "N does the
following" phrasing used in some other sections is probably preferred,
as it avoids the passive voice.

Section 4.12

We might give some mnemonic explanation for how the name "FE2" was
chosen to identify the argument value.

   table T flooding.  The allocation of the argument values is local to
   the SR Endpoint Node instantiating this behavior and the signaling of
   the argument to other nodes for the EVPN functionality via control
   plane.

nit(?): s/via control plane/occurs via the control plane/?

Section 4.13

  S05.   If (IPv6 Hop Limit <= 1) {
  S06.       Send an ICMP Time Exceeded message to the Source Address,
               Code 0 (Hop limit exceeded in transit),

(nit) the indentation seems off by one space in the first line of S06
(it doesn't match the other two places where this chunk occurs).

   S14.  The SRH MAY be omitted when the SRv6 Policy B only contains one
   SID and there is no need to use any flag, tag or TLV.
   S17.  The Payload Length, Traffic Class, Hop Limit and Next-Header
   fields are set as per [RFC2473].  The Flow Label is computed as per
   [RFC6437].

(These look to be S15 and S18, respectively, now.)

Section 4.14

   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.

(nit) it's probably worth harmonizing the phrasing between here and the
note on S15 in §4.13 (specifically, "only contains one SID" vs "only
contains one segment").

Section 4.15

(nit) there's a blank line at the end of S06 that doesn't occur in the
other two locations where this pseudocode appears.

  S16.   Submit the packet to the MPLS engine for transmission to the
            topmost label.

(nit) I suggest rewording slightly so as to not imply that the node to
transmit to is determined by the topmost label -- IIUC it's determined
by the MPLS policy, since the interpretation of the label is in general
local to the receiving node.

Section 4.16.1.2

   As a reminder, [RFC8754] defines in section 5 the SR Deployment Model
   within the SR Domain [RFC8402].  Within this framework, the
   Authentication Header (AH) is not used to secure the SRH as described
   in Section 7.5 of [RFC8754].

(repeating from the previous thread as a placeholder) I think we need
another sentence or clause here to clarify why this statement is
relevant, e.g., "Thus, while the AH can detect changes to the IPv6
header chain, it will not be used in combination with the SRH, so use of
PSP will not cause delivery failure due to AH validation checks."

Section 5

(editorial) This is the first place in the document that we talk about
the "headend" or its policy at all, so a bit of background on why it's
useful to tabulate potential headend policy/behaviors might be helpful.

Section 5.x

Tying the other policies more precisely to the pseudocode for H.Encaps
(e.g., replacing S01) seems like it would be useful, to avoid the
appearance of specifying behavior by appealing to examples.

Section 5.1

   Note:
   S03: As described in [RFC6437] (IPv6 Flow Label Specification).

We need to pull in RFC 2473 for payload length, traffic class, and
next-header, IIUC.  (hop-limit is covered a few paragraphs down.) Also
to say how the next-header value is selected.

Section 8.1

   The presence of SIDs in the IGP does not imply any routing semantics
   to the addresses represented by these SIDs.  The routing reachability
   to an IPv6 address is solely governed by the non-SID-related IGP
   prefix reachability information that includes locators.  Routing is
   neither governed nor influenced in any way by a SID advertisement in
   the IGP.

It seems like this is trying to say "the IGP must not advertise prefixes
contained within the LOC part of an SID", but in a very roundabout way.

Section 8.3

   The End.DX4, End.DX6, End.DT4, End.DT6, End.DT46, End.DX2, End.DX2V,
   End.DT2U and End.DT2M SIDs can be signaled in BGP.

Since we said earlier that the signaling of SIDs needs to include the
behavior codepoint for each function bitstring, it seems like we should
provide a reference to how BGP will encode the behavior codepoint.

Section 9

There seem to be some security considerations relating to the use of
PSP, in that the egress node loses visibility into which policy was used
for a given packet, so all packets from all policies that egress via
that SID are in the same anonymity (and policy!) set.  In particular,
even if an HMAC TLV was present in the SRH, it is not available and
cannot be validated.  I recognize that the headend (or whatever entity
is assigning SR policy) should know when such validation would be
intended to occur and not assign a policy incompatible with it, but
there are still new considerations in the sense that the headend needs
to be aware of these considerations.

(repeating as a placeholder from the previous thread) I think we should
also say that in the absence of the HMAC TLV, valid FUNC and ARG values
on any given node may be guessable and spoofable, along with the
standard disclaimer that risks are minimal since all nodes in the SR
domain are assumed to be trusted.  This is distinct from the
already-extant ability to spoof a SID in that the underlying structure
in the SID may allow the attacker to induce behavior that was never
intended to be a SID, for example if the implementation logically
separates FUNC and ARG processing and the attacker makes a combination
that was never advertised.

Also, IIUC, the "Segments Left == 0" handling for, e.g., End.X is
important to prevent traffic loops -- if a node fails to perform that
check and blindly sends the packet to the interconnect it will get
returned to that node/SID and loop until the IP hop limit is exhausted.

Murray Kucherawy Abstain

Warren Kumari Abstain

Magnus Westerlund (was No Objection) Abstain