[bess] Shepherd review on draft-ietf-bess-mvpn-mib-01

[Mach Chen <[e-mail]>]

Hi Authors,

I am requested (by the WG chairs) to shepherd this draft, here are my shepherd review comments on this document.


1. To make the document more readable, expect for the well-known abbreviations, there are a lot of abbreviations (e.g., MVRF, I-/S-PMSI, etc.) that should be expanded when first use. 

2. Abstract and introduction section

s/ multicast in MPLS/BGP-based Layer-3 VPN (MVPN)/ Multicast in BGP/MPLS in IP VPN (MVPN),  to align with the definition in RFC6513

3.
Section 2.1

   - mvpnIpmsiTable/Entry

     This table contains all advertised or received intra-as I-PMSIs.
     With PIM-MVPN, it is applicable only when BGP-Based Autodiscovery
     of MVPN Membership is used.

   - mvpnInterAsIpmsiTable/Entry

     This table contains all advertised or received inter-as I-PMSIs.
     With PIM-MVPN, it is applicable only when BGP-Based Autodiscovery
     of MVPN Membership is used.
1) For each table, why is there a "/Entry" followed?  To avoid confusion, I'd suggest to remove them if there is no special intention or meaning. 

2 )In addition, from the above text, I have the feeling that a mvpnIpmsiTable or mvpnInterAsIpmsiTable can only contain advertised or received I-PMSIs, but cannot contain both. Is this the intention?


4. Section 2.2, the LAST-UPDATED, ORGANIZATION, CONTACT-INFO and the Revision history should be updated to reflect the latest status. 

5. mvpnMvrfNumber OBJECT-TYPE
     SYNTAX        Unsigned32
     MAX-ACCESS    read-only
     STATUS        current
     DESCRIPTION
         "The total number of MVRFs for IPv4 or IPv6 or mLDP
          C-Multicast that are present in this device."

Should the "or" be changed to "and"?

6.
There are several places in draft say the following or similar:
"An entry in this table is created for every MVRF in the device."
I'd suggest to replace the "every" as "each".

7.
Page 10:
mvpnGenCmcastRouteProtocol OBJECT-TYPE
     SYNTAX        INTEGER { pim (1),
                             bgp (2)
                           }
     MAX-ACCESS    read-write
     STATUS        current
     DESCRIPTION
         "Protocol used to signal C-multicast states across the
          provider core.

s/ Protocol /The protocol is

8.
Page 13:
mvpnBgpGenVrfRtImport OBJECT-TYPE
     SYNTAX             MplsL3VpnRouteDistinguisher
     MAX-ACCESS         read-write
     STATUS             current
     DESCRIPTION
         "The VRF Route Import Extended Community that this device
          adds to unicast vpn routes that it advertises for this mvpn."
     ::= { mvpnBgpGeneralEntry 2}

  mvpnBgpGenSrcAs      OBJECT-TYPE
     SYNTAX            Unsigned32
     MAX-ACCESS        read-only
     STATUS            current
     DESCRIPTION
         "The Source AS number in Source AS Extended Community that this
          device adds to the unicast vpn routes that it advertises for
          this mvpn."

Why should the "Source", "Extended", and "Community" be upper case? If there is no special intention, suggest to change to lower case. You may need to look through the whole document to do a text clean up. 
 
9.
Section 3, Security consideration.

Given that the document introduces some read-write objects, I don't think that the current statement of "This document does not introduce new security risks." will pass the IESG review. I'd suggest the authors to enhance the security consideration section. For the mib security consideration, you may refer to some existing mid documents (e.g, https://tools.ietf.org/html/draft-ietf-mpls-tp-oam-id-mib-11) that have already passed the IESG review. Also, you may refer to some history discussions on a mib security consideration (e.g., https://datatracker.ietf.org/doc/draft-ietf-trill-oam-mib/history/ ).

10.Please make sure that the MIB Modules are compiled cleanly.

11. BTW, I saw the WG chairs had issued the IPR poll, and only Jeffery replied, to progress this draft, the authors and contributors have to respond to the IPR poll.


Happy Holidays! 

Best regards,
Mach