[pim] Benjamin Kaduk's No Objection on draft-ietf-pim-igmp-mld-extension-06: (with COMMENT)

[Benjamin Kaduk via Datatracker <[e-mail]>]

Benjamin Kaduk has entered the following ballot position for
draft-ietf-pim-igmp-mld-extension-06: 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/blog/handling-iesg-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-igmp-mld-extension/



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

Thanks for this work; it's good to see a well-designed extension mechanism
be provided for these longstanding protocols.

Section 3

If there is to be no alignment or padding, having an example of that (or
of an unaligned "tail" with gap in the figure) would probably help drive
that point home.

Section 4

   Future documents defining a new type MUST specify any additional
   processing and validation.  These rules, if any, will be examined
   only after the general validation (above) succeeds.

Just to confirm: this means that if the extensions block is malformed per
the ruls above, all extensions in it (even ones that could be fully
parsed) are ignored?  There are probably security issues if some
implementations do partial processing and others don't, which we might
call out in the security considerations if needed.

Section 5

   IGMP and MLD implementations, particularly implementations on hosts,
   rarely change, and the adoption process of this extension mechanism
   is expected to be slow.  Also as new extensions are defined, it may
   take a long time before they are supported.  Due to this, defining
   extensions should not be taken lightly, and it is crucial to consider
   backwards compatibility.

In the vein of the intdir reviewer's remarks, it seems like it would be
useful for testing implementation correctness if *some* TLV type was
defined now, even if it's just a "padding" extension with semantics of
"value bytes must be zeros".  As written, this sounds like we're asking
people to implement something, wait years for it to be deployed and a
"real" TLV defined, and only then uncover implementation bugs.  Having
something defined now that people can play around with and use for testing
seems like it would be valuable for evaluating implementation quality.

   Implementations that do not support this extension mechanism will
   ignore it, as specified in [RFC3376] and [RFC3810].

The spec clearly says so, but are we sure?  Have we tested it?  Being able
to follow this sentence up with "and confirmed in real-world testing of
major implementations" would be a big confidence boost.

NITS

Section 1

   When this extension mechanism is used, it replaces the Additional
   Data section defined in IGMPv3/MLDv2 for TLVs.

The construction "<A> replaces the <X> defined in <Y> for <Z>" seems to only
be parsable as saying that X is defined for Z, not that A is used for Z.
"replaces ... with" or "repurposes ... for" seem like viable alternatives.

Section 3

   IGMPv3 and MLDv2 messages are defined so that they can fit within the
   network MTU, in order to avoid fragmentation.  When this extension
   mechanism is used, the number of Group Records in each Report message
   SHOULD be kept small enough that the entire message, including any
   extension TLVs can fit within the network MTU.

"Group Record" seems to be a defined term for IGMP but not MLD; do we want
to tweak the wording here to be fully generic?

Also, comma after "extension TLVs".

Section 4

   Unsupported types MUST be ignored.

Is this "the value portion, if any, of extensions with unsupported types
MUST be ignored"?  Or does ignoring even the type as well come into play?

Section 5

   message types.  It MUST also be specified what the behavior should be
   if a message is not used in the defined manner, e.g., if it is
   present in a query message, when it was only expected to be used in
   reports.

The bit after "e.g." provides an example only of the "not used in the
defined manner" part, and not the "what the behavior should be" part.
That's not inherently problematic, but perhaps an example of both is
possible.