[pim] John Scudder's No Objection on draft-ietf-pim-assert-packing-10: (with COMMENT)

[John Scudder via Datatracker <[e-mail]>]

John Scudder has entered the following ballot position for
draft-ietf-pim-assert-packing-10: No Objection

When responding, please keep the subject line intact and reply to all
email addresses included in the To and CC lines. (Feel free to cut this
introductory paragraph, however.)


Please refer to https://www.ietf.org/about/groups/iesg/statements/handling-ballot-positions/ 
for more information about how to handle DISCUSS and COMMENT positions.


The document, along with other ballot positions, can be found here:
https://datatracker.ietf.org/doc/draft-ietf-pim-assert-packing/



----------------------------------------------------------------------
COMMENT:
----------------------------------------------------------------------

# John Scudder, RTG AD, comments for candidate draft-ietf-pim-assert-packing-11
commit fc9d64f CC @jgscudder

## COMMENTS

(And some nits mixed in, marked as such.)

Thanks for addressing my earlier DISCUSS! I have a few residual comments, only
one of which I think is problematic (my comment on §3.3.1.1). Although I hope
you'll address that comment, I'm not keeping it as a DISCUSS point.

### Section 3.3.1

   When using assert packing, the regular [RFC7761] Assert message
   encoding with A=0 and P=0 is still allowed to be sent.  Routers are
   free to choose which PackedAssert message format they send.

Do you mean "free to choose which Assert message format they send"?

### Section 3.3.1

   *  Implementations SHOULD be able to indicate to the operator (such
      as through a YANG model) how many Assert and PackedAssert messages
      were sent/received and how many assert records where sent/
      received.

Nit: where -> were

### Section 3.3.1

   *  Implementations that introduce support for PackedAsserts after
      support for Asserts SHOULD support configuration that disables
      PackedAssert operations.

I get what you are trying to do here, mainly because you explained it in email.
Let me suggest some different wording. Maybe something like,

   *  A configuration option SHOULD be available to disable PackedAssert
      operations. An exception exists for implementations that do not
      have any native support for legacy Assert operations, these
      implementations MAY omit the configuration option.

### Section 3.3.1.1

I'm still having difficulty following this section, sorry.

   Asserts/PackedAsserts in this case are scheduled for serialization
   with highest priority, such that they bypass any potentially earlier
   scheduled other packets.

I guess this means, maintain a queue with at least two priority levels, with
the high priority being reserved for reception-triggered assert records. (I
know in your email you expressed allergy to the word "queue" but that's exactly
what it seems like you're describing, without using the word.)

                             When there is no such Assert/PackedAssert
   message scheduled for or being serialized,

The "such" is a point of difficulty here -- its referent seems to be "earlier
scheduled other packets" which are eligible to be bypassed. Or does it mean
reception-triggered asserts? I think you can fix this problem by spelling it
out instead of indirecting through "such".

                                              the router immediately
   serializes an Assert or PackedAssert message without further assert
   packing.

I'm also wondering what exactly you intend by "immediately serializes". Control
plane software I'm familiar with lives up in userspace and uses the socket
interface to push stuff in the direction of the network; for the messages to
get written out to the wire it's not uncommon for several additional layers to
be involved, different processors, a router backplane which itself may look
like a network interface, etc. I think/hope you just intend that the control
plane software should manage the order in which it writes things to the socket
interface, but your use of phrases like "the router... serializes" makes it
smell a little like you think this tuning is going to be applied all the way
down to writing the packet to the wire.

   If there are one or more reception triggered Assert/PackedAssert
   messages already serializing and/or scheduled to be serialized on the
   outgoing interface,

... the concern continues with the above. In cases I'm familiar with, I don't
have that level of detail available to me about the interface queues when I'm
sitting up in the routing daemon.

                       then the router can use the time until the last
   of those messages will have finished serializing for PIM processing
   of further conditions that may result in additional reception
   triggered assert records as well as packing of these assert records
   without introducing additional delay.

But no matter what the paradigm is I still don't get what the fragment above is
telling me. The best guess I have right now (but it is not a very strong guess)
is that you're saying, during the time a message is being written to the wire,
we can aggregate records, according to priority order, into the next-in-line
message buffer. Although of course, I don't know the details of every control
plane implementation, I find it somewhat difficult to believe that this level
of optimization will ever see the light of day.

More colloquially, maybe what you're saying is, "please pack the buffers as
full as can be done without inducing unnecessary latency"? And putting all my
guesses on this point together, maybe what this subsection is saying is,
"reception-triggered asserts should be prioritized above other asserts; when
constructing messages to send, please pack the buffers as full as can be done
without inducing unnecessary latency"?

Also, nit: I would use a hyphen in "reception-triggered" since the two words
are being used together as an adjective. Similarly in 3.3.1.2, "timer
expiry-triggered".

### Section 3.3.1.3

Nit, subsectons -> subsections
Nit, be achieve -> be achieved

### Section 3.3.1.5

Nit, througput -> throughput

### Section 6

I guess it may be so obvious as to not need spelling out, but greenfield
PackedAssert-only implementations (bullet 5 of §3.1.1) would be rendered
useless if placed on a LAN with a legacy Assert-only implementation, because of
the MUST in the first bullet of §3.3.1:

   *  When any PIM routers on the LAN have not signaled support for
      Assert Packing, implementations MUST send only Asserts and MUST
      NOT send PackedAsserts under any condition.

I wonder if this (the ability to completely silence such an implementation by
advertising non-support for PackedAssert) represents an interesting enough
attack that it's worth calling out in the Security Considerations? Perhaps this
is already captured in some of the underlying spec SecCons, but I'm not sure.

## Notes

This review is in the ["IETF Comments" Markdown format][ICMF], You can use the
[`ietf-comments` tool][ICT] to automatically convert this review into
individual GitHub issues.

[ICMF]: https://github.com/mnot/ietf-comments/blob/main/format.md
[ICT]: https://github.com/mnot/ietf-comments