Re: [VRRP] AD review of draft-ietf-vrrp-unified-mib

[Adrian Farrel <[e-mail]>]

Hi,

Sorry for the delay around the IETF meeting.

All agreed with a small suggestion around RFC 3413

> Thanks again for the review - I made most of the changes suggested by you - I
> listed some of the changes for any comments &
> I have couple of questions/clarifications inline:
> 
>> Creation and deletion of a vrrpv3OperationsTable row
> 
> [kalyan>>>] This is what i have based on your suggestions. Please comment if
this
> looks OK
> 
>     vrrpv3OperationsEntry OBJECT-TYPE
>         SYNTAX       Vrrpv3OperationsEntry
>         MAX-ACCESS   not-accessible
>         STATUS       current
>         DESCRIPTION
>             "An entry in the vrrpv3OperationsTable containing the
>              operational characteristics of a virtual router.  On a
>              VRRP router, a given virtual router is identified by a
>              combination of ifIndex, VRID and the IP version.
>              ifIndex represents a interface of the router.
> 
>              A row must be created with vrrpv3OperationsStatus
>              set to initialize(1) and cannot transition to
>              backup(2) or master(3) until vrrpv3OperationsRowStatus
>              is transitioned to active(1).
> 
>              The information in this table is persistent and when
>              written the entity SHOULD save the change to non-
>              volatile storage."
> 
>         INDEX    { ifIndex, vrrpv3OperationsVrId,
>                    vrrpv3OperationsInetAddrType
>                   }
>         ::= { vrrpv3OperationsTable 1 }
> 
> =======================================
>     vrrpv3OperationsRowStatus OBJECT-TYPE
>         SYNTAX       RowStatus
>         MAX-ACCESS   read-create
>         STATUS       current
>         DESCRIPTION
>             "The RowStatus variable should be used in accordance to
>             installation and removal conventions for conceptual
>             rows.
> 
>             To create a row in this table, a manager sets this
>             object to either createAndGo(4) or createAndWait(5).
>             Until instances of all corresponding columns are
>             appropriately configured, the value of the
>             Corresponding instance of the
>             `vrrpv3OperationsRowStatus' column will be read as
>             notReady(3).
> 
>             In particular, a newly created row cannot be made
>             active(1) until (minimally) the corresponding instance
>             of vrrpv3OperationsInetAddrType, vrrpv3OperationsVrId
>             and vrrpv3OperationsPrimaryIpAddr has been set and
>             there is at least one active row in the
>             `vrrpv3AssociatedIpAddrTable' defining an associated
>             IP address.
> 
>             notInService(2) should be used to administratively
>             bring the row down.
> 
>             A typical order of operation to add a row is:
>             1. Create a row in vrrpv3OperationsTable with
>             createAndWait(5).
>                2. Create one or more corresponding rows in
>             vrrpv3AssociatedIpAddrTable.
>             3. Populate the vrrpv3OperationsEntry.
>             4. set vrrpv3OperationsRowStatus to active(1).
> 
>             A typical order of operation to delete an entry is:
>             1. Set vrrpv3OperationsRowStatus to notInService(2).
>             2. Set the corresponding rows in
>             vrrpv3AssociatedIpAddrTable to destroy(6) to delete the
>             entry.
>             3. set vrrpv3OperationsRowStatus to destroy(6) to
>             delete the entry."
>         ::= { vrrpv3OperationsEntry 13 }

Looks good.

>> vrrpv3AssociatedIpAddr
>> 
>> You should add a statement the description that says that the content 
>> of the object is to be interpreted in the context of the setting of
>> vrrpv3OperationsInetAddrType in the index of this row.
> 
> [kalyan>>>]  Following is what i added :
>     vrrpv3AssociatedIpAddrAddress OBJECT-TYPE
>         SYNTAX       InetAddress (SIZE (0|4|16))
>         MAX-ACCESS   not-accessible
>         STATUS       current
>         DESCRIPTION
>             "The assigned IP addresses that a virtual router is
>             responsible for backing up.
> 
>             The IP address type is determined by the value of
>             vrrpv3OperationsInetAddrType in the index of this
>             row"
>         REFERENCE " RFC 5798 "
>         ::= { vrrpv3AssociatedIpAddrEntry 1 }

OK

>> VRRP Router Statistics
>> 
>> Do you need a discontinuity timer for the three global objects:
>> - vrrpv3RouterChecksumErrors
>> - vrrpv3RouterVersionErrors
>> - vrrpv3RouterVrIdErrors
> 
> [kalyan>>>]  I added the following :
> 
>    vrrpv3GlobalStatisticsDiscontinuityTime OBJECT-TYPE
>        SYNTAX     TimeStamp
>        MAX-ACCESS read-only
>        STATUS     current
>        DESCRIPTION
>            "The value of sysUpTime on the most recent occasion at
>             which one of vrrpv3RouterChecksumErrors,
>             vrrpv3RouterVersionErrors and vrrpv3RouterVrIdErrors
>             suffered a discontinuity.
> 
>             If no such discontinuities have occurred since the last
>             re-initialization of the local management subsystem,
>             then this object contains a zero value."
> 
>        ::= { vrrpv3Statistics 4 }

Fine

>> VRRP Router Statistics
>> 
>> In the presence of an attack or a broken router or host nearby, is it
possible
>> that Countr32 will not be large enough for the up-time of this router?
>> 
>> You can choose to use Counter64 or describe wrapping conditions.
> 
> [kalyan>>>] Changed all the statistics counters to Counter64 except for
>             vrrpv3StatisticsMasterTransitions

Great

>> vrrpv3StatisticsAdvIntervalErrors
>> 
>>                "The total number of VRRP advertisement packets
>>                received for which the advertisement interval is
>>                different than the one configured for the local virtual
>>                router.
>> 
>> Can you add a reference to vrrpv3OperationsAdvInterval
> 
> [kalyan>>>]  Following is the new text i have :
> 
>            The total number of VRRP advertisement packets
>             received for which the advertisement interval is
>             different from the vrrpv3OperationsAdvInterval
>             configured on this virtual router.

Perfect

>> vrrpv3ProtoError
>> 
>> Don't you think this is a *really* dangerous notification?
>> 
>> If a VR is under attack or receiving packets from a faulty speaker, it 
>> will spew notifications.
>> 
>> You should probably either add some thresholding objects (which is a fair 
>> bit of work) or a single object to turn notifications on and off (with the 
>> default being "off").
> 
> [kalyan>>>] I agree.  I had a notification control object in  earlier drafts
but was
> asked to be removed  during MIB DR. review. Following is the
> cut and paste of the comments:
> 
> >
> >> * vrrpTrapNewMasterCntl, vrrpTrapProtoErrorCntl
> >>
> >> Could notifications be generated or not, using
> >> RFC3413?
> >>
> > [Kalyan>] Notifications could be filtered/generated using RFC3413. I added
> > these to conveniently control notifications within this MIB (Based on
> > request from some one on the mailing list) You suggest we get rid of these?
> >
> 
> Yes, I think that most operators would likely use rfc3413.  Having this in 2
> places may create some confusion...

OK, we can go with 3413 if that is what you are happy with.

But I think you need a reference to 3413.
- Add it to the normative references
- Add it to some text early in the draft. There is not a lot
  of option. I suggest a new paragraph in the into saying:
    Notifications in this MIB module are controlled using the
    mechanisms defined in [RFC3413].
- Add a comment within the MIB module as:
OLD
   --   Notification Definitions
NEW
   --   Notification Definitions
   --   Notifications may be controlled using SNMP-NOTIFICATION-MIB
END

>> Notifications
>> 
>> As currently specified, both of the notifications will come from the
management
>> agent which will identify the physical router that sourced the notification,
but not
>> the VR.
>> 
>> Don't you need to add some index values to the notifications as well?
> 
> [kalyan>>>] Good point. Added the Following to uniquely identify row in
> vrrpv3OperationsTable:
> 
>     vrrpv3NewMaster NOTIFICATION-TYPE
>         OBJECTS      { ifIndex,
>                        vrrpv3OperationsVrId,
>                        vrrpv3OperationsInetAddrType,
>                        vrrpv3OperationsMasterIpAddr,
>                        vrrpv3StatisticsNewMasterReason
>                      }
>         STATUS       current
>         DESCRIPTION
>             "The newMaster notification indicates that the sending
>             agent has transitioned to 'Master' state."
>         ::= { vrrpv3Notifications 1 }
> 
>     vrrpv3ProtoError NOTIFICATION-TYPE
>         OBJECTS      { ifIndex,
>                        vrrpv3OperationsVrId,
>                        vrrpv3OperationsInetAddrType,
>                        vrrpv3StatisticsProtoErrReason
>                      }
>         STATUS       current
>         DESCRIPTION
>             "The notification indicates that the sending agent has
>             encountered the protocol error indicated by
>             vrrpv3StatisticsProtoErrReason."
>         ::= { vrrpv3Notifications 2 }

OK

>> Section 11
>> 
>> Since you are obsoleting RFC 2787, is it your intention to ask IANA to
deprecate
>> {mib-2 68} ?
> 
> [kalyan>>>] Yes.  I added the following to IANA considerations :
> 
>    [Editor's Note (to be removed prior to publication):  The IANA is
>    requested to assign a value for "ZZZ" under the 'mib-2' subtree
>    and to record the assignment in the SMI Numbers registry. When
>    the assignment has been made, the RFC Editor is asked to replace
>    "ZZZ" (here and in the MIB module) with the assigned value.
> 
>    This document obsoletes RFC 2787 and the IANA is requested to
>    deprecate the value 68 under 'mib-2' assigned to VRRP-MIB.]

OK

>> Would you please consider adding a short section on migrating from 
>> VRRP-MIB to VRRPV3-MIB?
> 
> [kalyan>>>] I am not sure there is a smooth migration path from VRRP-MIB to
> VRRPV3-MIB - I think we made a conscious decision that VRRV3 MIB will only
>  support RFC5798 and will not support earlier versions.
> 
> The way i see it -  Routers that implement  RFC2338 and VRRP-MIB will
> have to first migrate to RFC5798 and then make changes to
> VRRP-MIB implementation  to conform to  VRRPv3-MIB.
> 
> Should i mention this fact in the section?

No need. Your explanation here is clear.

Looking forward to seeing the revision.

Cheers,
Adrian