Subject: AD review of: draft-ietf-snmpconf-diffpolicy-06.txt

From: "Wijnen, Bert (Bert)" <[email-masked]>
Date: Tue, 29 Jul 2003 16:49:01 +0200

Here is my AD review:

1. First, a SMICng compiler (With strict checking) reveals:

  W: f(diffpol.mi2), (20,4) MODULE-IDENTITY diffServConfigMib should 
     have at least one REVISION clause
  W: f(diffpol.mi2), (68,4) Row "diffServConfigEntry" has indexing
     that may create variables with more than 128 sub-ids
   - bw: I think this is a bug in SMICng, cause your limit of 116 seems to
         be limiting it to exact 128 sub-ids
  E: f(diffpol.mi2), (204,12) Item "ifCounterDiscontinuityGroup" should 
     be IMPORTed
  E: f(diffpol.mi2), (207,15) module "DIFFSERV-MIB" may be specified 
     only once
  E: f(diffpol.mi2), (209,12) Item "diffServMIBDataPathGroup" should
     be IMPORTed
  E: f(diffpol.mi2), (209,38) Item "diffServMIBClfrGroup" should 
     be IMPORTed
  E: f(diffpol.mi2), (210,12) Item "diffServMIBClfrElementGroup" should 
     be IMPORTed
  E: f(diffpol.mi2), (210,41) Item "diffServMIBMultiFieldClfrGroup" 
     should be IMPORTed
  E: f(diffpol.mi2), (211,12) Item "diffServMIBActionGroup" should be IMPORTed
  E: f(diffpol.mi2), (211,36) Item "diffServMIBAlgDropGroup" should be IMPORTed
  E: f(diffpol.mi2), (212,12) Item "diffServMIBQGroup" should be IMPORTed
  E: f(diffpol.mi2), (212,31) Item "diffServMIBSchedulerGroup" should be IMPORTed
  E: f(diffpol.mi2), (213,12) Item "diffServMIBMaxRateGroup" should be IMPORTed
  E: f(diffpol.mi2), (213,37) Item "diffServMIBMinRateGroup" should be IMPORTed
  E: f(diffpol.mi2), (214,12) Item "diffServMIBCounterGroup" should be IMPORTed
  E: f(diffpol.mi2), (217,14) Item "diffServMIBMeterGroup" should be IMPORTed
  E: f(diffpol.mi2), (222,14) Item "diffServMIBTBParamGroup" should be IMPORTed
  E: f(diffpol.mi2), (227,14) Item "diffServMIBDscpMarkActGroup" should be IMPORTed
  E: f(diffpol.mi2), (232,14) Item "diffServMIBRandomDropGroup" should be IMPORTed
  E: f(diffpol.mi2), (237,15) Item "diffServDataPathStatus" should be IMPORTed
  E: f(diffpol.mi2), (243,15) Item "diffServClfrStatus" should be IMPORTed
  E: f(diffpol.mi2), (249,15) Item "diffServClfrElementStatus" should be IMPORTed
  E: f(diffpol.mi2), (255,15) Item "diffServMultiFieldClfrAddrType" should be
     IMPORTed
  E: f(diffpol.mi2), (261,15) Item "diffServMultiFieldClfrDstAddr" should be
     IMPORTed
  E: f(diffpol.mi2), (267,15) Item "diffServAlgDropStatus" should be IMPORTed
  E: f(diffpol.mi2), (273,15) Item "diffServRandomDropStatus" should be IMPORTed
  E: f(diffpol.mi2), (279,15) Item "diffServQStatus" should be IMPORTed
  E: f(diffpol.mi2), (285,15) Item "diffServSchedulerStatus" should be IMPORTed
  E: f(diffpol.mi2), (291,15) Item "diffServMinRateStatus" should be IMPORTed
  E: f(diffpol.mi2), (297,15) Item "diffServMaxRateStatus" should be IMPORTed

2. I am also missing the Copyright statement in the DESCRIPTION clause of
   the MODULE-IDENTITY.

3. I am also not sure we need to repeat the whole compliance statements from
   the DIFFSERV MIB. It seems to me it is an exact copy of the 
   diffServMIBFullCompliance, and if indeed so, then I think it is
   sufficient to state (within the DESCRIPTION clause of
   diffServConfigMIBFullCompliance, that the diffServMIBFullCompliance
   must also be met. See 2nd para page 25 of
      draft-ietf-ops-mib-review-guidelines-01.txt

4. When one reads the doc, one would get the impression that one
   MUST undestand the PM-MIB in order to understand this one. Not only
   because of the normative ref, but also reading the text one gets that
   impression. Harrie and Jon had mentioned that this document would
   not need a normative reference to the PM-MIB, but from the text
   it seems to me that it indeed DOES need to, and if not, then it
   does not make clear how this MIB could/should  be used without
   the PM-MIB.

5. I also suspect that the security-considerations section needs to list 
   which tables and objects are security sensitive and why.

6. I also miss the IPR section.

7. There may be other issues that one might find if one does a carefull
   check against:
      draft-ietf-ops-mib-review-guidelines-01.txt 

8. Page 14, 
   Is it really:
      if 
         return roleMatch("Administrator")
   Or is it suppused to be:
      if (roleMatch("Administrator")

   But even with that, I do not think the context is readily understandable
   is it? I think you want to show (possibly with some more complete
   PM-MIB code fragment) how this works and how th various entries
   get created.

   And/Or, show how it can be done without the use of PM-MIB

9. objectdiffServConfigID
       DESCRIPTION
          "A unique id for the per-hop-behavior policy.  The range
          of up to 116 characters is chosen to stay within the SMI
          limit of 128 subidentifiers in an index."
   s/character/octets/
   It is important to distinguis, UTF-8 characters may occupy more than
   one octet per character.

10. StorageType
    Might want to add a DEFVAL ??
    But in any event, you need to say something about write 
    access for permanent entries. See the StorageTypeTC in RFC2579

11. RowStatus
    Need to say which (if any) objects can be changed while a
    row is active. See RowStatus TC in RFC2579

12. Pls remove the IMPORT for InetAddressType, InetAddress,
    they are not used

13. RFC3411 needs to be normatively referenced, you are IMPORTing
    SnmpAdminString from it.

REAL NITS

- page 8, last line of first para
  ".. via a so called the element."  ???

- page 11 2nd para
  "... treatment of 1) is also in the tables."
  Does "1)" refer to section 6.2.1 ?? If so, better make that
  explicit

- Page 13
  I wonder if "DSCP(EF)" is a clear indication of the fact that this
  is entry diffServDscpMarkActDscp.<value for ef> 
  and same for DSCP(AF).

- Pages 12, 13 and 16
  I understand that the values (content) of things like diffServClfrStorage
  and diffServCountActOctets (etc) are irrelevant. But you may want to
  say something about that.

- I'd change
       ::= { mib-2 xxx }  -- Needs to be assigned by before publishing
  into:
       ::= { mib-2 xxx }  -- to be assigned by IANA

Thanks,
Bert

---

From: Harrie Hazewinkel <[email-masked]>
Date: Tue, 29 Jul 2003 20:22:57 +0200

HI all,

I am working on the AD review comments. In the meantime
I would like feedback on the following, item 1.

> 1. First, a SMICng compiler (With strict checking) reveals:

ONly I do not have a version of SMICng to compile it, but libsmi
is not mentioning problems with the arguments suggested in
draft-ietf-ops-mib-review-guidelines-01.txt.

>      have at least one REVISION clause

With respect to the REVISION clause, RFC 25780 says:
     RevisionPart ::=
                   Revisions
                 | empty
     Revisions ::=
                   Revision
                 | Revisions Revision
     Revision ::=
                   "REVISION" value(Update ExtUTCTime)
                   "DESCRIPTION" Text

Am I correct if I interpret this as a REVISION-clause is NOT required??
This is in contradiction with 
draft-ietf-ops-mib-review-guidelines-01.txt.

But I will just add:
     REVISION "20030730000Z" -- 30 July 2003
     DESCRIPTION
         "Initial version publicsh as RFC XXX" -- to be assigned by the 
RFC editor

>          be limiting it to exact 128 sub-ids

I agree.

>   E: f(diffpol.mi2), (297,15) Item "diffServMaxRateStatus" should be 

I am not sure about this, but are those IMPORTs not done
'implicit' by the MODULE clause of the compliance??

At least for the defined objects RFC 2580 says in section 5.4.3
    By definition, each object specified in an OBJECT clause follows a
    MODULE clause which names the information module in which that object
    is defined.  Therefore, the use of an IMPORTS statement, to specify
    from where such objects are imported, is redundant and is not
    required in an information module.

OK, it does not specify this for the MANDATROY-GROUPS or GROUP clause,
but I assumed the same as in the OBJECT clause. In that case I believe 
this
is a bug in the SMICng compiler.

Am I correct with this assumption??

regards,

Harrie