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

[Greg Mirsky <[e-mail]>]

Hi Stephane,
I've followed your suggestions and updated the working version of the draft
with adding:

   - explicit TLV in the format of BFD Discriminator attribute;
   - text to define check the proper format of the BFD Discriminator
   attribute and the handling in case it is malformed per RFC 7606.

I greatly appreciate your feedback. Attached please find the diff to the
updated working version of the draft and its copy.

Regards,
Greg

On Tue, Jan 7, 2020 at 5:59 AM <[e-mail]> wrote:

> Hi Greg,
>
>
>
> More inline,
>
>
>
>
>
> *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 ?
>
>
>
> GIM2>> My apologies, I've pasted the same text twice. I propose to remove
> "may be omitted" altogether. Hence the updated 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 the use of
> this
>
>    mechanism for the tunnel will not bring any specific benefit.
>
> Do you see this version without any normative language as acceptable?
>
>
>
> [SLI] Looks good thanks
>
>
>
>
>
>
> 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.
>
> GIM2>> The use of OAM in multi-layer fashion is a question I'd be
> interested to discuss. But I feel that it deserves a separate document and
> would prefer to leave the text as a note of caution for now.
>
>
>
> [SLI] Ok
>
>
>
>
>
> 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?
>
> GIM2>>  Thank you for the clarification. The UMH list will certainly be
> updated once the reachability of the downstream PE changes. In some
> scenarios, such an update may be immediate, i.e., ASAP, but in some, it
> might be better to delay it. Would you suggest adding a note about the
> option to delay the update?
>
>
>
> [SLI] Could you check with Jeffrey Zhang on this point ? I’m not enough
> expert here to tell what may be the best option. On my side, I just want
> the text to be clear 😊
>
>
>
>
>
>
>
>
>
>
>
> 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).
>
> GIM2>> Great idea, thank you! Please see the updated figure and the text:
>
>        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
>       +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
>       |    BFD Mode   |                  Reserved                     |
>       +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
>       |                       BFD Discriminator                       |
>       +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
>       |                         Reserved  TLV                         |
>       +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
>
>
>
>                  Format of the BFD Discriminator Attribute
>
>    Where:
>
>       BFD Mode is the one octet long field.  This specification defines
>       the P2MP value (TBA3) Section 7.1.
>
>       Reserved field is three octets long and the value MUST be zeroed
>       on transmission and ignored on receipt.
>
>       BFD Discriminator is four octets long field.
>
>       Reserved TLV field is four octets long.  It MAY be used for future
>       extensions of the BFD Discriminator Attribute using Type-Length-
>       Value format.  This specification defines that the value in
>       Reserved TLV field MUST be zeroed on transmission and ignored on
>
>       receipt.
>
>
>
> [SLI] If your field is 4-bytes long, it is not extensible, I was thinking
> of options encoded as TLVs.
>
> If there is no TLV, the attribute ends on BFD discriminator, the attribute
> length should tell if there are TLVs or not.
>
>
>
>        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
>       +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
>       |    BFD Mode   |                  Reserved                     |
>       +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
>       |                       BFD Discriminator                       |
>       +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
>       |                         optional TLVs                         |
>       +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
>
> Another point I have missed, you should define error handling procedures
> for your attribute as per RFC7606.
>
>
>
>
>
>
>
>
>
>
>
>
>
>
>
>
>