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

Rio Asnara <asnara101@gmail.com> Fri, 14 January 2011 17:15 UTC

Return-Path: <asnara101@gmail.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 9DB343A6C71 for <vrrp@core3.amsl.com>; Fri, 14 Jan 2011 09:15:54 -0800 (PST)
X-Virus-Scanned: amavisd-new at amsl.com
X-Spam-Flag: NO
X-Spam-Score: -2.998
X-Spam-Level:
X-Spam-Status: No, score=-2.998 tagged_above=-999 required=5 tests=[BAYES_00=-2.599, HTML_MESSAGE=0.001, J_CHICKENPOX_56=0.6, RCVD_IN_DNSWL_LOW=-1]
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 QTcEZwIBl8hn for <vrrp@core3.amsl.com>; Fri, 14 Jan 2011 09:15:51 -0800 (PST)
Received: from mail-qy0-f179.google.com (mail-qy0-f179.google.com [209.85.216.179]) by core3.amsl.com (Postfix) with ESMTP id 6B7613A6BA3 for <vrrp@ietf.org>; Fri, 14 Jan 2011 09:15:51 -0800 (PST)
Received: by qyj19 with SMTP id 19so3361925qyj.10 for <vrrp@ietf.org>; Fri, 14 Jan 2011 09:18:17 -0800 (PST)
DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=gamma; h=domainkey-signature:mime-version:in-reply-to:references:date :message-id:subject:from:to:cc:content-type; bh=lsY21Z2md0G7JHF6vttu/npv+SEABkKzaw+eZO1Q9w0=; b=pkDqVY8R1WLFmXwoAc88tg4gqLa5dL4hv+C8WnM/E6fH19qQU/YUeY+QLUjJf5ARPy 63H2pNZETT5QA9V+JJAcxW8ZNkZIlcVszdhUXSEnkf9BeIT+LLyM4MC+HAHk2chUsi/D QqU2xwV5BpxAnIduPRTqrZngYDZL6Vh1V6vIE=
DomainKey-Signature: a=rsa-sha1; c=nofws; d=gmail.com; s=gamma; h=mime-version:in-reply-to:references:date:message-id:subject:from:to :cc:content-type; b=FE63mept1KLWJilBmg1JaT39dAPZjN1c/DZd2PKEtpw9W1M4K//VNLVG8ndXZg+1nc K/4PuC/RKeuVqTzioF2vIbmn1ZsmiA14jecvIObU7ImCwkravOJxe2+NClrGn1+ofY97 pQoEmsR3xCJZPJt6i77rkDUy1kXE4q4AmESUc=
MIME-Version: 1.0
Received: by 10.229.99.130 with SMTP id u2mr913481qcn.87.1295025495941; Fri, 14 Jan 2011 09:18:15 -0800 (PST)
Received: by 10.220.192.197 with HTTP; Fri, 14 Jan 2011 09:18:15 -0800 (PST)
In-Reply-To: <497B6D90E0023142AF34948DEFFAB38D3B35815809@EMBX01-HQ.jnpr.net>
References: <00a701cbaf6b$6dfdc460$49f94d20$@huawei.com> <9FFC3234F1B7F0439C9B8BF94A83F482153242AA3F@USEXCHANGE.ad.checkpoint.com> <DD265143-A586-44F3-A9E8-EA3AEA7A90CF@juniper.net> <AANLkTi=n3=Z=Vmf0SmrCoSkUhHdAG0zei7JFscHVgHtb@mail.gmail.com> <497B6D90E0023142AF34948DEFFAB38D3B35815809@EMBX01-HQ.jnpr.net>
Date: Sat, 15 Jan 2011 00:18:15 +0700
Message-ID: <AANLkTinW6aJwARxs_k=sgSbqP9eARme5WEHHBZsWasMk@mail.gmail.com>
From: Rio Asnara <asnara101@gmail.com>
To: Mukesh Gupta <mukesh@juniper.net>
Content-Type: multipart/alternative; boundary=0016363b845ca921fa0499d19e6e
X-Mailman-Approved-At: Mon, 17 Jan 2011 11:40:31 -0800
Cc: "vrrp@ietf.org" <vrrp@ietf.org>, "vrrp-chairs@tools.ietf.org" <vrrp-chairs@tools.ietf.org>, "Adrian.Farrel@huawei.com" <Adrian.Farrel@huawei.com>, "draft-ietf-vrrp-unified-mib@tools.ietf.org" <draft-ietf-vrrp-unified-mib@tools.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: Fri, 14 Jan 2011 17:15:54 -0000

Please forgive my late to respond.
This weekend I had a conference with friends from china to discuss about the
SMSC, iVAS, CRBT as well as cellular network intelligence

yes, I had planned to immediately implement as soon as possible :)

Regards,
Asnara





On Fri, Jan 14, 2011 at 3:20 AM, Mukesh Gupta <mukesh@juniper.net>; wrote:

> Asnara,
>
>
>
> Thanks for your interest.  Do you have any timeline in mind?
>
>
>
> As you might know, IETF believes in rough consensus and running code.  So,
> having a few implementations of the draft would make all of us comfortable
> with the quality of the draft.
>
>
>
> Regards
>
> Mukesh
>
>
>
> *From:* Rio Asnara [mailto:asnara101@gmail.com]
> *Sent:* Tuesday, January 11, 2011 12:12 AM
> *To:* Mukesh Gupta
> *Cc:* Kalyan (Srinivas)Tata; vrrp-chairs@tools.ietf.org;
> Adrian.Farrel@huawei.com; draft-ietf-vrrp-unified-mib@tools.ietf.org;
> vrrp@ietf.org
> *Subject:* Re: [VRRP] AD review of draft-ietf-vrrp-unified-mib
>
>
>
> All,
> I'm interested to implemented on my virtual system :)
>
> Regards,
> Asnara
>
>
> On Tue, Jan 11, 2011 at 5:56 AM, Mukesh Gupta <mukesh@juniper.net>; wrote:
>
> Kalyan,
>
> Thanks a lot.  Please go ahead and address the comments and publish
> the next rev. We all want this draft done asap :)
>
> - Mukesh
>
> Sent from my iPhone
>
> On Jan 10, 2011, at 2:12 PM, "Kalyan (Srinivas)Tata" <stata@checkpoint.com
>
>  > wrote:
>
> > Thanks Adrian for the review. I will update the draft (Once WG
> > chairs confirm). I will getback if I need any clarifications.
> >
> > Thanks
> > Kalyan
> >
> > -----Original Message-----
> > From: vrrp-bounces@ietf.org [mailto:vrrp-bounces@ietf.org] On Behalf
> > Of Adrian Farrel
> > Sent: Saturday, January 08, 2011 11:37 AM
> > To: draft-ietf-vrrp-unified-mib@tools.ietf.org
> > Cc: vrrp-chairs@tools.ietf.org; vrrp@ietf.org
> > Subject: [VRRP] AD review of draft-ietf-vrrp-unified-mib
> >
> > Hi,
> >
> > Don't panic!
> >
> > I have performed my AD review of your draft. The purpose of the
> > review is to catch any nits or issues before the document goes
> > forward to IETF last call and IESG review. By getting these issues
> > out at this stage we can hope for a higher quality review and a
> > smoother passage through the process.
> >
> > There are a good number of small issues that I believe can be fixed
> > really easily.
> >
> > I appreciate that a number of these issues are inherited from RFC
> > 2787, but this is an ideal chance to clean up.
> >
> > You will need a new revision to address these points, and I'd ask
> > the WG chairs to evaluate whether the changes are large enough to
> > warrant a further WG last call.
> >
> > I have moved the draft into "AD-review:Revised-ID-needed" state in
> > the datatracker, and I look forward to seeing the new revision which
> > I can put forward for IETF last call.
> >
> > Thanks for all your work with this draft,
> >
> > Adrian
> >
> > ---
> >
> > Document header
> >
> > OLD
> >   Document: draft-ietf-vrrp-unified-mib-08.txt               July 2010
> >   Intended Status: Proposed Standard
> > NEW
> >   Document: draft-ietf-vrrp-unified-mib-08.txt               July 2010
> >   Obsoletes: 2787 (if approved)
> >   Intended Status: Proposed Standard
> > END
> >
> > ---
> >
> > Abstract
> >
> > OLD
> >   This specification defines a Management Information Base (MIB) for
> > NEW
> >   This specification defines a portion of the Management Information
> >   Base (MIB) for
> > END
> >
> > ---
> >
> > Section 2
> >
> > OLD
> >   This specification defines a Management Information Base (MIB) for
> > NEW
> >   This specification defines a portion of the Management Information
> >   Base (MIB) for
> > END
> >
> > ---
> >
> > Section 2 etc.
> >
> > Is it necessary to introduce the term "IPvX"? It is only used a
> > couple of times, and the time it is used in the MIB module is a
> > problem because the definition of the term is outside the module.
> > (Typically, MIB modules are extracted from RFCs and have to survive
> > as standalone
> > text.)
> >
> > Can you:
> > - remove "(IPvX)" form section 2
> > - remove the definition for section 3
> > - say "IPv4 and IPv6" in the two uses in section 6 and section 9
> >
> > ---
> >
> > Section 4
> >
> > You need to add text to describe what has changed from 2787. Not a
> > lot of details - perhaps a series of bullet points.
> >
> > --
> >
> > Section 6 etc.
> >
> > You need to say "MIB module" not "MIB" because there is only one
> > MIB, and you are making just a module in the MIB.
> >
> > s/This MIB is designed/This MIB module is designed/
> >
> >
> > ---
> >
> >
> > Section 7
> >
> >   Tables in the MIB include the following:
> >
> > In fact, there are exactly these three tables, so how about...
> >
> >   This MIB module contains three tables:
> >
> > ---
> >
> > Section 8
> >
> > Trivial, but...
> >
> >           -----   MIB Tables For VRRP Router "VR 1":   -----
> >
> > ...should read "VR1"
> > Similarly for VR2 later in the section.
> >
> > ---
> >
> > IMPORTS
> >
> > It is helpful, but mandatory, to show the RFC numbers from which
> > things are imported. Thus...
> >
> > OLD
> >       IMPORTS
> >           MODULE-IDENTITY, OBJECT-TYPE,
> >           NOTIFICATION-TYPE, Counter32,
> >           Integer32, mib-2, Unsigned32        FROM SNMPv2-SMI
> >
> >           TEXTUAL-CONVENTION, RowStatus,
> >           MacAddress, TruthValue, TimeStamp,
> >           TimeInterval                        FROM SNMPv2-TC
> >
> >           MODULE-COMPLIANCE, OBJECT-GROUP,
> >           NOTIFICATION-GROUP                  FROM SNMPv2-CONF
> >           ifIndex                             FROM IF-MIB
> >           InetAddressType, InetAddress        FROM INET-ADDRESS-MIB;
> > NEW
> >       IMPORTS
> >           MODULE-IDENTITY, OBJECT-TYPE,
> >           NOTIFICATION-TYPE, Counter32,
> >
> >           Integer32, mib-2, Unsigned32
> >               FROM SNMPv2-SMI                               -- RFC2578
> >
> >           TEXTUAL-CONVENTION, RowStatus,
> >           MacAddress, TruthValue, TimeStamp,
> >           TimeInterval
> >               FROM SNMPv2-TC                                -- RFC2579
> >
> >           MODULE-COMPLIANCE, OBJECT-GROUP,
> >           NOTIFICATION-GROUP
> >               FROM SNMPv2-CONF                              -- RFC2580
> >
> >           ifIndex
> >               FROM IF-MIB                                   -- RFC2863
> >
> >
> >           InetAddressType, InetAddress
> >               FROM INET-ADDRESS-MIB;                        -- RFC3291
> > END
> >
> > ---
> >
> > Section 9
> >
> > In order to ensure that references can be provided, it is customary
> > to begin Section 9 (i.e. before the module BEGIN statement) with
> > some text such as:
> >
> >   This MIB module makes reference to the following documents
> > [RFC2578],
> >   [RFC2579], [RFC2580], [RFC2863], [RFC3291], and [RFC4001].
> >
> > ---
> >
> > Vrrpv3VrIdTC
> >
> > Typos
> >               (ifIndex)and IP version, serves to uniquely identify a
> > Missing space.
> >
> >           REFERENCE " RFC 5798 (Sections 3 and 5.2.3"
> > Missing close brace.
> >
> > ---
> >
> > vrrpv3OperationsTable
> >
> >               "Unified Operations table for a VRRP router which
> >                consists of a sequence (i.e., one or more conceptual
> >                rows) of 'vrrpv3OperationsEntry' items which describe
> >                the operational characteristics of a virtual router."
> >
> > I think "which describe" should read "each of which describes"
> >
> > ---
> >
> > vrrpv3OperationsEntry
> >
> >                Rows in the table cannot be modified unless the value
> >                of 'vrrpv3OperStatus' has transitioned to
> >                'initialize' state.
> >
> > I think a little more precision would help...
> >
> >                A rows in this table cannot be modified unless the
> > value
> >                of 'vrrpv3OperStatus' in the row has transitioned to
> >                'initialize' state.
> >
> > ---
> >
> > vrrpv3OperationsInetAddrType
> >
> > As far as I can tell, you only support two values: ipv4(1) and ipv6
> > (2).
> > Other values of the InetAddressType textual conventions are, I
> > think, not supported.
> >
> > You should add this fact as a note to the DESCRIPTION clause.
> >
> > ---
> >
> > vrrpv3OperationsPrimaryIpAddr
> >
> >               "In the case where there are more than one IP
> >
> > s/are/is/
> >
> > ---
> >
> > vrrpv3OperationsVirtualMacAddr
> >
> >           REFERENCE "STD 58 RFC 2578"
> >
> > I am not clear why this reference is cited. MacAddress is defined in
> > RFC 2579, but you don't need to provide a reference because that is
> > implicit in the IMPORTS clause. Perhaps you mean to give a reference
> > to where the mapping from VRID to MAC address is defined?
> >
> > ---
> >
> > vrpv3OperStatus
> >
> > This object is incongruously named.
> > To fit with the naming convention for the table you need one of:
> > - vrpv3OperationsStatus
> > - vrpv3OperationsOperStatus
> >
> > Note that there are many references to this object throughout the
> > document.
> >
> > ---
> >
> > vrrpv3OperationsAcceptMode
> >
> >               "Controls whether a virtual router in Master state
> >               will accept packets addressed to the address owner's
> >               IPv6 address as its own if it is not the IPv6 address
> >               owner.  Default is False.
> >               This object is not relevant for rows representing VRRP
> >               over IPv4 and should be set to false."
> >
> > Should read...
> >
> >   Default is false(2)
> >
> > ...and...
> >
> >   should be set to false(2)."
> >
> > ---
> >
> > vrrpv3OperationsUpTime
> >           SYNTAX       TimeStamp
> >           MAX-ACCESS   read-only
> >           STATUS       current
> >           DESCRIPTION
> >               "This is the value of the `sysUpTime' object when this
> >               virtual router (i.e., the `vrrpv3OperStatus')
> >               transitioned out of `initialized'."
> >
> > I am not saying that you MUST change this, but I wonder how useful
> > it is, because to make sense of it, a management station must also
> > read the current value of sysUpTime.
> >
> >
> > An alternative is to supply the up time in timer ticks. That means
> > that the management agent has to perform a computation each the row
> > is read. If you did this you would have...
> >
> >           SYNTAX       TimeTicks
> >           MAX-ACCESS   read-only
> >           STATUS        current
> >           DESCRIPTION
> >               "This value represents the amount of time since this
> >               virtual router (i.e., the `vrrpv3OperStatus')
> >               transitioned out of `initialize'."
> >
> > I just need you to think about which you prefer (there is a trade-
> > off) and make a decision. No need to tell me which you chose, or why.
> >
> > Note also s/initialized/initialize/
> >
> > ---
> >
> > Creation and deletion of a vrrpv3OperationsTable row
> >
> > I'm slightly confused :-(
> >
> > We have...
> >
> >       vrrpv3OperationsEntry
> >
> >                Rows in the table cannot be modified unless the value
> >
> >                of 'vrrpv3OperStatus' has transitioned to
> >                'initialize' state.
> >
> > and
> >
> >       vrrpv3OperationsRowStatus
> >
> >               When `vrrpv3OperationsRowStatus' is set to
> >               active(1), no other objects in the conceptual row can
> >               be modified.
> >
> > In general, the instructions in the description of the rowStatus are
> > good an detailed. But I have some difficulty with row creation. What
> > I think is missing is a statement that the row must be created with
> > operStatus set to initialize(1) and cannot transition to backup(2) or
> > master(3) until rowStatus is transitioned to active(1).
> >
> > Similarly, there is an issue with row deletion. In order to delete,
> > the row I must first set rowStatus to notInService(2), and later to
> > delete(6). But, according to the description of operStatus, I cannot
> > modify the row (including the rowStatus) until operStatus has gone
> > to initialize(1). How do I get that to happen?
> >
> > I suspect this can be fixed by allowing rowStatus to be changed
> > regardless of the value of operStatus.
> >
> > ---
> >
> > vrrpv3AssociatedIpAddrTable
> >
> >               "The table of addresses associated with this virtual
> >                router."
> >
> > I think that there is just one table, and it contains the addresses
> > of all virtual routers. What about...
> >
> >               "The table of addresses associated with each virtual
> >                router."
> >
> > ---
> >
> > vrrpv3AssociatedIpAddrEntry
> >
> > OLD
> >               Rows in the table cannot be modified unless the value
> >               of `vrrpv3OperStatus' has transitioned to
> >               `initialize'.
> > NEW
> >               Rows in the table cannot be modified unless the value
> >               of `vrrpv3OperStatus' for the corresponding entry in the
> >               vrrpv3OperationsTable has transitioned to initialize(1).
> > END
> >
> > ---
> >
> > vrrpv3AssociatedIpAddr
> >
> > This object's name is odd given the convention for naming objects
> > within their tables. You probably need:
> >
> > vrrpv3AssociatedIpAddrAddress
> >
> > ---
> >
> > 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.
> >
> > ---
> >
> > VRRP Router Statistics
> >
> > Do you need a discontinuity timer for the three global objects:
> > - vrrpv3RouterChecksumErrors
> > - vrrpv3RouterVersionErrors
> > - vrrpv3RouterVrIdErrors
> >
> > ---
> >
> > 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.
> >
> > ---
> >
> > vrrpv3RouterVrIdErrors
> >
> >               "The total number of VRRP packets received with an
> >               invalid VRID for this virtual router."
> >
> > This object is global, so it is wider than the scope of a single VR.
> > I think you need:
> >
> >               "The total number of VRRP packets received with a
> >               VRID that is not valid for any virtual router on this
> >               router."
> >
> > ---
> >
> > 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
> >
> > ---
> >
> > 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").
> >
> > ---
> >
> > 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?
> >
> > ---
> >
> > Section 11
> >
> > Since you are obsoleting RFC 2787, is it your intention to ask IANA
> > to deprecate {mib-2 68} ?
> >
> > ---
> >
> > Would you please consider adding
> >
> > ---
> >
> > Section 12
> >
> > A reference to RFC 4001 needs to be added as it shows up in some
> > REFERENCE clauses.
> >
> > ---
> >
> > Would you please consider adding a short section on migrating from
> > VRRP-MIB to VRRPV3-MIB?
> >
> > _______________________________________________
> > vrrp mailing list
> > vrrp@ietf.org
> > https://www.ietf.org/mailman/listinfo/vrrp
> >
> > Scanned by Check Point Total Security Gateway.
> > _______________________________________________
> > vrrp mailing list
> > vrrp@ietf.org
> > https://www.ietf.org/mailman/listinfo/vrrp
> _______________________________________________
> vrrp mailing list
> vrrp@ietf.org
> https://www.ietf.org/mailman/listinfo/vrrp
>
>
>