MIB Doctor review (part-1) for: draft-ietf-ips-isns-mib-09.txt
"Wijnen, Bert (Bert)" <[email protected]>
| Newsgroups | gmane.ietf.ips |
|---|---|
| Message-ID | <7D5D48D2CAA3D84C813F5B154F43B1550A004AF9@nl0006exch001u.nl.lucent.com> |
Dan asked for a volunteer for MIB doctor review, and
I offered to do so. Here is my review part-1:
- Syntax checking
SMICng tells me:
W: f(isns.mi2), (2626,19) Row "isnsRegFcNodePortEntry" does not
have a consistent indexing scheme - cannot specify an index
item from additional "base row" isnsRegFcNodeEntry, since
can have only one "base row" which is isnsRegFcPortEntry
W: f(isns.mi2), (294,7) Textual convention "IsnsNodeIndexIdOrZero"
defined but not used
Both are probably OK as long as you are sure that this is what you
intend for the first warning.
For the second warning one could wonder wht the TC was defined if
it is not (yet?) used. Maybe another MIB module is using it?
- smilint has no complaints.
- I am somehwat confused by:
IsnsEntityProtocolId ::= TEXTUAL-CONVENTION
STATUS current
DESCRIPTION
"The type of protocol that is supported by this entity.
Type Value Entity Type
---------- -----------
1 No Protocol
2 iSCSI
3 iFCP
All Others As in the iSNS Specification
"
REFERENCE "RFC 4171, Section 6"
SYNTAX INTEGER { noProtocol(1),
iSCSI(2),
iFCP(3) }
Since this is an ENUMERATION, I have difficulty understanding what
"All Others" means in the DESCRIPTION clause.
Now if I go to the RFC4171, then I see that IANA assigns new values (and so
I think that that is meant here). So I then wonder if it would not be better
to move this to an IANA maintained MIB module that is kept in sync with the
registry that IANA already must maintain, i.e. the registry at
http://www.iana.org/assignments/isns-parameters ?
- I also get confused by:
IsnsPortalGroupTagIdOrZero ::= TEXTUAL-CONVENTION
DISPLAY-HINT "d"
STATUS current
DESCRIPTION
"The Portal Group Tag (PGT) TC for iSCSI Portal Group
objects registered in the iSNS. The value of zero
indicates a NULL value, or no association, between the
associated Portal and iSCSI Node."
REFERENCE "RFC 4171, Section 6"
SYNTAX Unsigned32 ( 0 .. 65535 )
Sect 6.5.4 of 4171 claims that zero is a valid PGT value/ID,
and that a NULL PFT would be expressed by using a zero length
in a TLV. So is the use of zero here in conflict with that sect 6.5.4?
If not, pls explain why not (and do so in the DESCRIPTION clause.
- When I see IsnsPortalSecurityBitmapId ::= TEXTUAL-CONVENTION
Then I first wonde if this is an "Id" (Identifier?) of if the name
would better be
IsnsPortalSecurityBitmap ::= TEXTUAL-CONVENTION
But I am more worried about the fact that the bit numbers are different
from what is described in sect 6.3.9 of RFC4171. If the WG wants to
do it this way conscuiously, then fine, but imagine what happens if
other bits get used in the fture (say 23 and 24) and we map those to
bits 7 and 8 in the TC, and then yet later bits 21 and 22 get used
and we map them to bit 9 and 10. Won;t that start to be confusing?
Would it not be handier to define it as
SYNTAX BITS {
reserved0(0),
reserved1)1),
...
reserved24(24),
tunnelModePreferred(25),
transportModePreferred(26),
pfsEnabled(27),
agressiveModeEnabled(28),
mainModeEnabled(29),
ikeIpsecEnabled(30),
bitmapVALID(31)
}
So that it maps directly onto the bits in 4171 sect 6.3.9 ??
- for IsnsNodeIndexIdOrZero I guess that the value zero often means
none, i.e. no NodeIndexID exists. But I could see it means
something else depending on the object that uses this syntax.
I would suggest to change the DESCRIPTION clause to something aka:
"This textual convention is an extension of the IsnsNodeIndexId
textual convention. The latter defines a greater than zero
value used to identify an IsnsNode. This extension permits the
additional value of zero. The value zero is object-specific
and MUST therefore be defined as part of the description of
any object which uses this syntax. Examples of the usage of
zero might include situations where the IsnsNode was unknown,
or when none or all IsnsNodes need to be referenced."
- IsnsNodeTypeId is it an Id (Identifier?)? or is it actually a map
(or list) of nodeTypes? Good names make sense in my view.
Again, I wonder if mapping it to actualy the same bit positions as in
RFC4171 would not be better.
- IsnsCosBitmapId is it an Id (Indetifier?)?
Same question on mapping bits
- Same for IsnsScnBitmapId
- Same for: IsnsSrvrDscvryMthdId
Seems like a map of (supported?) methods as opposed to an ID.
- When I see:
isnsSrvrInstPhyIndex OBJECT-TYPE
SYNTAX Unsigned32 (0..2147483647)
MAX-ACCESS read-only
STATUS current
DESCRIPTION
"An index indicating the location of this iSNS Server within
a larger entity, if one exists. If the iSNS Server instance
is not part of a larger entity, then the value is 0."
REFERENCE "RFC 4171"
::= { isnsSrvrInstEntry 5 }
Then I am not sure how this "index" tells me anything about the
location within a larger entity. Possibly it dioes because it is
an index into some other table, but can you pls specify which table
that would be. If it is not an index into some other table,
then pls explain how it helps in determining "location"?
- Why does
isnsSrvrInstRole OBJECT-TYPE
SYNTAX INTEGER { notSet(0),
server(1),
serverNotPrimary(2) }
not start ENUMerating at 1 instead of zero.
We recommend starting at 1 unless there is a good reason to start
at zero (which we then like to see mentioned in the DESCRIPTION clause)
I can't find why starting at zero is required.
Is there any specific section in RFC4171 about this?
I see a section on bacup (2.8), that speaks about an "active server"
which I do not see mentioned here. Is "serving as a primary"
teh same as "active server" ?? That section also speaks about
"backup server" which I do not see here? The section indeed also
speaks about a "primary server"
In any event, I am not sure if and how this object is related to
section 2.8. Maybe it is not and maybe it is related to something else?
- isnsSrvrInstDiscMcGrp
Whever you define an object with SYNTAX of InetAddress, then (according
to the DESCRIPTION clasue of InetAddress in RFC4001), you MUST state
WHICH object of SYNTAX InetAddressType specifies the format of this
object. It seems obvious that this is isnsSrvrInstDiscMcGrpType, yet
it is good to mention that.
Further, it states:
for this server instance. If not configured, then
the value is an empty string."
But, if it is not configured, then the isnsSrvrInstDiscMcGrpType has
a value of unknown (or so I assume), and the value of this object then
better be the "zero length string" as opposed to "empty". What does
"empty" mean?
I would personally rename isnsSrvrInstDiscMcGrp to isnsSrvrInstDiscMcGrpAddress
- W.r.t. isnsSrvrInstDiscMcGrpType and isnsSrvrInstDiscMcGrp, I think
one could say some more about the allowed InetAddressTypes (if not in the
DESCRIPTION clauses of these objects themselves, then at least in a
OBJECT clause in the MODULE-COMPLIANCE statements.
If I understand things correctly, it has to be an IP multicast address,
so possibly only the types "unknown", "ipv4" and "ipv6" are allowed?
If "dns| is allowed, then you need to add text as to when a DNS name
would be resolved (as per RFC4001).
- isnsSrvrInstEsiNonRespThrshld, isnsSrvrInstEnblCntrlNdeMgtScn and
isnsSrvrInstCntrlNodeAuth, isnsSrvrInstDfltDdDdsStatus and
isnsSrvrInstUpdateDdDdsSpprtd and a few more that follow
These objects have a REFERENCE "RFC 4171, Section 3.4"
but maybe you mean sect 2.4 ??
- I can't say that I find the DESCRIPTION clause for isnsSrvrInstCntrlNodeAuth
very well written. I still need to review the other tables it is pointing
to, so I can't say much more yet.
nits/typos/consistency/questions:
- I wonder why IsnsDdsStatusId ::= TEXTUAL-CONVENTION
is not just named IsnsDdsStatus ::= TEXTUAL-CONVENTION
I.e. I do nto see why it is an Id (Identifier?)??
Further, IsnsDdsStatusId ::= TEXTUAL-CONVENTION
STATUS current
DESCRIPTION
"The bitmap indicating the status of a Discovery Domain
Set (DDS) registered in the iSNS.
Bit Status
--------- ---------
0 enabled
If bit(0) is set to true then the DDS is Enabled. Otherwise
the DDS is disabled."
REFERENCE "RFC 4171, Section 6"
SYNTAX BITS {
enabled(0)
}
"If bit(0) is set to true" ??? I understand what is meant.
But I think it would be cleared to just say
"If bit(0) is set to 1" or "If bit(0) is set"
Or/and be consistent with how you describe the setting of a bit
with other BITS TCs like DdFeatureBitmapId
- For consistency, I would rename DdFeatureBitmapId ::= TEXTUAL-CONVENTION
to IsnsDdFeatureBitmapId ::= TEXTUAL-CONVENTION
or even better: IsnsDdFeatureBitmap ::= TEXTUAL-CONVENTION
again, I do not see how this is an Id (Identifier?)??
- isnsSrvrInstEsiNonRespThrshld ... is this an Id (Indetifier?) or is it a threshold.
From the descritpion clause it seems it is the latter. So I would rename to
isnsSrvrInstEsiNonRespThrsh
Mmm... now I see, the l is probably an el and not a one.
Why abbreviate "hold" to "hld" ??
In fact why abbreviate "Threshold" to "Thrshld". We (readers) are not all Americans
or native English speakers. In fact this whole doc uses (for my taste) far to
much (strange) abbreviations for object descriptors and labels. But who is me.
- isnsSrvrInstUpdateDdDdsSpprtd and isnsSrvrInstUpdateDdDdsSpprtd
use a TC for theri SYNTAX. The idea of a TC is that you only define the
semantics in teh DESCRIPTION clause of the TC so you do not have to
repeat it everytime that the TC is used as a SYNTAX.
admin/bureaucracy:
- You may want to check the occurences of "MIB", which in many cases woul be
better stated as "MIB module".
- references/citations:
!! Contains embedded space:
P001 L134: network [RFC 4171]. It has the capability to group devices into
!! Contains embedded space:
P001 L264: Specification [RFC 4171], a DDS can be enabled or disabled,
!! Contains embedded space:
P001 L307: As described in iSCSI [RFC 3347], Portal Groups provide a
The first two are indeed just what it says, namely a blank in between
RFC and the actual number. In the references section, you list it as [RFC4171]
without a space.
The 3rd does have an embadded space too, but also, that RFC does not
show up in the references section.
_______________________________________________
Ips mailing list
[email protected]
https://www1.ietf.org/mailman/listinfo/ips