>
> ----- Original Message -----
> From: <[email protected]>
> To: <[email protected]>; <[email protected]>
> Cc: <[email protected]>; <[email protected]>
> Sent: Tuesday, July 11, 2006 8:05 PM
> Subject: RE: MIB Dr. Review of draft-ietf-vrrp-unified-mib-05.txt
>
>
> Hi Joan,
> Thanks once again for the detailed review. I was shooting for some
> deadline at work and couldn't
> Reply in detail earlier.
>
> My comments/ explanation inline : (I removed the parts of your mail to
> which I agree and do not have comments )
>
>
Great! Thanks for your reply.
My comments are inline.
-Joan
>
> 5) I consulted with other MIB Doctors and the rough consensus was to
> leave deprected objects where they are in the MIB, even though they are
> deprecated but to make it very obvious that they are deprected.
> Also, Guidelines state to add the reason why they have been deprecated
> to the DESCRIPTION.
>
> -- I moved the order to the end of the MIB based on Bill Fenner's review
> comments in the last revision.
> -- Will move it back if it is required.
>
This was discussed in an email to Bill Fenner. Essentially, it is okay to
leave the objects where they are.
> vrrpNodeVersion OBJECT-TYPE -- **** DEPRECATED OBJECT ****
> SYNTAX Integer32
> MAX-ACCESS read-only
> STATUS deprecated
> DESCRIPTION
> "This value identifies the particular version of the VRRP
> supported by this node.
> This object is deprecated in the IP Version Independent
> MIB.
>
> This object has been deprecated and replaced by
> the vrrpOperationsVersion object in the
> vrrpOperationsTable."
> ::= { vrrpOperations 1 }
>
> Also, please add to the DESCRIPTION clause why the object has been
> deprecated/obsoleted.
>
> -- I thought "This object is deprecated in the IP Version Independent
> MIB." would indicate that
> -- the object is deprecated infavor of a version independent object.
> -- Is the following a better way of describing than what I have above?
> -- " This object has been deprecated and replaced by IP version neutral
> xxxx object."
Yes, please specify by name the new object which replaces this one.
> ----- Snip ---
>
> >2. Every deprecated object needs an explanation in the DESCRIPTION
> saying why it's deprecated (RFC 2578, section >10.2 (3)). Text like
> "This object is deprecated in favor of the IP Version Independent
> object, vrrpFoobarObject"
> >seems appropriate. Similarly for the deprecated Groups.
>
Yes.
> > 4. Is it feasible to the deprecated objects to the end (or, rather,
> middle, just before the groups) to make itmore > readable for someone
> who wants to come and see the current stuff?
>
> -- End Snip ----
>
Discussed in an email to Bill Fenner.
> 8) Could notifications be generated or not, using the
> the notification MIB and target MIB (RFC2583)?
> I don't see a need for this object, but please let me
> know the answer to the above question.
>
> -- I did not understand this. Are you saying a particular object is not
> Required in the notification?
>
>
Sorry, I meant to specify that the vrrpNotificationCntrl object
seemed to be specifying whether or not a notification would
be sent, but that is now done by the MIB Modules in RFC2583,
unless there is another purpose for this object, then I suggest
it be removed.
>
> Again, with regard to this DESCRIPTION clause:
>
> Rows in the table cannot be modified unless the value
> of 'vrrpOperationsState' has transitioned to
> 'initialize'"
>
> Why was the ability to change the OperationsState removed?
> In other words, the original table had an AdminState object, but this
> table does not. Please explain. Typically, there is an AdminState type
> object so that an operator is able to force a transition to a different
> state.
>
> -- This change was done after MIB Dr. review of Dave Thaler.
> -- Snip from previous discussion on this with Dave Thaler ---
>
> > 5) Regarding vrrpOperationsAdminState vs vrrpOperationsRowStatus, why
> > do you need both? Why isn't the row status sufficient?
> > What's the difference between CreateAndWait vs CreateAndGo with
> > AdminState=down?
> >
> > -- As you are aware, Rowstatus only indicates the status of the row
> > (whether the creation of the row is complete or not) while
> > OperationsState
> indicates
> > the
> > state of Virtual router the row indicates. I guess you are saying that
> we
> > could
> > use rowStatus valid/invalid to convey the meaning of adminstatus up &
> > down.
> > Is my understanding correct?
>
> RowStatus does not only indicate whether creation of the row is complete
> or not. The 'notInService' value of the RowStatus is meant to be used
> for adminstatus down for tables using RowStatus. From RFC 2579:
> - `notInService', which indicates that the conceptual
> row exists in the agent, but is unavailable for use by
> the managed device (see NOTE below); 'notInService' has
> no implication regarding the internal consistency of
> the row, availability of resources, or consistency with
> the current state of the managed device;
>
> As shown in the table on page 9 of 2579, a normal use case of
> notInService is to change the row to that from the active state (i.e.
> administratively
> take it down).
>
> > If you think this is required, I will
> > change it.
>
> I think it would make more sense, and reduce the implementation
> complexity, yes.
>
>
> -- end snip ---
I wil get back to you on this.
>
> 11) INDEX { vrrpOperationsInetAddrType,
> vrrpOperationsVrId, ifIndex }
>
> W: f(VRRP-MIB), (132,23) Row "vrrpOperationsEntry" does not have a
> consistent indexing scheme - index items from current table must
> come after index items from other tables
>
> Wrt to the above warning, did want to ask about the indexing used here?
> In the original MIB ifIndex appears first, so why is this changed here.
>
> -- I had a discussion with Dave Thaler and Wijnen Bert a while back on
> the order and was decided that
> -- operators would like to list all the v4 or v6 entries rather than
> based on VRID or ifIndex.
>
>
Okay. That is a good reason.
> Also, there is a blurb in the document itself which says that ifIndex
> represents the physical interface, is that still the case with this
> ifIndex? If so, please mention this someplace in the Table or Entry
> DESCRIPTION clause.
>
> -- Will clarify this.
>
>
>
> 13)
> vrrpOperationsVersion OBJECT-TYPE
> SYNTAX INTEGER {
> vrrpv2 (1),
> vrrpv3 (2)
> }
> MAX-ACCESS read-create
> STATUS current
> DESCRIPTION
> "This object contains the VRRP version on which this
> VRRP instance is running."
>
> ::= { vrrpOperationsEntry 6 }
>
>
> Wanted to check and make sure that the version of the VRRP protocol is
> settable (object is read-create).
> Did you intend for this to be read-only or read-create?
>
> The DESCRIPTION makes the object sound like a read-only object.
>
> Also, would the value of vrrpOperationsInetAddrType dictate the version?
> Not sure why this object is necessary.
>
> -- I was supposed to remove this object and I think I missed this one. I
> will remove it.
>
Thanks.
> 14)
> vrrpOperationsAddrCount OBJECT-TYPE
> SYNTAX Integer32 (0..255)
>
> Could you explain why there would only be a maximum of 255? Also, this
> should probably be Gauged32(0..255)
>
> Would also change the name of the object since it does not really seem
> to be a counter. Maybe something like vrrpOperationsCurrentAddrs
>
> -- VRRP protocol limits the number of associated addresses to be 255
> which is what this object indicates.
> -- "Count IP Addrs" field is 8 bits.
> -- I will change it to Gauge32(0..255), will also change the name.
Thank you. Please also, add a REF clause to refer to the
Count IP Addrs field. I see that the name is for the field now, so
that is why you used the word Count.
>
> 15) vrrpOperationsPrimaryIpAddr OBJECT-TYPE
>
> Does this object have a default value?
> The original object does, so wanted to check on that.
>
> -- I removed the default value because it will be different based on
> vrrpOperationsInetAddrType.
> -- I will probably mention this in the description of the object.
>
>
okay.
> 18) vrrpOperationsStorageType OBJECT-TYPE
>
> The description is confusing. I would think that most implementations
> would want to support nonVolatile(3) or something even more stable.
>
> Please explain the choice of volatile(2)? Also, please consider adding
> a DEFVAL for this object.
>
> -- I meant to say that the implimentations can implement as 'volatile'.
> If they chose to implement as 'nonVolatile'
> That would be good too. I will rephrase the description.
>
>
okay.
>
> 19) vrrpOperationsRowStatus OBJECT-TYPE
>
>
> notInService(2) should be used to administratively bring the row down,
> does this have an impact on the vrrpOperationsState object?
> (I think this gets back to my point of having an Admin object, the
> notInService refers to the row not being available for a manager, is
> this what you mean, or are you trying to say that the vrrp is
> notInService ??
>
> -- I mean that the row is 'notInService' and not VRRP .
>
> Adding a Row:
> Please list again the objects which need to have valid values for #2.
> 2. Populate the vrrpOperationsEntry with at least
> minimal elements.
>
>
> Are steps 2 and 3 out of order?
>
> Deleting a row:
> Can the same row in the vrrpAssociatedIpAddrTable be used by more than
> one row in the vrrpOperationsTable?
>
> - No.
>
> Also, if you set the vrrpAssociatedIpAddrTable row to notInService(2),
> do you need to go back and delete it, after you delete the corresponding
> row in the vrrpOperationsTable?
>
>
>
> 22) vrrpAssociatedStorageType
>
> Same questions/concerns about StorageType here as in the previous table.
>
> -- Again I meant that 'volatile' is allowed. I will change the wording.
>
okay.
>
> 24) vrrpRouterStatisticsTable
>
> The index is the same as the vrrpOperationsTable so have a question as
> to why this table does not use AUGMENTS?
>
> W: f(VRRP-MIB), (563,23) Row "vrrpRouterStatisticsEntry" does not
> have a consistent indexing scheme - cannot specify an index item
> from additional "base row" vrrpOperationsEntry, since can
> have only one "base row" which is ifEntry
>
> Also, as discussed above, have a question as to why ifIndex is not used
> first?
>
> -- Good point. I will change the table to use AUGMENTS.
>
okay.
> 25) naming of objects in vrrpRouterStatisticsTable: Counters usually
> end in "s". Some of these counters do, so please try to do this, for
> example: vrrpStatisticsBecomeMaster could be
> vrrpStatisticsMasterTransitions,
>
> vrrpStatisticsAdvertiseRcvd could be vrrpStatisticsRcvdAdvertisements
> and so forth.
>
> vrrpStatisticsInvldTypePktsRcvd could be
> vrrpStatisticsRcvdInvalidTypePkts
>
> (Also, please spell out some of these names, such as Priority/Pri,
> Advertisement/Adv).
>
> -- Will do.
>
okay.
> 26) vrrpStatisticsRefreshRate
>
> Not sure that I fully understand the benefit of this object.
> Could you please explain why this is needed for vrrp?
>
> -- I thought this would be a good idea for implementations that may have
> to do costly processing (like reading from kernel) to update the
> statistics.
>
>
okay.
> 29) Concern on the use of accessible-for-notify: it might be a good
> idea to make these objects read-only, along with a timestamp type of
> object. Notifications are unreliable and so, could be lost. Also, some
> customers do not like to enable a lot of notifications and prefer
> polling.
> Would you consider making these objects read-only?
>
> -- Will do.
>
>
Thanks,
-Joan
>
>
> Thanks again for the review.
> -Kalyan
_______________________________________________
vrrp mailing list
[email protected]
https://www1.ietf.org/mailman/listinfo/vrrp
lmpx.com only provides a reader for public news (NNTP) servers. It is not
affiliated with the servers or forums shown here and is not responsible for
the content of articles, which is written by their respective authors.