Hi Glenn,

Thanks for the reviews on the other mvpn-related MIB (draft-ietf-bess-l2l3-vpn-mcast-mib-03).

We would like to have an idea of what is the progress around this draft.
Do you know when you could do a review of the revision posted in early March ?

Thanks in advance,

-Thomas


2017-03-01, Hiroshi Tsunoda:
Dear Glenn,

I posted a new revision MVPN-MIB document.
This revision addresses mainly editorial parts of your comments.
Although so many TBDs have remained, I hope that this becomes
the re-starting point to finalize this document.

URL:
https://www.ietf.org/id/draft-ietf-bess-mvpn-mib-03.txt
Status:
https://datatracker.ietf.org/doc/draft-ietf-bess-mvpn-mib/
Htmlized:
https://tools.ietf.org/html/draft-ietf-bess-mvpn-mib-03
Diff:
https://www.ietf.org/rfcdiff?url2=draft-ietf-bess-mvpn-mib-03

Please see some notes below.

1.  Abstract:
1.1 Please give the full expansion of MPLS/BGP

Fixed.

1.2 "In particular, it describes managed objects to configure and/or
     monitor Multicast in MPLS/BGP IP VPNs (MVPN) on a router."
    Is this for any router or, a "Provider Edge" router ?
    Please fix accordingly.

This point will be fixed in the next revision.

2.  Introduction
2.1 PE - appears first time. Please give the full expansion.

Fixed.

2.2 Is the protocol for "exchanging VPN multicast" or
     "exchanging VPN multicast states"? Please fix appropriately.

"exchanging VPN multicast states" is correct. Fixed.

2.3 The expression "standard MIB" in the following is confusing:
    "This document defines a standard MIB for MVPN-specific
    objects that are generic to both PIM-MVPN and BGP-MVPN."
    Is there any special significance in the "standard" above?
    If no, then please drop the word.

Fixed. I dropped the word "standard".

    Are the objects "generic" to PIM-MVPN and BGP-MVPN or "common"
    to  PIM-MVPN and BGP-MVPN ? Please change accordingly.

This point will be fixed in the next revision.

2.4 Please give the full expansion of the abbreviations occuring
     for the first time in the document. (MPLS, L3VPN).

Fixed.

2.5 The terminology section is a bit terse. Explaining the terms
    that are used, with reference to the protocol documents will
    improve readability.
    e.g.
     - MVPN, PE, PMSI/tunnels,
     - C-multicast routing exchange protocol (PIM or BGP),
       C-multicast states
     - I-PMSI, S-PMSI, provider tunnels

Partially fixed. I will give more detailed explanation in the next revision.

3.  MVPN MIB.
    This gives the overview of the MVPN MIB.
    The MIB module aims to provide "configuring and/or monitoring"

3.1 In
     "This MIB enables configuring and/or monitoring of MVPNs on PE
     devices: the whole multicast VPN machinery....."
    "the whole multicast VPN machinery" is very difficult to define.
    Please use precisely defined terms.
3.2 In "To represent them,...."
    "them" seems ambiguous, please clarify.

These points will be fixed in the next revision.

3.3 The diagram needs some explanation.
    What do the boxes represent? Tables ? The labels are meant to be
    table names ? The table names do not match the labels.
    What do the arrows signify? Please explain.

Partially fixed. Each box represents a table defined in this document.
I fixed the labels in the boxes.

More detailed explanation will be added in the next revision.

3.4 The short explanation of the tables could be augmented with some
    information on what they represent and an idea of how they will
    be used. ( RFC 4382 provides a good example).

These points will be fixed in the next revision.

MIB definitions:
4. MIB syntax checking:
   smilint -s -e -l 5 mibs/MCAST-VPN-MIB 2>MCAST-VPN-MIB.txt

I fixed the most of the warnings except for "index-exceeds-too-large".
I think the restructuring of the MIB module may be required in order to
fix "index-exceeds-too-large".  Thus, I will need more time to work on this.

5. IMPORTS clause
    MIB modules from which items are imported must be cited and included
   in the normative references.
   The conventional style is
     mplsStdMIB
        FROM MPLS-TC-STD-MIB                            -- [RFC3811]

Fixed.

6. Please update the MODULE-IDENTITY. (There are no syntantic errors.)
6.1 'ORGANIZATION "IETF Layer-3 Virtual Private
                  Networks Working Group."'
    needs to be fixed to
    'ORGANIZATION "IETF BESS Working Group."'
    or something more appropriate.

Fixed.

6.2 CONTACT-INFO
    Following the conventions (including indentation style) will
    improve the readability. (e.g. RFC4382, RFC5132).

Fixed.

6.2 REVISION clause: follow the convention of RFC4131 sec 4.5
          REVISION    "200212132358Z"  -- December 13, 2002
          DESCRIPTION "Initial version, published as RFC yyyy."
   -- RFC Ed.: replace yyyy with actual RFC number & remove this note:

Fixed.

6.3 OID assignment: follow the convention of RFC4131 sec 4.5
    replace
          ::= { experimental 99 } -- number to be assigned
    by
          ::= { <subtree> XXX }
   -- RFC Ed.: replace XXX with IANA-assigned number & remove this note
   <subtree> will be the subtree under which the module will be
   registered.

Fixed.

7. Wherever possible, please provide references for objects used in the
    MIB. The references will point to specific sections/sub-sections of
    RFCs defining the protocol for which the MIB is being designed.

This will be addressed in the next revision.

8. MOs.
8.1 Scalar Objects: The name mvpnMvrfNumber may be misleading.
    I would recommend the usage of the same naming style
    as RFC 4382 e.g. mvpnMvrfs, mvpnV4Mvrfs, mvpnV6Mvrfs (instead of
    mvpnMvrfNumber, mvpnMvrfNumberV4, mvpnMvrfNumberV6, ...) unless
    there is some good reason to not do it.

Fixed.

8.2 mvpnMvrfNumber OBJECT-TYPE
       SYNTAX         Unsigned32
       DESCRIPTION
           "The total number of MVRFs that are present on this device,
            whether for IPv4, IPv6, or mLDP C-Multicast."
    o Please make the description precise. E.g. if it is the sum of
      IPv4 MVRFs, IPv6 MVRFs and mLDP C-Multicast MVRFs state it
      explicitly.
    o The expression "present on this device" is used.
      Does "present" imply "configured" MVRFs or "active" MVRFs.
      If it is number of active MVRFs then one would expect that
      the number will vary (increase or decrease). If that is the
      case:
      replace
       SYNTAX        Unsigned32
      by
       SYNTAX        Gauge32

Partially fixed. I replaced Unsigned32 by Gauge32.
I will try to update description in the next revision.

8.3 For all the following scalars:
                            mvpnMvrfNumber
                            mvpnMvrfNumberV4
                            mvpnMvrfNumberV6
                            mvpnMvrfNumberPimV4
                            mvpnMvrfNumberPimV6
                            mvpnMvrfNumberBgpV4
                            mvpnMvrfNumberBgpV6
                            mvpnMvrfNumberMldp
    same comments as 8.2.

Name and SYNTAX of these scalars were fixed.
Description will be fixed in the next revision.

8.4 mvpnGenAddressFamily OBJECT-TYPE
       DESCRIPTION
           "The Address Fammily that this entry is for"
     s/Fammily/Family/

Fixed.

8.5 mvpnGenOperStatusChange OBJECT-TYPE
        SYNTAX      INTEGER { createdMvrf(1),
                              deletedMvrf(2),
                              modifiedMvrfIpmsiConfig(3),
                             modifiedMvrfSpmsiConfig(4)
                            }
       DESCRIPTION
           "This object describes the last operational change that
    o The name does not look right. From the SYNTAX and the DESCRIPTION
      it appears that this is about config or MVRF change rather than
      "OperStatus" change. Please check and fix.
    o Please confirm that the values in the row itself will not be changed
      after creation. ( you do not have a 'modifiedMvrfConfig')

This will be addressed in the next revision.

8.6 mvpnGenCmcastRouteProtocol OBJECT-TYPE
       MAX-ACCESS    read-write
       ::= { mvpnGeneralEntry 4 }
    o You cannot have MAX-ACCESS    read-write for a row that may be
      dynamically created.
      Replace
       MAX-ACCESS    read-write
      by
       MAX-ACCESS    read-create
      if you want to dynamically change that value, otherwise,
       MAX-ACCESS    read-only
      will suffice.

This will be addressed in the next revision.

8.8 mvpnGenIpmsiConfig OBJECT-TYPE
        DESCRIPTION
           "This points to a row in mvpnPmsiConfigTable,
            for I-PMSI configuration."
    o Please specify the expected behaviour when it is not an I-PMSI
8.9 mvpnGenInterAsPmsiConfig
    o same comment as above

These will be addressed in the next revision.

8.10 mvpnGenRowStatus
    mvpnGenRowStatus OBJECT-TYPE
       SYNTAX        RowStatus
       DESCRIPTION
           "This is used to create or delete a row in this table."
    o The description is inadequate for an implementor (and
      others too).
    o You must have a mvpnGenRowStorageType or the DESCRIPTION of
      mvpnGenRowStatus must indicate what will happen to the row
      after an agent restart

I will try to address this comment in the next revision.

9. Similar comments (8.1 ~ 8.10) for the remaining tables in the MIB
    Particularly 8.10 for the RowStatus type objects
                         mvpnGenRowStatus
                        mvpnPmsiConfigRowStatus
                        mvpnSpmsiConfigRowStatus.
   Please check and fix.

I will try to address this comment in the next revision.

10. mvpnMvrfChange NOTIFICATION-TYPE
        OBJECTS     {
                     mvpnGenOperStatusChange
                   }
       ::= { mvpnNotifications 2 }

    o should be  { mvpnNotifications 1 }
    o Include the MOs that the administrator/manager may want to
      see in OBJECTS.

The first comment is addressed, the second one is TBD.

11. The Security Considerations section does not follow the Security
     Guidelines for IETF MIB Modules
     http://trac.tools.ietf.org/area/ops/trac/wiki/mib-security.

I rewrite this part according to the guideline described in RFC4181 Sec.3.4.
However, there are some TBDs in this part that should be updated according
to the update in the main body of MIB module.


12.  COMPLIANCE.
12.1 You seem to mandate MAX-ACCESS read-write/read-create for
      compliance. Is this intended? Configuration capability MUST be
      supported?  Please note that sec 2.  MVPN MIB says
      "This MIB enables configuring and/or monitoring of MVPNs ..."
     The current compliance requirement contradicts the above claim.
     Please check and fix.

     It is general and sound practice to allow for MAX-ACCESS
     read-only compliance. Some implementations may support
     monitoring but not configuration.
     Please check and fix.

This will be addressed in the next reivision.

General nits:
13.1  Page-1  s/an portion/a portion/
13.2  Page-1  s/we'll/we will/
13.3  Page-5  s/ mvpnSpmsiTable\/Etnry/mvpnSpmsiTable/
         I think that the "/Entry" was removed from similar titles
         in the earlier draft as adivised by the document shepherd.
         This one should be removed too.
13.4 ID-nits:

Fixed.

14. There is another WIP L2L3-VPN-MCAST-MIB in the WG.
     Is there a good reason for not merging the 2 documents?
     Some clarification or pointers will be helpful.

In my understanding, L2L3-VPN-MCAST-MIB is designed for both
L2VPN multicast and L3VPN multicast, but MVPN-MIB is designed
only for L3VPN multicast. I think this is a reason why there are
two separate documents.

-- tsuno

2016-06-07 0:44 GMT+09:00 Jeffrey (Zhaohui) Zhang <[email protected]>:
Mach, Glenn,

I've addressed most of the comments from Glenn on the l2l3 mvpn mib and had 
started on addressing some comments on mvpn mib, but I've been side tracked and 
am making very slow progress.

I will try to pick it up again soon.

Thanks.
Jeffrey

-----Original Message-----
From: BESS [mailto:[email protected]] On Behalf Of Mach Chen
Sent: Wednesday, June 01, 2016 3:30 AM
To: Glenn Mansfield Keeni <[email protected]>; Benoit Claise
<[email protected]>; EXT - [email protected]
<[email protected]>
Cc: Jeffrey (Zhaohui) Zhang <[email protected]>; [email protected]; Martin
Vigoureux <[email protected]>; [email protected]
Subject: Re: [bess] MIBDoc review of draft-ietf-bess-mvpn-mib-02.txt

Hi authors,

I saw the discussions between you and Glenn about l2l3 mvpn mib and you
have already submitted the 02 version to address the comments.

But I did not see any response to the below mvpn mib review and
comments(maybe I missed something), given that we have plan to progress
the mvpn-mib and l2l3-mvpn-mib documents together, what's your plan about
this document?

Best regards,
Mach

-----Original Message-----
From: Glenn Mansfield Keeni [mailto:[email protected]]
Sent: Wednesday, April 13, 2016 12:04 PM
To: Benoit Claise; [email protected]
Cc: [email protected]; Martin Vigoureux; Mach Chen; Jeffrey (Zhaohui)
Zhang;
[email protected]
Subject: MIBDoc review of draft-ietf-bess-mvpn-mib-02.txt

Hi,
I have been asked to do a MIB Doctors review of
draft-ietf-bess-mvpn-mib-02.txt.

The comments are attached.
You will note that this is preliminary review. There are some generic
comments
which apply to all the scalars and tables. Please take care of those and
then we
will get onto the next phase.

Hope this helps.


Glenn

_______________________________________________
BESS mailing list
[email protected]
https://www.ietf.org/mailman/listinfo/bess

_______________________________________________
BESS mailing list
[email protected]
https://www.ietf.org/mailman/listinfo/bess


_________________________________________________________________________________________________________________________

Ce message et ses pieces jointes peuvent contenir des informations 
confidentielles ou privilegiees et ne doivent donc
pas etre diffuses, exploites ou copies sans autorisation. Si vous avez recu ce 
message par erreur, veuillez le signaler
a l'expediteur et le detruire ainsi que les pieces jointes. Les messages 
electroniques etant susceptibles d'alteration,
Orange decline toute responsabilite si ce message a ete altere, deforme ou 
falsifie. Merci.

This message and its attachments may contain confidential or privileged 
information that may be protected by law;
they should not be distributed, used or copied without authorisation.
If you have received this email in error, please notify the sender and delete 
this message and its attachments.
As emails may be altered, Orange is not liable for messages that have been 
modified, changed or falsified.
Thank you.

_______________________________________________
BESS mailing list
[email protected]
https://www.ietf.org/mailman/listinfo/bess

Reply via email to