Hi Elwyn, First of all, please take my apology for the delay in getting back to you.
Thanks for your very thorough review and excellent comments on this draft. For each comment, please see inline for my response. I believe almost of all your comments are agreed and as such incorporated in the revision. Please see the diff file and let me know if this revision would satisfy you or further revision is required to move to the next step. https://www.ietf.org/rfcdiff?url2=draft-ietf-pce-wson-rwa-ext-11 Thanks & Best regards, Young -----Original Message----- From: Elwyn Davies [mailto:[email protected]] Sent: Friday, December 28, 2018 10:39 AM To: [email protected] Cc: [email protected]; [email protected]; [email protected] Subject: Genart last call review of draft-ietf-pce-wson-rwa-ext-10 Reviewer: Elwyn Davies Review result: Ready with Nits I am the assigned Gen-ART reviewer for this draft. The General Area Review Team (Gen-ART) reviews all IETF documents being processed by the IESG for the IETF Chair. Please treat these comments just like any other last call comments. For more information, please see the FAQ at <https://trac.ietf.org/trac/gen/wiki/GenArtfaq>. Document: draft-ietf-pce-wson-rwa-ext-10 Reviewer: Elwyn Davies Review Date: 2018-12-28 IETF LC End Date: 2018-12-27 IESG Telechat date: Not scheduled for a telechat Summary: Ready but with a lot of nits. The amount of cross referencing to other documents that supply objects or object formats, some of which are repurposed, make it difficult to be certain that all requisite definitions are present and exactly which external objects may be present. Major issues: None Minor issues: For avoidance of error during editing, the various TBD values included in the specifications and the requests in s8 should be linked by using TBDx values specific to each allocation. YL>> Added TBDx values to each allocation. s3, para 6: Given that the additional multiplexing techniques are being developed, is the resulting extensibility easy to cater for in the capabilities defined here? YL>> Unfortunately, there have not been other multiplexing technologies to date that can replace WSON (Wavelength Switched Optical Networks) that operates on the principle of wavelength continuity on the optical path. s4.3: Various encoding errors are possible with this TLV (e.g., not exactly two link identifiers with the range case, unknown identifier types, no matching link for a given identifier). Should some error behaviour be specified here (or is it covered generically - if so where?) YL>> Yes, indeed in Sections 5.1 and 5.2, we have defined error handling of various cases. But in light of your later comment on adding additional value, I agree it would be helpful to add another flag that indicates various syntactical/encoding errors as you pointed out. s6.2: Including instructions for future work to be done as per this section is inappropriate. It will necessarily be overtaken by events one way or the other. Unless there is already work in progress that can be referenced, the section should be removed. YL>> Agree. I will remove this section. Nits/editorial comments: Tool issue: Note that the layout of 2nd and 3rd level headings does not correspond to IETF draft/RFC conventions (they are indented rather than at the left edge of the page). YL>> Fixed. General: The encoding of numeric fields should be specified (once for all cases is all that is needed). YL>> Done. General: s/TDB/TBD/ (7 places) (I assume) YL>> Yes. Replaced all cases. s3, para 4: It would be helpful to insert a new second sentence that explains the term Lambda Switch Capable. Maybe The devices used in WSONs that are able to switch signals based on signal wavelength are known as Lambda Switch Capable (LSC). The expansion of LSC can then be removed from para 5. YL>> Thanks for the text. Added this sentence in Section 3, para 4. But I think para 5 does still add values to the general audience who may not be familiar with optical networks. s3, para 4: It would be helpful to expand the 3R acronym for those less familiar with optical network jargon. YL>> Expanded: Re-amplification, Re-shaping, Re-timing s3, para 4: Is it possible to say briefly in the document why all optical wavelength converters are out of scope? YL>> I deleted this sentence, which is not true anymore. s3, para 6: s/both signals/the two signals/ YL>> Done. s3, para 9: s/generic property/generic properties/ YL>> Done. s3, para 10: s/advanced modulation processing/advanced modulation processing capabiities/; s/Those modulation properties/These modulation capabilities/; s/by the means of/indicated by means of/ YL>> All corrected. s4, para 1: s/that are going to be/that are/ YL>> Done. s4, in the two paras before figure 3: The expansion of WA should be done on first occurrence (one para earlier). s4, specification of WA Object: YL>> Corrected. - Reserved bits ought to be explicitly zeroed (applies to various other places) YL>> Corrected. - Unused flag bits should be zeroed. YL>> Done. - s/The following new flags should be set/One flag bit is allocated as follows:/ - In specification of M: s/needs not be explicit/need not be explicit/ - In specification of M: s/Otherwise,/Otherwise (M bit set to 0),/; s/In such case,/If M is 0/ YL>> All done. s4.3, above Fig 4: > The Wavelength Restriction Constraint TLV type is TBD, recommended > value is TBD. Putting in ' recommended value is TBD' is pointless if a value is not given and it needs to be removed from the published version in any case. If the authors have a preferred value for this then put in a note to IANA; otherwise leave it as TBDx for IANA to determine. YL>> Replaced TBD/TBD3. Same applies in s5 above Fig 6. YL>> Replaced TBD/TBD4. s4.4, paras 1 and 2: OLD: Path computation for WSON includes the check of signal processing capabilities, those capability MAY be provided by the IGP. Moreover, a PCC should be able to indicate additional restrictions for those signal compatibility, either on the endpoint or any given link. The supported signal processing capabilities are the one described in [RFC7446]: NEW: Path computation for WSON includes checking of signal processing capabilities at each interface against requested capability; this requirement MAY be implemented by the IGP. Moreover, a PCC should be able to indicate additional restrictions to signal processing compatibility, either on the endpoint or any given link. The supported signal processing capabilities are those described in [RFC7446]: ENDS YL>> Accepted and replaced. s4.4: It is not clear to me where the XRO and IRO sub-objects would be used in this specification. YL>> XRO/IRO is a part of PCEP Objects used in PCEP Request/Reply. I would add this in s.4.4.1 and s.4.4.2: [RFC5440] defines how XRO sub-object is used. In this draft, we add a new XRO sub-object, signal processing sub-object. (in the first paragraph in s.4.4.1) [RFC5440] defines how IRO sub-object is used. In this draft, we add a new IRO sub-object, signal processing sub-object. (s.4.4.2) (in the first paragraph in s.4.4.2) s4.4.1: Expand XRO (Exclude Route Object?) on first use. YL>> Yes, added in s.4.4.1; also expanded IRO (Include Route Object) in Section 4.4.2. s4.4.1, last para: which sub-sub objects are permitted/expected here? I couldn't be sure which part of RFC 7689 I should be looking at. YL>> Sorry it was not clear. I added the following text at the end of 4.4.1. Deleted RFC 7689 reference. "The permitted sub-sub objects are the Optical Interface Class List and the Client Signal information whose encodings are described in Section 4.1 and Section 4.2 of [RFC7581], respectively." s4.4.2: Provide an expansion of IRO. YL>> Expanded. s4.4.2, last para: There is no sub-object called just 'processing' in RFC 7689. Is WSON Processing Hop Attribute TLV (Section 4.2) what is meant? YL>> Yes. I specified the full reference. s5, definitions of Link Identifier and Allocated Wavelengths: What is the meaning of '(variable)' in these definitions? I suspect they can be removed - am I right in thinnking they just flag up that there are various possibilities on each case? If so, this is clear from the ss4.3.x definitions and 'variable' is just confusing. YL>> Yes, I agree with you. I removed them. s5, Allocated Wavelengths definition: s/to the link identifier/to be associated with the Link Identifier/ YL>> Corrected. s5, last sentence: > The type > value of the Wavelength Restriction Constraint TLV is TBD by IANA. THis duplicates the heading above Figure 6 and should be deleted. YL>> Yes, I agree and it is deleted. s5.1: Should there be a a separate error values for flagging syntactical errors in the constraint specifications - see Minor Issue relating to s4.3 above. Alternatively maybe it could be an additional value on PCEP-ERROR type 10 (Reception of Invalid Object). YL>> Agreed. At the last paragraph In Section 4.3, the following text is added: Various encoding errors are possible with this TLV (e.g., not exactly two link identifiers with the range case, unknown identifier types, no matching link for a given identifier, etc.). To indicate errors associated with this type, a new Error-Type (TBD8) and an Error-value (Error-value=3) MUST be defined so that the PCE MUST send a PCErr message with a PCEP-ERROR Object. See Section 5.1 for the details. In Section 5.1, this new value is also added. s6.1, para 1 and para 3: s/allow configuring/allow configuration of/ YL>> corrected. s6.5: Will advertisement of WSON RWA capability need additional options in the [PCEP-LS] specification? It isn't clear to me that the existing draft of PCEP-LS caters for the required adverts. If it does, a little more detal here might help. If not then liaison with the authors of PCEP-LS is needed before both dcouments are approved. YL>> There is capability advertisement in PCEP-LS in which we can add this option. I am a co-author of PCEP-LS and will add this addition in the next revision. s8.1: add an extra line to the assignment request with an entry for Object Type: 2-15: Reserved YL>> Actually this is Object Encoding which needs to be assigned new Object Class Value and Object Type = 1. I am don’t think we need to define Object Type other than 1 as typical for all PCEP Objects. Please see RFC5440 Section 7.2. s10: RFC 2119 and RFC 8174 MUST be normative. YL>> Agree. Moved to be normative. AFAICS almost all the 'Informative' references in s10.2 are actually normative because they define objects used in this specification. Looking through the text, I think RFC 6163, RFC 7581 (just about) and the PCEP-LS draft (probably) are really informative; it is unclear whether RFC 3630 and RFC 5329 (associated with IPv4 and IPv6 addresses) are useful references at all. All the others are normative. YL>> RFC3630 and 5329 were added per AD's request to reference the IPv4/v6 encoding. I would keep them here. I would keep RFC 3471, 3630, 4203, 5329, 5420, 7446, 8253 in the informative reference section (as they are not directly related with this draft). I moved RFC 5440, 6205, 7581, 7579 to be normative. _______________________________________________ Pce mailing list [email protected] https://www.ietf.org/mailman/listinfo/pce
