John Scudder has entered the following ballot position for draft-ietf-bess-evpn-virtual-eth-segment-11: Discuss
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/about/groups/iesg/statements/handling-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-virtual-eth-segment/ ---------------------------------------------------------------------- DISCUSS: ---------------------------------------------------------------------- # John Scudder, RTG AD, comments for draft-ietf-bess-evpn-virtual-eth-segment-11 CC @jgscudder I found this document hard to review, for various reasons, so I don't really consider this a complete review. In particular, I can't say with confidence that it would be possible to build an interoperable implementation using this spec. Despite that I have a number of comments I hope may be useful. Although I haven't currently chosen to make them a DISCUSS, I hope you will consider them, especially the ones that relate to elements of procedure that are unclear. ## DISCUSS ### Section 9 I agree with Paul Wouters that Section 9 is unusual, to say the least. I am going to dial up Paul's suggestion that the section be removed, to a strong suggestion, and a request that if it's not removed, we have a conversation about why it is needed and what value it adds to the finished specification. I would be curious to know why this section was added, to begin with, it's unique in my experience. But it's optional to provide the background -- the necessary part is to either remove it or explain why it's needed. ---------------------------------------------------------------------- COMMENT: ---------------------------------------------------------------------- ## COMMENTS ### Abstract + Section 1 Please expand "EVC" on first use in Section 1, and please simply write it out in words in the abstract (you could also include the "EVC" initialism if you think it's valuable to some readers; from my point of view it's not needed in the abstract though). ### Section 1.1 "ENNIs are commonly used to reach off-network / out-of-franchise customer sites via independent Ethernet access networks or third- party Ethernet Access Providers (EAP) (see Figure 1)." As far as I can tell, Figure 1 doesn't elucidate this sentence. If it's supposed to, I think the figure needs more labeling. Nothing in the figure is labeled as an "independent Ethernet access network" or "EAP" and no context is provided to suggest what's considered off-network vs on-network (I see there's a box called "Carrier Ethernet Network" and another called "IP/MPLS Core Network" but those are technologies, not owners). Given that the ENNI presumably is the demarcation between the SP and the third party, and from other context, I'm guessing the "Carrier Ethernet Network" is considered the third party in this case, and the IP/MPLS Core Network the SP. Probably it would be enough to add those notations to the figure. ### Section 1.1 and throughout This document has specific numbers at various places, for example, in Section 1.1 we have "ENNIs can aggregate traffic from hundreds to thousands of vESes". Surely the exact number (even order-of-magnitude) is a moving target (unless it's bounded by some protocol constant, which it's not AFAICT). Probably the numbers should either be made abstract, or qualified by something like "at time of writing". ### Section 1.1, off-networks "As a result, ENNIs and their associated EVCs are a key element of SP off-networks" What is an "SP off-network"? This isn't a well-known term of art. ### Section 1.1 "Figure 1 depicts two PE devices (PE1 and PE2) each with an ENNI where a number of vESes are aggregated on - each of which through its associated EVC." AFAICT there is a many:one relationship between EVCs and vESes. Is that right? If so I think it's important to establish that context in this section, and in that case, the sentence needs a rewrite, as in, NEW: Figure 1 depicts two PE devices (PE1 and PE2) each with an ENNI that aggregates a number of vESes. Each vES is associated with an EVC, or in the case of the multi-homed vES connected to CE3, two EVCs. (I've also fixed some unrelated grammatical errors.) ### Section 1.2 ES or vES? Would it be more correct to rewrite "PW3 and PW5 would belong to ES-1, whereas PW4 and PW6 would be associated to ES-2" adding the "v" to "ES", as "PW3 and PW5 would belong to vES-1, whereas PW4 and PW6 would be associated to vES-2"? It's hard enough to follow the abbreviations without casually using "ES" to mean different things in different places. :-( I'm relying on the definition of "ES" from the Abstract: "Ethernet Segment (ES), itself defined as a set of physical links". ### Section 1.2, EVI Please expand EVI (or put it in your glossary but since you don't use it repeatedly, expanding it on use is better). ### Section 2 Shouldn't CE and PE be defined as "Customer Edge *Router*" and "Provider Edge *Router*"? ### Section 2 The following terms are used but never defined, please add definitions: A-D, Ethernet A-D BUM (although since this is used in only one place you could also just expand it on use) The following definitions are never referenced and should be removed: CFM LACP The following definitions are referenced only once and should probably just be expanded where used instead of defining them here: BEB DHD AA The following definitions are referenced only a few (2-3) times and should probably just be expanded where used: DHN MHD MHN SH SA ### Section 3 There are a few actual requirements in this section, and a lot of things that I would characterize as aspirations, design principles, design goals, or simply elements of procedure. Ideally, one would cut the requirements down to only the things that are actually needed (if any), redistribute the elements of procedure to their respective sections, and maybe write a more casual section on design principles, but I realize that's not too likely. There is one subsection that seems more problematic than the others, though, called out next. ### Section 3.2 I don't see how it is that these can be called "requirements", complete with RFC 2119 keywords, since they're couched in terms so imprecise it's not possible to say if an implementation is compliant or not. That is, "very large number" and "large number" are completely subjective. The examples given ("e.g. thousands") provide some context but are explicitly non-normative, and that's as it should be, since (as I mention in my comment on Section 1.1) these numbers are a moving target. I don't see that this section is serving any useful purpose and suggest removing it. If you think that these really are "requirements" maybe you should think harder about how to write them down so that a third party could impartially evaluate whether they are met or not. I do see that perhaps part of the goal here is to say "our design goal was to be able to support a lot of single-homed vESes, and we were ok with a lower scale of single-active and all-active vESes". That seems like something that could be mentioned in the introduction, or a subsection called something like "qualitative design goals". The line items about exactly how different service mixes are handled by different ports seem completely unnecessary; as far as I can tell the elements of procedure don't relate to these "requirements" at all. In short, it seems to me this section can be removed. If you think it's important to the finished RFC, please explain why? ### Section 3.3, misuse of 2119 keyword "For example, in Figure 1, PE1 MUST support". By definition a 2119 keyword doesn't make sense in an example. I guess s/MUST/has to/. ### Section 3.4 "However, there is no restriction on an EVC to carry more than one VLAN." and "there is no restriction on the PW to carry more than one VLAN if the PW is of type Raw mode." I guess you mean "However, there is no restriction preventing an EVC from carrying more than one VLAN." and "there is no restriction preventing the PW from carrying more than one VLAN if the PW is of type Raw mode"? If my wording is incorrect, please explain what you did mean? If it's correct, I suggest adopting it or something like it. ### Section 3.7, misuse of 2119 keyword "(R7d) Failure and failure recovery of an EVC for a Single-Active vES MAY only impact C-MACs associated with MHD/MHNs for that service instance. In other words, MAC flushing SHOULD be limited to single service instance (I-SID in the case of PBB-EVPN) and only C-MACs for Single-Active MHD/MHNs." That first MAY seems misused. Did you mean it in the normal English sense, i.e. if I say "foo may only bar" I mean "foo must not do anything other than bar"? Because that's not how the 2119 MAY works at all. Probably the fix is just s/MAY/may/... although the "in other words" sentence tells us something different again, so I guess then the fix would be s/MAY/SHOULD/, as in NEW: (R7d) Failure and failure recovery of an EVC for a Single-Active vES SHOULD only impact C-MACs associated with MHD/MHNs for that service instance. In other words, MAC flushing SHOULD be limited to single service instance (I-SID in the case of PBB-EVPN) and only C-MACs for Single-Active MHD/MHNs. I guess the SHOULD means that in the opinion of the authors, "there may exist valid reasons in particular circumstances to ignore" this requirement. Please elaborate on when it would be OK to do MAC flushing across multiple service instances in the context of this item? (If you think it is never OK then I guess this is actually MUST.) ### Section 4 Figure 3 is never referred to. Seems like it should be deleted. Or, if you keep it, it seems like it would be good to connect it to the document somehow. (If you delete the figure, you can also delete the definition of BEB since this is the only use.) ### Section 4.1 "For the sake of clarity and completeness, the default DF election procedure of [RFC7432] is repeated below:" Did you mean "... is repeated below, **with necessary changes**:"? The procedure you show is of course NOT a direct quotation of the default DF election procedure of RFC 7432, and it's not even that procedure with s/ES/vES/g. For example, RFC 7432: 1. When a PE discovers the ESI of the attached Ethernet segment, it advertises an Ethernet Segment route with the associated ES-Import extended community attribute. This draft: 1. When a PE discovers the vESI or is configured with the vESI associated with its attached vES, it advertises an Ethernet Segment route with the associated ES-Import extended community attribute. The changes I see are: - s/ESI/vESI/ - Addition of "or is configured with the vESI associated with its attached vES" ### Section 4.1 "In case of a Single-Active, when a service moves from one PE in the Redundancy Group to another PE as a result of DF re-election, the PE, which ends up being the elected DF for the service, SHOULD trigger a MAC address flush notification towards the associated vES." What are examples of a circumstance where it would be OK for the PE to *not* trigger a MAC address flush notification? Also, in "This can be done, for e.g. using IEEE 802.1ak MVRP 'new' declaration", when you write "for e.g." do you actually mean "for example" or "e.g."? Please rewrite as one, or the other, but not the mixed "for e.g.". It seems like 802.1ak needs a reference. ### Section 4.1 "For LSP-based and PW-based vES, the non-DF PE SHOULD signal PW-status 'standby' to the Aggregation PE (e.g., AG PE in Figure 2)," I don't see an "AG PE" in Figure 2. I see two AG boxes (AG1 and AG2) which the figure carefully does not refer to as PEs, and I see two boxes call PE1 and PE2. Please refer to the actual labels from the figure for clarity (so for example if you meant AG1 and AG2, you'd write "e.g., AG1 and AG2 in Figure 2"). ### Section 4.1, EVI/BD Please expand "EVI/BD" (or put it in your glossary but since you don't use it repeatedly, expanding it on use is better). ### Section 4.2.1, misused RFC 2119 keyword "The ESI label extended community MAY not be added to Grouping Ethernet A-D per ES and SHOULD be ignored on receiving PE." You don't mean "MAY not be", do you, you mean "MUST NOT be". Right? Also, please give an example of when it is OK for the receiving PE to process the ESI label extended community instead of ignoring it. ### Section 4.2.1, ES vs vES Since you've made a special point of introducing "vES" as something different from "ES", I am trying to parse out whether there's any special meaning to your uses of "ES" in this section. I think "ES route" just refers to a specific kind of route, which can carry a regular ES or a vES -- I wonder if it's worth saying anything about this? But then we have The ESI label extended community (Section 7.5 of [RFC7432]) is not relevant to Grouping Ethernet A-D per ES. and The ESI label extended community MAY not be added to Grouping Ethernet A-D per ES and SHOULD be ignored on receiving PE. and This Grouping Ethernet A-D per ES is advertised with a list of Route Targets corresponding to the impacted service instances. These all just say "per ES" and *not* "per ES route" or "per vES". Is there significance to that? Should it say something different? ### Section 4.2.1 Previous comment aside, I don't understand this: "This Grouping Ethernet A-D per ES is advertised with a list of Route Targets corresponding to the impacted service instances. If the number of Route Targets is more than can fit into a single attribute, then a set of Grouping Ethernet A-D per ES routes are advertised." I suppose by "single attribute" you mean "single BGP path attribute"? But a BGP path attribute is essentially unlimited in length -- the path attribute length field is up to two bytes wide, and the total BGP update message length is fixed at 4096 bytes, or 64k if extended messages are in use. So a single path attribute can occupy the entire update message if needed. So, I don't understand the "more than can fit" problem scenario. "More than can fit" aside, I also find the paragraph terribly hard to make sense of at all. :-( ### Section 4.2.2 "For PBB-EVPN, especially where there here are large number of service instances (i.e., I-SIDs) associated with each EVC the PE MAY color each vES B-MAC route with an attribute representing their association to a physical port (e.g. ENNI)." I think "where there here are" should just be "where there are" (remove "here"). In reading it I mentally deleted the "here". If that's NOT the correct fix, please explain. ### Section 5.3 "5. When this message is received, the remote PE MAY detect the special vES mass‑withdraw message by identifying the Grouping Ethernet A-D per ES route. The remote PEs MAY then access the list created in (1) of the vES's for the specified color, and initiate locally MAC address invalidating procedures for each of the vES's in the list." Your use of MAY (twice) in the above implies that it's totally fine for the remote PE to *not* detect the special vES mass-withdraw, and/or to *not* then initiate MAC address invalidating procedures. Is it true that this is completely fine, and that everything will still work (including performing in compliance with the "requirements" in Section 3) if the remote PE doesn't do step 5? ### Section 5.4 Similar question to the previous, with respect to step 4. ### Section 5.5 "This section assumes that the MAC flushing mechanism described in Section 5.4, bullet (1) is used" There are no bullet items in Section 5.4. There are two numbered lists, so two things that could be considered item (1) in Section 5.4, so this reference isn't unambiguously resolvable. Please clarify/correct. ### Section 5.5 Similar to the previous two sections, everything in the procedures is MAY, which means the procedures are completely optional and in fact, that it would be technically permissible to implement (for example) only points 2 and 5 and not points 1, 3, 4, and 6. Is this intended? What is the result if some or all of the MAY are disregarded by an implementation? ### Section 5.5 Figure 5 is never referred to. Seems like it should be deleted. Or, if you keep it, it seems like it would be good to connect it to the document somehow. ## Notes This review is in the ["IETF Comments" Markdown format][ICMF], You can use the [`ietf-comments` tool][ICT] to automatically convert this review into individual GitHub issues. [ICMF]: https://github.com/mnot/ietf-comments/blob/main/format.md [ICT]: https://github.com/mnot/ietf-comments _______________________________________________ BESS mailing list BESS@ietf.org https://www.ietf.org/mailman/listinfo/bess
