RE: draft-ietf-sip-mib-11.txt [ was: [MIB-DOCTORS] FW: Agenda and Package for August 3, 2006 Telechat PRELIMINARY]

"Jean-Francois Mule" <jf.mule@cablelabs.com> Thu, 03 August 2006 14:34 UTC

Received: from [127.0.0.1] (helo=stiedprmman1.va.neustar.com) by megatron.ietf.org with esmtp (Exim 4.43) id 1G8eH7-0005p4-9C; Thu, 03 Aug 2006 10:34:13 -0400
Received: from [10.91.34.44] (helo=ietf-mx.ietf.org) by megatron.ietf.org with esmtp (Exim 4.43) id 1G8eEP-0004L5-3k for mib-doctors@ietf.org; Thu, 03 Aug 2006 10:31:25 -0400
Received: from ondar.cablelabs.com ([192.160.73.61]) by ietf-mx.ietf.org with esmtp (Exim 4.43) id 1G8eEO-0001Sw-7T for mib-doctors@ietf.org; Thu, 03 Aug 2006 10:31:25 -0400
Received: from srvxchg.cablelabs.com (srvxchg.cablelabs.com [10.5.0.20]) by ondar.cablelabs.com (8.13.7/8.13.7) with ESMTP id k73EUt26019722; Thu, 3 Aug 2006 08:30:55 -0600 (MDT)
X-MimeOLE: Produced By Microsoft Exchange V6.0.6249.0
Content-class: urn:content-classes:message
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: quoted-printable
Subject: RE: draft-ietf-sip-mib-11.txt [ was: [MIB-DOCTORS] FW: Agenda and Package for August 3, 2006 Telechat PRELIMINARY]
Date: Thu, 03 Aug 2006 08:30:55 -0600
Message-ID: <CD6CE349CFD30D40BF5E13B3E0D8480401A4085C@srvxchg.cablelabs.com>
X-MS-Has-Attach:
X-MS-TNEF-Correlator:
Thread-Topic: draft-ietf-sip-mib-11.txt [ was: [MIB-DOCTORS] FW: Agenda and Package for August 3, 2006 Telechat PRELIMINARY]
Thread-Index: Aca03UyeyPbyIWShSyekxmI/ED4vZwCK+QTQ
From: Jean-Francois Mule <jf.mule@cablelabs.com>
To: "Wijnen, Bert (Bert)" <bwijnen@lucent.com>, "Romascanu, Dan (Dan)" <dromasca@avaya.com>, klingle@cisco.com, drwalker@rogers.com, jmaeng@austin.rr.com
X-Approved: ondar
X-Spam-Score: 0.0 (/)
X-Scan-Signature: 9a0f005dcc96c32c6d659bdf82d519da
X-Mailman-Approved-At: Thu, 03 Aug 2006 10:34:11 -0400
Cc: "Cullen Jennings (E-mail)" <fluffy@cisco.com>, mib-doctors@ietf.org, "Dean Willis (E-mail)" <dean.willis@softarmor.com>
X-BeenThere: mib-doctors@ietf.org
X-Mailman-Version: 2.1.5
Precedence: list
List-Id: MIB Doctors list <mib-doctors.ietf.org>
List-Unsubscribe: <https://www1.ietf.org/mailman/listinfo/mib-doctors>, <mailto:mib-doctors-request@ietf.org?subject=unsubscribe>
List-Archive: <http://www1.ietf.org/pipermail/mib-doctors>
List-Post: <mailto:mib-doctors@ietf.org>
List-Help: <mailto:mib-doctors-request@ietf.org?subject=help>
List-Subscribe: <https://www1.ietf.org/mailman/listinfo/mib-doctors>, <mailto:mib-doctors-request@ietf.org?subject=subscribe>
Errors-To: mib-doctors-bounces@ietf.org

Hi Bert and all,
 
   First, thank you for the detailed review and comments. After a quick
scan, there are some technical comments that need to be addressed in a
draft update.

   I'm traveling until 8/14 but Kevin and I will look at these in more
details and provide a detailed response shortly. We will issue a draft
12 addressing the comments.

Jean-Francois.

> -----Original Message-----
> From: Wijnen, Bert (Bert) [mailto:bwijnen@lucent.com]
> Sent: Monday, July 31, 2006 2:10 PM
> To: Romascanu, Dan (Dan); klingle@cisco.com; Jean-Francois Mule;
> drwalker@rogers.com; jmaeng@austin.rr.com
> Cc: mib-doctors@ietf.org; Cullen Jennings (E-mail); Dean Willis (E-
> mail)
> Subject: draft-ietf-sip-mib-11.txt [ was: [MIB-DOCTORS] FW: Agenda and
> Package for August 3, 2006 Telechat PRELIMINARY]
> 
> Dan, w.r.t
> > 2.1 WG Submissions
> > 2.1.1 New Item
> >   o draft-ietf-sip-mib-11.txt
> >     Management Information Base for the Session Initiation Protocol
> (SIP)
> >     (Proposed Standard) - 1 of 3
> >     Note: PROTO Shepherd is Dean Willis
> >     Token: Cullen Jennings
> 
> In general, I do have some concerns/issues that I think BEST be fixed.
> I have numerous editorial suggestions. Once could claim that such can
> be ignored at this stage in the process, and I would have to agrre.
> At the other hand, specifically the naming consistency bothers/worries
> me a lot, even though it is not a fatal flaw. I understand that it
> will be quite an effort to adopt to a consistent naming scheme, yet I
> personally think it would be very worthwhile. Not only for helping to
> avoid future name clashes, but also to make the descriptors more
> self-explanatory as to which MIB module and/or which table they
> belong to.
> 
> Sorry for not having done a review earlier (like at IETF last call or
> so).
> 
> Bert
> --- SIP-TC module
> 
> None of this is blocking or a fatal flaw. But pls DO consider my
> well-intended comments to try and help make this MIB module better
> and easier to understand by MIB implementers and NM application
> developers.
> 
> When I see:
>       SipTransportProtocol ::= TEXTUAL-CONVENTION
>               STATUS current
>               DESCRIPTION
>                    "This convention is a bit map.  Each bit represents
> a
>                     transport protocol.  If a bit has value 1, then
> that
>                     selected transport protocol is in some way
> dependent
>                     on the context of the object using this
convention.
>                     If a bit has value 0, then that transport protocol
>                     is not selected.  Combinations of bits can be
>                     set when multiple transport protocols are
selected.
> 
>                     bit 0 : a protocol other than those defined here
>                     bit 1 : User Datagram Protocol
>                     bit 2 : Transmission Control Protocol
>                     bit 3 : Stream Control Transmission Protocol
>                     bit 4 : Transport Layer Security Protocol over TCP
>                     bit 5 : Transport Layer Security Protocol over
> SCTP"
>               SYNTAX     BITS {
>                                other(0),  -- none of the following
>                                udp(1),
> 
>                                tcp(2),
>                                sctp(3),   -- RFC4168
>                                tlsTcp(4),
>                                tlsSctp(5) -- RFC 4168
>               }
>    --         REFERENCE "RFC 3261, Section 18"
>    --         REFERENCE "RFC 4168"
> 
> Then I wonder why the REFERENCE is done with ASN.1 comments instead of
> using a real REFERENCE clause. The following would achive that:
>       SipTransportProtocol ::= TEXTUAL-CONVENTION
>               STATUS current
>               DESCRIPTION
>                    "This convention is a bit map.  Each bit represents
> a
>                     transport protocol.  If a bit has value 1, then
> that
>                     selected transport protocol is in some way
> dependent
>                     on the context of the object using this
convention.
>                     If a bit has value 0, then that transport protocol
>                     is not selected.  Combinations of bits can be
>                     set when multiple transport protocols are
selected.
> 
>                     bit 0 : a protocol other than those defined here
>                     bit 1 : User Datagram Protocol
>                     bit 2 : Transmission Control Protocol
>                     bit 3 : Stream Control Transmission Protocol
>                     bit 4 : Transport Layer Security Protocol over TCP
>                     bit 5 : Transport Layer Security Protocol over
> SCTP"
>               REFERENCE "RFC 3261, Section 18 and RFC 4168""
>               SYNTAX     BITS {
>                                other(0),  -- none of the following
>                                udp(1),
> 
>                                tcp(2),
>                                sctp(3),   -- RFC4168
>                                tlsTcp(4),
>                                tlsSctp(5) -- RFC 4168
>               }
> 
> Similar comment for: SipOptionTagHeaders
> 
> Would be good to have a REFERENCE for the SipEntityRole and
> SipMethodName as well.
> 
> It is interesting to see that the DESCRIPTION clause of the
> MODULE-IDENTITY in the SIP-COMMON-MIB has all the details
> about the different roles of SIP entities, while the SIP-TC
> MIB module, not the specific SipEntityRole TC do NOT have
> such a nice/good description. I think that all that text
> would be much better provided in the SipEntityRole TC.
> 
> Specifically for SipMethodName I seem unable to find what
> values exactly this SYNTAX (i.e. ASCII? UTF-8 ?? any octet
> value?) can take and why its range is (1-128).
> The 128 max length causes a warning from SMI SYNTAX checkers
> because it can cause OIDs with ore than 128 subIDs when used
> as an index (which is done in for example the SIP-COMMON-MIB).
> I see the RFC-Editor notes suggesting to change it to max 119
> octets, but I see no base as to why that would be a reasonable
> limit. In the SIP-COMMON-MIB I see sipMethodName for the ??same??
> methodName, and the use of SnmpAdminString for that occurrence
> seems to indicate it IS UTF-8 and max 32 octets !!??
> 
> 
> ----- SIP-COMMON-MIB
> 
> First some comments that are flawed (item 3 below, the badValue
> error code is (in my view) even fatally flawed in that it breaks
> the SNMPv2 protocol operations of RFC3416).In any event, I think
> these should be SERIOUSLY considerede for a fix/change
> 
> 1. SYNTAX checking:
> 
>   C:\bwijnen\smicng\work>smicstrict
>   C:\bwijnen\smicng\work>smicng SIP-COMMON-MIB.inc
>   W: f(rfc2788.mi2), (562,3) OBJECT-GROUP "applGroup" is not
>      used in a MODULE-COMPLIANCE in current module
>   W: f(SIP-COMMON-MIB), (1026,7) Row "sipMethodStatsEntry" has
>      indexing that may create variables with more than 128 sub-ids
>   W: f(SIP-COMMON-MIB), (1098,7) Row "sipStatusCodesEntry" has
>      indexing that may create variables with more than 128 sub-ids
>   W: f(SIP-COMMON-MIB), (1206,7) Row "sipStatusCodeNotifEntry" has
>      indexing that may create variables with more than 128 sub-ids
>   W: f(SIP-COMMON-MIB), (1405,7) Row "sipCommonStatsRetryEntry" has
>      indexing that may create variables with more than 128 sub-ids
> 
>   *** 0 errors and 5 warnings in parsing
> 
>   The first warning is from another MIB-module, so we can ignore it.
> 
>   W.r.t. the last 4 warnings, I think it would be good (as recommended
>   by RFC4381 sect 4.6.6.) to add some words in teh DESCRIPTION clauses
>   about OID length limitations.
> 
> 2.sipServiceNotifEnable is a writable object, but I see not details
>   about the expected persistenmce behaviour! I strongly recommend to
>   add that.
> 
> 3.sipServiceNotifEnable DESCRIPTION clause suggests to return a
> badValue
>   error. a badValue is NOT a value to be returned by SNMPv2c/v3. It is
>   only there for backward SNMPv1 compatibility. So suggesting that as
>   an error value in an SMIv2 MIB module is not correct!
> 
>   Not sure I completely understand DESCRIPTION. It seems like a SIP
> entity can
>   choose to support notifications or not. In the former case it must
> support
>   all of them, in the latter case it would support none at all. So it
> seems
>   one would not see an entity that supports only 2 uut of 3 or some
> other
>   mix?
> 
>   If my interpretation/understanding is correct (in any event, it
> would be
>   good to make it clear in the DESCRIPTION clause) I think the correct
>   approach would be that the object is allowed to be supported in
> read-only
>   mode in case the entity does NOT support any notifications. This is
> done
>   via a MIN-ACCESS in the MODULE-COMPLIANCE.
>   Another approach (which also allows a mix of supported/non-supported
>   noticifcations, and so is the prefered way in my view) would be to
>   return a error of "inconsistentValue".
>   It cannot return badValue (see Elements of Procedure pages 19/20 of
>   RFC3416). It could return inconsistentValue (step 10 on page 19/20
>   of RFC3416) to achieve what you seem to want.
> 
>   So the simplest fix is to: s/badValue/inconsistentValue/
>   and to clarify the DESCRIPTION clause as to what is exactly intended
>   (no support for ALL notifications, or an option to support only
>    a subset)
> 
> 4.I see no mention of which discontinuity timer applies to the
> counters
>   in sipSummaryStatsTable. Same for sipStatusCodesTable,
>   sipCommonStatsRetryTable and sipOtherStatsTable.
> 
> 5.I see no persistency behaviour defined for created entries in the
>   sipStatusCodesTable
> 
> 
> None of the following is blocking and none of it causes a fatal flaw.
> I think they would be good editorial changes though:
> 
> - I find the use of MAY in the DESCRIPTION clause of the MODULE-
> IDENTITY
>   not correct. I would just use lowercase "may". It is not specifying
>   any compliance requirements here (or so I think/perceive).
> 
> - I personally would rename sipCommonMIBNotifs into
> sipCommonMIBNotifications
>   Most other MIB modules use the longer "Notifications" instead of
> abbreviations
>   so I think it helps in overall MIB-wide naming consistency.
> 
> - Simliarly I would rename sipCommonMIBConform into
> sipCommonMIBConformance
>   as is done in most otehr MIB modules.
> 
> - it seems to me that sipServiceNotifEnable could have a DEFVAL clause
> as
>   follows:
>     DEFVAL { { sipServiceColdStart, sipServiceWarmStart } }
>   At least, that is also what the DESCRIPTION clause tells me. Would
>   be good to specify that in machine redable form, i.e. using a DEFVAL
>   clause.
> 
> - for sipPort, the DESCRIPTION clause should indicate what a value of
> zero
>   means. Probably a value of zero makes no sense here, and so the
>   SYNTAX would probably best be specified as
>          SYNTAX    InetPortNumber (1..65535)
> 
> - In terms of naming consistency, I would rename
>       SipCommonCfgEntry ::=
>           SEQUENCE {
>                    sipProtocolVersion        SnmpAdminString,
>                    sipServiceOperStatus      INTEGER,
>                    sipServiceStartTime       TimeTicks,
>                    sipServiceLastChange      TimeTicks,
>                    sipOrganization           SnmpAdminString,
>                    sipMaxTransactions        Unsigned32,
>                    sipServiceNotifEnable     BITS,
>                    sipEntityType             SipEntityRole
>           }
>   into:
>       SipCommonCfgEntry ::=
>           SEQUENCE {
>                    sipCommonCfgProtocolVersion        SnmpAdminString,
>                    sipCommonCfgServiceOperStatus      INTEGER,
>                    sipCommonCfgServiceStartTime       TimeTicks,
>                    sipCommonCfgServiceLastChange      TimeTicks,
>                    sipCommonCfgOrganization           SnmpAdminString,
>                    sipCommonCfgMaxTransactions        Unsigned32,
>                    sipCommonCfgServiceNotifEnable     BITS,
>                    sipCommonCfgEntityType             SipEntityRole
>           }
>    and I would also rename:
>       SipPortEntry ::=
>           SEQUENCE {
>                    sipPort                 InetPortNumber,
>                    sipTransportRcv         SipTransportProtocol
>           }
>    into something like:
>       SipPortEntry ::=
>           SEQUENCE {
>                    sipPort                 InetPortNumber,
>                    sipPortTransportRcv     SipTransportProtocol
>           }
>    maybe that while table should even have sipCommon as a prefix.
> 
>    That is, I would have all SIP-COMMON-MIB descriptors start with
> sipCommon
>    It is easy to ensure that there are no naming conflicts today, but
> if
>    changes/additions/extensions are made in the future, it becomes all
>    so much easier if one has started with a very clear naming scheme;
>    specifically with using a consistent prefix for descriptor-names
>    within the same MIB module.
> 
>    So this also applies to several other tables. For example,
>    I would rename sipOptionTagTable to sipCommonOptionTagTable and
>    have all objects there also start with sipCommonOption prefix.
> 
> - NIT: sipOptionTagEntry
>   in description clause: s/at reboot/over a reboot or restart/
> 
> - consider sipOptionTagEntry
>     SHOULD be non-volatile
>   So an NM application has to do a trial and error to figure ity out?
>   Why could this not be made a: MUST be non-volatile
>   But then checking further, I see that the whole table is read-only,
>   so no need to talk about persistency of the entries.
> 
> - sipOptionTag
>   I would recommend to add a REFERENCE clause to point to sect 27.1
>   of RFC3261. That helps understand what the actual values can be.
>   Because acording to the SYNTAX, it could contain chinese, but
> according
>   to sect 27.1 of RFC3261 it cannot.
> 
>   I kind of like that you use SnmpAdminString (i.e. UTF-8 based), so
> that
>   you recognize that at some time there might be non US ASCII tags?
>   But the SYNTAX does not represent the actual allowed content of the
>   TAG. If that content is fixed to ALPHA/DIGIT from US ASCII set, then
>   maybe it would be better to define your own TC as ipposed to using
>   a TC that is intened for internationalized human readable strings.
> 
>   It might also be helfull to add some text that explains what to do
>   if the length of the sipOptionTag exceed the max length of
>   SnmpAdminString which seems allowed (MAY text in 27.1 of RFC3216).
> 
> - sipMethodSupportedTable
>   Seems to have only read-only objects/columns, so I do not see
>   a need or the usefullness to talk about persistyency (non-volatile)
>   in the DESCRIPTION claise of sipMethodSupportedEntry
> 
> - sipMethodSupportedTable
>   Maybe it is just me. But I am confused by the use of MAY (2 times).
>   It gives me the impression that you do not HAVE TO include anything.
>   But I would think that one MUST include ALL METHODS that the
>   represented SIP instance supports, no?
> 
> - sipMethodName
>   I think I have already spoken about this better be named
>   sipCommonMethodSupportedMethodName.
>   But the SYNTAX is SnmpAdminString and I wonder why it would not
>   be SipMethodName as from your own SIP-TC mib module.
> 
> - I would rename
>       SipCommonCfgTimerEntry ::=
>           SEQUENCE {
>                    sipCfgTimerA               Unsigned32,
>                    sipCfgTimerB               Unsigned32,
>                    sipCfgTimerC               Unsigned32,
>                    sipCfgTimerD               Unsigned32,
>                    sipCfgTimerE               Unsigned32,
>                    sipCfgTimerF               Unsigned32,
>                    sipCfgTimerG               Unsigned32,
>                    sipCfgTimerH               Unsigned32,
>                    sipCfgTimerI               Unsigned32,
>                    sipCfgTimerJ               Unsigned32,
>                    sipCfgTimerK               Unsigned32,
>                    sipCfgTimerT1              Unsigned32,
>                    sipCfgTimerT2              Unsigned32,
>                    sipCfgTimerT4              Unsigned32
>           }
>    into (for naming consistency):
>       SipCommonCfgTimerEntry ::=
>           SEQUENCE {
>                    sipCommonCfgTimerA               Unsigned32,
>                    sipCommonCfgTimerB               Unsigned32,
>                    sipCommonCfgTimerC               Unsigned32,
>                    sipCommonCfgTimerD               Unsigned32,
>                    sipCommonCfgTimerE               Unsigned32,
>                    sipCommonCfgTimerF               Unsigned32,
>                    sipCommonCfgTimerG               Unsigned32,
>                    sipCommonCfgTimerH               Unsigned32,
>                    sipCommonCfgTimerI               Unsigned32,
>                    sipCommonCfgTimerJ               Unsigned32,
>                    sipCommonCfgTimerK               Unsigned32,
>                    sipCommonCfgTimerT1              Unsigned32,
>                    sipCommonCfgTimerT2              Unsigned32,
>                    sipCommonCfgTimerT4              Unsigned32
>           }
> 
>   Further, I cannot say that I find the naming of Timers with A, B,
> C,D etc
>   verty intuitive or user friendly.
> 
> - In the sipCommonCfgTimerTable I find it strange to see DEFVALs for
>   read-only objects.   Specifically strange if I see for example:
>       sipCfgTimerD OBJECT-TYPE
>           SYNTAX      Unsigned32 (0..300000)
>           UNITS       "milliseconds"
>           MAX-ACCESS  read-only
>           STATUS      current
>           DESCRIPTION
>                "This object reflects the amount of time that the
> server
>                 transaction can remain in the 'Completed' state when
>                 unreliable transports are used. The default value MUST
>                 be greater than 32000 for UDP transport and its value
> 
>                 MUST be 0 for TCP/SCTP transport."
>           REFERENCE
>                 "RFC 3261, Section 17.1.1.2"
>               DEFVAL { 32000 }
>    where the DESCRIPTION clause suggests that the value might in fact
>    be EITHER >32000 or 0. So how can there be a DEFVAL that covers
> both?
> 
> - I see:
>       sipStatusCodeMethod OBJECT-TYPE
>           SYNTAX      SipMethodName
>           MAX-ACCESS  not-accessible
>           STATUS      current
>           DESCRIPTION
>                "This object uniquely identifies a conceptual row
>                 in the table and reflects an assigned number used
>                 to identifier a specific SIP method."
>    s/to identifier/to identify/ !!??
> 
> -    sipStatusCodeRowStatus OBJECT-TYPE
>          SYNTAX      RowStatus
>          MAX-ACCESS  read-create
>          STATUS      current
>          DESCRIPTION
>               "The row augmentation in sipStatusCodeNotifTable
>                will be governed by the value of this RowStatus.
> 
>                This object is REQUIRED to create or delete rows
>                by a manager.
> 
>                The values 'createAndGo' and 'destroy' are the
>                only valid values allowed for this object.
>                If a row exists, it will reflect a status of
>                'active' when queried."
>          ::= { sipStatusCodesEntry 5 }
> 
>    You are missing a statement that explains if writable columns
>    (they would be columns in the augmenting sipStatusCodeNotifTable)
>    can be changed if the value of this object is 'active'. I would
>    think the answer is YES< they can be modified, and RFC2579
>    in RowStatus TC DESCRIPTION clause mandates that you specify
>    this in the DESCRIPTION clause of an object that uses the
>    RowStatus TC as its SYNTAX.
> 
>    The 2nd para in the DESCRIPTION clause seems redundant. But is
>    acceptable. I would remove it though.
> 
>    The last para seems material that needs to go into the
>    MODULE-COMPLIANCE. you are basically stating that a createAndWait
>    and notInService are not required. I find it strange that you
>    would want to PROHIBIT support for those. If the latter, than
>    you would normally do that by a SYNTAX of:
>       sipStatusCodeRowStatus OBJECT-TYPE
>           SYNTAX      RowStatus {
>                         active(1),
>                         createAndGo(4),
>                         destroy(6)
>                       }
>    If the former (I think I would prefer the former), in which case
>    you leave the SYNTAX as is, but in the MODULE-COMPLIANCE, you add
>    an OBJECT clause, as follows.
>    I would replace (i.e. add):
>       sipCommonCompliance MODULE-COMPLIANCE
> 
>           STATUS     current
>           DESCRIPTION
>                "The compliance statement for SIP entities."
> 
>           MODULE -- this module
>                MANDATORY-GROUPS { sipCommonConfigGroup,
>                                   sipCommonStatsGroup }
>    with:
>       sipCommonCompliance MODULE-COMPLIANCE
> 
>           STATUS     current
>           DESCRIPTION
>                "The compliance statement for SIP entities."
> 
>           MODULE -- this module
>                MANDATORY-GROUPS { sipCommonConfigGroup,
>                                   sipCommonStatsGroup }
> 
>           OBJECT sipStatusCodeRowStatus
>           SYNTAX RowStatus { active(1) }
>           WRITE-SYNTAX RowStatus { createAndGo(4), destroy(6) }
>           DESCRIPTION
>             "Support for createAndWait and notInService is not
> required."
> 
> - I would think that the DEFVAL for sipStatusCodeNotifEmitMode would
>   better be oneShot so as to minimize network load. In any event, it
> seems
>   wierd to me that the value would be "triggered" by default (I see
> that
>   the DESCRIPTION clause says that this is also possibly the default
> value).
> 
> 
> - The way the MODULE-COMPLIANCE is defined means that IF you claim
>   compliance, then for all writable objects you DO support them in
>   read-write or read-create (as the case may be). As long as the
>   WG and authors are aware of that and if that is what they want,
>   then fine. I am assuming so, but did want to point it out during
>   my review.
> 
> 
> ------ SIP-UA-MIB
> 
> I see no blocking issues or fatal flwas. However, pls consider:
> 
> - I would rename sipUAMIBConform to sipUAMIBConformance for
> consistency
>   with many otehr MIB modules.
> 
> - I see:
>       sipUACfgServerEntry OBJECT-TYPE
>           SYNTAX     SipUACfgServerEntry
>           MAX-ACCESS not-accessible
>           STATUS     current
>           DESCRIPTION
>                "A row of server configuration.
> 
>                 Each row represents those objects for a particular SIP
>                 user agent present in this system.  applIndex is used
> to
>                 uniquely identify these instances of SIP user agents
> and
>                 correlate them through the common framework of the
>                 NETWORK-SERVICES-MIB (RFC 2788). The same value of
>                 applIndex used in the corresponding SIP-COMMON-MIB is
>                 used here.
>                 The objects in this table entry SHOULD be non-volatile
>                 and their value SHOULD be kept at reboot."
> 
>    But the whole table is read-onlyu, so the last 2 lines seem useless
>    and so not needed to me.
> 
> - I see:
>      sipUACfgServerAddr OBJECT-TYPE
>          SYNTAX     InetAddress
>          MAX-ACCESS read-only
>          STATUS     current
>          DESCRIPTION
>               "This object reflects the address of a SIP server
>                this user agent will use to proxy/redirect calls."
>          REFERENCE "INET-ADDRESS-MIB (RFC 4001)"
>          ::= { sipUACfgServerEntry 3 }
> 
>    RFC4001, requires (MUST language) that every object with syntax
> InetAddress
>    specifies WHICH object of SYNTAX InetAddressType controls the
> format of the
>    InetAddress. It is simple. Add this text:
> 
>          The type of this address is determined by the value of the
>          sipUACfgServerAddrType
>          object.  Note that implementations must limit themselves
> 
> - real NIT.
>   from a user-friendlyness/intuitiveness point of view, I think I
> would
>   rename
>   -  sipUACfgServerAddrType to sipUACfgServerAddressType
>   -  sipUACfgServerAddr     to sipUACfgServerAddress
>   -  sipUACfgServerFunction to sipUACfgServerRole.
> 
> 
> ------- SIP-SERVER-MIB
> 
> maybe in need of a fi:
> 1.sipNumProxyRequireFailures does not specify which (if any)
>   discontinuity timer applies.
>   Same for: sipUserAuthenticationFailures
>   Same for counters in sipRegStatsTable
> 
> Editorial concerns (includes naming concern)
> 
> - I would rename sipServerMIBConform to sipServerMIBConformance for
>   naming consistency with so many other MIB modules
> 
> - sipServerCfgEntry talks about "SHOULD be non-volatile"
>   but the whole table is read-only, so I see no use/need for that
text.
> 
> - In the DESCRIPTION clause of sipServerHostAddr, I see:
>                   sipServerHostAddrType formalizes the type of address
>                   given by this object.  It is the user's
>                   responsibility to maintain consistency between this
>                   object and the type specified by
>                   sipServerHostAddrType."
> 
>   It does explain how the format is controlled (by value of
>   sipServerHostAddrType). But it is unclear who the "user" is who is
> made
>   responsible for consitency here. I think it is an implementation
>   responsibility, since both objects are read-only.
> 
> - I would rename sipProxyCfg to sipServerProcyCfg (and all descriptors
>   with same prefix too).
>   I would rename sipProxyStats to sipServerProxyStats (and all
>   descriptors with same prefix as well).
> 
> - sipProxyCfgEntry talks about "SHOULD be non-volatile" while the
> whole
>   table is read-only. So that text is meaningless/not-needed.
> 
> - I personally would rename sipProxyAuthRealm to
> sipProxyAuthDefaultRealm
>   to make the descriptor better cover the content.
> 
> 
> Bert



_______________________________________________
MIB-DOCTORS mailing list
MIB-DOCTORS@ietf.org
https://www1.ietf.org/mailman/listinfo/mib-doctors