remaining comments on draft-ietf-ipoib-ibif-mib-05.txt

Hi -

Here are the rest of my review comments on sections 5 and 6 of
draft-ietf-ipoib-ibif-mib-05.txt, resuming with section 5.2.7.
Executive overview: it needs another cycle as an i-d.

Details:

"MTU" should be expanded on first use.

ifInOctets: "should" -> "MUST"

ifInUcastPkts: "Note," -> "Note"

ifInErrors: it looks like the text is what should have been
in the entry for ifInDiscards.  Is this intentional?

ifInUnknownProtos: is an unknown protocol really *impossible*?

ifOutOctets: "should" -> "MUST"

ifOutUcastPkts: "Note," -> "Note"

ifInMulticastPkts:  Remove the sentence "Always 0 unless proprietary
counters are implemented."

ifOutMulticastPkts:  Remove the sentence "Always 0 unless proprietary
counters are implemented."

ifInBroadcastPkts
ifOutBroadcastPkts
ifHCInBroadcastPkts
ifHCOutBroadcastPkts: remove the "always 0" language.  If broadcast
is impossible, say so, and indicate that as a consequence, these
counters have nothing to count.


ifHCInUcastPkts
ifHCInMulticastPkts
ifHCOutUcastPkts
ifHCOutMulticastPkts: remove the fragment "Always 0 unless proprietary
counters implemented."

If you know that there is a substantial installed based of
systems incapable of counting some of these events, it
might not hurt to put in a "reminder" reference to section
3.1.18 of RFC 2863 or to simply clone that text.


Section 5.3:  expand the VL (virtual Lane) abbreviation on first use.

Section 5.4:
"Note," -> "Note"

Replace this sentence: "Note, that since multicast counters have not been
defined by the IBA specification, IF-MIB multicast counters should
be set to 0 (in the absence of any proprietary IBA multicast
counters)."  Just what you replace it with should probably be
hammered out in the WG.  :-)  My foremost concern is that we do
not violate section 3.1.18 of RFC 2863.

Replace this sentence: "Also, based on the IBA counter definitions,
multicast (packets and bytes) will be part of the IF-MIB unicast counters."
It should be clear that, for example, ifInOctets MUST include the multicast
octets, but ifInUcastPkts MUST NOT include multicast packets.

The mapping .PortRcvPkts = IF-MIB:ifInUcastPkts sounds like it's
not right, since it would incorrectly include non-unicast packets.
(If it *is* right, then the other mappings are suspect.)  Ditto
the formula for IF-MIB:ifOutUcastPkts

What is the rationale behind this note?  "Note: that
PortCounters.PortRcvSwitchRelayErrors is not counted in
either IF-MIB:ifInDiscards or IF-MIB:ifOutDiscards."

Section 5.5:
(the diagram leaves of a lot of interesting stuff, like how
ibIfPortStatInactDiscards fits into the tallies.)

Modify this sentence:
"Again it should be noted that since no multicast
counters are defined by the IBA specification they are not covered here."
I'd be happy with something to the effect that since the IBA specs do not
define the necessary multicast counters, implementation-specific
techniques may be needed to compute those values.

ifOutErrors:
rather than specifying zero, it should just say that it's
impossible for anything to fail in such a way that a packet
would not be able to be transmitted.  Of course, if that's
not true, we have a bigger problem here.

In the case diagram, I think it would be beneficial to tweak
the ASCII art so that labels like PC.PortXmitConstraintErrors
aren't split across lines.  I played around with it a bit, and
it IS possible without too much effort.

More Globals:
Replace "[Infini v1.1]" with "[INFINIV1] to match what's in the
references section.

Section 6.0:
CONTACT-INFO clause MUST include WG mailing list information,
etc.  See <draft-ietf-ops-mib-review-guildelines-02.txt> section 4.5.

Page 13: the REVISIONs and DESCRIPTIONs should be collapsed to a
single pair, in accordance with page 12 of the MIB review guidelines.

Need to add copyright notices to MODULE-IDENTITY per MIB review
guidelines section 3.8.

ibIfPortSymbolErrs (and other counters): The word "total" isn't
quite right, since these are *counters* with no pre-defined
starting value and the potential for wrapping.  This may seem
like an editorial nit, but I think it's good to be mindful of it,
especially since application writers will get themselves into
trouble if they take the word "total" literally.  A more accurate
description would say something to the effect that this counter
is incremented whenever X occurs for some Y.


The DESCRIPTION of ibIfPortStatSwLifetimeDiscards and
ibIfPortStatHOQLifetimeDiscards says:
"Note, applies only to switches."  The MIB conformance
material doesn't mention switches, so I left wanting to know
what a system which is not a switch is supposed to do:
    a) implement this object, but since the associated event
 never happens, its value won't be incremented
    b) not implement this object
I personally prefer (a), but the MIB should clearly state
what you want implementors to do.

HOQ should be expanded on first use. (page 18)

ibIfPortStatLinkIntergrityErrs mentions "the LocalPhyErrors threshold"
but doesn't say what it is or how one would set or get it.  Furthermore,
if it's really a counter, then this description is probably incorrect,
since the behaviour described would be more appropriate for a gauge
reporting the change in a counter over some period of time.

ibIfPortStatExcBufOverrunErrs mentions "OverrunErrors" without
describing how one would get or set this attribute.  Also, the
current text is ambiguous.  For example, if OverrunErrors is
two, and errors have occured in three consecutive periods,
would this counter be incremented once or twice?

ibIfPortStatVL15Dropped: what is meant by VL15 here, and why is it
special?

Does the language about packets not included in ibIfVLInPkts also
apply to ibIfVLInOctets?  Ditto ibIfVLOutPkts and ibIfVLOutOctets.

ibIpoibLinkLayerIndex should be Unsigned32 rather than INTEGER,
per the MIB review guidelines (top of page 14).

However, a more fundamental question is whether there's any
need for ipIpoibLinkLayerIndex at all.  It seems to me that
the uniqueness requirements (which aren't at all clear from
the current DESCRIPTION text) could at least be partly expressed
by making ibIpoibLinkLayerAddr the second index of the
ibIpoibLinkLayerAddrTable, which would need to be accessible
since there are no other accessible objects in this table.

In any case, the DESCRIPTION for ibIpoibLinkLayerAddr needs
to be cleaned up; the note appears to start out as a restatement
of the description, but then appears a baffling "in either case".

What is the rationale for defining ibIfVlTrafficIfIndex,
ibIfPortStatIfIndex, and ibIpoibLinkLayerIfIndex as separate
objects?  The MIB review guidelines (page 20) recommend using common
indexes in cases like these.  Indeed, for a "sparse augments"
like this I would read the guidelines as saying one should just
use ifIndex, just as other media specific MIBs have done.

Randy