Subject: [bess] AD Review of draft-ietf-bess-mvpn-bidir-ingress-replication-02

From: "Alvaro Retana (aretana)" <[email-masked]>
Date: Thu, 24 Sep 2015 15:33:17 +0000

Hi!

I just finished reading this document.  In general I think that there are some things that could be done to improve the readability/clarity.

While the authors address the items below, I’ll start he IETF Last Call and put the document on the IESG Agenda (probably for Oct/15).  Please post an update before Oct/8.

Thanks!

Alvaro.

Major:

  1.  I-D.ietf-bess-ir and I-D.ietf-bess-mvpn-extranet should be Normative References.

Minor:

  1.  Update to rfc6514:  Please include a short paragraph (or a couple of sentences) explaining how this document updates rfc6514 (maybe in the Introduction).
  2.  Even though the text says that terminology is somewhere else, please expand the acronyms on first use.  Also, some of the text ("C-G-BIDIR refers to a C-G where G is a Bidir-PIM group”, for example) may not be accesible to the average reader, even if extensions are in place — you already included some background in the Introduction  and I’m not sure we want to rehash everything again..just something to think about.  You might want to refer to the RFC Editor’s list of well known abbreviations: https://www.rfc-editor.org/rfc-style-guide/abbrev.expansion.txt
  3.  I think there are some references missing, for example: "Bidir-PIM, or via MP2MP mLDP”
  4.  Section 3.1. (Control State):
     *   What do you mean by "just like how any other S-PMSI A-D routes are triggered”?  I’m thinking that at least a reference is needed.  Later is the same paragraph you include an example as well.  Please clarify what you’re referring to.
     *   Do we really need this text 3 times in the section?  "The label may be shared with other P-tunnels, subject to the anti-ambiguity rules for extranet [I-D.ietf-bess-mvpn-extranet].  For example, the (C-*,C-*-BIDIR) and (C-*,C-G-BIDIR) S-PMSI A-D routes originated by a given PE can optionally share a label."
     *   The paragraphs that start with "The encoding of the Leaf A-D route…” are also duplicates.
  5.  Section 4. (Security Considerations)  Are there really no security considerations?
     *   Section 3.1. (Control State)   Says that: "To speed up convergence…PEy MAY advertise a Leaf A-D route even if does not choose PEx as its Upstream PE…With that, it will receive traffic from all PEs, but some will arrive with the label corresponding to its choice of Upstream PE while some will arrive with a different label, and the traffic in the latter case will be discarded.”  I’m assuming that all the traffic (specially the discarded one) belongs to the same VPN, so there’s no danger of leaking information, right?  It might be worth including something in the Security Consideration so that it’s easier for the readers (Security Directorate, for example) to grasp the context.

Nits:

  1.  Abstract  s/These specifications/This specification
  2.  s/wrt/with regards to
  3.  Introduction: s/Ingress Replication,this/Ingress Replication this

---

From: "Jeffrey (Zhaohui) Zhang" <[email-masked]>
Date: Fri, 25 Sep 2015 18:11:04 +0000

Alvaro,

Thanks you for the detailed review and suggestions. Please see zzh> below for some responses, and see attached diff for a new revision that I plan to upload.

From: Alvaro Retana (aretana) [mailto:[email-masked]]
Sent: Thursday, September 24, 2015 11:33 AM
To: [email-masked]
Cc: [email-masked]; EXT - [email-masked] <[email-masked]>; [email-masked]
Subject: AD Review of draft-ietf-bess-mvpn-bidir-ingress-replication-02

Hi!

While the authors address the items below, I'll start he IETF Last Call and put the document on the IESG Agenda (probably for Oct/15).  Please post an update before Oct/8.

Thanks!

Alvaro.

Major:

Zzh> Done.
Minor:

  1.  Update to rfc6514:  Please include a short paragraph (or a couple of sentences) explaining how this document updates rfc6514 (maybe in the Introduction).
Zzh> Done - please see the 2nd diff in the introduction.

  1.  Even though the text says that terminology is somewhere else, please expand the acronyms on first use.  Also, some of the text ("C-G-BIDIR refers to a C-G where G is a Bidir-PIM group", for example) may not be accesible to the average reader, even if extensions are in place - you already included some background in the Introduction  and I'm not sure we want to rehash everything again..just something to think about.  You might want to refer to the RFC Editor's list of well known abbreviations: https://www.rfc-editor.org/rfc-style-guide/abbrev.expansion.txt
Zzh> I removed the "... refers to ..." text. I had thought that term was not in RFC 7582 (hence the text) but it actually is already covered in 7582.

  1.  I think there are some references missing, for example: "Bidir-PIM, or via MP2MP mLDP"
Zzh> I added those as informational.

  1.  Section 3.1. (Control State):

     *   What do you mean by "just like how any other S-PMSI A-D routes are triggered"?  I'm thinking that at least a reference is needed.  Later is the same paragraph you include an example as well.  Please clarify what you're referring to.
Zzh> I changed it to "if selective distribution trees are needed for those C-G-BIDIR groups".

     *   Do we really need this text 3 times in the section?  "The label may be shared with other P-tunnels, subject to the anti-ambiguity rules for extranet [I-D.ietf-bess-mvpn-extranet].  For example, the (C-*,C-*-BIDIR) and (C-*,C-G-BIDIR) S-PMSI A-D routes originated by a given PE can optionally share a label."
Zzh> They are for three types of routes: S-PMSI route, Leaf A-D route for (C-*,C-*-BIDIR), and leaf A-D route for (C-*,C-G-BIDIR). I did remove the second "for example", and reduced the paragraph for the (C-*,C-G-BIDIR) Leaf A-D route encoding to "The encoding of the Leaf A-D route is the similar to the (C-*,C-*-BIDIR) case".

     *   The paragraphs that start with "The encoding of the Leaf A-D route..." are also duplicates.
Zzh> See above.

  1.  Section 4. (Security Considerations)  Are there really no security considerations?

     *   Section 3.1. (Control State)   Says that: "To speed up convergence...PEy MAY advertise a Leaf A-D route even if does not choose PEx as its Upstream PE...With that, it will receive traffic from all PEs, but some will arrive with the label corresponding to its choice of Upstream PE while some will arrive with a different label, and the traffic in the latter case will be discarded."  I'm assuming that all the traffic (specially the discarded one) belongs to the same VPN, so there's no danger of leaking information, right?  It might be worth including something in the Security Consideration so that it's easier for the readers (Security Directorate, for example) to grasp the context.
Zzh> There is indeed no new issues. The quoted text refers to the possible arrival of duplication for the same flow that the receiving PEs need to receive, and they will be discarded anyway. There is no deliver of duplication to CEs, and certainly there is no leaking. I am not sure if that needs to be called out.
Zzh> The nits below are also addressed.
Zzh> Thanks!
Zzh> Jeffrey
Nits:

---

From: "Alvaro Retana (aretana)" <[email-masked]>
Date: Fri, 25 Sep 2015 18:54:51 +0000

On 9/25/15, 2:11 PM, "Jeffrey (Zhaohui) Zhang" <[email-masked]<mailto:[email-masked]>> wrote:

Jeffrey:

Hi!

. . .
Major:

I-D.ietf-bess-ir wasn’t moved.

. . .

You don’t have to..but saying that there are no issues usually raises a flag for more thorough review by the SecDir/ADs.  You can leave it as is and address any issues that may come up later.

Thanks!

Alvaro.

---

From: <[email-masked]>
Date: Mon, 28 Sep 2015 09:28:47 +0200

Hi Alvaro,

2015-09-24, Alvaro Retana (aretana):
> Also, some of the text [...] may not be accesible to the average 
> reader,...

As a side note, while what you wrote is true, I would say that it will 
be hard to fix.
Most mVPN-related specs are deemed to remain fairly inaccessible to 
somebody not already familiar with RFC6513/6514, which does not fit any 
conceivable definition of an average reader... ;-)

-Thomas

_________________________________________________________________________________________________________________________

Ce message et ses pieces jointes peuvent contenir des informations confidentielles ou privilegiees et ne doivent donc
pas etre diffuses, exploites ou copies sans autorisation. Si vous avez recu ce message par erreur, veuillez le signaler
a l'expediteur et le detruire ainsi que les pieces jointes. Les messages electroniques etant susceptibles d'alteration,
Orange decline toute responsabilite si ce message a ete altere, deforme ou falsifie. Merci.

This message and its attachments may contain confidential or privileged information that may be protected by law;
they should not be distributed, used or copied without authorisation.
If you have received this email in error, please notify the sender and delete this message and its attachments.
As emails may be altered, Orange is not liable for messages that have been modified, changed or falsified.
Thank you.

---

From: <[email-masked]>
Date: Mon, 28 Sep 2015 09:31:33 +0200

Jeffrey, all,

(below)

2015-09-25, Jeffrey (Zhaohui) Zhang:
>
> *From:*Alvaro Retana (aretana) [mailto:[email-masked]]
>
>       * Section 3.1. (Control State)   Says that: "To speed up
>         convergence…PEy MAY advertise a Leaf A-D route even if does
>         not choose PEx as its Upstream PE…With that, it will receive
>         traffic from all PEs, but some will arrive with the label
>         corresponding to its choice of Upstream PE while some will
>         arrive with a different label, and the traffic in the latter
>         case will be discarded.”  I’m assuming that all the traffic
>         (specially the discarded one) belongs to the same VPN, so
>         there’s no danger of leaking information, right?  It might be
>         worth including something in the Security Consideration so
>         that it’s easier for the readers (Security Directorate, for
>         example) to grasp the context.
>
> Zzh> There is indeed no new issues. The quoted text refers to the 
> possible arrival of duplication for the same flow that the receiving 
> PEs need to receive, and they will be discarded anyway. There is no 
> deliver of duplication to CEs, and certainly there is no leaking. I am 
> not sure if that needs to be called out.
>
>

I agree on the analysis.
Refering to both RFC6513 and RFC6514 (instead of RFC6514 alone) is the 
only improvement I can think of.

Best,

-Thomas

---

From: <[email-masked]>
Date: Mon, 28 Sep 2015 09:33:37 +0200

Hi Alvaro,

[resending, because I had mailer daemon from 
junipernetworks.onmicrosoft.com]

-Thomas

---

From: <[email-masked]>
Date: Mon, 28 Sep 2015 09:34:24 +0200

Jeffrey, all,

[resending this one too...]

(below)

I agree on the analysis.

Best,

-Thomas

---

From: "Jeffrey (Zhaohui) Zhang" <[email-masked]>
Date: Mon, 28 Sep 2015 14:51:48 +0000

Alvaro,
> I-D.ietf-bess-ir and I-D.ietf-bess-mvpn-extranet should be Normative References.
I thought about this further, and would like to keep them both as informational for the following reasons.
The extranet draft is referred to in the draft as following:
   ... The label may be shared
   with other P-tunnels, subject to the anti-ambiguity rules for
   extranet [I-D.ietf-bess-mvpn-extranet<https://tools.ietf.org/html/draft-ietf-bess-mvpn-bidir-ingress-replication-02#ref-I-D.ietf-bess-mvpn-extranet>].
Both extranet and label sharing are optional, not required for implementing the procedures in this draft.

As for draft-ietf-bess-ir: RFC 6514 specifies the use of IR P-tunnels, though there are some problems with RFC 6514's specification of IR P-tunnels, which are addressed in detail in draft-ietf-bess-ir.

Draft-ietf-bess-mvpn-bidir-ingress-replication explains how to support a VPN customer's use of BIDIR-PIM when the service provider uses IR P-tunnels, but it doesn't really depend on those details specified in draft-ietf-bess-ir. Thus draft-ietf-bess-ir should not be a normative reference for it.

Thanks!

Jeffrey

---

From: <[email-masked]>
Date: Mon, 28 Sep 2015 17:39:17 +0200

Hi Jeffrey, Alvaroo,

2015-09-28, Jeffrey (Zhaohui) Zhang:
>    … The label may be shared
>
>    extranet [I-D.ietf-bess-mvpn-extranet 
> <https://tools.ietf.org/html/draft-ietf-bess-mvpn-bidir-ingress-replication-02#ref-I-D.ietf-bess-mvpn-extranet>].
>
> Both extranet and label sharing are optional, not required for 
> implementing the procedures in this draft.
>

I agree with that.
To make it more obvious that this is an informative comment, I think you 
could rewrite as: "These specifications do not prevent sharing of labels 
between P-tunnels (note that other specs put constraints on how that can 
be done, such as [I-D.ietf-bess-mvpn-extranet 
<https://tools.ietf.org/html/draft-ietf-bess-mvpn-bidir-ingress-replication-02#ref-I-D.ietf-bess-mvpn-extranet>])".

> As for draft-ietf-bess-ir: RFC 6514 specifies the use of IR P-tunnels, 
> though there are some problems with RFC 6514's specification of IR 
> P-tunnels, which are addressed in detail in draft-ietf-bess-ir.
>
> Draft-ietf-bess-mvpn-bidir-ingress-replication explains how to support 
> a VPN customer's use of BIDIR-PIM when the service provider uses IR 
> P-tunnels, but it doesn't really depend on those details specified in 
> draft-ietf-bess-ir. Thus draft-ietf-bess-ir should not be a normative 
> reference for it.
>

Given that draft-ietf-bess-ir updates RFC6514/RFC6513 which our draft 
depends on Normatively, we might as well avoid mentioning it; making it 
a Normative ref would not buy much (but would delay publication until 
draft-ietf-bess-ir is published).

Since its helpful to remind the reader of its existence, I would think 
it's good to keep it, and Informative is fine.

Best,

-Thomas

---

From: "Alvaro Retana (aretana)" <[email-masked]>
Date: Wed, 30 Sep 2015 04:08:04 +0000

On 9/28/15, 10:39 AM, "[email-masked]<mailto:[email-masked]>" <[email-masked]<mailto:[email-masked]>> wrote:

Jeffrey/Thomas:

Hi!

First of all, please note that the Gen-ART review that came in today had the same comment about the Normative references…which really indicates that if you want to change them, then (at least) the text needs to be clarified.

I agree with that.
To make it more obvious that this is an informative comment, I think you could rewrite as: "These specifications do not prevent sharing of labels between P-tunnels (note that other specs put constraints on how that can be done, such as [I-D.ietf-bess-mvpn-extranet<https://tools.ietf.org/html/draft-ietf-bess-mvpn-bidir-ingress-replication-02#ref-I-D.ietf-bess-mvpn-extranet>])".

This proposed text changes what seems to be the original intent..if that was the goal, then I’m ok with something like it.  In fact, if the rules in ietf-bess-mvpn-extranet are not needed then don’t even mention them — referring to them just causes confusion as to whether they were needed.

The text says this:  "This document describes how the MP2MP tunnel can be simulated with a mesh of P2MP tunnels, each of which is instantiated by Ingress Replication [I-D.ietf-bess-ir].”  What you say above is that the reference here should be RFC6514, is that right?

Given that draft-ietf-bess-ir updates RFC6514/RFC6513 which our draft depends on Normatively, we might as well avoid mentioning it; making it a Normative ref would not buy much (but would delay publication until draft-ietf-bess-ir is published).

It has to be Normative it the functionality depends on it (see me question above).  There’s no way around that.

Note that the fact that I-D.ietf-bess-ir may update RFC6513/RFC6514 (if approved!) doesn’t mean anything at this point because the functionality that this document relies on (??) is not in those RFCs, or in any other RFC that updates them.

If the functionality in I-D.ietf-bess-ir is not needed, then just change the reference above.

Since its helpful to remind the reader of its existence, I would think it's good to keep it, and Informative is fine.

Why?  If the functionality in I-D.ietf-bess-ir is not needed, then just don’t mention it at all.

Thanks!

Alvaro.

---

From: <[email-masked]>
Date: Wed, 30 Sep 2015 09:34:38 +0200

Hi Alvaro,

2015-09-30, Alvaro Retana (aretana):
>     To make it more obvious that this is an informative comment, I
>     think you could rewrite as: "These specifications do not prevent
>     sharing of labels between P-tunnels (note that other specs put
>     constraints on how that can be done, such as
>     [I-D.ietf-bess-mvpn-extranet
> This proposed text changes what seems to be the original intent..if 
> that was the goal, then I’m ok with something like it.

I would tend to think that the reformulation I proposed preserves the 
original intent.
I would propose improving the rephrasing by taking into account the 
sentence that follows:

Current:

    The label may be shared
    with other P-tunnels, subject to the anti-ambiguity rules for
    extranet [I-D.ietf-bess-mvpn-extranet 
<https://tools.ietf.org/html/draft-ietf-bess-mvpn-bidir-ingress-replication-02#ref-I-D.ietf-bess-mvpn-extranet>].  For example, the (C-*,C-*-
    BIDIR) and (C-*,C-G-BIDIR) S-PMSI A-D routes originated by a given PE
    can optionally share a label.

Proposed:
These specifications do not prevent sharing of labels between P-tunnels, 
such as a label being shared by a (C-*,C-*- BIDIR) and a (C-*,C-G-BIDIR) 
S-PMSI A-D route originated by a given PE(note that other specs put 
constraints on how that can be done, e.g. [I-D.ietf-bess-mvpn-extranet 
<https://tools.ietf.org/html/draft-ietf-bess-mvpn-bidir-ingress-replication-02#ref-I-D.ietf-bess-mvpn-extranet>]).

> In fact, if the rules in ietf-bess-mvpn-extranet are not needed then 
> don’t even mention them — referring to them just causes confusion as 
> to whether they were needed.

I think that if this is clearly provided as an example then it can help 
the reader have the right context information to better understand the spec.

>
>>     As for draft-ietf-bess-ir: RFC 6514 specifies the use of IR
>>     P-tunnels, though there are some problems with RFC 6514's
>>     specification of IR P-tunnels, which are addressed in detail in
>>     draft-ietf-bess-ir.
>>
>>     Draft-ietf-bess-mvpn-bidir-ingress-replication explains how to
>>     support a VPN customer's use of BIDIR-PIM when the service
>>     provider uses IR P-tunnels, but it doesn't really depend on those
>>     details specified in draft-ietf-bess-ir. Thus draft-ietf-bess-ir
>>     should not be a normative reference for it.
>>
> The text says this:  "This document describes how the MP2MP tunnel can 
> be simulated with a mesh of P2MP tunnels, each of which is 
> instantiated by Ingress Replication [I-D.ietf-bess-ir].”  What you say 
> above is that the reference here should be RFC6514, is that right?

I'm the one who suggested referencing [I-D.ietf-bess-ir], but I have to 
admit this creates confusion.

The best reference to provide for ingress replication is in fact RFC6513 
section 2.1.2 (could be RFC6514 as well, but 6513 is the one having the 
text explaining the concept).  Both are updated by [I-D.ietf-bess-ir].

>
>
>     Given that draft-ietf-bess-ir updates RFC6514/RFC6513 which our
>     draft depends on Normatively, we might as well avoid mentioning
>     it; making it a Normative ref would not buy much (but would delay
>     publication until draft-ietf-bess-ir is published).
>
>
> It has to be Normative it the functionality depends on it (see me 
> question above).  There’s no way around that.

Well, to my knowledge draft-ietf-bess-mvpn-bidir-ingress-replication 
does not particularly depends on [I-D.ietf-bess-ir], but the 
clarifications and minor fixes in [I-D.ietf-bess-ir] benefit to any mVPN 
spec (including draft-ietf-bess-mvpn-bidir-ingress-replication).

>
> Note that the fact that I-D.ietf-bess-ir may update RFC6513/RFC6514 
> (if approved!) doesn’t mean anything at this point because the 
> functionality that this document relies on (??) is not in those RFCs, 
> or in any other RFC that updates them.

In fact, the functionality that this document depends on is described in 
RFC6513 section 2.1.2.

>     Since its helpful to remind the reader of its existence, I would
>     think it's good to keep it, and Informative is fine.
>
>

I think that a fine balance can sometimes be found and allow providing 
more context.
I agree with you that this it not the case here.

Let's use RFC6513 section 2.1.2 as the reference for ingress 
replication, and eventually let the reader find out by himself when/if 
the IR-replication related materiel in this RFC is updated...

Best,

-Thomas

---

From: "Alvaro Retana (aretana)" <[email-masked]>
Date: Wed, 30 Sep 2015 16:01:05 +0000

On 9/30/15, 2:34 AM, "[email-masked]<mailto:[email-masked]>" <[email-masked]<mailto:[email-masked]>> wrote:

Thomas:

Hi!

. . .
Current:

   The label may be shared
   with other P-tunnels, subject to the anti-ambiguity rules for
   extranet [I-D.ietf-bess-mvpn-extranet<https://tools.ietf.org/html/draft-ietf-bess-mvpn-bidir-ingress-replication-02#ref-I-D.ietf-bess-mvpn-extranet>].  For example, the (C-*,C-*-
   BIDIR) and (C-*,C-G-BIDIR) S-PMSI A-D routes originated by a given PE
   can optionally share a label.

Proposed:
    These specifications do not prevent sharing of labels between P-tunnels, such as a label being shared by a (C-*,C-*- BIDIR) and a (C-*,C-G-BIDIR) S-PMSI A-D route originated by a given PE (note that other specs put constraints on how that can be done, e.g. [I-D.ietf-bess-mvpn-extranet<https://tools.ietf.org/html/draft-ietf-bess-mvpn-bidir-ingress-replication-02#ref-I-D.ietf-bess-mvpn-extranet>]).

In fact, if the rules in ietf-bess-mvpn-extranet are not needed then don’t even mention them — referring to them just causes confusion as to whether they were needed.

I think that if this is clearly provided as an example then it can help the reader have the right context information to better understand the spec.

That works for me.   Just a nit: s/These specifications/This specification

. . .
Let's use RFC6513 section 2.1.2 as the reference for ingress replication, and eventually let the reader find out by himself when/if the IR-replication related materiel in this RFC is updated...

Fine with me too.

Thanks!

Alvaro.