Subject: [pim] AD Review of draft-ietf-pim-source-discovery-bsr-06

From: Alvaro Retana <[email-masked]>
Date: Wed, 6 Dec 2017 07:12:30 -0800

Dear authors/Chairs/WG:

I just finished reading this document.  In general, this is a straight
forward mechanism, but it needs some more details around the specification
(even if its Status is Experimental).

What is the experiment?  I am happy that the mechanism has been tested in
the field (from Section 2) — the question here is: why is this document
Experimental?  What do you hope to learn from experimenting with this
proposal?  The answer could be as simple as “to gain experience in other
types of deployments” (which ones?) to something like “experimentation with
different holdtimes and origination intervals” (just making that up —
obviously!).  Maybe it is to get experience and/or more information to
answer some of the questions below. ;-)

I am also asking the question about the Status because this document
defines two related, but different things: PFM ("generic mechanism that can
flood any kind of information”) and the GSH TLV (which is one of the
potential applications for PFM).  Do you (the WG) want to tie the source
discovery experiment to the generic flooding mechanism?  I didn’t find
other drafts referencing this one, indicating that there are no other
published extensions — so maybe the answer to the question is “Yes”.

Note that I’m generally ok with the Experimental Status, I just want to
better understand the experiment and confirm that you considered the 2
technical specifications independently.

There is one more reason I’m asking about the Status:  the No-Forward bit
(in 3.1) is assigned out of a field that rfc7761 marks as Reserved ("Set to
zero on transmission.  Ignored upon receipt.”).  An Experimental document
can’t update a Standards Track one (and rfc7761 is now an Internet
Standard) — not to mention that this Reserved field in the header common to
all PIM messages.  As far as I can tell, no other PIM message has used the
Reserved field for anything, and the application here seems very specific
to PFM (or even PFM-SA) — so there are a few different ways forward to
allow this document to use bits in the Reserved field.  In general the
options (that pop into my head right now) are:

(1) Define a registry that specifies how the Reserved bits are to be
allocated (common to all PIM messages).  Some of those bits could be
message-specific.

(2) Declare the Reserved bits message-specific…so that every message can
make use of them as it sees fit.  Then each message (PFM, for example) can
assign the bits (and define how others could be assigned).

(3) The No-Forward bit could be defined elsewhere (not in the common
header) — this is the easy solution.

(4) …there might be other options…

In the meantime, I have other comments below.

Thanks!

Alvaro.

Major:

M1. In 3.1. (PFM message format): "PIM Version:   Reserved, Checksum
Described in [RFC7761].”  I guess you mean that all 3 fields are described
in rfc7761, right?

M2. From 3.2.1. (Initial checks):

M2.1. What does it mean to have an “active Hello state”?  I can guess, but
no one should have to.

M2.2. “...the interface MUST NOT be an administrative boundary for PFM” —
what is an administrative boundary interface?  How is the boundary
different for PFM (when compared to PIM)?  Section 3. (A generic PIM
flooding mechanism) says that PFM "can flood...throughout a PIM domain.  It
is not necessarily a domain though, it depends on the administrative
boundaries being configured”, but no details of what that means are
provided anywhere.

M2.3. For my own education, why is this check neede: “If No-Forward is set,
we MUST have restarted within 60 seconds.”?   BTW, “MUST have” doesn’t make
sense Normatively.

M3. In 3.2.2. (Processing and forwarding of PFM messages): “...TLVs in the
received message...if the most significant bit in the type field is set
(the type value is larger than 32767) and we do not support the type, then
that particular type should be omitted from the forwarded messages”.  I’m
not sure if you mean that Types > 32767 should be used if you want every
node to be aware (which is why TLVs are not forwarded if not supported),
OR, that the Types are really 15 bits long and that the most significant
bit is reserved to indicate the forwarding behavior.  In either case, the
text here, in the Type description in 3.1 and in the IANA Considerations
section should be updated to clarify.

M4. From 4.1. (Group Source Holdtime TLV): "Length:   The length of the
value.”…in octets, bytes, bits??

M5. Section 4.2. (Originating PFM messages):

M5.1. "Note that this address SHOULD be routeable due to RPF checking.”
 What are the conditions when it is ok for the address not to be routable?
Why is the SHOULD not a MUST?

M5.2. I think you need an “Originating PFM Messages” Section as 3.* to talk
about origination in the general case.  For example, should origination
always be periodic, does it depend on the type of TLVs carried, what if
different TLVs have different periods, can the receiver infer anything from
not receiving a TLV (and what should the sender do about it), etc. ??
 [Suggestion: rename 4.2 as “Originating GSH TLVs”.]

M6. Section 4.3. (Processing GSH TLVs)

M6.1. "A router that receives a PFM message containing GSH TLVs SHOULD
parse the message and store each of the GSH TLVs as SG mappings with a
holdtimer started with the advertised holdtime.”  What are the conditions
where a router wouldn’t parse and store?  IOW, why is this SHOULD not a
MUST?

M6.2. "For each group that has directly connected receivers, this router
SHOULD send PIM (S,G) joins for all the SG mappings advertised in the
message for the group.”  Again, why is this SHOULD not a MUST?  When would
the router not send a join?

M6.3. [minor] "For instance, if there is a large number of sources for a
group, there may be multiple PFM messages, each message containing a
different list of sources for the group.”  s/PFM messages/GSH TLVs.   As I
mentioned above, I think you need a section talking about PFM-specific
behavior and this one about GSH-specific behavior.

M7. Section 5. (Security Considerations)

M7.1. "The security considerations are mainly similar to what is documented
in [RFC5059].”  “Mainly similar”???  What is different?  Just one item that
jumped at me: rfc5059 talks about the risk of DoS using IPsec, but rfc7761
took IPsec authentication out of PIM-SM.  I wonder what would be
easier/clearer: to mention the exceptions wrt rfc5059 or to roll your own
considerations based on rfc7761.  Either way would work for me, but (to
avoid rehashing the rfc7761 discussions) making rfc7761 a significant part
of the security considerations would be good.

M7.2. "PFM packets must only be accepted from a PIM neighbor.”  Should that
“must” be a MUST?

M7.3. Because PFM "can flood any kind of information throughout a PIM
domain”, are there additional considerations around that?  I’m thinking
privacy, for example: if I can really flood anything, then I could put in
location information of the sources/receivers, which could reveal things
about them (not to mention whatever “any kind of information” could
include).  You might want to rethink the description of the expected use
scope for PFM.

M8. References:  rfc5226 has been obsoleted by rfc8126 — this reference
should be Normative.

Minor:

P1. In 4.5. (Resiliency to network partitioning): "As soon as the network
heals, the SG Mappings are re-flooded into the other partition(s) and other
receivers can join to the newly learned sources.”  It is not clear in the
text in Section 3.2.2. (Processing and forwarding of PFM messages) that
this result would be achieved as it says that "After processing, we forward
the message.”, and not that the messages are forwarded when a new neighbor
is found (which would be the equivalent of the network healing).

Nits:

N1. Please don’t use “we” throughout the specification.  For example:
instead of "After processing, we forward the message.”, perhaps “After
processing, the message is forwarded.”

N2. The numbers on top of the packet formats are misaligned.

N3. s/[N]o-Forward bit/N bit (No-Forward)   Note that No-Forward is used
later in the text.