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

I understand David's line of thought. Separation between data model and
protocol is important, but let us not forget reality which is that by
now SNMP is the only protocol used with SMIv2.  I believe that there is
a thin line that we need to walk between assuming that everybody
understands well the principle of separation and helping implementers of
MIB modules with as much information as possible in their
implementations. As a reviewer and author I believe that there is no
great sin in making in some cases a recommendation about what error is
being returned while mentioning clearly that this recommendation applies
for SNMP. 

Dan

> -----Original Message-----
> From: Wijnen, Bert (Bert) [mailto:bwijnen@lucent.com] 
> Sent: Tuesday, August 01, 2006 1:00 AM
> To: David Harrington
> Cc: 'MIB Doctors (E-mail)'
> Subject: RE: draft-ietf-sip-mib-11.txt [ was: [MIB-DOCTORS] 
> FW: Agenda andPackage 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
> 

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