Subject: RtgDir review: draft-ietf-l2vpn-vpls-mcast-14.txt

From: "Bocci, Matthew (Matthew)" <[email-masked]>
Date: Thu, 26 Sep 2013 14:56:43 +0000

Hello,

I have been selected as the Routing Directorate reviewer for this draft.
The Routing Directorate seeks to review all routing or routing-related
drafts as they pass through IETF last call and IESG review, and
sometimes on special request. The purpose of the review is to provide
assistance to the Routing ADs. For more information about the Routing
Directorate, please see
http://www.ietf.org/iesg/directorate/routing.html

Since IETF LC has ended for this draft, I am primarily providing comments to
aid the ADs, and I hope the authors will also take them into consideration to improve
the readability  of the draft.

Document: draft-ietf-l2vpn-vpls-mcast-14.txt
Reviewer: Matthew Bocci
Review Date: 26 September 2013
IETF LC End Date: date-if-known
Intended Status: Proposed Standard

Summary:

I have some minor concerns about this document that I think should be resolved before
publication.

Comments:

The draft appears to be complete and accurate form a technical perspective. However,
the style of the draft and the density of the text make it quite difficult to
read at times. It took several passes of some sections of the document, and returning
to earlier sections, to properly follow the text. I believe this is because the basic
multicast architecture could be better-explained earlier in the draft.

Major Issues:

No major issues found.

Minor Issues:

1) The draft contains both introductory architectural / framework
descriptions, and more detailed specifications of the protocols required
to efficiently support multicast in VPLS.  However, the introductory sections (e.g. the
 overview in section 5) are presented in the form of extremely dense text, and while
 they seem to be trying to explain architectural concepts, they do not use a single
 diagram to help the reader visualise the solutions in the draft. In fact, the first
 figures illustrating the protocol hierarchies and relationships do not appear until the
 label stacks are described in Section 13 on page 43.

 I think inserting some figures in section 5 to illustrate the different multicast
 schemes, and also moving the data forwarding section earlier in the document, would
 greatly help the reader.

2) Cross-references in the text use the title of the referenced section, but do not
include a section number. Personally, I found that this made the draft much more difficult
to navigate than it should be. It would help to add section numbers to each instance of
a cross reference.

Nits:

Abstract, 2nd paragraph, 1st sentence:
s/solution/solutions

Section 3: This section would be clearer of it was in tabular form, in a similar manner
to other RFCs.

You could also add short (one sentence) definitions of 'Inclusive Trees', 'Selective
Trees', and 'Aggregation'.

Sec 4. Introduction: 3rd para:
s/when large/when a large

Sec 4. Introduction: 5th para:
s/RSVP-PE/RSVP-TE

Sec 5.2. BGP-Based VPLS Membership Auto-Discovery: 1st para:
s/that have membership these/that have membership of these

Sec 5.4 Advertising P-Multicast..., 2nd para:
Grammar: The decision to bind a set of VPLS instances or customer multicast group is
local to the ingress PE matter, / The decision to bind a set of VPLS instances or
customer multicast groups is a local matter for the ingress PE ,

Sec 5.4 Advertising P-Multicast..., 3rd para:
s/be included. / be included

Section 5.5 Aggregation:

The following algorithm is hard to parse:
+ ((Average number of VPLS instances on a PE / Aggregation Factor)
       * number of PEs) / (Average number of P-Multicast trees that
       transit a given P router)

I think this could be restated in a more readable manner. Just as an example:
      AveVPLS            NPE
      -------    X    --------
        Aggr          AvePTree

    Where: AveVPLS is the average number of VPLS instances on a PE
           Aggr is the aggregation factor
           NPE is the number of PEs
           AvePTree is the average number of P-multicast that transit a given P-router

Section 6: Inter-AS Inclusive P-Multicast..., 5th Paragraph
This paragraph mentions FEC128 and FEC 129. It would be better to call these by their
formal names used in RFC4447 i.e. The PWid FEC and the Generalized PWid FEC

---

From: Yakov Rekhter <[email-masked]>
Date: Sun, 29 Sep 2013 17:07:21 -0700

Matthew,

> Since IETF LC has ended for this draft, I am primarily providing comments t=
> o
> aid the ADs, and I hope the authors will also take them into consideration =
> to improve

Thanks for your review and comments.

> I have some minor concerns about this document that I think should 
> be resolved before publication.
> 
> Comments:
> 
> The draft appears to be complete and accurate form a technical perspective.=
> However,
> the style of the draft and the density of the text make it quite difficult =
> to
> read at times. It took several passes of some sections of the document, and=
> returning
> to earlier sections, to properly follow the text. I believe this is because=
> the basic
> to efficiently support multicast in VPLS.  However, the introductory sections
> (e.g. the overview in section 5) are presented in the form of extremely 
> dense text, and while
> they seem to be trying to explain architectural concepts, they do not use 
> a single diagram to help the reader visualise the solutions in the draft. 
> In fact, the first
>  I think inserting some figures in section 5 to illustrate the different 
>  multicast schemes, and also moving the data forwarding section earlier in 
>  the document, would greatly help the reader.

Wrt "inserting some figures in section 5", I would be glad to do this,
provided you elaborate a bit on what specific figures you would like
to insert. 

Wrt moving the data forwarding section earlier in the document,
the placement of the data forwarding section follows the structure
of rfc6513.

> 2) Cross-references in the text use the title of the referenced section, bu=
> t do not
> include a section number. Personally, I found that this made the draft much=
>  more difficult
> to navigate than it should be. It would help to add section numbers to each=
>  instance of
> a cross reference.

This will be fixed as part of the RFC Editor process.

> Abstract, 2nd paragraph, 1st sentence:

Fixed.

> Section 3: This section would be clearer of it was in tabular form, 
> in a similar manner to other RFCs.

I would prefer to keep it in the present form, as there are plenty
of other RFCs that have similar form.
  
> You could also add short (one sentence) definitions of 'Inclusive Trees', '=
> Selective Trees', and 'Aggregation'.

Will do.

> s/when large/when a large

Fixed.

> Sec 4. Introduction: 5th para:

Fixed.

> s/that have membership these/that have membership of these

Fixed.

> Grammar: The decision to bind a set of VPLS instances or customer multicast=
>  group is
> local to the ingress PE matter, / The decision to bind a set of VPLS instan=
> ces or

Fixed.

> s/be included. / be included

Fixed.

> I think this could be restated in a more readable manner. Just as an exampl=
> e:
>       AveVPLS            NPE
>       -------    X    --------
>         Aggr          AvePTree
> 
>            AvePTree is the average number of P-multicast that transit a giv=
en P-router

Will fix.

> This paragraph mentions FEC128 and FEC 129. It would be better to call thes=
> e by their

Will fix.

Yakov.