Subject: [Sip] Comments to draft-ietf-sip-mib-05.txt

From: "Romascanu, Dan (Dan)" <[email-masked]>
Date: Sun, 27 Apr 2003 13:45:14 +0300

Here are my comments to the latest SIP MIB I-D. I have divided them into three categories - design and architecture, MIB and SMI, and editorials and formatting. Concerning the latest category, I did not go yet into a detailed search for all editorial nits, as the current version has more important issue to solve and will need more iterations anyway.

Regards,

Dan

-------------------------------------------------------------

A - Design and Architecture Issues

A1 - Although the declared goal of this version was to align the MIB with the evolution of SIP from RFC 2543 to RFC 3261, this evolution was not completed. A large part of the MIB is still referring to RFC 2543. Moreover, in the absence of an explicit note in the text, the implementer of the current version of this MIB risks to work with a hybrid that may cause problems.
A2 - One of the issues future implementers of this MIB will deal with is the issue of extensibility. How new methods that may be added to SIP in the future will be accommodated. The authors chose the path to support RFC 3261 and the INFO method defined in RFC 2976, as a basic common package, and defined an extensibility mechanism for the rest. This can work - with some problems defined later - but there is a need to explain this and maybe provide some examples in the introductory sessions.
A3 - Section 4, page 4, last paragraph. As far as I understand the position this document takes is that' if a device has more than one SIP entity, or multiple instances of the same entity, it MUST implement multiple SIP modules' - so MUST and not SHOULD.
A4 - Section 4.2, page 4 - need to define the way sipServiceOperStatus maps into applOperStatus
A5 - sipFtrSupportedtable - I am questioning if these table design is correct. Are the features similar (identically supported) for a multiple entity (like a proxy and a registrar) which would generate just one row in this table
A6 - sipExtMethodSupportedTable - It is not clear why this table is not the common key for all ExtMethod tables in the MIB. sipExtmethodSupportedIndex could be index for sipCommonCfgTimerExtMethodTable, sipCommonCfgRetryExtnMethodTable, sipStatsExtMethodTable, sipCommonStatsRetryExtMethodTable. Beyond providing a common index for extended methods to be used by applications, this would avoid defining again name objects in each of these tables
A7 - it is not clear why sipCommonStatusCodeNotifyTable is defined as a separate table, and not part of sipCommonStatusCideTable? Was the intention to say that if the table (which is optional) is not implemented, there are no notifications at all? If this is the case, this should be explicitly described
A8 - if the result of setting sipCommonStatusCodeNotifEmitMode to 'normal' is one notification for each SIP response message, is not it more safe to define the DEFVAL for this object something else than 'normal'?
A9 - How does an application receiving a sipStatusCodeThreshExceededNotif know that the notification applies for codeIns or for codeOuts? Maybe it's better to define two distinct notifications
A10 - Implementing this MIB has strong security implications. This MIB has a large number of read-write objects, including the servers, presents security mode objects, and transaction tables with client information. All this needs to be detailed in the Security Considerations section, including the explicit lists of all read-write and security sensitive objects (even if not read-write), and the specific security risks that derive from having this objects exposed through a MIB interface

B - MIB and SMI Issues

B1 - There is no reference in the document to the persistence properties of the defined objects. It is RECOMMENDED that the DESCRIPTION clauses specify what happens to the value after an agent reboot. Among the possibilities are that the value remains unchanged, that it reverts to a well-defined default value, or that the result is implementation-dependent
B2 - sipServiceOperStatus - it would be better to define unknown as the first value in the enumeration
B3 - same - a REFERENCE clause for the operStatus values would help
B4 - sipExtMethodName - need to define rules for the method names - e.g. string 'REFER' for the REFER method, etc.
B5 - sipCfgRetryExtMethodStatus - what does the last phrase in the DESCRIPTION clause mean ('If that method already exists for a particular applIndex, the row create operation will fail'). How are values in a row changed? 
B6 - The behavior of the counters in the tables sipStatsExtMethodTable and sipCommonStatusCodeTable are inconsistent. For the later, if the RowStatus object is set to 'notInService' counters MAY be reset, while for the former they interrupt counts and re-start from the same place. Ambiguous behavior is strongly discouraged, and in any case a counter discontinuity object must be added to mark the timestamp of the last reset of the counters in the row
B7 - from the semantics of sipCommonStatusCodeNotifEmitMode, one derives that sipCommonStatusCodeNotifSend needs to be true at the time this object is set. Does this mean that a typical sequence of row creation in this table involves needs to be a two step process?
B8 - There is no reason to define sipTransActivityInfo as a concatenation of two objects, each with a different semantics. I suggest braking this object into two objects with direction and request/response semantics
B9 - If only a subset of the values of a TC is supported, like in the case of sipUACfgSipServerAddrStatus, this needs to be reflected in the compliance clauses. See Section 4.8 of http://www.ietf.org/internet-drafts/draft-ietf-ops-mib-review-guidelines-01.txt for an example
B10 - why the requirement to provide a varbind for sipUACfgSipServerAddr object in the same set operation that sets a sipUACfgSipServerAddrStatus to 'createAndGo'? Cannot the object have been set previously? 
B11 - sipServerHostAddress - as a subset of InetAddress TC is used, this needs to be reflected in the compliance clauses. What is the reason to limit the size to 254, while the InetAddress TC in RFC 3291 allows for a size of 255?
B12 - why is sipContactPreference coded as a string, and not as a sized integer? I actually do not see preference in section 6.13 of RFC 2543 as indicated by the REFERENCE clause, but from the DESCRIPTION it looks like it could be coded as number 1 to 99.

C - Editorial and formatting issues

C1 - Proxy Servers, Redirect Servers, etc. are capitalized in an inconsistent manner, starting with the very visible Abstract section
C2 - The key words rule - though mentioned in the Introduction section is not followed in much of the document. I strongly suggest that the authors make a search through the document for all key words, and use capitals wherever it applies, including the DESCRIPTION clauses of the MIB objects
C3 - RFC 2119 should be in the normative references category
C4 - Section 4 - Structure of the MIB should list the MIB groups, and not the conformance groups. I strongly recommend to add a short explanation for each one of the MIB groups functionality
C5 - SIP is inconsistently spelled in the document. While in the majority of places it is "SIP", in a few you may find "sip" (e.g. Section 4, page 4, last paragraph, where both variants may be found in the same phrase)
C6 - Section 4.2 should rather be named 'Relationship to the Network Services MIB'.
C7 - Section 4.2, page 4 - applVersion - why the if here? Can something else than SIP version be used? I think not
C8 - Replace in the CONTACT-INFO clauses 'SIP MIB email' by SIP WG e-mail
C9 - Insert notes to the RFC editor after the DESCRIPTION clauses of the MIB to remind that RFC number and OID assignment need to be provided, and the notes removed prior to publication
C10 - page 11 - It would help if a short commented explanation would be added for each MIB group like sipCommionCfgBase, sipCommonCfgTimer, etc 
C11 - DESCRIPTION clause of sipOrganization - typo it is probably 'string' and not 'sting'
C12 - DESCRIPTION clause of sipPortStatus - I am not sure what is the intent of the word 'remotely'. I suggest to take it out from here and other similar RowStatus objects definition
C13 - same - It should be 'The rows associated...' (plural) as there might be one row for each applIndex value
C14 - RFC 2396 and RFC 2368 mentioned in the DESCRIPTION clause of sipUriSupportedTable need to be added to the reference clauses
C15 - sipCfgTimerC - delete the last phrase in the DESCRIPTION clause - this is redundant as it can be deduced from the syntax
C16 - add more detailed DESCRIPTION clauses for sipCfgTimerT2, sipCfgTimerT4 - consistent with the rest of the DESCRIPTION clauses in the same table
C17 - DESCRIPTION clause for sipMethodStatsTable - delete the word 'per'. This table does not include per method statistics, but method statistics per entity
C18 - provide more details in the DESCRIPTION clauses of sipStatsRedirCalssIns and sipStatsRedirClassOuts, consistent with other DESCRIPTION clauses in the same table
C19 - append at the end of the DESCRIPTION clause of sipCurrentTransactions '... corresponding to the entity identified by applIndex.'
C20 - delete the last phrase in the DESCRIPTION clause of sipStatsRetryExtMethodName
C21 - DESCRIPTION of sipNotifApplIndex - explain that this object (which is a duplication of applIndex) is created in order to allow an accessible-to-notify object included in the notifications varbind
C22 - change names of sipUACfgSipServerAddrIndex to sipUACfgSipServerIndex, and sipUACfgSipServerAddrStatus to sipUACfgSipServerStatus
C23 - Section 5.4 seems to have formatting problems related tot he number of allowed characters in a row
C24 - DESCRIPTION clause of sipProxyAuthMethod - more than one method may be used, change the clause to reflect this
C25 - sipRegMaxContactExpiryDate - calling this object ...Date is mis-leading. I suggest to rename it to ...Duration or something else.
C26 - The Normative References section is inconsistent, and does not comply with the format recommended in http://www.ops.ietf.org/mib-boilerplate.html
C27 - RFC 3291 (INET-MIB) needs to be added to the Normative References list