Subject: [bess] Alvaro Retana's Discuss on draft-ietf-bess-mvpn-fast-failover-13: (with DISCUSS and COMMENT)

From: Alvaro Retana via Datatracker <[email-masked]>
Date: Tue, 15 Dec 2020 15:09:54 -0800

Alvaro Retana has entered the following ballot position for
draft-ietf-bess-mvpn-fast-failover-13: Discuss

When responding, please keep the subject line intact and reply to all
email addresses included in the To and CC lines. (Feel free to cut this
introductory paragraph, however.)

Please refer to https://www.ietf.org/iesg/statement/discuss-criteria.html
for more information about IESG DISCUSS and COMMENT positions.

The document, along with other ballot positions, can be found here:
https://datatracker.ietf.org/doc/draft-ietf-bess-mvpn-fast-failover/

----------------------------------------------------------------------
DISCUSS:
----------------------------------------------------------------------

(1) This document describes several methods to determine the status of a
tunnel (in §3), none of which "provide a "fast failover" solution when used
alone, but can be used together with the mechanism described in Section 4"
(§1).  §3 also says this:

   An implementation may support any combination of the methods
   described in this section and provide a network operator with control
   to choose which one to use in the particular deployment.

While §3.1 is clear in the fact that it is not a requirement for all
downstream PEs to use the same mechanism, there are no guidelines to aid the
operator to chose which mechanism to use.  Some cases may be obvious (e.g.
§3.1.3 applies to tunnels of a specific type), but others are not.  I would
like to see deployment considerations related to the advantages/disadvantages
that each method may have in specific situations (including their possible
combination).

(2) The BFD Discriminator Attribute has a very narrow application in this
document when compared to the potential other uses given the extensibility
possibilities related to bootstrapping BFD.  I have serious concerns about
the attribute being defined in this document, amongst a series of other
mechanisms.

(2a) The tunnel can be monitored without the new BGP Attribute (assuming
proper configuration of course).  Why is that option is not even mentioned in
the document?

In fact, the document recommends deleting the BFD session if the Attribute is
not present.  Why is that recommendation in place, and what are the cases when
it can not be followed?

(2b) The fact that BFD monitoring can be achieved without the new attribute
makes me think that the bootstrapping of BFD using BGP would be better served
in a document produced by the BFD WG.  One of the editors has expressed the
same opinion [1] [2].  Has a discussion taken place in the BFD WG (or at least
with the Chairs) about this work?  Why was it not taken up there?

[1] https://mailarchive.ietf.org/arch/msg/rtg-bfd/T1jVpgyXuPatTpuD_wA0JC3CT1c/
[2] https://tools.ietf.org/wg/bess/minutes?item=minutes-96-bess.html

----------------------------------------------------------------------
COMMENT:
----------------------------------------------------------------------

(0) I support Ben's DISCUSS.

(1) This document (currently) specifies a new BGP attribute, but I couldn't
find any discussion about it in the idr mailing list.  Has idr been given the
opportunity to review?

(2) s/is an OPTIONAL procedure/is an optional procedure
This is not a normative statement to require capitalization.

(3) "VRF Route Import BGP attribute" is mentioned twice (§3 and §5), but it is
not an attribute; it is an extended community (rfc6514).

(4) §3.1.1: "similar to BGP next-hop tracking"  Is this specified somewhere?
I don't remember seeing a specification for next-hop tracking, but do know
that implementations do it -- in an implementation-specific way.  Please add a
little more text about what is meant/expected.

(5) §3.1.1: "If BGP next-hop tracking is done...and the root
address...[is]...the same as the next-hop address...then checking...whether
the tunnel root is reachable, will be unnecessary duplication...."  I guess
this means that the existing next-hop tracking functionality can be used and
that it doesn't have to be function-specific (root vs next-hop).  This
paragraph seems to assume a specific implementation -- but again, given that
next-hop tracking implementations are internal then this paragraph doesn't
make much sense to me.

(6) The "reachability condition" is mentioned in §3.1.1/§3.1.3/§3.1.4.  Does
this mean that that root tracking (§3.1.1) should be used with the other
mechanisms?  The specific text says that "the downstream PE can immediately
update its UMH when the reachability condition changes", giving the impression
that the combination is possible but not required.

Note that §4.3 is titled "Reachability Determination", which I hoped would
shed more light, but all it does is point back to §3.1.

(7) §3.1.2: What is the "last-hop link" of the tunnel?  Is it the physical
link, or the virtual hop from the previous waypoint?

(8) §3.1.2 mentions that "careful consideration and coordination" is needed
when using other mechanisms such as rfc4090 "because uncorrelated timers might
cause unnecessary switchovers and destabilize the network."  What are the
associated timers related to the mechanisms in this section?

(9) §3.1.3:

   When using this method and if the signaling state for a P2MP TE LSP
   is removed (e.g., if the ingress of the P2MP TE LSP sends a PathTear
   message) or the P2MP TE LSP changes state from Up to Down as
   determined by procedures in [RFC4875], the status of the
   corresponding P-tunnel MUST be re-evaluated.  If the P-tunnel
   transitions from Up to Down state, the Upstream PE that is the
   ingress of the P-tunnel MUST NOT be considered a valid UMH.

These two sentences are redundant.  It seems to me that they could be
replaced by:

   When using this method and if the signaling state for a P2MP TE LSP
   is removed (e.g., if the ingress of the P2MP TE LSP sends a PathTear
   message) or the P2MP TE LSP changes state from Up to Down as
   determined by procedures in [RFC4875], the Upstream PE that is the
   ingress of the P-tunnel MUST NOT be considered a valid UMH.

(10) §3.1.4: "An Upstream PE SHOULD be removed from the UMH candidate
list...if...the upstream one-hop branch of the tunnel from P to PE cannot be
built."   When is it ok to not remove the PE?  IOW, why is this action not
required?

(11) [nit] s/mechanism to execute its actions/mechanism execute its actions

(12) §3.1.5 says that "where this mechanism is used in conjunction with the
method described in Section 5...downstream PEs can compare reception on the
two P-tunnels to determine when one of them is down", but §5 says that
"downstream PEs accept traffic from the primary or standby tunnel, based on
the status of the tunnel (based on Section 3)".  IOW, §3.1.5 points at §5 as
providing a way to determine if a tunnel is down, while §5 points back at §3
as the way to determine which tunnel to receive from.  This pointing back and
forth is not a total contradiction, but it needs to be clarified.

(13) §3.1.6: "An implementation that does not recognize or is configured not
to support this attribute MUST follow procedures defined for optional
transitive path attributes in Section 5 of [RFC4271]."

There cannot be a Normative action specified for a node that "does not
recognize...this attribute" because, by definition, it can't be assumed that
it is aware of this specification.  In this case, it is not necessary to say
anything about unrecognized attributes because that is already specified in
rfc4271.

For the "configured not to support this attribute" case, it should be pointed
out that the node should operate as if the attribute was unrecognized.

Suggestion>
   An implementation that is configured not to support this attribute MUST
   follow the procedures defined in Section 5 of [RFC4271] as if the attribute
   was unrecognized.

(14) [nit] s/BFD Mode field is the one octet long./The BFD Mode field is one
octet long.

(15) §3.1.6: "The BFD Discriminator attribute MUST be considered malformed if
its length is not a non-zero multiple of four."  Ok, except that the
specification of the attribute doesn't mention the length (only the length of
the TLVs).  Please specify the length and any considerations related to the
Extended Length bit.  Also, given that this is a new attribute, with an
unspecified potential number of TLVs, and that the length is apparently
unbounded, all leading to the potential need for extended messages, please
specify how to handle peers that cannot accommodate more than 4k octet
messages (rfc8654).

(16) §3.1.6.1: "MUST set the IP destination address of the inner IP header to
one of the internal loopback addresses..."   Where do these addresses come
from?  How does the Upstream PE figure out which one to use?  At least please
include a reference to where that is explained.

(17) §3.1.6.1: "MUST use its IP address as the source IP address"  Which
address?  Please be specific.

(18) §3.1.6.2: If the IP address doesn't map correctly at the downstream PE
(for example, a different local address is used that doesn't correspond to the
information in the PMSI attribute), what action should it take?  Can the
tunnel still be monitored?

(19) §3.1.6.2: "SHOULD NOT switch the traffic to the Standby Upstream PE"
When is it ok to do it?  IOW, why is this action recommended, and not
required.

(20) §3.1.7: "set the bfd.LocalDiag of the P2MP BFD session to Concatenated
Path Down and/or Reverse Concatenated Path Down"    Which one?  bfd.LocalDiag
carries a single value.

(21) §3.1.7: "...it is desired for the downstream PEs to switch to a backup
Upstream PE.  To achieve that...it SHOULD set the bfd.LocalDiag..."  If not
set, then the objective won't be achieved.  When is it ok to not set the
bfd.LocalDiag to indicate the concatenated failure?

(22) §4: "Such behavior is referred to as "revertive" behavior and MUST be
supported."  The text around this sentence seems to indicate that the
revertive behavior is the default, is that the intent?  Or if the intent for
it just to be supported (as written)?  Please be clear.

(23) §4.1: "...routes that carry the "Standby PE" BGP Community MUST have the
LOCAL_PREF attribute set to zero."  What should a receiver do if the
LOCAL_PREF is not zero?

(24) §4.1: In the last paragraph of this section, if I follow correctly, the
text talks about the case where the standby becomes the primary and the
updated advertisement doesn't have the Standby PE community.  If that is
correct, then s/ presence/absence of the Standby PE BGP Community/ absence of
the Standby PE BGP Community

Also, the last sentence says that the "LOCAL_PREF attribute MUST be set to
zero".  If the community is not present, how can a receiver enforce this?
What action should it take if the LOCAL_PREF has a different value?

(25) §4.3: "other mechanisms MAY be used"   s/MAY/may   This is just a
statement of fact.

(26) §4.4.2: "MUST try to locate"   To try is not an action that can be
normatively enforced.   Also, I don't think that using "MUST" here adds value
since the normative action is in the next sentence ("MUST perform as
follows"). s/MUST/must

(27) s/"hot root standby" mode is used (Section 4)/"hot root standby" mode is
used (Section 5)

(28) §7.3: s/sub-TLV/TLV/g   The attribute includes TLVs, not sub-TLVs.

---

From: Jeffrey Haas <[email-masked]>
Date: Wed, 16 Dec 2020 16:21:42 -0500

Alvaro,

Thanks for drawing our attention to this document.  

> https://datatracker.ietf.org/doc/draft-ietf-bess-mvpn-fast-failover/

I have the following comments about the BGP Path Attribute in question:

- BGP Packet space is sometimes at a premium.  The initial 3 octet reserved
  padding should be removed.
- The TLVs having a required length of multiples of 4 similarly are space
  wasting.
- The attribute does not make adequate provision for more than one type of a
  BFD Mode.  This likely limits the general purpose re-use of this attribute
  for other purposes.
- The length handling text is likely not correct:
  "The BFD Discriminator attribute MUST be considered malformed if its
   length is not a non-zero multiple of four."
  The implication is a length of four, which would have no BFD
  Discriminator, should be considered a valid PDU.  Note that the length
  considerations are further compounded by the padding issue for optional
  sub-tlvs in the prior point.

Since a BFD discriminator is 4-octets, I'm curious as to what discussion the
working group may have had with regard to using a BGP Extended Community
instead?  In particular, the BGP Extended Community's Transitive bit might
be useful in many multicast VPN scenarios operating over a single AS to
limit the distribution scope of this Discriminator.

The IANA allocation strategies for the Path Attribute and the Optional
Sub-TLVs seems reasonable.  Note that Experimental code points have proven
to be poorly used in BGP and that their use in the public Internet may
result in unexpected discard of the attribute.

> [2] https://tools.ietf.org/wg/bess/minutes?item=minutes-96-bess.html

I will not speak for Reshad, but I don't recall this issue.  I may simply be
forgetting the e-mail brought to the list, if so.

The meta desire here is that communication of the BFD Discriminator for p2mp
sessions requires protocol help - in this case BGP.  While this could also
be discovered via provisioning, that would limit the flexibility of the
deployment of this feature.

For this specific internet-draft's purpose, dissemination of the
Discriminator is tightly scoped.  The fact that this happens under an
AFI/SAFI that is not expected to hit general purpose Internet routes limits
the blast radius of its use.  However, as you note, Alvaro, there may be
more general purpose desire to use this attribute for.

> specify how to handle peers that cannot accommodate more than 4k octet

Note: I wouldn't put special requirements here on the Extended Length bit.
That's core RFC 4271 Path Attribute behavior.

Similarly, overflow of Path Attributes is a general BGP consideration and it
is inapropriate to require this document to solve that.

-- Jeff

---

From: Greg Mirsky <[email-masked]>
Date: Sun, 20 Dec 2020 16:25:21 -0800

Hi Jeff,
your comments and suggestions are much appreciated. Please find my answers
and notes in-lined below and tagger by GIM>>.
Attached, please find the diff and the new working version of the draft.

Regards,
Greg

On Wed, Dec 16, 2020 at 1:04 PM Jeffrey Haas <[email-masked]> wrote:

> > Please refer to
> https://www.ietf.org/iesg/statement/discuss-criteria.html
GIM>> Agree. Removed

> - The TLVs having a required length of multiples of 4 similarly are space
GIM>> Agree. Updated the check on the receipt accordingly.

>   BFD Mode.  This likely limits the general purpose re-use of this
GIM>> In Section 7.2 we're requesting IANA to create a new sub-registry.
New types of a BFD session can be bootstrapped using the BGP BFD
Discriminator attribute. For example, draft-ietf-bess-evpn-bfd
<https://datatracker.ietf.org/doc/draft-ietf-bess-evpn-bfd/?include_text=1> may
introduce a new BFD Mode value. Would you suggest larger field for BFD Mode
or an alternative method to signal the type of the BFD session to which the
value in the BFD Discriminator field is applicable? I thought that, in some
cases, a new value for p2mp with Active Tail Unsolicited Notifications or
p2mp Active Tail with Polling would be added.

>   sub-tlvs in the prior point.
GIM>> Thank you for pointing this case out. I agree, that is not what we
wanted. Following updates noted above the new text is as follows:
NEW TEXT:
   The BFD Discriminator attribute MUST be considered malformed if its
   length is smaller than five octets or the value of the BFD Mode field
   is unrecognized.  If the attribute is deemed to be malformed, the
   UPDATE message SHALL be handled using the approach of Attribute
   Discard per [RFC7606].

> limit the distribution scope of this Discriminator.
GIM>> I agree that the BGP Extended Community attribute can address the
immediate problem and be used to bootstrap a p2mp BFD session. It would
even have space for the BFD Mode field. But because the length of the BFD
Extended Communities attribute is fixed, it seems that no further
extensions (e.g., providing the MultipointHead's source IP address) be
possible.

> > [1]
> https://mailarchive.ietf.org/arch/msg/rtg-bfd/T1jVpgyXuPatTpuD_wA0JC3CT1c/

---

From: Jeffrey Haas <[email-masked]>
Date: Mon, 21 Dec 2020 09:00:08 -0500

Greg,

> p2mp Active Tail with Polling would be added.

The property I'm talking about here is that the BFD Discriminator attribute
is not repeating.  You can have exactly one BFD mode in the Path Attribute.

If the intent was to permit more than one type, the BFD Mode would be
followed by a length field - likely 2 octets long.

>    UPDATE message SHALL be handled using the approach of Attribute

The 5 octet minimum length is probably worth having as a condition.

The unrecognized doesn't follow BGP philosophy very well.  Incremental
deployment of BGP features often requires some devices that are simply
transporting the routes, but not utilizing them, to pass things they don't
fully understand.

I suggest dropping the "or the value of the BFD Mode field is unrecognized".

Having text that says "if Optional TLVs are present, but not well formed,
the entire Path Attribute is malformed and should be handled using Attribute
Discard".

The implication is that if there's more than one BFD Discriminator that any
BFD Mode TLV (if the change is made above for multiple) that is malformed is
reason to discard the entire feature.

> extensions (e.g., providing the MultipointHead's source IP address) be

There we go - a motivation. :-)

-- Jeff

---

From: Greg Mirsky <[email-masked]>
Date: Mon, 21 Dec 2020 13:08:26 -0800

Hi Jeff,
thank you for your quick detailed response. Please find my follow-up notes
in-line tagged by GIM2>>.

Regards,
Greg

On Mon, Dec 21, 2020 at 5:42 AM Jeffrey Haas <[email-masked]> wrote:

> > <
> https://datatracker.ietf.org/doc/draft-ietf-bess-evpn-bfd/?include_text=1>
> may
GIM2>> At this time I don't see a use case for the same BGP speaker
advertising more than one type of BFD session.

> I suggest dropping the "or the value of the BFD Mode field is
GIM2>> Agree.

> the entire Path Attribute is malformed and should be handled using
GIM2>>  Could it be used to replace the "unrecognized value" clause to make
the updated text as follows:
NEW TEXT:
   The BFD Discriminator attribute MUST be considered malformed if its
   length is smaller than five octets or if Optional TLVs are present,
   but not well-formed.  If the attribute is deemed to be malformed, the
   UPDATE message SHALL be handled using the approach of Attribute
   Discard per [RFC7606].

> reason to discard the entire feature.
GIM2>> Do you see as useful a reference to clause g in Section 3 of the RFC
7606 that states:
       If any other attribute (whether
       recognized or unrecognized) appears more than once in an UPDATE
       message, then all the occurrences of the attribute other than the
       first one SHALL be discarded and the UPDATE message will continue
       to be processed.

> > > Since a BFD discriminator is 4-octets, I'm curious as to what
> discussion
> > > the

---

From: Jeffrey Haas <[email-masked]>
Date: Mon, 21 Dec 2020 18:46:22 -0500

Greg,

> advertising more than one type of BFD session.

Consider the example where BFD or S-BFD were options.  Consider that S-BFD
might be preferred, but regular BFD was being offered as an option.  How do
you encode that?

>    UPDATE message SHALL be handled using the approach of Attribute

That text is better. Thanks.

>        first one SHALL be discarded and the UPDATE message will continue

I wouldn't cite that one.  Its intent is that if you had more than one BFD
Path Attribute present in the Update, not that there may be more than one
BFD Mode in the BFD Path Attribute.

-- Jeff

---

From: Greg Mirsky <[email-masked]>
Date: Mon, 21 Dec 2020 17:10:14 -0800

Hi Jeff,
RE: the use case for multiple BFD discriminators listed in the BGP BFD
Discriminator attribute.
As I understand BFD and S-BFD, a discriminator value a node would advertise
has different contexts in BFD and S-BFD. In the former case - that is the
value node would use as My Discriminator, while in the latter- a remote
S-BFD system must use as Your Discriminator. Additionally, I don't think
that S-BFD has been defined for p2mp networks (I'm not sure it can work in
p2mp). And there's draft-ietf-idr-bgp-ls-sbfd-extensions
<https://tools.ietf.org/html/draft-ietf-idr-bgp-ls-sbfd-extensions-04> that
   defines extensions to the BGP Link-state address-family to
   carry the S-BFD Discriminators information via BGP.
As I understand it, to advertise a Discriminator for S-BFD, a node will
follow the mechanism defined in draft-ietf-idr-bgp-ls-sbfd-extensions.
What do you think?

Regards,
Greg

On Mon, Dec 21, 2020 at 3:28 PM Jeffrey Haas <[email-masked]> wrote:

> > > BFD Mode TLV (if the change is made above for multiple) that is
> malformed
> > > is