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 <[email protected]> Sent: mercredi 4 décembre 2019 23:22 To: [email protected]; BESS <[email protected]>; [email protected] 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.
_______________________________________________ BESS mailing list [email protected] https://www.ietf.org/mailman/listinfo/bess
