On January 18, 2019 at 11:45:52 AM, Jonathan Hardwick ( [email protected]) wrote:
Jon: Hi! Sorry for the empty message I sent before this one… I'm sorry that it has taken longer than I thought to reply to your comments! Please find our replies below. I will post an updated version of the document as soon as I can. I have some comments below. Thanks! Alvaro. ---------------------------------------------------------------------- DISCUSS: ---------------------------------------------------------------------- .... (2) Ability to signal the MSD per link, not just per node. Clearly the calculation of paths through specific links (using an Adjacency SID, for example) would require that information (if different/lower from what the node may support). Note that §6.1 seems to assume that the MSD will normally be advertised through different mechanisms, and it uses that to work around the fact that there's no link-specific information: "Furthermore, whenever a PCE learns the MSD for a link via different means, it MUST use that value for that link regardless of the MSD value exchanged in the SR-PCE-CAPABILITY sub-TLV." However, the text doesn't mandate the IGP/BGP-LS information to be available to the PCE. IOW, as written, the specification can't guarantee the proper calculation of paths that require the PCE to consider link MSDs. [Jon] In many deployments we anticipate that link MSDs are homogeneous. In those cases, link state routing might not distribute per-link MSDs (e.g. routers might not even support RFC 8491). In such cases, the per-node MSD on the PCEP session is sufficient. All the draft says is that MSD information available from link state routing, if any, must take priority over that defined on the PCEP session. I don't see a problem with that. Please include the description of the assumptions in the document. .... (4) §6.2.2 (Interpreting the SR-ERO): o If the subobjects contain NAI only, then the PCC first converts each NAI into a SID index value by looking it up in its local database, and then proceeds as above. I believe that this step in the interpretation of the SR-ERO is not properly specified. Which "local database" are you referring to? §6.2.2.1 mentions the SR-DB (when talking about errors)...but the specification should be clear about which database and what the specific procedure is. [Jon] You are right, this should be more explicit. How about this. NEW If the subobjects contain NAI only, the PCC first converts each NAI into a SID index value and then proceeds as above. To convert an NAI to a SID index, the PCC looks for a fully-specified prefix or adjacency matching the fields in the NAI. If the PCC finds a matching prefix/adjacency, and the matching prefix/adjacency has a SID associated with it, then the PCC uses that SID. If the PCC cannot find a matching prefix/adjacency, or if the matching prefix/adjacency has no SID associated with it, the PCC behaves as specified in section 6.2.2.1. END NEW This sounds fine. For example, what is the specific process that the PCC needs to follow to convert a Node ID/IP address to the SID/MPLS label? What if the SR-DB doesn't contain an SID associated to the specific Node ID/IP address? How should the router react to that? This case is not covered in the Error Handling section (6.2.2.1) either. [Jon] This is specified in 6.2.2.1. First bullet - if the prefix is found in the SR-DB but has no SID, send error TBD3. Second bullet - if the NAI is not found in the SR-DB, send error TBD4. A pointer to the SR-DB (as defined in I-D.ietf-spring-segment-routing-policy) is not enough because it is composed of optional information (according to the description in §3 (Segment Routing Database)). This document should be specific about what information must be contained in the SR-DB for the conversion to be successful. [Jon] Hopefully the new text proposed above will do that. The requirement of the information to be contained in the SR-DB makes I-D.ietf-spring-segment-routing-policy a Normative reference. [Jon] Rather than add an extra normative dependency, we would prefer to remove the dependency on the definition of the SR-DB and instead explicitly define in our document what information is to searched. That’s fine with me too. (5) §7 (Backward Compatibility) "Some implementations, which are compliant with an earlier version of this specification..." <sigh> I understand that there may be implementations that are non-compliant with this specification out in the field. However, why is this document making accommodations for them? Not only are these implementations not compliant with this document, but are also non compliant with rfc8408, which implies the use of a new sub-TLV per PST. I didn't find a discussion on the mailing list related to this issue. Specifying alternate behavior to accommodate non-compliant implementations is not the best way to define new functionality. If the support for those old implementations was an imperative then the new functionality should have been fully specified to seamlessly interoperate with what is already deployed. The current result is two ways to do the same thing... While I would prefer for this "backwards compatibility" not to be built into the specification, I am looking for discussion in the WG and a better justification that the current one (which can be reduced to "non-compliant implementations exist, so we need to fit them in here somehow"). [Jon] Yes, this section was painful to write, and was done after an on-list consultation with the working group. I can provide some references. The relevant thread starts here. https://www.ietf.org/mail-archive/web/pce/current/msg05397.html Ah..it was under the discussion for draft-ietf-pce-lsp-setup-type/rfc8404. Not that it matters now, but I got a little lost as to why these provisions were not put into rfc8404…maybe because of the SR-specific component... We got firm feedback from one vendor that the old behaviour needed to be accommodated. https://www.ietf.org/mail-archive/web/pce/current/msg05415.html There were good reasons for us changing the spec at a late stage, but this unfortunately left us needing to make a special case of the SR-CAPABILITY-TLV or else break a fielded implementation. So we collectively held our noses and did it. Hopefully, this plus the thread above gives you the background to the decision. <sigh!> :-( I wouldn’t call the feedback firm…and it makes me very sad to see the WG jumping through hoops to satisfy 1(!!) implementation, even if no one else replied on the list. :-( I realize it is too late to go back…and that I would probably end up in the rough if we did. Once we clear up the remaining DISCUSS points (+ publish an update), I will be ABSTAINing. I haven’t checked in detail, but I suspect that keeping §7 causes the Management Considerations to be incomplete because they don’t include the “backwards compatible” behavior. (6) sub-TLV Space for the PATH-SETUP-TYPE-CAPABILITY TLV rfc8408 failed to set up a sub-TLV registry for the PATH-SETUP-TYPE-CAPABILITY TLV. The bigger issue is that it also doesn't say that other PCE TLVs can be used as sub-TLVs (nor does it prohibit that). The Type for the SR-PCE-CAPABILITY sub-TLV is allocated from the PCEP TLV Type Indicators registry, making it a TLV. I also couldn't find any mention of sub-TLVs in rfc5440, or the potential intent to have a single space from which both TLVs and sub-TLVs could come. The question is: are all TLVs (defined in the PCEP TLV Type Indicators registry) able to be used as sub-TLVs? This question is general, but also specific to the PATH-SETUP-TYPE-CAPABILITY TLV. At a minimum, it should be made clear which can be used with the PATH-SETUP-TYPE-CAPABILITY TLV -- because this is the first document to define a new PST and sub-TLV, it seems appropriate to Update rfc8408 here...but rfc5440 may also need an Update. [Jon] I don't think there is an intent that any TLV can be used as a sub-TLV. This argues for making a new registry for the sub-TLVs of the PATH-SETUP-TYPE-CAPABILITY TLV (although we will have a vestigial code point allocation for SR-PCE-CAPABILITY as a top-level TLV because of section 7). I think therefore that this draft should create the registry and update RFC 8408. The other option (to avoid the duplication) is to make it explicit that the SR-PCE-CAPABILITY TLV can (only?) be used as a sub-TLV of the PATH-SETUP-TYPE-CAPABILITY TLV…. As mentioned above, rfc8408 doesn’t prohibit this, and because the space is so big it may not be an issue. I’m ok with your other answers. Thanks! Alvaro.
_______________________________________________ Pce mailing list [email protected] https://www.ietf.org/mailman/listinfo/pce
