Pages 45-71 of draft-ietf-ipoib-subnet-mgmt-agent-mib-05.txt

Hi -

Additional comments on <draft-ietf-ipoib-subnet-mgmt-agent-mib-05.txt>
from pages 45 to 71.

ibSmaSL2VLMapEntry: the INDEXing results in the table being
organized by output port.  This is reasonable, but not what
I would have expected.  Was the order of the indexes a
conscious design decision?

ibSmaIBAOutPortIndex & ibSmaIBAInPortIndex: these seem to
be a bizarre use of the TC.  Using 255 to represent 0,
especially when the TC says that it *doesn't* represent
the management port 0, is just too twisted.  Defining a
separate TC with a valid range covering all ports (0..254)
would seem to be a cleaner and less error-prone solution.

ibSmaServiceLevelIndex: service level appears in enough
places that I think you should consider defining a TC.
In any case, since the range is limited to positive numbers,
the underlying type should be Unsigned32.

ibSmaVirtualLane: should be Unsigned32.

ibSmaHiPriVlArbTable: what is the lifetime / persistence
of this information?  When do entries come and go?  How
does a management system detect whether the table has changed?

ibSmaHiPriIBAPortIndex: the description doesn't match up
with the fact that the contents are read-only.

ibSmaHiPriNIndex: the DESCRIPTION of this index is not helpful.
For example, what do the range constraints correspond to?
Should be Unsigned32

ibSmaHiPriVirtLane:  Should be Unsigned32.  Why is the range
different from that of ibSmaVirtualLane?

ibSmaHiPriWeight: should be Unsigned32.  A UNITS clause might
be nice.

ibSmaLowPriVlArbTable: what are lifetime / persistance characteristics?
How does management system learn that the table has changed?

ibSmaLowPriNIndex: should be Unsigned32.  Would it be worthwhile
to share a TC with ibSmaHiPriNIndex?

ibSmaLowPriVirtLane:  Should be Unsigned32.  Why is the range
different from that of ibSmaVirtualLane?  Should it share a TC
with ibSmaHiPriVirtLane?

ibSmaLowPriWeight: should be Unsigned32.  Units might be nice.
Could it share TC with ibSmaHiPriWeight?

One *MIGHT* consider combining these two tables into a single
one, in which the first index would be INTEGER { high(1),low(2) }
or something like that.

ibSmaLinForTable: lifecycle, persistance, etc.

ibSmaLinForwEgressPort: what happens when the value is 255?
It's impossible to forward to port zero?

ibSmaRandomForwardingTable: lifecycle, persistance, etc.

ibSmaRandomDestLID: why doesn't this use the
IbUnicastLid TC?  Should be Unsigned32 in any case.

ibSmaRandomLMC should be Unsigned32

ibSmaRandomIsValid: description should say "false" rather than "0".

ibSmaMulForTable: lifecycle, persistance, etc.

ibSmaSmInfoTable: lifecycle, persistance, etc.

ibSmaSmSmKey: it sounds like this merits a special mention in the
security considerations section.  it it's not a key, then the
description should explain just what it is.  In any case, access
control to this MIB data should be handled exactly like access
control to any other MIB data, so I think the stuff about
"this is shown as zeroes" etc. should be removed. Having mib-
specific access control decisions like this is extremely messy to
implement with subagent technologyies, and should be avoided
in favor of standard VACM.

ibSmaSmPriority: should be Unsigned32, looks ripe for a TC.
In both cases, is the "shall" supposed to be a "SHALL"?
I wouldn't object to a DEFVAL on the object definition.

ibSmaSmState: it seems very strange to have the SYNTAX
enumeration values and the ones in the DESCRIPTION not match.
Suggest using the inifiband numbers directly, unless there
is some good reason not to.

ibSmaVendDiagInfoTable: lifecycle, persistance, etc.

ibSmaVendDiagPortIndex:
a bizarre use of the TC.  Using 255 to represent 0,
especially when the TC says that it *doesn't* represent
the management port 0, is just too twisted.  Defining a
separate TC with a valid range covering all ports (0..254)
would seem to be a cleaner and less error-prone solution.

ibSmaPortGenericDiagCode: the DESCRIPTION should spell
out the significance of each of the enumeration values.

ibSmaPortVendorDiagCode: should be Unsigned32.

ibSmaPortVendorDiagIndexFwd & ibSmaPortVendorDiagData:  this
is a really strange pair of objects.  It would seem simpler
to define a single OCTET STRING with length of (0 | 62 | 124)
to achieve the same funtion, while avoiding the question of
what the 124-byte buffer would be filled with if only 62
bytes were actually used.  Also "n number" appears to be a typo.
Of course, the question of why one would only hold the first
two also comes to mind.  Other indexing structure could also help,
but it depends on what you really want to accomplish with this.

ibSmaLedIndex: the phrase "that can have an LED" raises an
interesting question about this table: will entries exist
for all ports, whether they have physical LEDs or not?  Also,
the DESCRIPTION makes it sound like only one port on the
system could have an LED, which seems odd to me.

For all NOTIFICATION-TYPEs: please take a look at the last paragraph
in draft-ietf-ops-mib-review-guidelines-02.txt section 4.7 on
notification throttling.  If doing bad things to the medium can
cause management stations to be flooded with notifications, this
needs to be addressed in the security considerations.

Randy