RE: [Ips] FW: Initial review of http://www.ietf.org/internet-draf

First, I still see:

    ::= { mib-2 999999 }             -- to be assigned by IANA

Keith, you know VERY WELL I think that we do NOT want that.
The strongly recommended notation is:

    ::= { mib-2 xxx }             -- to be assigned by IANA

Responses to things on which we're still not in sync 
> > > > ./FC-MGMT-MIB:62: [5] {type-without-format} warning: type 
> > > `DomainIdOrZero' has no format specification
> > > > ./FC-MGMT-MIB:94: [5] {type-without-format} warning: type 
> > > `FcBbCredit' has no format specification
> > > > ./FC-MGMT-MIB:106: [5] {type-without-format} warning: type 
> > > `FcDataFieldSize' has no format specification
> > > > ./FC-MGMT-MIB:171: [5] {type-without-format} warning: type 
> > > `PhysicalIndexOrZero' has no format specification
> > > > ./FC-MGMT-MIB:181: [5] {type-without-format} warning: type 
> > > `HrSWInstalledIndexOrZero' has no format specification
> > > > ./FC-MGMT-MIB:192: [5] {type-without-format} warning: type 
> > > `MilliSeconds' has no format specification
> > > > ./FC-MGMT-MIB:198: [5] {type-without-format} warning: type 
> > > `MicroSeconds' has no format specification
> > > 
> > > It seems to me like the compiler has a bug here, since 
> > > all of these are, or
> > > have a TC as their syntax and for all but two of them, 
> > > the TC is defined 
> > > with a range, e.g., DomainIdOrZero is defined as:
> > > 
> > >   DomainIdOrZero ::= TEXTUAL-CONVENTION
> > >       STATUS current
> > >       DESCRIPTION
> > >             "The Domain Id (of a FC switch), or zero if 
> > >              the no Domain Id has been assigned."
> > >       SYNTAX  Integer32 (0..239)
> > > 
> > > The two exceptions are MilliSeconds and MicroSeconds 
> > > which are both
> > > Unsigned32 and the inherent (0..4294967295) is correct for both.
> >  
> > I think the warning is cause by the fact that there is no 
> > DISPLAY-HINT.  Can you add those?
> 
> I can't because, as far as I know, there is no standard format for
> displaying these.  Anyway, RFC 2579 says that DISPLAY-HINT is 
> optional.
> So, this is another bug in that compiler.  Why is the IETF's 
> MIB review process relying upon a buggy compiler ??
> 

OK, I asked "can you add those?" I did not say "you MUST add those".
AS you note, they are optional. However the set of current MIB doctors
have composed a set of guidelines for review, and document
    draft-ietf-ops-mib-review-guidelines-02.txt
represents the current guidelines and sect 4.6.3 recommends to add
a DISPLAY HINT:

   4.6.3.  DISPLAY-HINT Clause

   The DISPLAY-HINT clause is used in a TC to provide a non-binding hint
   to a management application as to how the value of an instance of an
   object defined with the syntax in the TC might be displayed.  Its
   presence is optional.

   Although management applications typically default to decimal format
   ("d") for integer TCs which are not enumerations and to a hexadecimal
   format ("1x:" or "1x " or "1x_") for octet string TCs when the
   DISPLAY-HINT clause is absent, it should be noted that SMIv2 does not
   actually specify any defaults.  MIB authors should be aware that a
   clear hint is provided to applications only when the DISPLAY-HINT
   clause is present.

So... Can you add one please. If nothing else, then use "d"

If you disagree with the mib-review guideline. pls speak up on
ietfmibs@ops.ietf.org list, so we can re-evaluate. This guidelines doc
is on its way to become a BCP.

The compiler by the wayt only generates a warning, not an error!

... snip ...

> > > >   Reviewer Comments:  
> > > >    1. The TC's could possibly go into a separate TC 
> > > document for fiber channel: FcNameIdOrZero, DomainIdOrZero, 
> > > FcAddressId
> > > >       as it is mentioned there might be multiple Fiber 
> > > channel mib modules in the future.
> > > 
> > > While your "could" and "possibly" are correct, but we have already
> > > written several of these follow-on FC MIB modules, and they are
> > > IMPORT-ing TCs from this MIB without any problems.  So, this 
> > > would be a disruptive change for no benefit.
> > 
> > Keith, but the current spec locks all other MIB modules to this
> > one. I.e. the otehrs cannot advance unless this one also advances.
> > If you put TCs in a separate module then you do not have that issue.
> > Now it can still be changed (rather easily), later it becomes more
> > difficult. But it is not a MUST change
> 
> Since they are "follow-on FC MIB modules", it is entirely appropriate
> that they not advance until this one does, and anyway most of them
> would still be dependent on this MIB, even if the TC's were in a
> separate document.
> 
OK fine. as long as the WG makes a conscious and educated decision on
this, then I am happy.

> > > >    2. FcAddressId doesn't have an *OrZero version, which 
> > > might be needed at a later date.
> > > 
> > > It's syntax is "OCTET STRING (SIZE(0 | 3))" and the 
> > > zero-length value is already used (see fcmLinkEnd2FcAddressId).
> > 
> > You migth want to add text to the DESCRITPION clause that 
> > explains the special meaning of zero.
> > 
> > I think what Mike noticed is an inconsistency, because some 
> > of your other
> > TCs do have an xxxOrZero. Consistency is (I think ) a good thing.
> > But a again not a MUST change.
> 
> Because the TC is not currently imported by any other MIB, 
> I've made the
> change that you ask for (but not the addition of another TC that Mike
> asked for).
>  
> > > >    3. FcPortType enumeration "dynamic(3),  -- determined 
> > > dynamically"  seems redundant, prefer comment point to 
> > > underlying spec
> > > 
> > > No, it's not redundant because "dynamic" could also mean that 
> > > the value
> > > is constantly changing, and saying "determined dynamically" 
> > > removes that possibility.
> > 
> > You might want to enhance the description somewhat then.
> > And as Mike says, a ptr to the spec might be helpful too
> 
> I've added a couple of words and moved it inside the DESCRIPTION
> instead of being an ASN.1 comment.
> 
Thanks

> > > >    4. PhysicalIndexOrZero, HrSWInstalledIndexOrZero are 
> > > >       very useful and generally applicable to many mib modules.
> > > >       They look strangly wrong being defined here and 
> > > >       should be defined in another TC, like Entity-MIB, or some 
> > > >       common TC-MIB.
> > > 
> > > But they aren't, and thus, the best that this MIB module 
> > > can do is to define the TC's itself.
> > 
> > Could you rename them to RcPhysicalIndexOrZero and 
> > FcHrSWInstalledIndexOrZero
> > so that they will not conflict if Entity MIB does define them later.
> 
> There will be no conflict.  First, the possibility of a descriptor
> clash is ever-present in practically every deployment because of
> Enterprise MIBs.  Second, MIB compilers have to cope with it, and they
> do for the most part, although HP OpenView's compiler has had the same
> bug for the last ~10 years, and most often shows up because of
> OwnerString being defined as an OCTET STRING in IF-MIB and as a
> DisplayString in RMON-MIB; if you delete one of the two definitions or
> change them to both have the same syntax, OpenView stops complaining.
> 
In principle we try to NOT get name clashes in STDs track MIB modules.
Of course we have goofed up a few times, and we want to try to avoid so
in the future, that is why I am asking (pretty serious) to consider the 
name change and add the prefix. PLEASE!

> > But... SURPRISE (maybe), pls take a look at
> > 
> >    draft-ietf-entmib-v3-04.txt
> > 
> > and is on my AD plate for review!! So at least that one can be
> > imported from ENTITY-MIB.
> 
> First, we don't want to risk any further delays.  (In the update that I've
> just done, I changed the date from February 2003 to March 2004.  In other
> words, we have waited 13 months for the benefit of the minor changes that
> I'm making now :-(.)  So, let's not import from 
> draft-ietf-entmib-v3-04.txt.
> 
If you do not want to IMPORT from entmibv3, I can accept that. I may want
to re-open that if that entmib docs ends up as RFC earlier than yours.

But it DOES again show that you better add the requested prefix to the
TC names.

> Second, as you may have noticed all(*) the FC-specific TC's in this MIB
> are already prefixed with "Fc", and none of the generic TC's are
> prefixed with "Fc".  I want to keep it that way, and since neither of
> PhysicalIndexOrZero or HrSWInstalledIndexOrZero are FC-specific, I've
> just deleted them.  (* -- actually, all but one because I made a
> mistake with DomainIdOrZero.)
> 
So you have removed them I see.
And you have instead used Integer32.
Although this is perfectly legal SMIv2, I think you understand VERY WELL
that this is against the spirit of what we are trying to do with TCs.

If that is the way you want to go... fine, I guess we should make the docs
informational or experimental and then you can do whatever you want
as long as the SMIv2 use is legal to the letter instead of to teh spirit.

> > > >    5. MicroSeconds, MilliSeconds should also be in some 
> > > generic tc-mib...
> > > 
> > > But they aren't, and thus, the best that this MIB module 
> > > can do is to define the TC's itself.
>  > 
> > Well, the SSPM-MIB (draft-ietf-rmonmib-sspm-mib-11.txt) which is in
> > IETF Last Call DOES have a SspmMicroSeconds TC.
> > I made them prefix with Sspm.. because I do foresee a future generic
> > TC MIB for these things. So if you want to keep them in this MIB
> > module I strongly recommend to name it FcMicroSeconds and FcMilliSeconds.
> 
> Again, naming them FcMilliseconds and FcMicroseconds would give the
> reader the wromng impression.  These are NOT Fibre Channel units; they are
> generic units. Fibre Channel's milliseconds/microseconds are NO different
> from any other technology's milliseconds/microseconds.  Nevertheless,
> because of the comment, I've deleted the two TC's.
> 
And again use a plain Integer32 instead. OK... you win

> > > >    6. fcmInstanceIndex DESCRIPTION is problematic for me. 
> > > Either it stays static or it doesn't. 
> > > >       Having something "desireable" is worthless to those 
> > > writing mangement applications, they have to assume worst 
> > > case always.
> > > 
> > > Not worthless, because if agent implementors heed the advice, 
> > > then mgmt
> > > apps can coded to check whether the index values have 
> > > been retained
> > > over the last restart, and only if not, do they need to 
> > > re-synchronize
> > > their database.  That is, it is an optimization for the 
> > > common case of
> > > index values being retained, but the fallback still works 
> > > in the case 
> > > (hopefully less common) when all knowledge of index 
> > > values is lost.
> > 
> > So would it make sense to phrase it with a SHOULD ??
>  
> It wouldn't make sense to say a system SHOULD retain the 
> values if all of
> its non-volatile memory is replaced, i.e., to say that it 
> should when we
> know there are circumstances when it can't !!
> 
Would it be better then to just say instead of "desirable" to add
a sentence that says somethink aka
  "This should no tbe taken as an easy excuse to ignore the desire
   for keeping the value constant across restarts"
We did a similar thing for commitFailed and undoFailed.

> > > >       There are problems with the description. Sentences 
> > > start without capitalization.
> > > 
> > > "Sentences" ??   I see only one such sentence-break, for 
> > > which the fix
> > > is to delete the the period before "but" (because it's 
> > > bad grammar to start a sentence with "But".)
> > > 
> > > >       fcmInstanceTable has very critical data. Would like 
> > > to have a LastChange entry per row to know when values 
> > > were changed or
> > > >         a "configuration change" notification if lastChange 
> > > object is too much overhead.
> > > 
> > > But the data is not, in general, both critial and dynamic.  
> > > Specifically,
> > > the only things in this table that change are: 
> > > - the FabricId if it's based on the WWN of the Principal switch;
> > >     -- a notification of a Principal switch change is being 
> > > proposed as
> > >        an extension to this MIB.
> > > - the read-write objects (but only when an app changes them);
> > >     -- changes in these values are not critical to the 
> > > operation of the
> > >        network, and the synchronization of mgmt apps should 
> > > not have to
> > >        rely on notifications from agents for such non-critical data.
> > > - the Status object,
> > >     -- to monitor this object for changes, set an RMON 
> > > threshold on it.
> > >        Note that the typical situation is to have just one instance;
> > >        even if there are multiple instances, there will not be many.
> > >        Having an RMON threshold will be more effective than having a
> > >        LastChange object.
> > > 
> > > >    7. fcmInstanceWwn, but does it matter which WWN is 
> > > selected? Will managment stations expect one vendor 
> > > implemetnation over another?
> > > 
> > > The ability to have multiple instances is defined as a 
> flexibility to
> > > allow for many/any different choices by vendors.  Vendors 
> are free to
> > > choose to have one, or more, instances using whatever 
> > > methodology they wish.
> > > Thus, we cannot prescribe how the WWN is selected.
> > > 
> > > This flexibility is necessary because otherwise there is a 
> > > genuine risk
> > > that a vendor will implement a set of Fibre Channel 
> > > interfaces in a manner
> > > which cannot be represented by this MIB.
> > > 
> > > >       Wouldn't it be the same as fcmSwitchDomainId if 
> > > fcmSwitchPrincipal is True?
> > > 
> > > It could be but it needn't be, and the circumstance you 
> ask about is a
> > > corner case which for example never applies to HBAs.
> > > 
> > > >    8. fcmInstanceStatus could use an associated object to 
> > > get timestamp of the last state change
> > > 
> > > While implementation feedback might tell us that, I think it's too
> > > soon to be sure.  So, let's save this comment until we update 
> > > the MIB from
> > > Proposed to Draft Standard.
> > > 
> > > >    9. fcmInstanceStatus doesn't seem to be implementable in 
> > > a consistent way across vendors. Needs better definition
> > > >     ideally this state is defined in the underlying 
> > > technology specifications.
> > > 
> > > On the contrary, a) there is no one underlying 
> technology, and b) the
> > > underlying technology specifications are the subject of 
> future MIBs,
> > > not this one.  This object serves as a generic summary of the 
> > > underlying
> > > specifics (e.g., somewhat like ifOperStatus does for an 
> interface.)
> > > 
> > > >    10. fcmInstanceTextName/fcmInstanceDescr, are there any 
> > > DEFAULT values for these? Suspect vendor implementations will
> > > >       provide something. Better to define the intended 
> > > purpose of these objects better.
> > > >             "A textual name for this management instance 
> > > and the Fibre
> > > >             Channel entity/entities that it is managing."
> > > >       might be 
> > > >             "A textual name for this management instance 
> > > and the Fibre
> > > >             Channel entity/entities that it is managing set 
> > > by the operator or to a default string "XXXX" by the device."
> > > 
> > > "XXXX" can not be specified in this MIB without restricting 
> > > the necessary
> > > flexibility, anything that could be specified in this MIB 
> would not be
> > > specific enough.  The whole idea of this object is to be the 
> > > name specified
> > > by the operator/NM-app, to have a default value changes 
> the semantics.
> > > 
> > > >    11. fcmPortAdminType - Assuming a valid value change 
> > > can't be done for some device resource specific case, how can
> > > >                           the error be reported? A generic 
> > > snmp error to the set is necessary but not sufficient I think.
> > > 
> > > Why would it not be sufficient ??
> > 
> > cause genErr is also used for other (unspecified) errors.
> > Maybe this could be spelled out that the best would be to sent an
> > inconsistentValue error?
> 
> This comment applies to every read-write/read-create in this and every
> other MIB.  To explain when to return which error status for 
> a SetRequest
> requires most of pages 20-22 of RFC 3416 to be included in 
> the DESCRIPTION
> of each such MIB object, and that doesn't make sense.
> 
> It seems to me that the thing which prompted this comment was the
> second sentence in the DESCRIPTION:
> 
>             "The administratively desired type of this port.  
> Each port
>             will typically only be able to support a subset 
> of these types."
> 
> Is it really necessary for me to remove the second sentence ??
> 
Mike... you raised it?

> > > >    12. Not sure why these object's syntax wasn't a TC? They 
> > > will probably be added to in time as well.
> > > >             fcmPortCapProtocols
> > > >             fcmPortTransmitterType 
> > > >             fcmPortConnectorType -- this should probably 
> > > follow the MAU-MIB convention or we need to standardize 'JackType'
> > > >                                     in a new TC mib as 
> > > there is now overlap...
> > > 
> > > Actually, not for the latter two because they are not generic 
> > > definitions.
> > > Rather they are the specific enumerations listed for the 
> function in
> > > T11's GS-3 speciofication.  Sicne a future revision of 
> the GS-3 spec
> > > could revise them individually, they cannot be defined as 
> common TCs.
> > > The first one could, but I don't see any rule that it 
> violates which
> > > would justify a change at this time.
> > >
> > A change is justified as much as no change. My personal opinion.
> > Is it a MSUT change? no it is not.
> > 
> > > >    13. fcmPortStatsTable doesn't have an explicitly 
> > > specified discontinuity object -- is ifXTable entry the one 
> > > that applies?
> > > 
> > > Yes.
> > 
> > Then you want to state that in DESCRIPTION clause(s)
> 
> I've added this to all tables which have CounterNn objects.
> 
> > > >                Also, the counters described here should 
> > > have REFERNCE clauses to counters defined in the underlying 
> > > technology 
> > > >                specs. While the descriptions are generally 
> > > good, they do not cover all possible cases when 
> > > >                one should be incremented or not -- that 
> > > leads to incompatible implementations. 
> > > 
> > > Many of them are not specifically defined in any Fibre Channel
> > > specification.  If any are ambiguous, please suggest alternative
> > > wording.  Otherwise, I suggest we wait for implementation 
> feedback.
> > >                 
> > > >    14. fcmPortLcStatsTable doesn't have split counters?  Is 
> > > a SNMPv1 agent then really useful when running high speed 
> interfaces?
> > > 
> > > No, but at the time (several years ago now) when this was 
> discussed
> > > in the WG, many Storage vendors only had SNMPv1 agents, and as a
> > > result, this table was defined at the explicit direction 
> of the WG.
> > > 
> > I.e. to force them to SNMPv3? FIne by me!
> > 
> > > >             DESCRIPTION
> > > >             "A list of Counter32-based statistics which are 
> > > a shadow of
> > > >             the Counter64 statistics in the 
> fcmPortStatsTable, for
> > > >             systems which do not support Counter64."
> > > >       I suggest the text mention it should "shadow" the 
> > > lower 32 bits?
> > >  
> > > If that isn't what "shadow" means, then I don't know what it 
> > > does mean, and we shouldn't use the term at all.
> > > 
> > I prefer if you spell out they contain the low order 32 bits.
> 
> I removed the phrase containing "shadow".
> 
I do not think that addresses our concerns.
For an SNMPv1 manager which obtains those. I bet they are the lower32 bits
of the Counter64. What is so darn difficult to spell it out?

> > > >    15. If errors are not so common an occurance across 
> > > instances, the fcmPortErrorsTable might benefit from a 
> > > *LastChange object
> > > >        so not all objects have to be polled to determine 
> > > any error state change. Alternately provide a summary counter that
> > > >        is a simple sum of the number of errors. That should 
> > > allow for better scaling of this table for mgmt polling using 
> > > SNMP/UDP.
> > > 
> > > The summary counter for many of these is already available: 
> > > ifInErrors.
> > >        
> > > >    16. Error table should have REFERENCE clauses to point 
> > > out where in the underlying spec these counters are defined. 
> > > >        This appears to be done, but is inconsistent. 
> > > fcmPortLinkFailures has a REFERENCE but fcmPortLinkResets wasn't?
> > >  
> > > As I recall, it is consistent; specifically, all those for which
> > > there is an appropriate section to reference have a 
> REFERENCE clause.
> > > 
> > > >    17. fcmFxPortTable needs a discontinutity object. if 
> > > fcmPortOperType is changed from Fx to some other then back within
> > > >        a given polling interval, the data collected from 
> > > this table will be incorrect.
> > > 
> > > No, because there are no Counter-based objects in this table.
> > > 
> > > >    18. Objects in fcmFxPortTable, fcmFLoginTable could use 
> > > REFERENCE clauses.
> > > 
> > > Sigh !!  I will look again to see what I can find.
> > > 
> > > >    19. fcmFLoginNxPortIndex doesn't specify what behaviors 
> > > should be across SNMP agent reset or system reboots like
> > > >        fcmInstanceIndex is required to do. So I'm a bit 
> > > concerned what the indexing looks like for all the tables on
> > > >        system reset or agent reset.
> > > 
> > > I will add a sentence saying that after a value of 
> > > fcmFLoginNxPortIndex
> > > is assigned to a particular Nx_Port, it can be re-used 
> when and only
> > > when it is assigned to the same Nx_Port, or, after a reset of 
> > > the value
> > > of the relevant instance of ifCounterDiscontinuityTime.
> > > 
> > > >    20. fcmLinkEnd2AgentAddress OBJECT-TYPE is  SYNTAX      
> > > SnmpAdminString, not InetAddressType? Why?
> > > 
> > > Because the value is received from the otrher end of the 
> > > link, and if the
> > > other end supplies a bogus value,m then that bpgus value 
> > > needs to be stored
> > > in the MIB.
> > 
> > So in that case it can be stored with InetAddressType "unknown" and
> > the value that you received in the InetAddress, no?
> 
> No.  The GS-3 spec gives, as an example, the use of a URL in this
> string, with the URL being for an SNMP, or HTTP or XML agent. 
> I've added some words to the DESCRIPTION stating this.
> 
Look much better already.
How can I get access to the GS-3 spec?

> > So I do not see a reason to NOT use InetAddress(Type) combo.
> > > >    21. 
> > > >                       
> > > > End of Report
> > > > 
> > 
> > An additional remark from me:
> > 
> > I strongly recommend to rename DomainIdOrZero to FcDomainIdOrZero.
> > The current definition of the TC is certainly not a generic DomainId
> > definition is it?
> 
> Agreed.  DomainIdOrZero is FC-specific, and so it was my mistake to
> call it DomainIdOrZero.
> 
Peace.

When I do SMICng, I get:

   W: f(fcmgmt.mi2), (2211,1) OBJECT-GROUP "fcmLinkBasicGroup" is not
      used in a MODULE-COMPLIANCE in current module

That probably means that this is an optional GROUP. We recommend in mib
review guidelines to include it in a MODULE_COMPLIANCE and spell out that
it is optional. This to make it clear that it is intentional instead
of accidental.

Bert
> Keith.
> 

_______________________________________________
Ips mailing list
Ips@ietf.org
https://www1.ietf.org/mailman/listinfo/ips