Hi authors,
I'm your shepherd for the next stage of the journey. As part of that
process, I am doing a review during working group last call to be ready
to do the shepherd write-up. I found a number of issues and nits which
will require some discussion and a new revision.
Best,
Adrian
===
Author count. As you all know, the RFC Editor has a front page limit of
five authors unless the IESG can be persuaded to vary that. The shepherd
is expected to provide *good* reasons why a document has more than five
front-page authors.
The options here are:
1. Reduce the count to 5 by moving some to Contributors. Perhaps you
could limit to one per company.
2. Drop to just the Editor on the front page and move everyone else to
Contributors.
3. Come up with a really good reason that will convince me, and I will
try to convince the AD.
---
Are you really updating 8231 and 8281?
An update would mean that an implementation of either of those RFCs MUST
include what is in this document.
I think it is probable that you are simply defining a new tool that an
implementation may choose to include. This view is supported by the fact
that you have defined a mechanism to discovery whether the tool is
supported.
---
The Abstract got me quite worried because PCEP, as defined in 5440,
allows a PCE to supply multiple paths.
Section 6.5
The PCRep message may also contain multiple
acceptable paths corresponding to the same request.
...and then...
If the path computation request can be satisfied (i.e., the PCE finds
a set of paths that satisfy the set of constraints), the set of
computed paths specified by means of Explicit Route Objects (EROs) is
inserted in the PCRep message. The ERO is defined in Section 7.9.
The situation where multiple computed paths are provided in a PCRep
message is discussed in detail in Section 7.13. Furthermore, when a
PCC requests the computation of a set of paths for a total amount of
bandwidth by means of a LOAD-BALANCING object carried within a PCReq
message, the ERO of each computed path may be followed by a BANDWIDTH
object as discussed in section Section 7.16.
It was only when I got to "[RFC9862] specifically avoids defining how to
signal multiple Segment Lists" that it became clear that you are talking
about PCEP for SR.
I think your Abstract could use a little cleaning up. And the document
title is probably wrong as well.
The nub of the problem comes in section 2.1. It is not that multiple
EROs cannot be present on a PCRep; the problem is with the definition of
"path" and "LSP" in SR. We're not going to fix any of that (although,
IMHO, it would be really neat if we could), so what this document needs
to do, up front and in the Introduction, is set out the terminology...
- Path
- LSP
- Segment list
- ERO
Then everything else will fall into place.
You should also check through the rest of the text (especially the
Introduction) for places where it says "PCEP" but means "PCEP for SR"
or "multipath" where it means "SR multipath".
---
3.2
Why does this text use "SHOULD" and not "MUST"? Under what circumstances
would this not be the case (i.e., what is the missing "MAY")?
But, anyway, it reads very odd so say something about per PATH metrics
and then immediately say per-path metrics are out of scope.
Why do you mix "PATH" and "path"?
---
3.4
Why not move the definition of the MULTIPATH-BACKUP TLV to [I-D.draft-
ietf-pce-sr-p2mp-policy] since that appears to be the only use case for
it at the moment and since you are specifically ruling out P2P at this
time?
And given that you are ruling out P2P, why do you only say the an
implementation "SHOULD" reject a MULTIPATH-BACKUP TLV applied to a P2P
path?
On the other hand, I fail to see why p2p is such a hard case that has to
be excluded here. After all p2p is just p2mp is the special case of a
single destination. The example in 6.2 seems to work for p2p.
---
3.4
It would be really helpful to what the Backup Path IDs match against.
Where are the identifiers of the backup paths found?
---
3.5
New MULTIPATH-OPPDIR-PATH TLV is optional in the PATH-ATTRIB object.
Multiple instances of the TLV are allowed in the same PATH-ATTRIB
object. This TLV encodes a many-to-many mapping between forward and
reverse paths.
I don't think this TLV encodes a many-to-many mapping because it is
present in the PATH-ATTRIB object of a single path, and encodes a single
Opposite Direction Path ID. So, the TLV encodes a relationship between
two paths, one in each direction.
You cover this nicely, later, with:
Multiple instances of this TLV present in the same PATH-ATTRIB object
indicate that there are multiple opposite-direction paths
corresponding to the given path. This allows for many-to-many
relationship among the paths of two opposite direction LSPs.
---
3.5
I think you need some commentary on the non-setting of bits N and L.
In particular, not setting N does not mandate that the two paths use
different nodes: they may use the same or different nodes.
You might also note that setting L implies that the two paths also use
the same set of nodes, but it is not necessary to set N when L is set.
---
3.5
You describe the requirement that if path a shows an opposite direction
relationship with path b.
But (presumably) there is the possibility of setting up paths one at a
time. How does that work?
---
3.5
I wondered, given that the draft rules out using the backup processing
for p2p paths, whether you should rule out using the reverse path
association for p2mp paths. It is certainly the case that p2mp reverse
path is a complex issue.
---
3.6
To signal the Composite Candidate Path, we make use of the COLOR TLV,
defined in [RFC9863]. For a Composite Candidate Path, the COLOR TLV
is included in the PATH-ATTRIB Object, thus allowing each Composite
Candidate Path to do ECMP/W-ECMP among SR Policies identified by its
constituent Colors. Only one COLOR TLV MUST be included into the
PATH-ATTRIB object. If multiple COLOR TLVs are contained in the
PATH-ATTRIB object, only the first one MUST be processed and the
others MUST be ignored.
As written, the use of "MUST" is a little confusing.
Are you saying that it is a requirement that a COLOR TLV is present in
the PATH-ATTRIB object?
I think you can safely delete the "Only one..." sentence.
And you can change the second sentence to:
If multiple COLOR TLVs are contained in the
PATH-ATTRIB object, the first one is processed and the others MUST be
ignored.
---
3.6.1
Is there any guidance about the value of the FC field? Is it
---
4.1.1
When PCE computes the LSP path, it MUST NOT return more forward
multipaths than the corresponding value of "Number of Multipaths"
from the MULTIPATH-CAP TLV. If this TLV is absent (from both OPEN
and LSP objects), then the "Number of Multipaths" is assumed to be 1.
The value the PCE sent, or the value the PCC sent? Or both?
And what if it does?
---
5.
I wanted to check that the RBNF you present is really what you want. You
have:
<intended-path> ::= (<ERO>|
(<PATH-ATTRIB><ERO>)
[<intended-path>])
This is ambiguous because you can't see whether the repeated
<intended-path> happens only if (<PATH-ATTRIB><ERO>) is chosen.
I think you probably mean:
<intended-path> ::= ( <ERO> | (<PATH-ATTRIB><ERO>) )
[<intended-path>]
If so, you could write:
<intended-path> ::= [<PATH-ATTRIB>]<ERO>
[<intended-path>]
The same for <actual-path>
---
5.
Each path is preceded by a PATH-ATTRIB object that
describes it.
According to the RBNF (whether you make my change or not) this should
read:
Each path may be preceded by a PATH-ATTRIB object that
describes it.
---
6.
I believe your examples should use addresses from the ranges reserved
for documentation. That is 2001:db8::/32 and 3fff::/20
---
6.1
Consider the following sample SR Policy, taken from
[RFC9256].
I don't find that sample in 9256.
---
6.1
It would be useful to refer to 8231 for <state-report>
---
The inclusion of the Implementation Status section is appreciated
(although it is pretty thin). Obviously, the early assignments done by
IANA has made this a lot easier. Given that there are some code points
still marked as TBDx, I wonder how the "full" implementations have been
achieved.
---
8.1 correctly asks for IANA to confirm the assignment of object-class 45
and the object-type 1. However, it should also describe the other
object-type values shown in the registry.
---
== Nits
1.
There are no PCEP standards, so probably s/standards/specifications/
---
2.3
s/path(s)/paths/
---
3.1
"several ERO/Recorded Route Object (RRO) objects"
"ERO/RRO objects"
Seems to be tautology
---
3.1
The text should directly reference "Figure 1"
---
3.1
Missing description of "Optional TLVs"
---
3.3
You should probably say that Weight is an unsigned integer
---
3.4
s/path(s)/paths/ (four times)
s/ID(s)/IDs/
s/identifier(s)/identifiers/
---
4.3
Expand PLSP on first use. Does it also need a reference?
---
4.3
Steps 2 and 3 use the passive voice. It would be helpful to say which
component is responsible for each action.
---
_______________________________________________
Pce mailing list -- [email protected]
To unsubscribe send an email to [email protected]
