Mime-Version: 1.0 (Mac OS X Mail 6.3 \(1503\))
Content-Type: text/plain; charset=iso-8859-1
From: Sam Aldrin <aldrin.ietf@gmail.com>
In-Reply-To: <010401ce37e7$15b17ed0$6f01a8c0@JoanPC>
Date: Sat, 13 Apr 2013 23:05:14 -0700
Content-Transfer-Encoding: quoted-printable
Message-Id: <1A9C31CE-51E6-4BC4-A505-DB5EEDF10545@gmail.com>
References: <010401ce37e7$15b17ed0$6f01a8c0@JoanPC>
To: Joan Cucchiara <jcucchiara@mindspring.com>
Cc: mpls@ietf.org, Kannan Sampath <kannankvs@gmail.com>,
 "MIB Doctors \(E-mail\)" <mib-doctors@ietf.org>,
 Venkatesan Mahalingam <venkat.mahalingams@gmail.com>,
 Loa Andersson <loa@pi.nu>
Subject: Re: [mpls] MIB Dr. review of draft-ietf-mpls-tp-te-mib-05
Precedence: list

Hi Joan,

Thank you for your detailed review.
Will address all the comments you have raised.

An updated version with appropriate changes will be sent for your =
review, prior to publishing the new version.

thanks again.

-sam
On Apr 12, 2013, at 6:34 PM, Joan Cucchiara <jcucchiara@mindspring.com> =
wrote:

> Sam,
>=20
> I am sorry for such a long delay....here is the MIB Dr. Review.   Lots =
of progress on the draft!
>=20
> Please see the comments below.
>=20
> Thanks,
> -Joan
>=20
>=20
> smicng OUTPUT
> --------------
>=20
> MPLS-LSR-EXT-STD-MIB
> =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D
> W: f(MPLS-LSR-EXT-STD-MIB.my), (216,18) For "mplsXCExtTunnelPointer", =
syntax is identical
> W: f(MPLS-LSR-EXT-STD-MIB.my), (215,18) MIN-ACCESS value identical to =
access specified for "mplsXCExtTunnelPointer"
> W: f(MPLS-LSR-EXT-STD-MIB.my), (223,18) For =
"mplsXCExtOppositeDirXCPtr", syntax is identical
>=20
> *) The DESCRIPTION clauses related to these objects
> should be updated to include the information in this
> conformance section.
>=20
>=20
>=20
>=20
> MPLS-TE-EXT-STD-MIB
> =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D
> E: f(MPLS-TE-EXT-STD-MIB.my), (284,17) Index item =
"mplsTunnelExtNodeIpMapNodeId" must be defined with syntax that includes =
a range
>=20
> I see the following in MPLS-TC-EXT-STD-MIB
>  MplsNodeId ::=3D TEXTUAL-CONVENTION
>  ....
>     SYNTAX  Unsigned32  -- the default range: (0..4294967295)
>=20
> my suggestion is to include the range as part of the SYNTAX in the TC.
>=20
>=20
> smilint
> -----------
> MIBs compile cleanly.
>=20
>=20
> GENERAL COMMENTS
> ----------------
> *) The terms augments/sparse augments/extensions are all used to
> describe relationships with tables in this document and
> tables in rfc3812 and rfc3813.  For example,
> a table in this document says "sparse augments" but then has
> a value of noSuchInstance returned if the counter is not applicable
> for TP.
>=20
> Another part of the document says that a table in this document
> augments a table from elsewhere, but that does not seem to be the
> case when looking at the MIB module.
>=20
> The relationships beween tables in this document and
> the tables in rfc3812 and rfc3813 need to be specified clearly.
> I would ask that the authors please check these relationships between
> the tables.   This may be (simply?) using the term
> 'sparse augments' consistently.
>=20
>=20
>=20
> *) Appendix D of rfc4181 (MIB guidelines) suggests:
>=20
>=20
>        xxxMIB
>        |
>        +-- xxxNotifications(0)
>        +-- xxxObjects(1)
>        +-- xxxConformance(2)
>            |
>            +-- xxxCompliances(1)
>            +-- xxxGroups(2)
>=20
> The MIB Modules in this doc:
>=20
>  \-2 Conformance
>    \-v-1 Groups
>=20
>      \-2 Compliances
>=20
> So Groups and Compliances are not in the suggested order.
>=20
>=20
> Specific Comments
> =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D
>=20
> *) Introduction
>=20
> "This MIB module should be used...."
>=20
> Which MIB module are you referring to?   (Do you mean "These MIB =
modules"??)
>=20
> *) Same Section (Intro)
> "...for MPLS based traffic engineering configuration and management."
>=20
> I don't understand completely, but seems like MPLS TP is probably what =
is meant.
>=20
>=20
> *) 4. Motivations
>=20
> s/used in non-IP environment./used in non-IP environments.
>=20
> awkward sentence:  This MIB also defines three other MIB modules =
within this document.
> Maybe:  This document defines 4 MIB modules: (and then list all 4)
>=20
>=20
> 5. Feature List
> Could you refer to the MIB module by name?  So, instead of
> "MPLS transport profile MIB module" use the MPLS-TE-EXT-STD-MIB
> and so forth.
>=20
>=20
> 6. Brief description of MIB Objects
> This section focuses on MPLS-TE-EXT-STD-MIB, so maybe rename the =
section
> or also include the other MIB modules in this document.
>=20
> 6.5 mplsTunnelExtReversePerfTable
>=20
> "This table augments the mplsTunnelTable..."
>=20
> If it augments, then why is AUGMENTS not used in the
> MPLS-TE-EXT-STD-MIB?   I think this relationship is a
> sparce augments, not an actual augments?   Please
> explain.
>=20
>=20
> MPLS-TC-EXT-MIB
> =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D
> *)  Why is the CC ID not represented in this MIB Module?
>=20
> Basically, the question has to do with the following from
> draft-ietf-mpls-tp-itu-t-identifiers, specifically:
>=20
> "Together, the CC and the ICC form the ICC_Operator_ID as:
>=20
>     CC::ICC
>=20
>  The ICC_Operator_ID is used as a replacement for the Global_ID as
>  specified in [RFC6370], i.e. its purpose is to provide a globally
>  unique context for other MPLS-TP identifiers."
>=20
> ICC_Operator_ID appears to be the counterpart to GLOBAL_ID.  In other
> words, these both uniquely identify an operator. Whereas, ICC by =
itself
> does not.
>=20
>=20
> MPLS-ID-STD-MIB
> =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D
>=20
> *)What is the relationship between mplsIdGlobalId and mplsIdNodeId?
> Specifically, if mplsIdGlobalId has been set, can mplsIdNodeId be
> set to a different value?
>=20
>=20
> *) NIT:  Could the order reflect the same order as in =
MPLS-TC-EXT-STD-MIB?
> The reason is that GLobal_ID and NODE_ID are related so think would be
> logical to have them near each other.
>=20
> *) Also, I'm not really sure why you want a MIB module with only 3 =
scalars
> in it?  As far as I can see these scalars are only used in the
> MPLS-TE-EXT-STD-MIB, so why separate them?
>=20
> *) Error:  FullCompliance and ReadOnlyCompliance is EXACTLY the same.
>=20
> MPLS-LSR-EXT-STD-MIB
> =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D
>=20
> Compliance:
>    OBJECT      mplsXCExtTunnelPointer
>    SYNTAX      RowPointer
>    MIN-ACCESS  read-only
>    DESCRIPTION
>       "The only valid value for Tunnel Pointer is
>        mplsTunnelTable entry."
>=20
> *) This object is read-only, so no need for a read-only compliance
> since object is already read-only.
>=20
> *) The above DESCRIPTION needs to be included in the
> object's DESCRIPTION.
>=20
>=20
>    OBJECT      mplsXCExtOppositeDirXCPtr
>    SYNTAX      RowPointer
>    MIN-ACCESS  read-only
>    DESCRIPTION
>       "The only valid value for XC Pointer is
>        mplsXCTable entry."
>=20
>    ::=3D { mplsLsrExtCompliances 2 }
>=20
> *) The above DESCRIPTION needs to be included in the
> object's DESCRIPTION.
>=20
> *) There is nothing here that specifies the readOnly compliance.
> Would expect something like:
>=20
> OBJECT nameOfObject
> MIN-ACCESS  read-only
> DESCRIPTION "Write access is not required."
>=20
>=20
> MPLS-TE-EXT-STD-MIB
> =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D
>=20
> *)          DESCRIPTION
>          "This object indicates the Global Operator Identifier.
>           This object value should be zero when
>           mplsTunnelExtNodeConfigIccId is configured with non-null
>           value."
>=20
> DESCRIPTION should state that the object has no meaning when
> mplsTunnelExtNodeConfigIccId is valid.  Same comment for
> mplsTunnelExtNodeConfigId.
>=20
> In other words, maybe an additional object is needed to indicate
> Global or ICC?  What if a 3rd TP id is created, then what happens?
>=20
> Better to be explicite I think, than to try and overload these
> objects with values that they shouldn't have.
>=20
> mplsTunnelExtNodeConfigNodeId  OBJECT-TYPE
>        SYNTAX        MplsNodeId
>        MAX-ACCESS    read-create
>        STATUS        current
>        DESCRIPTION
>           "This object indicates the Node_ID within the operator.
>=20
> *) Sentence is awkward.
>=20
> mplsTunnelExtNodeConfigIccId
> Similar comment to the above.  Additionally, how can an OCTET STRING
> of size (1..6) have a zero value?
>=20
> *) mplsTunnelExtNodeConfigStorageType
>=20
> Not sure I see the advantage of having this as it is
> already in the MplsTunnelTable.  Why have 2 StorageType objects
> for the same row?
>=20
> *) mplsTunnelExtIngressLSRLocalIdValid and
> mplsTunnelExtEgressLSRLocalIdValid
> Please fix the DESCRIPTION and REFERENCE.
> (Looks like the DESCRIPTION clause has been continued after the
> REF clause.)
>=20
>=20
>=20
> *) mplsTunnelExtReversePerfTable
>=20
> I would like to understand why noSuchInstance would be needed.
> This table is a sparse augments,
> so either the entry would be there or it wouldn't, right?
>=20
> *) Probably could remove the following (comment applies to other
> MIB Modules also.)
>=20
>    -- Notifications
>    mplsTeExtNotifications OBJECT IDENTIFIER
>                                     ::=3D { mplsTeExtStdMIB 0 }
>=20
>    -- Notifications.
>    -- Notification objects need to be added here.
>    -- End of notifications.
>=20
>=20
> *) Compliance Section
> ReadOnly compliance is the same as the Full Compliance.
> In other words, readonly compliance does not really exist.
> Please fix this.
>=20
>=20
> 14. Security Considerations
> =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D
> Should specify which objects would jeopardize security.
>=20
> 15. IANA Considerations
> =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=

>=20
> Where is it?  This section needs to be done.
>=20
>=20
> -- the end --
>=20