Re: [pim] draft-ietf-pim-igmp-mld-extension WGLC

["Holland, Jake" <[e-mail]>]

Hi PIM wg,

I have a few minor comments on this draft.  I believe they’re
readily resolvable, but I think some go beyond nits.  Overall
the draft seems helpful to bier and possibly could support a
few more things (e.g. population count in igmp/mld proxying
might be useful if this gets any uptake).  So I support moving
it forward as soon as the issues can be resolved.

1.
As with Jeffrey’s comments, my main technical comment regards
the possibility of additional extra data beyond this extension,
from the last paragraph in section 3.

I think this possibility is confusing to handle, since if there
are 2 separate uses the ordering is not clear, so if we imagine
another bit indicating another kind of additional data, it would
strictly have to be aware of this draft to ensure its data came
afterward.

I think using some of the reserved bits as a magic number might
be a good call here, especially if we think there are existing
implementations doing something with this space.  But just
disallowing any other data when the E bit is set seems like
also a fine choice that clears up confusion.

2.
I think the doc should specify the reaction if the length fields
don't match up, or if anything exceeds the size of the packet or
the extra space.  (Do all the TLVs get ignored, or do the
apparently-good TLVs get handled an then the rest get ignored?)

3.
I think this draft's security considerations section is a bit
light for the complexity it introduces.  It might be worth checking
how they handled this in the UDP options draft, I think the first
and third paragraphs might have some useful language to copy:
https://tools.ietf.org/html/draft-ietf-tsvwg-udp-options-09#section-17

4.
The length fields described in section 3 should specify the units
(that the lengths are given in octets).

I also saw a few nits:
5.
In the 2nd paragraph of section 1, I don't know what "resp."
means here, and it's not defined.  (From context it seems to be
indicating that MLD and IGMP get the same language with different
references?):
   The extension will be part of additional data as mentioned in
   [RFC3810] Section 5.1.12 (resp.  [RFC3376] Section 4.1.10) for query
   messages and [RFC3810] Section 5.2.12 (resp.  [RFC3376]
   Section 4.2.11) for report messages.

6.
2nd to last paragraph of section 4, singular "host" should be plural:
   the behavior of host and routers supporting the new types, consider

7.
Last paragraph of section 4 has confusing phrasing with a couple of
plurality mismatche ("mechanism do not" and "nodes may send older
message"), and I think it's not quite right to say that a mechanism
supports a protocol, rather than that the document defines a mechanism
only for certain protocol versions.  You could just fix the plurality,
but I'll suggest some different text that sounds more correct to me, but
please feel free to use it or not:

OLD:
   The extension mechanism do not support IGMPv1, IGMPv2 and MLDv1.  As
   nodes may send older version message, they would also not be able to
   send messages using this extension.

Suggested:
   This document defines an extension mechanism only for IGMPv3 and MLDv2.
   No E bit or extension mechanism is defined here for IGMPv1, IGMPv2, or
   MLDv1.


Best regards,
Jake


From: Michael McBride <[e-mail]>
Date: Monday, December 7, 2020 at 4:23 PM
To: "[e-mail]" <[e-mail]>
Subject: [pim] draft-ietf-pim-igmp-mld-extension WGLC

Hello all,
 
Today begins a two week wglc for https://urldefense.com/v3/__https:/tools.ietf.org/html/draft-ietf-pim-igmp-mld-extension-02__;!!GjvTz_vk!EKc8vPWuhUwwzwkh8HXSN0mQ88VZX4J9iug9ei03YduV64M2tP9qW4Om3ngQn08$
 
Please review (it’s a quick read) and let us know your opinion on it’s readiness to progress to the iesg.
 
Thanks,
mike