Subject: [bess] Rtgdir last call review of draft-ietf-bess-mvpn-fast-failover-11

From: Adrian Farrel via Datatracker <[email-masked]>
Date: Mon, 19 Oct 2020 13:09:13 -0700

Reviewer: Adrian Farrel
Review result: Has Issues

Hello,

I have been selected as the Routing Directorate reviewer for this draft. The
Routing Directorate seeks to review all routing or routing-related drafts as
they pass through IETF last call and IESG review, and sometimes on special
request. The purpose of the review is to provide assistance to the Routing ADs.
For more information about the Routing Directorate, please see
http://trac.tools.ietf.org/area/rtg/trac/wiki/RtgDir

Although these comments are primarily for the use of the Routing ADs, it would
be helpful if you could consider them along with any other IETF Last Call
comments that you receive, and strive to resolve them through discussion or by
updating the draft.

Document: draft-ietf-bess-mvpn-fast-failover-11.txt
Reviewer: Adrian Farrel
Review Date: 2020-10-18
IETF LC End Date: 2020-10-19
Intended Status: Proposed Standard

==Summary:==

I have some minor concerns about this document that I think should be resolved
before publication.

==Comments:==

This document is fairly easy to read, but demands a thorough understanding of
RFCs 6513 and 6514. That is not unreasonable.

I also hope that the IDR working group has had a good opportunity to review
this work.

==Major Issues:==

None

==Minor Issues:==

Abstract

I think the Abstract should mention explicitly that this document
extends BGP (and how).

---

Section 3 notes that the procedure (presumably the procedure defined
in this section) is OPTIONAL. I didn't see anything similar in sections
4 and 5 stating that those procedures are optional. Presumably, since
this document is not updating any other RFCs, all of these procedures
are optional.

Actually it would be good to clarify how all these procedures fit in
with "legacy" deployments, and how they are all optional procedures. I
think that needs a short statement in the Introduction and a small
section of its own (maybe between 6 and 7).

---

It is curious (to me) that 3.1.1 describes a way to know that a P-tunnel
is up.  You don't say, however, if being unable to determine that the
P-tunnel is up using this method is equivalent to determining that the
P-tunnel is down. (Previously in 3.1 you have talked about the "tunnel's
state is not known to be down".)

By the way, do you ever say that a P-tunnel has just these two statuses
(up and down) because that could make a big difference?

Note that 3.1.2 etc also establish ways to know that the tunnel is up,
but not ways to determine whether the tunnel is down.

To reiterate, "I don't know if it is up" is not the same as "I know it
is down."

---

3.1.2

   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 protection
   methods at the same time.

OK, I considered them carefully. Now what? :-)

I think you have to give implementation guidance.

---

All of 3.1.x are timid about the use of the mechanisms they describe.

I think that the end of 3.1 should say that an implementation may choose
to use any of these mechanisms to determine the status of the P-tunnel.

This is quite stark, however, in 3.1.3 where you have...

   When 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 SHOULD be re-
   evaluated.  If the P-tunnel transitions from Up to Down state, the
   Upstream PE that is the ingress of the P-tunnel SHOULD NOT be
   considered a valid UMH.

The use of SHOULD and SHOULD NOT is puzzling. Is this "if this mechanism
is being used, the status SHOULD..." or is it "if a P2MP MPLS-TE tunnel
is being used, this mechanism SHOULD be used"? In the former case, the
SHOULD is presumably a MUST. In the latter case, why is this worthy of
BCP 14 language when:
- this whole document is optional
- the mechanisms in 3.1.x are all optional

But 3.1.4, 3.1.5, 3.1.6, 3.1.7 also use BCP 14 language. I'm pretty sure
you mean "if this mechanism is being used..."

In case you determine to keep any use of "SHOULD" you need to describe
under what circumstances an implementation might diverge from this
strong advice.

---

3.1.6

What should I do if I don't recognise or support the setting of the BFD
Mode field?

---

4.1

   The normal and the standby C-multicast routes must have their Local
   Preference attribute adjusted

Should this be "MUST"?

---

7.1

   IANA is requested to allocate the BGP "Standby PE" community value
   (TBA1) from the Border Gateway Protocol (BGP) Well-known Communities
   registry.

There are three ranges. You need to tell IANA which range to use.
Presumably not Private Use (because they are not assigned). But do you
want an assignment from the FCFS range or the Standards Action range?

==Nits:==

Abstract

Notwithstanding the terminology difference between "upstream" and
"Upstream" defined in Section 2, the distinction made in the text
here is unclear. I think that lowercase "upstream" would not be
confusing in this text.

---

Requirements Language

Please move this to a new section 2.1 to be consistent with the RFC
Editor style guide.

---

Section 1

   In the context of multicast in BGP/MPLS VPNs

That could use a reference.

---

Section 1

I don't think the description of what is in which section of the
document is quite accurate. Maybe the document has moved on? In any
case, a more specific mention of which protocols are extended/modified
would be good.

I am pretty sure that the reader has no hope of understanding this work
without having first read and absorbed RFC 6513 and RFC 6514. It would
be worth adding a short statement like "It is assumed that the reader is
familiar with the workings of multicast MPLS/BGP IP VPNs as described in
[RFC6513] and [RFC6514]."

---

Section 2

   x-PMSI: I-PMSI or S-PMSI

This is too brief!  I think you need.

   PMSI: P-Multicast Service Interface
   I-PMSI: Inclusive PMSI
   S-PMSI: Selective PMSI
   x-PMSI: Either an I-PMSI or an S-PMSI

It would be also good to list the other imported terms:

   P-tunnel: Provider-Tunnels
   UMH: Upstream Multicast Hop

I think you might collect some of the abreviations into a table in this
section. MVPN, RD, RP, NLRI, VRF, EC, AC, MED, ...

---

Section 3

s/Section 5.1 [RFC6513]/Section 5.1 of [RFC6513]/

---

Section 3 has

   selection, which will result in the downstream PE to failover to the
   Upstream PE, which is next in the list of candidates.

The language is a little unclear. Maybe...

   selection.  This will result in the downstream PE failing over to
   use the next Upstream PE in the list of candidates.

---

Section 3 has

   Because of that, procedures described in Section 9.1.1 of [RFC6513]
   MUST be used when using I-PMSI P-tunnels.

Aren't those procedures already mandatory? That section of 6513 already
uses "MUST" (although it oes go on to say that it might not be possible
to apply the procedure and delegates processing to 9.1.2 and 9.1.3 -
peculiarly using lowercase must for that delegation). I wonder whether
you are saying "this case is covered by the procedures of Section 9.1.1
of [RFC6513]" or are you actually defining new normative behaviour?

---

Section 3

s/tunnel' state/tunnel's state/

---

Section 3.1 has

   The
   optional procedures proposed in this section also allow that all
   downstream PEs don't apply the same rules to define what the status
   of a P-tunnel is (please see Section 6)

A little confusing. Maybe...

   The
   optional procedures described in this section also handle the case
   the downstream PEs do not all apply the same rules to define what the
   status of a P-tunnel is (please see Section 6)

---

3.1.2

   A condition to consider a tunnel status as Up can be that the last-
   hop link of the P-tunnel is Up.

I like that you are using "Up" rather than "up". Maybe change throughout
the document to use "Up" and "Down"?

---

3.1.6

s/TLV 's Type/TLV's Type/

---

3.1.6.1

You use "p2mp BFD Session" rather than using "P2MP". This looks
intentional but also looks really odd. Section 7.2 uses "P2MP
BFD Session".

---

3.1.7

s/section 6.8.17 [RFC5880]/Section 6.8.17 of [RFC5880]/

---

4.

s/section 5.1.3 [RFC6513]/Section 5.1.3 of [RFC6513]/

OLD
 VPN routes (VPN-IPv4 or VPN-IPv6) routes
NEW
 VPN routes (VPN-IPv4 or VPN-IPv6)
END

---

4.

s/would refer to/refers to/

---

4.1

   As long as C-S is reachable via the Primary
   Upstream PE and the Upstream PE is the Primary Upstream PE.

This sentence doesn't seem to be complete. What is the consequence of
this condition?

---

4.1

   o  SHOULD carry the "Standby PE" BGP Community (this is a new BGP
      Community.

I think this needs guidance on when to not include the Community

---

4.1

   Also, a LOCAL_PREF attribute MUST be set to zero.

Maybe...

   The LOCAL_PREF attribute MUST also be set to zero.

---

4.2

You might want to tidy up whether you use "a)" and "b)" or "(a)" and
"(b)"

---

4.4.1

s/Additionally, to?Additional to/

---

4.4.2

   When an Upstream ASBR receives a C-multicast route, and at least one
   of the RTs of the route matches one of the ASBR Import RT, the ASBR,
   that supports this specification, MUST locate an Inter-AS I-PMSI A-D
   Route whose RD and Source AS respectively match the RD and Source AS
   carried in the C-multicast route.  If the match is found, and the
   C-multicast route carries the Standby PE BGP Community, then the ASBR
   MUST perform as follows:

Is that "MUST try to locate"? Because it seems to be countenanced that
the attempt could fail.

---

4.4.2

s/MED attribute set of/MED attribute set to/

---

5.

   The mechanisms defined in sections Section 4 and Section 3 can be
   used together as follows.

That's an XML feature. If you do
"...defined in <xref target="section4"/><xref target="section3"/>..."
then XML2RFC will sort things out for you.  Seems to be OK a couple of
paragraphs later.

---

5.

s/semantic for is that/semantic is that/

---

6.

   Multicast VPN specifications [RFC6513] impose that a PE only forwards
   to CEs the packets coming from the expected Upstream PE
   (Section 9.1).

There being no section 9.1 in this document, I think you mean...
   "(see Section 9.1 of [RFC6513])."

Please also be clear in the next paragraph whether the references are to
sections of this document (no need to qualify) or sections of RFC 6513
(important to qualify).

---

6.

OLD
   We highlight the reader's attention to the fact that the respect of
NEW
   We draw the reader's attention to the fact that the respect of
END

---

From: Greg Mirsky <[email-masked]>
Date: Tue, 20 Oct 2020 12:02:09 -0700

Hi Adrian,
thank you for the review, detailed questions, and helpful suggestions. I'll
work through and respond within several days.

Regards,
Greg

---

From: Greg Mirsky <[email-masked]>
Date: Tue, 27 Oct 2020 19:52:26 -0700

Hi Adrian,
many thanks for the review and comments. Please find my notes in-line below
tagged GIM>>.
Attached, please find the diff highlighting the updates and new working
version of the draft.

Regards,
Greg

> I think the Abstract should mention explicitly that this document
GIM>> Update the Abstract to emphasize introduced extensions to BGP:
OLD TEXT:
   This document defines multicast VPN extensions and procedures that
   allow fast failover for upstream failures, by allowing downstream PEs
   to take into account the status of Provider-Tunnels (P-tunnels) when
   selecting the Upstream PE for a VPN multicast flow, and extending BGP
   MVPN routing so that a C-multicast route can be advertised toward a
   Standby Upstream PE.
NEW TEXT:
   This document defines multicast VPN extensions and procedures that
   allow fast failover for upstream failures by allowing downstream PEs
   to consider the status of Provider-Tunnels (P-tunnels) when selecting
   the Upstream PE for a VPN multicast flow.  The fast failover is
   enabled by using RFC 8562 BFD for Multipoint Networks and the new BGP
   Attribute - BFD Discriminator.  Also, the document introduces a new
   BGP Community, Standby PE, extending BGP MVPN routing so that a
   C-multicast route can be advertised toward a Standby Upstream PE.
> section of its own (maybe between 6 and 7).
GIM>> Thank you for the suggestion. I've updated the Introduction in this
way:
OLD TEXT:
   Section 4 describes protocol extensions that can speed up failover by
   not requiring any multicast VPN routing message exchange at recovery
   time.

   Moreover, section 5 describes a "hot leaf standby" mechanism, that
   uses a combination of these two mechanisms.  This approach has
   similarities with the solution described in [RFC7431] to improve
   failover times when PIM routing is used in a network given some
   topology and metric constraints.
NEW TEXT:
   Section 4 describes optional protocol extensions that can speed up
   failover by not requiring any multicast VPN routing message exchange
   at recovery time.

   Moreover, Section 5 describes a "hot leaf standby" mechanism that can
   be used to improve failover time in MVPN.  The approach combines
   mechanisms defined in Section 3 and Section 4 has similarities with
   the solution described in [RFC7431] to improve failover times when
   PIM routing is used in a network given some topology and metric
   constraints.

I think that Section 5 is intended to explain how introduced BGP extensions
and their use described in Section 3 and Section 4 enable operators to
provide protection for multicast services. Would you suggest adding a new
text to the section to highlight particular aspects of introducing
protection in MVPN?

> state is not known to be down".)
GIM>> This method, as noted in the document, is similar to BGP next-hop
tracking, may be computationally intensive, and cannot be run frequently.
So, in periods between checking whether the root address in the x-PMSI
Tunnel attribute is reachable the state is "not known to be down".

> (up and down) because that could make a big difference?
GIM>> I think that the document then needs to discuss what impact detection
time has on MVPN. For example, if the detection time is in single-digit
seconds, a two-state model can be used. But would it be a useful model if
the detection time is in tens of seconds? Should a "not known to be down"
state be introduced?

> but not ways to determine whether the tunnel is down.
GIM>> In this section the state of a P-tunnel is equated with the state of
the last link of that tunnel. The document notes that if the link is Up,
then the P-tunnel is considered down. It is implied, that if it is
determined that the link is Down, then the state of the P-tunnel is
considered Down. Would you recommend adding an explanation to the document?

> To reiterate, "I don't know if it is up" is not the same as "I know it
GIM>> Indeed. It is analogous to "it was Up the last time I've checked on
it". It meant to be used when the interval between checking is significant.

> I think you have to give implementation guidance.
GIM>> I agree, an operational recommendation could be helpful. Usually, in
case of multi-layered protection, detection intervals on the higher layer
are 10 times of guaranteed restoration time of the lower layer. Would you
recommend adding this to the text as an example of a deployment?

> to use any of these mechanisms to determine the status of the P-tunnel.
GIM>> Will the following text reflect that:
NEW TEXT:
   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.

> under what circumstances an implementation might diverge from this
GIM>> Thank you for pointing this ambiguity that, upon careful reading,
could be confusing. You're right, it is meant to follow the "if this
mechanism is used" clause. But once we define the mechanism itself, the
normative language to use here is MUST/MUST NOT. Below is the updated text
for your consideration:
NEW TEXT:
   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.

> What should I do if I don't recognise or support the setting of the BFD
GIM>> I think that the same handling applies as for the malformed attribute:
   If malformed, the UPDATE
   message SHALL be handled using the approach of Attribute Discard per
   [RFC7606].
I propose to extend the applicability of the rule with the following update
to the sentence:
NEW TEXT:
   The BFD Discriminator attribute MUST be considered malformed if its
   length is not a non-zero multiple of four.  If the setting of the BFD
   Mode field is not recognized or not supported, or the attribute
   considered malformed, the UPDATE message SHALL be handled using the
   approach of Attribute Discard per [RFC7606].

>    Preference attribute adjusted
GIM>> I think that is not an actionable 'must'. It could be expressed as

The Local Preference attribute of the normal and the standby C-multicast
route needs to be adjusted.

Would you recommend using the re-worded passage?

> want an assignment from the FCFS range or the Standards Action range?
GIM>> Thank you for pointing to the missing detail. FCFS range is fine. The
updated text:
NEW TEXT:
   IANA is requested to allocate the BGP "Standby PE" community value
   (TBA1) from the Border Gateway Protocol (BGP) Well-known Communities
   registry using the First Come First Served registration policy.

> here is unclear. I think that lowercase "upstream" would not be
GIM>> Agree. s/Upstream/upstream/

> Please move this to a new section 2.1 to be consistent with the RFC
GIM>> Done.

> That could use a reference.
GIM>> Added reference to RFC 6513.

> case, a more specific mention of which protocols are extended/modified
GIM>> I've updated the section to be more specific:
NEW TEXT:
   Section 3 describes local procedures allowing an egress PE (a PE
   connected to a receiver site) to take into account the status of
   P-tunnels to determine the Upstream Multicast Hop (UMH) for a given
   (C-S, C-G).  One of the optional methods uses [RFC8562] and the new
   BGP Attribute - BFD Discriminator.  None of these methods provide a
   "fast failover" solution when used alone, but can be used together
   with the mechanism described in Section 4 for a "fast failover"
   solution.

   Section 4 describes an optional BGP extension, a new Standby PE
   Community. that can speed up failover by not requiring any multicast
   VPN routing message exchange at recovery time.

> [RFC6513] and [RFC6514]."
GIM>> Thank you for the suggestion. Indeed, I kept both RFC open while
editing the draft.

> section. MVPN, RD, RP, NLRI, VRF, EC, AC, MED, ...
GIM>> Thank you for the suggested expansions. I've moved them all to a new
Acronyms section (except for EC and AC) (not in the alphabetical order):
2.3.  Acronyms

   I-PMSI: Inclusive PMSI

   S-PMSI: Selective PMSI

   VPN: Virtual Private Network

   MVPN: Multicast VPN

   RD: Route Distinguisher

   RP: Rendezvous Point

   NLRI: Network Layer Reachability Information

   VRF: VPN Routing and Forwarding Table

   MED: Multi-Exit Discriminator

> s/Section 5.1 [RFC6513]/Section 5.1 of [RFC6513]/
GIM>> Done.

>    use the next Upstream PE in the list of candidates.
GIM>> Thank you for the suggested text it is much better than what in the
document. Done.

> of [RFC6513]" or are you actually defining new normative behaviour?
GIM>> I think that the use of lower case 'must' is ambiguous and somewhat
confusing. You are right, the intention is to refer to Section 9.1.1 as the
mandatory behavior. But neither 9.1.2, nor 9.1.3 use the normative
language. What would you recommend?

> s/tunnel' state/tunnel's state/
GIM>> Fixed

>    status of a P-tunnel is (please see Section 6)
GIM>> Great suggestion and gladly applied to the text.

> the document to use "Up" and "Down"?
GIM>> I went through the document and uppercased when referring to the
status or state. I hope I've got them all.

> s/TLV 's Type/TLV's Type/
GIM>> Got it!

> intentional but also looks really odd. Section 7.2 uses "P2MP
GIM>> Changed to P2MP through the document (also added to the Acronyms)

> s/section 5.1.3 [RFC6513]/Section 5.1.3 of [RFC6513]/
GIM>> Done.

>  VPN routes (VPN-IPv4 or VPN-IPv6)
GIM>> Thank you.

> s/would refer to/refers to/
GIM>> Thanks!

> This sentence doesn't seem to be complete. What is the consequence of
GIM>> It suppose to be
   As long as
   C-S is reachable via the Primary Upstream PE, the Upstream PE is the
   Primary Upstream PE.
Is it better?

> I think this needs guidance on when to not include the Community
GIM>> The question made me think that for the implementation that supports
this specification there's no reason not to include the Standby PE
Community. s/SHOULD/MUST/

>    The LOCAL_PREF attribute MUST also be set to zero.
GIM>> Agree.

> You might want to tidy up whether you use "a)" and "b)" or "(a)" and
GIM>> I opted for 'a)' and 'b)'

> s/Additionally, to?Additional to/
GIM>> Got it

> Is that "MUST try to locate"? Because it seems to be countenanced that
GIM>> Thank you for pointing this out. Yes, the lookup might fail.
s/MUST/MUST try to/

> s/MED attribute set of/MED attribute set to/
GIM>> Thank you!

> then XML2RFC will sort things out for you.  Seems to be OK a couple of
GIM>> Cleaned up

> s/semantic for is that/semantic is that/
GIM>> Done

>    "(see Section 9.1 of [RFC6513])."
GIM>> Yes, added the reference to RFC 6513

> sections of this document (no need to qualify) or sections of RFC 6513
GIM>> Thank you for the question. Both references are to sections in this
document.

>    We draw the reader's attention to the fact that the respect of
GIM>> Thank you. "Highlighting" attention - awkward.

---

From: "Adrian Farrel" <[email-masked]>
Date: Wed, 28 Oct 2020 13:51:28 -0000

Hello Greg,

Thanks for this. I’m cutting down to places where we still need to interact. Look for [af] and blue text.

Nothing alarming.

Best,

Adrian

GIM>> Thank you for the suggestion. I've updated the Introduction in this way:

OLD TEXT:

NEW TEXT:

I think that Section 5 is intended to explain how introduced BGP extensions and their use described in Section 3 and Section 4 enable operators to provide protection for multicast services. Would you suggest adding a new text to the section to highlight particular aspects of introducing protection in MVPN?

[af] OK I obviously wasn’t clear. What I’m looking for is something like…

The procedures described in this document are optional to enable an operator to provide protection for multicast services. An operator would enable these mechanisms using <foo> and it is assumed that these mechanisms would be supported by all <what?> in the network for the procedures to work. In the case that a BGP implementation does not recognise or is configured to not support the extensions defined in this document, it will respond <somehow> as described in <rfc????>. This would result in <something>.

GIM>> This method, as noted in the document, is similar to BGP next-hop tracking, may be computationally intensive, and cannot be run frequently. So, in periods between checking whether the root address in the x-PMSI Tunnel attribute is reachable the state is "not known to be down".

[af] Well, OK. Can you add to say that, “If it is not possible to determine whether the state of a tunnel is ‘up’, the state shall be considered as ‘not known to be down’, and it may be treated as if it is ‘up’ so that attempts to use the tunnel are acceptable.” This is probably “obvious to one skilled in the art,” but would help this reader.

GIM>> I think that the document then needs to discuss what impact detection time has on MVPN. For example, if the detection time is in single-digit seconds, a two-state model can be used. But would it be a useful model if the detection time is in tens of seconds? Should a "not known to be down" state be introduced?

[af] Yes, that *seems* to be the implication. But is there any different action between “up” and “not known to be down”? If you have three states then there is (possibly) an implication that tunnels are prioritised by state. I think, however, that it is OK to use “not known to be down” as if it was “up”.

GIM>> In this section the state of a P-tunnel is equated with the state of the last link of that tunnel. The document notes that if the link is Up, then the P-tunnel is considered down. It is implied, that if it is determined that the link is Down, then the state of the P-tunnel is considered Down. Would you recommend adding an explanation to the document? 

GIM>> Indeed. It is analogous to "it was Up the last time I've checked on it". It meant to be used when the interval between checking is significant.

[af] Assuming there is typo in what you just wrote – link not up ==> tunnel down.

Also assuming that we don’t go down the “three state” path, then “not checked for a while” is still “Up”.

I think it was the phrasing in 3.1.2. It sounded like “Here is a way to know that the tunnel is up” which is a good thing, but does not say that it is exclusive. So, avoiding the “implication” would be a good thing. Something like 

OLD

NEW

   hop link of the P-tunnel is Up.  Conversely, if the last-hop link of

   the P-tunnel is Down then this can be taken as an indication that

  the P-tunnel is Down.

END

3.1.2

GIM>> I agree, an operational recommendation could be helpful. Usually, in case of multi-layered protection, detection intervals on the higher layer are 10 times of guaranteed restoration time of the lower layer. Would you recommend adding this to the text as an example of a deployment? 

[af] An example would be fine (and a forward reference from here). But it would be fine, maybe better, to offer half a sentence of guidance. So…”not practical to use both protection methods at the same time because <adverse interactions?>….”

NEW TEXT:

[af] Good.

3.1.6

   If malformed, the UPDATE

   [RFC7606]. 

I propose to extend the applicability of the rule with the following update to the sentence:

NEW TEXT:

[af] This is a bit subtle and refers also to my first point in this email. If the setting of the BFD Mode is not recognised or not supported, then it is likely because this specification is not supported. Therefore, this specification cannot mandate how the implementation will behave. I think you have to separate:

*	The malformed SHALL be handled using Attribute Discard according to [RFC7606]
*	An unknown or unsupported attribute will be handled by implementations according to the procedures for unknown attributes described in <foo>

4.1

Should this be "MUST"?

The Local Preference attribute of the normal and the standby C-multicast route needs to be adjusted.

[af] The alternative text is good.

==Nits:==

Section 3 has

GIM>> I think that the use of lower case 'must' is ambiguous and somewhat confusing. You are right, the intention is to refer to Section 9.1.1 as the mandatory behavior. But neither 9.1.2, nor 9.1.3 use the normative language. What would you recommend?

[af] Maybe…

“Because of that, the procedures of Section 9.1.1 of [RFC6513] are applicable. That document is a foundation for this document and its processes all apply here. Section 9.1.1 mandates the use of specific procedures for sending intra-AS I-PMSI A-D Routes.”

4.1

GIM>> It suppose to be

   As long as

Is it better?

[af] That makes sense

---

From: Greg Mirsky <[email-masked]>
Date: Wed, 28 Oct 2020 14:34:12 -0700

Hi Adrian,
many thanks for your detailed clarification and helpful suggestions. Please
find my follow-up notes under GIM2>> tag in green. Also, attached are the
new diff and working version of the draft.

Regards,
Greg

On Wed, Oct 28, 2020 at 6:51 AM Adrian Farrel <[email-masked]> wrote:

> I think that Section 5 is intended to explain how introduced BGP
> extensions and their use described in Section 3 and Section 4 enable
> operators to provide protection for multicast services. Would you suggest
> adding a new text to the section to highlight particular aspects of
> introducing protection in MVPN?
>
>
>
> The procedures described in this document are optional to enable an
> operator to provide protection for multicast services. An operator would
> enable these mechanisms using <foo> and it is assumed that these mechanisms
> would be supported by all <what?> in the network for the procedures to
> work. In the case that a BGP implementation does not recognise or is
> configured to not support the extensions defined in this document, it will
> respond <somehow> as described in <rfc????>. This would result in
> <something>.
>
GIM2>> I think I've got the idea now. Would appending the new paragraph to
the Introduction address your comment:
NEW TEXT:
   The procedures described in this document are optional to enable an
   operator to provide protection for multicast services in BGP/MPLS IP
   VPNs.  An operator would enable these mechanisms using a method
   discussed in Section 3 in combination with the redundancy provided by
   a standby PE connected to the source of the multicast flow, and it is
   assumed that all PEs in the network would support these mechanisms
   for the procedures to work.  In the case that a BGP implementation
   does not recognize or is configured to not support the extensions
   defined in this document, it will continue to provide the multicast
   service, as described in [RFC6513].

> [af] Well, OK. Can you add to say that, “If it is not possible to
> determine whether the state of a tunnel is ‘up’, the state shall be
> considered as ‘not known to be down’, and it may be treated as if it is
> ‘up’ so that attempts to use the tunnel are acceptable.” This is probably
> “obvious to one skilled in the art,” but would help this reader.
>
GIM2>> Thank you for the contributed text. I've added in before "not known
to be Down" used in the text (with the yellowish background):
NEW TEXT:
   The procedure described here is an OPTIONAL procedure that is based
   on a downstream PE taking into account the status of P-tunnels rooted
   at each possible Upstream PE, for including or not including each
   given PE in the list of candidate UMHs for a given (C-S, C-G) state.
   If it is not possible to determine whether a P-tunnel's current
   status is Up, the state shall be considered "not known to be Down",
   and it may be treated as if it is Up so that attempts to use the
   tunnel are acceptable.  The result is that, if a P-tunnel is Down
   (see Section 3.1), the PE that is the root of the P-tunnel will not
   be considered for UMH selection.  This will result in the downstream
   PE failing over to use the next Upstream PE in the list of
   candidates.  Some downstream PEs could arrive at a different
   conclusion regarding the tunnel's state because the failure impacts
   only a subset of branches.  Because of that, procedures described in
   Section 9.1.1 of [RFC6513] MUST be used when using I-PMSI P-tunnels.
> GIM>> I think that the document then needs to discuss what impact
> detection time has on MVPN. For example, if the detection time is in
> single-digit seconds, a two-state model can be used. But would it be a
> useful model if the detection time is in tens of seconds? Should a "not
> known to be down" state be introduced?
>
>
>
> [af] Yes, that **seems** to be the implication. But is there any
> different action between “up” and “not known to be down”? If you have three
> states then there is (possibly) an implication that tunnels are prioritised
> by state. I think, however, that it is OK to use “not known to be down” as
> if it was “up”.
>
GIM2>> Thank you.

> [af] Assuming there is typo in what you just wrote – link not up è tunnel
> down.
>
> I think it was the phrasing in 3.1.2. It sounded like “Here is a way to
> know that the tunnel is up” which is a good thing, but does not say that it
> is exclusive. So, avoiding the “implication” would be a good thing.
> Something like
>
> OLD
>
> GIM2>> Many thanks for the new text. Updated accordingly.

> [af] An example would be fine (and a forward reference from here). But it
> would be fine, maybe better, to offer half a sentence of guidance. So…”not
> practical to use both protection methods at the same time because <adverse
> interactions?>….”
>
GIM2>> Thank you for the explanation. Extended the last sentence:
NEW TEXT:
   In many cases, it is not practical to use both protection
   methods at the same time because uncorrelated timers might cause
   unnecessary switchovers and destabilize the network.

> [af] This is a bit subtle and refers also to my first point in this email.
> If the setting of the BFD Mode is not recognised or not supported, then it
> is likely because this specification is not supported. Therefore, this
> specification cannot mandate how the implementation will behave. I think
> you have to separate:
>
>    - The malformed SHALL be handled using Attribute Discard according to
>    [RFC7606]
>    - An unknown or unsupported attribute will be handled by
>    implementations according to the procedures for unknown attributes
>    described in <foo>
>
>
>
GIM2>> I thought of a different "unsupported" scenario. Consider BFD
Discriminator attribute is supported but an implementation does not
recognize the value in the BFD Mode field. In the case the BFD
Discriminator is unknown or unsupported procedures defined for the optional
transitive path attribute in Section 5 of RFC 4271 must be followed. I've
removed the recent updated and added the reference to RFC 4271 (added as
the Normative reference) following the definition of the new attribute:
NEW TEXT:
   This document defines the format and ways of using a new BGP
   attribute called the "BFD Discriminator".  It is an optional
   transitive BGP attribute.  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].

> [af] The alternative text is good.
GIM2>> Done.

> “Because of that, the procedures of Section 9.1.1 of [RFC6513] are
> applicable. That document is a foundation for this document and its
> processes all apply here. Section 9.1.1 mandates the use of specific
> procedures for sending intra-AS I-PMSI A-D Routes.”
>
GIM2>> Thank you for the text. Updated accordingly with noting "when using
I-PMSI P-tunnels"
NEW TEXT:
Because of that, the procedures of
   Section 9.1.1 of [RFC6513] are applicable when using I-PMSI
   P-tunnels.  That document is a foundation for this document, and its
   processes all apply here.  Section 9.1.1 mandates the use of specific
   procedures for sending intra-AS I-PMSI A-D Routes.

---

From: "Adrian Farrel" <[email-masked]>
Date: Wed, 28 Oct 2020 22:34:24 -0000

Hi again,

Thanks for rapid convergence.

All good.

Adrian

OLD TEXT:

NEW TEXT:

GIM2>> I think I've got the idea now. Would appending the new paragraph to the Introduction address your comment:

NEW TEXT:

[af] Perfect

GIM2>> Thank you for the contributed text. I've added in before "not known to be Down" used in the text (with the yellowish background):

NEW TEXT:

[af] Thanks

GIM2>> Thank you.

3.1.2

NEW TEXT:

[af] Great

3.1.6

   If malformed, the UPDATE

   [RFC7606]. 

NEW TEXT:

GIM2>> I thought of a different "unsupported" scenario. Consider BFD Discriminator attribute is supported but an implementation does not recognize the value in the BFD Mode field. In the case the BFD Discriminator is unknown or unsupported procedures defined for the optional transitive path attribute in Section 5 of RFC 4271 must be followed. I've removed the recent updated and added the reference to RFC 4271 (added as the Normative reference) following the definition of the new attribute:

NEW TEXT:

[af] That captures what I wanted to see. Thanks

==Nits:==

Section 3 has

[af] Maybe…

GIM2>> Thank you for the text. Updated accordingly with noting "when using I-PMSI P-tunnels"

NEW TEXT:

[af] Fine