[MIB-DOCTORS] FW: The MIB doctor review comments to the draft-ietf-pwe3-cep-mib-13.txt

["Romascanu, Dan (Dan)" <[e-mail]>]

Richard tried twice to send this message to the MIB Doctors list, but it was not distributed or archived. 
 
Let me see if coming from me it is accepted. 
 
Dan
 


________________________________

	From: young [mailto:[e-mail]] 
	Sent: Monday, April 26, 2010 3:29 PM
	To: [e-mail]; [e-mail]; [e-mail]
	Cc: [e-mail]; [e-mail]; Romascanu, Dan (Dan); [e-mail]; [e-mail]; [e-mail]; [e-mail]; [e-mail]
	Subject: The MIB doctor review comments to the draft-ietf-pwe3-cep-mib-13.txt
	
	
	
	        Hi, All:
	 
	        Since I was told that the [e-mail] has not got the message, I resend the following review comments again.
	 
	        Regards
	        Richard
	 
	    ////  ///////////////////////////////////////
	Hi, authors:
	 
	Here is Richard and I was asked to do the MIB doctor review on the draft-ietf-pwe3-cep-mib-13.txt.
	Sorry for the quite delay of giving review comments.
	 
	Since it is my first review, Bert asked Orly to give the guidance. 
	Although Orly and I finished the Individual review on time, a unexpected failure of my email box
	blocked us to merge our review comments together in time. 
	Sorry always makes Orly very inconvenient.
	I appreciated Orly’s patience and guidance, and she gives a lot of valuable comments. 
	 
	The comments contain two parts. 
	Part 1 is for the content (exception section 7) ; Part2 is for MIB definition (Section 7).
	Notes: We give both the text and line number in the comments.
	If the line number is not matched to yours, please search them by the text.
	 
	The Part 1
	//////////////////////////////////////////////////////////////////////
	1) Idnits check
	Idnits check has the following comments:
	1- boilerplate required by RFC 5378 and the IETF Trust:the draft is using the IETF Trust Provisions' Section 6.b License Notice from
	     12 Sep 2009 rather than the newer Notice from 28 Dec 2009.  (See
	     http://trustee.ietf.org/license-info/ <http://trustee.ietf.org/license-info/> )
	 
	2- nits according to http://www.ietf.org/id-info/1id-guidelines.txt <http://www.ietf.org/id-info/1id-guidelines.txt> :
	  page 1 has 67 lines where only 58 are allowed
	 
	2) abstract is ok
	 
	3) Requirements Language- I would not put it a separate section, it exist already in the introduction section
	 
	4) txt suggestion:
	96       The model for CEP management is a MIB module.  The PW-CEP-STD-MIB
	97       module described in this document works closely with the MIB modules
	98       described in [RFC5601] and the textual conventions defined in
	99       [RFC5542]. In the spirit of the [RFC2863], a CEP connection will be
	100      a pseudowire(PW) connection.  It will not be treated as an interface and will therefore not be represented in the
	101      ifTable
	 
	5) TDM can be used once it is defined, no need to repeat the long wording like in line 124
	 
	6) 140         Conversely, "inbound" is references the traffic the direction
	 
	7) Co-Authors section-
	I suggest to remove it all together and add ack to the people in the ack section
	 
	8) Feature Checklist section I suggest to call it overview
	 
	9)txt suggestion:
	180        -  The MIB module is designed to work with the PW-STD-MIB[RFC5601]
	181           module.
	 I think this is listed in the intro it is more important that these lines will say:
	Fit within the architecture defined by [RFC3985] and [PWMIB].
	 
	10) txt suggestion
	198     6.1.  PW-CEP-STD-MIB Summary
	I would change the title to “ Structure of the MIB 
	 
	11) Suggestion (Not Must do this change)
	Current the sections are organized by:
	MIB Module Description and Usage . . . . . . . . . . . . . . .  5
	     6.1.  PW-CEP-STD-MIB Summary . . . . . . . . . . . . . . . . . .  5
	     6.2.  MIB modules required for IMPORTS . . . . . . . . . . . . .  6
	     6.3.  PW-STD-MIB Modules Usage . . . . . . . . . . . . . . . . .  6
	     6.4.  PW-CEP-STD-MIB Module Usage  . . . . . . . . . . . . . . .  7
	     6.5.  Example of PW-CEP-STD-MIB Usage  . . . . . . . . . . . . .  7
	 
	I suggest (I think this way is more common)
	6.  Structure of the MIB Module  . . . . . . . . . . . . . . . . .  
	------- To replace the PW-CEP-STD-MIB Summary
	   7.  Relationship to Other MIB Modules  . . . . . . . . . . . . . .  
	Besides the relationship, you could also descript that the  MIB Modules Required for IMPORTS here. 
	8. MIB Module Description and Usage
	     8.1.  PW-CEP-STD-MIB Module Usage  . . . . . . . . . . . . . . .  7
	     8.1.  Example of PW-CEP-STD-MIB Usage  . . . . . . . . . . . . .  7
	 
	12) txt suggestion
	221        -  The CEP Performance 1 day Table (pwCepPerf1DayTable) contains historical intervals accumulated per day.  
	Usually 30 one-day entries to cover a monthly period.
	 
	222           statistics accumulated during the current day and contains
	223         previous days historical statistics.
	 Note---Current day is fractional data so I believe it should be just complete days after first 24 hrs.
	 
	13) txt suggestion
	312        In this This section we provides an example of using the MIB objects
	313        described in Section 7 to set up a CEP PW.  While this example is not
	314        meant to illustrate every permutation of the MIB, it is intended as
	315        an aid to understanding some of the key concepts.  It is meant to be
	316        read after going through the MIB itself.  See [RFC5601] for an
	317        example
	318        of setting up PSN Tunnels.
	 
	14) Lines 404, 405 409, 410 should be removed
	 
	15) Acknowledgements
	 The section can address the original authors as I mention abouve and anyothers who made contribution like:
	 
	   This document was produced by the PWE3 Working Group.  Special thanks
	   to xxxx who were the editor sof this document at the pre-WG versions of the drafts and to YYYY for 
	   close review and good suggestions.
	 
	16)
	Line 263: ifIndex, SONET/SDH Path Time slot, the pwCepCfgTable index, config
	Should be configuration instead of config.  There are multiple such mistakes.  Please fix them.
	 
	 
	Part 2: MIB definition 
	////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////
	 1) 449     -- Revision history.
	450       REVISION "200712091200Z"  
	Should reflect current date with no History. Or for last submission the full history and not just 1st.
	 
	2) Suggest the pwCepPerfIntervalNumber and pwCepConformanceCfgIndex
	use Unsigned32 instead of Integer32
	 
	3)
	lines:466 to 494 are confusing. Specially reading:
	489               all four bits are set to 1 in this objects it represents a
	And 
	492               The relevant bits are the 28 least significant bits out of
	493               the 32 bit value."
	 Lines 489 says all 4 bits assuming there are 4 only and 493 address 28 out of 32.
	 
	4)
	500               or TUG-3 containing TU-3 circuit. The value of 'other' MUST
	501               be used if the Uuse of this object is not applicable."
	 
	5)
	503         SYNTAX INTEGER {
	504                   other ( 1), 
	If other means not applicable as the text suggested why not to call it NA?
	 
	6) 
	608              removed, the agent MAY SHOULD delete the associated PW rows
	609              (e.g., this pwCepTableEntry). If the agent does not
	610               delete the rows, it is RECOMMENDED MUST that the agent set this
	 
	7)
	646              The peerZZZIncompatible bits are set if the local configuration
	647              is not compatible with the peer configuration as available
	648              from the CEP option received from the peer through the
	649              signaling process and the the local node cannot support such
	650              asymmetric configuration.
	 Lines 646 to 650 should be removed seems duplicates
	 
	8)
	661 VT to a PW. This table is indexed by the pwIndex of the
	662         applicable PW entry in the pwTable
	 
	should give the reference ([RFC5601]) to the pwTable. 
	Please also check whether should add the reference in other places. 
	It is same for "pwCepSonetExt tables" (line 1100).
	 
	9)
	667     All read-write object in this table MAY be changed at any
	         time, however change of some objects (for example
	         pwCepCfgIndex) during PW forwarding state MAY cause traffic
	670      disruption.
	[Richard] If all entries behave the same we can specify that in the table entry for all. If there are exception, 
	the entry will indicate such, and then per exception, the relevant object will state how.
	 
	10)
	Inconsistent prefix of object name:
	[Richard]
	pwCepConfigError should be pwCepCfgConfigError, pwCepSonetPayloadLength should be PwCepCfgCepSonetPayloadLength 
	 
	11）The description of pwCepCfgRowStatus
	It says: None of the read-create objects values can be changed
	when pwCepCfgRowStatus is in the active(1) state. Changes
	are allowed when the pwRowStatus is in notInService(2) or
	notReady(3) states only.
	[Richard]
	Suggest that the value of pwCepCfgName can be changed when 
	pwCepCfgRowStatus is in state 'active' or in 'notInService'.
	Otherwise, it would be inconvenient for NMS operation.
	Please check whether any other MIB object in the table have this 
	similar requirement.
	    There are two ways to make changes:

		
	a.	Only do some change on the description of pwCepCfgRowStatus 

	    You could refer to http://www.rfc-editor.org/authors/rfc5833.txt <http://www.rfc-editor.org/authors/rfc5833.txt> 
	    See the description of capwapBaseAcNameListRowStatus, capwapBaseWtpProfileRowStatus.

		
	b.	Or you give some text such as ” Object MAY be changed at any time” in the 

	description of pwCepCfgRowStatus, and add some row operation description in
	each object’s  (such as pwCepCfgName) description.
	 
	12) Question:
	Should the MIB define some notification such as CEP Failure event, CEP PW
	down event?
	 
	13)The description of pwCepFracRowStatus looks a bit simple. Suggest
	to give some text similar to pwCepCfgRowStatus.
	 
	14)  in the conf section it says
	This group is only mandatory….., maybe better say This group is mandatory only…
	 
	15) 
	In the section 7, some key words such as PW , NSP should give expanded
	words.
	* Notes:  To get a MIB file, the guy usually just take a copy of section 7.  Object Definitions.
	The words such as PW better have a expansion.
	  
	16) Question:
	Line 764: Why "-- Status Only" is only for peerDbaIncompatible ( 3), 
	not for peerEbmIncompatible and peerRtpIncompatible?
	Same question is for peerEbmAsymmetric, peerRtpAsymmetric,
	peerAsyncAsymmetric.
	 
	17)
	The description of pwCepFracTable does not give any text about the
	persistence of writable objects in the table.
	 
	18) Suggestion:
	I think that the pwCepCfgIndex is a better name than pwCepCfgTableIndex
	 
	 
	Regards
	Richard