[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

review of draft-ietf-snmpconf-pm-09.txt




Review of draft-ietf-snmpconf-pm-09.txt
Andy Bierman

I will try not to list issues already raised by Dave Harrington.
In general, there are some very powerful features in this
complex MIB. The document lacks clarity and organization
in some aspects, which makes it difficult to understand.
I attended some SNMPCONF WG meetings, and talked to the
authors at length about the design, and it was still very
difficult to figure out the big picture or specific complex 
interactions, by reading the draft.  (It's an Easter Egg hunt 
to figure out any particular feature.)

[pg 4, sec 3, Overview]
There were so many good slides presented in the WG describing
the layering from independent to dependent data abstractions,
but none of that made it into the draft. (The entire overview 
is less than a page.)

There is no attempt to describe how this draft addresses any
of the motherhood & apple pie goals listed in para 2.

There is no attempt (anywhere in the draft) to describe the
overall system that is defined within the draft. The draft
reads like a parts list without instructions on how to put
all the parts together.  Some of the missing details:
 - a diagram showing the components of the PM system
 - a diagram showing how the PM system fits within the overall
   managed device 
 - a diagram showing how the PM system fits within the overall
   NM system as a mid-level manager
 - explanations of all the components, such as:
    - element discovery
    - element registration
    - capabilities registration
    - roles
    - policies
    - policy groups
    - policy selection
    - policy distribution
    - policy organization
    - policy conditions
    - policy actions
    - policy scheduling
    - policy script execution environment
    - element filtering
    - accessor library
    - policy validation
    - schedule group
 - how the components relate to each other
 - how the entire system fits within the SNMP Architecture

[pg 5, sec 4, Arch.]
This section would make more sense if the system was explained
first. This section doesn't really describe an architecture,
but rather the notion of policy based management (again).
It doesn't really explain the system that implementors are
supposed to create, and how operators are supposed to use it.

[pg 6, para 1, sec 4, Arch.]
The description of "this element" is confusing because there
has been no detailed explanation (and won't be) of the runtime
environment. 

[pg 7, para 3, sec 4, Arch.]
Accessor functions have not yet been introduced, so this para
seems out of place.

[pg 7, para 4, sec 4, Arch.]
"The role group ..." sounds like you are introducing a term, but I
think this refers to a MIB group. Clarify.

[pg 8, para 1, sec 4, Arch.]
(M-F 9-5) should be expanded to full words

[pg 8, para 2, sec 5.1, Terminology]
This section should be moved to a general terms section either before 
or after the overview.  The details about RTE should stay in section 5.
Some terms that need introduction:
    - element type
    - element
    - "this" element
    - system element 0.0
    - role
    - policy
    - policy group
    - policy condition
    - policy action
    - accessor function
    - schedule group
    - precedence group

[pg 8, para 4, sec 5.2, Element Discovery]
"as 1 element" -> "as one element".

In the last sentence, it's not clear how this applies to
the runtime environment.  It would help to explain the runtime
environment at a high-level (iterative engine, and how it
operates on elements) somewhere in section 5.

[pg 9, sec 5.2, Element Discovery]
It isn't clear why the details of the element registration
MIB are explained, because the rest of the discussion is
about element discovery. How does NMS registration relate
to discovery?  It is confusing to mix MIB object
details in with this high level discussion.  (They should
be in a separate MIB overview section.) In general, the
MIB objects are discussed in detail before the MIB is introduced.

[pg 10, sec 5.2,1 Implementation Notes]
This section should be moved to later in the document.
because the MIB has not been introduced, and all the
implementation notes should be grouped together.

[pg 11, para 2, sec 5.3 Element Filtering]
This first paragraph contains MIB details about policy
scheduling, and should go in a section for that (added
before this one). This concept is called 'policyCondition' 
everywhere but this section.

[pg 11, sec 5.3,1 Implementation Notes]
"It is an implementation-dependent matter as to how policy
conditions are scheduled."  This is pretty much the entire
description of the script invocation and scheduling details.
I think it will be difficult for script developers to port
scripts across PM implementations with this approach. There
are also many constraints placed on scheduling throughout
the document, so this statement is misleading.

[pg 11, sec 5.4 Policy Enforcement]
This concept is called 'policyAction' everywhere but this section.

[pg 12, sec 5.4,1 Implementation Notes]
I think it will be difficult for script developers to port
scripts across PM implementations without specifying scheduling
algorithms, scheduling conflict resolution, etc.)

It is not clear what by the 2nd setence "each ... pair 
its own process ... run simultaneously."

The execution control flow should be explained (from engine 
to condition script to action script within a specific iteration 
sequence, based on registered elements, configured latencies, 
system events, etc.)

[pg 12, sec 5.5, Definitions]
These terms should be moved to a general Terms section.

set to 'active -> set to 'active'

If their -> If there

term "precedence policy" should be explained in the Terms section

[pg 13, para 2, sec 6, PolicyScript Language]
It is a real stretch to call this language a subset of C++,
and talk about operator overloading solely as an aid to documentation.  
The added features like 'var' class or '$*' construct in strings, 
specialized execution environment, lack of user-defined
functions or data types, lack of step-debugging, and monolithic 
code organization make this language (and programming environment) 
very different than C++.

[pg 13, para 4, sec 6, PolicyScript Language]
"This language is formally defined as a subset of ISO C++ [19],"
I don't think this is true. The 'var' and '$' features are not
part of C++. An "extended subset" is marketing-speak for "new language".

[pg 13-14, para 6, sec 6, PolicyScript Language]
"within a PolicyScript program"; do you really mean within an
single instance of a policy script? The term 'PolicyScript program'
is not used in other places.

[pg 18, para 2, sec 6.2, Variables]
 "typedef ... var;"

This example is meaningless and confusing, since there are no typedefs 
in this language. This and the last para should be clarified or removed.

[pg 18, last para, sec 6.2.1,  Var Class]
It is very confusing to define these operators and use them
in examples, and bury in the text that they are not really
part of the language. This should be in a separate section
on Operators.

[pg 19, para 4, sec 6.2.1, Var Class]
Why is the EBNF for numeric_string in this section instead of 6.1) 
Formal Definition? Why do you need EBNF for something that's not 
in the language?

[pg 21, para 1, sec. 6.2.1, Var Class]
These rules seem to belong with the formal definition in sec 6.1

The term 'OP' is confusing because it is not explained,
and it looks like there should be a construction for OP.
It's being used in a generic sense, so a term like <operator>
would be more clear.

[pg 22, para 3, sec. 6.2.1, Var Class]
    "getVar("ifSpeed.$*") < 128000"

This example is confusing because the string 'ifString' isn't
actually allowed, and  the "$*" mechanism has not been introduced 
or defined yet.

"an integer than" -> "an integer, then"

[pg 22-23, sec. 6.3, QuickStart Guide]
It's very confusing to start out with a gibberish example.
Most people will glaze over the explanation for why it's there
and attempt to figure out what the code is trying to accomplish.

The Quickstart guide should be moved to a later section because 
the rest of the language and runtime features have not been
introduced yet, so this section is full of forward references.

   "return (inSubtree(elementName(), "ifEntry")"

This example is misleading because the string 'ifEntry' is 
not actually allowed. In general, it's a bad idea to
provide code examples that won't actually work. People
learn from examples, so they should be correct.

The other examples of finding the gold interface or
killing all application processes that have run for > 5 min
are not very compelling.  IMO. the document doesn't effectively
demonstrate that the PM MIB can be used to perform the
initial configuration of a real managed device, such as a router.

[pg 26, para 1, sec 6.4, return values]
Does the 1st sentence on this page mean that all policy scripts
are required to return a Boolean value, and the only time they don't
is if an RTE occurs?  The structure of the policy scripts should
have been explained already.  None of the examples to this point
have returned a value -- they show the opposite (simple 'return;');

It is confusing to define PolicyCondition and PolicyAction scripts 
as if they're the same, and then sprinkle text throughout the document
that says the opposite (e.g. all scripts return a Boolean, but all
PolicyAction return codes are ignored.) This doesn't make PM easier
to understand or use.

It should be more clear that a script is very dependent on the element
type is is intended to act upon.  Execution of a script on the wrong
element type, or a wrong parameter in a call to 'ev()' could cause
incorrect results and be hard to detect.

[pg 27, sec 8, Accessor Functions]
Why are assessor libraries registered in the role table instead
of the capabilities table, and accessed with 'roleMatch'?

There are no mechanisms to add new accessor functions or change an
accesssor function over time.  

The ability to determine that a PM platform supports every possible
parameter for every accessor function in a library, or determine 
nothing at all, is not very useful. Vendors will probably
register libraries even if they don't support every option to 
every function.

[pg 29, sec 9.1.1, SNMP Operations]
>From time -> From time

   "function(...[, integer mPModel,
               string tDomain, string tAddress,
               integer secModel, string secName,
               integer secLevel, string contextEngineID ])"

This function prototype is confusing, function looks like a keyword,
and ... is legal is C, but not the first param.  Add a real example
just after this to show what a real function prototype with these
extra parameters would look like. Explain up-front that these are
called 'NonLocalArgs' throughout the document, instead of at the end
on pg 31.

[pg 30, sec 9.1.1, SNMP Operations]
ascii -> ASCII   (throughout document)

[pg 31, sec 9.1.2, Form of SNMP Values]
ToInteger() function -> ToInteger() operator

The 'enum_decimal' EBNF should be in section 6.1, Formal Definitions,
not here.

[pg 32, sec 9.1.2, Form of SNMP Values]
The paragraph explaining that "ifIndex" is not actually encoded on the
wire is incomplete. Scripts passing parameters to accessor functions
are also affected, not just setting MIB objects that happen to have
a syntax of OBJECT IDENTIFIER.

[pg 35, sec 9.1.3.4, searchColumn]
no distinct section for the 'pattern' parameter

[pg 36, sec 9.1.3.4, searchColumn]
set to the oid of -> set to the OID of
(all instances of oid should be capitalized unless 
refering to the 'oid' parameter to an accessor function.)

example with "ifType" is misleading because this string
value is not actually allowed.

[pg 37, sec 9.1.3.5, setRowStatus]
It's not clear why or when this function should be used
instead of createRow. Some explanation would help.

A code example of this accessor function would help.

[pg 39, sec 9.1.3.6, createRow]
There should be a distinct section explaining the 'index'
parameter.

[pg 42, sec 9.1.4, General SNMP Functions]
readVar and writeVar -> readVar, and writeVar

arrays and -> arrays, and

varbindlist -> varbind list

[pg 43, sec 9.1.4, General SNMP Functions]
  "writeVar(pdu, nVars++, "sysDescr.0", ...);"

Code examples should only contain valid syntax.
This is confusing because the "..." is valid C syntax
for function prototypes, but PolicyScript doesn't have
prototypes, and the example is a code fragment anyway.

Why would a varbind get created automatically by a readVar
or snmpSend operation? Is this a typo? Seems like only writeVar
should actually create a varbind in a PDU data store.

Is there a maximum number of varbinds allowed in this data store?

The writeVar code examples would be more helpful in section 9.1.4.2.

[pg 44, sec 9.1.4.2, writeVar]
Does this function have a return code? It isn't documented.

[pg 45, sec 9.1.4.3, readVar]
Does this function have a return code? It isn't documented.

Some code examples for readVar would be helpful.

and it's type -> and its type

[pg 46, sec 9.1.4.4, snmpSend]
The values for the opcode parameter are 'defined elsewhere', 
yet later on the same page the opcode Trap(4) is given as an example.
The opcodes should be listed or a specific reference given.

[pg 47, sec 9.1.4.6, writeBulkParameters]
Are there any ordering issues between writeVar and writeBulkParameters,
besides that they both have to be called before a GetBulk PDU is sent
with the snmpSend function?  Is it an error in snmpSend if the
number of varbinds present is not > the nonRepeaters parameter?

[pg 47, sec 9.1.4, general]
These functions are very important and it would help to have
a meaningful example that used all of them.

[sec 9, general]
It is short-sighted to place all of the accessor functions
in one library, and place that library in the PM spec itself.
The inability to ever change anything in a library after
initial publication is also a significant weakness.

It would help implementors to have a header file with the
complete and correct prototype for every function in an
accessor library, in a concise form. 

There seems to be no recognition of vendor provided accessor
libraries, other than registering the library name as a capability 
to check with roleMatch("libname", "0.0");.

Some accessor function prototypes start with a return value
(like C) and others start with the function name (means an
int is returned in K&R C, what does it mean here?)
All the accessor prototypes should be consistent.

[pg 48, sec 9.2, Constants]
The examples are misleading because the 'const' keyword
is used. Why not just have a table, and list Constant Name, Type,
and Value in three columns.

Some explanation of why the Data type constants are not
sequential enums would help. Same for origin of the 
SNMP Error and Exception Constants.

The term 'integer' is used throughout the document, except here,
where the term 'int' is used instead.

[pg 48-49, sec 9.2, Constants]
Why are all the constants named without Caps other than the
first char (e.g. Badparameter) except GeneralFailure?
Some constants like noAuthNoPriv don't start with a cap.
These constant names should be consistent.  

Mention that these constants are reserved words, and cannot be
used for any variable or accessor function name. That is
the case, right?

All system constants should be listed in this section. Some
are spread out in other sections (e.g. 9.3.9).

[pg 50, sec 9.3.1, roleMatch]
        "'roleString' is a string. The optional argument
        'element' contains the OID name of an element, defaulting
        to the current element if 'element' is not supplied."

Isn't there more to say about the 'roleString' param?

Is the HEX encoding of contextEngineID defined somewhere else?

Meaningful examples of this accessor function would help.

[pg 52, sec 9.3.7, setScrathpad]
The concept of scope should be buried in this accessor function
definition. These definitions (Global, Policy, PolicyElement)
should be explained along with the use of scratchpads, in a
previous section on execution environment.

It's not clear that the scope values are constants, because they
are defined later (pg 55).

There are minimum requirements on the newPDU() function
(5 per invocation, 64 varbinds each). There should be
minimum requirements for the number of scratchpad variables
as well.

[pg 54, sec 9.3.8, getScrathpad]
There doesn't seem to be any way to discover or dump all
or some of a scratchpad contents for debugging purposes.
Unless the same string is used for 'varName' as in the 
setScratchpad call, the variable is invisible to the application.

The list of usage examples is very confusing, not just because
of the "== 55" constructs that should be "--> 55", but it's
hard to tell what the example is trying to point out.
(I think it's the effect of the scope parameter on the
values set and retrieved.)

[pg 55. sec 9.3.9, Constants]
This section should be moved to a centralized constants section,
and the const keyword avoided (see above).
Same consistent naming comment as above.

[pg 55. sec 9.3.10, signalError]
The last sentence is confusing and should be changed or removed.
("script no longer calls this function" ... ?)

Is the agent required to detect the setting of these signalError 
bits immediately? Is there a max latency for this event defined?
It should be mandatory to check this at each control point
(although the spec doesn't define control point in the runtime
section.) An accessor function that causes an action (e.g. 
snmpSend or setScratchpad) must not be called after an 
RTE occurs or signalError is called. Does it say that
somewhere?

[pg 56. sec 9.3.11, defer]
The grammar in last sentence is incorrect (bronze policy isn't.)

The concept of precedence groups should be explained
way before here, in an accessor function description.
The whole concept is fuzzy, potentially complicated,
and never explained or shown in examples. How is
the system configured to utilize "runner-up" policies?
The mechanism should be better explained.

the code determines -> the policy script determines 

The defer prototype should be formatted in a consistent
manner, with a return code. The parameter should not
be the same as the function name.

   "If defer(0) is called, the script will not defer."

What does this sentence mean? What is the point of calling
defer with a parameter value of zero? The parameter seems
useless.

Does defer cause an RTE if there is nothing to defer to,
or an RTE in any other cases?

The debugging message (present for 'fail') would also
be useful for debugging scripts using the defer function.

It should be extra clear that the defer call is like
a return statement, and no code after this line will
actually be executed. An example would help here.

[pg 57. sec 9.3.12, fail]
    "If 'defer' is 1, this script will defer to the next lower
    precedence ready policy in the same precedence group whose
    condition matches. If 'defer' isn't 1, it will not defer.  Note
    that if a condition defers, it is functionally equivalent to
    the condition returning false."

This defer parameter is confusing. The runtime environment
should be defined clearly in one place, instead of scattering
tidbits like this throughout the document.

Not clear what the last sentence of this para is supposed
to mean. Does the fail (or defer) function force a specific
return value for a policy script? Isn't it always false,
even though it's ignored by the PM agent?

The free parameter seems to control whether the function exit
is treated as an RTE or not. It doesn't explicitly cause
anything to be freed unless 'freeOnException' was previously
set. 

The free parameter seems useless because the scope of
a variable (which previously called newPDU) can only
be global to a single script invocation. Even if defer==1,
any such PDUs will be freed by the agent upon exit of
the script calling fail. 

It should be extra clear that the fail call is like
a return statement, and no code after this line will
actually be executed. An example would help here.

[pg 57. sec 9.3.13, getParameters]
>From time -> From time

may desire -> may require

Mention that this string is only set via the pmPolicyParameters
MIB object.

[pg 58. sec 9.3.13, getParameters]
    getVar("ifSpeed.$*") < integer(getParameters())
    // The call to 'integer()' is meant to force a numeric '<'
    // operator instead of the string comparison that normally
would
    // result from comparing two strings.

This is a forward reference to the integer() function.

The comment needs to be reformatted because 'would' wrapped.

Then one can store -> In this example, 

to cause -> will cause

[pg 58. sec 9.4.1, regexp]
Example code showing meaningful usage of the match
parameter would be helpful.

[pg 58-59. sec 9.4.2, regexpReplace]
Example code would be helpful.

Are there restrictions on the value of 'replacement'?

Does an RTE occur if the string replacement causes
the 'src' string to overflow?

[pg 60. sec 9.4.8, oidSplice]
What happens if offset > oidlen(oid1)?

The oid1 may be extended even if offset + len < oidlen(oid1).
The text also implies that oid1 will be padded with some 
unspecified value.

[pg 62. sec 9.4.9, parseIndex]
An example for each variant of the function would be helpful.

It would help in the example to show what OID fragment values
are actually stored in each var = parseIndex() call.
The example is misleading because the string ipForwardIfIndex
is not actually allowed.

How does the agent know where the instance portion begins
in the arbitrary OID passed in 'oid';

What does the function return if index > oidlen(oid),
or if type==Oid and len==0, and the count octet read is >
the length of the remaining OID fragment?

This function is confusing because of the complex
parameter overloading. 

[pg 63. sec 9.4.11-13, integer, string, type]
These accessor functions should be explained earlier
and mentioned with the sections on ToInteger and
ToString.  

It is very confusing to see all the ToInteger() text
throughout the document, and then find the accessor
functions with different names.  It would be much more
clear and consistent if the ToFoo operators were removed 
and replaced with these accessor functions.

Meaningful code examples of these functions would help.

[pg 64. sec 9.4.14-16, chr, ord, substr]
Meaningful code examples of these functions would help.

"abs(len) octets" : the abs() function is not defined anywhere.

[pg 65. sec 9.5, Library Accessor Functions]
The full function prototypes for these POSIX functions
should be provided.

No mention of the sprintf format codes is made.

How does each and every format code variant map to a data type
in PolicyScript?

Is an RTE generated if the number of parameters passed
to sprintf does not match the format string?

Is an RTE generated if the type of a parameter passed
to sprintf does not match the format string type?

[pg 65. sec 10, Schedule Table]
There is no overall explanation of time-based scheduling.
This should be in the section on the runtime environment.
This section is out of place here.

The term 'schedule group' is used here but not defined anywhere.

The discussion of the pmScheduleTable should be in
an overview of the MIB.

and it's conditions -> and its conditions

it's pmPolicySchedule variable -> its pmPolicySchedule variable

[pg 68. sec 11, Definitions]
There is no overview section to introduce the MIB groups
as is customary in standard MIBs.  The MIB is harder
to read because of this omission.

A diagram showing the different groups in the MIB
and explanations of each group and its relationship
to the other groups (if relevant) would be helpful.

[pg 69. sec 11, UTF8String]
The UTF8String TC seems very generic, and not specific
to PM at all. It should be defined in a more general TC
document, not here.

the 10646 standard -> the ISO/IEC IS 10646 standard 
  or 
the 10646 standard -> this standard 

[pg 72, sec 11, pmPolicyIndex]
This description is rather terse, I guess because
this INDEX component isn't really used for anything.
It could have served as the pmPolicyPrecedence, but
I suppose the WG figured out a corner case where
it helps to have a arbitrary local integer separate
from the policy precedence.

[pg 72, sec 11, pmPolicyPrecedenceGroup]
There are no examples of this potentially complicated
mechanism in the document. Many runtime environment
features (like this one) are explained and defined 
via MIB objects, which makes them hard to figure out.

[pg 72, sec 11, pmPolicyPrecedence]
it's action -> its action 

The complex runtime behavior should be defined in
a section on the runtime environment. The document
says scheduling an implementation-specific matter,
but there are little rules scattered throughout the
document that contradict that statement.

Why is precedence tie-breaking left to the implementor?
This will make scripts hard to port. It's easy enough
to pick an algorithm (e.g. highest pmPolicyIndex wins).

[pg 73, sec 11, pmPolicySchedule]
Is the terminology used here inconsistent? Other
objects refer to a ready policy, not an active policy.

The complicated interactions between precedence groups
and schedule groups deserve some mention in the MIB 
overview section.  Debugging could be interesting,
especially since the agent isn't even required to
resolve a precedence conflict the same way each time.
(This means a different policyAction can run each time 
as well.)

[pg 75, sec 11, pmPolicyActionScriptIndex]
The term pmPolicyAction is not used anywhere else.
This should be policyAction.

[pg 76, sec 11, pmPolicyConditionMaxLatency]
   "... may wish to re-run .. change to the role strings"
This is another execution environment tidbit scattered
in the document.  All of these should be collected
and placed outside the MIB description clauses.

[pg 77, sec 11, pmPolicyMaxIterations]
This object is allowed to change at any time, which is
difficult to implement.  If an NMS lowers this object
while a script is running in a while loop, will the
agent instantly detect the change? On the next policy
task invocation?  Within some max latency?

Is there a specific RTE error code for maxIterationsExceeded?
I didn't see one. (I guess because there's no mechanism
to expose RTE error code values anyway.)

[pg 77-78, sec 11, pmPolicyTable gauges]
It's not clear what the underlying execution model is,
so it's also unclear what starts the counting and
stops the counting for a particular gauge value.

[pg 79, sec 11, pmPolicyAdminStatus]
it's rowstatus set -> its rowStatus set

This object refers to active policy, not ready policy.
This concept should have a consistent label.

It is odd that this object locks the row parameters
and not the RowStatus object, but okay.

[pg 80, sec 11, Policy Code Table]
This long ASN.1 comment should be in a MIB overview
section instead.

[pg 83, sec 11, Element Type Registration Table]
This long ASN.1 comment should be in a MIB overview
section instead.

What happens to the running script if an entry is removed
from this table. What does the runtime do if the current
loop is the elementType removed? Is there an RTE error
code for this condition?

What happens to the running script if an entry is added
to this table that changes an element filter evaluation?

The document never actually defines what constitutes
an element type, so it's hard to tell how the agent
is supposed to auto-populate this table.

[pg 85, sec 11, pmElementTypeRegOIDPrefix]
The "0.0" system entry is not mandatory (none are),
yet there are accessor functions (like roleMatch)
that need this entry. Is this entry required or
do those accessor functions cause an RTE if they
reference the non-existent 0.0 entry?

[pg 86-87, sec 11, Role Table]
This long ASN.1 comment should be in a MIB overview
section instead.

[pg 88, sec 11, pmRoleElement]
Does this re-indexing rule apply if the element is
in on a different engine? Is there a max latency
for discovering re-indexing?  This is a tall order,
considering other text in the document says the
agent doesn't really know (and doesn't enforce)
the structure of the OID instance in any other cases.

[pg 89, sec 11, pmRoleContextEngineID]
that this role string assignment is valid for. ->
for which this role string assignment is valid.

[pg 90, sec 11, pmCapabilitiesEntry]
This table is confusing because the indexing is flat.
All other accessor functions and MIB tables account for
a context and engineID (like the pmRoleTable) except
this one.  The scripts are supposed to act upon
targets in different contexts and on different
engines, but this table only provides for the
capabilities of the PM agent, not the capabilities
of a target element. This is broken because the
PM agent capabilities are defined by this MIB.

[pg 92, sec 11, pmCapabilitiesOverrideTable]
It's unclear why this is a separate table, instead
of combined in the pmCapabilitiesTable.
If the NMS wants to disable a capability, then
a read-write switch in that table (i.e. move
pmCapabilitiesOverrideState to the other table).

[pg 93-94, sec 11, pmCapabilitiesOverrideState]
What is supposed to happen when the NMS sets this
object to 'valid'? If the agent doesn't have a
particular capability, then is the code for this
"override MIB" supposed to magically appear in the
agent when this object is set to valid?  If not,
then the agent doesn't really have the capability,
so what is the purpose of this feature?

[pg 104, sec 11, pmTrackingEPTable]
Some explanation of why this table is read-create
and the PETable is read-only would be nice.

Does the agent ever create entries in this table?

What happens when corresponding PEEntries are
deleted, or the policy is disabled or deleted?

[pg 105-106, sec 11, pmTrackingEPStatus]
This object is read-create, but there is no RowStatus
in this table. How do entries ever get deleted?

   "This entry will only exist if the calendar for 
    the policy is active..."

How can this be true if the NMS is creating the entry?

What happens if this is set to forceOff while a policy
is running? Does the script complete if it was just
forced off? Does it end with an RTE?

Do other MIB status objects change elsewhere
in the pmPolicyTable when this is set to forceOff? 

exists it's value -> exists, its value

[pg 106, sec 11, pmDebuggingTable]
Some sort of indication of how many log entries
can be supported and how long the agent keeps them
would be useful.  I suspect they were left out
because the memory allocation can get so complicated,
these objects would be hard to implement.

Aa MIB object to identify an application error code
(in addition to the error string) would be useful.

[pg 107, sec 11, pmDebuggingElement]
Why is this INDEX component read-only? It isn't
used in any notification (which would be 
accessible-for-notify anyway).

[pg 108, sec 11, pmNewRoleNotification]
in it's index -> in its index

[pg 112, sec 14, Acknowledgements]
No mention whatsoever, or reference, is given for the
Scheduling MIB, even though several hundred lines of
text were lifted from that MIB.  The cut-and-paste
is bad enough!

[pg 116, sec 17, Intellectual Property]
I think different text is supposed to go here if the WG
knows of any claims of patented IP pertaining to this work,
and I believe BMC Software is making such a claim.

[pg 118, ToC]
This should go in the front of the document.

[SMILINT WARNINGS]
./POLICY-BASED-MANAGEMENT-MIB:2013: [4] object identifier name `pmAbnormalTerminationNotification' longer than 32 characters
./POLICY-BASED-MANAGEMENT-MIB:801: [5] index of row `pmElementTypeRegEntry' can exceed OID size limit
./POLICY-BASED-MANAGEMENT-MIB:971: [5] index of row `pmRoleEntry' can exceed OID size limit
./POLICY-BASED-MANAGEMENT-MIB:1086: [5] index of row `pmCapabilitiesEntry' can exceed OID size limit
./POLICY-BASED-MANAGEMENT-MIB:1212: [5] index of row `pmCapabilitiesOverrideEntry' can exceed OID size limit
./POLICY-BASED-MANAGEMENT-MIB:1670: [5] index of row `pmTrackingPEEntry' can exceed OID size limit
./POLICY-BASED-MANAGEMENT-MIB:1782: [5] index of row `pmTrackingEPEntry' can exceed OID size limit
./POLICY-BASED-MANAGEMENT-MIB:1891: [5] index of row `pmDebuggingEntry' can exceed OID size limit