RSVP-TE Extensions for Collecting Shared Risk Link Group (SRLG) Information
draft-ietf-teas-rsvp-te-srlg-collect-08

My comments are relatively minor but I would like to see them addressed before publication.

1. Section 4.1. (SRLG Collection Flag): "…this document defines a new flag in the Attribute Flags TLV…which MAY be carried…"  I think a clearer description would be something worded more along the lines of "…which MUST be set in…to indicate that SRLG information SHOULD be reported…"   It would also be good if in this section the difference between LSP_REQUIRED_ATTRIBUTES  and LSP_ATTRIBUTES is also explained.

2. Section 4.2. (RRO SRLG sub-object): "…The SRLG sub-object SHOULD be pushed…before the node IP address…SHOULD be pushed after the Attribute sub-object, if present, and after the LABEL sub-object, if requested."  Knowing that it is a stack, does it really make a difference where the SRLG sub-object is pushed?  Put another way, why are you using "SHOULD" and not "MUST"?

3. Section 5.1. (SRLG Collection)  "A node SHOULD NOT add SRLG information without an explicit request…"  What happens if a node does?  Should the "SHOULD NOT" be "MUST NOT"?

4. Section 5.2. (SRLG Update): "If local policy is that the SRLG change SHOULD be suppressed…"  s/SHOULD/should   The policy is being described, so a normative keyword is not appropriate.

5. Section 5.3 (Domain Boundaries)  "If mandated by local policy, a node…MAY add a summary of the removed SRLGs or map them to other SRLG values."  How is this (summary or mapping) done?  If specified somewhere else, please add a reference.

6. Section 6.2. (Coherent SRLG IDs): "…SRLG IDs SHOULD be unique."  Why is this not a MUST?

- 5.3: "...this SHOULD NOT be done unless explicitly mandated by local
   policy."

Is that the same as saying this should default to off unless the administrator chooses to turn it on?

opsdir review by Niclas Comstedt <nco@comstedt.net>

Minor comments/questions:

- Please spell out RRO in section 4.2

- Why are the following SHOULDs not MUSTs?
"[...] the Path message SHOULD NOT be rejected due to the SRLG recording
   restriction and the Path message SHOULD be forwarded without any SRLG
   sub-object(s) added to the RRO of the corresponding outgoing Path
   message."

- Why do you need two (potentially different) policies for the two points below. Shouldn't a node that provides SRLG information initially, also always provide updates (as the initial information might otherwise be wrong and therefore not be able to address the originial intention anymore - disjoint paths)?
   "o  Whether the node is allowed to participate in SRLG collection.
   o  Whether the node should notify changes to collected SRLG
      information to endpoint nodes as described in section 5.2."

"It is recommended that domain/layer boundary policies
take the implications of releasing SRLG information into
consideration and behave accordingly during LSP
signaling." Eh, that's a bit opaque for me at least.  Can
you say a bit more about what those implications might be
and how one might take them into account, and why that
doesn't need to be mentioned in the document?  I'm asking
since there is a bit of a breach of the blood-brain
barrier going on here (as is ack'd in the draft) and while
it's hard to envisage that much going wrong if providers
expose this information, I guess there might easily be
something too subtle for this particular reader:-)

Adrian's explanation makes sense to me. I would have liked to see something similar to be added in the draft to clarify the behavior, but I leave it up to the authors/shepherds to decide if they want to do this or not.

In Section 5.1 when the SRLG collection request was contained in an LSP_REQUIRED_ATTRIBUTES and the RRO would become too big, a node drops the RRO from the Path message entirely. It is not clear what the next node that receives this SRLG collection request without an RRO would need to do as the spec only says that the RRO is inserted at the ingress. What is the expected behavior here on the subsequent node?