Hi, please find below my review of draft-ietf-pce-pcep-mib-06. Overall, the document is in a pretty good shape. It is nice to review MIB modules that are well put together.
The MIB module compiles fine with smiling 0.4.8 and produces no warnings. Below is a list of comments in document order of appearance. Some may require an action other maybe not. I have marked the ones that I find more important with (*). a) I guess you should spell out PCE In the title. b) The abstract says the I-D defines an "experimental portion" of the MIB while the document is marked as standards-track. This does not seem to be consistent. c) In section 2, ATD should STD. d) The LAST-UPDATED and REVISION clauses as well as the Copyright year in the DESCRIPTION clause of the MODULE-IDENTITY all date back to 2013 even though the I-D was last updated in 2014. You may want to move to a date in 2014. e) I suggest to replace pcePcepMIBObjects with pcePcepObjects. This aligns more with the recommendation given in appendix D of RFC 4181. f) I think the description of pcePcepEntityTable should use plural, that is, "... about PCEP Entities". Or will this table always have only a single row? g) If you want to describe the enums of an INTEGER, please do so in the DESCRIPTION clause and not in comments (since comments may not be preserved by tools processing MIB modules). h) pcePcepEntityAddr seems to serve two purposes, depending on whether the peer is a pce and a pcc. The pcePcepPeerRole object makes me believe an entity can also be both at the same time. If so, then how does one interpret pcePcepEntityAddr? (*) i) pcePcepEntityConnectMaxRetry has a UNITS clause saying it is in seconds. This might be a cut and paste error? j) pcePcepPeerAddr has a length restriction which essentially also restricts pcePcepPeerAddrType. pcePcepPeerAddrType kind of has a restriction inline but not explicit via subtyping. It might be more inline with the INET-ADDRESS-MIB to not restrict this. If you want to restrict things, you may want to do that in a compliance statement. (*) Despite future proofing considerations, the restrictions make it impossible to have PCE entities with non-global that is non-unique addresses (because you may not be able to disambiguate them). k) pcePcepPeerDiscontinuityTime refers to RFC 1907. I think the current version it should refer to is RFC 3418. In any case, if you think explicit text concerning sysUpTime is needed, I think this warrants a normative reference added to RFC 3418 and a statement in section 5.5. (I am not sure this is really needed since you kind of import knowledge about sysUpTime via the TimeStamp TC you are using. j) The *AvgRspTime, *LWMRspTime, *HWMRspTime objects have a resolution of seconds. This means that times < 500ms will be represented as 0 seconds and thus indistinguishable from no message at all. I do not know whether this is an issue or not. Is it possible to have sub-second response times? (*) k) pcePcepSessState typo: entiries -> entries l) pcePcepSessPeerDeadTimer typo: for for -> for -- Juergen Schoenwaelder Jacobs University Bremen gGmbH Phone: +49 421 200 3587 Campus Ring 1, 28759 Bremen, Germany Fax: +49 421 200 3103 <http://www.jacobs-university.de/> _______________________________________________ Pce mailing list [email protected] https://www.ietf.org/mailman/listinfo/pce
