[Lsr] FW: A review of draft-zhu-lsr-isis-sr-vtn-flexalgo-04

[Adrian Farrel <[e-mail]>]

Hi LSR and draft authors,

I read this draft, and it seems to me that it provides a useful transitional
mechanism. It can obviously only support a relatively small number of VTNs
(128 due to the limited number of Flex-Algos the network devices can
support), but it looks to be a worthwhile first step because it can be
achieved with a very minor control plane extension.

Perhaps this document is a good first step while we work on a longer term
and more scalable control plane solution. It would also give us the chance
to determine the level of interest in VTNs.

My comments, below, are mainly editorial, but there are a couple of issues
around the use of the E flag.

Best,
Adrian

(PS. For those of you saying "What's a VTN?" the "Virtual Transport Network"
is a network construct described in the TEAS WG to support Enhanced VPNs
(https://datatracker.ietf.org/doc/draft-ietf-teas-enhanced-vpn/) and network
slicing
(https://datatracker.ietf.org/doc/draft-ietf-teas-ietf-network-slices/)
where it maps to a "Network Resource Partition".)

===

As currently defined, I think this document needs to update RFC 8668. This
is because that RFC says that other flags in the flag field of the Parent L3
Neighbor Descriptor in the L2 Bundle Member Attributes TLV "MUST be
ignored".

That's easy enough to handle:
- You add the "updates 8668" element to the XML
- You add a final paragraph to the Abstract to say
    This document updates RFC 8668 by defining a new flag in the Parent
    L3 Neighbor Descriptor in the L2 Bundle Member Attributes TLV.
- You add a final paragraph to the Introduction to say
    This document updates [RFC8668] by defining a new flag in the Parent
    L3 Neighbor Descriptor in the L2 Bundle Member Attributes TLV. 
    [RFC8668] states that all bit fields not defined in that document
    "MUST be set to zero on transmission and ignored on receipt".  
    Section 3 of this document defines a new flag and specifies both
    when it is set and how it should be processed.

However, a purist might point out that RFC 8668 should be fixed so that:

- The unused bits are defined as reserved for future use
- The text should be updated to describe how the bits are set and handled
  by implementations that don't understand them
- There should be an IANA registry for the bits

You're not responsible for fixing RFC 8668, but you could if you want to.

Making this document an "update" is also important because of the absence of
an IANA registry -- it is important to provide a way for people to track the
bits so that there is no collision when another bit is defined.
    
You could use definitely use this document to create the necessary IANA
registry, even if you are not fixing the other parts of 8668.

---

Would be worth expanding "VTN" in the title to read...
  Using Flex-Algo for Segment Routing based Virtual Transport Networks

---

Abstract

The first mention of "Flex-Algo" needs to have some explanation of the
concept. Not many words, but something like...

OLD
   The topological constraints of a VTN can be defined using Flex-Algo.
NEW
   The topological constraints of a VTN can be defined using Flex-Algo,
   a mechanism to provide distributed constraint-path computation.
END

---

Abstract

"SR" should be spelled out as "Segment Routing (SR)"

---

Abstract

s/L2 bundle/L2 bundles/

---

1.

The word "traditional" has other meanings in American English and we are
now asked to avoid using it.

OLD
   than that can be provided with traditional overlay VPNs.
NEW
   than that could be provided with existing overlay VPNs techniques.
END

---

1.

s/resource-aware SIDs/resource-aware segment identifiers (SIDs)/
s/With segment routing/With a segment routing/
s/Segment Identifiers (SIDs) can be used/SIDs can be used/
s/using control plane/using the control plane/
s/scalable Segment Routing (SR)/scalable SR/
s/a unique Flex-Algo/a unique Flex-Algo [I-D.ietf-lsr-flex-algo]/

---

Section 1 has just one sentence on the purpose and content of this
document.

   This document
   describes a mechanism to build the SR based VTNs using SR Flex-Algo
   and IGP L2 bundle with minor extensions.

This text is fine, but rather limited.
I suggest:
- Make it a separate paragraph so that it stands out.
- Add at least two more sentences describing what is found in this
  document. Probably you can just summarise what the mechanism is.
- Describe the purpose and intended use of the mechanism.

---

1.1

The boilerplate here is slightly wrong. Should read...

   The key words "MUST", "MUST NOT", "REQUIRED", "SHALL", "SHALL NOT",
   "SHOULD", "SHOULD NOT", "RECOMMENDED", "NOT RECOMMENDED", "MAY", and
   "OPTIONAL" in this document are to be interpreted as described in BCP
   14 [RFC2119] [RFC8174] when, and only when, they appear in all
   capitals, as shown here.

---

3.

s/can be allocated with a set/can be allocated a set/

---

3.

OLD
   In order to perform constraint
   based path computation for each VTN on network controller and the
   ingress nodes, the resource attribute of each VTN also needs to be
   advertised.
NEW
   In order for a network controller or the ingress nodes to perform
   constraint based path computation for each VTN, the resource
   attributes of each VTN need to be advertised.
END

---

3.

s/resource attribute of the VTN/resource attributes of the VTN/

---

3.

OLD
   The layer-3 link may or may not be a bundle of layer-2 links, as long
   as it has the capability of partitioning the link resources into
   different subsets for different VTNs it participates in.
NEW
   The layer-3 link must have the capability of partitioning the link
   resources into different subsets for the different VTNs it
   participates in.  It may or may not be a bundle of layer-2 links to
   achieve this.
END

---

3.

s/set of link resource allocated/set of link resources allocated/
s/the Parent L3 link are used/the Parent L3 link is used/

---

3.

Add to the paragraph that begins "E flag:" ...

   Note that legacy implementations of [RFC8668] will set the E flag to
   zero (clear) meaning that load balancing among component links is the
   default behavior. Further, when a legacy implementation receives an
   E flag that is set, it will ignore the flag and so will assume that
   load balancing among component links is allowed even when the sender
   has requested it to not be used.

NOTE WELL! If this is not the behaviour you want to see, you need to do
something different with the E flag.

---

3.

   For each virtual or physical layer-2 member link, the TE attributes
   defined in [RFC5305] such as the Maximum Link Bandwidth and Admin
   Groups SHOULD be advertised using the mechanism as defined in
   [RFC8668].

a. You need to say why an implementation might choose to not do this
   (to explain your use of SHOULD), what the consequences would be, and
   what it might do instead.

b. s/[RFC5305]/[RFC5305],/

c. s/Groups/Groups,/

---

3.

   The SR-MPLS Adj-SIDs or SRv6 End.X SIDs associated with
   each of the virtual or physical Layer-2 member links SHOULD also be
   advertised according to [RFC8668] and [I-D.dong-lsr-l2bundle-srv6].

You need to say why an implementation might choose to not do this
(to explain your use of SHOULD), what the consequences would be, and
what it might do instead.

---

3.

   In order to correlate the virtual or physical layer-2 member links
   with the Flex-Algo ID which is used to identify the VTN, each VTN
   SHOULD be assigned with a unique Admin Group (AG) or Extended Admin
   Group (EAG), and the virtual or physical layer-2 member links
   associated with this VTN SHOULD be configured with the AG or EAG
   assigned to the VTN.  The AG or EAG of the parent layer-3 link SHOULD
   be set to the union of all the AGs or EAGs of its virtual or physical
   layer-2 member links.

I think the three instances of "SHOULD" here can be:
s/SHOULD be/is/
s/SHOULD be/are/
s/SHOULD be/is/

---

3.

s/VTN, It/VTN, it/

---

4.

s/For SR-MPLS data plane/For the SR-MPLS data plane/

---

4.

   The Adj-SIDs associated
   with the virtual or physical member links of a VTN MAY be used with
   the prefix-SIDs of the same VTN together to build SR-MPLS TE paths
   with the topological and resource constraints of the VTN.

I recommend s/MAY/can/

Similarly in 

   The
   End.XU SIDs associated with the virtual or physical member links of a
   VTN MAY be used with the SRv6 Locator prefix of the same VTN together
   to build SRv6 paths with the topological and resource constraints of
   the VTN.

---

4.

s/For SRv6 data plane/For the SRv6 data plane/

---

5.

OLD
   which is related to the number of Flex-Algos defined
NEW
   which is related to the maximum number of Flex-Algos supported
END

OLD
   described in [I-D.dong-teas-nrp-scalability].
NEW
   found in [I-D.dong-teas-nrp-scalability].
END