[mpls] Review of draft-ietf-mpls-tp-linear-protection-mib-07
"Joan Cucchiara" <jcucchiara@mindspring.com> Mon, 04 April 2016 15:09 UTC
Return-Path: <jcucchiara@mindspring.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 B7D2512D78A; Mon, 4 Apr 2016 08:09:39 -0700 (PDT)
X-Virus-Scanned: amavisd-new at amsl.com
X-Spam-Flag: NO
X-Spam-Score: -2.72
X-Spam-Level:
X-Spam-Status: No, score=-2.72 tagged_above=-999 required=5 tests=[BAYES_00=-1.9, DKIM_SIGNED=0.1, DKIM_VALID=-0.1, DKIM_VALID_AU=-0.1, RCVD_IN_DNSWL_LOW=-0.7, RCVD_IN_MSPIKE_H3=-0.01, RCVD_IN_MSPIKE_WL=-0.01] autolearn=ham autolearn_force=no
Authentication-Results: ietfa.amsl.com (amavisd-new); domainkeys=pass (384-bit key) header.from=jcucchiara@mindspring.com header.d=mindspring.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 rLUpDbdGkD5T; Mon, 4 Apr 2016 08:09:36 -0700 (PDT)
Received: from elasmtp-masked.atl.sa.earthlink.net (elasmtp-masked.atl.sa.earthlink.net [209.86.89.68]) by ietfa.amsl.com (Postfix) with ESMTP id 71CFB12D724; Mon, 4 Apr 2016 08:09:36 -0700 (PDT)
DomainKey-Signature: a=rsa-sha1; q=dns; c=nofws; s=dk20050327; d=mindspring.com; b=MygzPr+zgZb6FBxH1RvETUH8btSmpb5L1ZRY9qD33xPsXlNtbsNElxhQUqQCjFgB; h=Received:From:To:Cc:Subject:Date:Message-ID:MIME-Version:Content-Type:Content-Transfer-Encoding:X-Mailer:Thread-Index:Content-Language:X-ELNK-Trace:X-Originating-IP;
Received: from [72.93.239.57] (helo=JoanTower) by elasmtp-masked.atl.sa.earthlink.net with esmtpsa (TLSv1:AES128-SHA:128) (Exim 4.67) (envelope-from <jcucchiara@mindspring.com>) id 1an67l-0006wp-KB; Mon, 04 Apr 2016 11:09:05 -0400
From: Joan Cucchiara <jcucchiara@mindspring.com>
To: Loa Andersson <loa@pi.nu>, mpls@ietf.org, draft-ietf-mpls-tp-linear-protection-mib@ietf.org, mpls-chairs@ietf.org
Date: Mon, 04 Apr 2016 11:10:49 -0400
Message-ID: <00a901d18e84$2c19a340$844ce9c0$@mindspring.com>
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
X-Mailer: Microsoft Outlook 14.0
Thread-Index: AdGOgu3jx1rdAtVCTCy7UrFLqFTuiw==
Content-Language: en-us
X-ELNK-Trace: 4d68bbe9cb71969ea344cf2d1a8e60840a9da525759e2654dda0a30e53f1c0b99b09c09edee31f90676ae045857c2f36350badd9bab72f9c350badd9bab72f9c
X-Originating-IP: 72.93.239.57
Archived-At: <http://mailarchive.ietf.org/arch/msg/mpls/Wkwzb_6mTs_MA41gJ02uN-6-fwM>
Cc: mib-doctors@ietf.org
Subject: [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: Mon, 04 Apr 2016 15:09:40 -0000
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 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 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. 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. * 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. 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. * 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. *) 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 *) 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? Specific Comments: Section 1. Introductions Mentions multiple MIB Modules but there is only one. Please clarify the text: "However, since the MIB modules ..." <-- plural 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....." Section 5.1 Textual Conventions * I don't see a separate MIB Module for TCs. Please clarify. 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)? 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. "The other attributes in this table", do you mean objects? * The mplsLpsStatusTable There is no mention that the mplsLpsStatusTable's Entries have an AUGMENTS relationship with the mplsLpsConfigTable Entries. Please add. 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. 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. 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? 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.) * 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. * mplsLpsConfigMode - Needs REFERENCE (and please try to be specific). Under what circumstances can this be modified? * 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. * 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. *mplsLpsConfigCommand is read-write. Is this supposed to be read-create? *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. *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. *mplsLpsMeStatusSwitchoverSeconds Needs a units clause for Seconds 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. * mplsLpsEventFopTimOut Notification Please rename this to mplsLpsEventFopTimeout * Compliance/Conformance Section of the MIB Module Currently, there is only FullCompliance. Why is there no ReadOnlyCompliance? --- end of comments ---
- Re: [mpls] 회신: Review of draft-ietf-mpls-tp-linea… Loa Andersson
- [mpls] Review of draft-ietf-mpls-tp-linear-protec… Joan Cucchiara
- [mpls] 회신: Review of draft-ietf-mpls-tp-linear-pr… 류정동
- Re: [mpls] 회신: Review of draft-ietf-mpls-tp-linea… Joan Cucchiara
- Re: [mpls] 회신: Review of draft-ietf-mpls-tp-linea… Ryoo, Jeong-dong