Re: [bess] Lars Eggert's No Objection on draft-ietf-bess-evpn-igmp-mld-proxy-13: (with COMMENT)

["Mankamana Mishra (mankamis)" <[e-mail]>]

Hi Lars,
Please find inline .  fixing major comment in next revision.

From: Lars Eggert via Datatracker <[e-mail]>
Date: Monday, October 18, 2021 at 4:40 AM
To: The IESG <[e-mail]>
Cc: [e-mail] <[e-mail]>, [e-mail] <[e-mail]>, [e-mail] <[e-mail]>, [e-mail] <[e-mail]>
Subject: Lars Eggert's No Objection on draft-ietf-bess-evpn-igmp-mld-proxy-13: (with COMMENT)
Lars Eggert has entered the following ballot position for
draft-ietf-bess-evpn-igmp-mld-proxy-13: 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-bess-evpn-igmp-mld-proxy/



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

"Abstract", paragraph 1, comment:
>    This document describes how to support efficiently endpoints running
>    IGMP for the above services over an EVPN network by incorporating
>    IGMP proxy procedures on EVPN PEs.

It would be nice if acronyms were spelled out at least in the abstract (esp.
since it doesn't say where to go for further reading.) Also, "for the above
services"? Above where?

Mankamana :  modified,  spelled acronyms too

Section 9.4. , paragraph 6, comment:
>         0                   1                   2                   3
>         0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1
>        +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
>        | Type=0x06   |  Sub-Type=0x09|       Flags (2 Octets)      |M|I|
>        +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
>        |                           Reserved=0                          |
>        +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+

The "Type" field is only seven bits long - that seems to be an error?
Mankamana :  Thanks for catching, fixed one bit which was moved at wrong place


Section 9.5. , paragraph 20, comment:
>                              1                   2                   3
>          0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1
>          +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
>          | Type=0x06   |  Sub-Type=n   |       RT associated with EVI  |
>          +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
>          |             RT associated with the EVI  (cont.)             |
>          +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+

This diagram is only 31 bits wide?
Mankamana : Thanks for catching, fixed one bit which was missing.


You might want to use something like https://www.luismg.com/protocol/ to make
sure your header diagrams are accurate.

-------------------------------------------------------------------------------
All comments below are about very minor potential issues that you may choose to
address in some way - or ignore - as you see fit. Some were flagged by
automated tools (via https://github.com/larseggert/ietf-reviewtool), so there
will likely be some false positives. There is no need to let me know what you
did with these suggestions.

Section 11. , paragraph 2, nit:
-    provision to support IGMPv1.  There may be an implemention which is
+    provision to support IGMPv1.  There may be an implementation which is
+                                                           ++

Section 12. , paragraph 2, nit:
-    TThis document does not add any new security considirattions, Same
-                                                       ^  -
+     This document does not add any new security considerations, Same
+    -                                                  ^

"Table of Contents", paragraph 2, nit:
>  . . . . . . 18 9.2. Multicast Join Synch Route . . . . . . . . . . . . . . .
>                                     ^^^^^
Do not mix variants of the same word ("synch" and "sync") within a single text.

"Table of Contents", paragraph 2, nit:
> on of servers and switches are self contained and may have their own control
>                                ^^^^^^^^^^^^^^
This word is normally spelled with a hyphen.

Section 3. , paragraph 18, nit:
> triggering mechanism for the PEs to setup their underlay multicast tunnels.
>                                     ^^^^^
The verb "set up" is spelled as two words. The noun "setup" is spelled as one.

Section 3. , paragraph 23, nit:
> sage at the recipient EVPN speaker. Thus it helps create an IGMP overlay subn
>                                     ^^^^
A comma may be missing after the conjunctive/linking adverb "Thus".

Section 4.1.1. , paragraph 3, nit:
> et (and v2 reset). The IE flag also need to be set accordingly. Since source
>                                     ^^^^
The verb form "need" does not seem to match the subject "flag".

Section 5. , paragraph 4, nit:
> with the v3 and exclude flag set. Finally when PE1 receives the IGMPv3 Join
>                                   ^^^^^^^
A comma may be missing after the conjunctive/linking adverb "Finally".

Section 5.2. , paragraph 2, nit:
> GMP Membership Report was received. Thus it MUST only be imported by the PEs
>                                     ^^^^
A comma may be missing after the conjunctive/linking adverb "Thus".

Section 6.1. , paragraph 4, nit:
> ses an IGMP Leave Synch route for that that (ES,BD). This route notifies the
>                                   ^^^^^^^^^
Possible typo: you repeated a word.

Section 9.1. , paragraph 5, nit:
>  hosts by TORs. Upon receiving the hosts expression of interest of a particu
>                                    ^^^^^
An apostrophe may be missing.

Section 9.3. , paragraph 3, nit:
> not advertise this extended community so its absence indicates that the adver
>                                      ^^^
Use a comma before "so" if it connects two independent clauses (unless they are
closely connected and short).

Section 9.3. , paragraph 12, nit:
>  for bit 15 means that PE does not supports IGMP Proxy. * Bit 14 (shown as M
>                                    ^^^^^^^^
The auxiliary verb "do" requires the base form of the verb.

Section 9.3.1. , paragraph 4, nit:
> re attached to the associated ES. Therefore these routes carry the ES-Import
>                                   ^^^^^^^^^
A comma may be missing after the conjunctive/linking adverb "Therefore".

Section 9.3.1. , paragraph 12, nit:
> BD. The route is said to be associated associated with that BD. For each BD,
>                             ^^^^^^^^^^^^^^^^^^^^^
Possible typo: you repeated a word.

Section 9.5. , paragraph 12, nit:
>  registration policy should be "First Come First Served". 14. Acknowledgement
>                                       ^^^^
It seems that a comma is missing.

Document references draft-ietf-bess-evpn-bum-procedure-updates-08, but -11 is
the latest available revision.