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

[Jonathan Hardwick <[e-mail]>]

Tom, many thanks for your comments.  See [jon]... [/jon] inline below.
Cheers
Jon


-----Original Message-----
From: t.petch [mailto:[e-mail]] 
Sent: 15 August 2014 12:33
To: Jonathan Hardwick
Cc: [e-mail]; [e-mail]
Subject: Re: [Pce] Regarding draft-ietf-pce-pcep-mib-09

Jon

Picking up two of Adrian's points:

- I would never have known the idea was to have multiple entities in a
single 'box' modelled in the one MIB  module - I think this unusual and
while fine to do it, I would like to see it spelt out in s.4.1.  It is
often a source of confusion what an instance of an 'object class'
actually is, which I think confused Adrian as well as me

[jon] Agreed, I will spell it out more clearly as this has obviously caused some confusion. [/jon]


- on the question of how many supported protocols, there used to be an
enumeration
" ipV4: PceRoutingDomainID is an InetAddressIPv4 ..
  ipV6:  PceRoutingDomainID is an InetAddressIPv6
  nsap:  PceRoutingDomainID type is OCTET STRING (SIZE (0..20)),
               corresponding to the name of an ISIS area
  asNumber:  PceRoutingDomainID type is OCTET STRING (SIZE (2))
               corresponding to the name of an Autonomous System."
but I assume that such concepts are long gone.

[jon] Yes, the TC MIB has expired and is not needed by the current MIB.  I don't think this TC was ever used by the PCEP MIB. [/jon]


A trivial point of my own
 s/estbalished./established./

[jon] Thanks - will fix. [/jon]


And does pcePcepSessState need to reflect the various contortions a
shutdown can go through

[jon] We decided to keep the states in line with RFC 5440 which does not specify any additional states during shutdown (the FSM goes straight to "idle").  It is possible that implementations may use intermediate shutdown states such as "quiescing" but I think these are probably best done in an implementation-specific field which modifies the "sessionUp" state. [/jon] 


, or all the various wait states of no interest?

[jon] I think the wait states are useful. If a session is stuck not coming up, the operator will find it helpful to know if this is because the session is stuck in tcpPending, openWait or keepWait. [/jon]


A sometime practice with other models of sessions is to keep information
around for a while so that it is possible to tell what caused the
shutdown.

[jon] I think that the relevant information (which may be quite detailed) should be available in logs, and that logging interfaces would be better suited to it.  Is it common to provide this type of thing in MIBs - can you point me to any examples? [/jon]


Tom Petch

----- Original Message -----
From: "Jonathan Hardwick" <[e-mail]>
To: "Dhruv Dhody" <[e-mail]>
Cc: <[e-mail]>; <[e-mail]>
Sent: Thursday, August 14, 2014 12:27 PM
Subject: Re: [Pce] Regarding draft-ietf-pce-pcep-mib-09


> Hi Dhruv
>
> Thanks for the comments - see replies below (Jon> ...).  I'll make
sure these are addressed in the next revision.
>
> Cheers
> Jon
>
> -----Original Message-----
> From: Pce [mailto:[e-mail]] On Behalf Of Dhruv Dhody
> Sent: 31 July 2014 19:30
> To: [e-mail]
> Cc: [e-mail]
> Subject: [Pce] Regarding draft-ietf-pce-pcep-mib-09
>
> 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.
> Jon> OK.
>
>   ~ PCEP speaker and PCEP entity are used interchangeably, perhaps we
can
>     unify?
> Jon> The aim is to use PCEP entity when referring specifically to a
local entity (that is, one that appears in the pcePcepEntityTable) and
PCEP speaker to refer more generally to any device that is speaking
PCEP.  I'll re-review and make sure there is consistency here.
>
>   ~ 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).
> Jon> Happy to have this - please could you propose some text?
>
> - Abstract
>   Add MIB as the abbreviation
> Jon> OK
>
> - Introduction
>   Add TE as the abbreviation for Traffic Engineering
> Jon> OK
>
> - Shouldn't Section 3 'Requirements Language' about RFC2119 keywords
>   be part of the introduction itself?
> Jon> This boilerplate text is usually in its own section in my
experience.
>
> - 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.
>
> Jon> Not sure I understand - why?
>
>    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?
>
> Jon> We are removing the range restriction instead.
>
>   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.
>
> Jon> OK.
>
>    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.
>
> Jon> OK
>
>
>    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
>
> Jon> OK
>
> - 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.
>
> Jon> How about the following change (plagiarising text from RFC 6825):
>
> OLD
>
>    It is RECOMMENDED that implementers consider the security features
as
>    provided by the SNMPv3 framework (see [RFC3410], section 8),
>    including full support for the SNMPv3 cryptographic mechanisms (for
>    authentication and privacy).
>
> NEW
>
>    Implementations MUST provide the security features described by the
>    SNMPv3 framework (see [RFC3410]), including full support for
>    authentication and privacy via the User-based Security Model (USM)
>    [RFC3414] with the AES cipher algorithm [RFC3826].  Implementations
>    MAY also provide support for the Transport Security Model (TSM)
>    [RFC5591] in combination with a secure transport such as SSH
>    [RFC5592] or TLS/DTLS [RFC6353].
>
>
> Thank You!
>
> Dhruv
>
>
>
>
>
>
>
> _______________________________________________
> Pce mailing list
> [e-mail]
> https://www.ietf.org/mailman/listinfo/pce
>
> _______________________________________________
> Pce mailing list
> [e-mail]
> https://www.ietf.org/mailman/listinfo/pce