[mpls] WG last call review of draft-ietf-mpls-tp-linear-protection-mib

"Adrian Farrel" <adrian@olddog.co.uk> Fri, 13 January 2017 17:31 UTC

Return-Path: <adrian@olddog.co.uk>
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 B30AA129CD8; Fri, 13 Jan 2017 09:31:15 -0800 (PST)
X-Virus-Scanned: amavisd-new at amsl.com
X-Spam-Flag: NO
X-Spam-Score: -2.6
X-Spam-Level:
X-Spam-Status: No, score=-2.6 tagged_above=-999 required=5 tests=[BAYES_00=-1.9, RCVD_IN_DNSWL_LOW=-0.7] autolearn=ham autolearn_force=no
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 EUDUlEp0Juup; Fri, 13 Jan 2017 09:31:05 -0800 (PST)
Received: from asmtp2.iomartmail.com (asmtp2.iomartmail.com [62.128.201.249]) (using TLSv1 with cipher DHE-RSA-AES256-SHA (256/256 bits)) (No client certificate requested) by ietfa.amsl.com (Postfix) with ESMTPS id 7CE69129CCA; Fri, 13 Jan 2017 09:31:04 -0800 (PST)
Received: from asmtp2.iomartmail.com (localhost.localdomain [127.0.0.1]) by asmtp2.iomartmail.com (8.13.8/8.13.8) with ESMTP id v0DHV2IL009930; Fri, 13 Jan 2017 17:31:02 GMT
Received: from 950129200 ([176.241.251.3]) (authenticated bits=0) by asmtp2.iomartmail.com (8.13.8/8.13.8) with ESMTP id v0DHUwUh009920 (version=TLSv1/SSLv3 cipher=AES256-SHA bits=256 verify=NO); Fri, 13 Jan 2017 17:31:01 GMT
From: Adrian Farrel <adrian@olddog.co.uk>
To: mpls@ietf.org
Date: Fri, 13 Jan 2017 17:30:59 -0000
Message-ID: <034501d26dc2$d0e07660$72a16320$@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: AdJtwjJvjjJIdY0YTDWna6dfeB8arg==
Content-Language: en-gb
X-TM-AS-MML: disable
X-TM-AS-Product-Ver: IMSS-7.1.0.1679-8.0.0.1202-22822.001
X-TM-AS-Result: No--10.599-10.0-31-10
X-imss-scan-details: No--10.599-10.0-31-10
X-TMASE-MatchedRID: NLkmLLXlgXLOzFp3x8qWhpYsKSXWWrsHOL9BEHibhsjyy6AwyS9iukxO 7+3QV5trZH7HC+RxfH1+c4KDq3sC/gDBaeVuVh0CTd1FGyH+HrIP3XmN4dR/Q1vym/gvSH4iDpb rdhcd5/kEi4xvl9OQe+xOy/ns23N/xb2ZByTuHA8D2WXLXdz+AYLsLasl5ROhgU3gSqgUZvLSQH RfQjw/HxXOVP8R0A+OIHvJWyKGPXgpStHXmQedPd35+5/2RxqmcmsHQK7cMOexI6SgGDB3ced/a KciDvl7wXtozz/dHrEzJ2zCRPArPaOz+j0QSEPdh2VzUlo4HVN51GakW92m5qB81nGQQJDpMCXX Lxgy2JROmlU9SUbASOrFhCVn6Ze0mlLXzDiDGEMG1NkcAmdR4FxIyn/X4SmnHFSQk97VYGozBHK sDHLonzfXUO8XguJL7pfToFEZ52ZNlZ1zEcyAY0hEDfw/93Bu8kM1bk8+2qd3qTPyPrXqnKPFjJ EFr+olfeZdJ1XsoriiqdulWBsMFwtuKBGekqUpI/NGWt0UYPDiTzJdB7aqt9eGGGe+T9SN8o1nI lXZVRl+JacOh12SVSHWPYzouJUy
Archived-At: <https://mailarchive.ietf.org/arch/msg/mpls/DqqJpEJ6Rdug9-7J5j6rzVvd_GA>
Cc: draft-ietf-mpls-tp-linear-protection-mib@ietf.org
Subject: [mpls] WG last call review of draft-ietf-mpls-tp-linear-protection-mib
X-BeenThere: mpls@ietf.org
X-Mailman-Version: 2.1.17
Precedence: list
Reply-To: adrian@olddog.co.uk
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: Fri, 13 Jan 2017 17:31:16 -0000

WG LC review

I have reviewed the -11 version of this document. It is well written and
clear to read. I particularly welcome the explanations of the various
objects.

Here are a very few minor points and some nits.

I think the document is ready to proceed to publication.

Thanks,
Adrian

===

Section 1 concludes with 

   Although the example
   described in Section 7 specify means to configure OAM identifiers for
   MPLS-TP tunnels, this should be seen as indicating how the MIB values
   would be returned in the specified circumstances having been
   configured by alternative means.

Two thoughts:

1. This text needs to be repeated in Section 7

2. This would read better as:

   Although the example
   described in Section 7 shows a way to configure OAM identifiers for
   MPLS-TP tunnels, this also indicates how the MIB values would be
   returned if they had been configured by alternative means.

---

Section 4.

The first sentence is a little hard to read...

   RFC 6378 [RFC6378] defines the protocol to provide a linear
   protection switching mechanism for MPLS-TP with protection domain as
   a point-to-point LSP.

Looking at section 1.1. of RFC 6378, I think you could write...

   RFC 6378 [RFC6378] defines the protocol to provide a linear
   protection switching mechanism for MPLS-TP for a point-to-point LSP
   within the protection domain bounded by the end points of the LSP.

---

Is Section 5.1 too terse? Maybe a two line explanation of each new TC?

---

Please have a quick check to see whether sometimes when you say "this
MIB" you mean "this MIB module" (etc., for other uses of "MIB").

For example the Description clause of mplsLpsNotificationEnable
         "Provides the ability to enable and disable notifications
          defined in this MIB.

---

It is a little odd that MplsLpsReq has a syntax of 
OCTET STRING (SIZE (2)), a display hint of "1d", and you list the
potential values in binary.

I should think that the values should be listed in decimal as they
are shown in RFC6378 and RFC7271.

Then it is just a question of whether this should be a text string or
an integer, which probably doesn't matter, but if your keep it as a 
octet string, you do need to say how the numbers are encoded (presumably
ASCII?).

---

For MplsLpsFpathPath why do you say...
  Bits are numbered from left to right.
...I don't see any reference to bits.

---

mplsLpsConfigSdBadSeconds and mplsLpsConfigSdGoodSeconds could use a
Units clause (although it should be pretty obvious from the name and
description!)

---

Is there a reason why you used 
      SYNTAX     INTEGER {true (1), false (2)}
instead of TruthValue in 
   mplsLpsStatusRevertiveMismatch
   mplsLpsStatusProtecTypeMismatch 
   mplsLpsStatusCapabilitiesMismatch
   mplsLpsStatusPathConfigMismatch OBJECT-TYPE

---

   mplsLpsStatusPathConfigMismatch OBJECT-TYPE
      SYNTAX     INTEGER {true (1), falsmplsOamIdMeMpIndexNexte (2)}

Looks like a typo although it will compile :-)



===

Nits
---
Section 1
s/read- write/read-write/
s/document is consistent/document are consistent/
---
Section 4
s/alternate/alternative/
---
Section 5.3
s/failures of linear protection/failures of the linear protection/
---
mplsLpsMeStatusTable
s/liear/linear/