Re: [Gen-art] Genart last call review of draft-ietf-teas-gmpls-signaling-smp-10

[Lars Eggert <[e-mail]>]

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 <[e-mail]> 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
> [e-mail]
> https://www.ietf.org/mailman/listinfo/gen-art