[imss] MIB doctor review part 1 (SYNTAX Checks): draft-ietf-imss-fc-fcsp-mib-00.txt

"WIJNEN, Bert \(Bert\)" <bwijnen@alcatel-lucent.com> Mon, 26 November 2007 14:47 UTC

Return-path: <imss-bounces@ietf.org>
Received: from [127.0.0.1] (helo=stiedprmman1.va.neustar.com) by megatron.ietf.org with esmtp (Exim 4.43) id 1IwfEp-00073N-PQ; Mon, 26 Nov 2007 09:47:07 -0500
Received: from imss by megatron.ietf.org with local (Exim 4.43) id 1IwfEo-000735-Il for imss-confirm+ok@megatron.ietf.org; Mon, 26 Nov 2007 09:47:06 -0500
Received: from [10.90.34.44] (helo=chiedprmail1.ietf.org) by megatron.ietf.org with esmtp (Exim 4.43) id 1IwfEo-00072b-6M for imss@ietf.org; Mon, 26 Nov 2007 09:47:06 -0500
Received: from ihemail2.lucent.com ([135.245.0.35]) by chiedprmail1.ietf.org with esmtp (Exim 4.43) id 1IwfEn-00022s-A3 for imss@ietf.org; Mon, 26 Nov 2007 09:47:06 -0500
Received: from ilexp02.ndc.lucent.com (h135-3-39-2.lucent.com [135.3.39.2]) by ihemail2.lucent.com (8.13.8/IER-o) with ESMTP id lAQEl12n016395; Mon, 26 Nov 2007 08:47:01 -0600 (CST)
Received: from DEEXP01.de.lucent.com ([135.248.187.65]) by ilexp02.ndc.lucent.com with Microsoft SMTPSVC(6.0.3790.1830); Mon, 26 Nov 2007 08:47:01 -0600
Received: from DEEXC1U02.de.lucent.com ([135.248.187.29]) by DEEXP01.de.lucent.com with Microsoft SMTPSVC(6.0.3790.1830); Mon, 26 Nov 2007 15:46:58 +0100
x-mimeole: Produced By Microsoft Exchange V6.5
Content-class: urn:content-classes:message
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: quoted-printable
Date: Mon, 26 Nov 2007 15:46:51 +0100
Message-ID: <D4D321F6118846429CD792F0B5AF471F7E5CE9@DEEXC1U02.de.lucent.com>
In-Reply-To: <D4D321F6118846429CD792F0B5AF471F7E5CA6@DEEXC1U02.de.lucent.com>
X-MS-Has-Attach:
X-MS-TNEF-Correlator:
Thread-Topic: MIB doctor review part 1 (SYNTAX Checks): draft-ietf-imss-fc-fcsp-mib-00.txt
Thread-Index: AcgK0vLbbD32Q+kDSKqF52DDcvyXNQR1sm7QACzkDYACxRH+oAAEOUpwAevc29A=
References: <FF29F13E2D78C047B4B79F4E062D036338796C@CORPUSMX20A.corp.emc.com><FF29F13E2D78C047B4B79F4E062D0363387A95@CORPUSMX20A.corp.emc.com><D4D321F6118846429CD792F0B5AF471F7E5BF4@DEEXC1U02.de.lucent.com><FF29F13E2D78C047B4B79F4E062D0363C2DC05@CORPUSMX20A.corp.emc.com> <D4D321F6118846429CD792F0B5AF471F7E5CA6@DEEXC1U02.de.lucent.com>
From: "WIJNEN, Bert (Bert)" <bwijnen@alcatel-lucent.com>
To: Black_David@emc.com, imss@ietf.org
X-OriginalArrivalTime: 26 Nov 2007 14:46:58.0880 (UTC) FILETIME=[32846400:01C8303B]
X-Scanned-By: MIMEDefang 2.57 on 135.245.2.35
X-Spam-Score: 0.0 (/)
X-Scan-Signature: b84f8c8fba0e1389e5eb998b64078964
Cc:
Subject: [imss] MIB doctor review part 1 (SYNTAX Checks): draft-ietf-imss-fc-fcsp-mib-00.txt
X-BeenThere: imss@ietf.org
X-Mailman-Version: 2.1.5
Precedence: list
List-Id: Internet and Management Support for Storage Working Group <imss.ietf.org>
List-Unsubscribe: <https://www1.ietf.org/mailman/listinfo/imss>, <mailto:imss-request@ietf.org?subject=unsubscribe>
List-Post: <mailto:imss@ietf.org>
List-Help: <mailto:imss-request@ietf.org?subject=help>
List-Subscribe: <https://www1.ietf.org/mailman/listinfo/imss>, <mailto:imss-request@ietf.org?subject=subscribe>
Errors-To: imss-bounces@ietf.org

1. T11-FC-SP-TC-MIB

   C:\bwijnen\smicng\imss>smicng T11-FC-SP-TC-MIB.inc
   W: f(T11-FC-SP-TC-MIB), (40,15) The first revision should match
      the last update for MODULE-IDENTITY t11FcTcMIB
   *** 0 errors and 1 warning in parsing

   C:\bwijnen\smicng\imss>smilint ./T11-FC-SP-TC-MIB
   ./T11-FC-SP-TC-MIB:44: revision for last update is missing

   I think it would be good to to get the REVISION and LAST-UPDATED
   timestamps in sync. No big deal though.

2. T11-FC-SP-ZONING-MIB

   C:\bwijnen\smicng\imss>smicng T11-FC-SP-ZONING-MIB.inc
   W: f(T11-FC-SP-ZONING-MIB), (404,20) Variable "ifIndex" in
notification
      "t11FcSpZsFabricJoinSuccessNotify" is an index for a table
   W: f(T11-FC-SP-ZONING-MIB), (420,20) Variable "ifIndex" in
notification
      "t11FcSpZsFabricJoinFailureNotify" is an index for a table

   *** 0 errors and 2 warnings in parsing

   C:\bwijnen\smicng\imss>smilint -l 6 -m -s ./T11-FC-SP-ZONING-MIB
   .. OK

   I think this is OK in this situation.

3. T11-FC-SP-SA-MIB

   C:\bwijnen\smicng\imss>smicng T11-FC-SP-SA-MIB.inc
   E: f(T11-FC-SP-SA-MIB), (1781,36) Item "t11FcSpSaTSelSpiDirection" in
      sequence "T11FcSpSaTSelSpiEntry" has conflicting syntax specified

   bw: caused by INTEGER and T11FcSaDirection TC (which has underlying
       SYNTAX of INTEGER (enumerated). So I am not sure it is a real
       error. At the other hand, the/a fix is easy, namely change

       T11FcSpSaTSelSpiEntry ::= SEQUENCE {
          t11FcSpSaTSelSpiInboundSpi     T11FcSpiIndex,
          t11FcSpSaTSelSpiTrafSelIndex   Unsigned32,
          t11FcSpSaTSelSpiDirection      INTEGER,
          t11FcSpSaTSelSpiTrafSelPtr     Unsigned32
       }

       into:

       T11FcSpSaTSelSpiEntry ::= SEQUENCE {
          t11FcSpSaTSelSpiInboundSpi     T11FcSpiIndex,
          t11FcSpSaTSelSpiTrafSelIndex   Unsigned32,
          t11FcSpSaTSelSpiDirection      T11FcSaDirection,
          t11FcSpSaTSelSpiTrafSelPtr     Unsigned32
       }

       It would also make it more consistent with other places where you
       have used T11FcSaDirection TC.

   E: f(T11-FC-SP-SA-MIB), (512,14) Index item "t11FcSpSaPropIndex" must
      be defined with syntax that includes a range

   bw: I tend to agree with that. For example I wonder of zero is a
valid
       index value. If it is, then we better explicitly include it (and
add 
       text to the DESCRIPTION clause what that means). If it is not,
       then we better exclude it.

   E: f(T11-FC-SP-SA-MIB), (684,14) Index item
"t11FcSpSaTSelPropListIndex"
      must be defined with syntax that includes a range
   E: f(T11-FC-SP-SA-MIB), (684,42) Index item "t11FcSpSaTSelPropIndex"
      must be defined with syntax that includes a range

   bw: same story for those above 2 error messages.

   E: f(T11-FC-SP-SA-MIB), (928,14) Index item "t11FcSpSaTransListIndex"
      must be defined with syntax that includes a range
   E: f(T11-FC-SP-SA-MIB), (928,39) Index item "t11FcSpSaTransIndex"
must
      be defined with syntax that includes a range

   bw: same story for those above 2 error messages.

   E: f(T11-FC-SP-SA-MIB), (1069,42) Index item
"t11FcSpSaTSelDrByPrecedence"
      must be defined with syntax that includes a range

   bw: would be easy to fix in TC module:

   T11FcSpPrecedence ::= TEXTUAL-CONVENTION
      DISPLAY-HINT "d"
      STATUS       current
      DESCRIPTION
           "The precedence of a Traffic Selector.  If a frame
           matches with two or more Traffic Selectors, then the match
           which takes precedence is the one with the Traffic Selector
           having the numerically smallest precedence value.  Note that
           precedence values are not necessarily contiguous."
      SYNTAX   Unsigned32  -- the default range: (0..4294967295)


    Why not just specify that range. It makes sense (I guess) that
    a value of zero in this case is valid, and so would also be
    a valid/acceptable INDEX value. We betetr make that explicit
    (or so I think).

   E: f(T11-FC-SP-SA-MIB), (1259,38) Index item
"t11FcSpSaPairInboundSpi"
      must be defined with syntax that includes a range

   bw: same as above.

   E: f(T11-FC-SP-SA-MIB), (1457,38) Index item
"t11FcSpSaTSelNegInIndex"
      must be defined with syntax that includes a range

   bw: SO is zero a valid/acceptable index? If so we better explicitly
       include that. If not, then we must exclude it.

   E: f(T11-FC-SP-SA-MIB), (1624,38) Index item
"t11FcSpSaTSelNegOutPrecedence"
      must be defined with syntax that includes a range

   bw: see comment above on the use of T11FcSpPrecedence TC

   E: f(T11-FC-SP-SA-MIB), (1774,14) Index item
"t11FcSpSaTSelSpiInboundSpi"
      must be defined with syntax that includes a range

   bw: see above

   E: f(T11-FC-SP-SA-MIB), (1774,42) Index item
"t11FcSpSaTSelSpiTrafSelIndex"
      must be defined with syntax that includes a range

   bw: Is zero a valid/acceptable value. It seems to me that in this
case
       we prefer NOT to allow for zero. So make that explicit.

   W: f(T11-FC-SP-SA-MIB), (1257,32) Row "t11FcSpSaPairEntry" does not
have
      a consistent indexing scheme - index items from current table must
come
      after index items from other tables
   W: f(T11-FC-SP-SA-MIB), (1456,32) Row "t11FcSpSaTSelNegInEntry" does
not
      have a consistent indexing scheme - cannot specify an index item
from
      additional "base row" t11FcSpSaPairEntry, since can have only one
      "base row" which is t11FcSpSaIfEntry
   W: f(T11-FC-SP-SA-MIB), (1623,32) Row "t11FcSpSaTSelNegOutEntry" does
not
      have a consistent indexing scheme - cannot specify an index item
from
      additional "base row" t11FcSpSaPairEntry, since can have only one
      "base row" which is t11FcSpSaIfEntry
   W: f(T11-FC-SP-SA-MIB), (1772,32) Row "t11FcSpSaTSelSpiEntry" does
not
      have a consistent indexing scheme - cannot specify an index item
      from additional "base row" t11FcSpSaPairEntry, since can have only
      one "base row" which is t11FcSpSaIfEntry

   The "non-consistent indexing scheme" issue is possibly just a
warning.
   In these cases I always ask the author: Pls check that what you did
   is what you intended. If so, then I think the above 4 are fine.
   I must say that the indexing certainly is not immediately obvious.
   I will need to go look in detail to convince myself.

   *** 12 errors and 4 warnings in parsing


   C:\bwijnen\smicng\imss>smilint -l 6 -m -s ./T11-FC-SP-ZONING-MIB
   .. OK

4. T11-FC-SP-POLICY-MIB

   C:\bwijnen\smicng\imss>smicng T11-FC-SP-POLICY-MIB.inc
   E: f(T11-FC-SP-POLICY-MIB), (1528,14) Index item
      "t11FcSpPoAuthProtIdentifier" must be defined with syntax
      that includes a range

   bw: Assuming that http://www.t11.org/ftp/t11/pub/fc/sp/06-157v3.pdf 
       is the proper document to look at, for table 11.
       From that table one could conclude that value zero is part
       of the "all others", but at the other hand, I would have put
       value zero as reserved at the top of the table if value
       zero is valid. 
   
   So... is it valid? In any event, I would make the range (0-4billion)
   or 1-4billion) explicit I think.

   E: f(T11-FC-SP-POLICY-MIB), (3443,14) Index item
      "t11FcSpPoNaAuthProtIdentifier" must be defined with syntax
      that includes a range

   bw: same story.

   *** 2 errors and 0 warnings in parsing


   C:\bwijnen\smicng\imss>smilint -l 6 -m -s ./T11-FC-SP-POLICY-MIB
   .. OK

5. T11-FC-SP-CERTS-MIB

   C:\bwijnen\smicng\imss>smicng T11-FC-SP-CERTS-MIB.inc
   E: f(T11-FC-SP-CERTS-MIB), (90,38) Index item "t11FcSpCertIndex"
      must be defined with syntax that includes a range

   bw: seems (to me) that we would want to exlude zero as an
       acceptable value.

   *** 1 error and 0 warnings in parsing

   C:\bwijnen\smicng\imss>smilint -l 6 -m -s ./T11-FC-SP-CERTS-MIB
   .. OK

6. T11-FC-SP-AUTHENTICATION-MIB

   C:\bwijnen\smicng\imss>smicng T11-FC-SP-AUTHENTICATION-MIB.inc
   Successful parsing

   C:\bwijnen\smicng\imss>smilint -l 6 -m -s
./T11-FC-SP-AUTHENTICATION-MIB
   .. OK


Bert 


_______________________________________________
imss mailing list
imss@ietf.org
https://www1.ietf.org/mailman/listinfo/imss