Last Call Review of draft-ietf-bess-mvpn-bidir-03
review-ietf-bess-mvpn-bidir-03-genart-lc-davies-2015-03-16-00

I am the assigned Gen-ART reviewer for this draft. For background on
Gen-ART, please see the FAQ at

<

http://wiki.tools.ietf.org/area/gen/trac/wiki/GenArtfaq>.

Please resolve these comments along with any other Last Call comments
you may receive.

Document: draft-ietf-bess-mvpn-bidir-03.txt
Reviewer: Elwyn Davies
Review Date: 2015/03/06
IETF LC End Date: 2015/03/18
IESG Telechat date: (if known) -

Summary:


Ready with nits.  I think this draft must have about the highest density 


of acronyms per line of draft of any I have ever read!  In spite of this 


the result seems good, and while I can't claim I am anywhere near 


knowledgeable enough to know if there are any lurking problems, it is 


clearly written and apparently self-consistent. Very well done for such 


a complex and "bitty" story!  It would be extremely useful to 


implementers (and future reviewers) if the authors could bring 


themselves to put together a section pointing out which pieces of the 


three RFCs being updated are affected and by which pieces of this 


document.  If there is anything interesting or useful to say about 


privacy in the context of MVPNs this update gives a venue.  Other than 


that there are some missing terminology cross refs and a few very minor 


typos.




Major issues:
None.

Minor issues:
None.

Nits/editorial comments:
General:


The draft has a lot of scattered small scale updates to the three RFCs 


it updates.  It would be useful to implementers to provide a section 


that cross references the sections of the updated drafts with the 


updating sections in this draft.






General:  Given the update, is there anything useful in the whole MVPN 


sphere worth saying about privacy?




s1.1:


Various pieces of terminology and acronyms are brought over from RFC 


6513 without note and without re-expanding them here.  It would be good 


to at least list what they are and their expansions.  My list so far is:


P-tunnel, PMSI, I-PMSI (and that this includes UI-PMSI and MI-PMSI) and 


S-PMSI




s1.1, Defn of "C-multicast flow or C-flow", para 3: s/the/then/...
OLD:
the some or all of the customer's C-flows
NEW:
then some or all of the customer's C-flows
[I think]

s1.2, para 1:
OLD:
governing the use bidirectional P-tunnels;
NEW:
governing the use of bidirectional P-tunnels;



s1.2.1, 1st bullet: This would be a convenient point to define the mLDP 


(Multipoint LDP) acronym for the MP2MP extended version of LDP.  mLDP is 


used later without definition/expansion.




s1.2.2, para 5:
A reference to Section 4 of RFC 6513 for PE Distinguisher Labels would help.

s1.2.4, 1st para after "Unpartitioned Method" bullet, para 3:


A reference to Section 8 of RFC 6514 for the PE Distinguisher Labels 


attribute would help.



[Repeating the reference in the last para of s3.1.1 would also help].

s1.2.4, "Partitioned Methods" bullet,
OLD:
If a bidirectional P-tunnels are being used
NEW:
If bidirectional P-tunnels are being used

s1.2.4,  2nd and 3rd paras after "Unpartitioned Method" bullet:



It SHOULD be possible to provision this on a per-MVPN basis,


I believe this is an implementation decision rather than a protocol 


requirement in each case: Suggest



s/SHOULD be/is desirable to be able/



s2:  The second paragraph needs to be explicit that it is defining a new 


value pair for the MCAST-VPN NLRI group fields and reference RFC 6625 


and RFC 6514.






s3.1.1, last para/s3.2.1, para 6:  The text below is more or less 


duplicated in these two sections for more or less the same case AFAICS.  


Would it possible/useful to avoid the duplication, especially of the 


parenthetical part?



    When this method is used, I-PMSI and/or S-PMSI A-D
    routes SHOULD NOT contain a PE Distinguisher Labels attribute; if
    such an attribute is present in a received I-PMSI or S-PMSI A-D
    route, it MUST be ignored.  (When we say the attribute is "ignored",
    we do not mean that its normal BGP processing is not done, but that
    the attribute has no effect on the data plane.  It MUST however be
    treated by BGP as if it were an unsupported optional transitive
    attribute.)



s3.2.2, para 4:
OLD:
The PEs are REQUIRED to originate
NEW:
The PEs that are REQUIRED to originate

s3.2.2.1, para 4:
OLD:
the root node address of an MP2MP LSP an IP address
NEW:
the root node address of an MP2MP LSP is an IP address