Hi Jonathan, Thanks for the review, please see inline .
Mit freundlichen Grüßen / Best Regards Cyril Margaria From: Jonathan Hardwick [mailto:[email protected]] Sent: Tuesday, July 23, 2013 4:22 PM To: [email protected] Cc: [email protected] Subject: Comments on draft-ietf-pce-gmpls-pcep-extensions-08 Hello I have reviewed this draft and have the following comments and questions. Best regards Jon Section 2 RBNF The RBNF implies that BANDWITH and GENERALIZED-BANDWIDTH can be mixed on a single request & response. That looks wrong to me, I think that either one or the other should be supplied but not both. If both really can be used, please could you add text explaining why? Same comment for LOAD-BALANCING and GENERALIZED-LOAD-BALANCING. [[Margaria.C]] This is for backward-compatibility : for instance for reoptimization request the BANDWIDTH MUST be provided. We did not want to change existing logic but to enhance the information. The GENERALIZED-BANDWIDTH is present as a detailed expression of the BANDWIDTH. Section 2.1 RP object The bits used for the RG flag are not defined until section 5.4. Please at least xref from 2.1 to 5.4 for the definition. Better still, define them in section 2.1. [[Margaria.C]] OK "The RG flag is backward-compatible with [RFC5440<http://tools.ietf.org/html/rfc5440>]: the value sent by an implementation (PCC or PCE) not supporting it will indicate a node granularity." Pre-existing implementations may calculate at node or link granularity so I don't think you can assume that a back-level implementation will always return node-level granularity. So, backwards-compatibility does not work as you expect. [[Margaria.C]] In that case the ERO contains nodes, the intent is more to ask for a more detailed route rather than to force a less detailed route. The values should be read as node (or better) , not node-only ERO (Wording can be added). Do you see a use case for node-only ERO (or link-only ERO) ? My suggestion is to make 0 the reserved value for the RG flags (not 3). Then the backwards compatibility problem is resolved, since the value RG==0 implies that the sender is a back-level node and so is ignoring the RG field. "If a PCE honored the the requested routing granularity for a request, it SHOULD indicate the selected routing granularity in the RP object included in the response" Suggest changing the above excerpt to: "The PCE MUST indicate the selected routing granularity in the RP object included in the response" Justification: I think you mean MUST, not SHOULD. Otherwise the PCC can't rely on the RG flag, rendering it useless. [[Margaria.C]] In the absence of flag the PCC should check. The SHOULD is here to allow PCEs supporting the other object but not filling this bit, but in any case they can put 0 (PCC has to check then), so a MUST can be added. Also, why should the PCE do this only if it honoured the RG? I think the PCE should be free to use policy to choose to route at a different RG than that given in the request, and whatever RG is chooses to do the routing at, it should return that RG in the response. [[Margaria.C]] OK Along the same lines: "The PCE MAY return finer granularity on the route based on its policy. The PCC can decide if the ERO is acceptable based on its content." Suggest changing the above excerpt to "The PCE MAY return any granularity it likes on the route based on its policy. The PCC can decide if the ERO is acceptable based on its content." [[Margaria.C]] OK Comment on the following statement: "The PCE MAY try to follow this granularity and MAY return a NO-PATH if the requested granularity cannot be provided." If the PCE cannot supply a path at the given granularity then it is either a problem with the PCE's capabilities or a policy issue. As such, NO-PATH is not appropriate; the PCE should respond with PCErr with a suitable error code. [[Margaria.C]] The PCE may also miss TE information (loose routing over a domain or missing label information) , so a NO-PATH is valid in that case (if policy dictate it should not return a lower granularity). I will add the PCErr for the case you mentioned. Section 2.2 and 2.3 GENERALIZED-BANDWIDTH and GENERALIZED-LOAD-BALANCING I think it is better to specify a format for the GENERALIZED-BANDWIDTH and GENERALIZED-LOAD-BALANCING objects that allows a single instance of the object to specify both forwards and reverse bandwidth. By having two objects you complicate matters because you then need to specify (and write extra code for!) all the consistency rules between the objects (e.g. you don't say what happens if the max-LSPs field differs between the forwards and reverse direction GEN-LB object). This in turn will make implementation more fiddly and potentially less interoperable. How about including the reverse-direction TSPEC in the same object as the forwards-direction TSPEC if the R bit is set? [[Margaria.C]] Its a good point, I will consider it. You state that multiple objects with the same TSPEC type are dealt with by ignoring all but the first. Why do you allow multiple objects with a different TSPEC type? [[Margaria.C]] To be permissive, this is done in other protocols And what is the PCE to so with a request containing different types of TSPEC? I think that all TSPECs need to have the same type. [[Margaria.C]] This is to address draft-ietf-pce-inter-layer-ext, I do not think we should disallow requesting an Ethernet LSP using a SDH or OTN layer (and specifying if we do prefer ODU 0 or ODUFLex) Section 2.4.1 Generalized-Endpoint object type To aid clarity, suggest rewording this: "A PCE not supporting those TLVs but not being able to fulfill the label restriction MUST respond with a response with NO-PATH with the bit "No endpoint label resource" or "No endpoint label resource in range" in the NO-PATH- VECTOR TLV, the response SHOULD include the ENDPOINT object in the response with only the TLV where it could not met the constraint." to this: "A PCE supporting those TLVs but not being able to fulfil the label restriction MUST send a response with a NO-PATH object which has the bit "No endpoint label resource" or "No endpoint label resource in range" set in the NO-PATH- VECTOR TLV. The response SHOULD include an ENDPOINT object containing only the TLV where the PCE could not meet the constraint." [[Margaria.C]] OK Section 2.4.2.5 Labels TLV Suggest changing "This Bit SHOULD be set to 0 in a SUGGESTED-LABEL-SET TLV Set." to "This Bit SHOULD be set to 0 in a SUGGESTED-LABEL-SET TLV Set and ignored on receipt." [[Margaria.C]] OK Delete orphaned text which says "Table 5" - no table present in this section. [[Margaria.C]] OK Section 2.5 Stipulate that IP address subobject MUST be a link subobject. Clarify that >1 label subobject may follow each link-address or unnumbered-link subobject. [[Margaria.C]] OK Section 2.6 XRO Better to just define the X bit than refer to RFC 5521. The definition is simple enough. [[Margaria.C]] I would do both. You say the C-Type is "copied from the label object" - what label object? Isn't this just a number which identifies the type of the labels? [[Margaria.C]] Yes, I will correct. Does the label field specify a single label or an array of labels? (Suppose single label but it is not quite clear.) [[Margaria.C]] A single label, for multiple label this should be repeated Section 2.7 PROTECTION-ATTRIBUTE TLV I am not sure that re-using the format from RFC 4872/4873 is the right way to go. It leaves us with a format containing many elements that are superfluous (or at least, not obviously applicable) to path computation. It would help if you could add some text explaining why each field is applicable to path computation. [[Margaria.C]] --> keeping the same format is done for simplicity of the definition, and this object is used as policy input. For example, on the PROTECTION-ATTRIBUTE TLV flags: · S bit: since the PCE server does not assign resources, it seems that this bit will not be used in PCEP, correct? · N bit: I think this is irrelevant to PCEP. · O bit: ditto. · I bit: ditto. · R bit: ditto. Unless I have misunderstood, I think It would be better not to define these bits in the object format. If you must have them then I think you should at least stipulate that they are ignored on receipt. [[Margaria.C]] So it woud be better to state "contains the value of the PROTECTION object defined by RFC4872" and its used for policy input. You say "LSP Flags can be considered for routing policy based on the protection type." I think you must mean the Link Flags, since these specify the link attributes that the path requires. I am not sure what use the LSP flags are to the PCE; please could you explain? Same comment applies to segment recovery flags. Where you say "The other attributes are only meaningful for a stateful PCE", please could you say why a stateful PCE might make use of them, or point me at a draft where this is discussed? Some semantic are covered in draft-tanaka-pce-stateful-pce-mbb-01 , draft-crabbe-pce-stateful-pce-protection-00 and they also were present in draft-ietf-pce-stateful-pce .
_______________________________________________ Pce mailing list [email protected] https://www.ietf.org/mailman/listinfo/pce
