Hi Tsunoda,
> I have started to volunteer to help to move this document forward.
Great!
> I posted a new revision and addressed all editorial things in
> that revision.
   Got this. Looks good.
> Please give me some more time for revising other parts,
No problems. Will be looking forward to the revised document.

Glenn

On 2016/12/02 12:12, Hiroshi Tsunoda wrote:
Dear Glenn,

Thanks for your careful review and detailed comments/suggestions.
I have started to volunteer to help to move this document forward.
I posted a new revision and addressed all editorial things in that revision.
Please give me some more time for revising other parts,
in order to be familiar with the context of the original and related documents.

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

Please see some notes below.

0. Abstract.
0.1.
 it describes common managed objects used to configure
   and/or monitor both L2 and L3 VPN Multicast.

There are no writable MOs in this MIB. So it does not look
as though this MIB will be used for configuration directly.
The use case scenario for monitoring is not clear, either.
It appears that the MIB module(s) in this document will be
used by other modules which are designed for monitoring and/
or configuring L2 and L3 VPN Multicast. Please re-examine the
wording.

Fixed.

1.  Introduction

1.1
   Would be very nice if a short explanations of MVPN and
   L2 VPN Multicast were given. With emphasis on the operational
   aspects.

TBD. Please give me some more time to revise.

1.2
   s/referred to MVPN and L2 VPN Multicast respectively/
     referred to as MVPN and L2 VPN Multicast,respectively/

Fixed.

1.3
   s/MVPN [RFC6513] [RFC6514]/MVPN [RFC6513],[RFC6514]/.

Fixed.

1.4 .... there are 2 types of PMSIs ..

  o I-PMSI: Inclusive PMSI - to all PEs in the same VPN.
  o S-PMSI: Selective PMSI - to some of the PEs in the same VPN.

   please make these explanations more gentle(complete) to the reader.
   Also, give the references where these terms are defined.

TBD. Please give me some more time to revise.

3.  Summary of MIB Module
3.1
  Attributes (PTAs) advertised/received in I/S-PSMI Auto-Discovery
    Typo: I/S-PMSI,  (see 3.3 below).

Fixed.

3.2 some more text like the following will be good.
    L2L3-VPN-MCAST-MIB contains
    o a Textual Convention L2L3VpnMcastProviderTunnelType that provides
      an enumeration of the  provider tunnel types and,
    o a table l2L3VpnMcastPmsiTunnelAttributeTable. The table index is
      composed of multiple attributes that depend on the tunnel type and
      uniquely identify a tunnel. This table will be used to ... monitor
      the tunnels supported by the system at a given point of time (?)
      It may also be used in conjunction with XXXX-mib to obtain the
      other details of a tunnel by following the row pointer of the
      corresponding tunnel's row in this table.
    [ Please treat the above as a template and modify the text as
      appropriate ..]

TBD. Please give me some more time to revise this point.

3.3 Since this will become a standard document, please take care of
    definitions and notations used in the document.
    The notation I/S-PMSI is not defined. If you must use a new
    term/notation,  define it before use.

TBD. Please give me some more time to revise this point.

4.  Definitions

 IMPORTS
   MODULE-IDENTITY, OBJECT-TYPE, experimental
4.1 Since this is not a Experimental MIB do not import use experimental.
    It is good practice to keep the draft in the as "close to final form"
    as possible. (See below)

Fixed.

4.2
  LAST-UPDATED "201310141200Z"  -- October 14, 2013
    Please update this date.

Updated.

4.3
  DESCRIPTION
   "This MIB contains common managed object definitions for
    multicast in Layer 2 and Layer 3 VPNs, defined by
    [RFC7117] and [RFC6513] [RFC6514] respectively.
    Would be good if you could rearrange the text. Something like
     "This MIB module will be used for managing multicast in Layer 2
      VPNs [RFC7117] and Layer 3 VPNs [RFC6513], [RFC6514].
    Or, even better
     "This MIB module will be used by other MIB modules designed for
      managing multicast in Layer 2 VPNs [RFC7117] and Layer 3 VPNs
      [RFC6513], [RFC6514]
    Or, a combination of both, depending on the envisaged use case
    scenarios.

Rearranged the text along with your comment.

4.4
   ::= { experimental 999 }
    Please
      o Replace "experimental" by the branch where this mib module will
        be anchored; that is a decision that the WG will take, probably.
      o Import the branch in the IMPORTS statement
      [ In the IANA Considerations section a branch in the mib-2 subtree
        is requested. In that case this must be
         ::= { mib-2 XXX }
      ]

Fixed.

4.5
  -- Please also remove the ", experimental" text from earlier
  -- IMPORTS section.
    Remove these instructions.

Removed.

4.5.2
 -- Texual convention
   Typo: -- Textual convention

Fixed.

4.6
L2L3VpnMcastProviderTunnelType ::= TEXTUAL-CONVENTION
  DESCRIPTION
      "Types of provider tunnels used for multicast in
       BGP/MPLS L2 or L3 VPN. Additional types may be defined
       in future RFCs, and those will be allowed as
       valid types for L2L3VpnMcastProviderTunnelType."
    The part
                              Additional types may be defined
       in future RFCs, and those will be allowed as
       valid types for L2L3VpnMcastProviderTunnelType."
    may be deleted.

Deleted.

4.7
-- Top level components of this MIB.
-- tables, scalars, conformance information

l2L3VpnMcastObjects     OBJECT IDENTIFIER ::= { l2L3VpnMcastMIB 1 }
l2L3VpnMcastConformance OBJECT IDENTIFIER ::= { l2L3VpnMcastMIB 2 }
  l2L3VpnMcastStates  OBJECT IDENTIFIER ::= { l2L3VpnMcastObjects 1 }

 -- Table of PMSI Tunnel Attributes

l2L3VpnMcastPmsiTunnelAttributeTable OBJECT-TYPE

should be

  -- Top level components of this MIB.

  l2L3VpnMcastObjects     OBJECT IDENTIFIER ::= { l2L3VpnMcastMIB 1 }
  l2L3VpnMcastConformance OBJECT IDENTIFIER ::= { l2L3VpnMcastMIB 2 }
  l2L3VpnMcastStates      OBJECT IDENTIFIER ::= { l2L3VpnMcastObjects 1 }

  -- tables, scalars, conformance information
  -- Table of PMSI Tunnel Attributes

  l2L3VpnMcastPmsiTunnelAttributeTable OBJECT-TYPE

Fixed.

4.8
l2L3VpnMcastPmsiTunnelAttributeTable OBJECT-TYPE
   SYNTAX        SEQUENCE OF L2L3VpnMcastPmsiTunnelAttributeEntry
   MAX-ACCESS    not-accessible
   STATUS        current
   DESCRIPTION
       "This table is for PMSI Tunnel Attributes (PTAs)
        advertised/received in I/S-PSMI Auto-Discovery routes.
        The entries may be referred to by I-PMSI or S-PMSI table
        entries defined in other MIBs, e.g. mvpnMIB in
        [I-D.ietf-bess-mvpn-mib]."

  It would seem that each row in this table is an index for a PTA
  and may contain pointers to rows in tables of other MIB modules
  which may contain more details for the PTA. Is that correct?
  Please reword the DESCRIPTION acordingly.
  Also see comments in 4.15

TBD. I need some more time to understand the original context.

4.9
l2L3VpnMcastPmsiTunnelAttributeEntry OBJECT-TYPE
       "An entry in this table corresponds to a PTA
        that is advertised/received on this router.
  We are in the description of "l2L3VpnMcastPmsiTunnelAttributeEntry"
  so "entry in this table" does not fit in well.
  A rewording like
         "A conceptual row corresponding to a PTA
          that is advertised/received on this router.
          ....
  would be better.

Fixed.

4.10
        For BGP-based signaling (for I-PMSI via auto-discovery
        procedure, or for S-PMSI via S-PMSI A-D routes),
        they are just as signaled by BGP.
        For UDP-based S-PMSI signaling for PIM-MVPN,
        they're derived from the S-PMSI Join Message.

        Note that BGP-based signaling may be used for
        PIM-MVPN as well."
   Is the signaling mechanism important here? If it isn't then the
   above part of the description is redundant.

Removed the above part.

4.10-2
  PIM-MVPN appears for the first time.

Defined the notation of PIM-MVPM as follows
  Protocol Independent Multicast - MVPN (PIM-MVPN)
However, I think that some descriptions may be required for this
somewhere in this document. That is TBD.

4.10-3
  the phrase UDP-based S-PMSI appears here for the first time.
  Somewhere earlier it should be made clear that UDP too may be used
  in signaling.

TBD.

4.11
  l2L3VpnMcastPmsiTunnelAttributeFlags OBJECT-TYPE
        "For UDP-based S-PMSI signaling for PIM-MVPN, this is 0.
     "this" is unclear.
     Something like "the value of this object is 0"  will be better.

Fixed.

         More bits may be defined in the future and
         they will be registered in IANA Registry xxxx."
  This part is probably redundant.

Removed.

4.12
  -- RFC Ed. replace xxxx with the actual registry name
  -- that is being created via [I-D.ietf-bess-mvpn-mib]
  -- and remove this note.

  Look at the comments in 6.0

The above description ("IANA Registry xxxx.") was removed,
thus this part was also removed.

4.13
  l2L3VpnMcastPmsiTunnelAttributeType OBJECT-TYPE
   DESCRIPTION
       "As defined for L2L3VpnMcastProviderTunnelType.
        For UDP-based S-PMSI signaling for PIM-MVPN,
        this is pim-asm (3), pim-ssm (4), or pim-bidir (5).
        For BGP-based I/S-PMSI signaling, this is the Tunnel Type
        field in PMSI Tunnel Attribute of the corresponding
        I/S-PMSI A-D or Leaf A-D route."
  o Does this description cover all the types? If not, then cover all the
    types unless there is a good reason to focus only on the above types.
  o I/S-PMSI: unexplained notation.

TBD. Please give me some more time to address this point.

4.14

  l2L3VpnMcastPmsiTunnelAttributeId OBJECT-TYPE
   SYNTAX        OCTET STRING ( SIZE (0|4|8|12|17|24|29) )
  It appears that you also allow sizes "16" and "32"; these must be included.

Fixed.

           IPv4/IPv6     l2L3VpnMcastPmsiTunnelAttributeType
  Please indicate that the first column gives the size

I made a change as follows.

                Size        l2L3VpnMcastPmsiTunnelAttributeType
           (IPv4/IPv6)
--------------------------------------------------
                       (snip)
                 8/32       pimAsm
                       (snip)

Is this OK?

              8/32       pimAsm
              8/32       pimSsm
              8/32       pimBidir
              4/16       ingressReplication

        For UDP-based S-PMSI signaling for PIM-MVPN, the first
        8 or 32 octets of this attribute are filled with
        the provider tunnel (source, group) IPv4/IPv6 addresses.
        For BGP-based I/S-PMSI signaling, this is the Tunnel
        Identifier field in PMSI Tunnel Attribute of the
        corresponding I/S-PMSI A-D route."

  A more generous description of the AttributeID would be good. All the
  cases must be covered. Section 5 of RFC 6514 does it nicely. A simple
  summary would be very nice.

TBD. Please give me some more time to revise this point.

4.15
  l2L3VpnMcastPmsiTunnelPointer OBJECT-TYPE
   SYNTAX        RowPointer
   DESCRIPTION
       "If the tunnel exists in some MIB table, e.g. mplsTunnelTable
        [RFC3812], this is the row pointer to it. Otherwise, the
        pointer is null."
  I am having problems understanding this. Will help if you can give
  a use case of how this will be used. As of now the intent is unclear.
  A RowPointer cannot be pointing to "some MIB table". It must be
  pointer to a specific row in a specific table. If this is a pointer to
  a row in the mplsTunnelTable spell it out clearly and unambiguously.

TBD. I will need some more time to understand the original context.

4.16
  l2L3VpnMcastPmsiTunnelIf OBJECT-TYPE
     DESCRIPTION
       "If the tunnel has a corresponding interface, this is the
        row pointer to ifXTable. Otherwise, the pointer is null."
  This description is better.  Would be even better with
         "If the tunnel has a corresponding entry in the ifXTable,
          this object will point to the row pertaining to the entry .....

Fixed.

4.17
  l2L3VpnMcastOptionalGroup    OBJECT-GROUP
    DESCRIPTION
        "Support of these object is not required."
           Support of these objects is not required.

Fixed.

5.0
5.  Security Considerations
   TBD

Still TBD.

6.0
6.  IANA Considerations

 IANA is requested to root MIB objects in the MIB module contained in
 this document under the mib-2 subtree.

   Please Note:
   To make the L2L3VpnMcastProviderTunnelType TC maintainable you need to
   put the definitions in a separate MIB module. That would mean a
   separate  branch in the mib-2 subtree. Then the maintenance of the
   TC can be carried out by some entity ( IANA or, some WG or, whoever is
   responsible for maintaining the TC) independent of other MIB objects.
   If that is the intent you will need to define 2 mib modules and you will
   need to request 2 branches in the mib-2 subtree- one for the module
   containing the L2L3VpnMcastProviderTunnelType TC and another for the
   module containing the l2L3VpnMcastPmsiTunnelAttributeTable.

TBD. I will address this point in the next revision.

2016-06-07 18:39 GMT+09:00 Glenn Mansfield Keeni <gl...@cysols.com>:
Hi Jeffrey,
   Thanks for the good work on draft-ietf-bess-l2l3-vpn-mcast-mib
document. It took me some time to do this review. But now here it
is. A (near complete) review of draft-ietf-bess-l2l3-vpn-mcast-mib-04.txt is
attached. Hope this helps.
   I understand that the Security Considerations section is TBD.

   Glenn

On 2016/05/19 4:48, Jeffrey (Zhaohui) Zhang wrote:

Hi Glenn,

-----Original Message-----
From: Glenn Mansfield Keeni [mailto:gl...@cysols.com]
Sent: Sunday, May 08, 2016 11:02 AM
To: Jeffrey (Zhaohui) Zhang <zzh...@juniper.net>; Benoit Claise
<bcla...@cisco.com>; EXT - thomas.mo...@orange.com
<thomas.mo...@orange.com>
Cc: Mach Chen <mach.c...@huawei.com>; ops-...@ietf.org; Martin Vigoureux
<martin.vigour...@nokia.com>; bess@ietf.org; mib-doct...@ietf.org
Subject: Re: [bess] MIBDoc review of draft-ietf-bess-l2l3-vpn-mcast-mib-
02.txt

Jeffrey,
 > Thanks for your comments. I've addressed most of your comments
 > in the new revision:
Thanks for your cooperation. I will need at least one more revision
with the following comments/recommendations addressed before I will
be able to complete the detailed review. In the following the numbers
refer to the issue numbers in the initial review. The issues that are
addressed and closed are not listed. For brevity, the issue
descriptions have been trimmed. In case of doubts please look at the
response mail appended below.
Hope this helps.


Thanks for your detailed comments/suggestions. I posted a new revision
with the following issues addressed.

URL:
https://www.ietf.org/internet-drafts/draft-ietf-bess-l2l3-vpn-mcast-mib-04.txt
Status:
https://datatracker.ietf.org/doc/draft-ietf-bess-l2l3-vpn-mcast-mib/
Htmlized:
https://tools.ietf.org/html/draft-ietf-bess-l2l3-vpn-mcast-mib-04
Diff:
https://www.ietf.org/rfcdiff?url2=draft-ietf-bess-l2l3-vpn-mcast-mib-04

Please see some notes below.


Glenn

-------------------------------------------------------------------

Comments:

1.1
 >  I had thought this would be standard/obvious for all MIB objects -
We will comeback to this time and again, whereever possible make
matters explicit and clear. That will help.
 >  Is it enough to say something similar? For example:
 >          In particular, it describes common managed objects used
 >          to configure and/or monitor both L2 and L3 VPN Multicast.
That is better.


I take it that this is already closed in -03 revision.


2.2
 >  Having said that, I'll explain PMSI a bit further.
PMSI explanation is good.
Please use the same style/format for I-PMSI and S-PMSI.


I think -03 revision already use the same style/format for I-PMSI and
S-PMSI?


2.3
 >  No difference. I was using "Layer 3" or "L3" but it was pointed out
 > that the layer 3 VPN is often referred to IP VPN in other RFCs and I
 > was advised to change it accordingly. Looks like I did not change all
 > the cases.
 >  On the other hand, I noticed that RFC 4382 does use "Layer 3 VPN" so
 > I'll change it back.
No problems. just make sure that the same expression/notation is used
uniformly.


I take it that this is also addressed in -03 already.

3.
 >  > > 3.  Summary of MIB Module.
 >  > >     An overview of the L2L3-VPN-MCAST-MIB will be good- the
 >  > >     structure of the MIB, short descriptions of the table(s)
 >  > >     including usage of the table(s) for management and/or by
 >  > >     other MIB(s).
 >
 >  I had that, but have added one sentence about the only table.
A sentence or two about the textual convention will be good.


Added in -04.

 >  > > 4. MIB syntax checking:
 >  > >    smilint -s -e -l 5 mibs/L2L3-VPN-MCAST-MIB
2>L2L3-VPN-MCAST-MIB.txt
 >
 >  I used simpleweb's validation tool but looks like I did not use the
 > strictest level of validation. I've now fixed the following issues and
 > verified.
Good.
5.
 >  > >
 >  > > 5. REFERENCE clauses: Please use REFERENCE clauses liberally.
 >  > >    Wherever possible, provide references for objects used in
 >  > >    the MIB. The references will point to specific sections/
 >  > >    sub-sections of the RFCs defining the protocol for which the
 >  > >    MIB is being designed. It will greatly improve the readability
 >  > >    of the document.
 >
 >  Added.
I would recommend using the REFERENCE clause as in rfs4382 and
improve on it.
Specifically, instead of keeping the reference in the DESCRIPTION
clause move it to a separate REFERENCE clause. The addition of the
section number is an improvement. It is friendlier to the reader.
Note. Same comment for other OBJECTs too.


Oh I missed that. All fixed.

7.1
 >  > > 7.1 CONTACT-INFO
 >  > >     Following the conventions (including indentation style) will
 >  > >     improve the readability. (e.g. RFC4382, RFC5132).
 >  > >     Will be good if it does not overflow into the next page.
 >
 >  Fixed.
The format is OK. The Postal address etc., need not have been
deleted. Please put the complete contact information as in the
Author's Address. (RFC 2578 section 5.7 gives a usage example).


Fixed.

7.3
 >  I kept "experimental 99" so that I could continue to use mib tools
 > to validate; but I added notes for the editor to replace them as you
 > indicated.
Use of "experimental 99" is not recommended.


Do you mean 99 is not a good number? What about 9999? As I explained, I
kept it so that we can use mib tools to validate, and I've added detailed
notes for the editor.

8
 >  > > 8. Specific MO and TC related comments.
 >  Are spaces allowed? I don't know so I used hyphen. For now I replace
 > with things like rsvpP2mp.
Yes. Camelcase is an allowed practice. SMI does not mind it.


Ok this is closed already then.

8.2
 >  > > 8.2   l2L3VpnMcastPmsiTunnelAttributeFlags OBJECT-TYPE
 >  The intent is to simply return the octet value of the flags
 > field, w/o listing individual bits like "Leaf Information Required".
 > More bits could be defined in the future but the MIB would not change.
 >
 >  Is that OK?
As far as possible, the meaning of the objects must be made clear.
That will help implementors and operators- users of the MIB.


I added the definition for one existing bit and reference to the IANA
registry being created for this flag field.


8.3
 >  > > 8.3   l2L3VpnMcastPmsiTunnelAttributeId OBJECT-TYPE
 >  Depending on the tunnel type, there could be different sizes.
 > Future tunnel types could have other sizes that not specified
 > today. I was thinking to just give a size
 > tPmsiTunnelAttributeId OBJECT-TYPE range so that it is flexible.
 > Is that ok?
I see that you have changed the size upper limit to 50.
If the size varies continuously from 0 to 50 the above description
is correct.
Please confirm, explain and cite appropriate reference. If the size
may change in the future that must be stated too.


I changed to discrete sizes for currently defined tunnel types.


8.4
 >  > > 8.4  l2L3VpnMcastPmsiTunnelIf OBJECT-TYPE
 >  > >         SYNTAX        RowPointer
 >  > >         MAX-ACCESS    read-only
 >  > >         STATUS        current
 >  > >         DESCRIPTION
 >  > >             "If the tunnel has a corresponding interface,
 >  > >              this is the row pointer to the ifName table."
 >  > >      o DESCRIPTION looks incorrect. Please fix it. Do you
 >  > >        want to say this object points to the corresponding
 >  > >        row in the ifTable?
 >
 >  Yes. Fixed.
Not quite.
    What is ifName table ? ifName is a columnar object in the ifXTable.
    Is l2L3VpnMcastPmsiTunnelIf a pointer to the corresponding row in the
    ifXTable table ? Please fix accordingly.


You're right. Fixed.


9.
 >  > > 9. 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.
 >  > >    Please fix.
 >
 >  I was really hoping that it would not have to be that
 > tedious. SNMP/MIB secur
ity should be no different from the
 > CLI security - once you secure the infrastructure
 > then what's more to do?
 >
 >  I'll need more time to work on this. Let me try to address
 > the issues in the other mib first and come back to this.

Please take your time. Looking at examples will help. And let me
know where I can help.


I will need to work on that later.


10.1
 >  > > 10.1 Checking nits according to
 >  > > http://www.ietf.org/id-info/checklist :
 >  Should I break them into different lines or just keep them
 >  as is? Any example of expected indentation if I break the
 >  lines?
No problems at all to  break lines.
      l2L3VpnMcastGroups      OBJECT IDENTIFIER
                              ::= {l2L3VpnMcastConformance 1}
Should do.


Done.


10.2
 >  > > 10.2 Checking references for intended status: Proposed Standard
 >  > >      == Missing Reference: 'RFC 7117' is mentioned on line 76,
 >  > >          but not defined
 >  > >         'described in [RFC6513, RFC6514, RFC 7117] and other
 >  I hope I understood and fixed it (removing the space in "RFC 7117").
I would recommend that you put it as [RFC6513], [RFC6514], [RFC7117]
That is simpler to parse.


I see some other documents do not have comma between multiple references
so I followed that.


 >  > > 11.  There is another WIP MVPN-MIB in
 >  > >      draft-ietf-bess-mvpn-mib-02.txt
 >  > >      MVPN-MIB has objects that refer to L2L3-VPN-MCAST-MIB.
 >  > >      Is there a good reason for not merging the 2 documents?
 >  > >      I have not seen any discussion or explanation on this.
 >  > >      I may have missed it.
 >  > >      Please clarify or, give some pointers.
 >
 >  As mentioned in the introduction:
 >
 >     this memo describes managed objects common to both VPLS
 >     Multicast [RFC7117] and MVPN [RFC6513, RFC6514].
 >     MVPN-MIB is for MVPN. There was another VPLS Multicast MIB
 >     in the work and both would reference common

 >     objects defined in this MIB.

OK. So you are saying that this MIB contains core objects that
will be used to manage implementations of various multicast VPN
protocols e.g. [RFC7117], [RFC6513],[RFC6514] ? It will help if
you spell it out at the beginning.


Yes. I thought I did it already:

1.  Introduction

   ... and this memo describes managed objects common to both VPLS
   Multicast [RFC7117] and MVPN [RFC6513, RFC6514].

Thanks!
Jeffrey


----------------------------------------------------------------------
On 2016/04/16 21:47, Jeffrey (Zhaohui) Zhang wrote:

Glenn,

Thanks for your comments. I've addressed most of your comments in the

new revision:


URL:            https://www.ietf.org/internet-drafts/draft-ietf-bess-

l2l3-vpn-mcast-mib-03.txt

Status:         https://datatracker.ietf.org/doc/draft-ietf-bess-l2l3-

vpn-mcast-mib/

Htmlized:       https://tools.ietf.org/html/draft-ietf-bess-l2l3-vpn-

mcast-mib-03

Diff:           https://www.ietf.org/rfcdiff?url2=draft-ietf-bess-l2l3-

vpn-mcast-mib-03


Please see below.

1.  Abstract:
1.1 A sentence on how the managed objects will be used by
    applications for operations, monitoring and management
    would be good.


I had thought this would be standard/obvious for all MIB objects - the

read-write ones are used to control how a device works, and the read-only
ones are used for monitoring. Do I really need to say it explicitly?


I see RFC 4382 has the following:

   This memo defines a portion of the Management Information Base (MIB)
   for use with network management protocols in the Internet community.
   In particular, it describes managed objects to configure and/or
   monitor Multiprotocol Label Switching Layer-3 Virtual Private
   Networks on a Multiprotocol Label Switching (MPLS) Label Switching
   Router (LSR) supporting this feature.

Is it enough to say something similar? For example:

        In particular, it describes common managed objects used to

configure

        and/or monitor both L2 and L3 VPN Multicast.


2.  Introduction
2.1 Please give the full expansion of the abbreviations
    appearing for the first time.  (PE, VPLS,..)


Fixed.


2.2 The terminology section is a bit terse. Explaining the
    terms that are used, nicely with reference to the protocol
    documents will improve readability.
    e.g.
     - PMSI, I-PMSI, S-PMSI, provider tunnels


As the paragraph alluded to, this MIB needs to be understood in the

general context of L2/L3 multicast VPN and providing good explanation of
the terms is not attempted. The references for the terms are the the RFCs
for the relevant technologies.


Having said that, I'll explain PMSI a bit further.

2.3 Is there a difference between
       "multicast in Layer 2 and Layer 3 VPNs , defined by
        RFC 7117 and RFC 6513/6514"
    used in the DESCRIPTION in the MODULE-IDENTITY
    and
       "multicast in BGP/MPLS L2 or IP VPN"
    used in the DESCRIPTION of L2L3VpnMcastProviderTunnelType ?
    If these are the same, it will be helpful to stick to the
    same expression. If these are not the same, the dictinction
    should be clarified.


No difference. I was using "Layer 3" or "L3" but it was pointed out that

the layer 3 VPN is often referred to IP VPN in other RFCs and I was
advised to change it accordingly. Looks like I did not change all the
cases.


On the other hand, I noticed that RFC 4382 does use "Layer 3 VPN" so

I'll change it back.




3.  Summary of MIB Module.
    An overview of the L2L3-VPN-MCAST-MIB will be good- the
    structure of the MIB, short descriptions of the table(s)
    including usage of the table(s) for management and/or by
    other MIB(s).


I had that, but have added one sentence about the only table.


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


I used simpleweb's validation tool but looks like I did not use the

strictest level of validation. I've now fixed the following issues and
verified.



   mibs/L2L3-VPN-MCAST-MIB:63: [4] {hyphen-in-label} warning: named

number `rsvp-p2mp' must not include a hyphen in SMIv2

   mibs/L2L3-VPN-MCAST-MIB:64: [4] {hyphen-in-label} warning: named

number `ldp-p2mp' must not include a hyphen in SMIv2

   mibs/L2L3-VPN-MCAST-MIB:65: [4] {hyphen-in-label} warning: named

number `pim-asm' must not include a hyphen in SMIv2

   mibs/L2L3-VPN-MCAST-MIB:66: [4] {hyphen-in-label} warning: named

number `pim-ssm' must not include a hyphen in SMIv2

   mibs/L2L3-VPN-MCAST-MIB:67: [4] {hyphen-in-label} warning: named

number `pim-bidir' must not include a hyphen in SMIv2

   mibs/L2L3-VPN-MCAST-MIB:68: [4] {hyphen-in-label} warning: named

number `ingress-replication' must not include a hyphen in SMIv2

   mibs/L2L3-VPN-MCAST-MIB:69: [4] {hyphen-in-label} warning: named

number `ldp-mp2mp' must not include a hyphen in SMIv2


See later question/comments below.

   mibs/L2L3-VPN-MCAST-MIB:215: [5] {group-unref} warning: current

group `l2L3VpnMcastOptionalGroup' is not referenced in this module

   mibs/L2L3-VPN-MCAST-MIB:4: [5] {import-unused} warning: identifier

`NOTIFICATION-TYPE' imported from module `SNMPv2-SMI' is never used

   mibs/L2L3-VPN-MCAST-MIB:5: [5] {import-unused} warning: identifier

`Unsigned32' imported from module `SNMPv2-SMI' is never used

   mibs/L2L3-VPN-MCAST-MIB:8: [5] {import-unused} warning: identifier

`NOTIFICATION-GROUP' imported from module `SNMPv2-CONF' is never used

   mibs/L2L3-VPN-MCAST-MIB:11: [5] {import-unused} warning: identifier

`TruthValue' imported from module `SNMPv2-TC' is never used

   mibs/L2L3-VPN-MCAST-MIB:11: [5] {import-unused} warning: identifier

`RowStatus' imported from module `SNMPv2-TC' is never used

   mibs/L2L3-VPN-MCAST-MIB:12: [5] {import-unused} warning: identifier

`TimeStamp' imported from module `SNMPv2-TC' is never used

   mibs/L2L3-VPN-MCAST-MIB:12: [5] {import-unused} warning: identifier

`TimeInterval' imported from module `SNMPv2-TC' is never used

   mibs/L2L3-VPN-MCAST-MIB:15: [5] {import-unused} warning: identifier

`SnmpAdminString' imported from module `SNMP-FRAMEWORK-MIB' is never used

   mibs/L2L3-VPN-MCAST-MIB:18: [5] {import-unused} warning: identifier

`InetAddress' imported from module `INET-ADDRESS-MIB' is never used

   mibs/L2L3-VPN-MCAST-MIB:18: [5] {import-unused} warning: identifier

`InetAddressType' imported from module `INET-ADDRESS-MIB' is never used


Removed the above unused imports.


5. REFERENCE clauses: Please use REFERENCE clauses liberally.
   Wherever possible, provide references for objects used in
   the MIB. The references will point to specific sections/
   sub-sections of the RFCs defining the protocol for which the
   MIB is being designed. It will greatly improve the readability
   of the document.


Added.


6. 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]


Added.


7. Please update the MODULE-IDENTITY. (There are no syntantic errors.)
7.1 CONTACT-INFO
    Following the conventions (including indentation style) will
    improve the readability. (e.g. RFC4382, RFC5132).
    Will be good if it does not overflow into the next page.


Fixed.


7.2 REVISION clause: follow the convention recommended in RFC4181
    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.

7.3 OID assignment: follow the convention recommended in RFC4181
    sec 4.5 i
    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.


I kept "experimental 99" so that I could continue to use mib tools to

validate; but I added notes for the editor to replace them as you
indicated.



8. Specific MO and TC related comments.
      L2L3VpnMcastProviderTunnelType ::= TEXTUAL-CONVENTION
        STATUS       current
        DESCRIPTION
            "Types of provider tunnels used for multicast in
             BGP/MPLS L2 or IP VPN."
        SYNTAX       INTEGER { unconfigured (0),
                               rsvp-p2mp (1),
                               ldp-p2mp (2),
                               pim-asm (3),
                               pim-ssm (4),
                               pim-bidir (5),
                               ingress-replication (6),
                               ldp-mp2mp (7)

    o Would be nice to align the enumeration labels with the
      labels in the protocol document RFC 6514 unless there is
      a good reason for not doing so. (You will have to take
      care of the smi compilation errors too; '-' is not allowed ).


Are spaces allowed? I don't know so I used hyphen. For now I replace

with things like rsvpP2mp.

Or could/should I just remove the definitions, so that if a new type is

defined in the future there is no need to update the MIB?



8.1  l2L3VpnMcastPmsiTunnelAttributeEntry OBJECT-TYPE
         SYNTAX        L2L3VpnMcastPmsiTunnelAttributeEntry
         MAX-ACCESS    not-accessible
         STATUS        current
         DESCRIPTION
             "An entry in this table corresponds to an PMSI attribute
              that is advertised/received on this router.
              For BGP-based signaling (for I-PMSI via auto-discovery
              procedure, or for S-PMSI via S-PMSI A-D routes),
              they are just as signaled by BGP (RFC 6514 section 5,
              'PMSI Tunnel attribute').
              For UDP-based S-PMSI signaling for PIM-MVPN,
              they're derived from S-PMSI Join Message
              (RFC 6513 section 7.4.2, 'UDP-based Protocol')..

              Note that BGP-based signaling may be used for
              PIM-MVPN as well."
    o Fix the ".." in "'UDP-based Protocol').." above.
    o Please give the reference for this Table.
      Is it-  "PMSI Tunnel attribute" in RFC 6513 Sec.4  ?
              "PMSI Tunnel attribute" in RFC 6514 Sec.5  ?
               both?
      Any other pointers?


Fixed.


8.2   l2L3VpnMcastPmsiTunnelAttributeFlags OBJECT-TYPE
         SYNTAX        OCTET STRING (SIZE (1))
         MAX-ACCESS    not-accessible
         STATUS        current
         DESCRIPTION
             "For UDP-based S-PMSI signaling for PIM-MVPN, this is 0.
              For BGP-based I/S-PMSI signaling, this is the Flags
              field in PMSI Tunnel Attribute of the corresponding
              I/S-PMSI A-D route."
         ::= { l2L3VpnMcastPmsiTunnelAttributeEntry 1 }
    o  Please confirm that the above is a complete enumeration of the
       types of signalling.
    o  RFC 6514 Sec.5 says that the Flags field indicates
       "Leaf Information Required". That is useful information.
       Please include in the description.


The intent is to simply return the octet value of the flags field, w/o

listing individual bits like "Leaf Information Required". More bits could
be defined in the future but the MIB would not change.


Is that OK?


8.3   l2L3VpnMcastPmsiTunnelAttributeId OBJECT-TYPE
         SYNTAX        OCTET STRING ( SIZE (0..37) )
         MAX-ACCESS    not-accessible
         STATUS        current
         DESCRIPTION
             "For UDP-based S-PMSI signaling for PIM-MVPN, the first
              four or sixteen octets of this attribute are filled with
              the provider tunnel group address (IPv4 or IPv6)..
              For BGP-based I/S-PMSI signaling, this is the Tunnel

Identifier

              Field in PMSI Tunnel Attribute of the corresponding I/S-

PMSI

              A-D route."
    o Check the size specifications. The specs above say it can be
      all sizes 0..37. That is not clear from the DESCRIPTION clause.
    o Fix the ".." in "(IPv4 or IPv6).." above.
    o RFC 6514 Sec 5.  PMSI Tunnel Attribute gives the Tunnel

Identifiers

      for mLDP, PIM-SM, PIM-SSM, BIDIR-PIM,Ingress Replication,MP2MP.
      It appears that the sizes (range) for each case will be
different.
      Please clarify that, and if there are discrete sizes, specify
      accordingly.


Depending on the tunnel type, there could be different sizes. Future

tunnel types could have other sizes that not specified today. I was
thinking to just give a size range so that it is flexible. Is that ok?




8.3  l2L3VpnMcastPmsiTunnelPointer OBJECT-TYPE
        SYNTAX        RowPointer
        MAX-ACCESS    read-only
        STATUS        current
        DESCRIPTION
            "If the tunnel exists in some MIB table, this is the
             row pointer to it."
    o "some MIB table" : specify which MIB table.


I can give an example, like mplsTunnelTable [RFC 3812]. It could be

whatever table that a tunnel may be put into.


    o In what case will the tunnel exist and in what case will it not?


If a device supports mplsTunnelTable and the tunnel is represented
there,

then it exists.


    o What will be the behaviour if the above condition is not

satisfied?


A null pointer should be given.


8.4  l2L3VpnMcastPmsiTunnelIf OBJECT-TYPE
        SYNTAX        RowPointer
        MAX-ACCESS    read-only
        STATUS        current
        DESCRIPTION
            "If the tunnel has a corresponding interface, this is the
             row pointer to the ifName table."
     o DESCRIPTION looks incorrect. Please fix it. Do you want to say
       this object points to the corresponding row in the ifTable?


Yes. Fixed.

     o In what case does the TunnelIf exist and in what case will it

not?


Some tunnels may not have a corresponding interface.

     o What will be expected if the tunnel does not have a

corresponding

       interface?


Null row pointer.


9. 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.
   Please fix.


I was really hoping that it would not have to be that tedious. SNMP/MIB

security should be no different from the CLI security - once you secure
the infrastructure then what's more to do?


I'll need more time to work on this. Let me try to address the issues in

the other mib first and come back to this.




10.ID-nits
10.1 Checking nits according to http://www.ietf.org/id-info/checklist :
     ------------------------------------------------------------------

---------


     ** There are 4 instances of too long lines in the document, the

longest one

        being 3 characters in excess of 72.


I fixed some but there still three too long lines:

     l2L3VpnMcastPmsiTunnelAttributeType
L2L3VpnMcastProviderTunnelType,

  l2L3VpnMcastGroups      OBJECT IDENTIFIER ::= {l2L3VpnMcastConformance

1}

  l2L3VpnMcastCompliances OBJECT IDENTIFIER ::= {l2L3VpnMcastConformance

2}


Should I break them into different lines or just keep them as is? Any

example of expected indentation if I break the lines?



10.2 Checking references for intended status: Proposed Standard
     ------------------------------------------------------------------

---------


     == Missing Reference: 'RFC 7117' is mentioned on line 76, but not
        defined
        'described in [RFC6513, RFC6514, RFC 7117] and other documents

tha...'


I hope I understood and fixed it (removing the space in "RFC 7117").


11.  There is another WIP MVPN-MIB in draft-ietf-bess-mvpn-mib-02.txt
     MVPN-MIB has objects that refer to L2L3-VPN-MCAST-MIB.
     Is there a good reason for not merging the 2 documents? I have not

seen

     any discussion or explanation on this. I may have missed it.

Please

     clarify or, give some pointers.


As mentioned in the introduction:

   this memo describes managed objects common to both VPLS
   Multicast [RFC7117] and MVPN [RFC6513, RFC6514].

MVPN-MIB is for MVPN. There was another VPLS Multicast MIB in the work

and both would reference common objects defined in this MIB.


Thanks!
Jeffrey

-----Original Message-----
From: BESS [mailto:bess-boun...@ietf.org] On Behalf Of Glenn Mansfield
Keeni
Sent: Tuesday, April 12, 2016 2:28 AM
To: Benoit Claise <bcla...@cisco.com>; EXT - thomas.mo...@orange.com
<thomas.mo...@orange.com>
Cc: Jeffrey (Zhaohui) Zhang <zzh...@juniper.net>; ops-...@ietf.org;

Martin

Vigoureux <martin.vigour...@nokia.com>; bess@ietf.org; Mach Chen
<mach.c...@huawei.com>
Subject: [bess] MIBDoc review of draft-ietf-bess-l2l3-vpn-mcast-mib-

02.txt


Hi,
I have been asked to do a MIB Doctors review of
draft-ietf-bess-l2l3-vpn-mcast-mib-02.txt.
My knowledge of L2L3VPN Multicast is limited to the reading
of this document and browsing through the documents referred
to in the draft and bess-wg mailing list archives.( read "shallow").
So some of the doubts and questions may sound trivial or
strange. Please bear with me and help me help you make
this into a better document :-)

The comments are attached.

Glenn


_______________________________________________
BESS mailing list
BESS@ietf.org
https://www.ietf.org/mailman/listinfo/bess





_______________________________________________
BESS mailing list
BESS@ietf.org
https://www.ietf.org/mailman/listinfo/bess



_______________________________________________
BESS mailing list
BESS@ietf.org
https://www.ietf.org/mailman/listinfo/bess

Reply via email to