Subject: [MIB-DOCTORS] FW: MIB Doctor review for: draft-ietf-pwe3-pw-atm-mib-04.txt

From: "Bert Wijnen - IETF" <[email-masked]>
Date: Mon, 28 Apr 2008 16:22:24 +0200

I am basically happy with revision 4 of the document.
But I still get:

  E: f(pwAtm.mi2), (797,23) Index item "pwAtmPerfIntervalNumber"
     must be defined with syntax that includes a range
  E: f(pwAtm.mi2), (944,22) Index item "pwAtmPerf1DayIntervalNumber"
     must be defined with syntax that includes a range

The first one is caused by:
  pwAtmPerfIntervalNumber OBJECT-TYPE
    SYNTAX        Unsigned32
    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 96."
    ::= { pwAtmPerfIntervalEntry 1 }

And easily fixed with a 
    SYNTAX        Unsigned32 (1..96)
and that would match what the DESCRIPTION clause states.

The second one isa cuased by:

  pwAtmPerf1DayIntervalNumber OBJECT-TYPE
    SYNTAX        Unsigned32
    MAX-ACCESS    not-accessible
    STATUS        current
    DESCRIPTION
        "The number of interval, where 1 indicates current day
         measured period and 2 and above indicate previous days
         respectively"
    ::= { pwAtmPerf1DayIntervalEntry 1 }

Clearly the value zero is not valid according to the DESCRIPTION clause
So this
    SYNTAX        Unsigned32 (1..4294967295)
would fix it. But maybe a somewhat smaller max would make sense too.

I see 2 typos:

- page 16: 
     -- In such implementation, user can creat an entry for either
  and
       -- will be an automatic creat of the respective line in the

  s/creat/create/   -- 2 times

- page 17:
           Unless otherwise specified, all Read-Creat objects in this

  s/Read-Creat/Read-Create/
  or maybe even better:  s/Read-Creat/read-create/

Could all be fixed with a Note-to-RFC-editor.

Bert Wijnen 
-----Original Message-----
From: WIJNEN, Bert (Bert) [mailto:[email-masked]] 
Sent: Tuesday, October 16, 2007 5:06 PM
To: Romascanu, Dan (Dan); [email-masked]
Cc: MIB Doctors (E-mail)
Subject: MIB Doctor review for: draft-ietf-pwe3-pw-atm-mib-03.txt

First, draft-ietf-pwe3-pw-atm-mib-03.txt has expired. 
But I still have a copy of that document.
 
Here are my comments:

1. I see 4 REVISISON clauses:

        REVISION "200704030000Z"  -- April 2007
        DESCRIPTION "Third updated version"

        REVISION "200702040000Z"  -- February 2007
        DESCRIPTION "Second updated version"

        REVISION "200608210000Z"  -- 21 August 2006
        DESCRIPTION "First updated version"

        REVISION "200310160000Z"  -- 16 October 2003 12:00:00 EST
        DESCRIPTION "Original version."

   Normally we only use one single REVISION clause during draft
revisions.
   And it normally has text aka
 
         DESCRIPTION "Initial version, published as RFC XXXX."

2. I see that the MIB modules is rooted under

           ::= { transmission YYY }  -- To be assigned by IANA

   Any justification as to why this belongs under transmission?
   It would normally go together with a IfType defined for the
   specific transmission method.

3. I see

  pwAtmOutboundTable OBJECT-TYPE
      SYNTAX    SEQUENCE OF PwAtmOutboundEntry
      MAX-ACCESS        not-accessible
      STATUS            current
      DESCRIPTION
          "This table specifies the information for an ATM PW to
          be carried over PSN in the outbound direction. An
          entry is created in this table for every entry in
          the pwTable with a pwType equal to one of the following:
          atmAal5SduVcc(2), atmCell1to1Vcc(12), atmCell1to1Vpc(13)
          or atmAal5PduVcc(14), or atmTransparent(3). All objects in
          this table MUST NOT be changed after row activation
          (see [PWMIB]) and all objects should remain unchanged
          after reboot."

   Normally the sentence about "..MUST NOT be changed after row
activation"
   belongs in the DESCRIPTION clause of the RowStatus object (as per
   RFC2579). I am not sire what [PWMIB] has to do with it.
   (as an aside, we tend t NOT use citations of this form in DESCRIPTION
    clauses; Instead one would use the REFERENCE clause).

   I guees that with the text 
        "all objects should remain unchanged after reboot."
   You mean that all object should be peristsent over a reboot/restart?
   If so, pls use more preceise wording.
   Also  "should remain" is pretty flexible. a MUST (or at least SHOULD)
   would be much more helpfull for an NMS.
   
   And... the same text is also in the DESCRIPTION clause of the 
   pwAtmOutboundEntry, which seems redundant to me.

4. I see:

  pwAtmOutboundTrafficParamDescr OBJECT-TYPE
      SYNTAX        RowPointer
      MAX-ACCESS    read-create
      STATUS        current
      DESCRIPTION
          "This object represents a pointer to an ATM traffic parameter
           specification table which will be employed while receiving
           cells from the ATM network. This table should contain a
           set of self-consistent ATM traffic parameters including
           the ATM traffic service category. A value of 0.0 indicates
           Best Effort"
      ::= { pwAtmOutboundEntry 4 }

  It seems to me that a DEFVAL of 0.0 would make sense here.

  I am surprised to see that a ROW pointer points to a TABLE as stated
  in the first sentence of the DESCRIPTION clause.
  Is there an example table that has rows to which this row pointer
  could point? I think that would be helpful. Can I point to just a
  private (enterprise specific table that hppenes to have some ATM
  (proprietary) traffic parms?

5. I wonder if for the same pwIndex, does an entry in the
pwAtmOutboundTable
   always have the same values for interfaceIndex, VPI, VCI as for the
   pwAtmInboundTable (with same pwIndex)?
   If so, then some of that data seems redundant?
   Is there always for the same pwIndex and Outbound and an Inbound
entry?

6. pwAtmCfgTable seems to be a read-write table.
   So an NMS cannot create entries. 
   It is unclear when entries are created by the agent?
   Are they always created for every ATM PW that gets created in the
pwTable?

7. In DESCRIPTION clause of pwAtmOutboundNto1Entry

        .....    all RC objects in this table

   Does "all RC" mean all "read-create" ??
   I think I would spell that out.

8. I see
  pwAtmOutboundNto1Vpi OBJECT-TYPE
      SYNTAX        AtmVpIdentifier
      MAX-ACCESS    not-accessible
      STATUS        current
      DESCRIPTION
          "VPI value of this ATM PW. In atmTransparent(3),
           Vpi will be the equivalent of 0xFFFFFFFF"

      ::= { pwAtmOutboundNto1Entry 2 }

  But pls note that AtmVpIdentifier (RFC2514) has this undelying
  SYNTAX of:

    SYNTAX    INTEGER (0..4095)

  so a value of 0xFFFFFFFF is impossible.

9. I also see
  pwAtmOutboundNto1Vci OBJECT-TYPE
      SYNTAX        AtmVcIdentifier
      MAX-ACCESS    not-accessible
      STATUS        current
      DESCRIPTION
          "VCI value of this ATM PW. In atmTransparent(3), or
           VP case, the value will be the equivalent of
           0xFFFFFFFF"
      ::= { pwAtmOutboundNto1Entry 3 }

  while the underlying SYNTAX for AtmVcIdentifier (RFC2514) is

    SYNTAX   INTEGER (0..65535)

  so again, a value of 0xFFFFFFFF is impossible.

10. I see:
  PwAtmOutboundNto1Entry ::= SEQUENCE {
        pwAtmOutboundNto1AtmIf                    InterfaceIndex,
        pwAtmOutboundNto1Vpi                      AtmVpIdentifier,
        pwAtmOutboundNto1Vci                      AtmVcIdentifier,
        pwAtmOutboundNto1RowStatus                RowStatus,
        pwAtmOutboundNto1TrafficParamDescr        RowPointer,
        pwAtmOutboundNto1MappedVpi                AtmVpIdentifier,
        pwAtmOutboundNto1MappedVci                AtmVcIdentifier
       }

   So the RowSTatus object is somewhere in the middle of the table.
   Technically there is nothing wrong with that. But typically we do not

   see it done that way. I just mention it so you are aware as a WG and
   can decide if you want to deviate or not.

11. Several of my comments above apply to multiple similar cases within
the
    MIB module. I just do nto want to repeat it many times. Pls check
    for your self all occurences of the situtaions I describe. 

12. For
  pwAtmPerfIntervalNumber OBJECT-TYPE
    SYNTAX        Unsigned32
    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."
    ::= { pwAtmPerfIntervalEntry 1 }

  It seems to me (from the DESCRIPTION clause, but also from the
  descriptions in RFC2493 from which the TCs in this table
  are IMPORTED) that you would give this a SYNTAX of

    SYNTAX        Unsigned32 (1..96)

  Possibly (as per RFC2493) even (but I am not sure about that)

    SYNTAX        INTEGER (1..96) 

  Maybe some ATM experts or DS1 experts know better. This is an old
  telecom approach of keeping measurements in 15 minute intervals.

13. I see
      pwAtmGroups      OBJECT IDENTIFIER ::= { pwAtmConformance 1 }
      pwAtmCompliances OBJECT IDENTIFIER ::= { pwAtmConformance 2 }

    Normally (and as suggested by RFC4181) we order then with
    Compliances first and trhen Groups. See RFC4181 page38, 

       xxxMIB
       |
       +-- xxxNotifications(0)
       +-- xxxObjects(1)
       +-- xxxConformance(2)
           |
           +-- xxxCompliances(1)
           +-- xxxGroups(2)

--- amib/editorial/bureaucratic comments

- Title page says:
     Pseudo Wire Edge to Edge Emulation
     Internet-Draft
     Intended status: Standards Track
  But abstract states it is experimental:
    Abstract

      This memo defines an experimental portion of the Management
      Information Base (MIB) for use with network management protocols
in
      the Internet community.  In particular, it describes managed
objects
      for modeling ATM Pseudowire (PW) carrying ATM cells over Packet

  So what is it?

- Abstract states:

      This memo defines an experimental portion of the Management
      Information Base (MIB) for use with network management protocols
in
      the Internet community.  In particular, it describes managed
objects
      for modeling ATM Pseudowire (PW) carrying ATM cells over Packet
      Switch Network (PSN) as defined in the PWE3 [ATMENCAP]and
[ATMTRANS].

  The abstract is not supposed to have citations.

- Introcution
  s/MIBs/MIB modules/ or so I think

  Same in sect 5 (Overview). I think that in most cases I would change
  "MIB" into "MIB Module".

  You may want to check otehr cases in the document as well.

- section 7

   This section provides an example of using the MIB objects described
   in section 8 to set up an ATM PW.  While this example is not meant to

  I think you mean section 9 instead of 8.

- page 7 3rd line

      table entry exist in all configurati tables of this MIB module.

  s/configurati/configuration/ ??

- page 33

   Even if the network itself is secure "for example by using IPSec",

  s/IPSec/IPsec/  so all lower case for "sec".

Bert Wijnen 

________________________________

	From: Romascanu, Dan (Dan) [mailto:[email-masked]] 
	Sent: Sunday, June 10, 2007 7:44 AM
	To: MIB Doctors (E-mail)
	Subject: [MIB-DOCTORS] Call for volunteers for MIB DR review for
PWE MIBdocuments

	This is the second call for a volunteer to perform MIB Doctor
reviews on the PW ATM and TDM MIB modules in PWE3. The documents seem to
be in good shape, they passed WGLC and are authored by Orly, but
certainly having them looked at by an independent MIB doctor pair of
eyes would help. 

http://ietf.org/internet-drafts/draft-ietf-pwe3-pw-atm-mib-03.txt

	http://ietf.org/internet-drafts/draft-ietf-pwe3-tdm-mib-08.txt

	Thanks and Regards,

	Dan

_______________________________________________
MIB-DOCTORS mailing list
[email-masked]
https://www.ietf.org/mailman/listinfo/mib-doctors

---

From: Thomas Nadeau <[email-masked]>
Date: Mon, 28 Apr 2008 10:32:05 -0400

On Apr 28, 2008, at 10:22 AM, Bert Wijnen - IETF wrote:

	Thanks for the review Bert. Comment below.

> Could all be fixed with a Note-to-RFC-editor.

	Cool.  We will do this and push out a new rev.

	--Tom

> pwe3 mailing list
> [email-masked]
> https://www.ietf.org/mailman/listinfo/pwe3
>

---

From: "Romascanu, Dan (Dan)" <[email-masked]>
Date: Mon, 28 Apr 2008 16:36:16 +0200

> From: Thomas Nadeau [mailto:[email-masked]] 
> >

Actually a note to the RFC editor would preempt the need to issue a new
rev (on these issues, at least). 

Dan