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

From: <[email-masked]>
Date: Thu, 22 Jul 2021 07:42:32 +0800 (CST)

Hi Alvaro,
I've got through your detailed comments and prepared an updated version of the draft. Once again, many thanks for your comments and helpful suggestions. I agree with the principle that a document is ready not when there's nothing to add but when there's nothing left to be removed without reducing the value. With that, please find my answers and notes in-lined below under the GIM>> tag. A new version and its diff are attached for your review and comments.

Regards
Greg Mirsky
Sr. Standardization Expert
预研标准部/有线研究院/有线产品经营部  Standard Preresearch Dept./Wireline Product R&D Institute/Wireline Product Operation Division
E: [email-masked]
www.zte.com.cn
------------------Original Mail------------------
Sender: AlvaroRetana
To: [email-masked];
CC: Mike McBride;[email-masked];[email-masked];
Date: 2021/07/13 13:59
Subject: [pim] AD Review of draft-ietf-pim-bfd-p2mp-use-case-05
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? 
GIM>> Yes, absolutely correct.
 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. ;-)
GIM>> I agree that the defined PIM Hello BFD Discriminator option can be used by not only PIM DR/BDR nodes. Indeed, there are other use cases where a faster detection will improve the convergence in the control plane and minimize the negative impact on the multicast data plane. These use cases may be covered in the future. As I think of it, one aspect would be homogeneity of p2mp BFD capability throughout the domain. In other words, what happens if some PIM nodes don't support the BFD Discriminator option and do not use p2mp BFD? What their slow (regular) convergence impact other nodes? But that, I think, is for further discussion, work. Would you agree?

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".
GIM>> As you've pointed out in the general comment, the scope of this document does not include all the possible cases where p2mp BFD can be applied in the PIM environment. Hence, describing it as a use case might be reasonable. But I am not stuck on it and would change the title if it seems confusing to a reader.

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.
GIM>> Thank you. All three are applied in the working version.

..
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
GIM>> Yes to all suggestions, thanks.

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.
GIM>> Agreed and removed the extra reference.

..
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"
GIM>> Done, thx.

..
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.
GIM>> Thank you for pointing it out to me. I agree and the section is removed from the new version.

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.
GIM>> Changed to "BFD Discriminator PIM Hello Option". Hope that is acceptable

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).
GIM>> Removed the text that re-states RFC 8562.

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

[minor] "head-tails relationship"   This may require a definition
somewhere -- the terminology section seems like a good place.
GIM>> As per the previous comment, that text is removed. Now the sections starts with the following:
    Figure 1 displays the new optional BFD Discriminator PIM Hello option    
    to bootstrap a tail of the p2mp BFD session.    
Would you agree?

[nit] s/as p2mp BFD session/as a p2mp BFD session
GIM>> Done

[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.
GIM>> Agreed and converged to using only "head".

[nit] s/bootstrap tail/bootstrap the tail
GIM>> Done

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.
GIM>> Done them all as proposed.

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.
GIM>> I've re-worked that part of the document as follows (and the changes reflect your other comments and suggestions):
  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.  The head MUST include the BFD
   Discriminator option in its Hello messages.

   If a PIM-SM router is configured to monitor the head by using p2mp
   BFD, referred to through this document as 'tail', receives PIM-Hello
   packet with BFD Discriminator PIM Hello option, the tail MAY create a
   p2mp BFD session of type MultipointTail, as defined in [RFC8562].

   The node that includes the BFD Discriminator PIM Hello option
   transmits BFD Control packets periodically.  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 that is not the case, the tail BFD
   node would not be able to monitor the state of the PIM-SM node, that
   is the head of the p2mp BFD session, though the regular PIM-SM
   mechanisms remain fully operational.

   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.

[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.
GIM>> I think that the node that is the MultipointHead and uses the BFD Discriminator option to bootstrap its PIM neighbors does not need to wait for any of the neighbor to create its MultipointLeaf. Consider the case of a new PIM node coming up in the segment with the already existing p2mp BFD session. The node will learn the BFD Discriminator, create the p2mp BFD session as the MultipointLeaf and will monitor the state of the head independently of other PIM nodes in the segment. And the head wouldn't notice that the new MultipointLeaf is created.

[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.
GIM>>In fact, bootstrapping p2mp BFD session and transmitting BFD control packets are independent processes because the head is not monitoring the state of the mulricast tree or of leaves (That is possible but is not needed in the case considered in the draft.) The updated text, listed above, includes changes to this as follows:
   The node that includes the BFD Discriminator PIM Hello option
   transmits BFD Control packets periodically.

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.
GIM>> Thank you for the suggested update, applied.

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.
GIM>> I've added the following note to make that clearer:
   If that is not the case, the tail BFD
   node would not be able to monitor the state of the PIM-SM node, that
   is the head of the p2mp BFD session, though the regular PIM-SM
   mechanisms remain fully operational.

[major] "tail MAY create a p2mp BFD session".  Again, the process for
the tail is specified in rfc8562, no need for it here.
GIM>> I think that is an important step that ties BFD Discriminator option with p2mp BFD. The updated text now reads as:
   If a PIM-SM router is configured to monitor the head by using p2mp
   BFD, referred to through this document as 'tail', receives PIM-Hello
   packet with BFD Discriminator PIM Hello option, the tail MAY create a
   p2mp BFD session of type MultipointTail, as defined in [RFC8562].

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.
GIM>> I've removed the first five sentences. Hope it is clearer without any loss of information.

[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).
GIM>> In Section 4.3.1 RFC 7761 is noted that
   This will cause PIM neighbors
   to remove this neighbor (or its old IP address) immediately.
I've updated the draft to the following text:
   If the tail detects a MultipointHead failure [RFC8562], it MUST
   delete the corresponding neighbor state.

[] "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.
GIM>> All updates are accepted and applied in the working version.

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?
GIM>> Yes, that is what we've intended - revert to "slow" detection.

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.   ???
GIM>> Yes, the head's My Discriminator value can be provisioned using the management plane. If that is the case, then I think this document is not applicable as the head and leaves use RFC 8562 without any additions.

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.
GIM>> All proposed and suggested updates are accepted with much gratitude and applied in the working version of the document.

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.
GIM>> Thank you for pointing out this ambiguity. "MUST" is intended for "shut down the multipoint session". In fact, the intention is that the head, as you've mentioned, can choose how to close the session - transmit three BFD control packets with State Down or stop transmitting packets. I've updated text as follows:
   If the router is no longer the
   GDR, then it MUST shut down following the procedures described in
   Section 5.9 [RFC8562].

[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
GIM>> Done them all.

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.
GIM>> Changed TTL/Hop Limit to 255. RFC 8562 doesn't discuss the TTL/Hop Limit because p2mp BFD may be used in single-hop or multi-hop scenario.

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?
GIM>> I agree and removed the reference to other options but ALL-PIM-ROUTERS.

[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.
GIM>> Agreed and removed it.

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],
GIM>> All sugegstion above accepted and applied.

[minor] But not rfc8775?
GIM>> Added RFC 8775.

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.
GIM>> Done, thank you.

[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.
GIM>> Agree and removed it.

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.
GIM>> Done.

[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.
GIM>> Then that "rogue" node is the PIM DR/BDR, not a man-in-the-middle. And the attack is by making the p2mp BFD session expire on leaves while still periodically sending PIM Hello. AFAIK, since PIM DR/BDR election takes several Hello cycles, I don't think that that behavior will affect the multicast service. Perhaps I'm missing something, please advise.

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                  .

[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, .

[minor] This reference can be Informative.
GIM>> Done, thank you.
[End of review -05.]

_______________________________________________
pim mailing list
[email-masked]
https://www.ietf.org/mailman/listinfo/pim