Re: [bess] Shepherd's review of draft-ietf-bess-mvpn-fast-failover

[<[e-mail]>]

Hi Greg,

 

I wish you an happy new year.

 

Thanks for your feedback, please find my replies inline.

(Trimming solved issues)

 

Brgds,

 

 

From: Greg Mirsky <[e-mail]> 
Sent: mercredi 4 décembre 2019 23:22
To: [e-mail]; BESS <[e-mail]>; [e-mail]
Subject: RE: Shepherd's review of draft-ietf-bess-mvpn-fast-failover

 

Hi Stephane,

thank you for the review and your thoughtful comments. Please find my answers and notes in-lined under GIM>> tag.

Attached, please find the diff and copy of the working version.

 

Regards,

Greg

 

Hi,

 

Please find below my review of the document.

 

Nits:

 


Section 3.1.1: 

As the document is standard track, could you introduce normative language in the expected behavior description ?


GIM>> I can propose the following modification of the text: 

 OLD TEXT:

   If BGP next-hop tracking is done for VPN routes and the root address
   of a given tunnel happens to be the same as the next-hop address in
   the BGP auto-discovery route advertising the tunnel, then this
   mechanisms may be omitted for this tunnel, as it will not bring any
   specific benefit.

NEW TEXT:

   If BGP next-hop tracking is done for VPN routes and the root address
   of a given tunnel happens to be the same as the next-hop address in
   the BGP auto-discovery route advertising the tunnel, then this
   mechanisms may be omitted for this tunnel, as it will not bring any
   specific benefit.

 

 

[SLI] I don’t see a difference between the two.

I was expecting a “MAY” instead of “may”.

 


Section 3.1.2:

As the document is standard track, could you introduce normative language in the expected behavior description ?

 

“This method should not be used”. Wouldn’t this be a normative statement ?

GIM>> Would the following modification of the text be acceptable:

OLD TEXT:

   This method should not be used when there is a fast restoration
   mechanism (such as MPLS FRR [RFC4090]) in place for the link.

NEW TEXT:
    Using this method when a fast restoration mechanism (such as MPLS FRR
   [RFC4090]) is in place for the link requires careful consideration
   and coordination of defect detection intervals for the link and the
   tunnel.  In many cases, it is not practical to use both methods at
   the same time.

 

[SLI] Are we strongly disencouraging the practice ? if yes, “it is not practical” is a bit too soft. I’m wondering if “is NOT RECOMMENDED” could be a good wording. But it is up to you.

 



Section 3.1.4:

As the document is standard track, could you introduce normative language in the expected behavior description ?
GIM>> Updating to the normative language as follows:

OLD TEXT:

   A PE can be removed from the UMH candidate list for a given (C-S,
   C-G) if the P-tunnel (I or S, depending) for this (S, G) is leaf
   triggered (PIM, mLDP), but for some reason internal to the protocol
   the upstream one-hop branch of the tunnel from P to PE cannot be
   built.  In this case, the downstream PE can immediately update its
   UMH when the reachability condition changes.

NEW TEXT:

   A PE MAY be removed from the UMH candidate list for a given (C-S,
   C-G) if the P-tunnel (I-PMSI or S-PMSI) for this (S, G) is leaf-
   triggered (PIM, mLDP), but for some reason internal to the protocol
   the upstream one-hop branch of the tunnel from P to PE cannot be
   built.  In this case, the downstream PE MAY immediately update its
   UMH when the reachability condition changes.
 

[SLI] I understand the first “MAY” as optional feature, however the second “MAY” is more a “SHOULD” IMO. Thoughts? 

 



 





 

Section 3.1.5:

As the document is standard track, could you introduce normative language in the expected behavior description ?
GIM>> I propose the following update to the second paragraph of this sub-section:

OLD TEXT:

   When such a procedure is used, in the context where fast restoration
   mechanisms are used for the P-tunnels, downstream PEs should be
   configured to wait before updating the UMH, to let the P-tunnel
   restoration mechanism happen.  A configurable timer MUST be provided
   for this purpose, and it is recommended to provide a reasonable
   default value for this timer.

NEW TEXT:

   When such a procedure is used, in the context where fast restoration
   mechanisms are used for the P-tunnels, a configurable timer MUST be
   configured on the downstream PE to wait before updating the UMH, to
   let the P-tunnel restoration mechanism happen.  It is RECOMMENDED to
   provide a reasonable default value for this timer.  An implementation
   MAY use three seconds as the default value for this timer.



[SLI] Do you want to standardize the default timer value ? If yes, a “SHOULD” will be better than “MAY”.

 

 






 
Section 3.1.6:

As the document is standard track, could you introduce normative language in the expected behavior description ?

GIM>> Sub-sections of 3.1.6 define the use of RFC 8562 and the new attribute. In the introduction to these sub-sections, I propose s/can/MAY/





>From a wider perspective, do you foresee other use case of signaling BFD information in BGP ? I’m just wondering if we may need something extensible for future use or not.

GIM>> Great question. BGP, and I'm speculating here, may be used to for other BFD-related scenarios. I think that we may use the Flags field.
[SLI] Is it enough or should you add some optional TLVs behind the discriminator ? (with nothing defined yet).



[SLI] New typo s/Reserved fieled/Reserved field/

 


 

Section  <http://3.1.6.2> 3.1.6.2:

s/Then The/ Then the/

GIM>> Thank you. Done.

“A different p2mp BFD session MAY monitor the state of the
   Standby Upstream PE. »




 
Section 3.1.7

This is not clear to me.

Are you using BFD only to signal a Down state without really “probing” ?
GIM>> This section describes how the existing p2Mp BFD session from the upstream PE to downstream PEs can be used to relay the state change (failure) of the upstream PE-CE link. The exact mechanism to detect the failure of the PE-CE link is not specified though it could be a single-hop BFD session. 
 

Section 4

“This non-revertive behavior can also be optionally supported by an
   implementation and would be enabled through some configuration.”

 





Section 4.1:


It would be great to add a text talking about what happens if an implementation does not understand the standby community. 

GIM>> As I understand, if a BGP peer does not support Standby PE community then it does not support this specification. If that is the case, then the following may happen (as described in the document):

... two different downstream PEs consider different upstream PEs to be the

   primary one; in that case, without any precaution taken, both
   upstream PEs would process a standby C-multicast route and possibly

   stop forwarding at the same time.

Would adding to "Such a situation can happen when, for instance, due to transient unicast routing inconsistencies ..." "or lack of support of the Standby PE community" address your concern?



[SLI] Yes it works.

 

[SLI] The text talks about “Standby PE” attribute while it is a BGP community.  This should be fixed to be consistent.

 

 

[SLI] As soon as we agree, I’ll send the draft to be proof-readed by IDR before moving to IESG.