MIB doctor re-review: draft-ietf-ips-isns-mib-10.txt
"Wijnen, Bert \(Bert\)" <[email protected]>
| Newsgroups | gmane.ietf.ips |
|---|---|
| Message-ID | <[email protected]> |
All, pls note that my email address changed to:
[email protected]
The old one will work for a few months, but will disappear
sometime early next year (2007).
My comments on the revision 9 are attached at the very bottom,
in case someone wants quick check of that.
In general, I think good improvements have been made.
For rev 10, SMICng tells me:
W: f(isns.mi2), (2648,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
*** 1 warning in parsing
As I said last time, I think it is OK, as long as the WG agrees
that this is what they want to do. I think I did see a reponse
that explained that this is indeed wanted/intended. I won't
comment on it anymore and assume that the WG chair makes sure
that it is indeed intended. I can use a switch in SMICng to turn
off the warning, which I have now done.
I do not know what the answer is to my comment on rev 9:
> - When I looked at IsnsDdDdsModificationBitmap again, I was
> somewhat surprised by bit field zero:
> instance. Although this MIB does not allow modification
> of DD's and DDS's, SNMP may be used to modify them via
> another MIB.
> 0 SNMP protocol is allowed to modify
> DD's/DDS's
> This MIB module is read-only as you say. SOme other MIB module
> may allow changes. Mmm... is it then logical to express in this
> MIB module if such can be possibly be done somewhere else? Is
> that somewhere else on the same system as where this MIB module
> is instantiated? If not, how easy is it to represent that here
> (if at all possible)??
>
In the rev 10 of the doc, this is about the IsnsDdDdsModificationType
Textual Convention.
It seems that my comment on rev 9:
> - isnsNumObjTable OBJECT-TYPE
> SYNTAX SEQUENCE OF
> IsnsNumObjEntry
> MAX-ACCESS not-accessible
> STATUS current
> DESCRIPTION
> "Table providing the number of registered objects of each
> type in the iSNS Server instance. This table is optional
> to implement. The number of entries is dependent upon the
> number of iSNS Server instances being managed."
> ::= { isnsSrvrInfo 2 }
>
> The fact that this table is "optional" should not be stated here
> in the DESCRIPTION clause. Nether should in the DESCRIPTION clause
> of isnsServerNumObjGroup OBJECT-GROUP
> be stated that:
> associated with a registered Entity. These managed objects
> are optional to implement."
> The fact if objects are optional should be expressed by properly
> grouping them (which I think has been done) in an OBJECT-GROUP and
> then make that OBJECT-GROUP and optional group in the
> MODULE-COMPLIANCE.
> The latter has not been done, because:
> isnsIscsiServerCompliance MODULE-COMPLIANCE
> STATUS current
> DESCRIPTION
> "Initial compliance statement for an iSNS Server
> providing support to iSCSI clients."
> MODULE -- this module
> MANDATORY-GROUPS {
> isnsServerAttributesGroup,
> isnsServerIscsiCntlNodeGroup,
> isnsServerIscsiDdsDdObjGroup,
> isnsServerRegIscsiObjGroup,
> isnsServerNumObjGroup,
> isnsNotificationsObjGroup,
> isnsServerNotificationGroup
> }
> ::= { isnsCompliances 1 }
> shows that those claims in the DESCRIPTION clauses are INCORRECT.
> The above MODULE-COMPLIACNE shows this group as a MANDATORY-GROUP.
> In fact in the 2nd MODULE-COMPLIANCE, the group is also listed as
> MANDATORY. So it seems it is always mandatory according to the
> currently know MODULE-COMPLIANCE statements.
>
> Pls remove also the "optional" language in isnsRegEntityNumObjTable
>
got ignored? The current rev 10 has it in isnsNumObjectsTable,
isnsRegEntityNumObjectsTable, isnsServerNumObjectsGroup.
For the isnsServerNumObjectsGroup for example, the DESCRIPTION
clause says it is an optional group, while this module compliance
says it is mandatory:
isnsIscsiServerCompliance MODULE-COMPLIANCE
STATUS current
DESCRIPTION
"Initial compliance statement for an iSNS Server
providing support to iSCSI clients."
MODULE -- this module
MANDATORY-GROUPS {
isnsServerAttributesGroup,
isnsServerIscsiControlNodeGroup,
isnsServerIscsiDdsDdObjGroup,
isnsServerRegIscsiObjGroup,
isnsServerNumObjectsGroup,
isnsNotificationsObjGroup,
isnsServerNotificationGroup
}
::= { isnsCompliances 1 }
In fact, the 2nd module copmpliance: isnsIfcpServerCompliance
also has it listed as a mandatory group.
Mmm... the more I am re-checking, the more I wonder if my
comments that came in part 2 were looked at and/or responded to
at all. I saw the responses that Kevind did send on Dec 10th,
but I believe they are all just responses to part-1 of my review.
Below is both part 2 (at top) and part 1 (at bottom) of my earlier
review.
Can someone let me know before I continue on re-reviewing part2?
Remaining NITs:
- I still see "IPSec" at various places. I believe the Security Ads
rather see "IPsec", so with a lowercase s.
Bert
> -----Original Message-----
> From: Wijnen, Bert (Bert)
> Sent: woensdag 7 juni 2006 17:59
> To: 'Kevin Gibbons'; 'David Black (E-mail)'; 'Scott Kipp';
> '[email protected]'
> Cc: '[email protected]'; Dan Romascanu (E-mail)
> Subject: MIB Doctor review (part-2) for:
> draft-ietf-ips-isns-mib-09.txt
>
> [copied Dan Romascanu; not sure if he is on IPS mlist]
>
> Sorry it took a while. My excuse is that the MIB module alone
> is over 3000 lines long, not small. I am also not an IPS
> expert, so I did have to go and check 4171 many times.
> I am still not able to fit the whole picture of tables with
> all their aspects in my head. May I assume that the subject
> matter experts have done that or will do so?
>
> part-1 is appended at the bottom, so people have it all in
> one email if such is easier.
>
> - My last point in part 1 was:
> > - 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.
>
> that Description clause states as a last point:
> If SNMP is not allowed to view or modify the list of control
> nodes, then this managed object SHALL be set to
> noSnmpAccess."
> so does that mean that if the value is noSnmpAccess that there
> then are no entries to be shown in the isnsCntlNodeIscsiTable?
> The description clause of isnsCntlNodeIscsiTable says so.
> So it would be good to repeat that here in the DESCRIPTION
> clause of isnsSrvrInstCntrlNodeAuth as well (I think).
> Maybe it should also state that isnsCntlNodeFcPortTable
> is empty in that case.
> By the way, it might be good to also explain that SnmpAccess
> is read-only (although that shouldbe clear seeing that the
> two tables are both read only.
>
> - When I looked at IsnsDdDdsModificationBitmap again, I was
> somewhat surprised by bit field zero:
> instance. Although this MIB does not allow modification
> of DD's and DDS's, SNMP may be used to modify them via
> another MIB.
> 0 SNMP protocol is allowed to modify
> DD's/DDS's
> This MIB module is read-only as you say. SOme other MIB module
> may allow changes. Mmm... is it then logical to express in this
> MIB module if such can be possibly be done somewhere else? Is
> that somewhere else on the same system as where this MIB module
> is instantiated? If not, how easy is it to represent that here
> (if at all possible)??
>
> Further, I see that everytime this TC is used as a SYNTAX, the
> same sort of DESCRIPTION clause gets repeated. The idea of a TC
> is to describe the semantics ONCE and not to repeat that many
> times (with the risk of creating inconsistencies or conflicts
> or ambiguity). So for example, I would limit the DESCRIPTION
> clause in isnsSrvrInstUpdateDdDdsSpprtd as follows:
>
> isnsSrvrInstUpdateDdDdsSpprtd OBJECT-TYPE
> SYNTAX IsnsDdDdsModificationBitmap
> MAX-ACCESS read-only
> STATUS current
> DESCRIPTION
> "The methods that this iSNS Server instance supports
> to modify Discovery Domains and Discovery Domain Sets."
> REFERENCE "RFC 4171, Section 3.4"
> ::= { isnsSrvrInstEntry 17 }
>
> If you agree, pls look at other uses of ths (and other) TCs as well.
>
> - isnsNumObjTable OBJECT-TYPE
> SYNTAX SEQUENCE OF
> IsnsNumObjEntry
> MAX-ACCESS not-accessible
> STATUS current
> DESCRIPTION
> "Table providing the number of registered objects of each
> type in the iSNS Server instance. This table is optional
> to implement. The number of entries is dependent upon the
> number of iSNS Server instances being managed."
> ::= { isnsSrvrInfo 2 }
>
> The fact that this table is "optional" should not be stated here
> in the DESCRIPTION clause. Nether should in the DESCRIPTION clause
> of isnsServerNumObjGroup OBJECT-GROUP
> be stated that:
> associated with a registered Entity. These managed objects
> are optional to implement."
> The fact if objects are optional should be expressed by properly
> grouping them (which I think has been done) in an OBJECT-GROUP and
> then make that OBJECT-GROUP and optional group in the
> MODULE-COMPLIANCE.
> The latter has not been done, because:
> isnsIscsiServerCompliance MODULE-COMPLIANCE
> STATUS current
> DESCRIPTION
> "Initial compliance statement for an iSNS Server
> providing support to iSCSI clients."
> MODULE -- this module
> MANDATORY-GROUPS {
> isnsServerAttributesGroup,
> isnsServerIscsiCntlNodeGroup,
> isnsServerIscsiDdsDdObjGroup,
> isnsServerRegIscsiObjGroup,
> isnsServerNumObjGroup,
> isnsNotificationsObjGroup,
> isnsServerNotificationGroup
> }
> ::= { isnsCompliances 1 }
> shows that those claims in the DESCRIPTION clauses are INCORRECT.
> The above MODULE-COMPLIACNE shows this group as a MANDATORY-GROUP.
> In fact in the 2nd MODULE-COMPLIANCE, the group is also listed as
> MANDATORY. So it seems it is always mandatory according to the
> currently know MODULE-COMPLIANCE statements.
>
> Pls remove also the "optional" language in isnsRegEntityNumObjTable
>
> - I wonder if the following would be better represented by a SYNTAX
> of Gauge32:
> IsnsNumObjEntry ::= SEQUENCE {
> isnsNumDds Unsigned32,
> isnsNumDd Unsigned32,
> isnsNumEntities Unsigned32,
> isnsNumPortals Unsigned32,
> isnsNumPortalGroups Unsigned32,
> isnsNumIscsiNodes Unsigned32,
> isnsNumFcPorts Unsigned32,
> isnsNumFcNodes Unsigned32
> }
> There are probably other objects that are better
> represented as a Gauge32
> as well. I can live with Unsigned32 though.
>
> Question for my understanding: Are these counters intended
> to help determine
> the max-repetitions for a getbulk?
>
> same for objects in isnsRegEntityNumObjTable
>
> - isnsCntlNodeFcPortName OBJECT-TYPE
> SYNTAX FcNameIdOrZero
> MAX-ACCESS not-accessible
> STATUS current
> DESCRIPTION
> "The FC Port WWN that can and/or is acting as a control
> node for the specified iSNS Server. Zero is not a valid
> value for this managed object. This managed object,
> combined with the isnsSrvrInstIndex, is the key for this
> table."
> ::= { isnsCntlNodeFcPortEntry 1 }
>
> I think it is better to s/Zero/A zero length string/
>
> - In addition to my earlier comment on
> IsnsDdsStatusId ::= TEXTUAL-CONVENTION
> As far as I see, it is only used once. So that begs the question
> why it is a TC.
> But even so, in the case where it is used in this MIB module:
> isnsDdsStatus OBJECT-TYPE
> SYNTAX IsnsDdsStatusId
> MAX-ACCESS read-only
> 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. If set
> to false then the DDS is disabled."
> REFERENCE "RFC 4171, Section 6"
> DEFVAL { { enabled } }
> ::= { isnsDdsEntry 3 }
>
> It seems to me that it would be more appropriate to rename the
> isnsDdsStatus objct to isnsDdsEnabled and *re-(use the TruthValue
> TC from RFC2579.
>
> - naming consistency:
> IsnsDdIscsiMemberEntry::=
> SEQUENCE {
> isnsDdMemberIscsiIdx IsnsNodeIndexId,
> isnsDdMemberIscsiName SnmpAdminString,
> isnsDdMemberIsRegistered TruthValue
> }
> why are they not named:
> IsnsDdIscsiMemberEntry::=
> SEQUENCE {
> isnsDdIscsiMemberIdx IsnsNodeIndexId,
> isnsDdIscsiMemberName SnmpAdminString,
> isnsDdIscsiMemberIsRegistered TruthValue
> }
>
> I would myself also rename isnsDdIscsiMemberIdx to
> isnsDdIscsiMemberIndex
>
> - naming consistency
> IsnsDdPortalMemberEntry ::=
> SEQUENCE {
> isnsDdMemberPortalIdx IsnsPortalIndexId,
> isnsDdMemberPortalAddrType InetAddressType,
> isnsDdMemberPortalAddr InetAddress,
> isnsDdMemberPortalPortType IsnsPortalPortTypeId,
> isnsDdMemberPortalPort InetPortNumber,
> isnsDdMemberPortalIsRegistered TruthValue
> }
> I would start all names with isnsDPortalMember.
>
> - isnsDdMemberPortalAddr OBJECT-TYPE
> SYNTAX InetAddress
> MAX-ACCESS read-only
> STATUS current
> DESCRIPTION
> "The Inet Address for the portal."
>
> According to RFC4001, you MUST specify which object of SYNTAX
> InetAddressType specifies/controls the format of an InetAddress
> object. I guess it is clear that isnsDdMemberPortalAddrType is
> that object. But pls make that statement in description clause.
> Is dns a valid type or does it need to be supported?
> If not, may wnt to express that in MODULE-COMPLIANCE.
> If yes, may want to say somethingas to when that dns name is
> resolved (as required per rfc4001)?
> Maybe it is never a dns name (if I understand that it is
> max 16 octets
> as per sect 6 of rfc4171) Maybe it is only IPv4 and/or IPv6?
> You could add that to the SYNTAX of the InetAddressType object if
> it will never be different.
>
> same for: isnsRegEntityMgtAddr
>
> and maybe others? pls check.
>
> - isnsDdMemberPortalPort OBJECT-TYPE
> SYNTAX InetPortNumber
> MAX-ACCESS read-only
> STATUS current
> DESCRIPTION
> "The port number for the portal. Whether the portal
> type is TCP or UDP is indicated by isnsDdPortalPortType."
>
> Is a port number of zero valid? And if so, then what does it mean?
> If not, then maybe use
> SYNTAX InetPortNumber (1..65535)
>
> same for: isnsRegPortalPort
>
> maybe others? pls check
>
> - naming consitency
> IsnsDdFcPortMemberEntry ::=
> SEQUENCE {
> isnsDdMemberFcPortName FcNameIdOrZero,
> isnsDdMemberFcIsRegistered TruthValue
> }
> I would start the names with isnsDdFcPort..
>
> - isnsDdMemberFcPortName
> and isnsDdId are the key for this table. Zero is not a
> valid value for this managed object."
> I guess you mean: a Zero length string is not a valie value?
>
> This issue is in several (if not all) object DESCRIPTIONs
> of objects with SYNTAX FcNameIdOrZero.
>
> - isnsRegEntityVersionMin OBJECT-TYPE
> SYNTAX Unsigned32 ( 0 .. 65535 )
> MAX-ACCESS read-only
> STATUS current
> DESCRIPTION
> "The iSNS Entity Protocol Version Range minimum value. A
> value of x'FF' is a wildcard value indicating no minimum to
> the protocol versions supported by this Entity. Entity
> registrations with isnsRegEntityProtocol set to No Protocol
> always have a minimum version of 0."
>
> are you sure you mean a value of x'FF'?? That is value 255
> in decimal,
> maybe you mean that you want to use x'FFFF' which is the
> value 65535?
> In that case, I think I would express it as:
> SYNTAX Unsigned32 ( 0 .. 65534 | 65535 )
> and use the value 65535 in de DESCRIPTION clause instead of some
> hex representation.
>
> same for: isnsRegEntityVersionMax
>
> - isnsRegEntityRegPeriod
> Pls add a UNITS clause:
> UNITS "seconds"
>
> - isnsRegPortalEsiInterval
> Pls add UNITS clause. here the DESCRIPTION clause does not even say
> in what units this is measured.
>
> - isnsRegFcPortType OBJECT-TYPE
> SYNTAX Unsigned32 ( 0 .. 65535 )
> MAX-ACCESS read-only
> STATUS current
> DESCRIPTION
> "The FC Port Port Type as defined in the iSNS Specification,
> RFC 4171, and the Fibre Channel Generic Services
> Specification. Current values are as shown below:
> unknown (0),
> nPort (1),
> nlPort (2),
> fNlPort (3),
> fPort (129), -- x'81'
> flPort (130), -- x'82'
> ePort (132), -- x'84'
> bPort (133), -- x'85'
> mFcpPort (65297), -- x'FF11'
> iFcpPort (65298), -- x'FF12'
> unknownEnd (65535)
> ."
>
> Would this not be better an ENUM. I understand you would want to
> break the rule/guidline to start at 1 and to be consecutive. But
> an ENUM seems so much nicer, no?
>
> - isnsRegFcPortFc4Types OBJECT-TYPE
> SYNTAX OCTET STRING (SIZE (32))
> MAX-ACCESS read-only
> STATUS current
> DESCRIPTION
> "The FC Port FC-4 Types as defined in the iSNS
> Specification, RFC 4171."
> REFERENCE "RFC 4171, Section 6"
> ::= { isnsRegFcPortEntry 10 }
>
> The size seems to allow for 8 types?
> Is that correct? How do I know? I do not find an explanation in
> sect 6 of 4171 for that either.
>
> - isnsRegFcPortFc4Descr OBJECT-TYPE
> SYNTAX OCTET STRING(SIZE(0..255))
> MAX-ACCESS read-only
>
> accoding to RFC4171, sect 6:
> FC-4 Descriptor 4-256
>
> so how does a 256-size value fit into 255-size string?
> should minumum size be 4 octets?
> So I would expect a SYNTAX of:
> SYNTAX OCTET STRING(SIZE(4..256))
>
> and RFC4171 sect 6.6.10 speaks about it being a NULL terminated
> UTF-8 string, so maybe the best SYNTAX would be
> SYNTAX SnmpAdminString (SIZE(4..255))
> and then add in DESCRIPTION clause that the termninating NULL is
> not included in the object (as you have done for some other UTF-8
> based objects).
>
> - for:
> isnsInstInfo OBJECT-TYPE
> SYNTAX SnmpAdminString (SIZE (0..80))
> MAX-ACCESS accessible-for-notify
> STATUS current
> DESCRIPTION
> "Textual information about the iSNS Server notification.
> An example is: iSNS Server Started, information that
> would be included in the appropriate notification."
> ::= { isnsNotificationsInfo 1 }
>
> It is nice that it is SnmpAdminString (i.e. UTF-8) so that you can
> represent international human readable text. But in some scripts,
> one character can occupy up to 5 or 6 octets. So a max size of 80
> allows for some 15 or so characters. Why do we want to limit this
> size to 80? Why can we not allow up to 255 (the max size of an
> SnmpAdminString) ???
>
> more nits/typos etc.
>
> - IsnsScnBitmapId ::= TEXTUAL-CONVENTION
> STATUS current
> DESCRIPTION
> "The State Change Notification (SCN) bitmap for a node as
> defined in the iSNS Specification, RFC 4171. A set bit (1)
> indicates the type of SCN for the bitmap as follows:
>
> Bit Field Flag Description
> --------- ----------------
> 0 INITIATOR AND SELF INFORMATION ONLY
> 1 TARGET AND SELF INFORMATION ONLY
> 2 MANAGEMENT REGISTRATION/SCN
> 3 REGISTERED OBJECT REMOVED
> 4 REGISTERED OBJECT ADDED
> 5 REGISTERED OBJECT UPDATED
> 6 DD/DDS MEMBER REMOVED (MGT REG/SCN
> ONLY)
> 7 DD/DDS MEMBER ADDED (MGT REG/SCN
> ONLY)
> "
>
> Why are you using all the UPPER CASE TEXT ??
> Makes me go back in time to IBM Mainframe MVS and JCL times.
>
> - I really wonder why isnsCntlNodeIscsiNodeIdx is not named
> isnsCntlNodeIscsiNodeIndex. It adds 2 characters to the descriptor,
> but gives the name so much more meaning.
> Maybe it is just me.
>
> - in DESCRIPTION clause of: isnsCntlNodeIscsiNodeName
> the storage node. The iSCSI Name can not be longer then
> should be "can not be longer than: ??
>
> - I do not understand why
> isnsAddrTypeNotifctn,
> isnsAddrNotifctn,
> isnsTcpPortNotifctn,
> isnsUdpPortNotifctn
> are not named
> isnsAddrTypeNotification,
> isnsAddrNotification,
> isnsTcpPortNotification,
> isnsUdpPortNotificationn
> in other words I can not see the tradeoff between a clear
> descriptor and
> a shorter descriptor here. For me the longer name would
> win. So is there
> an explanation why you use the shorter descriptor names?
>
> This theme of unexplanatory/not-understandable abbreviations for
> descriptor names or label names occurs many times. I will
> not continue to
> list them. I suggest that the editors and WG take a serious
> look at this
> and use clearer names whereever possible.
>
> - isnsDdsSymbolicName OBJECT-TYPE
> SYNTAX SnmpAdminString (SIZE (0..255))
> The SnmpAdminString has exactly the same range, so it is
> superfluous to
> repeat it here. So I would do:
> isnsDdsSymbolicName OBJECT-TYPE
> SYNTAX SnmpAdminString
>
> - same for
> isnsDdsMemberSymName OBJECT-TYPE
> SYNTAX SnmpAdminString (SIZE (0..255))
> and
> isnsRegEntityEID OBJECT-TYPE
> SYNTAX SnmpAdminString (SIZE (0..255))
> and maybe others. Pls check.
>
>
> Bert
> > -----Original Message-----
> > From: Wijnen, Bert (Bert)
> > Sent: Wednesday, May 10, 2006 23:17
> > To: Kevin Gibbons; David Black (E-mail); Scott Kipp;
> > [email protected]
> > Cc: [email protected]
> > Subject: MIB Doctor review (part-1) for:
> > draft-ietf-ips-isns-mib-09.txt
> >
> >
> > 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