Last Call Review of draft-ietf-pim-source-discovery-bsr-07
review-ietf-pim-source-discovery-bsr-07-genart-lc-bryant-2018-01-09-00

I am the assigned Gen-ART reviewer for this draft. The General Area
Review Team (Gen-ART) reviews all IETF documents being processed
by the IESG for the IETF Chair.  Please treat these comments just
like any other last call comments.

For more information, please see the FAQ at

<https://trac.ietf.org/trac/gen/wiki/GenArtfaq>.

Document: draft-ietf-pim-source-discovery-bsr-07
Reviewer: Stewart Bryant
Review Date: 2018-01-09
IETF LC End Date: 2018-01-10
IESG Telechat date: 2018-01-25

Summary: A well written document, but which could usefully be polished to make
it even better.

Major issues: None

Minor issues:

   E.g., if
   some information needs to be sent every 60 seconds and a triggered
   PFM is about to be sent 20 seconds before the next periodic PFM was
   scheduled, the triggered PFM might include the periodic information
   and the next periodic PFM can then be scheduled 60 seconds after
   that, rather than 20 seconds later.

SB> I am not sure is the complexity of this optimization is worth the
SB> gain. In general optimizations are a source of implementation bugs
SB>

Nits/editorial comments:

In the Abstract  RP need expanding.

topology changes, including frequest link or router failures. (frequent
incorrectly spelled)

no bandwidth left for prune message (message should be plural)

If all the TLVs in the
message are boundary TLVs, then the message is effectively ignored.
SB> Ignored or discarded?

Then a couple of lines later:
If the message was discarded or all the
TLVs were ignored, then no message is forwarded.
SB> You have both terms (ignored and discarded) which confuses me.

In 3.4.1 you pull 60s out of a hat. All becomes clearer later. It would be 
better for the reader if you explained that this was your choice of default
timer before first use.

   if the most significant bit in the type field is set (the type value
   is larger than 32767) and this system does not support the type, then
   that particular type should be omitted from the forwarded messages.
SB> What the authors describe is fine and correct, but it might be
SB> cleaner to call the top bit the "omit bit" or some memorable name.
SB>  Of course the fact that it is the top bit of a TLV type make make such
SB> a definition strange.

A Group Source Holtime ( typo Holdtime)

  The PFM messages containing the GSH
   TLV are periodically sent for as long as the multicast source is
   active, similar to how PIM registers are periodically sent.  The
   default announcement period is 60 seconds,

SB> This should be called out earlier in the text to show why you keep
SB> using the 60s value.

  The holdtime for the source is by default 210
   seconds.  Other values MAY be configured, but the holdtime MUST be
   either zero, or larger than the announcement period.  It is
   RECOMMENDED to be 3.5 times the announcement period.  A source MAY be
   announced with a holdtime of zero to indicate that the source is no
   longer active.

SB> This would be clearer if you started with the 3.5 AP recommendation
SB> and then got to 210s

  When a new PIM neighbor is detected, or an existing neighbor changes
   GenID, an implementation MAY send a triggered PFM message containing

SB> GenID seems to be used but not defined.

   This document requires the assignment of a new PIM message type for
   the PIM Flooding Mechanism (PFM).

SB> It is normal to be more explicit in describing the IANA action.

   IANA is also requested to create a
   registry for PFM TLVs,

SB> It is usual to specify the namespace for the registry.

   with type 0 assigned to the "Source Group
   Holdtime" TLV.

SB> use of zero is a common cause of uninitialized variable error.