[RTG-DIR] Rtgdir last call review of draft-ietf-bess-mvpn-fast-failover-11

Adrian Farrel via Datatracker <noreply@ietf.org> Mon, 19 October 2020 20:09 UTC

Return-Path: <noreply@ietf.org>
X-Original-To: rtg-dir@ietf.org
Delivered-To: rtg-dir@ietfa.amsl.com
Received: from ietfa.amsl.com (localhost [IPv6:::1]) by ietfa.amsl.com (Postfix) with ESMTP id 78EFC3A09EB; Mon, 19 Oct 2020 13:09:13 -0700 (PDT)
MIME-Version: 1.0
Content-Type: text/plain; charset="utf-8"
Content-Transfer-Encoding: 7bit
From: Adrian Farrel via Datatracker <noreply@ietf.org>
To: rtg-dir@ietf.org
Cc: draft-ietf-bess-mvpn-fast-failover.all@ietf.org, bess@ietf.org, last-call@ietf.org
X-Test-IDTracker: no
X-IETF-IDTracker: 7.20.0
Auto-Submitted: auto-generated
Precedence: bulk
Message-ID: <160313815345.29014.16143591054021036590@ietfa.amsl.com>
Reply-To: Adrian Farrel <adrian@olddog.co.uk>
Date: Mon, 19 Oct 2020 13:09:13 -0700
Archived-At: <https://mailarchive.ietf.org/arch/msg/rtg-dir/EWyrrBzG9o0MH4w3yzr4cPALJ80>
Subject: [RTG-DIR] Rtgdir last call review of draft-ietf-bess-mvpn-fast-failover-11
X-BeenThere: rtg-dir@ietf.org
X-Mailman-Version: 2.1.29
List-Id: Routing Area Directorate <rtg-dir.ietf.org>
List-Unsubscribe: <https://www.ietf.org/mailman/options/rtg-dir>, <mailto:rtg-dir-request@ietf.org?subject=unsubscribe>
List-Archive: <https://mailarchive.ietf.org/arch/browse/rtg-dir/>
List-Post: <mailto:rtg-dir@ietf.org>
List-Help: <mailto:rtg-dir-request@ietf.org?subject=help>
List-Subscribe: <https://www.ietf.org/mailman/listinfo/rtg-dir>, <mailto:rtg-dir-request@ietf.org?subject=subscribe>
X-List-Received-Date: Mon, 19 Oct 2020 20:09:19 -0000

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