Re: [Teas] AD review of draft-ietf-teas-mpls-tp-rsvpte-ext-associated-lsp

["Rakesh Gandhi (rgandhi)" <[e-mail]>]

Hi all,

We have posted a new revision (01) to address the comments listed below
(except the last one, <RG2>).

Welcome your review comments.

Many thanks,
Rakesh


On 2015-01-24 10:23 AM, "Rakesh Gandhi (rgandhi)" <[e-mail]>
wrote:

>Thank you Adrian for your detailed review comments.
>
>Please see inline .. <RG>..
>
>On 2015-01-22 9:37 AM, "Adrian Farrel" <[e-mail]> wrote:
>
>>Authors,
>>                 
>>Thanks for this document which I found clear and not too long.
>>
>>I have done my usual AD review to try to flush out any issues that
>>might otherwise show up in IETF last call or IESG review.
>>
>>The only largish issue I have is about error conditions. Pretty much the
>>only error handling I see is in 5.3 where you have:
>>
>>   An egress node, upon receiving a Path message containing an
>>   ASSOCIATION or Extended ASSOCIATION Object with Association Type set
>>   to "Single Sided Associated Bidirectional LSP" MUST create an LSP in
>>   the reverse direction or reject the Path message by sending a
>>   PathErr.
>>
>>...but you don't say what reason code to give.
>>
>>You need to handle some other error conditions such as...
>>
>>**** race conditions
>>**** what if the association is triggered by both ends at the same time?
>>**** what if the reverse dirn LSP has already been associated with
>>     something else?
>>
>>**** failure conditions
>>**** what if not willing or able to associate?
>>**** what if set-up of reverse LSP fails?
>>**** what if requested REVERSE LSP parameters are not acceptable?
>>**** what if the remote end point doesn't support the association
>>
>>     mechanism defined here? how will the initiator know?
>
>
><RG> Please refer to another email thread to address this comment.

<RG2> Updated in the new revision.

>
>
>>
>>
>>I also have some smaller points as set out below.
>>
>>I'll put the I-D into "revised I-D needed" state while we discuss these
>>points and while you produce an update.
>>
>>Thanks for the work,
>>Adrian
>>
>>====
>>
>>You have a different number of font-page authors and names in the
>>Authors' Addresses section.
>>
>>Looks like Fan Yang and Weilian Jiang should be moved to a
>>Contributors section.
>
><RG> Fixed.
><RG> FYI, I also fixed WG from CCAMP to TEAS at the top.
>
>>
>>---
>>
>>Section 1 is OK, but it would have been better to just list the relevant
>>requirements (7, 11, 12, 50) and then the additional requirement (14).
>>
>>Probably no need to change now, but my preference is to avoid the risk
>>of introducing an error during the copying or editorial process. If you
>>feel like making this change, I would be happier.
>
><RG> Find it easier to read the document when this text is in the same
>document. Prefer to keep it:)
>
>
>>
>>---
>>
>>Because you use RBNF in section 4, you need to add a statement to
>>Section 2 like:
>>
>>   This document uses the Routing Backus-Naur Form (RBNF) to define
>>   message formats as defined in [RFC5511].
>>
>>And then, of course, add a normative reference.
>
><RG> Added.
>
>>
>>---
>>
>>I think 2.1.1 is missing an important statement about co-routing.
>>
>>"A pair of reverse unidirectional LSPs that are associated to form an
>>associated bidirectional LSP do not necessarily follow the same path
>>through the network. If they do follow the same path, they are known as
>>'co-routed'."
>
>
><RG> Saw the email exchange with Lou. Thanks Lou. I am assuming we are
>keeping the text as is.
><RG> Note that this document does not discuss anything about co-routed
>LSPs.
>
>
>>
>>---
>>
>>3.1
>>
>>   This section provides an overview and definition of the models for
>>   provisioning bidirectional LSPs.
>>
>>This is missing the word "associated" to read "associated bidirectional"
>
><RG> Added.
>
>>
>>---
>>
>>Shouldn't 3.1.1 also mention the REVERSE_LSP Object. Something like...
>>
>>   For the single sided provisioning, the Traffic Engineering (TE)
>>   tunnel is configured only on one endpoint.  An LSP for this tunnel is
>>   initiated by the initiating endpoint with the (Extended) ASSOCIATION
>>   Object inserted in the Path message.  The other endpoint then creates
>>   the corresponding reverse TE tunnel and signals the reverse LSP in
>>   response using information from the REVERSE_LSP Object if present.
>
><RG> Yes, added.
>
>>
>>---
>>
>>3.1.2
>>
>>Pedantry, but...
>>
>>OLD
>>   For the double sided provisioning, two unidirectional TE tunnels are
>>   configured independently on both endpoints.
>>NEW
>>   For the double sided provisioning, two unidirectional TE tunnels are
>>   configured independently, one on each endpoint.
>>END
>
><RG> Added. 
>
>>
>>---
>>
>>3.2
>>
>>   This section provides an overview of the association signaling
>>   methods for the bidirectional LSPs.
>>
>>Again, this is missing "associated"
>
><RG> Added.
>
>>
>>---
>>
>>3.2.1
>>
>>OLD
>>   LSP1 is then signaled with an (Extended) ASSOCIATION Object inserted
>>   in the Path message, in which the Association Type indicating single
>>   sided provisioning.
>>NEW
>>   LSP1 is then signaled with an (Extended) ASSOCIATION Object inserted
>>   in the Path message, in which the Association Type indicating single
>>   sided provisioning is included.
>>END
>
><RG> Fixed.
>
>>
>>---
>>
>>3.2.2
>>
>>OLD
>>   For the double sided provisioning model, both LSP1 and LSP2 are
>>   signaled independently with (Extended) ASSOCIATION Object inserted in
>>   the Path message, in which the Association Type indicating double
>>   sided provisioning.
>>NEW
>>   For the double sided provisioning model, both LSP1 and LSP2 are
>>   signaled independently with (Extended) ASSOCIATION Object inserted in
>>   the Path message, in which the Association Type indicating double
>>   sided provisioning is included.
>
><RG> Fixed.
>
>>
>>---
>>
>>3.2.2 needs to include some comment about how the two LSPs are selected
>>to be associated. I think this is simply "by management action applied
>>at both end points", although I wonder whether it is enough to apply the
>>action at just one end point because the association object will achieve
>>the desired result.
>>
>>Please think about this and add a note.
>
>
><RG> This is somewhat in Section 3.1.2.
><RG> We could add something like:
>The LSPs to be selected for association are provisioned by the management
>action applied at both endpoints.
>
>
>
>>
>>---
>>
>>Section 3.3.1 is fine as it is, but I think that it makes it look like
>>the REVERSE LSP object is only used for asymmetric b/w. Perhaps...
>>
>>OLD
>>   As
>>   described in this document, addition of the REVERSE_LSP Object also
>>   allows the initiating node to control the reverse LSP by including
>>   other existing objects in a REVERSE_LSP Object.
>>NEW
>>   As
>>   described in Section 4.4, addition of the REVERSE_LSP Object also
>>   allows the initiating node to control other aspects of the reverse
>>   LSP (such as its path) by including other subobjects in a REVERSE_LSP
>>   Object.
>>END
>
><RG> Fixed.
>
>>
>>---
>>
>>For clarity 3.4 needs to observe that multiple association objects may
>>be present in the signaling of a single LSP.
>
><RG> Adding following:
> An LSP MAY signal multiple (Extended) ASSOCIATION Objects with different
>Association Types.
>
>
>>
>>---
>>
>>4.2
>>
>>      The new Association Types are defined as follows (values are
>>      temporary early allocations as per RFC7120):
>>
>>Delete the bit in brackets, please, so that we don't accidentally find
>>it in the published RFC. Your message is already in 6.1 (which is good).
>
><RG> Fixed.
>
>>           
>>---
>>
>>4.4.1
>>
>>   Class_Num = 203 (of the form 11bbbbbb), C_Type = 1 (values are
>>   temporary early allocations as per RFC7120)
>>
>>Same thing. Please remove the bit about early allocation.
>>
>>(BTW, you have "of the form 11bbbbbb" twice in this section.
>
><RG> Fixe both.
>
>>
>>---
>>
>>4.4.2 or 5.2
>>
>>Maybe...
>>
>>   The REVERSE_LSP object MUST NOT be included in a REVERSE_LSP object.
>
><RG> Added in Section 5.2.
>
>>
>>---
>>
>>5.2
>>
>>   An
>>   egress node MUST tear down and reestablish a new reverse LSP when
>>   REVERSE_LSP Object is either added or removed in the received Path
>>   message.
>>
>>That's confusing!
>>- Why is modification of an existing LSP not allowed?
>>- Why does removal of the REVERSE_LSP object require tearing down and
>>  reestablishment since all that has happened is that the requirements
>>  for the reverse LSP have been relaxed?
>>- Why does addition of the REVERSE_LSP necessarily require tear down
>>  and reestablishment when the existing LSP might already satisfy the
>>  requirements?
>>- Is make-before-break used or is this assumed to be an out-of-service
>>  operation? If out-of-service you need to describe how that is
>>  achieved (e.g., using LI).
>
>
><RG> Understand and agree with above comments. There are various scenarios
>where change may lead to in-place modification, MBB or setup and teardown
>(which may depend on implementation).
>
><RG> How about something like following:
>An egress node MAY tear down and reestablish a new reverse LSP, or trigger
>re-optimization or in-place modification when
>   REVERSE_LSP Object is either added or removed in the received Path
>   message (which may depend on the local policy).
>
>
>>
>>---
>>
>>5.3 has
>>
>>   If REVERSE_LSP Object is not present in the received Path message of
>>   the LSP, the egress node SHOULD use the LSP properties from the
>>   received LSP Path message to signal the LSP in the reverse direction
>>   (which may depend on the local policy).
>>
>>This worries me. Do you mean that in the absence of a REVERSE_LSP object
>>containing an ERO, the reverse LSP MUST be co-routed with the forward
>>LSP?
>>
>


<RG2> Ack, Will address this comment.


>
>
>
>
>Many thanks Adrian for the great review.
>
>Regards,
>Rakesh
>
>
>>
>