Subject: [Pce] Regarding draft-ietf-pce-pcep-mib-09

From: Dhruv Dhody <[email-masked]>
Date: Thu, 31 Jul 2014 18:30:04 +0000

Hi Authors, 

I re-read the MIB document in preparation for the last call. 
You may consider these comments/nits before or alongside the WG last call. 

- General 
  ~ Expand PCReq, PCRep, PCNtf, SVEC, RP etc on first use, using terminology
    Section may also be useful. 
  ~ PCEP speaker and PCEP entity are used interchangeably, perhaps we can 
    unify?
  ~ a new object for corrupted messages (note that corrupted messages are 
    different from unknown messages and this cannot be derived from number of
    error messages sent either).  	

- Abstract
  Add MIB as the abbreviation
  
- Introduction
  Add TE as the abbreviation for Traffic Engineering  

- Shouldn't Section 3 'Requirements Language' about RFC2119 keywords 
  be part of the introduction itself? 
 
- Section 5.1
   pcePcepEntityEntry OBJECT-TYPE
       SYNTAX      PcePcepEntityEntry
       MAX-ACCESS  not-accessible
       STATUS      current
       DESCRIPTION
           "An entry in this table represents a PCEP entity."
       INDEX       {  pcePcepEntityIndex  }
       ::= { pcePcepEntityTable 1 }    

  ~ I think the description should not say 'this table' while describing 
    an entry. Also true for pcePcepSessEntry.
  
   pcePcepEntityIndex OBJECT-TYPE
       SYNTAX      Unsigned32 (1..2147483647)
       MAX-ACCESS  not-accessible
       STATUS      current
       DESCRIPTION
           "This index is used to uniquely identify the PCEP entity."
       ::= { pcePcepEntityEntry 1 }

  ~ Wouldnt Integer32 (1..2147483647) be a better fit?
  
  Suggest to reorder pcePcepEntityMaxKeepAliveTimer, 
  pcePcepEntityMaxDeadTimer, pcePcepEntityAllowNegotiation,
  pcePcepEntityMinKeepAliveTimer, pcePcepEntityMinDeadTimer 
    
  ~ by moving the pcePcepEntityAllowNegotiation first, you can use it in 
    the description for both max and min timers. 
   
   pcePcepEntitySyncTimer OBJECT-TYPE
       SYNTAX      Unsigned32 (1..65535)
       UNITS       "seconds"
       MAX-ACCESS  read-only
       STATUS      current
       DESCRIPTION
           "The value of SYNC timer is used in the case of synchronized
            path computation request using the SVEC object...

  ~ Use SyncTimer (as used in 5440) instead of SYNC timer.
  
   pcePcepPeerNumSessSetupFail OBJECT-TYPE
       SYNTAX      Counter32
       MAX-ACCESS  read-only
       STATUS      current
       DESCRIPTION
           "The number of PCEP sessions with the peer that have been
            attempted but failed before being fully estbalished.
            This counter is incremented each time a session with this
            peer fails before reaching session state pceSessionUp."
       ::= { pcePcepPeerEntry 8 }
       
  ~  the state is called sessionUp (refer pcePcepSessState) and not 
     pceSessionUp
  
- Security Considerations
  You might think of removing the text about SET operation, as this 
  MIB is read-only. 

  You might also add reference to SNMPv3 security like USM with AES as 
  well to use of secure transport like SSH or TLS/DTLS. 

Thank You! 

Dhruv