Synchronous Optical Network/Synchronous Digital Hierarchy (SONET/SDH) Circuit Emulation over Packet (CEP) MIB Using SMIv2
draft-ietf-pwe3-cep-mib-16

I'm entering a 'Yes' ballot because this work is technically sound and useful. However, I found a slew of nits that really should be worked on to make the RFC more valuable.

---

The various write-ups and announcements should be updated to reflect the new responsible AD

---

The double page-throws are a nuisance

---

Section 1

Need to expand CEP on first use

---

Section 2

   The mechanism for structured
   emulation (as outlined in the CEP draft)

Hmmm? do you mean RFC 4842?

---

Section 2

s/Since A SONET/Since a SONET/

---

6.3.  PW-STD-MIB Modules Usage

s/Modules/Module/

---

Section 7

The comments on the IMPORT clauses are welcome, but should not show
in square brackets as they are not references (because the MIB module 
is standalone with section 10.

---

Section 7

You can remove the two notes to the RFC Editor in the IMPORTS
clause as you have already fixed up the RFC numbers yourselves.

---

MODULE-IDENTITY DESCRIPTION CLAUSE

 -- RFC Editor: Please replace yyyy with actual RFC number and
 -- remove this note

I think this is xxxx

---

TEXTUAL CONVENTIONS

I'm a bit disappointed that the TCs defined here don't come with
REFERENCE clauses.

---

PwCepFracAsyncMap, pwCepType, pwCepFracMode, pwCepFracSdhVc4Mode, and
pwCepPerfIntervalReset

Although not a requirement, it is usual for INTEGER enumerations to
start at zero. Sometimes other schemes are used to stay in synch with
protocols - if so, it is nice to say so and give a reference.

---

pwCepEntry

         however change of some objects (for example
         pwCepCfgIndex) during PW forwarding state MAY cause traffic
         disruption.

s/MAY/may/

---

pwCepValidIntervals

Telling us the default value for a read-only object is a little
distracting.

---

pwCepPeerCepOption

How is this object set when the PW is statically provisioned?

---

pwCepCfgIndex

Should indicate what meaning is assigned to the value zero since
zero is not a valid index to pwCepCfgTable

---

pwCepCfgJtrBfrDepth

         The actual jitter buffer MUST be at least twice this
         value for proper operation.
                                       
I think this warrants a REFERENCE

---

pwCepFracSdhVc4Tu3Map1 and similar objects

         "If the first TUG-3 within the VC-4 contains a TU-3, this
          variable must be set to the required mode. "

    DEFVAL { other }

So, I was going to say s/must/MUST/ but since you have a DEFVAL
defined for each case, I don't understand the meaning of the text.

---

pwCepPerfCurrentAbsPtrAdjust. pwCepPerfIntervalAbsPtrAdjust, and
pwCepPerf1DayIntervalAbsPtrAdjust

Are the Description clauses in English?

---

 pwCepPerfIntervalNumber OBJECT-TYPE
    SYNTAX        Integer32 (1..96)
    MAX-ACCESS    not-accessible
    STATUS        current
    DESCRIPTION
        "A number (normally between 1 and 96 to cover a 24 hour
         period) which identifies the interval for which the set
         of statistics is available. The interval identified by 1
         is the most recently completed 15-minute interval and
         the interval identified by N is the interval immediately
         preceding the one identified by N-1. The minimum range of
         N is 1 through 4. The default range is 1 through 32. The
         maximum value of N is 1 through 96."

I'd be interested in the non-normal case given the SYNTAX !

I find the text about ranges clumsy. Anyway, since the object is
not-accessible, it is moot.

---
                                                   
UNITS clauses would be nice in objects like pwCepPerfIntervalTimeElapsed
and pwCepPerfIntervalInPtrAdjustSecs

---

 pwCepPerf1DayIntervalNumber OBJECT-TYPE
    SYNTAX      Unsigned32(1..31)
    MAX-ACCESS  not-accessible
    STATUS      current
    DESCRIPTION
      "History Data Interval number. Interval 1 is the current day
       measurement period, interval 2 is the most recent previous
       day; interval 30 is 31 days ago. Intervals 3..31 are
       optional."
    ::= { pwCepPerf1DayIntervalEntry 1 }

What does "optional" mean in a not-accessible object?

---

pwCepPerf1DayIntervalUASs looks like it needs a Reference clause

---

Section 10.1
As indicated by idnits...                                                                           

The RFC number is missing from the BCP14 reference.

---

This is a placeholder comment to note that the following were not addressed as of version 15 of the document.

Lars Eggert
Comment (2011-02-03)

Section 7., paragraph 54:
>          An agent with CEP capability MUST be capable of supporting
>          at least n intervals. The minimum value of n is 4, the
>          default of n is 32 and the maximum value of n is 96.

  I don't get this. How can you state a MUST requirement for a specific
  value, and then give a range for that value?


SB> I see no text change.

Adrian Farrel

Comment (2011-02-02)



Section 2

   The mechanism for structured
   emulation (as outlined in the CEP draft)

Hmmm? do you mean RFC 4842?


---

Section 7

You can remove the two notes to the RFC Editor in the IMPORTS
clause as you have already fixed up the RFC numbers yourselves.

SB> The Editors note is still there for PWMIB - isn't that an RFC?

---

TEXTUAL CONVENTIONS

I'm a bit disappointed that the TCs defined here don't come with
REFERENCE clauses.

---

PwCepFracAsyncMap, pwCepType, pwCepFracMode, pwCepFracSdhVc4Mode, and
pwCepPerfIntervalReset

Although not a requirement, it is usual for INTEGER enumerations to
start at zero. Sometimes other schemes are used to stay in synch with
protocols - if so, it is nice to say so and give a reference.

SB> Does not seem to be addressed

---

pwCepValidIntervals

Telling us the default value for a read-only object is a little
distracting.

---

pwCepPeerCepOption

How is this object set when the PW is statically provisioned?

---

pwCepPerfCurrentAbsPtrAdjust. pwCepPerfIntervalAbsPtrAdjust, and
pwCepPerf1DayIntervalAbsPtrAdjust

Are the Description clauses in English?

---

 pwCepPerfIntervalNumber OBJECT-TYPE
    SYNTAX        Integer32 (1..96)
    MAX-ACCESS    not-accessible
    STATUS        current
    DESCRIPTION
        "A number (normally between 1 and 96 to cover a 24 hour
         period) which identifies the interval for which the set
         of statistics is available. The interval identified by 1
         is the most recently completed 15-minute interval and
         the interval identified by N is the interval immediately
         preceding the one identified by N-1. The minimum range of
         N is 1 through 4. The default range is 1 through 32. The
         maximum value of N is 1 through 96."

I'd be interested in the non-normal case given the SYNTAX !

I find the text about ranges clumsy. Anyway, since the object is
not-accessible, it is moot.

---

pwCepPerf1DayIntervalUASs looks like it needs a Reference clause