RE: AD review of: draft-ietf-ipcdn-bpiplus-mib-12.txt - part 1
"Eduardo Cardona" <[email protected]>
| Newsgroups | gmane.ietf.ipcdn |
|---|---|
| Message-ID | <[email protected]> |
Thanks Bert for the detail revision. I will review and propose the corresponding actions shortly Thanks Eduardo -----Original Message----- From: Wijnen, Bert (Bert) [mailto:[email protected]] Sent: Wed 7/14/2004 10:03 AM To: Ipcdn (E-mail) Cc: Subject: [ipcdn] AD review of: draft-ietf-ipcdn-bpiplus-mib-12.txt - part 1 Sorry, it took too long to finally get this done. Not sure this document is ready for IETF Last Call. Actaully I think it is not. This is part 1. Mainly serious things. I may have more nits/admin stuff later. Here are my findings. More or less serious: 1. I see various read-write and/or read-create objects and I do not see any text in the DESCRIPTION clauses (non STorageType objects) that tell me what the expected behaviour is w.r.t. persistency of such objects. SO what happens after a restart/reboot? 2. I see a number of ZeroBasedCOunter32 objects that have text aka: (for example docsBpi2CmAuthentInfos) DESCRIPTION "The value of this object is the count of times the CM has transmitted an Authentication Information message, since reboot." It is OK to tell us that a ZeroBasedCOunter32 object must start with zero at (re-)boot time or at row creation. But from that point on, a ZeroBasedCounter32 behaves exactly the same as a Counter32, and so it is incorrect to say "count of X since reboot" because the Counter32 may have wrapped!. Possibly this is not gonna happen in practice, but literally the claim is incorrect. 3. For these ZeroBasedCounter32 objects, I also see no word about a possible discontinuity timer. Why not. Can there NEVER be a discontinuity? Of if there is one does that mean ALL counters experience a discontionuity? The latter is what you basically state (or cause) by not pointing to a specific discontinuity timer. Because then by default it is sysUpTime, and so that means that when you DO experience a discontinuity, then you MUST reset sysUpTime and that means that a discontinuity for EVERYONE (object) that assumes the default. If such is intended, then fine, but it would be good to then state that sysUpTime is the discontinuity timer. 4. I see that for some objects you speak about a "null string" or "NULL string". The base data type for such objects is OCTET STRING. In all those cases I suspect (but I am not sure) that you mean the zero-length octet string. Otherwise I do not understand what "null string" means. Pls explain and fix. 5. For docsBpi2CmtsAuthCmExpiresOld I see: Note: For CMs running in BPI mode, implementation of this object is optional and MAY vary." Mmm... that sounds like a MODULE-COMPLIANCE aspect and I would rather see such things in MODULE-COMPLIANCE and not in object DESCRIPTION clauses. 6. I see: -- Note: the following object has been obsoleted docsBpi2CmtsAuthCmReset OBJECT-TYPE SYNTAX INTEGER { noResetRequested(1), invalidateAuth(2), sendAuthInvalid(3), invalidateTeks(4) } MAX-ACCESS read-write STATUS current So the --Note: is out of sync with the actual status!? What is it? If it IS obsoleted, then status should sya so, And DESCRIPTION clause should explain why it was obsoleted. And the ASN.1 comment line then of course is no longer needed. 7. I see: docsBpi2CmtsAuthCACertIndexPtr OBJECT-TYPE SYNTAX Integer32 (0..10000) And find that a strange limit (range). And nowhere, not even in the docsBpi2CmtsCACertTable do I see an explanation why that range makes sense (assuming that it does). 8. When I see: docsBpi2CmtsIpMulticastAddressType InetAddressType, docsBpi2CmtsIpMulticastAddress InetAddress, docsBpi2CmtsIpMulticastMaskType InetAddressType, docsBpi2CmtsIpMulticastMask InetAddress, I wonder if (in the same row) docsBpi2CmtsIpMulticastMaskType will ever have a different value then docsBpi2CmtsIpMulticastAddressType !?? It seems to me that should NOT be allowed, cause otherwise I am not sure how the ANDing of the Mask is going to work/happen. So the next question then is why you do not do: docsBpi2CmtsIpMulticastAddressType InetAddressType, docsBpi2CmtsIpMulticastAddress InetAddress, docsBpi2CmtsIpMulticastMask InetAddress, And let docsBpi2CmtsIpMulticastAddressType be the discriminator for both InetAddresses. One less object, and less change for error/conflict. But thinking even further, Possibly the best thing to do is to use docsBpi2CmtsIpMulticastAddressType InetAddressType, docsBpi2CmtsIpMulticastAddress InetAddress, docsBpi2CmtsIpMulticastPrefixLength InetAddressPrefixLength, Are not such masks always setup that they basically specify a prefix length? If so, then this is the way to do it with the TCs from INET-ADDRESS-MIB. 9. I see various uses of InetAddress as for example here: docsBpi2CmtsIpMulticastAddress OBJECT-TYPE SYNTAX InetAddress MAX-ACCESS read-create STATUS current DESCRIPTION "This object represents the IP multicast address to be mapped, in conjunction with docsBpi2CmtsIpMulticastMask." The TC for InetAddress (in RFC3291 or its follow on) clearly state that you MUST specify which InetAddressType controls the format of this object as per DESCRIPTION from InetAddress TC: An InetAddress value is always interpreted within the context of an InetAddressType value. Every usage of the InetAddress textual convention is required to specify the InetAddressType object which provides the context. ... 10. I see: docsBpi2CmtsIpMulticastMapControl OBJECT-TYPE SYNTAX RowStatus MAX-ACCESS read-create STATUS current DESCRIPTION "This object controls and reflects the IP multicast address mapping entry. There is no restriction on the ability to change values in this row while the row is active. Inactive rows need not be timed out." Mmm... that "need not be timed out" seems in conflict with the RowStatus TC DESCRIPTION clause in RFC2579. Can you explain why this is? Also, a RowSTatus object MUST specify in its DESCRIPTION clause under which conditions - the row can be activated - which columns (if any) can bve changed while in the active state. I am missing the first. 11. You specify: -- -- The BPI+ MIB Conformance Statements (with a placeholder for -- notifications) -- docsBpi2Notification OBJECT IDENTIFIER ::= { docsBpi2MIB 2 } docsBpi2Conformance OBJECT IDENTIFIER ::= { docsBpi2MIB 3 } docsBpi2Compliances OBJECT IDENTIFIER ::= { docsBpi2Conformance 1 } docsBpi2Groups OBJECT IDENTIFIER ::= { docsBpi2Conformance 2 } Why not be (more) consistent with other MIB modules and follow the suggested OID subtrees as per MIB guidelines, draft-ietf-ops-mib-review-guidelines-03.txt, appendix D: xxxMIB | +-- xxxNotifications(0) +-- xxxObjects(1) +-- xxxConformance(2) | +-- xxxCompliances(1) +-- xxxGroups(2) This is not mandatiory, but consistency is always useful/helpful Nits/administrativia: 1. The RFC editor wants all references to have at least one citation in the document. ALso for the normative references to RFCs from which you import. See MIB review guidelines, sect 3.5 You need to add a citation somehwere for [RFC3411], [RFC2021], [RFC3291], [RFC2670] Thanks, Bert _______________________________________________ IPCDN mailing list [email protected] https://www1.ietf.org/mailman/listinfo/ipcdn _______________________________________________ IPCDN mailing list [email protected] https://www1.ietf.org/mailman/listinfo/ipcdn