Telechat Review of draft-ietf-pim-null-register-packing-14
review-ietf-pim-null-register-packing-14-intdir-telechat-eastlake-2023-02-28-00

I am an assigned INT directorate reviewer for
<draft-ietf-pim-null-register-packing-14>. These comments were written
primarily for the benefit of the Internet Area Directors. Document editors
and shepherd(s) should treat these comments just like they would treat
comments from any other IETF contributors and resolve them along with any
other Last Call comments that have been received. For more details on the
INT Directorate, see https://datatracker.ietf.org/group/intdir/about/ <
https://datatracker.ietf.org/group/intdir/about/>.

I am not intimately familiar with PIM so my apologies if some of my
comments are due to confusion.

Based on my review, if I was on the IESG I would ballot this document as
DISCUSS.

*I have the following DISCUSS level issue:*

Maybe it is just me but I have problems with the last paragraph of Section
6.3. If the packing capability is disabled by management and the
implementation chooses not to parse any packed registers, I would say it
MUST NOT falsely advertise the Packing Capability but the draft says
"SHOULD NOT". If there is a good reason why "SHOULD" is OK, perhaps that
reason should be mentioned in the draft.


*The following are other issues I found with this document that SHOULD be
corrected before publication:*
Section 1, Page 2, 2nd paragraph. Text should be worded to be correct when
the RFC is published. "proposes'' -> "specifies''.
    It seems to me that the new messages pack multiple group and source
address pairs, they don't actually pack multiple Null-Register or
Register-Stop "messages" into one message.
    Also, more minor, but the bit about "do not contain encapsulated data"
should either be dropped or it should be made clearer why that matters. I
suggest something like
OLD
   This draft proposes a method to efficiently pack multiple PIM Null-
   Registers and Register-Stops [RFC7761] into a single message as these
   packets do not contain encapsulated data.
NEW
   This draft specifies a method to, in effect, efficiently pack
   multiple group and source address pairs into PIM Null Register and
   PIM Register-Stop [RFC7761] messages. The resulting messages have a
   different type/subtype and are referred to as PIM Packed
   Null-Register and PIM Packed Register-Stop messages. Since these
   packets do not contain encapsulated data, there is space for a
   significant number of group and source address pairs.

Sections 3 and 4: In my opinion, these need to say something about message
parsing. Perhaps something like the following:  "The number of group and
source address pairs present is determined by the size of the PIM Packed
{Null-Register, Register-Stop} and the size of each group and source
address."
    Also, if a message doesn't parse because it ends truncating a
group/source pair, should the whole message be discarded on the grounds
that it is corrupt and untrustworthy or should any earlier complete
group/source pairs still be used?

Section 6.2 and other sections: This draft seems to be worded confusingly
about whether it is talking about "packing" or not packing a message (which
seems to not be true) or whether it is sending the old message or the new
message with a new type/subtype which is packed (which seems to be the
actual case). For example, the first sentence of this section says
"... decide to pack multiple Null-Register messages ..." but that seems
misleading. Shouldn't it be "... decide whether to send a PIM Packed
Null-Register message or multiple PIM Null-Register messages ..." The
wording of the draft should be checked overall for issues like this.


*The following are minor issues with the document:*
Section 1.2: I think there should be more entries in this section, such as
MSDP and MTU.

Section 2, Page 3, below Figure 1. The new bit is shown as "P" in the
Figure. So, in the text, it should be indicated as such. Something like

P: Packing capability bit (Flag Bit TBD1) which, when set, indicates the
ability of the RP to receive PIM Packed Null-Register messages and send PIM
Packed Register-Stop messages.


Section 7 and 8. I found the sudden introduction of the term "record" in
these two secontions late in the draft quite jarring. Either "record"
should be used throughout the draft and defined up front or "record" should
be replaced with "group and source address pair". With this second choice,
you get to eliminate the last sentence of Section 7.

Thanks,
Donald
===============================
 Donald E. Eastlake 3rd   +1-508-333-2270 (cell)
 2386 Panoramic Circle, Apopka, FL 32703 USA
 d3e3e3@gmail.com