Re: Comments on draft-ietf-l2vpn-vpls-mcast-08

Rahul Aggarwal <rahul@juniper.net> Tue, 05 July 2011 11:15 UTC

Return-Path: <rahul@juniper.net>
X-Original-To: l2vpn@ietfa.amsl.com
Delivered-To: l2vpn@ietfa.amsl.com
Received: from localhost (localhost [127.0.0.1]) by ietfa.amsl.com (Postfix) with ESMTP id A711321F8776 for <l2vpn@ietfa.amsl.com>; Tue, 5 Jul 2011 04:15:29 -0700 (PDT)
X-Virus-Scanned: amavisd-new at amsl.com
X-Spam-Flag: NO
X-Spam-Score: -6.599
X-Spam-Level:
X-Spam-Status: No, score=-6.599 tagged_above=-999 required=5 tests=[BAYES_00=-2.599, RCVD_IN_DNSWL_MED=-4]
Received: from mail.ietf.org ([64.170.98.30]) by localhost (ietfa.amsl.com [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id 1tJv2D42DyvY for <l2vpn@ietfa.amsl.com>; Tue, 5 Jul 2011 04:15:27 -0700 (PDT)
Received: from exprod7og101.obsmtp.com (exprod7og101.obsmtp.com [64.18.2.155]) by ietfa.amsl.com (Postfix) with ESMTP id 16FC321F8773 for <l2vpn@ietf.org>; Tue, 5 Jul 2011 04:15:26 -0700 (PDT)
Received: from P-EMHUB01-HQ.jnpr.net ([66.129.224.36]) (using TLSv1) by exprod7ob101.postini.com ([64.18.6.12]) with SMTP ID DSNKThLyTH3eNoaFxxfOmYNryITt3PwviwCh@postini.com; Tue, 05 Jul 2011 04:15:26 PDT
Received: from magenta.juniper.net (172.17.27.123) by P-EMHUB01-HQ.jnpr.net (172.24.192.33) with Microsoft SMTP Server (TLS) id 8.2.254.0; Tue, 5 Jul 2011 04:14:01 -0700
Received: from sapphire.juniper.net (sapphire.juniper.net [172.17.28.108]) by magenta.juniper.net (8.11.3/8.11.3) with ESMTP id p65BDxv01125; Tue, 5 Jul 2011 04:13:59 -0700 (PDT) (envelope-from rahul@juniper.net)
Date: Tue, 05 Jul 2011 04:14:01 -0700
From: Rahul Aggarwal <rahul@juniper.net>
To: Eric Rosen <erosen@cisco.com>
Subject: Re: Comments on draft-ietf-l2vpn-vpls-mcast-08
In-Reply-To: <2717.1301948396@erosen-linux>
Message-ID: <20110704094331.C67332@sapphire.juniper.net>
References: <2717.1301948396@erosen-linux>
MIME-Version: 1.0
Content-Type: text/plain; charset="US-ASCII"; format="flowed"
Cc: l2vpn@ietf.org
X-BeenThere: l2vpn@ietf.org
X-Mailman-Version: 2.1.12
Precedence: list
List-Id: <l2vpn.ietf.org>
List-Unsubscribe: <https://www.ietf.org/mailman/options/l2vpn>, <mailto:l2vpn-request@ietf.org?subject=unsubscribe>
List-Archive: <http://www.ietf.org/mail-archive/web/l2vpn>
List-Post: <mailto:l2vpn@ietf.org>
List-Help: <mailto:l2vpn-request@ietf.org?subject=help>
List-Subscribe: <https://www.ietf.org/mailman/listinfo/l2vpn>, <mailto:l2vpn-request@ietf.org?subject=subscribe>
X-List-Received-Date: Tue, 05 Jul 2011 11:15:29 -0000

Hi Eric,

Thank you for the comments. Sorry for the delay.
A revised draft that addresses your comments has been posted:

http://www.ietf.org/internet-drafts/draft-ietf-l2vpn-vpls-mcast-09.txt


Also please see below:

> Since I haven't been following this mailing list regularly, I missed the WG
> LC on draft-ietf-l2vpn-vpls-mcast-08.  I do have a few comments to make
> though; fortunately IETF LC has not occurred yet.
>
> 1. "Snooped State"
>
>> From section 11.3:
>
>       If as a result of IGMP or PIM snooping on the PE-CE interfaces,
>       the PE has snooped state for at least one multicast join that
>       matches the multicast source and group advertised in the S-PMSI
>       A-D route.
>
> The document contains no definition of "IGMP snooping" or "PIM snooping",
> nor any reference to another document defining these terms.  The document
> proceeds to freely use the term "snooped state", without any definition.
>

I have added a reference to RFC 4541 for IGMP snooping and clarified that
PIM snooping procedures require future work.

> Without a definition of (or normative reference to such a definition)
> "snooping" and "snooped state", I don't see how this document can claim to
> be implementable.
>
> 2. How does traffic get from a multicast source to the designated router?
>
> Consider group G.  Suppose site 1 contains the DR (but no sources or
> receivers), site 2 contains the source S, site 3 contains a receiver R.  The
> PE attached to site 2 might set up a selective tree for (S,G), and the PE
> attached to site 3 will join the selective tree, no doubt due to it's having
> "snooped state" indicating that there is a receiver at site 3.  The PE
> attached to site 1 also needs to join the selective tree, or S will be cut
> off from its DR.  However, I don't see where in the draft it provides a
> procedure causing the site 1 PE to join the selective tree.
>
> Presumably there has to be some "snooping" of the DR election.  But this
> doesn't seem to be mentioned at all.
>

Please see response to your comment above - PIM snooping procedures 
require further work.

> 3. Applicability Restrictions
>
> The following applicability restriction is stated:
>
>   These procedures are applicable when IGMP is used as the multicast
>   routing protocol between the VPLS CEs. They are also applicable when
>   PIM is used as the multicast routing protocol between the VPLS CEs
>   and PIM join suppression is disabled on all the CEs. However these
>   procedures do not apply when PIM is used as the multicast routing
>   protocol between the VPLS CEs and it not possible to disable PIM join
>   suppression on all the CEs
>
> This suggests, incorrectly, that PIM and IGMP will not be running on the
> same LAN. (Not sure whether that is the intention or not.)  I think the
> correct statement of the applicability restriction is the following:
>

That is not the intent. I think the text is clear as it stands.

>   These procedures are not applicable if any VPLS CE, or any other host or
>   router that is bridged to the VPLS, uses a multicast control protocol
>   that does not periodically retransmit control messages stating the set of
>   (S,G) and (*,G) flows in which it is interested.
>
>   Among such CEs are the following:
>
>   - CEs that run PIM, but that have join suppression enabled (i.e., the
>     vast majority of systems running PIM).  Note that the issue is not
>     whether it is "possible to disable PIM join suppression on all the
>     CEs", but whether it has actually been disabled.
>
>   - CEs that run the protocol specified in draft-ietf-pim-port-06.txt.
>
>   - CEs that run the protocol specified in draft-ietf-l3vpn-2547bis-mcast-
>     bgp.
>

The applicability is limited to IGMP and PIM as in RFC 4601.

> There should also be a clear statement to SPs that if a customer deploys
> such a CE, the VPLS multicast service will create black holes that are
> going to be difficult for the SP to troubleshoot.
>
> If the document is going to recommend that join suppression be disabled (if
> possible), there should be some discussion of the costs of disabling join
> suppression.
>
> 4. Allowable P-Tunnel Technologies
>
> It is clear that the document allows the use of RSVP-TE P2MP and allows the
> use of mLDP.  It is very difficult to understand just what the intention is
> with respect to other tunneling technologies.  The document claims that the
> architecture is flexible enough to support other technologies, but it also
> says that other technologies are not supported. Right now the document does
> not appear to make a consistent statement about this, leaving it unclear
> whether the document means to prohibit other technologies or not.
>

The text has been clarified to limit support to P2MP RSVP-TE or P2MP LDP.

> Perhaps what it should say is just that other technologies are outside the
> scope of this document.
>
> 5. Options, models, variants
>
> The terminology in section 10 is very confusing.  The terms "options",
> "models", and "variants" are not used in a consistent manner.
>
> - Section 10 mentions "options" a, b, and c, then proceeds almost
>  immediately (section 10.2) to an option e, which is never explicitly
>  defined.
>
> - Options a, b, and c are called "three models", and there is a requirement
>  to support all three "models".  Then the text goes on to say that options a
>  and b use a single "model" ("where inter-AS VPLS service can be offered
>  without requiring a single P-multicast tree to span multiple ASes".)  Then
>  there are two "variants" of this "model".
>
> - I think the intention is that there are two "variants": "VSIs on the
>  ASBRs", and "Segmented Inter-AS Trees", and that these two variants
>  combine with the four "options" (a, b, c, and e) to make a total of 5
>  models:
>
>  * Model 1: Option (a) plus the variant "VSIs on the ASBRS"
>
>  * Model 2: Option (e) plus the variant "VSIs on the ASBRS"
>
>  * Model 3: Option (b) plus the variant "Segmented Inter-AS Trees"
>
>  * Model 4: Option (c) plus the variant "Segmented Inter-AS Trees"
>

What you call Model 4 is not supported by this specification.

>  * Model 5: Option (c) plus non-segmented Inter-AS trees but without VSIs
>    on the ASBRs (not sure I got this one right, this seems like a hitherto
>    unmentioned "variant").
>
>  The terminology in section 10 really needs to be cleaned up for clarity
>  and consistency.
>

Done.

> 6. Section 11.2.1
>
>   This section is referred to multiple times, as the place where the
>   explicit tracking procedures are specified.  However, there is no section
>   11.2.1. I suspect the proper reference is 11.3.
>

Fixed.

> 6. Miscellaneous comments
>
>> From the introduction (section 4), page 5:
>
>   [RFC4761] and [RFC4762] describe a solution for VPLS multicast that
>   relies on ingress replication.
>
> "Ingress replication" should be defined; I don't think this term appears in
> either RFC4761 or RFC4762.
>

Done.

> Also from the introduction, page 5:
>
>   It provides mechanisms that allow a single multicast distribution
>   tree in the Service Provider (SP) network to carry all the multicast
>   traffic from one or VPLS sites connected to a given PE, irrespective
>
> "from one or VPLS" --> "from one or more VPLS"
>

Fixed.

>> From the overview (section 6), page 6:
>
>   This document describes procedures for using multicast trees for VPLS
>   multicast when the provider tunneling technology is either P2MP RSVP-
>   PE or mLDP [MLDP]. The protocol architecture described herein is
>   considered to be flexible to support other P-tunneling technologies
>   as well.
>
> The phrase "either P2MP RSVP-PE or mLDP" is mangled.  I think what is
> intended is "P2MP LSPs created either by RSVP-TE or mLDP".
>

Fixed.

> Page 7
>
>   The ability to carry the traffic of more than one VPLS on the
>   same tree is termed of the VPLSes that are using the tree. This
>
> "is termed of the VPLSes", looks like a cut-and-paste error of some sort.
>

Fixed.

>   An Inclusive P-Multicast tree as defined in this document is a P2MP
>   tree.  A P2MP tree is used to carry traffic only for VPLS sites that
>   are connected to the PE that is the root of the tree.
>
> I suggest "carry traffic only for" be replaced by "carry traffic only from",
> as "for" could be taken to mean either "to" or "from" or "both" in this context.
>

Done.

> Note that this paragraph seems to imply that MP2MP LSPs or PIM-BIDIR trees
> would not be Inclusive P-multicast trees "as defined in this document".
> Perhaps what it should say is "This document only defines procedures for
> instantiating an inclusive P-multicast tree as a P2MP LSP; other possible
> methods of instantiation are outside the scope of this document".  This
> would be more consistent with the prior statement that the protocol
> architecture of the document is flexible enough to support other tunneling
> technologies.
>

The text has been clarified to restrict the scope to P2MP RSVP-TE or P2MP 
LDP LSPs.

> Also on page 7:
>
>       2. Selective trees. A Selective P-Multicast tree is used by a PE
>   to send IP multicast traffic for one or IP more specific multicast
>   streams, originated within sites connected to the PE, that belong to
>   the same or different VPLSes, to a subset of the PEs that belong to
>   those VPLSes. Each of the PEs in the subset should be on the path to
>   a receiver of one or more multicast streams that are mapped onto the
>   tree. The ability to use the same tree for multicast streams that
>   belong to different VPLSes is termed a PE the ability to create
>
> "Is termed a PE the ability to create"?  Maybe this should be "gives a PE
> the ability to create".
>

It was an error in the source formatting. Fixed.

> I don't think it is correct to say that the traffic must have originated
> within a site connected to the PE; the traffic could have originated
> somewhere far away on the Internet, in a television studio, etc.  Instead of
> "originated within sites connected to the PE", maybe what is meant is
> "received by the PE over a PE-CE interface".  This error occurs in multiple
> places.
>

Fixed.

> Page 7 again:
>
>   A SP can use both Inclusive P-Multicast trees and Selective P-
>   Multicast trees or either of them for a given VPLS on a PE, based on
>   local configuration.  Inclusive P-Multicast trees can be used for
>   both IP and non-IP data multicast traffic, while Selective P-
>   Multicast trees can be used only for IP multicast data traffic.
>
> It might be better to say "using selective P-trees for anything other than
> IP multicast data traffic is outside the scope of this spec."  If this is
> really meant to prohibit such use entirely, normative language ("MUST NOT")
> should be used.
>

Fixed.

> Once more, page 7:
>
>   A variety of transport technologies may be used in the SP network.
>   For inclusive P-Multicast trees, these transport technologies include
>   point-to-multipoint LSPs created by RSVP-TE or mLDP. For selective P-
>   Multicast trees, only unicast PE-PE tunnels (using MPLS or IP/GRE
>   encapsulation) and P2MP LSPs are supported, and the supported P2MP
>   LSP signaling protocols are RSVP-TE, and mLDP.
>
> It's not very clear what this paragraph is saying.  If a technology is "not
> supported", does that mean it is outside the scope of this document, or that
> it is prohibited for some unspecified reason.  (If the latter, the reason
> should be specified.)
>
> It's hard to understand why unicast tunnels are supported only for selective
> tunnels, why GRE encaps is supported only for unicast tunnels, etc.
>

I clarified that other transport technologies are outside the scope.

> Page 8:
>
>   However these
>   procedures do not apply when PIM is used as the multicast routing
>   protocol between the VPLS CEs and it not possible to disable PIM join
>   suppression on all the CEs.  Procedures for this case are for further
>   study.
>
> Change "it not possible to disable PIM Join suppression on all the CEs" to
> "PIM Join Suppression is not disabled on all the CEs".

Done.

>  But see also my
> prior comment on other multicast protocols for which every CE does not
> periodically retransmit every Join.
>

I commented on that above.

> Page 10:
>
>   Aggregation requires a mechanism for the egresses of the P-Multicast
>   tree to demultiplex the multicast traffic received over the P-
>   Multicast tree. This document describes how upstream-assigned labels
>   can be assigned and distributed by the root of aggregate P-Multicast
>   tree and then used by the egresses to perform this demultiplexing.
>
> I think it would be clearer to replace the last sentence by "To enable the
> egress nodes to perform this demultiplexing, upstream-assigned labels
> [RFC5331] MUST be assigned and distributed by the root of the aggregate
> P-multicast tree".  This would make it clear that upstream-assigned labels
> are a mandatory feature if (and only if) aggregation is used.
>

Done.

> Page 11:
>
>   It is to be noted that BGP A-D is an inherent feature of BGP-VPLS.
>   However it is not an inherent feature of LDP-VPLS. Infact there are
>   deployments and/or implementations of LDP-VPLS that require
>   configuration to enable a PE in a particular VPLS to determine other
>   PEs in the VPLS and exchange PW labels using FEC 128 [RFC4447]. The
>   use of BGP A-D for LDP-VPLS [L2VPN-SIG], to enable automatic setup of
>   PWs, requires FEC 129 [RFC4447]. However FEC 129 is not required in
>   order to use BGP A-D for the setup of P-Multicast trees for LDP-VPLS
>   as described in this document. An LDP-VPLS implementation that
>   supports P-Multicast trees described in this document, MUST support
>   the BGP A-D procedures to setup P-Multicast trees and it MAY support
>   FEC 129 to automate the signaling of PWs.
>
> It's very unclear to me just what is and is not being required by this
> paragraph.  If it's just that an LDP-VPLS implementation may support the
> procedures in this draft even if it doesn't support FEC 129, that seems fair
> enough.

That is what is being referred to.

> If it's that an LDP-VPLS implementation that uses P-multicast trees
> to optimize IP multicast MUST support the BGP A-D procedures described
> herein, that seems gratuitous.  The phrase "supports P-multicast trees
> described in this document" is also unclear.
>

I clarified this. Please see new text.

> Page 12:
>
>   A PE that uses an Inclusive P-Multicast tree to instantiate the
>   provider tunnel MAY aggregate two or more VPLSes present on the PE
>   onto the same tree. If the PE decides to perform aggregation after it
>   has already advertised the intra-AS VPLS auto-discovery routes for
>   these VPLSes, then aggregation requires the PE to re-advertise these
>   routes.
>
> Should there be some statement about when it's okay to start sending the
> data on the newly aggregated tunnel?  Is it necessary or desirable to wait a
> certain amount of time?
>

This is no different than sending data on a new P-Tunnel. I don't think 
any further clarification is necessary.

> Page 12:
>
>   The re-advertised routes MUST be the same as the original
>   ones, except for the PMSI Tunnel attribute. If the PE has not
>   previously advertised intra-AS auto-discovery routes for these
>   VPLSes, then the aggregation requires the PE to advertise (new)
>   intra-AS auto-discovery routes for these VPLSes.  The P-Tunnel
>   attribute in the newly advertised/re-advertised routes MUST carry the
>   identity of the P-Multicast tree that aggregates the VPLSes, as well
>   as an MPLS upstream-assigned label [RFC5331]. Each re-advertised
>   route MUST have a distinct label.
>
> For consistency, "P-Tunnel attribute" --> "PMSI Tunnel attribute"
>
> The requirement for a distinct label applies to the newly advertised routes
> as well as the re-advertised routes, no?
>

Done.

> Page 13:
>
>     + If the Tunnel Type in the PMSI Tunnel attribute is set to RSVP-TE
>       P2MP LSP, the receiving PE has to establish the appropriate state
>       to properly handle the traffic received over that LSP. The PE
>       that originated the route MUST establish an RSVP-TE P2MP LSP with
>       the local PE as a leaf. This LSP MAY have been established before
>       the local PE receives the route.
>
> Isn't a normative reference to draft-ietf-mpls-rsvp-te-no-php-oob-mapping-
> 05.txt needed here, for the case where the TE tunnel is established before
> its use has been communicated?
>

Section 9.2.1 describes this along with a normative reference to RSVP-OBB.

> Page 14:
>
>   When a P-Multicast tree is mapped to only one VPLS, determining the
>   tree on which the packet is received is sufficient to determine the
>   VPLS instance on which the packet is received. The tree is determined
>   based on the tree encapsulation. If MPLS encapsulation is used, eg:
>   RSVP-TE P2MP LSPs, the outer MPLS label is used to determine the
>   tree. Penultimate-hop-popping MUST be disabled on the MPLS LSP (RSVP-
>   TE P2MP LSP or LDP P2MP LSP).
>
> Doesn't this require a normative reference to draft-ietf-mpls-rsvp-te-no-
> php-oob-mapping-05.txt?
>

Please see above.

> Page 14, section 8.2:
>
>   This document requires the use of upstream label assignment by the
>   ingress PE [RFC5331].
>
> It might be better to use 2119 language here, "If traffic from multiple
> VPLSes is carried on a single tree, upstream-assigned labels [RFC5331] MUST
> be used.
>

Done.

> Also page 14:
>
>   If the tree uses MPLS encapsulation, as in RSVP-TE P2MP LSPs, the
>   outer MPLS label and the incoming interface provides the label space
>   of the label beneath it. This assumes that penultimate-hop-popping is
>   disabled. The egress PE MUST NOT advertise IMPLICIT NULL or EXPLICIT
>   NULL for that tree.
>
> If the P2MP LSP is signaled before the BGP route is received, how does the
> egress PE know to avoid advertising NULL for that LSP?
>

Clarified this with following text:

"The egress PE MUST NOT advertise IMPLICIT NULL or EXPLICIT NULL for that 
tree once its known to the egress PE that the tree is bound to one or more VPLSes."

> Page 15:
>
>   9. Establishing P-Multicast Trees
>
>   This document does not place any fundamental restrictions on the
>   multicast technology used to setup P-Multicast trees. However
>   specific procedures are specified only for RSVP-TE P2MP LSPs and LDP
>   P2MP LSPs. An implementation that supports this document MUST support
>   RSVP-TE P2MP LSPs and LDP P2MP LSPs.
>
>   The P-Multicast trees supported in this document are P2MP trees.
>
> It would be less confusing to say that "specific procedures are specified
> only for P2MP LSPs", otherwise it is hard to know what to make of these two
> paragraphs together.
>
>   A
>   P2MP tree is used to carry traffic originated in sites connected to
>   the PE which is the root of the tree, irrespective of whether these
>   sites belong to the same or different VPLSes.
>
> This suggests, incorrectly, that aggregation is always done. Maybe change
> "is used" to "MAY be used".
>

The text has been clarified.

>
> 9.1. Common Procedures
>
>   The following procedures apply to both RSVP-TE P2MP and LDP P2MP
>   LSPs.
>
>   Demultiplexing the C-multicast data packets at the egress PE requires
>   that the PE must be able to determine the P2MP LSP that the packets are
>   received on. This enables the egress PE to determine the VPLS that the
>   packet belongs to. To achieve this the LSP MUST be signaled with
>   penultimate-hop-popping (PHP) off as described in section 8.  In other
>   words an egress PE MUST NOT advertise IMPLICIT NULL or EXPLICIT NULL for
>   a P2MP LSP that is carrying traffic for one or more VPLSes.
>
> As I commented earlier, I don't think this draft explains how the egress
> knows to do this in the case of RSVP-TE P2MP LSPs.
>

This just works if the LSP signaling is received after the BGP A-D route 
that carries the accompanying PMSI Tunnel attribute. If the LSP signaling 
is received earlier then section 9.2.1 has details.

> Page 17:
>
> 9.4. Encapsulation of Aggregate P-Multicast Trees
>
>   An Aggregate Inclusive P-Multicast tree or an Aggregate Selective P-
>   Multicast tree MUST use a MPLS encapsulation. The protocol type in
>   the data link header is as described in [RFC5332].
>
> Presumably the encapsulation used to send data on a certain tunnel is
> determined by the tunnel type.  I'm not sure why this short section is
> needed, there is no section on encapsulation of non-aggregate trees.
>

Its useful to refer to RC5332 regarding the encaps.

> Page 27:
>
>   These procedures are applicable when IGMP is used as the multicast
>   routing protocol between the VPLS CEs. They are also applicable when
>   PIM is used as the multicast routing protocol between the VPLS CEs
>   and PIM join suppression is disabled on all the CEs. However these
>   procedures do not apply when PIM is used as the multicast routing
>   protocol between the VPLS CEs and it not possible to disable PIM join
>   suppression on all the CEs.  Procedures that allow the setup of
>   Selective trees for this case are for further study.
>
> This paragraph is just repeated from earlier in the document.  See my
> earlier comments. (Regardless of the disposition of those comments, this
> paragraph should probably not be repeated here.)
>

Deleted the paragaraph here.

> Page 28:
>
>   If the Selective tree is instantiated by a RSVP-TE P2MP LSP the PE at
>   the root of the tree MUST establish the P2MP RSVP-TE LSP to the
>   leaves. This LSP MAY have been established before the leaves receive
>   the Selective tree binding, or MAY be established after the leaves
>   receives the binding. A leaf MUST not switch to the Selective tree
>   until it receives the binding and the RSVP-TE P2MP LSP is setup to
>   the leaf.
>
> This paragraph has already occurred before.
>

In this case its good to repeat it for context.

> Page 31:
>
>     + If as a result of IGMP or PIM snooping on the PE-CE interfaces,
>       the PE has snooped state for at least one multicast join that
>       matches the multicast source and group advertised in the S-PMSI
>       A-D route. Further if the oif (outgoing interfaces) for this
>       state contains one or more interfaces to the locally attached
>       CEs.
>
>
> As previously pointed out, "IGMP or PIM snooping" has to be defined and
> specified (perhaps via a normative reference).  Otherwise there is no way to
> know what is meant below by "snooped state" how such states are created
> and destroyed, and exactly what states the PEs need to keep track of.
>

Fixed as stated in the response above.

> Pages 31-32
>
>   The snooped state is said to "match" the S-PMSI A-D route if any of
>   the following is true:
>
>     +  The S-PMSI A-D route carries (C-S, C-G) and the snooped state is
>       for (C-S, C-G). OR
>
>     +  The S-PMSI A-D route carries (C-*, C-G) and (a) the snooped
>       state is for (C-*, C-G) OR (b) the snooped state is for at least
>       one multicast join with the multicast group address equal to C-G
>       and there doesn't exist another S-PMSI A-D route that carries (C-
>       S, C-G) where C-S is the source address of the snooped state.
>
>     +  The S-PMSI A-D route carries (C-S, C-*) and (a) the snooped
>       state is for at least one multicast join with the multicast
>       source address equal to C-S, and (b) there doesn't exist another
>       S-PMSI A-D route that carries (C-S, C-G) where C-G is the group
>       address of the snooped state.
>
>     +  The S-PMSI A-D route carries (C-*, C-*)
>
> As this algorithm is specified, every "snooped state" matches an S-PMSI A-D
> route specifying (C-*,C-*), even if (C-*,C-*) is not the "longest match".  I
> think what is meant is that the snooped state matches the S-PMSI A-D route
> that meets the first condition, if there is one; otherwise it matches the
> S-PMSI A-D route that meets the second condition, if there is one; otherwise
> it matches the S-PMSI A-D route that meets the third condition, if there is
> one; otherwise it matches the S-PMSI A-D route that meets the fourth
> condition, if there is one; otherwise there is no match. However, this is
> not what it says.
>

Added text to fix this.

> Something should be specified to cover the case where the S-PMSI A-D route
> does not match any "snooped state",

This is quite explicit as the text in the section applies only 
when matching snooped state exists.

> and the case where a newly created
> "snooped state" matches an S-PMSI A-D route that is already present.
>

Added text to cover this.

> Page 36:
>
> 12.1. Inclusive Tree/Selective Tree Identifier
>
>   Inclusive P-Multicast tree and Selective P-Multicast tree
>   advertisements carry the P-Multicast tree identifier.
>
>   This document reuses the BGP attribute, called PMSI Tunnel Attribute
>   that is defined in [MVPN].
>
>   Only the following Tunnel Types MUST be used when PMSI Tunnel
>   attribute is carried in VPLS A-D or VPLS S-PMSI A-D routes:
>
>     + 0 - No tunnel information present
>     + 1 - RSVP-TE P2MP LSP
>     + 2 - LDP P2MP LSP
>     + 6 - Ingress Replication
>
> If the protocol architecture is flexible enough to handle other tunnel
> types, as is stated several times earlier, this should say that the use of
> other tunnel types is outside the scope of the document, rather than saying
> that other tunnel types MUST NOT be used.  (Actually, the phrase "only the
> following tunnel types MUST be used ..." is not really comprehensible, I'm
> just guessing that it means that other tunnel types MUST NOT be used.)
>

Clarified the text.

> Page 38:
>
>   The Multicast Source field contains the C-S address i.e the address
>   of the multicast source. This may be 0 to indicate a wildcard. If the
>   Multicast Source field contains an IPv4 address, then the value of
>   the Multicast Source Length field is 32. If the Multicast Source
>   field contains an IPv6 address, then the value of the Multicast
>   Source Length field is 128.
>
>   The Multicast Group field contains the C-G address i.e. the address
>   of the multicast group. This may be 0 to indicate a wildcard.  If the
>   Multicast Group field contains an IPv4 address, then the value of the
>   Multicast Group Length field is 32. If the Multicast Group field
>   contains an IPv6 address, then the value of the Multicast Group
>   Length field is 128.
>
> The wildcard encoding does not appear to be consistent with that used in
> MVPN.  It is also not clear what the length of a wildcard field is supposed
> to be, or whether a wildcard is a wildcard only for a particular address
> family.
>

Fixed in the revised version.

> Please state how to determine the address family of the "originating
> router's IP address".
>

Added text on this.

>   Usage of Selective Tree auto-discovery routes is described in Section
>   12.
>
> Well, this is section 12!
>

It should read section 11. Fixed.

>
> Page 38:
>
>   A leaf auto-discovery route type specific MCAST-VPLS NLRI consists of
>   the following:
>
>                +-----------------------------------+
>                |      Route Key (variable)         |
>                +-----------------------------------+
>                |   Originating Router's IP Addr    |
>                +-----------------------------------+
>
>   Usage of Leaf auto-discovery routes is described in sections "Inter-
>   AS Inclusive P-Multicast tree A-D/Binding" and "Optimizing Multicast
>   Distribution via Selective trees".
>
> Please state how to determine the address family of the "originating
> router's IP address".
>

Added text on this.


> Page 39:
>
>     13. Aggregation Methodology
>
> A better title would be "Aggregation Considerations", as this section
> contains some discussion and observations, but does not specify a
> methodology.
>

Done.

> Page 41:
>
>   If the destination MAC address of a VPLS packet received by a PE from
>   a VPLS site is a multicast adddress, a P-Multicast tree SHOULD be
>   used to transport the packet, if possible. If the packet is an IP
>   multicast packet and a Selective tree exists for that multicast
>   stream, the Selective tree SHOULD be used. Else if an Inclusive tree
>   exists for the VPLS, it SHOULD be used.
>
> So a transmitter is not required to use the selective tree for a given flow,
> even if it exists?  Is that really the intention?
>

No. Replaced with "Selective tree MUST be used"

> It is not completely clear what the intention is for a packet that has an
> ethernet multicast DA but is not an IP multicast packet.  Would it ever be
> sent on a selective tree (perhaps one bound to (C-*,C-*))?
>

Added text to clarify this.

rahul