Re: WG Last Call - draft-ietf-hubmib-power-ethernet-mib-04.txt
"C. M. Heard" <[email protected]>
| Newsgroups | gmane.ietf.hubmib |
|---|---|
| Message-ID | <[email protected]> |
On Mon, 23 Dec 2002, Romascanu, Dan (Dan) wrote: > >This is the official announcement for the Working Group Last Call for >the Power Ethernet MIB. The Internet-Draft is available at > >http://www.ietf.org/internet-drafts/draft-ietf-hubmib-power-ethernet-mib-04.txt > >Please send your comments until the latest January 15, 2003, to >[email protected]. In an effort to help the WG produce the cleanest possible document I've attempted to do a fairly detailed "MIB Doctor" on this draft. Please note that I've not been able to check the document for editorial and technical consistency with the 802.3af draft, since I have not been participating in the Power Ethernet work and do not have access to the specification. Someone else should do that check. The following list of comments goes through the sections of the document in the order that they appear. 1.) I-D Boilerplate -- OK (required stuff is present, and there are no references or section numbers). 2.) Abstract -- references are not allowed in the abstract; please change "Ethernet-like Interfaces MIB [RFC2665]" to "Ethernet-like Interfaces MIB". 3.) Introduction (Section 1) -- in the first paragraph the acronyms MDI and arguably DTE are not common terms and therefore need to be expanded on first use, or else removed. My suggestion would be to change "Power Ethernet (DTE Power via MDI)" to "Power Ethernet [IEEE-802.3af]" which will circumvent the need to define those terms here. In the second paragraph please change the reference from [RFC2863] to [RFC2119]. 4.) MIB Boilerplate (Section 2) -- the official MIB boilerplate has been changed owing to the publication of the full-standard SNMPv3 RFCs. So, please change the section heading to "Internet-Standard Management Framework" and use the boilerplate text from the OPS area web site (http://www.ops.ietf.org/mib-boilerplate.html). Note that the new boilerplate uses [RFC3410] instead of [RFC2570] and does not refer to the SNMP protocol documents themselves. 5.) Overview (Section 3) -- OK 6.) MIB Structure (Section 4) -- in the first sentence please change "and the fourth scalar objects" to "and the fourth contains scalar objects." 7.) Evolution of the Document, Limitations and Future Work (Section 5) -- in its present form this section would not be appropriate in the final published RFC. It should either be reworded, eliminated from the draft, or moved to the back of the draft with a prominent note telling the RFC Editor that it is to be removed prior to publication. 8.) Changes Log (Section 6) -- this section would not be appropriate in the final published RFC. It should be moved to the back of the draft with a prominent telling the RFC Editor that it is to be removed prior to publication. In addition, please change "[PWR-MIB]" to "draft-romascanu-hubmib-power-ethernet-mib-00.txt"; this reference needs to be removed (see below) since it points to an expired draft. 9.) Definitions (Section 7) -- there are two MIB compiler warnings related to unreferenced notification groups plus several other things that either need correction or that could (in the opinion of this reviewer) be improved. I'll go through them in the order in which they appear in the MIB module. (a) Our AD has requested that working group mailing list information appear in the CONTACT-INFO clause of the MODULE-IDENTITY invocation. You may therefore want to add the following lines at the top: WG charter: http://www.ietf.org/html.charters/hubmib-charter.html Mailing Lists: General Discussion: [email protected] To Subscribe: [email protected] In Body: subscribe your_email_address and change the indentation of the rest of the stuff to match (you should not go beyond column 72, see http://www.ietf.org/ID-nits.html). (b) IESG policy now requires that a copyright notice appear in the DESCRIPTION clause of the MODULE-IDENTITY invocation (see http://www.ietf.org/IESG/STATEMENTS/MIB-COPYRIGHT.txt for details). Here is a suitable template: DESCRIPTION "[ ... ] Copyright (C) The Internet Society (date). This version of this MIB module is part of RFC yyyy; see the RFC itself for full legal notices." -- RFC Ed.: replace yyyy with actual RFC number & remove this notice Prior to submission '(date)' must be replaced with the current year, i.e., '(2003)'. (c) The "box in the stack, module in a rack" language that appears in the DESCRIPTION clauses for pethPsePortGroupIndex, pethMainPseGroupIndex, and pethNotificationControlGroupIndex suggests that the same value will be used in all three of these indices to refer to a given physical entity. If that is true, it would be best to say so explicitly, and in that case I would recommend modifying the DESCRIPTION clause for pethPsePortGroupIndex to something along the following lines: "This variable uniquely identifies the group containing the port to which a power Ethernet PSE is connected. 'Group' means (box in the stack, module in a rack) and the value 1 MUST be used for non-modular devices. Furthermore, the same value MUST be used in this variable, pethMainPseGroupIndex, and pethNotificationControlGroupIndex to refer to a given (box in the stack, module in a rack)." In any case, the sentence "pethPseMidSpanGroupCapacity is the number of Mid-Span PSE groups that can be contained within the Mid-Span PSE" needs to be removed because there no such object (indeed, there is no mention of pethPseMidSpanGroupCapacity anywhere else in the document). (d) In the DESCRIPTION clause for pethPsePortIndex please change "pethPseGroupIndex" to "pethPsePortGroupIndex". (e) In the following object definition pethPsePortAdminEnable OBJECT-TYPE SYNTAX INTEGER { enable(1), disable(2) } MAX-ACCESS read-write STATUS current DESCRIPTION "Enables power supply on this port. Setting this object at a value enable(1) enables power and detection mechanism for this port. Setting this object at a value disable(2) disables power for this port." ::= { pethPsePortEntry 3 } use of the name "pethPsePortAdminEnable" rather than "pethPsePortAdminStatus" suggests that a more appropriate SYNTAX would be TruthValue. Thus, it is recommended that the SYNTAX be changed to TruthValue (both in the object definition and in the SEQUENCE) and that the DESCRIPTION clause be updated to match by changing "the value enable(1)" to "the value true(1)" and by changing "the value disable(2)" to "the value false(2)". [It would also be nice to have the object name appear a few columns to the left of the SYNTAX, MAX-ACCESS, STATUS, and DESCRIPTION clauses as it does in most other object definitions; maybe indent all the other clauses three spaces, as was done in the definitions of the index objects?] (f) In the following object definition pethPsePortDetectionStatus OBJECT-TYPE SYNTAX INTEGER { disabled(1), searching(2), deliveringPower(4), fault(5), test(7), denyLowPriority(8) } MAX-ACCESS read-only STATUS current DESCRIPTION "Describes the operational status of the port PD detection. A value of disabled(1)- indicates that the PSE State diagram is in the state IDLE A value of searching(2)- indicates that the PSE State diagram is in the state DETECTION, CLASSIFICATION, SIGNATURE_INVALID or BACKOFF. A value of deliveringPower(4) - indicates that the PSE State diagram is in the state POWER_UP, POWER_ON or POWER_OFF. A value of fault(5) - indicates that the PSE State diagram is in the state TEST_ERROR or the state IDLE due to the variable error condition. Faults detected are vendor specific. A value of test(7) - indicates that the PSE State diagram is in the state TEST_MODE. A value of denyLowPriority(8) indicates that the port was disabled by the power management system, in order to keep active higher priority ports. " ::= { pethPsePortEntry 7 } the enumerations are not contiguous, which is allowed but contrary to the recommendations in RFC 2578 Section 7.1.1. Is there a reason for this? If there isn't, please change to conform to the recommendations. If there is, it would be a good idea to document it (e.g., if you are aligning with the numbering of states in the IEEE specification then say so, and maybe even add a REFERENCE clause). (g) In the following object definition pethPsePortPowerMaintenanceStatus OBJECT-TYPE SYNTAX INTEGER { ok(1), underCurrent(2), mPSAbsent(3) } MAX-ACCESS read-only STATUS current DESCRIPTION "The value ok(1) indicates the Power Maintenance Signature is present and the overcurrent condition has not been detected. The value overCurrent (2) indicates an overcurrent condition has been detected. The value mPSAbsent(3) indicates that the Power Maintenance Signature is absent." REFERENCE "[IEEE Draft P802.3af/D4.0, November, 2002, 30.9.1.1.8 aPSEPowerMaintenanceStatus]" ::= { pethPsePortEntry 10 } the enumeration labels and the DESCRIPTION clauses don't match. This needs to be fixed; it appears that "underCurrent" should be "overCurrent". [ Scott Mains has also pointed this out in a message posted 19 Dec 2002.] (h) In the DESCRIPTION clause for this object pethPsePortOverCurrentCounter OBJECT-TYPE SYNTAX Counter32 MAX-ACCESS read-only STATUS current DESCRIPTION "Counts the number of times that the aPSEPowerCurrentStatus attribute changes from any value to the value overCurrent(2)." REFERENCE "[IEEE Draft P802.3af/D4.0, November, 2002, object 30.9.1.1.1 aPSEOverCurrentCounter]" ::= { pethPsePortEntry 12 } please change "aPSEPowerCurrentStatus" to "pethPsePortPowerMaintenanceStatus". [ Scott Mains has also pointed this out in a message posted 19 Dec 2002.] (i) In the following object pethPsePortPowerClassifications OBJECT-TYPE SYNTAX INTEGER { class0(1), class1(2), class2(3), class3(4), class4(5) } MAX-ACCESS read-only STATUS current DESCRIPTION "Classification is a way to tag different terminals on the Power over LAN network according to their power consumption. Devices such as IP telephones, WLAN access points and others, will be classified according to their power requirements. The value is only valid while a valid PD is being detected as indicated by the attribute pethPsePortDetectionStatus reporting the value or deliveringPower(4)." REFERENCE "[IEEE Draft P802.3af/D4.0, November, 2002, object 30.9.1.1.7 aPSEPowerClassification]" ::= { pethPsePortEntry 14 } are the classification labels 'class0' and so on defined in the IEEE specification? If so, please add a sentence along the lines of The meaning of the classification labels is defined in the IEEE specification. at the end of the first paragraph of the DESCRIPTION clause. (Also, if the IEEE document uses numeric values 0 through 4 for class0 through class 4 you _might_ want to do the same thing -- this would fall under the "valid reason" exemption to the "start at 1" recommendation.) (j) You may want to add the following REFERENCE clause to the definition of pethPdPortIndex: REFERENCE "RFC 2863, ifIndex" (k) In the following object pethPdPortAdminEnable OBJECT-TYPE SYNTAX INTEGER { enable(1), disable(2) } MAX-ACCESS read-write STATUS current DESCRIPTION "This value identifies the operational state of the PD functions. An interface which can provide the PD functions will be enabled to do so when this attribute has the value enable. When this attribute has the value disable the interface will act as it would if it had no PD function." ::= { pethPdPortEntry 2 } the name and semantics (in particular the fact that it is writeable) suggests that it is an administrative state while the DESCRIPTION says that it is an operational state. If it is indeed an admin state, please change the words "operational state" to "administrative state" in the DESCRIPTION. Also, use of the name "pethPdPortAdminEnable" rather than "pethPdPortAdminStatus" suggests that a more appropriate SYNTAX would be TruthValue. Thus, it is recommended that the SYNTAX be changed to TruthValue (both in the object definition and in the SEQUENCE) and that the DESCRIPTION clause be updated to match by changing "the value enable" to "the value true(1)" and by changing "the value disable" to "the value false(2)". (l) The "box in the stack, module in a rack" language that appears in the DESCRIPTION clauses for pethPsePortGroupIndex, pethMainPseGroupIndex, and pethNotificationControlGroupIndex suggests that the same value will be used in all three of these indices to refer to a given physical entity. If that is true, it would be best to say so explicitly, and in that case I would recommend modifying the DESCRIPTION clause for pethMainPseGroupIndex to something along the following lines: "This variable uniquely identifies the group to which a power Ethernet PSE is connected. 'Group' means (box in the stack, module in a rack) and the value 1 MUST be used for non-modular devices. Furthermore, the same value MUST be used in this variable, pethPsePortGroupIndex, and pethNotificationControlGroupIndex to refer to a given (box in the stack, module in a rack)." (m) Making the same assumptions as above, the equivalent modification for pethNotificationControlGroupIndex would be: "This variable uniquely identifies the group. 'Group' means (box in the stack, module in a rack) and the value 1 MUST be used for non-modular devices. Furthermore, the same value MUST be used in this variable, pethPsePortGroupIndex, and pethMainPseGroupIndex to refer to a given (box in the stack, module in a rack)." In any case, the current language "it is RECOMENDED that the value 1 be used for non-modular devices" is not consistent with that in the DESCRIPTION clauses for pethPsePortGroupIndex and pethMainPseGroupIndex; the same imperative should be used in all three places. I think that the MUST wording is the right way to go, since it does not cost the agent anything and makes life easier for the manager. (n) In the following object pethNotificationControlEnable OBJECT-TYPE SYNTAX INTEGER { enable(1), disable(2) } MAX-ACCESS read-write STATUS current DESCRIPTION "Enable Notification from Agent" use of the name "pethNotificationControlEnable" rather than "pethNotificationControlStatus" suggests that a more appropriate SYNTAX would be TruthValue. Thus, it is recommended that the SYNTAX be changed to TruthValue (both in the object definition and in the SEQUENCE) and that the DESCRIPTION clause be updated to read "This object controls, on a per-group basis, whether or not notifications from the agent are enabled. The value true(1) means that notifications are enabled; the value false(2) means that they are not." (o) Regarding the notification definitions: it was recently noted on the [email protected] mailing list that MIB doctors should make sure that notifications defined in IETF MIB modules cannot "flood" the communication network in case of a high rate of occurrence of the events that trigger them. Hence, it is recommended that the notification definitions in this MIB module include a rate limiting mechanism, e.g., a statement in the DESCRIPTION clause that at least x milliseconds must elapse between successive notifications triggered by any one object instance (this works for the notifications defined in this MIB module because they are triggered by changes in the values of MIB objects). One could either choose a fixed value for x (e.g., 500 msec) or allow it to be configured (another object would need to be defined for that purpose, and it would need to be mentioned in the Security Considerations section). (p) The following warnings are generated from the command smilint -l 9 -s -i namelength-32 ./POWER-ETHERNET-MIB ./POWER-ETHERNET-MIB:667: [6] current group `pethPsePortNotificationGroup' is not referenced in this module ./POWER-ETHERNET-MIB:674: [6] current group `pethMainPowerNotificationGroup' is not referenced in this module These warnings are issued because it it not uncommon for an object group or notification group to be omitted from the conformance statements by mistake. The module is still well-formed, because an object group or notification group that is not mentioned in any MANDATORY-GROUPS clause or in any GROUP clause is unconditionally optional; but it's not altogether clear whether this was the MIB designer's intent or just an oversight. For this reason, it is RECOMMENDED that object groups and notification groups be mentioned in a GROUP clause even when they are unconditionally optional. Appropriate GROUP clauses might be: GROUP pethPsePortNotificationGroup DESCRIPTION "Implementation of the pethPsePortNotificationGroup is RECOMMENDED for systems which implement PSE ports." GROUP pethMainPowerNotificationGroup DESCRIPTION "Implementation of the pethMainPowerNotificationGroup is RECOMMENDED for systems which implement main power supply within a PSE Device." If "RECOMMENDED" is too strong, then "encouraged" or "optional" could be used instead. These GROUP clauses should be added to the end of pethCompliance. [Note: if it was intended that notifications be mandatory, then the language from the other GROUP clauses in the pethCompliance statement should be adapted instead.] (q) If it is indeed the case that the notifications are intended to be optional, then the GROUP clause (in pethCompliance) for pethNotificationControlGroup has an erroneous DESCRIPTION clause. It says "The pethNotificationControlGroup is mandatory for systems which implement PSE ports." It should say "The pethNotificationControlGroup is mandatory for systems which implement notifications." (r) The compliance statements in this MIB module are redundant. It defines pethPseCompliance, which applies to PSEs, and pethPdCompliance, which applies to PDs. But it also defines pethCompliance, which applies to any device, and which is a superset of the other two (it does this by making all groups conditionally mandatory depending on what functions are implemented). This redundancy is not necessary. My recommendation is to remove pethPseCompliance and pethPdCompliance. (s) General comment on REFERENCE clauses: I notice that some of the object definitions have REFERENCE clauses that point to the IEEE specification while others do not. It would indeed be desirable to add reference clauses for all objects that have equivalent GDMO objects in 802.3af clause 30.9; and for those that do not but which require hardware support, it would be good to refer to the clauses in the IEEE specification that mandate the necessary hardware support, as was done in the ETHER-WIS MIB module for the objects in etherWisDeviceGroupBasic and etherWisDeviceGroupExtra (see draft-ietf-hubmib-wis-mib-05.txt or its successor). One word of caution, though: the REFERENCE clauses will need to point to the final published IEEE STD 802.3af and not to IEEE Draft P802.3af/D4.0, and you will need to re-check the clause numbers each time a new version appears. 10.) References (Sections 8 and 9) -- the references need to be updated owing mainly to the publication of the new SNMPv2 RFCs but also to a few errors/oversights. Here are the details: Remove normative reference [RFC2570]. RFC 2570 has been replaced by RFC 3410, which is supposed to be an informative reference. Remove normative references [RFC2571], [RFC2572], [RFC2573], [RFC2574], and [RFC2575] since they are no longer required by the boilerplate and are not used elsewhere. Add a normative reference [RFC2863] to RFC 2863, since InterfaceIndex is imported from the IF-MIB. Remove informative references [RFC1155], [RFC1212], [RFC1215], [RFC1157], [RFC1901], [RFC1905], and [RFC1906] since they are no longer required by the boilerplate and are not used elsewhere. Remove the informative reference [PWR-MIB], since it points to an expired draft that defined a predecessor to this MIB module. Add an informative reference [RFC3410] to RFC 3410 since it is required by the boilerplate and security sections (see http://www.ops.ietf.org/mib-boilerplate.html and comment 11 below). Please revise the style of the references to match that of recently published RFCs (see RFC 3418 or the MIB boilerplate http://www.ops.ietf.org/mib-boilerplate.html for an example). The authors need to be aware that this document can go through the IETF approval process but cannot be published as an RFC as long as [IEEE-802.3af] points to IEEE Draft P802.3af/D4.0 (or any other version of the draft) rather than IEEE STD 802.3af. That reference, however, should _not_ be updated until the final standard is actually approved, and then there should be a final review of this document to ensure that it is editorially and technically consistent with the final IEEE standard ... things like clause numbers (which are used in the REFERENCE clauses) are subject to change even very late in the process. 10.) IPR Notices (Section 10) -- probably OK: the draft contains verbatim copies of the IPR notices specified in bullets (A) and (B) of Section 10.4 of RFC 2026 as required, but the AUTHORS NEED TO CONFIRM that the notice in bullet (D) is not needed. 11.) Security Considerations Section -- the MIB security guidelines are in the process of being updated, so the text on the OPS area web site (http://www.ops.ietf.org/security.html) is out of date. However, based on the latest wisdom to appear on the [email protected] mailing list, I would recommend a rewrite along the following lines: There are a number of management objects defined in this MIB module with a MAX-ACCESS clause of read-write. These are: pethPsePortAdminEnable pethPsePortPowerPairs pethPsePortPowerDetectionControl pethPsePortPowerPriority pethPsePortType pethPdPortAdminEnable pethMainPseUsageThreshold pethNotificationControlEnable Such objects may be considered sensitive or vulnerable in some network environments. The support for SET operations in a non-secure environment without proper protection can have a negative effect on network operations. Some of the readable objects in this MIB module (i.e., objects with a MAX-ACCESS other than not-accessible) contain information that may be considered sensitive in some network environments. These are: pethPsePortPowerPairsControlAbility pethPsePortPowerPriority pethPsePortPowerClassifications It is thus important to control even GET and NOTIFY access to these objects and possibly to encrypt their values when sending them over the network via SNMP. SNMP versions prior to SNMPv3 did not include adequate security. Even if the network itself is secure (for example by using IPSec), even then, there is no control as to who on the secure network is allowed to access and GET/SET (read/change/create/delete) the objects in this MIB module. It is RECOMMENDED that implementers consider the security features as provided by the SNMPv3 framework (see [RFC3410], section 8), including full support for the SNMPv3 cryptographic mechanisms (for authentication and privacy). Further, deployment of SNMP versions prior to SNMPv3 is NOT RECOMMENDED. Instead, it is RECOMMENDED to deploy SNMPv3 and to enable cryptographic security. It is then a customer/operator responsibility to ensure that the SNMP entity giving access to an instance of this MIB module is properly configured to give access to the objects only to those principals (users) that have legitimate rights to indeed GET or SET (change/create/delete) them. 12.) IANA Considerations section (none present) -- NEED TO CHECK WITH OUR AD. An IANA Considerations section probably should not be needed since this document does not define a name space. However, the current version of http://www.ietf.org/ID-nits.html states that there is supposed to be an IANA considerations section whenever IANA is reequired to make an assignment in a registry prior to RFC publication. I have raised objections about this policy, and an answer from our AD is pending. For the time being I think that the notation powerEthernetMIB MODULE-IDENTITY [ ... ] ::= { mib-2 XXX } -- RFC Ed.: replace XXX with IANA-assigned number & remove this notice that is already present will suffice; but if an IANA Considerations section has to be written, I would suggest something along the lines of The POWER-ETHERNET-MIB module defined in this document is registered under an IANA-assigned node in the mib-2 OBJECT IDENTIFIER subtree [RFC2578]. That node is defined as "powerEthernetMIB" in the POWER-ETHERNET-MIB MODULE-IDENTITY invocation. Subordinate assignments under powerEthernetMIB will be managed by the IETF working group responsible for maintaining the ETHER-WIS MIB module; no further IANA action is required. ******************************************************* * EDITOR'S NOTE (to be removed prior to publication): * * before this draft is published as an RFC, a subid * * under the mib-2 OBJECT IDENTIFIER subtree needs to * * be assigned by IANA for powerEthernetMIB, and this * * subid needs to be inserted by the RFC Editor into * * the POWER-ETHERNET-MIB -WIS MODULE-IDENTITY value. * ******************************************************* 13.) Copyrights -- the full copyright notice should start with the line Copyright (C) The Internet Society (date). All Rights Reserved. The absence of a copyright notice in the DESCRIPTION clause of the MODULE-IDENTITY invocation was noted in 9(b) bove. Note: when the draft is resubmitted, please be sure to change the year to 2003 in all three places (on the front page, in the DESCRIPTON clause of the MODULE-IDENTITY invocation, and in the full copyright statement). 14.) General comments on formatting -- the layout of the MIB module is not uniform, as noted in a couple of places in (9) above, and it would be nice to have that fixed. Also, the draft does not fully adhere to the document formatting guidelines in Instructions to RFC Authors, <draft-rfc-editor-rfc2223bis-03.txt>. That can be fixed during the final editing process, but it would speed up the process (and reduce the chances for error) if the next version of the draft would attempt to comply more closely with those guidelines. //cmh