[pim] AD Review of draft-ietf-pim-bfd-p2mp-use-case-05

Alvaro Retana <aretana.ietf@gmail.com> Tue, 13 July 2021 20:58 UTC

Return-Path: <aretana.ietf@gmail.com>
X-Original-To: pim@ietfa.amsl.com
Delivered-To: pim@ietfa.amsl.com
Received: from localhost (localhost [127.0.0.1]) by ietfa.amsl.com (Postfix) with ESMTP id 8C4E83A0D27; Tue, 13 Jul 2021 13:58:58 -0700 (PDT)
X-Virus-Scanned: amavisd-new at amsl.com
X-Spam-Flag: NO
X-Spam-Score: -2.096
X-Spam-Level:
X-Spam-Status: No, score=-2.096 tagged_above=-999 required=5 tests=[BAYES_00=-1.9, DKIM_SIGNED=0.1, DKIM_VALID=-0.1, DKIM_VALID_AU=-0.1, DKIM_VALID_EF=-0.1, FREEMAIL_FROM=0.001, RCVD_IN_DNSWL_BLOCKED=0.001, SPF_HELO_NONE=0.001, SPF_PASS=-0.001, UNPARSEABLE_RELAY=0.001, URIBL_BLOCKED=0.001] autolearn=ham autolearn_force=no
Authentication-Results: ietfa.amsl.com (amavisd-new); dkim=pass (2048-bit key) header.d=gmail.com
Received: from mail.ietf.org ([4.31.198.44]) by localhost (ietfa.amsl.com [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id p4ngF36VRHg0; Tue, 13 Jul 2021 13:58:54 -0700 (PDT)
Received: from mail-ej1-x62b.google.com (mail-ej1-x62b.google.com [IPv6:2a00:1450:4864:20::62b]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by ietfa.amsl.com (Postfix) with ESMTPS id 2DEF33A0D24; Tue, 13 Jul 2021 13:58:50 -0700 (PDT)
Received: by mail-ej1-x62b.google.com with SMTP id gn32so43975250ejc.2; Tue, 13 Jul 2021 13:58:49 -0700 (PDT)
DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=from:mime-version:date:message-id:subject:to:cc :content-transfer-encoding; bh=fiyDaM1uAMwC6C1l5wSVIP06dAt7x1ioboTwdsH458s=; b=hruu189x1LPVZ3mDRtajFw8dviS5enEE8SdHLkjGf+ZKkOomD8cYN9/XFNcBEMe8A7 L0FSBkyGiwImuj1CSsyYjOGmwvTZAagSUBgdM3t6zqThaip3s2A8Y+Vbh6XZeqvEwbgi mCK+D97nQJx5hg4QvPtvsOg6Ic1QeQtX9zJKCy2LuYHlfbw/799kUg8Ir0TKnRYMsHf0 Wnv1jzSfHYFDvKO/u5pGbNRGsVA4r5nmJH4MwE8tMlJw/hEnezEdMRj3/xaZoJFxPfcr sJ0M3cvxKK5h7OKnLJBQ9fJIygKmcvMGtviviuCV+RtSsQbBMl0DtIDzOkLWK8pQfwU/ LuIQ==
X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:mime-version:date:message-id:subject:to:cc :content-transfer-encoding; bh=fiyDaM1uAMwC6C1l5wSVIP06dAt7x1ioboTwdsH458s=; b=SeK9b0Y4xan3HFdrNtpVPFTSIH8s+V/Fp6Te6UxgKV8q03B0sZuoS8xQ+1/oZx7Xwv HsZXOUTDK/67U0KD4u3ND0a8Aa/ceBgGS/MPlwvh6sCHPk8KH0JgUE7tFKKgDDmr6t19 bbCju2v6PmNu6G6L3YC3s5symXlLR5RnLEnLNyoP0FuEWufOwZ0W7z3ENU0dk+7f2r1r wbjcFv/aMaU7wG5JAY48L5guj3+JepviItX8wq/+T/w2dSy17RNdqoEQWW3J4reoidTN cQP4vvZVpjeUR9vmfb6mwH3s4TaIIitsWbA9ecqvKpHh3EbIN2NdZQKrLKP0HDuwflfB xaSw==
X-Gm-Message-State: AOAM533XuWr4uWCmyHgacWyA5HLlU7WuRSMPZNC3X1pP6v9ZqK2vhBWz 9DqQldlScfgg3zfK8yZ5TuLxeu/5tBoQSO/j4813cvwwL0U=
X-Google-Smtp-Source: ABdhPJxdacL+8PJHT6Ri42aroJ2g1hK2s8T7AzQpREqdse9hsU68JX4bOzM/KQj4wj+U9rb3ZGAfeL6VTtbZpAO9K3M=
X-Received: by 2002:a17:906:1cd5:: with SMTP id i21mr7794206ejh.478.1626209926261; Tue, 13 Jul 2021 13:58:46 -0700 (PDT)
Received: from 1058052472880 named unknown by gmailapi.google.com with HTTPREST; Tue, 13 Jul 2021 20:58:44 +0000
From: Alvaro Retana <aretana.ietf@gmail.com>
MIME-Version: 1.0
Date: Tue, 13 Jul 2021 20:58:44 +0000
Message-ID: <CAMMESsxoMySh810X98kzHPGupxWFjCiRAaBK4rtt3HUi0VANAA@mail.gmail.com>
To: draft-ietf-pim-bfd-p2mp-use-case@ietf.org
Cc: Mike McBride <mmcbride7@gmail.com>, pim-chairs@ietf.org, pim@ietf.org
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Archived-At: <https://mailarchive.ietf.org/arch/msg/pim/oSOkYL6y9cFihBTSYoF45MR9y40>
Subject: [pim] AD Review of draft-ietf-pim-bfd-p2mp-use-case-05
X-BeenThere: pim@ietf.org
X-Mailman-Version: 2.1.29
Precedence: list
List-Id: Protocol Independent Multicast <pim.ietf.org>
List-Unsubscribe: <https://www.ietf.org/mailman/options/pim>, <mailto:pim-request@ietf.org?subject=unsubscribe>
List-Archive: <https://mailarchive.ietf.org/arch/browse/pim/>
List-Post: <mailto:pim@ietf.org>
List-Help: <mailto:pim-request@ietf.org?subject=help>
List-Subscribe: <https://www.ietf.org/mailman/listinfo/pim>, <mailto:pim-request@ietf.org?subject=subscribe>
X-List-Received-Date: Tue, 13 Jul 2021 20:58:59 -0000

Dear authors:

Here is my review of this document.  I have a significant number of
comments for such a simple specification -- please see the details
inline.

This document specifies two things: (1) a Hello option to help the
tail bootstrap the BFD session, and, (2) the actions that the tail
takes when a failure is detected.  The former is common to all cases,
while the latter depends on the role of the head with respect to the
tail.  The actions are basically an acceleration of what would
naturally happen (if BFD was not used and the failure detection was
"slow").  Is this a fair characterization of the document?  Many of my
comments are about avoiding duplication (some of the descriptions are
unnecessary) and letting the base documents (rfc7761, etc.) take care
of the actual specification of the actions.  It would be ideal if the
use of the Hello option was specified once -- and the actions are
described per-case, if needed.

Are there other cases that could use this mechanism to track?  I can
think of a couple of cases: monitor PIM neighbors that send Joins, RPF
neighbors, Assert winners...  As with the DR case, for example, these
cases don't require actions beyond an acceleration.  It would be ideal
if the document could cover these cases, and possibly others, in a
generic way -- I can't think of good phrasing with now, but I'm sure
you can. ;-)


For the Chairs:  I couldn't find a request to the bfd WG for review --
did I miss it?  While this document doesn't make any changes to BFD,
it is always good practice to get an extra pair of eyes.  I'll make
sure they are aware of the document when we get to IETF LC.


Thanks!

Alvaro.


[Line numbers from idnits.]

...
8	 Bidirectional Forwarding Detection (BFD) for Multi-point Networks and
9	     Protocol Independent Multicast - Sparse Mode (PIM-SM) Use Case
10	                  draft-ietf-pim-bfd-p2mp-use-case-05

[major] This document specifies the use of BFD in a PIM network --
calling this document "use case" is misleading.  Perhaps: "Use of p2mp
BFD for PIM-SM".


12	Abstract

14	   This document discusses the use of Bidirectional Forwarding Detection
15	   (BFD) for multi-point networks to provide nodes that participate in
16	   Protocol Independent Multicast - Sparse Mode (PIM-SM) with the sub-
17	   second convergence.  Optional extension to PIM-SM Hello, as specified
18	   in RFC 7761, to bootstrap point-to-multipoint BFD session. also
19	   defined in this document.

[major] s/discusses/specifies


[nit] s/with the sub-second/with sub-second


[minor] s/[last sentence]/An extension to PIM Hello messages used to
bootstrap point-to-multipoint BFD sessions are also defined in this
document.


...
72	1.  Introduction
...
80	   [RFC7761] is the current specification of the Protocol Independent
81	   Multicast - Sparse Mode (PIM-SM) for IPv4 and IPv6 networks.
82	   Confirming implementation of PIM-SM elects a Designated Router (DR)
83	   on each PIM-SM interface.  When a group of PIM-SM nodes is connected
84	   to shared-media segment, e.g., Ethernet, the one elected as DR is to
85	   act on behalf of directly connected hosts in the context of the PIM-
86	   SM protocol.  Failure of the DR impacts the quality of the multicast
87	   services it provides to directly connected hosts because the default
88	   failure detection interval for PIM-SM routers is 105 seconds.
89	   Introduction of Backup DR (BDR), proposed in
90	   [I-D.ietf-pim-dr-improvement], improves convergence time in the PIM-
91	   SM over shared-media segment but still depends on long failure
92	   detection interval.

[nit] s/Confirming/A conforming  (??)


[nit] s/the one elected/the node elected


[nit] s/Backup DR (BDR), proposed in
[I-D.ietf-pim-dr-improvement],/the Backup DR (BDR)
([I-D.ietf-pim-dr-improvement])


[nit] s/in the PIM-SM/in PIM-SM


[nit] s/depends on long failure/depends on a long failure


94	   Bidirectional Forwarding Detection (BFD) [RFC5880] had been
95	   originally defined to detect failure of point-to-point (p2p) paths -
96	   single-hop [RFC5881], multihop [RFC5883].  In some PIM-SM
97	   deployments, a p2p BFD can be used to detect a failure and enable
98	   faster conversion.  [RFC8562] extends the BFD base specification
99	   [RFC5880] for multipoint and multicast networks, which precisely
100	   characterizes deployment scenarios for PIM-SM over a LAN segment.
101	   Among specific characteristics of p2mp BFD that are particularly
102	   benefit PIM-SM over a LAN segment is a faster transition to the Up
103	   state of the p2mp BFD session due to avoidance of the three-way
104	   handshake required in p2p BFD [RFC5880].  Also, because the router
105	   that transmits BFD Control messages uses the BFD Demand mode
106	   [RFC5880] it maintains less BFD state comparing to the Asynchronous
107	   mode.  This document demonstrates how point-to-multipoint (p2mp) BFD
108	   can enable faster detection of PIM-SM router failure and thus
109	   minimize multicast service disruption.  The document also defines the
110	   extension to PIM-SM [RFC7761] and [I-D.ietf-pim-dr-improvement] to
111	   bootstrap a PIM-SM router to join in p2mp BFD session over shared-
112	   media link.

[minor] s/This document demonstrates how point-to-multipoint (p2mp)
BFD/Point-to-multipoint (p2mp) BFD


[major] s/extension to PIM-SM [RFC7761] and
[I-D.ietf-pim-dr-improvement]/extension to PIM-SM [RFC7761]

Anything that depends on rfc7761 can use the extension.


...
116	1.1.1.  Acronyms

[minor] All of these acronyms come from other documents.  Instead of
repeating them here, it would be better to explicitly say that
familiarity with the terminology in xxx is expected.

Consider renaming this section to "Terminology"


...
146	2.  Problem Statement

148	   [RFC7761] does not provide a method for fast, e.g., sub-second,
149	   failure detection of a neighbor PIM-SM router.  BFD already has many
150	   implementations based on HW that are capable of supporting multiple
151	   sub-second sessions concurrently.

[major] What's the problem?   This paragraph reads like this: "PIM
can't detect failures fast, but BFD can."  Given the Introduction,
this section is not needed.


153	3.  Applicability of p2mp BFD

[major] This section defines the new Hello option.  Please change the
title to reflect that.  The subsections can be moved to a new section.


155	   [RFC8562] may provide an efficient and scalable solution for the
156	   fast-converging environment that demonstrates the head-tails
157	   relationship.  Each such group presents itself as p2mp BFD session
158	   with its head being the root and other routers being tails of the
159	   p2mp BFD session.  Figure 1 displays the new optional BFD
160	   Discriminator PIM Hello Option to bootstrap tail of the p2mp BFD
161	   session.

[] Except for the last sentence, this whole paragraph simply restates
what rfc8562 already does (and which should already be covered in the
Introduction).


[minor] "[RFC8562] may provide" -- may??
s/[RFC8562] may provide/[RFC8562] specifies


[minor] "head-tails relationship"   This may require a definition
somewhere -- the terminology section seems like a good place.


[nit] s/as p2mp BFD session/as a p2mp BFD session


[minor] "head being the root"  Most of this document uses "head", but
"root" is only used twice (here and below to describe "My
Discriminator").  The terminology in rfc8562 uses "head".  If the root
is the head, then it seems unnecessary to use both.


[nit] s/bootstrap tail/bootstrap the tail


163	        0                   1                   2                   3
164	        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
165	       +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
166	       |          OptionType           |         OptionLength          |
167	       +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
168	       |                       My  Discriminator                       |
169	       +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+

171	               Figure 1: BFD Discriminator PIM Hello Option

173	   where new fields are interpreted as:

175	      OptionType is a value (TBA1) assigned by IANA Section 4 that
176	      identifies the TLV as BFD Discriminator TLV;

178	      OptionLength value is always 4

180	      My Discriminator - My Discriminator value allocated by the root of
181	      the p2mp BFD session.

[] Suggestion>

   OptionType: TBD

   OptionLength: MUST be set to 4.

   My Discriminator: the discriminator [rfc5880] allocated by the head.


[major] What should the receiver do if the OptionLength is not 4.


[major] What should the receiver do if "My Discriminator" is set to 0.


183	3.1.  Using P2MP BFD in PIM DR/BDR Monitoring

185	   If PIM-SM routers that support this specification are configured to
186	   use p2mp BFD for faster convergence, then the router to be monitored,
187	   referred to as 'head', MUST create a BFD session of type
188	   MultipointHead, as defined in [RFC8562].  Note that any PIM-SM router
189	   that supports this specification, regardless of its role in PIM-SM,
190	   MAY become a head of a p2mp BFD session.  If the head doesn't support
191	   [I-D.ietf-pim-dr-improvement], but, for example, uses procedures
192	   defined in [I-D.mankamana-pim-bdr], then it MUST include BFD TLV in
193	   its PIM-Hello message.  If the head uses extensions defined in

195	   [I-D.ietf-pim-dr-improvement], then DR MUST include BFD TLV in its
196	   Hello message.  The DR Address TLV also MUST be included in the Hello
197	   message.  For a BDR, it is RECOMMENDED to include BFD TLV in its
198	   Hello message.  If BDR includes BFD TLV, then the BDR Address TLV
199	   also MUST be present in the Hello message.  As mentioned earlier, any
200	   non-DR and non-BDR MAY include BFD TLV in its Hello message.  Then
201	   the head MUST begin periodic transmission of BFD Control packets.
202	   The Source IP address of the BFD Control packet MUST be the same as
203	   the source IP address of the PIM-Hello with BFD TLV messages being
204	   transmitted by the head.  My Discriminator's field value in the BFD
205	   Control packet and My Discriminator field of the BFD TLV in PIM-
206	   Hello, transmitted by the head, MUST be the same.  When a PIM-SM
207	   router is configured to monitor the head by using p2mp BFD, referred
208	   to through this document as 'tail', receives PIM-Hello packet with
209	   BFD TLV, the tail MAY create a p2mp BFD session of type
210	   MultipointTail, as defined in [RFC8562].

[] This is a long paragraph, consider breaking it up.  Talk about the
I-D.ietf-pim-dr-improvement case first.


[minor] There's no need to repeat about support for this spec, it is
assumed for anyone reading.

OLD>
   If PIM-SM routers that support this specification are configured to
   use p2mp BFD for faster convergence, then the router to be monitored,
   referred to as 'head', MUST create a BFD session of type
   MultipointHead, as defined in [RFC8562].  Note that any PIM-SM router
   that supports this specification, regardless of its role in PIM-SM,
   MAY become a head of a p2mp BFD session.

NEW>
   The head MUST create a BFD session of type MultipointHead [RFC8562].  Note
   that any PIM-SM router, regardless of its role MAY become a head of a p2mp
   BFD session.

Note that this text is generic to any application.


[] Several comments on this text.
OLD>
   If the head doesn't support [I-D.ietf-pim-dr-improvement], but, for
   example, uses procedures defined in [I-D.mankamana-pim-bdr], then it MUST
   include BFD TLV in its PIM-Hello message.  If the head uses extensions
   defined in [I-D.ietf-pim-dr-improvement], then DR MUST include BFD TLV in
   its Hello message.  The DR Address TLV also MUST be included in the Hello
   message.  For a BDR, it is RECOMMENDED to include BFD TLV in its Hello
   message.  If BDR includes BFD TLV, then the BDR Address TLV also MUST be
   present in the Hello message.  As mentioned earlier, any non-DR and
   non-BDR MAY include BFD TLV in its Hello message.

- [major] Using the Hello options from I-D.ietf-pim-dr-improvement is already
  required there, no need to specify the use here too.  Also, because anyone
  can include the BFD option, then the requirement to include the DR/BDR
  Address option cannot be enforced.

- [major] I guess that by "BFD TLV" you mean the Hello option, right?  Please
  call things by its name.

- [major] If the BDR is a head, why is it only recommended to use the new
  option?  When it is ok if the option is not included?  If it's not included
  then the tail can't be bootstrapped.  ??

- [major] If I-D.ietf-pim-dr-improvement is not supported, then there's no way
  (on the wire) to know the difference between using I-D.mankamana-pim-bdr or
  simply rfc7761 (that I know of).  The result (assuming that my question
  above about the BDR results in it being required to send the new option) is
  that the head MUST always send the new option...which is what I was
  expecting.  IOW, the text about I-D.ietf-pim-dr-improvement/
  I-D.mankamana-pim-bdr is not needed.


NEW>
   The head MUST include the BFD Discriminator option in its Hello messages.

Note that the order of the specification may be backwards: you
probably want the tail to receive the bootstraping information before
the BFD session is set up, right?  If so, please reorder the
specification.  You may also want to wait for the PIM neighbor state
to be established before starting the BFD session.


[major] "Then the head MUST begin periodic transmission of BFD Control
packets."   Then, when?   Because there's no 3-way handshake, I guess
this is when when head considers the session to be up.  Note that this
process is already specified in rfc8562, so there's no need to specify
it here again, much less using normative language.

Note that §3.2 does mention an exception, but in general rfc8562 is unchanged.


[minor] rfc8562 already requires that the session be identified using
the source address and discriminator.

OLD>
   The Source IP address of the BFD Control packet MUST be the same as
   the source IP address of the PIM-Hello with BFD TLV messages being
   transmitted by the head.  My Discriminator's field value in the BFD
   Control packet and My Discriminator field of the BFD TLV in PIM-
   Hello, transmitted by the head, MUST be the same.

NEW>
   In order for the tail to correctly demultiplex BFD [rfc8562], the source
   address and My Discriminator values of the BFD packets MUST be the same
   as those used in the PIM Hello message.

If the values are not the same, the PIM adjacency and the head will
still send BFD packets.  It may not be obvious to the reader that the
session won't be able to be monitored.


[major] "tail MAY create a p2mp BFD session".  Again, the process for
the tail is specified in rfc8562, no need for it here.


212	   Because p2mp BFD doesn't use the three-way handshake and the head
213	   transmits BFD Control packets with the value of Your Discriminator
214	   field set to zero, [RFC8562] modified how a BFD system demultiplexes
215	   received BFD Control packet.  The tail demultiplexes p2mp BFD test
216	   session based on head's source IP address, the My Discriminator value
217	   it learned from BFD Discriminator TLV and the identity of the
218	   multipoint path that the BFD Control packet was received from.  The
219	   Detection Time for p2mp BFD sessions is defined differently from the
220	   definition provided in [RFC5880].  The Detection Time for each
221	   MultipointTail session is calculated as the product of the last
222	   received values of Desired Min TX Interval and Detect Mult.  A tail
223	   declares the BFD session down after the Detection Timer expires.  If
224	   the tail has detected MultipointHead failure, it MUST remove the
225	   neighbor.  If the failed head node was PIM-SM DR or BDR, the tail MAY
226	   start DR Election process as specified in Section 4.3.2 [RFC7761] or
227	   Section 4.1 [I-D.ietf-pim-dr-improvement] respectively.

[minor] The first few sentences are basically what the last paragraph
and rfc8562 already say...so they are redundant.  In fact, the first 5
sentences provide a summary of that rfc8562 already specifies -- not
needed here.


[minor] "it MUST remove the neighbor"  I'm assuming you mean the PIM
neighbor, right?  Please be explicit.  Note that rfc7761 talks about
deleting the neighbor state (not removing a neighbor).


[] "If the failed head node was PIM-SM DR or BDR, the tail MAY start
DR Election process as specified in Section 4.3.2 [RFC7761] or Section
4.1 [I-D.ietf-pim-dr-improvement] respectively."   Several comments...

- [minor] "DR or BDR"  Note that in the latest version of
  I-D.ietf-pim-dr-improvement the BDR is not sticky, so the election process
  is "always" running.  IOW, I don't think there's a need to mention the BDR
  here as the action of deleting the neighbor would naturally result in a BDR
  election.  In fact, if all the tails detect the failure and delete the DR
  neighbor state then that election would also naturally happen (for both
  I-D.ietf-pim-dr-improvement/rfc7761).

- [major] "the tail MAY start DR Election process"  If the DR is lost, the
  election process is not optional in either
  rfc7761/I-D.ietf-pim-dr-improvement, so there is a normative contradiction.

- [major] "Section 4.1 [I-D.ietf-pim-dr-improvement]" doesn't talk about this.
  Please don't mention specific sections to avoid falling out of sync.

- [minor] "DR or BDR...[RFC7761] or...[I-D.ietf-pim-dr-improvement]
  respectively."  "respectively" is out of place because
  I-D.ietf-pim-dr-improvement defines it's own algorithm for DR election.


OLD>
   If the tail has detected MultipointHead failure, it MUST remove the
   neighbor. If the failed head node was PIM-SM DR or BDR, the tail MAY
   start DR Election process as specified in Section 4.3.2 [RFC7761] or
   Section 4.1 [I-D.ietf-pim-dr-improvement] respectively.

NEW>
   If the tail detects a MultipointHead failure [rfc8562] it MUST delete
   the corresponding neighbor state.  If the failed head was the DR (or
   BDR), the DR (or BDR) election mechanism in [rfc7761] or
   [I-D.ietf-pim-dr-improvement] is followed.


229	   If the head ceased to include BFD TLV in its PIM-Hello message, tails
230	   MUST close the corresponding MultipointTail BFD session.  Thus the
231	   tail stops using BFD to monitor the head and reverts to the
232	   procedures defined in [RFC7761] and [I-D.ietf-pim-dr-improvement].

[minor] s/ceased to include/ceases to include


[major] Let me see if I understand: if the head doesn't use the BFD
hello option anymore then the tail can gracefully stop using BFD.
IOW, this way the BFD session does not expire and result in the DR
being declared dead.  Is that it?

Given that the BFD session can be bootstrapped at the tail by manually
configuring the corresponding discriminator, it seems that stopping
the use of the BFD hello option may not result in the expected
outcome.   ???


234	3.2.  P2MP BFD in PIM DR Load Balancing

236	   [RFC8775] defined the modification, Designated Router Load Balancing
237	   (DRLB), to the PIM-SM protocol that allows for distribution of PIM-SM
238	   DR responsibilities on a multi-access network segment.  [RFC8775]
239	   introduced the new PIM Hello options - Load Balancing Capability
240	   (DRLB-Cap) and DR Load Balancing List (DRLB-List).  PIM router that
241	   includes DRLB-Cap Hello Option MAY include BFD Discriminator PIM
242	   Hello Option (Figure 1).  That router MUST create a BFD session and
243	   set itself as MultipointHead [RFC8562].  The router MUST set
244	   bfd.SessionState in the MultipointHead session to Down.  If a PIM
245	   router that includes BFD Discriminator Option in its Hello finds its
246	   address in DRLB-List PIM Hello Option as Group Designated Router
247	   (GDR) Candidate for the first time, the router MUST set
248	   bfd.SessionState to Up and start periodically transmit BFD Control
249	   messages.  If the PIM router that was GDR Candidate doesn't find its
250	   address in the most recent DRLB-List Option, the router MUST set
251	   bfd.SessionState to Down and cease transmitting BFD Control messages.
252	   For each GDR Candidate that includes BFD Discriminator Option in its
253	   PIM Hello, PIM DR creates a MultipointTail session [RFC8562].  PIM DR
254	   demultiplexes BFD sessions based on the value in My Discriminator
255	   field and the source IP address.  If PIM DR detects a failure of one
256	   of the sessions, it MUST remove that router from the GDR Candidate
257	   list and immediately transmit a new DRLB-List Option.

[minor] The first couple of sentences of this (long!) paragraph simply
try to summarize the functionality in rfc8775.

Suggestion>
   [RFC8775] specifies the PIM Designated Router Load Balancing (DRLB)
   functionality.


[major] "PIM router that includes DRLB-Cap Hello Option MAY include
BFD Discriminator PIM Hello Option (Figure 1)."   As has been
mentioned before, any router can become a head -- this optional
specification (MAY) it then not dependent on the use of the DRLB-Cap.

OLD>
   PIM router that includes DRLB-Cap Hello Option MAY include BFD
   Discriminator PIM Hello Option (Figure 1).  That router MUST create
   a BFD session and set itself as MultipointHead [RFC8562].

NEW>
   Any PIM router that advertises the DRLB-Cap Hello Option can become
   the head of a p2mp BFD session, as specified in Section 3.1.

As I mentioned before, it would be ideal to specify the generic part
of the mechanism (bootstrapping) just once.  If anyone can become a
head, there's no need to keep repeating it.


[major] The treatment of bfd.SessionState is related to §5.9/rfc8562
(Bringing Up and Shutting Down Multipoint BFD Service), but not the
same...and in some cases normatively conflicting (requiring vs
recommending).  Please use the existing specification as the base.

OLD>
   The router MUST set bfd.SessionState in the MultipointHead session
   to Down. If a PIM router that includes BFD Discriminator Option in
   its Hello finds its address in DRLB-List PIM Hello Option as Group
   Designated Router (GDR) Candidate for the first time, the router MUST
   set bfd.SessionState to Up and start periodically transmit BFD Control
   messages.  If the PIM router that was GDR Candidate doesn't find its
   address in the most recent DRLB-List Option, the router MUST set
   bfd.SessionState to Down and cease transmitting BFD Control messages.

NEW>
   The head router administratively sets the bfd.SessionState to Up in
   the MultipointHead session [RFC8562] only if it is a Group Designated
   Router (GDR) Candidate, as specified in Sections 5.5 and 5.6 of
   [RFC8775].  If the router is no longer the GDR, then it MUST shut down
   the multipoint session and stop transmitting BFD Control packets.

I used the the requirement (MUST) as expressed above, but I still have
a question about the text from §5.9/rfc8562 where stopping the
transmission of BFD packets is only optional (MAY).  Is this case one
that warrants not following the recommendation (SHOULD) make there?
Maybe the answer is "yes" -- I would simply want to understand why.


[nit] s/PIM DR/the PIM DR


259	3.3.  Multipoint BFD Encapsulation

261	   The MultipointHead of p2mp BFD session when transmitting BFD Control
262	   packet:

[nit] s/of p2mp/of a p2mp


264	      MUST set TTL value to 1;

[major] rfc5880 requires the use of GTSM.  Is there a reason to change
that here?  rfc8562 doesn't mention TTL at all.


266	      SHOULD use group address ALL-PIM-ROUTERS ('224.0.0.13' for IPv4
267	      and 'ff02::d' for IPv6) as destination IP address

[nit] s/use group/use the group

269	      MAY use network broadcast address for IPv4 or link-local all nodes
270	      multicast group for IPv6 as the destination IP address;

[major] Under which circumstances would an operator select a broadcast
over ALL-PIM-ROUTERS?  Why are options necessary?


[minor] Note that the "network broadcast address for IPv4" is called
"limited broadcast" -- if we're talking about 255.255.255.255.


[major] Why "link-local all nodes"?  Should this be "all routers
address" instead?  Please add an informative reference to rfc4291.


[minor] As with ALL-PIM-ROUTERS, please include the specific addresses
here as well.


272	      MUST set destination UDP port value to 3784 when transmitting BFD
273	      Control packets, as defined in [RFC8562].

[major] This setting is already specified in rfc8562; there's no need
to specify it again using normative language.


275	4.  IANA Considerations

277	   IANA is requested to allocate a new OptionType value from PIM Hello
278	   Options registry according to:

[nit] s/PIM Hello/PIM-Hello


280	   +-------------+----------------+-------------------+---------------+
281	   | Value Name  | Length Number  | Name Protocol     | Reference     |
282	   +-------------+----------------+-------------------+---------------+
283	   | TBA         | 4              | BFD Discriminator | This document |
284	   +-------------+----------------+-------------------+---------------+

[major] Please use the same field names as in the registry.


286	                  Table 1: BFD Discriminator option type

288	5.  Security Considerations

290	   Security considerations discussed in [RFC7761], [RFC5880], and
291	   [RFC8562], and [I-D.ietf-pim-dr-improvement] apply to this document.

[nit] s/Security/The security


[nit] s/RFC5880], and/RFC5880],


[minor] But not rfc8775?


293	   PIM-SM link-local messages can be authenticated using various
294	   mechanisms, as described in Section 6.3 [RFC7761].  Authentication of
295	   BFD Control messages defined in Section 6.7 [RFC5880].  Each protocol
296	   MAY use authentication of its messages independently of the mode used
297	   by the other protocol.

[major] s/MAY use authentication/may use authentication
This is not a normative statement, just a fact.


[minor] The first paragraph already points at the security in
rfc5880/rfc7761.  I don't think there's a need to add this second
paragraph given that the sessions are independent.


299	   An implementation that supports this specification SHOULD use a
300	   mechanism to control the maximum number of BFD sessions that can be
301	   active at the same time.

[major] rfc8562 already requires "protective measures to prevent an
infinite number of MultipointTail sessions from being created".   It
is then not needed for this document to recommend anything that is
required elsewhere.


[major] What new security risks are introduced by the mechanism in
this draft?  In general, a rogue node can stop sending or delay BFD
packets causing the tail to conclude that the head is down: the DR/BDR
may change causing instability.  I was surprised that rfc8562 did not
mention the interaction risk, but rfc5880 already does.  I feel that
something needs to be mentioned specific to this document, even if it
is highlighted that the risk is not new.


303	6.  Acknowledgments

305	   Authors cannot say enough to express their appreciation of comments
306	   and suggestions we received from Stig Venaas.

[nit] s/Authors/The authors


...
310	7.1.  Normative References
...
327	   [RFC5881]  Katz, D. and D. Ward, "Bidirectional Forwarding Detection
328	              (BFD) for IPv4 and IPv6 (Single Hop)", RFC 5881,
329	              DOI 10.17487/RFC5881, June 2010,
330	              <https://www.rfc-editor.org/info/rfc5881>.

[minor] This reference can be Informative.


332	   [RFC5883]  Katz, D. and D. Ward, "Bidirectional Forwarding Detection
333	              (BFD) for Multihop Paths", RFC 5883, DOI 10.17487/RFC5883,
334	              June 2010, <https://www.rfc-editor.org/info/rfc5883>.

[minor] This reference can be Informative.

[End of review -05.]