Re: [Gen-art] Genart last call review of draft-ietf-bess-evpn-igmp-mld-proxy-12

[Lars Eggert <[e-mail]>]

Matt, thank you for your review. I have entered a No Objection ballot for this document.

Lars


> On 2021-8-27, at 18:39, Matt Joras via Datatracker <[e-mail]> wrote:
> 
> Reviewer: Matt Joras
> Review result: Ready with Nits
> 
> I am the assigned Gen-ART reviewer for this draft. The General Area
> Review Team (Gen-ART) reviews all IETF documents being processed
> by the IESG for the IETF Chair.  Please treat these comments just
> like any other last call comments.
> 
> For more information, please see the FAQ at
> 
> <https://trac.ietf.org/trac/gen/wiki/GenArtfaq>.
> 
> Document: draft-ietf-bess-evpn-igmp-mld-proxy-??
> Reviewer: Matt Joras
> Review Date: 2021-08-26
> IETF LC End Date: 2021-09-07
> IESG Telechat date: Not scheduled for a telechat
> 
> Review
> 
>   Ethernet Virtual Private Network (EVPN) solution is becoming
>   pervasive in data center (DC) applications for Network Virtualization
>   Overlay (NVO) and DC interconnect (DCI) services, and in service
>   provider (SP) applications for next generation virtual private LAN
>   services.
> 
> This intro to the abstract could use some rewording. For example, "is becoming"
> does not read well in a standards document. It is sufficient to describe what
> this document is specifying. Also "next generation" is overused and doesn't
> usually read well retrospectively.
> 
>   This draft describes how to support efficiently endpoints running
>   IGMP for the above services over an EVPN network by incorporating
>   IGMP proxy procedures on EVPN PEs.
> 
> Avoid using the term "draft" as this will have to be edited out since the idea
> is for this to be standards track.
> 
> 1.  Introduction
> 
>   Ethernet Virtual Private Network (EVPN) solution [RFC7432] is
>   becoming pervasive in data center (DC) applications for Network
>   Virtualization Overlay (NVO) and DC interconnect (DCI) services, and
>   in service provider (SP) applications for next generation virtual
>   private LAN services.
> 
> This is a copy of the abstract, and has similar issues. The introduction serves
> a different purpose beyond the abstract, it also has the same grammatical
> issues as the abstract.
> 
>   In DC applications, a point of delivery (POD) can consist of a
>   collection of servers supported by several top of rack (TOR) and
>   Spine switches.  This collection of servers and switches are self
>   contained and may have their own control protocol for intra-POD
>   communication and orchestration.  However, EVPN is used as standard
>   way of inter-POD communication for both intra-DC and inter-DC.  A
>   subnet can span across multiple PODs and DCs.  EVPN provides robust
>   multi-tenant solution with extensive multi-homing capabilities to
>   stretch a subnet (VLAN) across multiple PODs and DCs.  There can be
>   many hosts/VMs(virtual machine) (several hundreds) attached to a
>   subnet that is stretched across several PODs and DCs.
> 
> Why is "Spine" capitalized? I'm also wondering whether another term might be
> appropriate here that doesn't imply a certain topology. Also,  "provides robust
> multi-tenant solution" should probably be "provides a robust multi-tenant
> solution".
> 
>   These hosts/VMs express their interests in multicast groups on a
>   given subnet/VLAN by sending IGMP Membership Reports (Joins) for
>   their interested multicast group(s).  Furthermore, an IGMP router
>   periodically sends membership queries to find out if there are hosts
>   on that subnet that are still interested in receiving multicast
>   traffic for that group.  The IGMP/MLD Proxy solution described in
>   this draft accomplishes three objectives:
> 
> I don't think you need "/VMs". They are a kind of host. There is also another
> use of "draft" in this paragraph.
> 
>   3.  Selective Multicast: to forward multicast traffic over EVPN
>       network such that it only gets forwarded to the PEs that have
>       interest in the multicast group(s), multicast traffic will not be
>       forwarded to the PEs that have no receivers attached to them for
>       that multicast group.  This draft shows how this objective may be
>       achieved when Ingress Replication is used to distribute the
>       multicast traffic among the PEs.  Procedures for supporting
>       selective multicast using P2MP tunnels can be found in
>       [I-D.ietf-bess-evpn-bum-procedure-updates]
> 
> The first sentence is very long and could probably be reworded to be less
> redundant. Also there is another instance of "draft".
> 
>   The first two objectives are achieved by using IGMP/MLD proxy on the
>   PE and the third objective is achieved by setting up a multicast
>   tunnel (e.g., ingress replication) only among the PEs that have
>   interest in that multicast group(s) based on the trigger from IGMP/
>   MLD proxy processes.  The proposed solutions for each of these
>   objectives are discussed in the following sections.
> 
> The first sentence can probably be split into two. Also, is "(e.g., ingress
> replication)" really an example? "multicast tunnel" probably suffices.
> 
>   o  Ethernet Segment (ES): When a customer site (device or network) is
>      connected to one or more PEs via a set of Ethernet links, then
>      that set of links is referred to as an 'Ethernet Segment'.
> 
>   o  Ethernet Segment Identifier (ESI): A unique non-zero identifier
>      that identifies an Ethernet Segment is called an 'Ethernet Segment
>      Identifier'.
> 
> Both of these terminology definitions can drop the end part where they quote
> the thing they're defining. It is implied by the colon.
> 
>   o  Ethernet Tag: An Ethernet tag identifies a particular broadcast
>      domain, e.g., a VLAN.  An EVPN instance consists of one or more
>      broadcast domains.
> 
> Same issue here more or less. You don't need to start out a sentence saying
> "Ethernet tag" again.
> 
> 4.1.  Proxy Reporting
> 
>   When IGMP protocol is used between hosts/VMs and their first hop EVPN
>   router (EVPN PE), Proxy-reporting is used by the EVPN PE to summarize
>   (when possible) reports received from downstream hosts and propagate
>   them in BGP to other PEs that are interested in the information.
>   This is done by terminating the IGMP Reports in the first hop PE, and
>   translating and exchanging the relevant information among EVPN BGP
>   speakers.  The information is again translated back to IGMP message
>   at the recipient EVPN speaker.  Thus it helps create an IGMP overlay
>   subnet using BGP.  In order to facilitate such an overlay, this
>   document also defines a new EVPN route type NLRI, the EVPN Selective
>   Multicast Ethernet Tag route, along with its procedures to help
>   exchange and register IGMP multicast groups Section 9.
> 
> Another usage of hosts/VMs.
> 
>   1.  When the first hop PE receives several IGMP Membership Reports
>       (Joins), belonging to the same IGMP version, from different
>       attached hosts/VMs for the same (*,G) or (S,G), it SHOULD send a
>       single BGP message corresponding to the very first IGMP
>       Membership Request (BGP update as soon as possible) for that
>       (*,G) or (S,G).  This is because BGP is a stateful protocol and
>       no further transmission of the same report is needed.  If the
>       IGMP Membership Request is for (*,G), then multicast group
>       address MUST be sent along with the corresponding version flag
>       (v2 or v3) set.  In case of IGMPv3, the exclude flag MUST also be
>       set to indicate that no source IP address must be excluded
>       (include all sources"*").  If the IGMP Join is for (S,G), then
>       besides setting multicast group address along with the version
>       flag v3, the source IP address and the IE flag MUST be set.  It
>       should be noted that when advertising the EVPN route for (S,G),
>       the only valid version flag is v3 (v2 flags MUST be set to zero).
> 
> Another hosts/VMs usage. Also missing a space after "include all sources".
> 
>   7.  Upon receiving EVPN SMET route(s) and before generating the
>       corresponding IGMP Membership Request(s), the PE checks to see
>       whether it has any CE multicast router for that BD on any of its
>       ES's . The PE provides such a check by listening for PIM Hello
>       messages on that AC (i.e, ES,BD).  If the PE does have the
>       router's ACs, then the generated IGMP Membership Request(s) are
>       sent to those ACs.  If it doesn't have any of the router's AC,
>       then no IGMP Membership Request(s) needs to be generated.  This
>       is because sending IGMP Membership Requests to other hosts can
>       result in unintentionally preventing a host from joining a
>       specific multicast group using IGMPv2 - i.e., if the PE does not
>       receive a join from the host it will not forward multicast data
>       to it.  Per [RFC4541] , when an IGMPv2 host receives a Membership
>       Report for a group address that it intends to join, the host will
>       suppress its own membership report for the same group, and if the
>       PE does not receive an IGMP Join from host it will not forward
>       multicast data to it.  In other words, an IGMPv2 Join MUST NOT be
>       sent on an AC that does not lead to a CE multicast router.  This
>       message suppression is a requirement for IGMPv2 hosts.  This is
>       not a problem for hosts running IGMPv3 because there is no
>       suppression of IGMP Membership Reports.
> Need a "the" before "host in "and if the PE does not receive an IGMP Join from
> host it will not forward multicast data to it."
> 
>   2.  When a PE receives an EVPN SMET route for a given (*,G), it
>       compares the received version flags from the route with its per-
>       PE stored version flags.  If the PE finds that a version flag
>       associated with the (*,G) for the remote PE is reset, then the PE
>       MUST generate IGMP Leave for that (*,G) toward its local
>       interface (if any) attached to the multicast router for that
>       multicast group.  It should be noted that the received EVPN route
>       SHOULD at least have one version flag set.  If all version flags
>       are reset, it is an error because the PE should have received an
>       EVPN route withdraw for the last version flag.  Error MUST be
>       considered as BGP error and the PE MUST apply the "treat-as-
>       withdraw" procedure of [RFC7606].
> 
> Consider rewording the latter part of this paragraph, note that "Error MUST be
> considered" is not quite grammatical.. Also if this is an error condition,
> should the "SHOULD at least have one version flag set" be a MUST?
> 
> 5.  Operation
> ...
>   o  only hosts/VMs
> 
>   o  mix of hosts/VMs and multicast source
> 
>   o  mix of hosts/VMs, multicast source, and multicast router
> 
> More hosts/VMs. I will stop mentioning this nit.
> 
> 6.  All-Active Multi-Homing
> 
>   Because the LAG flow hashing algorithm used by the CE is unknown at
>   the PE, in an All-Active redundancy mode it must be assumed that the
>   CE can send a given IGMP message to any one of the multi-homed PEs,
>   either DF or non-DF; i.e., different IGMP Membership Request messages
>   can arrive at different PEs in the redundancy group and furthermore
>   their corresponding Leave messages can arrive at PEs that are
>   different from the ones that received the Join messages.  Therefore,
>   all PEs attached to a given ES must coordinate IGMP Membership
>   Request and Leave Group (x,G) state, where x may be either '*' or a
>   particular source S, for each BD on that ES.  This allows the DF for
>   that (ES,BD) to correctly advertise or withdraw a Selective Multicast
>   Ethernet Tag (SMET) route for that (x,G) group in that BD when
>   needed.  All-Active multihoming PEs for a given ES MUST support IGMP
>   synchronization procedures described in this section if they need to
>   perform IGMP proxy for hosts connected to that ES.
> 
> "LAG" is undefined. Should "All-Active" really be capitalized?
> 
> 6.2.2.  Common Leave Group Synchronization
> ...
>   When the Maximum Response Timer expires a PE that has advertised an
>   IGMP Leave Synch route, withdraws it.  Any PE attached to the
>   multihomed ES, that started the Maximum Response Time and has no
>   local IGMP Membership Request (x,G) state and no installed IGMP Join
>   Synch routes, it removes IGMP Membership Request (x,G) state for that
>   (ES,BD).  If the DF no longer has IGMP Membership Request (x,G) state
>   for that BD on any ES for which it is DF, it withdraws its SMET route
>   for that (x,G) group in that BD.
> 
> The first sentence should be reworded, ending the sentence with ", withdraws
> it." reads awkwardly. The next sentence is also long and is confusing to read,
> I'm actually not quite sure what it is trying to convey.
> 
> 6.3.  Mass Withdraw of Multicast join Sync route in case of failure
> 
>   A PE which has received an IGMP Membership Request, would have synced
>   the IGMP Join by the procedure defined in section 6.1.  If a PE with
>   local join state goes down or the PE to CE link goes down, it would
>   lead to a mass withdraw of multicast routes.  Remote PEs (PEs where
>   these routes were remote IGMP Joins) SHOULD NOT remove the state
>   immediately; instead General Query SHOULD be generated to refresh the
>   states.  There are several ways to detect failure at a peer, e.g.
>   using IGP next hop tracking or ES route withdraw.
> 
> The first sentence has an extraneous comma after "IGMP Membership Request".
> 
> 9.1.  Selective Multicast Ethernet Tag Route
> ...
>   o  The least significant bit, bit 7 indicates support for IGMP
>      version 1.  Since IGMP V1 is being deprecated , sender MUST set it
>      as 0 for IGMP and receiver MUST ignore it.
> 
> Extraneous comma and space in the second sentence, and missing article before
> "sender".
> 
>   o  The second least significant bit, bit 6 indicates support for IGMP
>      version 2.
> 
>   o  The third least significant bit, bit 5 indicates support for IGMP
>      version 3.
> 
>   o  The fourth least significant bit, bit 4 indicates whether the
>      (S,G) information carried within the route-type is of an Include
>      Group type (bit value 0) or an Exclude Group type (bit value 1).
>      The Exclude Group type bit MUST be ignored if bit 5 is not set.
> 
> Is it typical to express this in terms of which least significant bit and the
> bit number? It reads a bit oddly. It's also only done in some of the
> descriptions so it is not consistent.
> 
>   o  If route is used for IPv6 (MLD) then bit 7 indicates support for
>      MLD version 1.  The second least significant bit, bit 6 indicates
>      support for MLD version 2.  Since there is no MLD version 3, in
>      case of IPv6 route third least significant bit MUST be 0.  In case
>      of IPv6 routes, the fourth least significant bit MUST be ignored
>      if bit 6 is not set.
> 
> Will there never be a MLD version 3? Also again missing an article before
> "third least significant bit", though I have similar commentary as above about
> how to refer to bits individually.
> 
> 9.1.1.  Constructing the Selective Multicast Ethernet Tag route
> ...
>   Reserved bits MUST be set to 0.  They can be defined in future by
> 
> Why are these a MUST whereas the earlier reserved bits in section 9.1 were
> SHOULDs?
> 
> 9.1.2.  Default Selective Multicast Route
> ...
>   Consider the EVPN network of Figure-2, where there is an EVPN
>   instance configured across the PEs.  Lets consider PE2 is connected
>   to multicast router R1 and there is a network running PIM ASM behind
>   R1.  If there are receivers behind the PIM ASM network, the PIM Join
>   would be forwarded to the PIM RP (Rendezvous Point).  If receivers
>   behind PIM ASM network are interested in a multicast flow originated
>   by multicast source S2 (behind PE1), it is necessary for PE2 to
>   receive multicast traffic.  In this case PE2 MUST originate a (*,*)
>   SMET route to receive all of the multicast traffic in the EVPN
>   domain.  To generate Wildcards (*,*) routes, prcedure from [RFC6625]
>   SHOULD be used.
> 
> "Lets" should be "Let's", also probably should be "consider that PE2 is
> connected". The comma in " If there are receivers behind the PIM ASM network, "
> is extraneous. The last sentence has a typo in "procedure" and is missing "the"
> before it.
> 
> 9.2.  Multicast Join Synch Route
> ...
> Similar commentary for this section about how bits are referred to, both by
> index and which "least significant bit" they are.
> 
>   o  Reserved bits MUST be set to 0.  They can be defined in future by
>      other document.
> Probably don't need the second sentence at all as that's implicit, also "future
> document" is more grammatical.
> 
> 9.2.1.  Constructing the Multicast Join Synch Route
> ...
> 
>   The Flags field indicates the version of IGMP protocol from which the
>   Membership Report was received.  It also indicates whether the
>   multicast group had INCLUDE or EXCLUDE bit set.
> 
> Earlier in the section "INCLUDE" and "EXCLUDE" were not capitalized. One should
> be picked.
> 
>   Reserved bits MUST be set to 0.  They can be defined in future by
>   other document.
> 
> Same commentary as before.
> 
> 
> 
> _______________________________________________
> Gen-art mailing list
> [e-mail]
> https://www.ietf.org/mailman/listinfo/gen-art