Re: [bess] Rtgdir last call review of draft-ietf-bess-mvpn-evpn-aggregation-label-08

2022-12-12 Thread Antoni Przygienda
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.  This will be explained in more detail later.)". This separate
   sentence in () is funny, make is a composite sentence 

Re: [bess] Rtgdir last call review of draft-ietf-bess-mvpn-evpn-aggregation-label-08

2022-12-12 Thread Jeffrey (Zhaohui) Zhang
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 

[bess] I-D Action: draft-ietf-bess-mvpn-evpn-aggregation-label-09.txt

2022-12-12 Thread internet-drafts


A New Internet-Draft is available from the on-line Internet-Drafts directories.
This draft is a work item of the BGP Enabled ServiceS WG of the IETF.

Title   : MVPN/EVPN Tunnel Aggregation with Common Labels
Authors : Zhaohui Zhang
  Eric Rosen
  Wen Lin
  Zhenbin Li
  IJsbrand Wijnands
  Filename: draft-ietf-bess-mvpn-evpn-aggregation-label-09.txt
  Pages   : 16
  Date: 2022-12-12

Abstract:
   The MVPN specifications allow a single Point-to-Multipoint (P2MP)
   tunnel to carry traffic of multiple VPNs.  The EVPN specifications
   allow a single P2MP tunnel to carry traffic of multiple Broadcast
   Domains (BDs).  These features require the ingress router of the P2MP
   tunnel to allocate an upstream-assigned MPLS label for each VPN or
   for each BD.  A packet sent on a P2MP tunnel then carries the label
   that is mapped to its VPN or BD (in some cases, a distinct upstream-
   assigned label is needed for each flow.)  Since each ingress router
   allocates labels independently, with no coordination among the
   ingress routers, the egress routers may need to keep track of a large
   number of labels.  The number of labels may need to be as large (or
   larger) than the product of the number of ingress routers times the
   number of VPNs or BDs.  However, the number of labels can be greatly
   reduced if the association between a label and a VPN or BD is made by
   provisioning, so that all ingress routers assign the same label to a
   particular VPN or BD.  New procedures are needed in order to take
   advantage of such provisioned labels.  These new procedures also
   apply to Multipoint-to-Multipoint (MP2MP) tunnels.  This document
   updates RFCs 6514, 7432 and 7582 by specifying the necessary
   procedures.



The IETF datatracker status page for this draft is:
https://datatracker.ietf.org/doc/draft-ietf-bess-mvpn-evpn-aggregation-label/

There is also an htmlized version available at:
https://datatracker.ietf.org/doc/html/draft-ietf-bess-mvpn-evpn-aggregation-label-09

A diff from the previous version is available at:
https://author-tools.ietf.org/iddiff?url2=draft-ietf-bess-mvpn-evpn-aggregation-label-09


Internet-Drafts are also available by rsync at rsync.ietf.org::internet-drafts


___
BESS mailing list
BESS@ietf.org
https://www.ietf.org/mailman/listinfo/bess