Re: [mpls] 회신: Review of draft-ietf-mpls-tp-linear-protection-mib-07

Joan Cucchiara <jcucchiara.ietf@gmail.com> Thu, 26 May 2016 14:09 UTC

Return-Path: <jcucchiara.ietf@gmail.com>
X-Original-To: mpls@ietfa.amsl.com
Delivered-To: mpls@ietfa.amsl.com
Received: from localhost (localhost [127.0.0.1]) by ietfa.amsl.com (Postfix) with ESMTP id EC93312D171; Thu, 26 May 2016 07:09:46 -0700 (PDT)
X-Virus-Scanned: amavisd-new at amsl.com
X-Spam-Flag: NO
X-Spam-Score: -2.699
X-Spam-Level:
X-Spam-Status: No, score=-2.699 tagged_above=-999 required=5 tests=[BAYES_00=-1.9, DKIM_SIGNED=0.1, DKIM_VALID=-0.1, DKIM_VALID_AU=-0.1, FREEMAIL_FROM=0.001, HTML_MESSAGE=0.001, RCVD_IN_DNSWL_LOW=-0.7, SPF_PASS=-0.001] autolearn=ham autolearn_force=no
Authentication-Results: ietfa.amsl.com (amavisd-new); dkim=pass (2048-bit key) header.d=gmail.com
Received: from mail.ietf.org ([4.31.198.44]) by localhost (ietfa.amsl.com [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id v074y5Dh8rAI; Thu, 26 May 2016 07:09:43 -0700 (PDT)
Received: from mail-io0-x232.google.com (mail-io0-x232.google.com [IPv6:2607:f8b0:4001:c06::232]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by ietfa.amsl.com (Postfix) with ESMTPS id 65EF012B018; Thu, 26 May 2016 07:09:43 -0700 (PDT)
Received: by mail-io0-x232.google.com with SMTP id p64so12714520ioi.2; Thu, 26 May 2016 07:09:43 -0700 (PDT)
DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=mime-version:in-reply-to:references:date:message-id:subject:from:to :cc; bh=W4H7oPllHvce4rkwpmqeSPReWYbmrl2vrmVCeVNqlj8=; b=gDg6/jdI9x+v8e3saIxZU7GQIzpBgnpzkblK6w4zOPrFPXvk/7ShAx16McK5rFziZg cPe1Q3xHx8tasJHUVdn1ITO0Bjw+YOnwxzafL/NVVwUTngHNdnQu4ciLSeJnEiZf4sJ9 u0gH+mjAofufZzJEDJgaM/oBjhVhT49ZtOm5vQUd8Gze9pmpXt8QYMxk4xoX6xp4hpAU RoWy98WuxnAymEGWqfHH1C28U9vr14nCNaWnrzLQ9/MCWXoAwNSe28IrVlcBan0/GvEZ 8+ZNOYErS2kdMPHC3w2ni8Lhp0NGd5taRC/vvysnr6BGA4uGiQVtZi+RikcW3hP3JV1P oBfg==
X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:mime-version:in-reply-to:references:date :message-id:subject:from:to:cc; bh=W4H7oPllHvce4rkwpmqeSPReWYbmrl2vrmVCeVNqlj8=; b=bH9SnXFbj1nTy5riincAfrt7vFAnzCMf46g7b6pDUVBon17oACHR7AQww9XD9qgR04 XCzMd+/Du9wIvDuWN3ISVPHu/kv8zUut78alhd9wU6GZB7KPe8eccf25WOwSalNosSLN 1wlLK3ufBrZ6fesF1RBQW6LrwfpDmjXT+avLVhY0PzqlbyRHGM3O02Iu7DHehAk54vSx R7/IP55igYe08Rh0Jznk/pn0wejhd0q1ytd+eARuJL6Zu8m6K1SIIo72AXMOaq0iSPIf Ki+AowCholMnFOi3hFpeFZ2n0ugclt4v4whZPJTl8kTN69tg0UGb8IJ/OM2eYyKR0l6R 9+1w==
X-Gm-Message-State: ALyK8tIZiP2WSVoYMQ0g9gVK9/VCnA1njIiKwKWd5y5yOlbGhcUTxsBKRnBHABbnfWHIltKtkeGsP8ExfE+t5g==
MIME-Version: 1.0
X-Received: by 10.107.38.79 with SMTP id m76mr10020301iom.7.1464271782631; Thu, 26 May 2016 07:09:42 -0700 (PDT)
Received: by 10.107.46.219 with HTTP; Thu, 26 May 2016 07:09:42 -0700 (PDT)
In-Reply-To: <5B4A6CBE3924BB41A3BEE462A8E0B75A291DCD53@SMTP2.etri.info>
References: <00a901d18e84$2c19a340$844ce9c0$@mindspring.com> <5B4A6CBE3924BB41A3BEE462A8E0B75A291DCD53@SMTP2.etri.info>
Date: Thu, 26 May 2016 10:09:42 -0400
Message-ID: <CANSkkO=LiZdt4eVXDC7y2K00A1K8tvD8uUuxBnkkxteyu0aPyw@mail.gmail.com>
From: Joan Cucchiara <jcucchiara.ietf@gmail.com>
To: 류정동 <ryoo@etri.re.kr>
Content-Type: multipart/alternative; boundary="001a113da8287560e90533bf5725"
Archived-At: <http://mailarchive.ietf.org/arch/msg/mpls/8HUri8gItbna3GlTO_LIbYTsbrg>
Cc: "mpls@ietf.org" <mpls@ietf.org>, "draft-ietf-mpls-tp-linear-protection-mib@ietf.org" <draft-ietf-mpls-tp-linear-protection-mib@ietf.org>, Joan Cucchiara <jcucchiara@mindspring.com>, "mib-doctors@ietf.org" <mib-doctors@ietf.org>, "mpls-chairs@ietf.org" <mpls-chairs@ietf.org>
Subject: Re: [mpls] 회신: Review of draft-ietf-mpls-tp-linear-protection-mib-07
X-BeenThere: mpls@ietf.org
X-Mailman-Version: 2.1.17
Precedence: list
List-Id: Multi-Protocol Label Switching WG <mpls.ietf.org>
List-Unsubscribe: <https://www.ietf.org/mailman/options/mpls>, <mailto:mpls-request@ietf.org?subject=unsubscribe>
List-Archive: <https://mailarchive.ietf.org/arch/browse/mpls/>
List-Post: <mailto:mpls@ietf.org>
List-Help: <mailto:mpls-request@ietf.org?subject=help>
List-Subscribe: <https://www.ietf.org/mailman/listinfo/mpls>, <mailto:mpls-request@ietf.org?subject=subscribe>
X-List-Received-Date: Thu, 26 May 2016 14:09:47 -0000

Jeong-dong,

Thank you for the responses to my comments.   I will take a look at the
updated draft soon.

-Joan

On Thu, May 19, 2016 at 9:24 AM, 류정동 <ryoo@etri.re.kr> wrote:

> Dear Joan,
>
> We resolved all of your comments and uploaded a revsion today.
>
> Please, see inlines starting with [Authors] in your previous email below.
>
> The revised draft can be found in:
>
> https://www.ietf.org/internet-drafts/draft-ietf-mpls-tp-linear-protection-mib-08.txt
>
> We appreciate your help on this draft.
>
> Best regards,
>
> Jeong-dong
>
> ________________________________________
> 보낸 사람: Joan Cucchiara [jcucchiara@mindspring.com]
> 보낸 날짜: 2016년 4월 5일 화요일 오전 12:10
> 받는 사람: Loa Andersson; mpls@ietf.org;
> draft-ietf-mpls-tp-linear-protection-mib@ietf.org; mpls-chairs@ietf.org
> 참조: mib-doctors@ietf.org
> 제목: Review of draft-ietf-mpls-tp-linear-protection-mib-07
>
> Comments for draft-ietf-mpls-tp-linear-protection-mib-07.txt
>
> Authors,
>
> Lots of work went into this draft.  The early MIB Doctor review
> comments have been incorporated, so thank you for that.   These
> comments are arranged in 3 sections:  MIB compiler outputs,
> General Comments which are observations that apply to several
> places in the MIB and should be checked for throughout the MIB.
>
> The last section is for specific comments.
>
> Please take these comments as part of the last call.
>
> Thanks,
> -Joan
>
> Compiles cleanly with smilint
>
> smicng flagged some errors
> Output from smicng
> E: f(MPLS-LSP-MIB.my), (370,4) Row "mplsLpsConfigEntry" may not
> have columns with MAX-ACCESS of read-write if any column is read-create
> E: f(MPLS-LSP-MIB.my), (378,15) Index item
> "mplsLpsConfigDomainIndex" must be defined with syntax that
> includes a range
> E: f(MPLS-LSP-MIB.my), (907,4) Item "mplsLpsMeConfigDomainIndex"
> has invalid value for MAX-ACCESS
>
>
> When looking at the MIB, I see that there do appear to be some potential
> errors:
>    mplsLpsConfigCommand OBJECT-TYPE
>       SYNTAX      MplsLpsCommand
>       MAX-ACCESS  read-write    <---- should be read-create
>
> because row created using RowStatus
>       STATUS      current
>
> [Authors] OK, Fixed.
>
> In general, indices should specify a range so this is why it was
> flagged by compiler.  Looking at this specific index would like to
> understand how the value is supposed to be assigned?   If this is
> assigned by an operator, perhaps there should be a mechanism for
> the operator to choose a value (for example, by using a
> IndexIntegerNextFree object)?  Please clarify.
>
>    mplsLpsConfigDomainIndex OBJECT-TYPE
>       SYNTAX        Unsigned32
>
> [Authors] Fixed by using the IndexIntegerNextFree object.
>
> mplsLpsMeConfigDomainIndex <--- name implies that this is an index
> but it is NOT included in the INDEX clause for this table.
>
> Please clarify what is intended.
>
> [Authors] This is not the INDEX for this table. It is used to identify the
> corresponding mplsLpsConfigDomainIndex value in the other table,
> “mplsLpsConfigTable”. The name of this object is changed to
> mplsLpsMeConfigDomainIndexValue to avoid confusion.
>
>
> General Comments:
> =====================
>
> * There are mentions of there being two MIB Modules in this
> document, but there is only one MIB Module. I have tried to note
> these statements under specific comments, but please check for
> such statements.   If the intention is to create two MIB Modules,
> that is fine, but currently, there is only one.
>
> [Authors] Yes, there is only one defined in this document. Fixed
>
> * The relationships of these Tables is not clear.
> MplsLpsConfigTable has an INDEX but how the operator
> is supposed to choose a value for this index is
> not specified.  The MplsLpsMeConfigTable indexing is confusing.
> Although the document states that this table is an extension
> of the MPLSOamIdMeTable, the name of the object,
> mplsLpsMeConfigDomainIndex is confusing because it suggests
> this is an INDEX (as does the status of not-accessible).   Please clarify.
>
> [Authors] Fixed by using the IndexIntegerNextFree object and changing the
> name of mplsLpsConfigDomainIndex to mplsLpsMeConfigDomainIndexValue
>
> Since the indexing for these Tables is confusing to me, then
> please realize that this MIB may have additional comments
> during the next review once the indexing is clarified.
>
> * In general more REFERENCE Clauses could/should be added throughout MIB.
>
> Objects such as mplsLpsMeConfigDomainIndex, mplsLpsMeStatusCurrent,
> mplsLpsConfigMode, mplsLpsConfigProtectionType, etc.   This was also
> mentioned during the early MIB Dr. review.
>
> [Authors] OK. Added more REFERENCE Clauses.
>
> * Some of the objects use Integer32 but they probably should
> use Unsigned32.   In other words, if the objects can only take on values 0
> and above, then
> they should use Unsigned32.
>
> e.g.     mplsLpsConfigSdBadSeconds, mplsLpsConfigSdGoodSeconds,
> mplsLpsConfigWaitToRestore, mplsLpsConfigHoldOff, etc.  Please
> check all the Integer32 objects to see if they should be Unsigned32.
>
> [Authors] OK, Fixed.
>
> *) Date is same a previous version.  This should be updated for
> every revision of the document.  Please update.
>       LAST-UPDATED  and REVISION clauses
>     "201512060000Z"  -- December 06, 2015
>
> [Authors] OK, Fixed.
>
> *) Only FullCompliance is done for this MIB Module.  As you
> probably are aware, not all operators want to configure
> using SNMP, if there is not a ReadOnly Compliance available, then
> they will not be compliant with the MIB.  I think a ReadOnly Compliance
> for a MIB is useful and would like to understand why this MIB doesn't have
> one.
> Could the authors please clarify?
>
> [Authors] OK, ReadOnly Compliance is added.
>
>
> Specific Comments:
>
> Section 1. Introductions
>
> Mentions multiple MIB Modules but there is only one.  Please
> clarify the text:  "However, since the MIB modules ..." <-- plural
>
> [Authors] Yes, there is only one. Fixed.
>
>
> Section 4.
>
> As mentioned before there is only 1 MIB module.  Please update.
>
>    "This document specifies a MIB module
>     for the Label Edge Router (LER)
>     that supports MPLS TP Linear protection and a MIB
>     module that defines textual conventions....."
>
> [Authors] OK. Fixed.
>
> Section 5.1 Textual Conventions
>
> * I don't see a separate MIB Module for TCs.  Please clarify.
>
> [Authors] Fixed.
>
> Section 5.4 The Table Structure
>
>  * The mplsLpsConfigTable
>
> "The protection domain is identified by mplsLpsConfigGroupName."
> This statement does not seem to be entirely accurate given the MIB
> design for 2 reasons, 1.  there doesn't seem to be an object
> mplsLpsConfigGroupName
> and 2. the INDEX is mplsLpsConfigDomainIndex Unsigned32 (which also appears
> in the
> mplsLpsMeConfigEntry with a status of not-accessible
> (and I think you intend for it to be an object)?
>
> [Authors] “mplsLpsConfigGroupName” should be “mplsLpsConfigDomainName”.
> It’s been corrected.
>
> As a reviewer, this is confusing because the relationship with
> these tables is unclear and so it is very difficult to
> review the MIB Module.  Please clarify the relationship with
> these tables and to the mplsOamIdMeTable in the
> MPLS-OAM-ID-STD-MIB.
>
> [Authors] A protection domain consists of two paths, working and
> protection paths, and requires two OAM MEs; one OAM ME for the working path
> and the other ME for the protection path. In other words, a row of
> “mplsLpsConfigTable” is for one protection domain, which requires two rows
> in “mplsOamIdMeTable”: one for the working path and the other for the
> protection path. Also note that an entry of “mplsOamIdMeTable” may not
> belong to any protection domain. The row of “mplsLpsMeConfigTable” defined
> in this document has a sparse relationship with that of the
> “mplsOAMIdMeTable” defined in RFC 7697.
>
>
> "The other attributes in this table", do you mean objects?
>
> [Authors] Yes. Fixed.
>
>
> * The  mplsLpsStatusTable
>
> There is no mention that the mplsLpsStatusTable's Entries have an
> AUGMENTS relationship with the mplsLpsConfigTable Entries.  Please add.
>
> [Authors] Added.
>
>
> 6.1  Relationship to the MPLS OAM maintenance identifier MIB Module
>
> The title needs to be capitalized correctly, Relationship to the
> MPLS OAM Maintenance Identifier MIB Module
>
> Please update this section to use RFC7697 (and in Informative References
> also)
> instead of draft-ietf-mpls-tp-oam-id-mib.
>
> [Authors] Ok. Fixed.
>
>
> As mentioned above, the mplsLpsMeConfigTable has an object
> mplsLpsMeConfigDomainIndex which is (not-accessible).
> Is this supposed to be an INDEX, or is this an object?   I am
> confused by what is intended.
>
> [Authors] The name of this object is changed to
> mplsLpsMeConfigDomainIndexValue to avoid confusion.
>
>
> 7.  Example of Protection switching configuration for
>
> MPLS-TP TE tunnel (Please change title:  Example of Protection Switching
> Configuration)
>
> * I am unclear how mplsLspConfigEntry is actually configured for
> use in this example.   Is an operator supposed to randomly choose an INDEX
> value?
>
> Would an IndexNext object be useful to use in conjunction with this INDEX?
>
> [Authors] Yes, it has been addressed with IndexIntegerNextFree.
>
>
>
> MIB Module
> ------------
>
> (general comment:  the DESCRIPTION clauses could be more readable
> if consistency was used.  Sometimes the
> value is listed on the side and the description follows on the
> same line and sometimes the value is listed
> on a single line and the description follows a couple of lines
> after.   Please be consistent.)
>
> [Authors] Ok. Fixed.
>
>
> * mplsLpsConfigDomainName  -- Is there a DEFAULT value for this
> object?   The string size is 1..32 with no
> option of 0 length string, so wanted to check about a default
> value?  Under what circumstances can this value
> be modified?   Please give a REFERENCE.
>
> [Authors] No DEFAULT value is needed. The size has been changed to 0..32.
>
>
> * mplsLpsConfigMode - Needs REFERENCE (and please try to be
> specific).  Under what circumstances can this be modified?
>
> [Authors] REFERENCE is given.
>
>
> * mplsLpsConfigWaitToRestore
> Why is this not in minutes?  If someone configures this to be 30
> seconds is that valid?  Doesn't seem so based on the DESCRIPTION.  Please
> clarify.
>
> [Authors] Fixed with “minutes”. The range is also corrected.
>
>
> * mplsLpsConfigHoldOff What is meant by "Can be configured in
> steps of 100?"   Is this 100 milliseconds?  If so then maybe a better unit
> choice would be
> centiseconds.   Please clarify.
>
> [Authors] It can be configured like: 0, 100 ms, 200 ms, … , 10 seconds.
> So, the units and the description are changed using “deciseconds”.
>
> *mplsLpsConfigCommand is read-write.  Is this supposed to be
> read-create?
>
> [Authors] Yes. Fixed
>
>
> *mplsLpsConfigRowStatus --  I think there is some conflicting
> advice given to the operator.  Several objects say that it is fine to
> change
> the
> value of the object when RowStatus is active, but this is not specified
> consistently.  Limiting the
> values of RowStatus in the Conformance Section
> may be the way to go.  Please clarify.
>
> [Authors] There are some objects that can be changed during protocol
> operation, while other objects cannot be changed but their values need to
> be given before the operation. In the revision, we specified them
> consistently.
>
>
> *mplsLpsMeConfigState is a read-create. This is probably okay, but
> again, that depends on if mplsLpsMeConfigDomainIndex
> is an INDEX for this table given that it has a status of not-accessible,
> etc.
>
> [Authors] “mplsLpsMeConfigDomainIndex” was not intended to be INDEX, but
> to contain the value of the value of protection domain index. We changed it
> to mplsLpsMeConfigDomainIndexValue to avoid confusion
>
>
> *mplsLpsMeStatusSwitchoverSeconds
> Needs a units clause for Seconds
>
> [Authors] Fixed.
>
>
> Notifications
>
> There are a couple Notifications that are send when values of certain
> counters increment.  Maybe this is valid, but it seems suspect.
> If a management stations needs information on counters,
> why can't it just retrieve them at that point?   I don't see any
> counter discontinuity objects, so was wondering about that too.
>
> [Authors] Whenever there is an increment in any of the enabled counters,
> network operators need to be alarmed.
>
>
> * mplsLpsEventFopTimOut Notification
>
> Please rename this to mplsLpsEventFopTimeout
>
> [Authors] OK. Fixed.
>
>
> * Compliance/Conformance Section of the MIB Module
>
> Currently, there is only FullCompliance.   Why is there no
> ReadOnlyCompliance?
>
> [Authors] OK. It’s been added.
>
>
> --- end of comments ---
>
>
> _______________________________________________
> mpls mailing list
> mpls@ietf.org
> https://www.ietf.org/mailman/listinfo/mpls
>