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 )
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.
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."
----- 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.
> 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 ----
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?
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 ---
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.
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.
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.
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.
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.
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.
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.
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.
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.
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 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.