[imss] Re: AD review of: draft-ietf-imss-fc-fspf-mib-01.txt

[Keith McCloghrie <[e-mail]>]

Hi Bert,
 
> Summary:
> 
> looks good. I would be OK with doing IETF Last Call and consider these
> comments as initial IETF Last Call comments. At the other hand, IETF
> LC right before/during an IETF will not get many people to pay
> attention to this. Maybe you rather do a new rev first?
 
I'll do a new rev first, but it won't now get posted until after the IETF.

> more serious questions/issues:
> 
> SMICng tells me:
>   E: f(t11fcfspf.mi2), (915,54) Index item "t11FspfLinkIndex" must be
>      defined with syntax that includes a range
>   W: f(t11fcfspf.mi2), (1007,19) Variable "ifIndex" in notification
>      "t11FspfNbrStateChangNotify" is an index for a table
> 
> The warning is OK/acceptable as far as I am concerned.
> The Error would probably be best fixed.
 
SMICng is confused:

- t11FspfLinkIndex is defined as Unsigned32, which is defined with a
  range of (0..4294967295).  So, it does have a range (but see below).

- ifIndex is read-only.  RFC 2578 says (in section 7.3):

    The value "not-accessible" indicates an auxiliary object (see Section
    7.7).  The value "accessible-for-notify" indicates an object which is
    accessible only via a notification (e.g., snmpTrapOID [5]).

    These values are ordered, from least to greatest:  "not-accessible",
    "accessible-for-notify", "read-only", "read-write", "read-create".

  Since "read-only" is greater than "accessible-for-notify", it is
  both readable and accessible via a notification.  Indeed, consider
  the situation described in bullet (2) of section 7.7 of RFC 2578 --
  the only way to have a notification include any information from a row
  of a table for which all of the row's objects are also specified in
  its INDEX clause, is for it to include an object from the INDEX clause.

> SMILINT tells me:
> 
> C:\smi\mibs\work>smilint -m -s -l 6 ./T11-FC-FSPF-MIB
> ./T11-FC-FSPF-MIB:212: [5] {identifier-case-match} warning: identifier `t11FspfA
> RegionNum' differs from `T11FspfARegionNum' only in case
> ./T11-FC-FSPF-MIB:61: [6] {previous-definition} info: previous definition of `T1
> 1FspfARegionNum'
> ./T11-FC-FSPF-MIB:824: [5] {identifier-case-match} warning: identifier `t11FspfL
> srType' differs from `T11FspfLsrType' only in case
> ./T11-FC-FSPF-MIB:71: [6] {previous-definition} info: previous definition of `T1
> 1FspfLsrType'
> ./T11-FC-FSPF-MIB:964: [5] {identifier-case-match} warning: identifier `t11FspfL
> inkType' differs from `T11FspfLinkType' only in case
> ./T11-FC-FSPF-MIB:89: [6] {previous-definition} info: previous definition of `T1
> 1FspfLinkType'
> ./T11-FC-FSPF-MIB:61: [5] {type-without-format} warning: type `T11FspfARegionNum
> ' has no format specification
> ./T11-FC-FSPF-MIB:71: [5] {type-without-format} warning: type `T11FspfLsrType' h
> as no format specification
> ./T11-FC-FSPF-MIB:89: [5] {type-without-format} warning: type `T11FspfLinkType'
> has no format specification
> ./T11-FC-FSPF-MIB:129: [5] {type-without-format} warning: type `T11FspfLastCreat
> ionTime' has no format specification
> 
> The identifier-case-match are acceptable. They may confuse people though.
> 
> The type-without-format warnings are for your TCs.
> You may want to add a SIPLAY-HINT
 
Both section 3.1 of RFC 2579 and section 4.6.3 of RFC 4181 say that
the DISPLAY-HINT clause is optional/need not be present.
 
> - sect 5 states:
>    protocol.  (Note that there are no definitions in this MIB module of
>    "managed actions" which can be invoked via SNMP.)
>   So what does that mean for t11FspfSetToDefault, which to me seems to
>   be an action that can be invoked via SNMP??
>   Same for t11FspfIfSetToDefault
 
It depends on how one defines "action", and you're right that the term
"managed action" is not well-defined.  That sentence was added based on
a question that arose in T11.  The term "managed action" was intended
to mean a setRequest which caused a switch to send out a command in one
of the T11-defined protocols; for comparison, see some of the objects
defined in draft-ietf-imss-fc-fam-mib-03.txt (e.g., t11FamRestart).
Since that sentence is adding confusion, I will delete it.

> - redundant objects??
>   I do not see what the difference is between the follow two
>   (well, except SYNTAX). Is there any? If so, can that be made
>   clear in the DESCRIPTION clauses? If not, can one be removed?
> 
>   t11FspfLsrs OBJECT-TYPE
>     SYNTAX      Gauge32
>     MAX-ACCESS  read-only
>     STATUS      current
>     DESCRIPTION
>            "The current number of entries for this Fabric in the
>            t11FspfLsrTable."
>     ::= { t11FspfEntry 19 }
> 
>   t11FspfLsrNumber  OBJECT-TYPE
>     SYNTAX      Unsigned32 (0..2147483647)
>     MAX-ACCESS  read-only
>     STATUS      current
>     DESCRIPTION
>            "The current number of rows for this fabric in the
>            t11FspfLsrTable."
>     ::= { t11FspfEntry 23 }
 
Thanks for spotting that.  I will remove one of them.

> - t11FspfStorageType
>   Which writeable objects must be writable for a 'permanent' row?
 
Since I already succumbed in adding the sentence that you're looking
for to t11FspfIfStorageType, I will (so as to be consistent) add it to
t11FspfStorageType also.

However, the requirement at the top of page 23 in RFC 4181 is bogus
because: a) a requirement which says that an "object must be writeable"
under some particular circumstance is for the minimum requried access,
i.e., MIN-ACCESS, and belongs in MODULE-COMPLIANCE, not in OBJECT-TYPE,
b) in this case, all of the accessible objects in this table have a
MIN-ACCESS of read-only.  So, the answer to your question is already
present in the document.

> - Mmm. I find the way you delete a row in t11FspfIfRowStatus
>   a bit strange/weird. But I guess I can live with it.
>   Can you explain why you do it this way?

Because:

1. having unnecessary rows in this table clutters it up, and makes
   those rows that are useful harder for a NMS to find.
2. having the current value always be in same object, irrespective of
   whether it is a default or a non-default value is desirable. 
3. when non-default values are specified for a port's parameters,
   they need to be retained in the MIB even when the port is 'isolated'.
4. when an E_Port becomes isolated, we want its row to get deleted
   if and only if all of its parameter values are the default values.

> - t11FspfLinkIndex probably better specifies a range that excludes zero ??
>   see also smicng error above.
 
The smicng error is confused, but I agree it would be better to exclude
zero, and I'll change it to do so.

> admin/nits and just questions for clarity/explanation
> 
> - I see:
>     t11FspfIfNbrPortIndex OBJECT-TYPE
>         SYNTAX      Unsigned32 (1..4294967295)
> 
>     t11FspfLinkPortIndex OBJECT-TYPE
>         SYNTAX      Unsigned32 (0..16777215)
> 
>     t11FspfLinkNbrPortIndex OBJECT-TYPE
>         SYNTAX      Unsigned32 (0..16777215)
> 
>   Is it just me who wonders about the different ranges??
>   Maybe I am missing something or misunderstanding.
     
The format of the FSPF protocol packets as shown in FC-SW-4 indicates
that the port number field is 3 bytes long.  Hence, (0..16777215) is
correct, and (1..4294967295) is incorrect -- I will change
t11FspfIfNbrPortIndex.

> - I wonder what a value of zero means for T11FspfLinkType
>   If it can never exist, would it be better to make the range
>   (1..255) ??
 
The only difference between the definition in the MIB and the definition
pointed to by the REFERENCE clause is that FC-SW-4 says "all others"
rather than just "others", i.e., zero is a reserved value.  I can add
"all" into the DESCRIPTION.

> - I wonder if T11FspfLsrType and T11FspfLinkType would be better
>   defined with a base type of INTEGER? In other words Enumerations?
 
This is a trade-off between having the MIB definition being more
explicit or more open-ended.  As these objects are currently defined,
they implicitly get any new values as and when T11 defines new values
for that field.  Having them as enumerations would be more explicit,
but would require a MIB change to incorporate newly defined values.  I
think this trade-off should be made individually for each object on its
own merits.

> - I wonder how new non-Vendor-Specific values for T11FspfLsrType 
>   and T11FspfLinkType will be assigned/allocated?
>   Probably by T11? Would not hurt to say something about that.

Both of those TC's have a REFERENCE clause pointing to a T11 spec.
That particular spec doesn't define the term "Vendor Specific" (and 
I'm not sure which, if any, T11 spec does define it), but if I
understand correctly, in T11 the term means that it is the vendors who
define such values and that different vendors can (and do) have
different meanings for the same value, i.e., they are reserved but
*not* allocated by an IANA-equivalent organization.  A network manager
managing a Fibre Channel has to know this, not because of SNMP or the
MIB, but because of Fibre Channel.

> - I suspect that the RFCyyyy in t11FspfAdminStatus REFERENCE clause
>   is (or should be) a different yyyy than the one in the
>   MODULE-IDENTITY macro?

Right. I'll change t11FspfAdminStatus to use zzzz.

> - In a number of places I would prefer to see "MIB module" instead
>   of just "MIB". But I cam live with if if this is the doc does not
>   need changing for anything else.

I'll look for them.

> - Section 7 is redundant and contains same material as the back matter.
 
Right.

Thanks,
Keith.

_______________________________________________
imss mailing list
[e-mail]
https://www1.ietf.org/mailman/listinfo/imss