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

From: Julien Meuric <[email-masked]>
Date: Fri, 01 Aug 2014 16:18:51 +0200

Hi Dhruv.

Just to be sure we're on the same page: the WG LC on this I-D ended on 
March, 31! All the same, your comments can be taken into account "before 
or alongside" the IETF LC (it's up to the editors).

Thanks for your review,

Julien

Jul. 31, 2014 - Dhruv Dhody:
> 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
>
>
>
>
>
>
>
> _______________________________________________
> Pce mailing list
> [email-masked]
> https://www.ietf.org/mailman/listinfo/pce
>

---

From: Dhruv Dhody <[email-masked]>
Date: Fri, 1 Aug 2014 21:00:52 +0530

Hi julien,

I meant IETF LC. Sorry for the confusion.

Dhruv
On Aug 1, 2014 7:49 PM, "Julien Meuric" <[email-masked]> wrote:

> March, 31! All the same, your comments can be taken into account "before or
> alongside" the IETF LC (it's up to the editors).
>
> Thanks for your review,
>
> Julien
>
>

---

From: Jonathan Hardwick <[email-masked]>
Date: Thu, 14 Aug 2014 11:27:50 +0000

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:[email-masked]] On Behalf Of Dhruv Dhody
Sent: 31 July 2014 19:30
To: [email-masked]
Cc: [email-masked]
Subject: [Pce] Regarding draft-ietf-pce-pcep-mib-09

Hi Authors, 

- General 
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
Jon> OK
  
- Introduction
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

Jon> Not sure I understand - why?

  
Jon> We are removing the range restriction instead.

Jon> OK.

Jon> OK

Jon> OK

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

---

From: Dhruv Dhody <[email-masked]>
Date: Thu, 14 Aug 2014 17:54:46 +0530

Hi Jon,

Trimming the mail to the points which need commenting...

> Jon> Happy to have this - please could you propose some text?

   pcePcepPeerNumCorruptRcvd OBJECT-TYPE
       SYNTAX      Counter32
       MAX-ACCESS  read-only
       STATUS      current
       DESCRIPTION
           "The number of corrupted PCEP message received from this peer. "

       ::= { pcePcepPeerEntry xx }

  pcePcepSessNumCorruptRcvd OBJECT-TYPE
       SYNTAX      Counter32
       MAX-ACCESS  read-only
       STATUS      current
       DESCRIPTION
           "The number of corrupted PCEP message received on this session. "
       ::= { pcePcepSessEntry yy }

> Jon> Not sure I understand - why?

OLD:
"An entry in this table represents a PCEP entity."
NEW:
"This entry represents a PCEP entity."

Just my preference, entirely up to you if you would like to change.

(snip)

Thanks!

Dhruv

---

From: t.petch <[email-masked]>
Date: Fri, 15 Aug 2014 12:32:47 +0100

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

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

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

And does pcePcepSessState need to reflect the various contortions a
shutdown can go through, or all the various wait states of no interest?
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.

Tom Petch

----- Original Message -----
From: "Jonathan Hardwick" <[email-masked]>
To: "Dhruv Dhody" <[email-masked]>
Cc: <[email-masked]>; <[email-masked]>
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.
> 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.

---

From: Jonathan Hardwick <[email-masked]>
Date: Mon, 18 Aug 2014 14:36:08 +0000

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

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

Jon

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

[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]

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

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]

[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

---

From: t.petch <[email-masked]>
Date: Mon, 18 Aug 2014 17:38:04 +0100

At the end ...

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

Jon

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

[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]

[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]

[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]

[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]

<tp>

Jon

There are not that many session protocols to choose from in the IETF but
TCP is richly endowed with information about its sessions, as in RFC4898
and RFC4022, but more humble protocols also tend to follow the practice,
such as draft-ietf-forces-mib or RFC7331 (bfd).

In the current climate, I expect that we should be grateful for anything
in a MIB module:-(

Tom Petch

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

---

From: "Adrian Farrel" <[email-masked]>
Date: Wed, 20 Aug 2014 10:09:18 +0100

> MIBs - can you point me to any examples? [/jon]

A way to handle this is that the peerEntry (which can persist) can have objects
that say "lastFailureTime" and "lastFailureReason" 

A