[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
snmpconf-pm-04 notes continued
Having finished reading the draft, here is another slew of notes I
jotted down while reading it. Again, I'll mark the more serious
issues (in my mind) with a "***" (ie, those more theoretical rather
than editorial and require more major structural changes though my
consistency for marking them probably isn't perfect).
Since I'm proposing a huge number of changes at this point (62 in
all!), I'd be happy to edit any deemed acceptable into the document if
you'd rather I do the work than one of the editors (I'll need the
source if thats the case).
22) The most blunt of my statements today: The pmNotificationRegTable
is redundant with the existence of the NOTIFICATION-MIB and, IMHO,
should be dropped from the mib entirely (but, obviously, the
notifications definitions themselves should remain). Duplication
of standardized work is a bad thing.
23) The mib is sprinkled with text in the form of comments that
describe the table(s) below them. This is a pet peeve of mine, as
I frequently use mib parsing tools to browse through mib
descriptions where I'd expect that information to be properly
kept. In particular, I'd like to suggest that the text preceding
the following mib items should be moved into the items themselves:
pmRoleESTable [this one has to be done carefully, as it
describes two tables not just one]
pmNotificatioonRegTable [unless this table is dropped; see above]
I'd actually suggest a note be put into the -bcp document as well
describing that comments should never be used to describe
functionality of objects, but rather to mark things to remember, etc.
24) the capMatch function definition is not clear as to what the OID
should be. After reading everything, I'm guessing that it should
be an oid corresponding to a pmCapabilitiesType definition, but I
could easily accidentally read it as possibly a
pmCapabilitiesSubType OID or even a row pointer into the
(while I'm at it, the capabilities table seems almost redundant
with respect to the sysORTable. There is a lot of overlap and it
took me a while to figure out why both are needed: the table
defined here references mib accessible changes, while the
sysORTable contains only a pointer to a mib object. I'd be
tempted to write a sysORTable agent-capabilities mib table instead)
25) section 5.3 implies that an action is always to be run after a
filter has evaluated to true, but 5.2 dictates that it's only to
be executed within the time frame of policyFilterActionMaxLatency.
It's possible that the action will not be run given a filter's
output if the corresponding value of policyFilterActionMaxLatency
is greater than twice the of pmPolicyFilterMaxLatency. Acutally,
the whole issue of those two objects is rather odd. The way I see
it, these two objects allow for two different management functions
to be run separetly:
1) runs a filter at least as frequently as
pmPolicyFilterMaxLatency and remembers the results.
2) runs a set of actions based on the results from #1 above with
at least the frequency of policyFilterActionMaxLatency.
The odd thing is that these items allow you to configure the row
in very odd ways, like the example described above where
policyFilterActionMaxLatency > pmPolicyFilterMaxLatency*2
which might produce both cases of an action not running for every
filter check, and a case where an action may not get run at all
(if a filter evaluates to true then false during the second
reading). (I guess 5.3 also states it must be run immediately
upon the first discovery of a filter that returns true, but future
invocations may not happen).
Another odd state would be where
policyFilterActionMaxLatency < pmPolicyFilterMaxLatency*2
which would mean that the action would run even though a recent
check hadn't been performed to make sure it should be run (which,
I guess, is not *that* odd).
So if an implementation didn't want to separate these two phases
of collection and responding, they should do filtering/action
running based on min(policyFilterActionMaxLatency,
26) setRowStatus() comes up with a random index based on maxTries, but
is somewhat inefficient if some knowledge of the table exists
already. I'd really like to specify a starting value as well, in
case the table contents were exceptionally large (and I'd default
to 0 if I knew nothing about the contents). If I need to create 5
rows, I'd like to start my 2nd-5th calls searching with the last
assigned value from the previous runs.
27) Its still unclear to me when in the API (if ever) you're allowed
to reference oids by textual name. I'm gathering that never is
the case for all APIs? If so, I'd replace the examples (like
11.1.2) that reference textual oids with numeric ones (even if you
state earlier than some management apps may convert them for you
before uploading the code).
type is the type of the value parameter and will be set to
one of the values for DataType Constants.
type will be the type of the value parameter and will be set to
one of the values for DataType Constants.
29) snmpsend() should really really really pass back the errindex (into
a new parameter). Efficiently parsing through a huge varbind list
to find the source of the error would be otherwise impossible.
30) I'd suggest a new function called "logDebugString()" that would
log a string to the pmDebuggingTable if the row was configured
with debugging turned on (otherwise it'll result in essentially a
31) It's kind of odd that you declare the constants as "const" even
though the language doesn't allow the "const" token. I'd be
happier if the document just declared that all constants can be
treated as an integer and simple list the name and value pairs
without trying to translate them into C-like statements at all.
32) I think all the constants should be matched with the similar BER
tag encodings where possible (just for ease-of-implementation
issues). For instance, a string shouldn't be const "2" but rather
"4". Similarly, the error constants like NosuchObject should
probably be "128" instead of "21", etc. Non-snmp related
constants should then be moved further away from the most likely
places for the SNMP protocol spec to declare future additions (bad
parameter might move to 32767 or something).
33) setScratchpad(varName, scope, value) should have its arguments
reordered to (scope, varName, value). Just makes more sense,
IMHO, to have name and value next to each other.
34) Many functions like getScratchpad, for example, don't state what
will be returned in variables like getScratchpad's value argument
when an error has occurred. I'd suggest explicitly stating that
the value is unchanged from before the call, or that it is
undefined (I'd prefer it remain the same for consistency of
35) Throughout the document strings and integers are frequently mixed
and shouldn't be unless this is legal (which from my reading, I
don't think it is). The "Scratchpad Usage Examples" section, for
instance, has the return values of the getScratchpad call being
'== 55' for example, when it should really be '== "55"'.
I'd be somewhat happier as a scripting programmer if the values
were indeed inter-usable (automatically converted as needed), as
perl's variables are (since you reference it earlier in the
36) oidncmp() has an argument "int n" that isn't defined in the text.
I assume its the length against which to compare the oids in
question. So if I want to compare 2 oids to their completion, do
a) oidncmp(oid1, oid2, 0)
b) oidncmp(oid1, oid2, 128)
c) int o1, o2, otmp;
o1 = oidlength(oid1);
o2 = oidlength(oid2);
if (o1 > o2)
otmp = o1;
otmp = o2;
oidncmp(oid1, oid2, otmp);
37) subidwrite() and subid() says their "int n" argument must
start at "0". I'd suggest replacing the argument instead with
"unsigned int n", which solves the problem the correct way (and
forces a complier check). In fact, I think all the function
definitions could probably use a once over for this type of
checking. The "int subid" of subidwrite(), for example, is
another case where it should probably be an unsigned int (since an
oid can't have negative values in it).
38) A note should be put in subidwrite() saying that appending is
possible through the string concatinator operator (+).
39) the definition of oidsplice() is somewhat ambiguous.
Specifically, "oidsplice replaces n subidentifiers in oid1 with
all of the subidentifiers from oid2 starting at the m'th
subidentifier of oid1". So, does this mean that:
oidsplice(".184.108.40.206", 2, ".220.127.116.11.9", 1)
will result in ".18.104.22.168.22.214.171.124"? Thats how I read it, but I
don't know if thats correct.
40) library accessor functions uses atoi, but I'd rather have strtoul
around instead if possible.
41) library accessor functions defines random, but doesn't state a
minimum acceptability of random-ness provided by the function.
Just a thought.
42) Why are memcmp() and memmove() in the library accessor functions?
We have no pointers?
43) substr() would be a good type of function to add to the list of
44) You should probably import UTF8String from SYSAPPL-MIB::Utf8String
instead of defining your own. Though you don't put a size
restriction on yours, but yet your display hint has one???
45) You're tables are frequently indexed by arbitrary integer numbers,
which is another pet-peeve of mine. This number has no meaning
what so ever and iterating through a table is always required if I
need information. I'd much rather that things be indexed by a
string where possible. For example, the pmPolicyIndex should be
"pmPolicyName" instead and be an snmpAdminString (IMHO, of
course). Most of the other tables could use this type of indexing
change as well. I'd be happy to come up with a list if the idea
46) If pmPolicyPrecedence values must be unique, then I *highly*
recommend making that the index (I know, I'm contradicting what
I'm suggesting in #45 above but now the number would have
47) A flow diagram or state table describing pmPolicyPrecedence is
48) Any reason why pmPolicyDescription can only be 255? I'd be
tempted to allow larger sizes (and implementations can reduce the
length and document it in their agent-capabilities statements).
49) pmPolicyMatches: "The number of elements that are currently
matched by the associated pmPolicyFilter". When is this number
determined? At the time of the last filter run or does the filter
run right now to come up with the number?
50) I don't understand the difference between
pmPolicyAbnormalTerminations and pmPolicyExecutionErrors?
51) Why are pmPolicyAbnormalTerminations and pmPolicyMatches writable
(and Gauges for that matter)???
52) pmPolicyStatus should indicate if any of the previous row columns
are mandatory to set before coming active? Certainly some objects
need setting, like the pmPolicyPrecedence object before it can
become operational. Default values might be needed for some of
the other objects as well (like the latency objects).
53) pmPolicyCodeText has a type "The code" in the middle of the
description that probably is either supposed to be removed or
added to the beginning of the next sentence.
54) pmElementTypeRegTable should be indexed by the
pmElementTypeRegOIDPrefix OID in question to help managers
determine if an entry already exists for that object.
Additionally, I'd suggest that the index should point to the table
object and a second "column number" object be created to indicate
what column should be used when searching for sub-objects. This
would allow guaranteed uniqueness of the pmElementTypeRegOIDPrefix
55) pmElementTypeRegMaxLatency should give a pointer as to what to do
after a new element is discovered.
56) pmCapabilitiesTable should be indexed by the type and/or subtype
for faster traversal when you're looking for a particular
capability. (The type object really needs a better description,
since I have no idea what its value should indicate other than it
has been "assigned by IANA".)
"The element this policy is configuring" -> um...
"The element that is configured by the policy defined in this row."
I don't really like that either, though, but it does need to
58) Why define "off" in pmTrackingElementToPolicyStatus (and
pmTrackingPolicyToElementStatus) at all? Why not just say that
the row doesn't exist unless the policy is on. The enum value
will *never* get used.
59) Can we force creation of rows into the
pmTrackingElementToPolicyTable by setting the
pmTrackingElemenToPolicyStatus object to "forceOff"? IE, so its
definition is remembered when it comes active via the calendar
60) most of these tables could use a "StorageType" object.
(note, the suggested use of the StorageType object should probably
be mentioned in the -bcp document as well).
61) A minimum/maximum number of support entries in the
pmDebuggingTable should be defined somewhere (the maximum being a
mib object returned by an object in the agent). Also, roll over
of the log entries is done by, what, time? A time index would
probably be a good thing for the table as well.
62) notification registration table coments, step 4:
downloaded -> uploaded & download -> upload