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

Well, we do so in a few of our STD62 MIB modules.

Examples (DESCRIPTION clauses of):
  vacmViewTreeFamilyTable
  usmUserCloneFrom
  usmUserAuthProtocol
  usmUserAuthKeyChange
  usmUserOwnAuthKeyChange
  usmUserPrivProtocol
  usmUserPrivKeyChange
  ... etc
  snmpTargetAddrRowStatus
  snmpTargetParamsSecurityModel
  snmpTargetParamsRowStatus

I guess you could claim/say that the above ARE SNMP specific to begin with.

I know that I have seen it in many places, and I know we have not been
consistent to push back on it.

For example:
   dot1dStpBridgeMaxAge even suggests a badValue error
   dot1dStpBridgeHelloTime ditto
   dot1dStpBridgeForwardDelay ditto
and you were WG co-chair for that. 
Oh well.... to be consistent with a team of reviewers is not easy.

Bert
> -----Original Message-----
> From: David Harrington [mailto:ietfdbh@comcast.net]
> Sent: Monday, July 31, 2006 23:15
> To: 'Wijnen, Bert (Bert)'
> Cc: 'MIB Doctors (E-mail)'
> Subject: RE: draft-ietf-sip-mib-11.txt [ was: [MIB-DOCTORS] FW: Agenda
> and Package for August 3, 2006 Telechat PRELIMINARY]
> 
> 
> Hi,
> 
> I must have missed the memo.  
> I was not aware we were advising MIB module designers to specify error
> codes in MIB descriptions.
> I think I have consistently objected to this when found in a MIB
> module I reviewed.
> I think it is a bad idea for the reasons stated.
> 
> dbh
> 
> > -----Original Message-----
> > From: Wijnen, Bert (Bert) [mailto:bwijnen@lucent.com] 
> > Sent: Monday, July 31, 2006 5:01 PM
> > To: David Harrington
> > Cc: MIB Doctors (E-mail)
> > Subject: RE: draft-ietf-sip-mib-11.txt [ was: [MIB-DOCTORS] 
> > FW: Agenda and Package for August 3, 2006 Telechat PRELIMINARY]
> > 
> > [dropped authors and non mib-doctors]
> > 
> > David, we have been promoting in the past to describe the kind of
> > error that one would return in certain failure cases where it was
> > not so clear as to what error to return.
> > 
> > Maybe in this specific case it is pretty clear, and maybe we could
> > indeed advise to leave it out completely.
> > 
> > Opinions?
> > 
> > In any event, I think we all agree that badValue is NOT correct
> > and not allowed according to our RFC3416 elements of procedure
> > on page 19/20.
> > 
> > Bert
> > 
> > 
> > > -----Original Message-----
> > > From: David Harrington [mailto:ietfdbh@comcast.net]
> > > Sent: Monday, July 31, 2006 22:50
> > > To: 'Wijnen, Bert (Bert)'; 'Romascanu, Dan (Dan)'; 
> > klingle@cisco.com;
> > > 'Jean-Francois Mule (E-mail)'; drwalker@rogers.com; 
> > > jmaeng@austin.rr.com
> > > Cc: 'Cullen Jennings (E-mail)'; mib-doctors@ietf.org; 'Dean Willis
> > > (E-mail)'
> > > Subject: RE: draft-ietf-sip-mib-11.txt [ was: [MIB-DOCTORS] 
> > FW: Agenda
> > > and Package for August 3, 2006 Telechat PRELIMINARY]
> > > 
> > > 
> > > Hi,
> > > 
> > > I especially want to mention the badValue usage. The 
> > Internet-Standard
> > > Management Framework [described in RFC3410] calls for a 
> > separation of
> > > the MIB from the protocol, so a MIB module should not specify the
> > > error codes, since that ties the MIB module to a specific protocol
> > > (and possibly version). Ideally, the MIB will be useable with
> other
> > > protocols, such as netconf, but netconf does not have the same
> error
> > > codes as SNMP.
> > > 
> > > David Harrington
> > > dharrington@huawei.com 
> > > dbharrington@comcast.net
> > > ietfdbh@comcast.net
> > > 
> > > 
> > > > -----Original Message-----
> > > > From: Wijnen, Bert (Bert) [mailto:bwijnen@lucent.com] 
> > > > Sent: Monday, July 31, 2006 4:10 PM
> > > > To: Romascanu, Dan (Dan); klingle@cisco.com; Jean-Francois 
> > > > Mule (E-mail); drwalker@rogers.com; jmaeng@austin.rr.com
> > > > Cc: Cullen Jennings (E-mail); mib-doctors@ietf.org; 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
> > > > 
> > > 
> > 
> 

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