[Gen-art] Gen-art LC review of draft-ietf-bess-mvpn-bidir-03

[Elwyn Davies <[e-mail]>]

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