Reviewer: Brian Trammell Review result: Ready with Issues This document has been reviewed as part of the transport area review team's ongoing effort to review key IETF documents. These comments were written primarily for the transport area directors, but are copied to the document's authors and WG to allow them to address any issues raised and also to the IETF discussion list for information. When done at the time of IETF Last Call, the authors should consider this review as part of the last-call comments they receive. Please always CC tsv-art@xxxxxxxx if you reply to or forward this review. Caveat: I've reviewed this document primarily for transport-related issues. I have not made an effort to comprehensively review the architectural context in which it is defined; this review assumes that the operation of IGMP and Ethernet and IP multicast over EVPN is fundamentally sound. tl;dr: This document is acceptable from a transport standpoint: the algorithm used to reduce IGMP flooding seems safe within the limits of the specification's scope, recovery aspects are addressed, and though there is some fuzziness with respect to some implicit temporal limits I don't see anything that would impact the transport layer inordinately. However, from an editorial standpoint, it was not a joy to review, because it appears to assume a fairly deep familiarity with terminology and notation used in some context (which I assume is local to the BESS WG). Most of my issues are therefore editorial, and assume that the document is intended for general consumption. I'll start with the design and specification level questions first. General observation: the correctness of design of a system such as this tends to hang on the interactions among various timers. The only timer I see defined in this document is the Maximum Response Timer, which handles leave group timeouts. There are temporal interactions in other parts of the specification which remain implicit or undefined, though: for instance, Step 1 in section 4.1.1 specifies that IGMP messages should be grouped, but specifies no temporal limit to this process. I assume that the temporal scope is "the corresponding BGP session", but it would be nice to make this explicit in the document (especially if I'm wrong). The algorithm (and that in section 4.1.2) assume statefulness on the proxy, but since both BGP and IGMP are fundamentally stateful that's perfectly fine. Section 6.3 adequately addresses the failure recovery aspects of this, but it would also be nice to see some guidance as to the temporal limits of the recovery process (e.g. if the proxy immediately group queries on link down, it seems like a link flap could lead to a group query flood). Editorial nit: the Terminology section does not define most of its terms, instead providing only expansions. This does not serve the reader well, and reinforces the impression of RFCs as acronym soup. If I don't know what Ingress Replication means in a BESS context, then "IR: Ingress Replication" serves only to tell the reader that we're not discussing infrared radiation. Please either provide definitions (with references; e.g. IGMP has a set of supporting RFCs), or reference another EVPN document providing definitions (as it is perfectly acceptable for a multi-document protocol suite like EVPN to have an introductory document). Even with the list of acronym expansions, some acronyms are missing: I had to google NLRI, for instance, and I'm assuming ES means Ethernet Segment. Indeed, I was pleasantly surprised to see Maximum Response Timer not abbreviated to MRT. This document needs a thorough review of acronym usage, expansion, and definition before it can be published for a general audience. Editorial nit: A VM is a kind of host; the repeated use of "hosts/VMs" doesn't add much and makes the doc a bit more awkward to follow -- where "hosts" appears alone am I to assume that the passage does not apply to VMs? Editorial nit: The notation used in the algorithms in Sections 4.1.1 and 4.1.2 "(*, G), (S, G)" is neither introduced by reference nor defined, and I can't find a reference to it in IGMP documentation. -- last-call mailing list last-call@xxxxxxxx https://www.ietf.org/mailman/listinfo/last-call
