Re: [bess] Rtgdir last call review of draft-ietf-bess-mvpn-evpn-aggregation-label-08
Thanks. All adressed AFAIS * Tony From: Jeffrey (Zhaohui) Zhang Date: Monday, 12 December 2022 at 17:50 To: Antoni Przygienda , rtg-...@ietf.org Cc: bess@ietf.org , draft-ietf-bess-mvpn-evpn-aggregation-label@ietf.org , last-c...@ietf.org Subject: RE: Rtgdir last call review of draft-ietf-bess-mvpn-evpn-aggregation-label-08 Hi Tony, Thanks for your thorough review and comments. I have posted the -09 version that I believe/hope have address your comments/concerns. https://datatracker.ietf.org/doc/html/draft-ietf-bess-mvpn-evpn-aggregation-label-09 Please see zzh> below for some details. Juniper Business Use Only -Original Message- From: Tony Przygienda via Datatracker Sent: Tuesday, November 1, 2022 4:02 PM To: rtg-...@ietf.org Cc: bess@ietf.org; draft-ietf-bess-mvpn-evpn-aggregation-label@ietf.org; last-c...@ietf.org Subject: Rtgdir last call review of draft-ietf-bess-mvpn-evpn-aggregation-label-08 [External Email. Be cautious of content] Reviewer: Tony Przygienda Review result: Has Issues As first, technically sound except point 18. Rest of the commentes provided are all for easier readability/clarity for a reader that may not be super instrinsically familiar with the world of overlay multicast tunnels underlying VPN technologies first. I am quite versant in it but even then, the complexity of the field made me stumble on some assertions given without explanation. Then, good amount of omitted words etc necessary to read as coherent English sentences. Ultimately, some of the transitions in terms of causality chains do not connect and can leave the reader stranded which I'm pointing out in specific comments. Numbered: 1. document only distincts between p2mp and mp2mp rather than using the PMSI terminology with inclusisve/selective [RFC6513]. the S/I is not relevant but it would help readability if that would be explained. Especially since the S/I starts to be introduced in 2.2.2.1 suddenly. Zzh> The difference between P2MP and MP2MP is actually not important and not the focus of the document. The document starts with P2MP because it is more common; it does talk about MP2MP's applicability with some specifics afterwards. Zzh> Use of per-VPN/BD/ES DCB labels does not work for tunnel segmentation, and entire section 2.2.2 is about how we mitigate that. Section 2.2.2.1 explains the need for per-PMSI labels using selective tunnels example, and section 2.2.2.2 extends it to inclusive tunnels. Zzh> For an average reader, selective/inclusive tunnels are easier to understand than S/I-PMSI terms so the sections are based on "tunnels" instead of PMSIs, but now I have added additional glossaries and text to tie the two together. 2. Terminology -- provide references for BD/BUM/PMSI/IMET/PTA in terms of RFCs defining them properly. Currently the section is just acronym expansion really. -- provide definition of "aggregate tunnel" in the terminology section rather than later in the document for consistency -- add definitons for "context space", "upstream assigned label" and "ESI Label" here as well rather than later in the document. This may lead to more conscise and readable introduction section -- add definition for DCB -- add def for SRGB with according SRGB --- I suggest to add upstream assigned (and downstream) labels to definitions as well since it's so central to the document. Acronym expansion + RFC ref is fine AFAIS but at least the reader can peddle back and know in one shot where all the stuff comes from or read things upfront to have an inkling. The drip-drip technique uesed in the document is ok'ish since it makes an illusion of gradual introduction into the solution space on first read but makes it hard to find things later, have in one easy shot a quick recall "what was that all about". IME good glossary goes a very long way when attempting a 2nd read or trying to do a fast page-in later. -- given 2.2.2.1 I suggest to add PMSI + S/I + PTA defintions + IMET + RBR. And maybe minimum definition (or where to find the terminology precisely) for the whole (C-*,C-*) machinery involved in context lookups you pull out in 2.2.2.3 Zzh> I added more terms. Zzh> I assume most people now just read the electronic copy instead of paper copy, so the drip-drip technique should not only work well for gradual introduction but also not make it hard to find things later? 3. "but each PE would know not to allocate labels from that block for other purposes" is difficult to read. Rewrite from negative. Zzh> I removed it entirely. Sometimes less is more 4. "1000 is for VPN 0, 1001 for VPN 1 ...". Write it clearer, maybe "label 1000 maps to VPN 0, 1001 to VPN1 and so forth" Zzh> Fixed as suggested. 5. " network. (Though if tunnel segmentation [RFC6514] is used, each segmentation region could have its own DCB.
Re: [bess] Rtgdir last call review of draft-ietf-bess-mvpn-evpn-aggregation-label-08
Hi Tony, Thanks for your thorough review and comments. I have posted the -09 version that I believe/hope have address your comments/concerns. https://datatracker.ietf.org/doc/html/draft-ietf-bess-mvpn-evpn-aggregation-label-09 Please see zzh> below for some details. Juniper Business Use Only -Original Message- From: Tony Przygienda via Datatracker Sent: Tuesday, November 1, 2022 4:02 PM To: rtg-...@ietf.org Cc: bess@ietf.org; draft-ietf-bess-mvpn-evpn-aggregation-label@ietf.org; last-c...@ietf.org Subject: Rtgdir last call review of draft-ietf-bess-mvpn-evpn-aggregation-label-08 [External Email. Be cautious of content] Reviewer: Tony Przygienda Review result: Has Issues As first, technically sound except point 18. Rest of the commentes provided are all for easier readability/clarity for a reader that may not be super instrinsically familiar with the world of overlay multicast tunnels underlying VPN technologies first. I am quite versant in it but even then, the complexity of the field made me stumble on some assertions given without explanation. Then, good amount of omitted words etc necessary to read as coherent English sentences. Ultimately, some of the transitions in terms of causality chains do not connect and can leave the reader stranded which I'm pointing out in specific comments. Numbered: 1. document only distincts between p2mp and mp2mp rather than using the PMSI terminology with inclusisve/selective [RFC6513]. the S/I is not relevant but it would help readability if that would be explained. Especially since the S/I starts to be introduced in 2.2.2.1 suddenly. Zzh> The difference between P2MP and MP2MP is actually not important and not the focus of the document. The document starts with P2MP because it is more common; it does talk about MP2MP's applicability with some specifics afterwards. Zzh> Use of per-VPN/BD/ES DCB labels does not work for tunnel segmentation, and entire section 2.2.2 is about how we mitigate that. Section 2.2.2.1 explains the need for per-PMSI labels using selective tunnels example, and section 2.2.2.2 extends it to inclusive tunnels. Zzh> For an average reader, selective/inclusive tunnels are easier to understand than S/I-PMSI terms so the sections are based on "tunnels" instead of PMSIs, but now I have added additional glossaries and text to tie the two together. 2. Terminology -- provide references for BD/BUM/PMSI/IMET/PTA in terms of RFCs defining them properly. Currently the section is just acronym expansion really. -- provide definition of "aggregate tunnel" in the terminology section rather than later in the document for consistency -- add definitons for "context space", "upstream assigned label" and "ESI Label" here as well rather than later in the document. This may lead to more conscise and readable introduction section -- add definition for DCB -- add def for SRGB with according SRGB --- I suggest to add upstream assigned (and downstream) labels to definitions as well since it's so central to the document. Acronym expansion + RFC ref is fine AFAIS but at least the reader can peddle back and know in one shot where all the stuff comes from or read things upfront to have an inkling. The drip-drip technique uesed in the document is ok'ish since it makes an illusion of gradual introduction into the solution space on first read but makes it hard to find things later, have in one easy shot a quick recall "what was that all about". IME good glossary goes a very long way when attempting a 2nd read or trying to do a fast page-in later. -- given 2.2.2.1 I suggest to add PMSI + S/I + PTA defintions + IMET + RBR. And maybe minimum definition (or where to find the terminology precisely) for the whole (C-*,C-*) machinery involved in context lookups you pull out in 2.2.2.3 Zzh> I added more terms. Zzh> I assume most people now just read the electronic copy instead of paper copy, so the drip-drip technique should not only work well for gradual introduction but also not make it hard to find things later? 3. "but each PE would know not to allocate labels from that block for other purposes" is difficult to read. Rewrite from negative. Zzh> I removed it entirely. Sometimes less is more 4. "1000 is for VPN 0, 1001 for VPN 1 ...". Write it clearer, maybe "label 1000 maps to VPN 0, 1001 to VPN1 and so forth" Zzh> Fixed as suggested. 5. " network. (Though if tunnel segmentation [RFC6514] is used, each segmentation region could have its own DCB. This will be explained in more detail later.)". This separate sentence in () is funny, make is a composite sentence or part of previous sentence in () zzh> I removed it. It's more accurate w/o it. 6. " However, that is not necessarily as the labels used by PEs for the purposes defined in this document will only rise to the top of the label stack when traffic arrives the PEs. " does not parse as English. this is not necessarily _true_ ? arrives
