Dale, thank you for your review. I have entered a No Objection ballot for this document. Lars On 2022-2-3, at 5:10, Dale Worley via Datatracker <noreply@xxxxxxxx> wrote: > > Reviewer: Dale Worley > 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-teas-gmpls-signaling-smp-10 > Reviewer: Dale R. Worley > Review Date: 2020-02-02 > IETF LC End Date: 2020-02-03 > IESG Telechat date: not known > > Summary: > > This draft is basically ready for publication, but has nits that > should be fixed before publication. > > Minor technical issue: > > These end nodes, in case > of a failure of the working LSP, MUST avoid trying to switch the > traffic to these protection LSPs that have been configured to use the > shared resources and try switching the traffic to other protection > LSPs, if available. > > It seems like this behavior is guaranteed without this specific > requirement: The node that detects the resource failure will signal > "Shared resources unavailable" to the end nodes of any lower-priority > LSPs that use the failed resources. Thus, the end nodes will not try > to switch the traffic of the working LSP to any of "these LSPs" > (lower-priority LSPs). And consequently, they will try to switch the > traffic to other protection LSPs. > > Indeed, it seems like the intermediate node should signal "Shared > resources unavailable" to the end nodes of all protecting LSPs that > use the failed resource, regardless of the LSPs' priorities -- all of > them are non-functional. > > I suspect there is some complexity here that I do not understand. It > may be worth describing that more explicitly. > > Nits/editorial comments: > > 1. Introduction > > The recovery resources for the > protecting LSPs are pre-reserved during the provisioning phase, and > explicit restoration signaling is required to activate (i.e., commit > resource allocation at the data plane) a specific protecting LSP > instantiated during the provisioning phase. > > At first, I misunderstood this to have the final "during the > provisioning phase" modify "explicit restoration signaling is > required". After re-reading it, I think that cannot be grammatically > correct, and "during the provisioning phase" is part of "instantiated > during the provisioning phase". But my confusion could have been > avoided if the text was amended to "... a specific protecting LSP > *that* was instantiated during the provisioning phase." > > o updates the definition of the 16-bit Reserved field [RFC4873] of > the PROTECTION object to take the new SMP type into account (see > Section 6.3). > > At first sight, this doesn't make sense -- how does adding SMP change > how the bits of an unused field are used? The concept is that SMP > signaling *uses* that field. So it would be clearer to say e.g. > > o updates the definition of the 16-bit Reserved field [RFC4873] of > the PROTECTION object to allocate 8 bits to signal the SMP > preemption priority (see Section 6.3). > > 2. Conventions Used in This Document > > In addition, the reader is assumed to be familiar with the > terminology used in [RFC4872], RFC 4426 [RFC4426], and RFC 6372 > [RFC6372]. > > presumably for consistency you want to say > > In addition, the reader is assumed to be familiar with the > terminology used in RFC 4872 [RFC4872], RFC 4426 [RFC4426], and RFC 6372 > [RFC6372]. > > Although there seems to be a style question, as the forms "RFC xyzw > [RFCxyzw]" and "[RFCxyzw]" (both as nouns) are both used frequently in > this document. Resolving this is probably something the Editor can > handle best. > > 3. SMP Definition > > Pre-configuring but not activating a > protecting LSP allows the common link and node resources in the > protecting LSP to be shared by multiple working LSPs that are > physically (i.e., link, node, Shared Risk Link Group (SRLG), etc.) > disjoint. > > If I understand this correctly, the point is that multiple working > LSPs may have protecting LSPs that are not disjoint, on the assumption > that only one of the protecting LSPs will be active at a time. But > this wording doesn't seem to mean that to me. Perhaps: > > Pre-configuring but not activating > protecting LSPs allows link and node resources to be shared by > the protecting LSPs of multiple working LSPs (that are > physically (i.e., link, node, Shared Risk Link Group (SRLG), etc.) > disjoint and thus unlikely to fail simultaneously). > > Perhaps that could be simplified to: > > Pre-configuring but not activating > protecting LSPs allows link and node resources to be shared by > the protecting LSPs of multiple working LSPs (that are > themselves disjoint and thus unlikely to fail simultaneously). > > 4. Operation of SMP with GMPLS Signaling Extension > > At the end of section 4, it seems desirable to further describe the > processing which it seems to me must happen: If F fails to allocate > the protection resource, it sends a failure message to E, which causes > E to remove the protection allocation, and then send a failure message > to A. And so on for further nodes in the LSP. > > 5. GMPLS Signaling Extension for SMP > > The following subsections detail how LSPs using SMP can be signaled > in an interoperable fashion using GMPLS RSVP-TE extensions (see RFC > 3473 [RFC3473]). This includes: > > As the first sentence is constructed, the "This" in the second > sentence doesn't have a clear referent. The most plausible referent > is the GMPLS RSVP-TE extensions, but those do not "include" point (5) > in the following link, since the extensions aren't directly involved > in activation. I think the sense would be improved if "This includes" > was replaced by "This signaling [or method] system enables". > > 5.1. Identifiers > > The LSP ID, however, > MUST be different to distinguish between the protected LSP carrying > normal traffic and the secondary LSP. > > You probably want to omit "carrying normal traffic" as it's not clear > what "normal" traffic is from the preceding text in the document. It > seems that it could only mean "traffic being carried by a protected > LSP", and in that case it's redundant. Although perhaps "normal > traffic" is a known term. > > 5.3. Signaling Secondary LSPs > > Support for extra traffic in SMP is for further study. Therefore, > mechanisms to set up LSPs for extra traffic are also for further > study. > > In the second sentence, you probably want to use the conventional > phrase "are outside the scope of this document" in place of "are also > for further study". > > 5.4. SMP Preemption Priority > > In SMP, the Setup and Holding priorities in the SESSION_ATTRIBUTE > object can be used by GMPLS to control LSP preemption, but, they are > not used by the APS to resolve the competition among multiple > protecting LSPs. > > This is probably clear to people who fully understand all of these > protocols. But naively, I see that in SMP, GMPLS does not do LSP > preemption. Then the interpretation is that GMPLS distributes the > Setup and Holding priorities, which APS then uses to resolve > competition that arises during preemption. But the last clause states > that APS does not use those priorities. I suspect that "In SMP, > ... can be used by GMPLS to control LSP preemption" doesn't mean what > I think it does (that is, control how APS does preemption). Perhaps > there is a better way of phrasing it. > > When resource competition among > multiple protecting LSPs occurs, the APS protocol will use their > priority values to resolve the competition. > > Since this section introduces the priority field, it's worth stating > here "A lower value has a higher priority." or appending "..., with a > lower value indicating a higher priority.". > > In SMP, a preempted LSP MUST NOT be torn down. > > Perhaps "torn down" has a specific meaning, but naively, a preempted > LSP has its resources de-allocated, and so is "torn down". I think > you could make this clearer as > > In SMP, a preempted LSP MUST NOT be torn down even after its > resources have been deallocated. > > But perhaps there is a more specific term that could be used instead > of "torn down". > > 5.5. Notifying Availability of Shared Resources > > When the shared resources become unavailable, including a case of the > shared resources failure, [...] > > Perhaps clearer as > > When any shared resources become unavailable for any other reason > (e.g. failure of the shared resources), [...] > > 5.6. SMP APS Configuration > > In order to allow exchange of APS protocol messages, an APS channel > has to be configured between adjacent nodes along the path of the SMP > protecting LSP. This should be done before any SMP protecting LSP > has been set up by other means than GMPLS signaling which are > therefore outside the scope of this document. > > This is unclear to me. Does "by other means than GMPLS signaling" > modify "SMP protecting LSP has been set up", as it appears to? It > seems to me to be likely that the meaning is > > This is done by other means than GMPLS signaling, before any SMP > protecting LSP has been set up. This means is outside the scope of > this document. > > -- > > Therefore, additional requirements > for APS configuration are outside the scope of this document. > > I think this can be clarified as > > Therefore, there are likely additional requirements > for APS configuration which are outside the scope of this document. > > 10. Contributor > > The following person contributed significantly to the content of this > document and should be considered as a co-author. > > This is an unusual statement. Presumably there is a good reason the > expedient of listing Yuji Tochio as a co-author has not been taken. > > [END] > > > > _______________________________________________ > Gen-art mailing list > Gen-art@xxxxxxxx > https://www.ietf.org/mailman/listinfo/gen-art
Attachment:
signature.asc
Description: Message signed with OpenPGP
-- last-call mailing list last-call@xxxxxxxx https://www.ietf.org/mailman/listinfo/last-call
