Subject: [ipcdn] Review of: draft-ietf-ipcdn-bpiplus-mib-08.txt

From: "Wijnen, Bert (Bert)" <[email-masked]>
Date: Thu, 13 Feb 2003 16:12:51 +0100

Here are my comments:

- references/citations in abstract are not allowed per
  RFC-Editor policy
- 2nd para in abstract seems not needed. but I can live
  with it
- Tiltle of sect 1 is not inline with MIB boilerplate
- 3rd para in section 1 is not needed, in fact I'd rather
  see it removed. The RFC3410 explains all that.
- DESCRIPTION clause of MODULE-IDENTITY is missing the
  MIB copyright statement
- I had asked in in earlier email:
  Mmm.. did I review rev 07. I can't quickly find that I
  did send a review report.
  I am assuming that you will take care of all the comments
  that are (in my view) to be expected based on the review
  of the subscriber mib I did, right?
- Is this MIB obsoleteing an earlier (RFC) version?
  If so, then that should noted on front page/abstract
  and we should see a revision statement that lists the
  original RFC. 
  If this is the first time this MIB gets publsihed as an
  RFC, then we want to see only one simple revisions clause
  with a DESCRIPTION aka:
     "Initial version, published as RFC xxxx."
     -- RFC-editor assigns xxxx
  We do not want to see long lists of changes that happened
  during initial revisions of Internet Drafts
- And how did you decide to assign the 
  docsBpi2MIB MODULE-IDENTITY
     ...
     ::= { docsIfMib 6 }
  I worry a lot about this practice, cause it is very difficult
  to keep track and to ensure that no conflicts arise.
- I worry about:
      X509Certificate ::= TEXTUAL-CONVENTION
           STATUS    current
           DESCRIPTION
               "An X509 digital certificate encoded as an ASN.1 DER
           object."
           SYNTAX    OCTET STRING (SIZE (0..1400))
   The reason is that this is not some X509 generic Certificate
   but a special one for your IPCDN use.
   How about renaming it to DocsX509ASN1DEREncodedCertificate ??
-       docsBpi2CmAuthReset OBJECT-TYPE
   Would it be wise to add a docsBpi2CmAuthLastReset object
   similar as what was done in the subscriber MIB module?
- objects like:
      docsBpi2CmAuthRejects    OBJECT-TYPE
           SYNTAX         Counter32
           MAX-ACCESS     read-only
           STATUS         current
           DESCRIPTION
                "The value of this object is the count of times the CM
           has received an Authorization Reject message, since reboot."
  require that a Counter starts at zero. But those are NOT the
  semanticws of a Counter32. If this is what you want, I think you
  need to use the ZeroBasedCounter32 as per RFC2021.
  Pls read draft-ietf-ops-mib-review-guidelines-00.txt sect 4.6
  specifically 4.6.1.2
  You have quite a few of those objetcs
-       docsBpi2CmAuthRejectErrorString    OBJECT-TYPE
           SYNTAX         SnmpAdminString (SIZE (0..128))
           MAX-ACCESS     read-only
           STATUS         current
           DESCRIPTION
                "The value of this object is the Display-String in
   S/Display-String/text string/ ??
   Same for docsBpi2CmAuthInvalidErrorString
   And for docsBpi2CmTEKKeyRejectErrorString
           docsBpi2CmTEKInvalidErrorString
      docsBpi2CmIpMulticastSAMapRejectErrorString
    docsBpi2CmtsAuthInvalidErrorString
-      docsBpi2CmTEKDataEncryptAlg   OBJECT-TYPE
          SYNTAX         INTEGER {
                                 none(0),
                                 des56CbcMode(1),
                                 des40CbcMode(2)
                                 }
   We recommend to start Enumerations from 1.
   Possibly the 1 and 2 for the desXxxx enumerations are so numbered
   to align with some other place. If so, then starting with zero
   seems acceptable. But pls add some explanantion in that case
   Same question for simial xxxCrypto object
-       docsBpi2CmTEKDataAuthentAlg   OBJECT-TYPE
           SYNTAX         INTEGER {
                                  none(0)
                                  }
   An object that can only have one value that is ALWAYS zero?
   What is the use of that? Maybe future extensibility, but if so,
   then please say so in DESCRIPTION clause, otherwise this
   seems nonsense and bloat.
   Same question for simial xxxCrypto object
-       docsBpi2CmIpMulticastIndex         OBJECT-TYPE
           SYNTAX         Integer32 (1..1000)
   It is valid. But for INDEX objects we prefer Unsigned32 with
   a range. see again the mib review guidelines doc
   I think this occurs a few more time in this MIB module.
   As I say, it is valid, but since your still working on this
   MIB module, might as well use the recommended method.
-       docsBpi2CmIpMulticastAddress  OBJECT-TYPE
           SYNTAX         InetAddress
           MAX-ACCESS     read-only
           STATUS         current
           DESCRIPTION
                "This object represents the IP multicast address
           to be mapped."
   you MUST specify with InetAddressType object defines the context
   or type of this object. You did it correct in subscriber mib.
-       docsBpi2CmtsDefaultSelfSignedManufCertTrust  OBJECT-TYPE
           SYNTAX    INTEGER {
                     trusted (1),
                     untrusted (2)
                     }
   Seems to me that docsBpi2CmtsDefaultSelfSignedManufCertTrusted
   descriptor with a syntax of TruthValue is more appropriate
-       docsBpi2CmtsCheckCertValidityPeriods    OBJECT-TYPE
           SYNTAX         TruthValue
           MAX-ACCESS     read-write
           STATUS         current
           DESCRIPTION
         "Setting this object to TRUE causes all chained and
   The better wording is:
         "Setting this object to 'true' causes all chained and
   This occurs a few more times in this MIB module
-     docsBpi2CmtsAuthCmBpiVersion  OBJECT-TYPE
           SYNTAX         INTEGER {
                        bpi (0),
                        bpiPlus (1)
                              }
   Any special reason why enumeration cannot start at 1?
-       docsBpi2CmtsAuthCmReset  OBJECT-TYPE
   Yet another way in which you guys do resets.
-       docsBpi2CmtsAuthCmInfos       OBJECT-TYPE
           SYNTAX         Counter32
           MAX-ACCESS     read-only
           STATUS         current
           DESCRIPTION
                "The value of this object is the count of times the
           CMTS has received an Authentication Information message from
           this CM, since entry creatiion."
   ZeroBasedCounter32 ??
   You have several of those
-       docsBpi2CmtsAuthBpkmCmCertValid         OBJECT-TYPE
           SYNTAX    INTEGER {
                             unknown (0),
                             validCmChained (1),
                             validCmTrusted (2),
                             invalidCmUntrusted (3),
                             invalidCAUntrusted (4),
                             invalidCmOther (5),
                             invalidCAOther (6)
                             }
     Any special reason to start ENUMERATION with zero ??
-       docsBpi2CmtsTEKSAType    OBJECT-TYPE
           SYNTAX         INTEGER {
                                  none(0),
                                  primary(1),
                                  static(2),
                                  dynamic(3)
                                  }
     Any special reason to start ENUMERATION with zero ??
     There are more of those... I won;t list them anymore,
     I guess you get the gist.
- I wonder if it would not be better to have 2 compliance
  statements, one for CM and one for CMTS.
  It is unclear to me if any GROUPS are mandatory. I think
  some are, but it depends if your are CM or CMTS
-       -- relaxation on IP addressing
      OBJECT    docsBpi2CmIpMulticastAddressType
             -- SYNTAX InetAddressType { ipv4(1) }
   Pls make it a real SYNTAX clause and not a comment.
   That SMICng barks at it is something we know and SMICng
   should be fixed.
- You have citations in DESCRIPTION clauses. We normally do not
  do that, cause they will be lost when people extract the
  MIB module from the RFC.
- Mmm.. you start off with some OBSOLETED object group right 
  away he? You might want to say something about when/why that
  happened.
- References need to be split in normative and informative
- You have picked up the new MIB boilerplate, but you still 
  have a lot of old MIB boilerplate references included, which
  make no sense anymore
- The new MIB boilerplate has [RFC2578], [RFC2579] and [RFC2580]
  as citations, but they do not show as such in the references
  section
- The security considerations section is not compliant with the
  new MIB security guidelines. The subscriber MIB has
  done a much better job. You should too.
- I see 3 names in CONTACT-INFO while only two are listed
  on front page and only two are listed in Author's addresses?

Oh well... lots of little nits that the WG could have found
without me having to do a review.

Bert 
_______________________________________________
IPCDN mailing list
[email-masked]
https://www1.ietf.org/mailman/listinfo/ipcdn