[Pce] draft-ietf-pce-multipath-25 ietf last call Tsvart review

[Vidhi Goel via Datatracker <[e-mail]>]

Document: draft-ietf-pce-multipath
Title: Path Computation Element Communication Protocol (PCEP) Extensions for
Signaling Multipath Information Reviewer: Vidhi Goel Review result: Ready with
Issues

This document has been reviewed as part of the transport area review team's
ongoing effort to review key IETF documents. These comments were written
primarily for the transport area directors, but are copied to the document's
authors and WG to allow them to address any issues raised and also to the IETF
discussion list for information.

When done at the time of IETF Last Call, the authors should consider this
review as part of the last-call comments they receive. Please always CC
[e-mail] if you reply to or forward this review.

and well-motivated, and the encoding fits cleanly into existing PCEP
constructs.

However, there are several normative inconsistencies, two important
internal contradictions, an IANA registry omission, and transport-related
guidance that should be added before publication. None are showstoppers,
but the spec-level bugs (items M1–M4) should be fixed.

---

## Major issues (M)

### M1. Load-balancing ratio wording excludes the referent path
Two places describe the traffic share using "all other", which in English
excludes the path being described and therefore does not describe a valid
distribution (the per-path fractions do not sum to 1).

- §3.3 (Weight field):
> "The fraction of flows that a specific ERO/RRO carries is derived
> from the ratio of its weight to the sum of the weights of **all
> other paths**: see Section 4.3 for details."

- §4.3 step 3:
> "The PCC derives the fraction of flows carried by a specific primary
> path from the ratio of its weight to the sum of **all other
> multipath weights**."

Suggested rewording (matches RFC 9256 §2.11 and standard W-ECMP):

- §3.3:
> "The fraction of flows that a specific ERO/RRO carries is derived
> from the ratio of its weight to the **sum of the weights of all
> paths in the multipath (including this path)**: see Section 4.3 for
> details."

- §4.3 step 3:
> "The PCC derives the fraction of flows carried by a specific primary
> path from the ratio of its weight to the **sum of the weights of all
> paths in the multipath**."

### M2. Contradiction on encoding "no opposite-direction path"
- §3.4 (MULTIPATH-OPPDIR-PATH TLV, "Opposite Direction Path ID"):
  "If no opposite-direction path exists, then this field MUST be set to 0,
  a value reserved to indicate the absence of a Path ID."
- §4.4 step 5: "For paths that have no opposite-direction counterpart, the
  MULTIPATH-OPPDIR-PATH TLV is **omitted** from the PATH-ATTRIB object."

These two normative statements directly conflict — implementers cannot tell
whether to omit the TLV or include it with ID=0. Appendix A.3 (POL1 CP2,
Path ID=2; POL2 CP2, Path ID=2) actually exercises the "include with ID=0"
form, contradicting §4.4 step 5. Pick one and align all three locations.

### M3. Wrong Error-Type for "Conflicting Path ID"
- §4.2: "MUST send PCError message with **Error-Type = 1** ('Reception of
  an invalid object') and Error-Value = 38 ('Conflicting Path ID')."
- §7.3 (IANA): allocation is under **Error-Type = 10** ("Reception of an
  invalid object").

Error-Type 1 in the IANA registry is "PCEP session establishment failure",
not "Reception of an invalid object". The IANA table is correct; §4.2
should read "Error-Type = 10".

### M4. IANA registry for MULTIPATH-OPPDIR-PATH TLV flags is missing bit 13
§7.6 lists:
```
0-12  Unassigned
14    L-flag: Link co-routed
15    N-flag: Node co-routed
```
Bit 13 is unaccounted for. Either extend "0-12" to "0-13" or add an
explicit "13 | Unassigned" row.

### M5. RBNF in §5 updates RFC 8281 but the document does not formally update it
§5 modifies `<PCE-initiated-lsp-instantiation>` from RFC 8281 (and the
`<intended-path>` / `<actual-path>` productions from RFC 8231). Documents
that change normative RBNF of an existing RFC should carry an explicit
`Updates: 8231, 8281` clause in the front matter. If the WG intent is
"extends but does not update", that should be stated and the RBNF changes
should be framed as additive grammar for the new capability path.

### M6. No flow-affinity / reordering guidance (transport concern)
§4.3 phrases the split as "fraction of **flows**", which implicitly assumes
flow-preserving load-balancing (per-5-tuple hashing or similar). This is
critical: per-packet striping across multiple Segment Lists in a Candidate
Path will reorder TCP/QUIC and break performance.

Recommend adding a normative SHOULD that PCC implementations preserve
flow-to-path affinity when distributing traffic across paths within an LSP,
and a non-normative note that per-packet load-balancing is harmful to
transport-layer performance unless paths have near-identical RTT/MTU
characteristics.

### M7. No discussion of asymmetric per-path properties
Paths within a single LSP may have very different MTU, PMTU, latency, and
loss characteristics. Two relevant operational hazards are not addressed:

- **MTU/PMTU divergence:** flow-hashing across paths with different MTUs
  yields per-flow black-holes for the high-MTU packets that happen to be
  hashed onto the low-MTU path. PMTUD then becomes path-specific, which
  hashing breaks. Worth at least an Operational Consideration.
- **Latency divergence:** advertising weighted ECMP across paths with very
  different one-way delay degrades transport performance regardless of
  flow affinity (head-of-line in shared queues, RTT estimation skew when
  paths rebalance, etc.). Worth a note.

### M8. Security Considerations is too thin for what this draft enables
§8 only inherits considerations from the base PCEP RFCs. Multipath
introduces specific new attack surfaces that should be called out:

- **State amplification / DoS.** A misbehaving (or compromised) PCE can
  push an order-of-magnitude more LSP state into a PCC by maximizing
  multipath fan-out. The "Number of Multipaths = 0 (unlimited)" sentinel
  in §4.1.1 makes this worse. Recommend the spec require a non-zero
  configured cap by default and discourage the unlimited value in
  production.
- **Traffic steering manipulation.** A compromised PCE can shape
  weighted-ECMP ratios to drive flows onto attacker-observed paths or
  congest a specific link. Worth one paragraph.
- **Bidirectional mapping abuse.** Forged MULTIPATH-OPPDIR-PATH bindings
  can cause a PCC to install/monitor return paths that don't actually
  carry the return traffic, defeating BFD/PM described in §2.3.

---

## Minor / semantic issues (S)

### S1. Operational state of the LSP vs. per-path O field (§3.1)
The 3-bit O field per path inherits RFC 8231 LSP-object semantics, but the
draft never says how per-path O combines into the LSP-level O state. If 2
paths are UP and 1 is DOWN, is the LSP UP, DOWN, or some new "partially
up" state? Either reference an existing rule or define one.

### S2. O-flag semantics in MULTIPATH-CAP LSP object (§4.1.1)
The same bit is overloaded as both "I support reverse paths" (in OPEN)
and "I want / am providing reverse path info" (in LSP object), and within
the LSP-object use it conflates request and provision. Recommend
explicitly stating the directional rule: PCC-sent bit = request,
PCE-sent bit = provision, and clarify behavior when both peers set
inconsistent values.

### S3. "Number of Multipaths" scope ambiguity (§4.1.1)
The text describes the field as "how many forward primary multipaths the
PCE can compute" — but doesn't state up front whether this is per-LSP or
per-session. The later text makes it per-LSP. State this in the field
definition.

### S4. Asymmetric error handling for over-cap signaling (§4.1.1)
"If a PCC receives more paths than it advertised support for, it MUST send
a PCError…" The reverse direction (PCE receiving more than its computed
cap from a PCC PCRpt) is not specified. Make the rule symmetric or
explicitly say it only applies in one direction and why.

### S5. Path ID ownership after delegation change (§4.2)
"If the LSP is delegated to the PCE, then the PCE allocates the Path IDs.
If the LSP is locally computed on the PCC, then the PCC allocates the
Path IDs." What happens on delegation revocation or re-delegation — do
Path IDs persist, get reallocated, or invalidated? Real implementations
will hit this; please clarify.

### S6. Many-to-many vs. "symmetric" constraint (§4.4)
§4.4 step 3 explicitly allows many-to-many opposite-direction mappings,
while a few lines later: "When path A references path B as its
opposite-direction path, path B MUST also reference path A." Both can
coexist (each (A,B) edge appears in both directions), but the wording
could be read as bijective. Add a sentence: "For each (A → B) reference,
the reverse (B → A) reference MUST also be present; multiple such
references per path are permitted."

### S7. "Informational only" reverse paths vs. §2.3 BFD/PM use
- §3.1 R-flag: "Paths with this flag set serve only informational purpose
  to the PCC."
- §2.3: reverse Segment Lists are used by PM/BFD to drive bidirectional
  liveness sessions.

§2.3 makes the reverse path operationally load-bearing, not just
informational. Soften §3.1 to "Reverse paths are not installed for
forward-direction load-balancing; they MAY be used for purposes such as
PM/BFD as described in §2.3."

### S8. Composite Candidate Path — no recursion-depth bound (§3.5)
A Composite CP signals traffic recursively into other SR Policies on the
same headend. Nothing in the draft bounds the recursion depth or
specifies loop-detection behavior if policy A recurses into B and B
recurses into A (directly or transitively). At minimum, an Operational
Consideration acknowledging this is needed; a normative MUST-NOT against
cycles would be stronger.

### S9. F-flag without C-flag has no defined semantics
§4.1.1: "the F-flag MAY be set without the C-flag" for future use cases.
There is no defined consumer of MULTIPATH-FORWARD-CLASS today outside
Composite/Per-Flow CP. Either state that "F-only" today has no defined
semantics (reserved for future use) or remove the allowance until a
consumer exists.

### S10. Inconsistent terminology: "Reserved" vs "Flags" (§3.5.1 vs §7.7)
In Figure 4 the area holding the T flag is labeled "Reserved", but §7.7
creates a "Flags in the MULTIPATH-FORWARD-CLASS TLV" registry covering
bits 0–31. Either rename Figure 4's field to "Flags" (consistent with
PATH-ATTRIB and OPPDIR-PATH TLVs) or rename the registry.

### S11. Normative vs informative reference classification
[I-D.ietf-pce-sr-bidir-path] is listed as **normative**, but the
MULTIPATH-OPPDIR-PATH TLV is defined **by this document**. The sr-bidir-path
draft is a *consumer*, not a producer, of definitions used here. Unless
implementing this document strictly requires reading sr-bidir-path,
demote it to informative — that also avoids a downref dependency.

### S12. Capability presence on a single side (§4.1.1)
"The PCC MUST NOT include this TLV in the LSP object if the TLV was not
present in the OPEN objects of **both** PCEP peers." Good. But the
inverse case isn't stated: if only one peer advertised in OPEN, what
happens at message processing time on the unaware peer? Presumably the
"Unexpected PATH-ATTRIB" error fires, but say so explicitly.

### S13. Appendix A.3 internal asymmetry
POL1 has 2 Segment Lists in CP2 (one with reverse, one without), while
POL2 CP2 only has 1 forward Segment List. The state-reports therefore
include "reverse" PATH-ATTRIBs whose Opposite Direction Path ID = 0,
which exercises the very ambiguity called out in M2. Once M2 is resolved
the example should be regenerated to match.

---

## Nits / grammar / wording (N)

(Line numbers from the html-rendered text; section numbers from the doc.)

- **§1.2 ECMP / W-ECMP**: prefer hyphenated "Equal-Cost Multi-Path" and
  "Weighted Equal-Cost Multi-Path" (RFC 2992 spelling).
- **§2.2**: "two paths, that can together carry 80 Gbps" — drop the
  comma; "that" introduces a restrictive clause.
- **§2.2**: "have only 60 Gbps capacity" → "have only 60 Gbps of capacity
  each" (or "are limited to 60 Gbps").
- **§3.1**: "preceded by **an** PATH-ATTRIB object" → "**a** PATH-ATTRIB
  object" (article matches the consonant sound).
- **§3.1**: "serve only informational purpose to the PCC" → "serve only
  an informational purpose for the PCC".
- **§3.3 / §3.4 / §3.5.1 / §4.1.1** all open with "New X TLV is defined…"
  → "A new X TLV is defined…" (missing indefinite article).
- **§3.3 / §3.4**: "Length (16 bits): 4 bytes." / "8 bytes." → IETF
  convention is "octets". RFC 5440 uses "octets" throughout.
- **§3.4 N-flag**: "node co-routed with its opposite direction path,
  specified in this TLV" — comma turns the clause non-restrictive;
  either remove the comma or change to "specified by this TLV".
- **§3.5**: "we make use of the COLOR TLV" → "this document makes use of
  the COLOR TLV".
- **§3.5**: "An ERO MUST be included as per the existing RBNF, this ERO
  MUST contain no sub-objects." — comma splice; use a period or
  semicolon.
- **§3.5.1**: "builds on top of the concept" → "builds on the concept".
- **§4.1.1**: "From the PCC, the MULTIPATH-CAP TLV MAY also be present in
  the LSP object" → "The PCC MAY also include the MULTIPATH-CAP TLV in
  the LSP object".
- **§4.2**: "MUST send PCError message" → "MUST send **a** PCError
  message".
- **§4.2**: "across all these path types" → "across forward and reverse
  paths" (forward/reverse are roles, not types).
- **§4.3**: "(un)equal load-balancing" → "equal or unequal load-balancing".
- **§4.4**: "established **atomically**" — too strong a claim for a
  message-level operation; suggest "established in a single message
  exchange".
- **§5**: "The RBNF of PCRpt and PCUpd messages, as defined in
  [RFC8231], **use** a combination…" → subject is "RBNF" (singular);
  should be "**uses**".
- **§9.4** last bullet: missing trailing period.
- **§6 Implementation Status**: the "Note to RFC Editor — remove" is
  present and correct, good.
- **§3.1 Path ID** vs **§4.2 Value 0 semantics**: §3.1 describes Path ID
  without mentioning 0=unallocated. Add a sentence pointing forward to
  §4.2.
---

## Suggested edit order for the authors

1. Fix M1 (load-balancing ratio wording: "all other" → "all paths
   including this one") — single-word change, two locations.
2. Resolve M2 (TLV omission vs ID=0) — pick one rule and propagate to
   §3.4, §4.4, and Appendix A.3.
3. Fix M3 (Error-Type 1 → 10) — one-character fix.
4. Fix M4 (missing bit 13 in IANA registry).
5. Decide M5 (Updates: 8231, 8281 header) with the WG / AD.
6. Add a paragraph in §4.3 covering M6 (flow-affinity SHOULD).
7. Expand §8 (Security) and §9.6 (Operational) per M7 and M8.