Re: [Teas] Benjamin Kaduk's practice ballot (No Objection) on draft-ietf-teas-rsvp-egress-protection-13 (with COMMENT)

[Huaimo Chen <[e-mail]>]

Hi Benjamin,

    Thank you very much for your time and valuable comments, which improve the quality of the document.
    Answers to them are inline below with prefix [HC].
    Sorry for the slow response.

Best Regards,
Huaimo
-----Original Message-----
From: Benjamin Kaduk [mailto:[e-mail]] 
Sent: Thursday, March 08, 2018 8:46 AM
To: Huaimo Chen <[e-mail]>
Cc: The IESG <[e-mail]>; [e-mail]; [e-mail]; Vishnu Beeram <[e-mail]>; [e-mail]
Subject: Re: Benjamin Kaduk's practice ballot (No Objection) on draft-ietf-teas-rsvp-egress-protection-13 (with COMMENT)

Hi Huaimo,

Sorry for the slow response.

On Sat, Mar 03, 2018 at 04:25:10AM +0000, Huaimo Chen wrote:
> Hi Benjamin,
> 
>     Thank you very much for your time and your valuable comments.
>     Answers to your comments are inline below with prefix [HC].
>     Would you mind reviewing them to see whether they address your comments?
> 
> Best Regards,
> Huaimo
> > -----Original Message-----
> > From: Benjamin Kaduk [mailto:[e-mail]]
> > Sent: Friday, March 02, 2018 2:26 PM
> > To: The IESG <[e-mail]>
> > Cc: [e-mail]; 
> > [e-mail]; Vishnu Beeram 
> > <[e-mail]>; [e-mail]
> > Subject: Benjamin Kaduk's practice ballot (No Objection) on 
> > draft-ietf-teas-rsvp-egress-protection-13 (with COMMENT)
> >
> > [incoming AD ramping up on document reviews; please consider this as 
> > if it was a ballot of No Objection with COMMENT]
> >
> > In general it's hard for me to get excited about publishing this document as-is, though most of this is not in areas that qualify for a DISCUSS ballot.  Perhaps the suggested security considerations changes could qualify for a DISCUSS, though.
> >
> > Mostly I feel that the overall document organization is a little jumbled and potentially under-specified.  I'd like to see in the Introduction some overview of the different ways this technology could be deployed -- it seems like there are options for both the ingress and the primary egress to initiate its use, and it could be helpful to get some advance indication of which subsections correspond to which of those.
> [HC] We have added some overview and references according to your suggestions as below:
> In the beginning, the primary P2MP LSP from ingress R1 to primary 
> egresses L1 and L2 is configured.  It may be used to transport the 
> traffic from source S connected to R1 to destinations CE1 and CE2 
> connected to L1 and L2 respectively.
> 
> When we want to protect the primary egresses L1 and L2, we may 
> configure on the ingress R1 a backup egress for L1, another backup 
> egress for L2 and other options.  After the configuration, the ingress 
> sends a Path message for the LSP with the information such as SEROs 
> (refer to section 4.1) containing the backup egresses for protecting 
> the primary egresses.
> 
> After receiving the Path message with the information, the upstream 
> node of a primary egress sets up a backup LSP to the corresponding 
> backup egress for protecting the primary egress.
> 
> ......
> 
> A backup egress MUST be configured on the ingress of an LSP to protect 
> a primary egress of the LSP if and only if the backup egress is not 
> configured on the primary egress (refer to section 5.2).
> ......
> 
> To protect a primary egress of an LSP, a backup egress MUST be 
> configured on the primary egress of the LSP to protect the primary 
> egress if and only if the backup egress is not configured on the 
> ingress of the LSP (refer to section 5.1).

That helps, thank you.

I think I also wanted a statement like "The decision to instantiate a backup egress for an LSP can be initiated by either the ingress or the primary egress for that LSP, but not both."  (Presumably before the previous two new sentences.)

[HC] We will add the text accordingly as follows (The text is added before the previous two new sentences as you suggested):
The decision to instantiate a backup egress for protecting the
primary egress of an LSP can be initiated by either the ingress or
the primary egress of that LSP, but not both.



> >
> > I also would like to see a little discussion about why this new protocol is useful -- I can understand what it *does*, but why is putting the PLR closer to egress beneficial, and why is the egress considered more likely to fail than an arbitrary node on the LSP?
> [HC] Putting the PLR closer to egress makes detecting the failure of 
> the egress faster and more reliable. Thus we can have faster 
> protection for egress. We have added some text accordingly as follows:
> The PLR R3 is closer to L1 than the ingress R1.  It may detect the 
> failure of the egress L1 faster and more reliable.  Thus we can have 
> faster protection for egress.

Thanks!

> 
> The transit nodes on the LSP may be protected quickly by RFC 4090. 
> However, RFC 4090 can not be used to protect the egress node of the 
> LSP (refer to the first paragraph in section 1).
> 
> >
> > There are also some areas where the organization seems surprising, 
> > such as Section 4.1 where we learn that the SERO to be used for this 
> > new protection scheme will have three subobjects, but we only get 
> > told what the first and third subobjects will do, not what type(s) 
> > they might be.  The second subobject does get a diagram for its 
> > structure, since it's new in this document, but the discussion of 
> > the third subobject in the SERO is interspersed between that 
> > description and the structure of the optional subobjects that it 
> > could contain (which could confuse the reader into thinking that 
> > these optional subobjects being defined are candidates for the third 
> > subobject in the SERO and not sub-subobjects within the C-Type 3 
> > subobject.  (The mathematician in me gets unhappy about seeing a 
> > thing called a "subobject" that itself has "subobjects" that are a 
> > different semantic type, but I expect this is common in the field 
> > and I should just get used to it.)
> [HC] We have put the descriptions for the second subobject together in 
> the document.

Great!  (Maybe also mention that their contents are described in sections 4.1.1 through 4.1.2.2, which I think also relates to a question from Eric.)

[HC] We will add the text accordingly as below (before "They have the following format:"):
Their contents are described in sections 4.1.1 through 4.1.2.2.


> >
> > There are also several places where we are told of a need to 
> > construct new SERO (or other) (sub)objects but only told about
> > (presumably) the "new" parts of them, with no description or reference for how the remaining parts should be constructed.
> [HC] We have added some references accordingly as below:
> The first subobject (refer to RFC 4873) indicates the branch node 
> ......
> 
> The third (final) subobject (refer to RFC 4873) in the SERO contains 
> ......

That helps, thanks.  It would be even better to have section numbers and/or type names from within RFC 4873, at least for me.  (Maybe people using this document are already sufficiently familiar with the material that it doesn't really matter.)

[HC] We will add more details accordingly as follows:
The first subobject (refer to RFC 4873 Section 4.2) indicates the branch node
......
The third (final) subobject (refer to RFC 4873 Section 4.2) in the SERO contains
......


> >
> >
> > Some more specific comments follow:
> >
> >
> > Section 4.1 says "Reserved parts MUST set to zero" (missing a "be"), but these should also be ignored by the recipient, presumably?
> [HC] We have added "be" accordingly as follows:
> The Reserved parts MUST be set to zero.

Should a peer that receives a message with the Reserved parts not set to zero abort the connection, ignore the entire message, or proceed as if the reserved bits were zero?

[HC] Proceed as if the reserved bits were zero. The related text will be updated as follows:
The Reserved parts MUST be set to zero on transmission and MUST be
ignored on receipt.


> >
> > Why do the optional subobjects get a 17-bit length when the outer 
> > subobject only has an 8-bit length?  (Also, if the length is 
> > intentionally 17 bits instead of 16, that should probably be called 
> > out as not an error in the figure.)
> [HC] We have changed it to 8-bit accordingly as below:
>       0                   1                   2                   3
>       0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1
>      +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
>      |      Type     |    Length     |           Reserved (zero)         |
>      +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
> 
> >
> > I would prefer to see a table of the type values for the optional subobjects instead of spreading them throughout the subsections, but concede that it is an editorial point.
> [HC] There is a table for the type values for the optional subobjects 
> in section " IANA Considerations" as follows:
> 
>      Value      Name                     Definition
>      -----      ----                     ----------
>       0         Reserved
>       1         IPv4_PRIMARY_EGRESS      Section 4.1.1
>       2         IPv6_PRIMARY_EGRESS      Section 4.1.1
>       3         IPv4_P2P_LSP_ID          Section 4.1.2
>       4         IPv6_P2P_LSP_ID          Section 4.1.2

Ah, I must have missed it in my first read-through.

> > For the "new SERO based on the SERO received" (in the second-to-last paragraph of section 4.1), what is the first subobject of the three?
> [HC] It is the one in the SERO received. We have added some text about 
> this accordingly as below:
> The first subobject in the new SERO indicates the upstream node, which 
> may be copied from the first subobject in the SERO received.

Excellent, thanks!

> >
> > Section 5.1:
> >
> >     The ingress MUST send a Path message for the LSP with the objects
> >     above and the SEROs for protecting egresses of the LSP.
> >
> > Presumably this is only if protection of the egress is desired?
> [HC] Yes. You are right. We have added some text for this as follows:
> The ingress MUST send a Path message for the LSP with the objects 
> above and the SEROs for protecting egresses of the LSP if protection 
> of the egresses is desired.
> >
> >
> > Section 5.2
> >
> >    If the LSP carries the service traffic with a service label, the
> >    primary egress sends its corresponding backup egress the information
> >    about the service label as a UA label and the related forwarding.
> >
> > Do you want to say anything about what protocol messages are used for this or give a reference?  Or is this the same sort of thing labelled as out-of-band or out-of-scope later in the document?
> [HC] This is the same sort of thing labelled as out-of-band or 
> out-of-scope later in the document.
> >
> >
> > Section 5.4
> >
> > I assume "upstream node" means the PLR, i.e., directly upstream (as opposed to just on the LSP) -- is this a term of art or should it be clarified?
> [HC] This seems a term of art.

Okay.

> >
> > Section 5.4.2
> >
> >    It indicates that the primary egress MUST send the backup egress
> >    the service label as UA label and the information about forwarding
> >    the traffic to its destination using the label if there is a service
> >    carried by the LSP and the primary LSP label as UA label if the label
> >    is not implicit null.  How UA label is sent is out of scope for this
> >    document.
> >
> > It's hard to be happy about MUST-level requirements that require the implementor to do their own protocol design.  Or could this be done by BGP, as implied later in the document?
> [HC] Yes.

Okay.

> >
> > Section 5.4.4
> >
> >    Moreover, the PLR MUST let the upstream part of the primary LSP stay
> >    after the primary egress fails through sending Resv message to its
> >    upstream node along the primary LSP.
> >
> > What does "stay" mean here?
> [HC] "stay" means "stay alive". We have revised the related text 
> accordingly as below:
> Moreover, the PLR MUST let the upstream part of the primary LSP stay 
> alive after the primary egress fails through sending Resv message to 
> its upstream node along the primary LSP.

Okay.

> >
> > Section 6.1, Figure 3, item 4 makes me think that the use or a virtual node and binding the same local address on both L1 and La is mandatory, but the earlier text in section 5.4 made me think that the use of a virtual node was optional, only needed in the case when the backup egress is not configured explicitly for protecting a primary address.  Is the use of a virtual node mandatory or not?
> [HC] It is optional.

I don't see a good way to change the text for item 4 in the description associated with figure 3 that would indicate that the use of the virtual node is only one possible solution, so perhaps this should just stay as-is.
[HC] Keep as-is.

> >
> > The security considerations refers to RFC 4090, which itself claims to add no new security considerations other than those of RFC 2205 (and then says a little bit more anyway).  Probably this document should reference RFC 2205 directly.  RFC 4875's security considerations also have this "In principle this document does not introduce any new security issues above those defined in [...]", followed by two additional paragraphs of additional security considerations.  It seems simpler to just say something like "The security considerations of RFCs 4090, 2205, and 4875 all apply here.  Additionally, [...]".  The cross-protocol identity management mentioned in this document is indeed a key issue, and could probably be worded more clearly.  Perhaps:
> >
> >     This document builds upon existing work, so in particular the security
> >     considerations of RFCs 4080, 2205, and 4875 continue to apply.  Additionally,
> >     this protocol introduces a requirement for out-of-band communication between
> >     primary egress and secondary egress, in order to convey UA label and LFIB
> >     information.  It is important to confirm that the identifiers used to identify
> >     the primary and backup egress nodes in the LSP are verified to match with the
> >     identifiers used in the out-of-band protocol (such as BGP).
> >
> > I would also encourage adding some text about providing integrity protection for the out-of-band channel between primary and backup egress (it's less clear to me whether confidentiality protection is desired), including the obligatory disclaimer about the sad state of affairs for BGP.
> [HC] We have updated the document according to your suggestions as follows:
> This document builds upon existing work, so in particular the security 
> considerations of RFCs 4090, 4875, 3209 and 2205 continue to apply.  
> Additionally, protecting a primary egress of a P2P LSP carrying 
> service traffic through a backup egress requires an out-of- band 
> communication between the primary egress and the backup egress, in 
> order for the primary egress to convey a service label as UA label and 
> its related forwarding information to the backup egress.  It is 
> important to confirm that the identifiers used to identify the primary 
> and backup egress nodes in the LSP are verified to match with the 
> identifiers used in the out-of-band protocol (such as BGP).

Thanks!

> >
> > The IANA considersations are a bit sparse, in particular a descriptive name could probably be used instead of just "Type 3" (the precedent notwithstanding), and IANA likes to get more information when creating new (sub)registries.  It's also useful to cite RFC 8126 for the "IETF Review"
> > policy.
> [HC] We have revised the related text accordingly as below:
>      Value      Description               Definition
>      -----      -----------               ----------
>       3         Egress Protection         Section 4.1.1
> 
>    IANA is to create and maintain a new registry under PROTECTION object
>    class (Class Number 37) and Egress Protection (C-Type 3).

Thanks.

-Benjamin