Re: [CCAMP] AD review of draft-ietf-ccamp-gmpls-ted-mib

"Adrian Farrel" <adrian@olddog.co.uk> Wed, 30 May 2012 13:07 UTC

Return-Path: <adrian@olddog.co.uk>
X-Original-To: ccamp@ietfa.amsl.com
Delivered-To: ccamp@ietfa.amsl.com
Received: from localhost (localhost [127.0.0.1]) by ietfa.amsl.com (Postfix) with ESMTP id 69CDC21F86DE for <ccamp@ietfa.amsl.com>; Wed, 30 May 2012 06:07:49 -0700 (PDT)
X-Virus-Scanned: amavisd-new at amsl.com
X-Spam-Flag: NO
X-Spam-Score: -2.053
X-Spam-Level:
X-Spam-Status: No, score=-2.053 tagged_above=-999 required=5 tests=[AWL=-0.054, BAYES_00=-2.599, J_CHICKENPOX_21=0.6]
Received: from mail.ietf.org ([12.22.58.30]) by localhost (ietfa.amsl.com [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id yw0Vzfc9x7-M for <ccamp@ietfa.amsl.com>; Wed, 30 May 2012 06:07:48 -0700 (PDT)
Received: from asmtp3.iomartmail.com (asmtp3.iomartmail.com [62.128.201.159]) by ietfa.amsl.com (Postfix) with ESMTP id 1118721F86C1 for <ccamp@ietf.org>; Wed, 30 May 2012 06:07:47 -0700 (PDT)
Received: from asmtp3.iomartmail.com (localhost.localdomain [127.0.0.1]) by asmtp3.iomartmail.com (8.13.8/8.13.8) with ESMTP id q4UD7kI2008002; Wed, 30 May 2012 14:07:46 +0100
Received: from 950129200 (dsl-sp-81-140-15-32.in-addr.broadbandscope.com [81.140.15.32]) (authenticated bits=0) by asmtp3.iomartmail.com (8.13.8/8.13.8) with ESMTP id q4UD7gLL007937 (version=TLSv1/SSLv3 cipher=AES128-SHA bits=128 verify=NO); Wed, 30 May 2012 14:07:45 +0100
From: Adrian Farrel <adrian@olddog.co.uk>
To: 'Masanori Miyazawa' <ma-miyazawa@kddilabs.jp>, draft-ietf-ccamp-gmpls-ted-mib.all@tools.ietf.org
References: <0b7101cd3de9$8bfadbc0$a3f09340$@olddog.co.uk> <006301cd3e63$fd362180$f7a26480$@kddilabs.jp>
In-Reply-To: <006301cd3e63$fd362180$f7a26480$@kddilabs.jp>
Date: Wed, 30 May 2012 14:07:40 +0100
Message-ID: <0c7a01cd3e65$338b5240$9aa1f6c0$@olddog.co.uk>
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
X-Mailer: Microsoft Outlook 14.0
Thread-Index: AQHQW7vPShMYMTWtVF3+Petz8y8AvgLiYJVqlsT+3gA=
Content-Language: en-gb
Cc: ccamp@ietf.org
Subject: Re: [CCAMP] AD review of draft-ietf-ccamp-gmpls-ted-mib
X-BeenThere: ccamp@ietf.org
X-Mailman-Version: 2.1.12
Precedence: list
Reply-To: adrian@olddog.co.uk
List-Id: Discussion list for the CCAMP working group <ccamp.ietf.org>
List-Unsubscribe: <https://www.ietf.org/mailman/options/ccamp>, <mailto:ccamp-request@ietf.org?subject=unsubscribe>
List-Archive: <http://www.ietf.org/mail-archive/web/ccamp>
List-Post: <mailto:ccamp@ietf.org>
List-Help: <mailto:ccamp-request@ietf.org?subject=help>
List-Subscribe: <https://www.ietf.org/mailman/listinfo/ccamp>, <mailto:ccamp-request@ietf.org?subject=subscribe>
X-List-Received-Date: Wed, 30 May 2012 13:07:49 -0000

Yes, please.

If changes are needed after looking at my comments and discussing them, then
please post a new revision.

Cheers,
Adrian

> -----Original Message-----
> From: Masanori Miyazawa [mailto:ma-miyazawa@kddilabs.jp]
> Sent: 30 May 2012 13:59
> To: adrian@olddog.co.uk; draft-ietf-ccamp-gmpls-ted-mib.all@tools.ietf.org
> Cc: ccamp@ietf.org
> Subject: RE: AD review of draft-ietf-ccamp-gmpls-ted-mib
> 
> Hi, Adrian
> 
> Thank you for reviewing the draft.
> 
> We will check and answer your comments shortly.
> When we want to modify the draft based on your comments, how should we
> update it?  We should post revised document as latest version (version 14)?
> 
> With Best Regards,
> Masanori
> 
> > -----Original Message-----
> > From: Adrian Farrel [mailto:adrian@olddog.co.uk]
> > Sent: Wednesday, May 30, 2012 7:23 AM
> > To: draft-ietf-ccamp-gmpls-ted-mib.all@tools.ietf.org
> > Cc: ccamp@ietf.org
> > Subject: AD review of draft-ietf-ccamp-gmpls-ted-mib
> >
> > Hi,
> >
> > I have carried out my normal AD review of your draft. There is no cause
> > for alarm!
> >
> > This looks to be a sound document, but as always with MIB modules it is
> > possible to find a number of nits and questions. I would appreciate a
> > revision of the document before we move it forward so that it will get
> > a smoother ride through the rest of the process. I will put the document
> > into "Revised ID needed" state in the tracker.
> >
> > As always, all issues and questions are open for discussion.
> >
> > Thanks,
> > Adrian
> >
> > ---
> >
> > Per idnits, please change the header
> > OLD
> > Intended status: Standards Truck
> > NEW
> > Intended status: Proposed Standard
> > END
> >
> > ---
> >
> > I am not comfortable with the two definitions
> >    OAM:   Operations and Management
> >    OA&M:  Operations, Administration and Maintenance.
> > This is counter to RFC 6291which, although only guidelines, should be
> > followed unless there is a good reason. That RFC has...
> >    o  O&M - OAM and Management
> >    o  OAM - Operations, Administration, and Maintenance Fortunately
> > neither term is used in your document, and so they can be deleted.
> >
> > ---
> >
> > There are some acronyms in 3.3 that are not used in the rest of the
> > document: FSC, LDP, LSC, LSR, OAM, OA&M, RSVP,
> >
> > ---
> >
> > Could you please sort out "MIB" versus "MIB module". There is only one
> > MIB. There are many MIB modules - this document defines a MIB module.
> >
> > ---
> >
> > Section 6 is missing a close brace "}" for tedTable
> >
> > ---
> >
> > The copyright date in the MODULE-IDENTITY is 2011.
> >
> > ---
> >
> > You need to avoid using citations (e.g. [RFC3630]) within the MIB module
> > itself. MIB modules are extracted from their documents and stand alone.
> >
> > ---
> >
> > The DESCRIPTION for the tedTable is
> >   "This table indicates multiple TED information which has been
> >    supported by [RFC3630]."
> >
> > This seems to imply this doesn't apply to ISIS. Maybe update the text?
> >
> > Similarly, a number of objects only give references to OSPF or OSPF-TE.
> > Not referencing the corresponding IS-IS specs looks like the object only
> > has meaning for OSPF.
> >
> > ---
> >
> >    tedEntry OBJECT-TYPE
> >        SYNTAX       TedEntry
> >        MAX-ACCESS   not-accessible
> >        STATUS       current
> >        DESCRIPTION
> >        "This entry contains TED information commonly utilized in both
> >    MPLS and GMPLS."
> >       INDEX { tedLocalRouterId, tedRemoteRouterId,
> >    tedLinkInformationSource, tedLinkIndex }
> >    ::= { tedTable 1 }
> >
> >    TedEntry ::= SEQUENCE {
> >        tedLinkInformationSource     INTEGER,
> >        tedLocalRouterId             TedRouterIdTC,
> >        tedRemoteRouterId            TedRouterIdTC,
> >        tedLinkIndex                 TedLinkIndexTC,
> >
> > It is unusual to list the index fields in a different order from that
> > which they appear in the table entry. Is this intended?
> >
> > ---
> >
> > Tom's surname is misspelled in several of the references!
> >
> > ---
> >
> > I'm having some trouble with the use of 2119 language in the Description
> > for tedLinkInformationData.
> >
> > Two examples...
> >
> >           If tedLinkInformationSource has the value unknown(0), this
> >        object SHOULD contain a value of zeroDotZero.
> >
> > Under what circumstances can this use of "SHOULD" be varied?
> >
> >           If tedLinkInformationSource has the value
> >        locallyConfigured(1), this object MAY contain the identifier of
> >        the corresponding row entry in the teLinkTable of TE-LINK-STD-
> >        MIB[RFC4220], the identifier of the corresponding row in a local
> >        proprietary TE link MIB module, or the value of zeroDotZero
> >        otherwise.
> >
> > The use of "MAY" here implies that an implementation can choose to fill
> > in the row pointer if it likes, but this is entirely an implementation
> > choice. Is this what you are saying, or do you want to constrain the
> > behavior more forcefully if TE-LINK-STD-MIB is implemented?
> >
> > ---
> >
> >    tedRouterIdAddrType OBJECT-TYPE
> >        SYNTAX       InetAddressType
> >        MAX-ACCESS   read-only
> >        STATUS       current
> >        DESCRIPTION
> >          " This object indicates the TE-Router ID address type. Only
> >    values unknown(0), ipv4(1) or ipv6(2) must be supported. "
> >
> > Do you mean s/must be/are/  ?
> >
> > Similarly for tedLinkIdAddrType
> >
> > ---
> >
> > It's a small point, but I would have preferred you to rename
> > tedRouterIdAddrType and tedRouterIdAddr as tedTeRouterIdAddrType and
> > tedTeRouterIdAddr to distinguish them from the normal router ID.
> >
> > ---
> >
> > I don't understand what values should be returned for objects that only
> > apply in some circumstances. For example, in a non-GMPLS system, what
> > would be returned for tedLinkProtectionType? In either a non-GMPLS system
> > or one with numbered links, what values would be returned for tedLocalId
> > and tedRemoteId? What about tedSwCapIndication for non-TDM links?
> >
> > Section 6 says in these cases the objects are not supported, but I don't
> > think it is that simple. Or is it?
> >
> > Even if it is that simple, I think the Description clauses should not
> > the circumstances under which the object is not supported.
> >
> > (And yes, I do see the compliance statements which are very comprehensive,
> > but I think they say what you have to support in specific circumstances,
> > not how to behave if you support an object but have no information to
> > return.)
> >
> > ---
> >
> >    tedLocalIfAddrEntry OBJECT-TYPE
> >        SYNTAX       TedLocalIfAddrEntry
> >        MAX-ACCESS   not-accessible
> >        STATUS       current
> >        DESCRIPTION
> >          "This entry contains the IP address information of the local
> > TE
> >    link."
> >        INDEX { tedLinkIndex, tedLocalIfAddr }
> >    ::= { tedLocalIfAddrTable 1 }
> >
> >    TedLocalIfAddrEntry ::= SEQUENCE {
> >        tedLocalIfAddrType    InetAddressType,
> >        tedLocalIfAddr        InetAddress
> >
> >        }
> >
> > What would it look like to walk this table by increasing index value?
> > Would all the shorter v4 addresses show first, or would the numerically
> > smaller v6 addresses get mixed in with the v4 addresses?
> >
> > If the latter is possible, you should use the address type as an index
> > as well.
> >
> > ---
> >
> > A broader point about the use of addresses as indexes is found in RFC
> > 4001 in the Description of the InetAddress TC...
> >
> >          When this textual convention is used as the syntax of an
> >          index object, there may be issues with the limit of 128
> >          sub-identifiers specified in SMIv2, STD 58.  In this case,
> >          the object definition MUST include a 'SIZE' clause to
> >          limit the number of potential instance sub-identifiers;
> >          otherwise the applicable constraints MUST be stated in
> >          the appropriate conceptual row DESCRIPTION clauses, or
> >          in the surrounding documentation if there is no single
> >          DESCRIPTION clause that is appropriate."
> >
> > ---
> >
> > tedStatusChangeNotificationMaxRate and
> > tedCreatedDeletedNotificationMaxRate
> >
> > I am surprised that the Defval for these is 0 (no throttling) since the
> > text recognises that this is a risky situation that may cause the box
> > to blow up. Surely the default should be some safe setting.
> >
> > But then I wondered what a suitable safe setting would be and realised
> > it would be "No Notifications". Except there is no way to say this with
> > these objects.
> >
> > What do you think?
> >
> > ---
> >
> > Would the Notifications be more helpful if they indicated the index of
> > the entry in the tedTable to which they applied?
> >
> > ---
> >
> > For completeness, we usually get asked to state in the Security section
> > what the risks are with write access objects. In this case it is really
> > easy...
> >
> >    There are only two write-access objects in this MIB module:
> >    tedStatusChangeNotificationMaxRate and
> >    tedCreatedDeletedNotificationMaxRate.  Malicious modification of
> >    these objects could cause the management agent, the network, or the
> >    router to become overloaded with Notifications in cases of high
> >    churn within the network.