Re: [Rserpool] Last Call: draft-ietf-rserpool-mib (Reliable ServerPooling: Management Information Base using SMIv2) toExperimental RFC)

"Bert Wijnen \(IETF\)" <bertietf@bwijnen.net> Tue, 03 February 2009 17:26 UTC

Return-Path: <rserpool-bounces@ietf.org>
X-Original-To: rserpool-archive@megatron.ietf.org
Delivered-To: ietfarch-rserpool-archive@core3.amsl.com
Received: from [127.0.0.1] (localhost [127.0.0.1]) by core3.amsl.com (Postfix) with ESMTP id 2DFC83A6B96; Tue, 3 Feb 2009 09:26:34 -0800 (PST)
X-Original-To: rserpool@core3.amsl.com
Delivered-To: rserpool@core3.amsl.com
Received: from localhost (localhost [127.0.0.1]) by core3.amsl.com (Postfix) with ESMTP id E1ADC3A6A40; Tue, 3 Feb 2009 08:15:55 -0800 (PST)
X-Virus-Scanned: amavisd-new at amsl.com
X-Spam-Flag: NO
X-Spam-Score: -0.072
X-Spam-Level:
X-Spam-Status: No, score=-0.072 tagged_above=-999 required=5 tests=[AWL=0.370, BAYES_00=-2.599, HELO_MISMATCH_NET=0.611, HOST_EQ_NL=1.545, HTML_MESSAGE=0.001]
Received: from mail.ietf.org ([64.170.98.32]) by localhost (core3.amsl.com [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id eyylTpgChwVs; Tue, 3 Feb 2009 08:15:53 -0800 (PST)
Received: from relay.versatel.net (beverwijk.tele2.vuurwerk.nl [62.250.3.53]) by core3.amsl.com (Postfix) with ESMTP id 3BA543A67FD; Tue, 3 Feb 2009 08:15:52 -0800 (PST)
Received: from [87.215.199.34] (helo=BertLaptop) by relay.versatel.net with smtp (Exim 4.69) (envelope-from <bertietf@bwijnen.net>) id 1LUNvu-0001m6-Vo; Tue, 03 Feb 2009 17:15:31 +0100
Message-ID: <910DB442FB3247CDB14FE49A17D96DA3@BertLaptop>
From: "Bert Wijnen (IETF)" <bertietf@bwijnen.net>
To: Thomas Dreibholz <dreibh@iem.uni-due.de>
References: <EDC652A26FB23C4EB6384A4584434A040132DFF5@307622ANEX5.global.avaya.com> <3CE21C4BF0A340DB959CEB3D6BE9DC9B@BertLaptop> <200902031349.19970.dreibh@iem.uni-due.de>
In-Reply-To: <200902031349.19970.dreibh@iem.uni-due.de>
Date: Tue, 03 Feb 2009 17:14:58 +0100
Organization: Consultant
MIME-Version: 1.0
X-Priority: 3
X-MSMail-Priority: Normal
X-Mailer: Microsoft Windows Mail 6.0.6001.18000
X-MimeOLE: Produced By Microsoft MimeOLE V6.0.6001.18049
X-Mailman-Approved-At: Tue, 03 Feb 2009 09:26:33 -0800
Cc: "MIB Doctors (E-mail)" <mib-doctors@ietf.org>, rserpool@ietf.org
Subject: Re: [Rserpool] Last Call: draft-ietf-rserpool-mib (Reliable ServerPooling: Management Information Base using SMIv2) toExperimental RFC)
X-BeenThere: rserpool@ietf.org
X-Mailman-Version: 2.1.9
Precedence: list
List-Id: Reliable Server Pooling <rserpool.ietf.org>
List-Unsubscribe: <https://www.ietf.org/mailman/listinfo/rserpool>, <mailto:rserpool-request@ietf.org?subject=unsubscribe>
List-Archive: <https://www.ietf.org/mailman/private/rserpool>
List-Post: <mailto:rserpool@ietf.org>
List-Help: <mailto:rserpool-request@ietf.org?subject=help>
List-Subscribe: <https://www.ietf.org/mailman/listinfo/rserpool>, <mailto:rserpool-request@ietf.org?subject=subscribe>
Content-Type: multipart/mixed; boundary="===============1804878683=="
Sender: rserpool-bounces@ietf.org
Errors-To: rserpool-bounces@ietf.org

Thomas, thanks for the responses.

W.r.t.
>> - According to RFC4181 this one
>>          rserpoolMIBConformance OBJECT IDENTIFIER ::= { rserpoolMIB 4 }
>>    should change to
>>             rserpoolMIBConformance OBJECT IDENTIFIER ::= { rserpoolMIB 2 }
>
> 1 is used for the ENRP servers branch, 2 is used for PE branch, 3 for PU 
> branch. The next available number is 4.

The normal setup (according to rfc41`81) would be something like:

rserpoolMIBObjects          OBJECT-IDENTIFIER ::= { rserpoolMIB 1 }
rserpoolMIBConformance  OBJECT IDENTIFIER ::= { rserpoolMIB 2 }

rserpoolENRPServers       OBJECT IDENTIFIER ::= { rserpoolMIBObjects 1 }
rserpoolPoolElements      OBJECT IDENTIFIER ::= { rserpoolMIBObjects 2 }
rserpoolPoolUsers           OBJECT IDENTIFIER ::= { rserpoolMIBObjects 3 }

Your new MIB module has no indertation at all. 
Not a fatal flaw, but does not help in readability.

The new MIB module causes these SMICng warnings:
W: f(rserpool.mi2), (137,1) Sequence "RSerPoolENRPEntry" and Row "rserpoolENRPEntry" should have related names
W: f(rserpool.mi2), (276,1) Sequence "RSerPoolENRPPoolEntry" and Row "rserpoolENRPPoolEntry" should have related names
W: f(rserpool.mi2), (315,1) Sequence "RSerPoolENRPPoolElementEntry" and Row "rserpoolENRPPoolElementEntry" should have related names
W: f(rserpool.mi2), (465,1) Sequence "RSerPoolENRPASAPAddrTableEntry" and Row "rserpoolENRPASAPAddrTableEntry" should have related names
W: f(rserpool.mi2), (520,1) Sequence "RSerPoolENRPUserAddrTableEntry" and Row "rserpoolENRPUserAddrTableEntry" should have related names
W: f(rserpool.mi2), (584,1) Sequence "RSerPoolENRPENRPAddrTableEntry" and Row "rserpoolENRPENRPAddrTableEntry" should have related names
W: f(rserpool.mi2), (636,1) Sequence "RSerPoolENRPPeerEntry" and Row "rserpoolENRPPeerEntry" should have related names
W: f(rserpool.mi2), (695,1) Sequence "RSerPoolENRPPeerAddrTableEntry" and Row "rserpoolENRPPeerAddrTableEntry" should have related names
W: f(rserpool.mi2), (753,1) Sequence "RSerPoolPoolElementEntry" and Row "rserpoolPEEntry" should have related names
W: f(rserpool.mi2), (941,1) Sequence "RSerPoolPEASAPAddrTableEntry" and Row "rserpoolPEASAPAddrTableEntry" should have related names
W: f(rserpool.mi2), (994,1) Sequence "RSerPoolPEUserAddrTableEntry" and Row "rserpoolPEUserAddrTableEntry" should have related names
W: f(rserpool.mi2), (1060,1) Sequence "RSerPoolPoolUserEntry" and Row "rserpoolPUEntry" should have related names

*** 0 errors and 12 warnings in parsing

Probably cause by sticking to a better naming convention. Bit it would be consistent throughout.
It seems likd what you have is not absolutely incorrect. Yet... it is certainly not following the
way things are normally done.

I think this is more what I would expect:

   rserpoolENRPTable OBJECT-TYPE
   SYNTAX     SEQUENCE OF RserpoolENRPEntry

Then the ENTRY spec should read like:

   rserpoolENRPEntry OBJECT-TYPE
   SYNTAX     RserpoolENRPEntry

And then:

   RserpoolENRPEntry ::= SEQUENCE {
   rserpoolENRPIndex                Unsigned32,


Same further down in the MIB module.

Hope this helps,

Bert Wijnen

----- Original Message ----- 
  From: Thomas Dreibholz 
  To: Bert Wijnen (IETF) 
  Cc: rserpool@ietf.org 
  Sent: Tuesday, February 03, 2009 1:49 PM
  Subject: Re: [Rserpool] Last Call: draft-ietf-rserpool-mib (Reliable ServerPooling: Management Information Base using SMIv2) toExperimental RFC)


  On Dienstag 27 Januar 2009, Bert Wijnen (IETF) wrote:

  Dear all,

  see my comments inline. Attached to this mail, you find an updated version of 
  the MIB file.


  > Pls note that I am not on the resepool mailing list, so send an explicit
  > cc/bcc if
  > you want me to see it.
  >
  > I am getting these SMICng (strict checking) errors/warnings:
  >
  > C:\bw\smicng\work>smicng rserpool.inc
  > W: f(rserpool.mi2), (133,4) Sequence "ENRPServerEntry" and Row
  > "enrpServerEntry" should have related
  > names
  > W: f(rserpool.mi2), (167,15) Item "enrpServerOperationScope" should have
  > SIZE specified
  > W: f(rserpool.mi2), (272,4) Sequence "ENRPServerPoolEntry" and Row
  > "enrpServerPoolEntry" should have
  > related names
  > W: f(rserpool.mi2), (295,15) Item "enrpServerPoolHandle" should have SIZE
  > specified
  > W: f(rserpool.mi2), (311,4) Sequence "ENRPServerPoolElementEntry" and Row
  > "enrpServerPoolElementEntry
  > " should have related names
  > W: f(rserpool.mi2), (461,4) Sequence "ENRPServerASAPAddrTableEntry" and Row
  > "enrpServerASAPAddrTableE
  > ntry" should have related names
  > W: f(rserpool.mi2), (515,4) Sequence "ENRPServerUserAddrTableEntry" and Row
  > "enrpServerUserAddrTableE
  > ntry" should have related names
  > W: f(rserpool.mi2), (560,15) Item "enrpServerUserL3Opaque" should have SIZE
  > specified
  > W: f(rserpool.mi2), (578,4) Sequence "ENRPServerENRPAddrTableEntry" and Row
  > "enrpServerENRPAddrTableE
  > ntry" should have related names
  > W: f(rserpool.mi2), (629,4) Sequence "ENRPServerPeerEntry" and Row
  > "enrpServerPeerEntry" should have
  > related names
  > W: f(rserpool.mi2), (688,4) Sequence "ENRPServerPeerAddrTableEntry" and Row
  > "enrpServerPeerAddrTableE
  > ntry" should have related names
  > W: f(rserpool.mi2), (784,15) Item "poolElementOperationScope" should have
  > SIZE specified
  > W: f(rserpool.mi2), (792,15) Item "poolElementPoolHandle" should have SIZE
  > specified
  > W: f(rserpool.mi2), (1026,15) Item "poolElementUserL3Opaque" should have
  > SIZE specified
  > W: f(rserpool.mi2), (1071,15) Item "poolUserOperationScope" should have
  > SIZE specified
  > W: f(rserpool.mi2), (1079,15) Item "poolUserPoolHandle" should have SIZE
  > specified
  >
  > *** 0 errors and 16 warnings in parsing

  I do not have SMICng installed, but these problems should be fixed now.


  > I wonder if
  >
  >        ::= { mib-2 xxx } -- To be IANA Assigned!!!
  >
  > is appropriate for an EXPERIMENTAL MIB module
  > Probably want to root it under the experimental tree?

  Okay, this is useful. Fixed.


  > Further I see a lot of naming inconsistencies
  >
  > - Normally, in a MIB module we prefix all TCs with a prefix that makes it
  > clear
  >   which module these TCs are defined in. This is to try and avoid that the
  > TC
  >   names/identifiers will not conflict with any existing or future other
  > TCs. Specifically for a experiemntal module you do not want to have
  > conflicts with standards track MIB modules.
  >   So for these
  >
  >    ENRPServerIdentifierType ::= TEXTUAL-CONVENTION
  >    OperationScopeType ::= TEXTUAL-CONVENTION
  >    PoolHandleType ::= TEXTUAL-CONVENTION
  >    DescriptionType ::= TEXTUAL-CONVENTION
  >    PoolElementIdentifierType ::= TEXTUAL-CONVENTION
  >    PolicyIDType ::= TEXTUAL-CONVENTION
  >    PolicyLoadType ::= TEXTUAL-CONVENTION
  >    PolicyWeightType ::= TEXTUAL-CONVENTION
  >    TransportUseType ::= TEXTUAL-CONVENTION
  >
  >   I would add a prefix aka
  >
  >    RserENRPServerIdentifierType ::= TEXTUAL-CONVENTION
  >    RserOperationScopeType ::= TEXTUAL-CONVENTION
  >    RserPoolHandleType ::= TEXTUAL-CONVENTION
  >    RserDescriptionType ::= TEXTUAL-CONVENTION
  >    RserPoolElementIdentifierType ::= TEXTUAL-CONVENTION
  >    RserPolicyIDType ::= TEXTUAL-CONVENTION
  >    RserPolicyLoadType ::= TEXTUAL-CONVENTION
  >    RserPolicyWeightType ::= TEXTUAL-CONVENTION
  >    RserTransportUseType ::= TEXTUAL-CONVENTION

  Fixed. Prefix is now "RSerPool" for everything.


  >   Or maybe Enrp is the prefix as late object naming seems toindicate.
  >   But then I would also make the MIB modulename ENRP-MIB and
  >   then change
  >           rserpoolMIB MODULE-IDENTITY
  >   into
  >           enrpMIB MODULE-IDENTITY
  >
  >   I am also not sure I would let the TC names have a suffix of "Type".
  >   But that may be personal taste.
  >
  > - further down in the MIB module we see another prefix
  >
  >        poolElementEntry OBJECT-TYPE
  >
  >   I would ALWAYS use the same prefix throught the MIB module!
  >
  > - I blieve that for this one
  >    enrpServerPort OBJECT-TYPE
  >    SYNTAX     Unsigned32 (1..65535)
  >   one should use the InetPort TC from RFC4001
  >
  >   this is true for other PORT definitions as well

  Fixed.


  > - I see various writable objects that do not describe what their expected
  >   persistency behaviour is

  Fixed.


  > -    enrpServerASAPAnnounceAddr OBJECT-TYPE
  >    SYNTAX     InetAddress
  >    MAX-ACCESS read-only
  >    STATUS     current
  >    DESCRIPTION
  >    "The destination multicast IP address ASAP multicast
  >    announce messages are sent to."
  >
  >    ::= { enrpServerEntry 9 }
  >
  >   RFC4001 (that defines the InetAddress TC) prescribes that the
  >   DESCRIPTION clause must indicate which object of SYNTAX
  >   InetAddressType controls the format of this object.

  Fixed.


  > - When I see somehting like
  >   enrpServerENRPL3Proto OBJECT-TYPE
  >   SYNTAX     InetAddressType
  >   MAX-ACCESS read-only
  >   STATUS     current
  >   DESCRIPTION
  >   "The network-layer protocol (IPv4 or IPv6) of an IP address of
  >   an ENRP transport endpoint."
  >
  >   ::= { enrpServerENRPAddrTableEntry 2 }
  >
  >   Then I wonder if it would not be better to SUBTYPE the TC
  >   So something like
  >
  >        SYNTAX     InetAddressType{ipv4(1), ipv6(2)}

  Fixed.

  smilint prints "warning: `InetAddressType' should not be subtyped" on this, 
  but this seems to be a false positive. The SCTP-MIB also uses sub-typed 
  InetAddressType.


  >   The associated
  >    enrpServerPeerL3Addr OBJECT-TYPE
  >    SYNTAX     InetAddress
  >
  >   would then become
  >    enrpServerPeerL3Addr OBJECT-TYPE
  >    SYNTAX     InetAddress (SIZE(4|16))
  >
  >   all this assuming that you explicitly want to only support IPv4 and IPv6
  > and
  >   not DNS and not Scoped IPv6 addresses

  Fixed.


  > - According to RFC4181 this one
  >          rserpoolMIBConformance OBJECT IDENTIFIER ::= { rserpoolMIB 4 }
  >    should change to
  >             rserpoolMIBConformance OBJECT IDENTIFIER ::= { rserpoolMIB 2 }

  1 is used for the ENRP servers branch, 2 is used for PE branch, 3 for PU 
  branch. The next available number is 4.


  >    I do not see a reason why the recommended MIb structure in RFC4181 would
  >    not be followed.
  >
  > - This
  >    DESCRIPTION "The group of ENRP servers"
  >
  >    ::= { rserpoolMIBGroups 1 }
  >
  >   is of course not a good DESCRITPION clause.
  >   It is I think "The group of objects to manage/monitor ENRP servers."
  >   or some such.
  >
  >   Same for otehr groups

  Fixed.


  > -
  > Abstract
  >
  >    RSerPool [RFC5351] is a framework to provide reliable server pooling.
  >    This document defines a SMIv2 compliant Management Information Base
  >    (MIB) providing access to managed objects in an RSerPool
  >    implementation.
  >
  > Normally, citations are not supposed to be in the abstract. But that is a
  > NIB,
  > The document however, does not define a MIB, but a MIB module.
  > I know some people think this is a nit too. The introduction has irt right.

  Okay.


  > Seuritty considerations is weak. It does not state anything about the
  > possible secuirty issues/concerns when peole get read and/or write
  > access to the various objects.
  >
  > s /IPSec/IPsec/ as well
  >
  > I think that RFC4001 is missing from the NORMATIVE references list

  Okay.


  > The REVISION clause should probably contain something like
  >
  >    REVISION "200901221012Z" -- January 22, 2009
  >    DESCRIPTION
  >    "This version of the MIB module published as RFC xxxx."

  I will update this.


  Best regards
  -- 
  =======================================================================
   Dr. Thomas Dreibholz

   University of Duisburg-Essen,                   Room ES210
   Inst. for Experimental Mathematics              Ellernstraße 29
   Computer Networking Technology Group            D-45326 Essen/Germany
  -----------------------------------------------------------------------
   E-Mail:     dreibh@iem.uni-due.de
   Homepage:   http://www.iem.uni-due.de/~dreibh
  =======================================================================
_______________________________________________
rserpool mailing list
rserpool@ietf.org
https://www.ietf.org/mailman/listinfo/rserpool