Thank you Benjamin for the review comments. We have posted a new revision that address the comments: URL: https://www.ietf.org/archive/id/draft-ietf-pce-association-bidir-13.txt Diff: https://www.ietf.org/rfcdiff?url2=draft-ietf-pce-association-bidir-13
Please see inline with <RG>… From: Benjamin Kaduk via Datatracker <nore...@ietf.org> Date: Thursday, February 18, 2021 at 1:58 AM To: The IESG <i...@ietf.org> Cc: draft-ietf-pce-association-bi...@ietf.org <draft-ietf-pce-association-bi...@ietf.org>, pce-cha...@ietf.org <pce-cha...@ietf.org>, pce@ietf.org <pce@ietf.org>, d...@dhruvdhody.com <d...@dhruvdhody.com>, d...@dhruvdhody.com <d...@dhruvdhody.com> Subject: Benjamin Kaduk's No Objection on draft-ietf-pce-association-bidir-12: (with COMMENT) Benjamin Kaduk has entered the following ballot position for draft-ietf-pce-association-bidir-12: No Objection When responding, please keep the subject line intact and reply to all email addresses included in the To and CC lines. (Feel free to cut this introductory paragraph, however.) Please refer to https://www.ietf.org/iesg/statement/discuss-criteria.html for more information about IESG DISCUSS and COMMENT positions. The document, along with other ballot positions, can be found here: https://datatracker.ietf.org/doc/draft-ietf-pce-association-bidir/ ---------------------------------------------------------------------- COMMENT: ---------------------------------------------------------------------- Alvaro makes a good point, and I'm interested in seeing the updates made in response to it. <RG> We have added Section 3.5 - Operational Considerations <RG> Please advise your feedbacks. Section 4.1 As per [RFC8697], LSPs are associated by adding them to a common association group. This document defines two new Association Types, called "Single-sided Bidirectional LSP" (TBD1) and "Double-sided Bidirectional LSP" (TBD2), based on the generic ASSOCIATION Object ((Object-Class value 40). [...] (nit) pedantically I might say that these are instances of the ASSOCIATION object, as "based on" carries some connotation that they are distinct but similar. So this might become "using the generic ASSOCIATION Object" or "for the generic ASSOCIATION Object". <RG> Fixed - "using the generic ASSOCIATION Object" o The Tunnel (as defined in [RFC3209]) containing the forward and reverse LSPs of the Single-sided Bidirectional LSP Association on the originating node MUST have the same LSP parameters (as described in section 3.1.1 of [RFC3209]), albeit both LSPs have reversed endpoint nodes. There is no Section 3.1.1 of RFC 3209 (and it hardly talks about "parameters", either). <RG> Fixed. The Association ID, Association Source, optional Global Association Source and optional Extended Association ID in the Bidirectional LSP Association Object are initialized using the procedures defined in [RFC8697] and [RFC7551]. (nit?) Is it better to talk about Global Association Source and Extended Association IDs as being TLVs? <RG> Added TLV. Section 4.2 The new TLV has a flag to indicate the forward LSP, but the forward/reverse directionality is also indicated by the respective node addresses (for double-sided bidirectional LSPs); is there any protocol element that should check and enforce consistency of the two representations? [I was going to ask about some other error handling cases here, but I see that Section 5.7 already covers many of them -- thank you!] <RG> The draft does not have specific check except the new error code should help with the error. If I understand correctly, the F and R flags are mutually exclusive. Can/should that also be enforced (and with what error handling)? I don't think I understand why they are separate bits instead of just a single bit (e.g., 1 for forward and 0 for reverse). <RG> Good idea, merged into one flag to simplify. Section 5.1 Some nits in the last four bullet points: IIUC all should start with the plural "Stateful PCEs" (for consistency), and then the verbs in the last two would have to be adjusted to match, so "Stateful PCEs establish and remove", and "Stateful PCEs create and update". <RG> Fixed. Section 5.2 Similarly (also nit-level), the bullet points here should probably all use the "PCCs" plural form for consistency, and some verb conjugations would need adjustment to match. <RG> Fixed. Section 5.4 Just to check my understanding: this text is saying that whenever you use the bidirectional LSP association group, you also set the B flag in the request parameters? <RG> Yes. Section 5.5 There is no change in the PLSP-ID allocation procedure for the forward LSP of the Single-sided Bidirectional LSP on the originating endpoint node. In case of Double-sided Bidirectional LSP Association, there is no change in the PLSP-ID allocation procedure. This snippet carefully does not say anything about the PLSP-ID allocation procedures for the reverse LSP of a single-sided bidirectional association. Do those procedures change? <RG> I changed the previous sentence before this snippet to cover this case. Section 5.7 If a PCE speaker receives an LSP with a Bidirectional LSP Association Type that it does not support, the PCE speaker MUST send PCErr with Error-Type = 26 (Association Error) and Error-Value = 1 (Association Type is not supported). This reads a little bit (but not entirely) like it's binding the behavior of implementations that don't implement this specification, which generally doesn't make sense to attempt to do. (But IIRC this is the behavior required by the core spec anyway, so it wouldn't really matter.) <RG> Right. Section 9.2.1 o Bit number (count from 0 as the most significant bit) Thank you for including this detail! <RG> Thanks. Section 10.2 The "RECOMMENDED" for RFC 8253 usage might arguably promote it to normative (per https://www.ietf.org/about/groups/iesg/statements/normative-informative-references/). <RG> Ok, Changed RFC 8253 to normative. Thanks, Rakesh
_______________________________________________ Pce mailing list Pce@ietf.org https://www.ietf.org/mailman/listinfo/pce