Document: draft-ietf-pce-pcep-ls Title: PCEP extensions for Distribution of Link-State and TE Information Reviewer: Andy Smith Review result: Has Issues
This draft is reasonable for Experimental, but I would like to see the authors fix the IANA object typo, resolve the error-value ambiguity, and rethink the LS-ID issue. All documented below. There are few minor suggestions as well. IANA: “LS Object Flag Field” text appears to refer to the wrong object 1) In Section 13.3, when requesting the new sub-registry “LS Object Flag Field”, the draft says it will “manage the Flag field of the LSP object.” That looks like a copy/paste error: it should be the LS object. This should be fixed because it affects IANA request clarity and long-term registry interpretation. Error-value usage for “Invalid Operation” looks inconsistent/ambiguous 2) In the capability section, the draft says a PCE should send error-type 19 (Invalid Operation), error-value TBD1 for: - receiving LSRpt when LS capability wasn’t advertised, and - receiving remote LS info when remote capability wasn’t advertised. But in the IANA section, the text attached to Error-Value=TBD1 is only described as “Attempted LS Report if LS remote capability was not advertised” (and the line breaks make it easy to miss whether the “LS capability not advertised” case is also intended to be covered). Consider: allocate two distinct error-values under error-type 19 (or otherwise make it crystal clear one codepoint covers both cases), and ensure the wording matches in both the procedure text and the IANA table. 3) LS synchronization end-marker using LS-ID=0 needs a clearer normative definition Figure 1 implies the end-of-sync marker is an LSRpt with SYNC=0 and “LS Report for LS-ID=0.” But the excerpted text I reviewed does not clearly and normatively define: - that LS-ID value 0 is reserved for this purpose, - whether it applies across all LS object types, and - what TLVs/fields must be present/absent in that end-marker report. Given interoperability implications, I suggest adding a crisp normative paragraph in the LS synchronization section (or LS object section) that reserves LS-ID=0 for sync completion and defines its exact encoding constraints. Minor issues / improvements Security considerations could be more specific about threat model and mitigations The security section correctly notes tampering/eavesdropping risks and recommends applying existing PCEP security mechanisms. However, since this extension exports detailed topology/TE state, it would be useful to add: - explicit callout that confidentiality is often required (topology sensitivity), - guidance on authorization/policy (which PCCs are allowed to report which topology), - operational mitigations (rate limiting / flap damping on updates, max database size, etc.—you already hint at strain from flaps). Boilerplate in Operational Considerations may be too strong Several operational sub-sections say “no new requirements/impact” beyond RFC 5440. Given that LSRpt can carry substantial amounts of topology and drive computation behavior, it’s arguably better to soften these statements or qualify them (e.g., “no new protocol mechanisms beyond…” but operationally there is impact in deployments). Session-lifetime uniqueness of LS-ID vs restart behavior The draft states LS-ID remains constant “for the lifetime of a PCEP session.” It would help interoperability to add a short note about what happens on session restart: - LS-ID values may be reassigned (since they are session-scoped), and - the PCE must flush state on session teardown (you do say cleanup on terminated sync). _______________________________________________ Pce mailing list -- [email protected] To unsubscribe send an email to [email protected]
