[ntpwg] My review of: draft-ietf-ntp-ntpv4-mib-05.txt
Bert Wijnen (IETF)
bertietf at bwijnen.net
Sun Feb 8 20:31:40 UTC 2009
Note: I am not subscribed to the ntpwg mailing list and do not plan to
subscribe either. So if you want me to see any responses, please
copy me explicitly
I did a review for:
http://www.ietf.org/internet-drafts/draft-ietf-ntp-ntpv4-mib-05.txt
Here are my findings:
SMICng results:
C:\bw\smicng\work>smicng ntpv4.inc
E: f(ntpv4.mi2), (272,14) Default value for "ntpEntStatusCurrentModeVal"
must be a name and not a number
*** 1 error and 0 warnings in parsing
This means changing
DEFVAL { 99 }
into
DEFVAL { unknown }
will fix it.
But I also find:
- writable objects for which I see no text in the DESCRIPTION clauses
about expected persistency behaviour
- Counter32 objects with no test in the DESCRIPTION clauses about
possible discontinuities and which timer object would indicate such.
Specifically, the fact that (I think) the ntpd keeps track of the
Counters,
it seems that there may be discontinuities at other times than when
the snmpd restarts (e.g. at other times than reset of sysUpTime)
- I see a whole list of REVISION clauses. Normally we only have a
single revision clause at RFC publication time. Not a list of revisions
that took place during I-D revisions.
- I am missing the Copyright statement in the DESCRIPTION clause
of the MODULE_IDENTITY
- I am missing that this revision of the document is published as RFC xxxx
(xxxx to be filled in by RFC-editor)
- I see a lot of objects with a SYNTAX of DisplayString.
I would like to point out "note 2" on web page
http://www.ops.ietf.org/mib-common-tcs.html
Note 2. DisplayString does not support internationalized text.
It MUST NOT be used for objects that are required to
hold internationalized text (which is always the case
if the object is intended for use by humans [RFC2277]).
Designers SHOULD consider using SnmpAdminString,
Utf8String, or LongUtf8String for such objects.
- Instead of
ntpEntNotifPrefix OBJECT IDENTIFIER ::= { ntpSnmpMIBObjects 6 }
...
ntpEntNotifications OBJECT IDENTIFIER ::= { ntpEntNotifPrefix 0 }
I propose/suggest to do
ntpEntNotifications OBJECT IDENTIFIER ::= { ntpSnmpMIB 0 }
This makes it more inline with what we often do in MIB module.
Also makes the OIDs of notifications shorter
- I strongly recommend to use the MIB structure as per RFC4181,
which suggests that you would replace
ntpEntConformance OBJECT IDENTIFIER ::= { ntpSnmpMIB 6 }
with
ntpEntConformance OBJECT IDENTIFIER ::= { ntpSnmpMIB 2 }
- I see OBJECT-GROUP definitions like
ntpEntObjectsGroup1 OBJECT-GROUP
OBJECTS {
ntpEntSoftwareName,
ntpEntSoftwareVersion,
ntpEntSoftwareVersionVal,
ntpEntSoftwareVendor,
ntpEntSystemType,
ntpEntStatusEntityUptime,
ntpEntStatusDateTime,
ntpAssocName,
ntpAssocRefId,
ntpAssocAddressType,
ntpAssocAddress
}
STATUS current
DESCRIPTION
"A collection of objects for the NTP MIB that all NTP
or SNTP entities should implement."
::= { ntpEntGroups 1 }
Text about "should implement" or "optional" or such is NOT supposed to be
in an OBJECT-GROUP DESCRIPTION clause. The OBJECT-GROUP is
only meant to group objects logically together.
If such a group is mandatory or optional is something that is expressed
via MANDATORY-GROUPS or via the GROUP clauses in the
MODULE-COMPLIANCE.
As another example, you have
ntpEntObjectsGroup2 OBJECT-GROUP
...
DESCRIPTION
"A collection of objects for the NTP MIB that are optional
for NTP or SNTP entities to implement."
::= { ntpEntGroups 2 }
So here you say that the group is optional while you have listed this
group
as a MANDATORY-GROUP here:
ntpEntNTPCompliance MODULE-COMPLIANCE
STATUS current
DESCRIPTION
"The compliance statement for SNMP entities which use NTP and
implement the NTP MIB"
MODULE -- this module
MANDATORY-GROUPS {
ntpEntObjectsGroup1,
ntpEntObjectsGroup2,
ntpEntNotifPrefixGroup
}
::= { ntpEntCompliances 1 }
So that is a conflict in your MIB-Module.
- I see a lot of MIB-level comment lines, aka:
ntpEntSoftwareVersion OBJECT-TYPE
SYNTAX DisplayString
MAX-ACCESS read-only
STATUS current
DESCRIPTION
"The software version of the installed NTP implementation."
-- full version string, e.g. "ntpd-4.2.0b at 1.1433 ..."
::= { ntpEntInfo 2 }
I think it is much better to include that informatrion in the DESCRIPTION
clauses. Otherwise it will often be lost when the MIB module is extracted
or used as input for generating help text etc.
- I see references/citations aka:
NtpDateTime ::= TEXTUAL-CONVENTION
DISPLAY-HINT "4d:4d:4d.4d"
STATUS current
DESCRIPTION
"NTP date/time on the device, in 128-bit
NTP date format. Ref: draft-ietf-ntp-ntpv4-proto-06,
section 6:
It includes a 64-bit signed seconds field
spanning 584 billion years and a 64-bit fraction
field resolving .05 attosecond (i.e. 0.5e-18).
For convenience in mapping between formats, the
seconds field is divided into a 32-bit era field
and a 32-bit timestamp field.
If time is not syncronized this field shall be a
zero-length string.
This TC is not to be used for objects that are used
to set the time of the node querying this object.
NTP should be used for this--or at least SNTP."
SYNTAX OCTET STRING (SIZE (0 | 16))
I suggest that the pointer to Ref: draft-ietf-ntp-ntpv4-proto-06
is changed to Ref: RFC yyyy (and ask RFC-editor to fill in
that yyyy value. I see that Ref: draft-ietf-ntp-ntpv4-proto-11 is
also in Last Call, so that would be fine.
In genreal though, it is probably better to use the REFERENCE
clause. So it would be something like
REFERENCE "Network Time Protocol Version 4 Protocol
And Algorithms Specification, sect 6, RFC
yyyy"
- It seems to me that for something like this:
ntpEntStatusEntityUptime OBJECT-TYPE
SYNTAX Unsigned32
MAX-ACCESS read-only
STATUS current
DESCRIPTION
"The uptime of the NTP entity in seconds."
-- time since ntpd was (re-)started (not sysUptime!)
DEFVAL { 0 }
::= { ntpEntStatus 9 }
I would add a UNITS clause.
I would also move the comment line inside the DESCRIPTION clause.
I am not sure if a DEFVAL of 0 (zero) makes sense.
Specifically... what would the value be if the snmp agent has started
later that ntpd? What if the snmp agent restarts and the ntpd does not?
In fact, I think that a DEFVAL clause makes no sense for this object.
I in fact wonder if most of the DEFVAL values for the read-only objects
make sense.
- For
ntpEntStatPktModeTable OBJECT-TYPE
SYNTAX SEQUENCE OF NtpEntStatPktModeEntry
MAX-ACCESS not-accessible
STATUS current
DESCRIPTION
"The number of packets sent and received by packet mode."
::= { ntpEntStatus 18 }
ntpEntStatPktModeEntry OBJECT-TYPE
SYNTAX NtpEntStatPktModeEntry
MAX-ACCESS not-accessible
STATUS current
DESCRIPTION
"The number of packets sent and received by packet mode."
INDEX { ntpEntStatPktMode }
::= { ntpEntStatPktModeTable 1 }
I am surprised by the SAME DESCRIPTION clause for each of these!!??
- When I see:
ntpAssocAddressType OBJECT-TYPE
SYNTAX InetAddressType
MAX-ACCESS read-only
STATUS current
DESCRIPTION
"The type of address of the association."
-- contains the type of address for uni/multi/broadcast associations
::= { ntpAssociationEntry 4 }
ntpAssocAddress OBJECT-TYPE
SYNTAX InetAddress
MAX-ACCESS read-only
STATUS current
DESCRIPTION
"The IP address (IPv4 or IPv6) of the association."
-- contains IP address of uni/multi/broadcast associations
::= { ntpAssociationEntry 5 }
Then I like to point out that RFC4001 states that the use of InetAddress
SYNTAX, MUST have a DESCRIPTION clause that explain./states
which object of SYNTAX InetAddressType controls the format of
the InetAddress object. It is most probably ntpAssocAddressType
but it would be good to state so.
Besides, if you are sure that this ALWAYS only applies to IPv4 and IPv6,
then I would do it as follows:
ntpAssocAddressType OBJECT-TYPE
SYNTAX InetAddressType { ipv4, ipv6 }
ntpAssocAddress OBJECT-TYPE
SYNTAX InetAddress SIZE(4|16)
You must check if you also want to use Zoned addressed, because then
that must be added too.
If, at the other hand, other values than ipv4/ipv6 are possible (maybe
in the future?) then it might be better to put the "minimum support for
ipv4/ipv6 requirement in the MODULE-COMPLIANCE via an OBJECT
and SYNTAX clause.
- I see some 8 notifications. Is there a risk that too many get generated
at any point in time, such that it would flood the network and/or
overload a NMS ??
It would be good to add some text about the expected max notification
rates and/or add a throthle or an enable/disable switch.
- It seems that RFC4001 is missing as a normative reference.
- I would think that the new draft-ietf-ntp-ntpv4-proto-11 (version 4 of
NTP)
should also be a normative reference
- The security section is not according to the guidelines provided at
http://www.ops.ietf.org/mib-security.html
- I have not checked the new NTP version 4 spec to see if the knobs/sensors
in that protocol have properly been modeled in this MIB module.
That is a separate task that may actually need much more time than the
review I have done for now.
Bert Wijnen
More information about the ntpwg
mailing list