ietf
[Top] [All Lists]

RE: Last call review of draft-ietf-mpls-tp-linear-protection-mib

2017-01-16 03:49:40



Adrian, thank you so much for the detailed review.





Please, see my response (starting with [JR]) to your comments below.





Best regards,





Jeong-dong






________________________________
보낸 사람 : "Adrian Farrel" <adrian(_at_)olddog(_dot_)co(_dot_)uk>
보낸 날짜 : 2017-01-15 02:08:15 ( +09:00 )
받는 사람 : ietf(_at_)ietf(_dot_)org <ietf(_at_)ietf(_dot_)org>
참조 : draft-ietf-mpls-tp-linear-protection-mib(_at_)ietf(_dot_)org 
<draft-ietf-mpls-tp-linear-protection-mib(_at_)ietf(_dot_)org>, 
mpls(_at_)ietf(_dot_)org <mpls(_at_)ietf(_dot_)org>
제목 : Last call review of draft-ietf-mpls-tp-linear-protection-mib



The IESG has received a request from the Multiprotocol Label Switching WG



(mpls) to consider the following document:



- 'MPLS Transport Profile Linear Protection MIB'



as Proposed Standard







The IESG plans to make a decision in the next few weeks, and solicits



final comments on this action. Please send substantive comments to the



ietf(_at_)ietf(_dot_)org mailing lists by 2017-01-26. Exceptionally, comments 
may be



sent to iesg(_at_)ietf(_dot_)org instead. In either case, please retain the



beginning of the Subject line to allow automated sorting.





I have reviewed the -11 version of this document. It is well written and



clear to read. I particularly welcome the explanations of the various



objects.





Here are a very few minor points and some nits.





I think the document is ready to proceed to publication.





Thanks,



Adrian





===





Section 1 concludes with





Although the example



described in Section 7 specify means to configure OAM identifiers for



MPLS-TP tunnels, this should be seen as indicating how the MIB values



would be returned in the specified circumstances having been



configured by alternative means.





Two thoughts:





1. This text needs to be repeated in Section 7





2. This would read better as:





Although the example



described in Section 7 shows a way to configure OAM identifiers for



MPLS-TP tunnels, this also indicates how the MIB values would be



returned if they had been configured by alternative means.



[JR] OK, those two points will be reflected to the revision.




---





Section 4.





The first sentence is a little hard to read...





RFC 6378 [RFC6378] defines the protocol to provide a linear



protection switching mechanism for MPLS-TP with protection domain as



a point-to-point LSP.





Looking at section 1.1. of RFC 6378, I think you could write...





RFC 6378 [RFC6378] defines the protocol to provide a linear



protection switching mechanism for MPLS-TP for a point-to-point LSP



within the protection domain bounded by the end points of the LSP.




[JR] OK.




---





Is Section 5.1 too terse? Maybe a two line explanation of each new TC?



[JR] Would the following text be ok?


The following new textual conventions are defined in this document:


o MplsLpsReq: This Textual Convention describes an object that


stores the PSC Request field of the PSC control packet.





o MplsLpsFpathPath: This Textual Convention describes an object


that stores the Fault Path (FPath) field and Data Path (Path)


field of the PSC control packet.





o MplsLpsCommand: This Textual Convention describes an object that


allows a user to perform any action over a protection domain.





o MplsLpsState: This Textual Convention describes an object that


stores the current state of the PSC state machine.






---





Please have a quick check to see whether sometimes when you say "this



MIB" you mean "this MIB module" (etc., for other uses of "MIB").





For example the Description clause of mplsLpsNotificationEnable



"Provides the ability to enable and disable notifications



defined in this MIB.


[JR] I checked the whole document, and that was the only place to add the word, 
“module”.





---





It is a little odd that MplsLpsReq has a syntax of



OCTET STRING (SIZE (2)), a display hint of "1d", and you list the



potential values in binary.





I should think that the values should be listed in decimal as they



are shown in RFC6378 and RFC7271.





Then it is just a question of whether this should be a text string or



an integer, which probably doesn't matter, but if your keep it as a



octet string, you do need to say how the numbers are encoded (presumably



ASCII?).



[JR] I will change it to an integer.


---





For MplsLpsFpathPath why do you say...



Bits are numbered from left to right.



...I don't see any reference to bits.




[JR] The notion of left/right was intended to indicate that FPath (the first 
octet, which is in left) is followed by Path (the second octet, which is in 
right). But, you are right and there is no reference to bits.


Would it be ok to remove the sentence, “Bits are numbered from left to right.” ?







---





mplsLpsConfigSdBadSeconds and mplsLpsConfigSdGoodSeconds could use a



Units clause (although it should be pretty obvious from the name and



description!)




[JR] UNITS “seconds” will be added.




---





Is there a reason why you used



SYNTAX INTEGER {true (1), false (2)}



instead of TruthValue in



mplsLpsStatusRevertiveMismatch



mplsLpsStatusProtecTypeMismatch



mplsLpsStatusCapabilitiesMismatch



mplsLpsStatusPathConfigMismatch OBJECT-TYPE




[JR] No reason. TruthValue will be used in the revision.




---





mplsLpsStatusPathConfigMismatch OBJECT-TYPE



SYNTAX INTEGER {true (1), falsmplsOamIdMeMpIndexNexte (2)}





Looks like a typo although it will compile :-)



[JR] Once TruthValue is in place, this mistake will go away.






===





Nits



---



Section 1



s/read- write/read-write/



s/document is consistent/document are consistent/



---



Section 4



s/alternate/alternative/



---



Section 5.3



s/failures of linear protection/failures of the linear protection/



---



mplsLpsMeStatusTable



s/liear/linear/




[JR] All the nits will be fixed. Thank you.



<Prev in Thread] Current Thread [Next in Thread>