[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
AD review of: draft-ietf-snmpconf-diffpolicy-06.txt
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