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

"Kalyan (Srinivas)Tata" <stata@checkpoint.com> Wed, 06 April 2011 18:20 UTC

Return-Path: <stata@checkpoint.com>
X-Original-To: vrrp@core3.amsl.com
Delivered-To: vrrp@core3.amsl.com
Received: from localhost (localhost [127.0.0.1]) by core3.amsl.com (Postfix) with ESMTP id 9A2A928C0D6 for <vrrp@core3.amsl.com>; Wed, 6 Apr 2011 11:20:06 -0700 (PDT)
X-Virus-Scanned: amavisd-new at amsl.com
X-Spam-Flag: NO
X-Spam-Score: -9.598
X-Spam-Level:
X-Spam-Status: No, score=-9.598 tagged_above=-999 required=5 tests=[AWL=1.002, BAYES_00=-2.599, RCVD_IN_DNSWL_HI=-8]
Received: from mail.ietf.org ([64.170.98.32]) by localhost (core3.amsl.com [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id 1X7FM9g4iq3Y for <vrrp@core3.amsl.com>; Wed, 6 Apr 2011 11:20:05 -0700 (PDT)
Received: from us.checkpoint.com (usmail2.us.checkpoint.com [216.200.240.146]) by core3.amsl.com (Postfix) with ESMTP id 5A3F53A690A for <vrrp@ietf.org>; Wed, 6 Apr 2011 11:20:05 -0700 (PDT)
X-CheckPoint: {4D9CAF85-2-8AF0C8D8-FFFF}
Received: from us-ex01.ad.checkpoint.com (us-ex01.ad.checkpoint.com [216.200.240.139]) by us.checkpoint.com (8.14.4/8.14.4) with ESMTP id p36ILGf8023236; Wed, 6 Apr 2011 11:21:16 -0700
Received: from USEXCHANGE.ad.checkpoint.com ([216.200.240.132]) by US-EX01.ad.checkpoint.com ([216.200.240.139]) with mapi; Wed, 6 Apr 2011 11:21:49 -0700
From: "Kalyan (Srinivas)Tata" <stata@checkpoint.com>
To: "Adrian.Farrel@huawei.com" <Adrian.Farrel@huawei.com>, "draft-ietf-vrrp-unified-mib@tools.ietf.org" <draft-ietf-vrrp-unified-mib@tools.ietf.org>
Date: Wed, 6 Apr 2011 11:21:18 -0700
Thread-Topic: [VRRP] AD review of draft-ietf-vrrp-unified-mib
Thread-Index: AQJpH+xhvhccnkER8FVizp8VUMATpQHheBPtkwcshlCAAJZqoA==
Message-ID: <9FFC3234F1B7F0439C9B8BF94A83F48215327F3B8E@USEXCHANGE.ad.checkpoint.com>
References: <00a701cbaf6b$6dfdc460$49f94d20$@huawei.com> <9FFC3234F1B7F0439C9B8BF94A83F48215326C4534@USEXCHANGE.ad.checkpoint.com> <02d901cbf441$81427960$83c76c20$@huawei.com>
In-Reply-To: <02d901cbf441$81427960$83c76c20$@huawei.com>
Accept-Language: en-US
Content-Language: en-US
X-MS-Has-Attach:
X-MS-TNEF-Correlator:
acceptlanguage: en-US
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: quoted-printable
MIME-Version: 1.0
Cc: "vrrp-chairs@tools.ietf.org" <vrrp-chairs@tools.ietf.org>, "vrrp@ietf.org" <vrrp@ietf.org>
Subject: Re: [VRRP] AD review of draft-ietf-vrrp-unified-mib
X-BeenThere: vrrp@ietf.org
X-Mailman-Version: 2.1.9
Precedence: list
List-Id: Virtual Router Redundancy Protocol <vrrp.ietf.org>
List-Unsubscribe: <https://www.ietf.org/mailman/listinfo/vrrp>, <mailto:vrrp-request@ietf.org?subject=unsubscribe>
List-Archive: <http://www.ietf.org/mail-archive/web/vrrp>
List-Post: <mailto:vrrp@ietf.org>
List-Help: <mailto:vrrp-request@ietf.org?subject=help>
List-Subscribe: <https://www.ietf.org/mailman/listinfo/vrrp>, <mailto:vrrp-request@ietf.org?subject=subscribe>
X-List-Received-Date: Wed, 06 Apr 2011 18:20:07 -0000

Thanks Adrian,
I will have new draft ready this weekend.

Thanks
Kalyan
-----Original Message-----
From: Adrian Farrel [mailto:Adrian.Farrel@huawei.com] 
Sent: Wednesday, April 06, 2011 3:01 AM
To: Kalyan (Srinivas)Tata; draft-ietf-vrrp-unified-mib@tools.ietf.org
Cc: vrrp-chairs@tools.ietf.org; vrrp@ietf.org
Subject: RE: [VRRP] AD review of draft-ietf-vrrp-unified-mib

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


Scanned by Check Point Total Security Gateway.