Hi Benjamin, Thanks for your review, please see inline..
> ---------------------------------------------------------------------- > COMMENT: > ---------------------------------------------------------------------- > > I'm always a little reluctant to publish framework documents without any > examples of using that framework (i.e., this document does not define any > association types), but this work seems well thought-out and it looks like > there are a handful of association types in development by the WG. Would > it be worth listing a few as informational references to give the reader a > broader sense of what associations might be used for? > [[Dhruv Dhody]] Added. > Thanks for the note in the shepherd writeup about the author count! > > Thank you also for the very readable document -- it's clear that the > authors took care to organize the content in a manner accessible to the > reader. > > As a general note, do we need to say anything about the multi-byte integer > values being encoded in network byte order? (Perhaps following RFC 5440's > implicit convention is the right thing to do.) > [[Dhruv Dhody]] In PCEP extensions, this has been implicit. > Section 1 > > [RFC6780] defines the RSVP ASSOCIATION object, which was defined in > the context of GMPLS-controlled Label Switched Paths (LSPs) to be > > nit: is this supposed to be RFC 4872? > [[Dhruv Dhody]] Ack. > Section 3.3 > > The dynamically created association can be reported to the PCEP peer > via the PCEP messages as per the stateful extensions. While the > operator-configured association is known to the PCEP peer before > hand, a PCEP peer could ask for a LSP to join the operator-configured > association via the stateful PCEP messages. > > nit: I suggest s/While/When/, if I understand correctly. > [[Dhruv Dhody]] Updated. > Multiple types of associations can exist, each with their own > association identifier space. The definition of the different > association types and their behaviours is outside the scope of this > document. The establishment and removal of the association > relationship can be done on a per LSP basis. An LSP may join > multiple association groups, of different or of the same association > type. > > Is it possible for an LSP to join multiple association groups of the same > type and then for configuration to be assigned to two groups in a manner > that conflicts? What procedure is used for conflict resolution in such a > case? > [[Dhruv Dhody]] This should be handled per association type. I will make sure that future documents that define association types include text for this. > Section 3.4 > > Association identifier range for sources other than the PCEP speaker > (for example an NMS system) is not communicated in PCEP and the > procedure for operator-configured association range setting is > outside the scope of this document. > > Given the discussion in the rest of the document, it seems that reserving > a specific range for operator configuration (across all association types) > is too rigid to meet the various anticipated use cases. Is that a correct > assessment? > [[Dhruv Dhody]] Yes. > Section 5.1 > > If the Assoc-type is not recognized or supported by the PCEP speaker, > it MUST ignore that respective Start-Assoc-ID and Range. If the > Start-Assoc-ID or Range are set incorrectly, the PCEP session MUST be > rejected with error type 1 and error value 1 (PCEP session > establishment failure / Reception of an invalid Open message). > > I could maybe see competent engineers disagreeing about which of these > MUSTs would take precedence in a case where both apply. [[Dhruv Dhody]] Updated. If the Assoc-type is not recognized or supported by the PCEP speaker, it MUST ignore that respective Start-Assoc-ID and Range. If the Assoc-type is recognized/supported but the Start-Assoc-ID or Range are set incorrectly, the PCEP session MUST be rejected with error type 1 and error value 1 (PCEP session establishment failure / Reception of an invalid Open message). > > The Assoc-type MAY appear more than once in the OP-CONF-ASSOC-RANGE > TLV in the case of a non-contiguous Operator-configured Association > Range. The PCEP speaker originating this TLV MUST NOT carry > overlapping ranges for an association type. If a PCEP peer receives > overlapping ranges for an association type, it MUST consider the Open > message malformed and MUST reject the PCEP session with error type 1 > and error value 1 (PCEP session establishment failure / Reception of > an invalid Open message). > > This might be a good place to specify the handling if a received range > would cross the 0xffff boundary. > [[Dhruv Dhody]] Added - The incorrect range include the case when the (Start-Assoc-ID + Range) crosses the association identifier range of 0xffff. > Section 6.1.1 > > The Global Association Source TLV is an optional TLV for use in the > Association Object. The meaning and the usage of Global Association > Source is as per [RFC6780]. > > Do we want to say Section 4 specifically of 6780? > (Similarly for Extended Association ID.) > [[Dhruv Dhody]] Ack. > Section 6.1.4 > > the association group. In this document, all these fields are called > 'association parameters'. Note that the ASSOCIATION object MAY > > I would humbly suggest s/all these fields are called 'association > parameters'/these fields are collectively called 'association parameters'/. > [[Dhruv Dhody]] Ack. > Section 6.3.1 > > The ASSOCIATION Object is OPTIONAL and MAY be carried in the Path > Computation Update (PCUpd), Path Computation Report (PCRpt) and Path > Computation Initiate (PCInitiate) messages. > > When carried in PCRpt message, it is used to report the association > group membership pertaining to a LSP to a stateful PCE. The PCRpt > message are used for both initial state synchronization operations > (Section 5.6 of [RFC8231]) as well as whenever the state of the LSP > changes. The associations MUST be included during the state > synchronization operations. > > I suspect this is just my hazy memory of OPTIONAL, but how does "MUST be > included" match up with "OPTIONAL". > [[Dhruv Dhody]] Added this - If the LSP belongs to an association group, then the associations MUST be included during the state synchronization operations. > Section 6.4 > > Do I understand correctly that for dynamically created association groups, > the creation is effected by just using the relevant parameters in a > request(/update/etc.) and there is no need to separately create or > allocate the association? > > If a PCE peer is unwilling or unable to process the ASSOCIATION > object, it MUST return a PCErr message with the Error-Type "Not > supported object" and follow the relevant procedures described in > [RFC5440]. [...] > > Does this imply that the P flag in the common header should always be set > for ASSOCIATION objects? > [[Dhruv Dhody]] No, that was not the intention. I have made this change - If a PCEP speaker does not recognize the ASSOCIATION object in the stateful message, it will return a PCErr message with Error-Type "Unknown Object" as described in [RFC5440]. In case of PCReq message, the PCE would react based on the P flag as per [RFC5440]. If a PCE peer is unwilling or unable to process the ASSOCIATION object in the stateful message, it MUST return a PCErr message with the Error-Type "Not supported object" and follow the relevant procedures described in [RFC5440]. In case of PCReq message, the PCE would react based on the P flag as per [RFC5440]. > In case the LSP is delegated to another PCE on session failure, the > associations (and association information) set by the PCE remains > intact, unless updated by the new PCE that takes over. > > This includes the association source address? > [[Dhruv Dhody]] Yes. If the association source needs to change, then the new PCE would create a new association with itself as source and remove the old one. > Section 8 > > attack vector. An attacker could report too many associations in an > attempt to load the PCEP peer. The PCEP peer responds with PCErr as > > "report" in the sense of causing the peer to create state to track them? > [[Dhruv Dhody]] Yes, basically to overwhelm the peer. > Section 12.2 > > Since the RFC 7525 procedures are RECOMMENDED, I think that reference > needs to be normative. > [[Dhruv Dhody]] Ack. Working Copy: https://raw.githubusercontent.com/dhruvdhody-huawei/ietf/master/draft-ietf-pce-association-group-10.txt Diff: https://tools.ietf.org/rfcdiff?url1=draft-ietf-pce-association-group-09&url2=https://raw.githubusercontent.com/dhruvdhody-huawei/ietf/master/draft-ietf-pce-association-group-10.txt Thanks! Dhruv > > _______________________________________________ > Pce mailing list > [email protected] > https://www.ietf.org/mailman/listinfo/pce _______________________________________________ Pce mailing list [email protected] https://www.ietf.org/mailman/listinfo/pce
