Re: [eman] WG Last Call for draft-ietf-eman-energy-monitoring-mib-08 and draft-ietf-eman-energy-aware-mib-13

[Benoit Claise <[e-mail]>]

Dear Alan,

Let me answer your comments on the draft-ietf-eman-energy-aware-mib.
The ones on draft-ietf-eman-energy-monitoring-mib will follow in a 
separate email.
> Hello,
>
> Here are comments about  draft-ietf-eman-energy-monitoring-mib-08 and
> draft-ietf-eman-energy-aware-mib-13.  Due to the holidays, I was unable
> to get these sent to the WG E-mail list sooner.
>
>
> Regarding draft-ietf-eman-energy-aware-mib-13:
> ----------------------------------------------
>
> Section 5, Page 6:
>
>     The UML on Page 6 _looks_ incorrect.  It _looks_ like the eoRelationTable
>     is subordinate to, or a child of, the eoEntry.  I'm not sure how the
>     actual relationship specified in the MIB is _supposed_ to be represented
>     in UML.
Good catch. Corrected.
>
>
> Page 22:
>
>     The eoRelationID in the eoRelationTable is "read-only".  In many cases,
>     I do not see how an energy object can determine the eoRelationID to the
>     Energy Objects to which it is connected, so I _think_ the eoRelationID
>     must be configured by the EnMS.  I _think_ the eoRelationID should be
>     a read-write object like the eoRelationship object.
Yes, that's an obvious mistake.
Thinking about it some more, I believe that we need a rowStatus textual 
convention in order to create new entries in there.
>
>     As an example, consider a manageable, metering power distribution
>     unit (PDU), I don't see how it can autodiscover and populate the
>     eoRelationID of the energy objects to which it is connected.  The useful-
>     ness of the eoRelationID MIB object is reduced if it is only populated
>     in devices where it can be autodiscovered.
>
>
> Regarding draft-ietf-eman-energy-monitoring-mib-08:
> ---------------------------------------------------
>
> Section 5, 3rd paragraph, Page 6:
>
>         "entity4CRCompliance.  This compliance requires that the
>          following 3 MIB objects from ENTITY-MIB [RFC6933]
>          (entPhysicalIndex, entPhysicalName and entPhysicalUUID) MUST be
>          implemented."
>
>     I think entPhysicalClass should be included in this list.  There are
>     other references to the entity4CRCompliance in this draft that need the
>     same update.
Good catch.
Also corrected in the draft-ietf-eman-energy-aware-mib

Regards, Benoit
>
>
> Section 5, Page 7:
>
>            |   +-- r-n INTEGER           eoMeasurementCaliber(5)
>
>     I think the above should be eoPowerMeasurementCaliber(5).
>
>            |      +-- r-n Interger32        eoPowerStateMaxPower(2)
>
>     Change "Interger32" to Integer.
>
>            |   +-- r-n Integer32    eoEnergyParametersIntervalMode(5)
>
>     I think the type of "eoEnergyParametersIntervalMode" should be INTEGER.
>
>
> Section 5, Page 8:
>
>     I suggest changing "powerAttributesMIB" to "POWER-ATTRIBUTES-MIB" for
>     for consistency with previous text that references the ENERGY-OBJECT-MIB
>     and POWER-ATTRIBUTES-MIB.
>
>     A quick summary of the function of each of the tables in the POWER-
>     ATTRIBUTES-MIB, similar to the quick summary of each of the tables in
>     the ENERGY-OBJECT-MIB (on Page 6) might be helpful and consisent.
>
>
>            |   +-- r-n Interger32 eoACPwrAttributesAvgVoltage(2)
>
>        |   +-- r-n Interger32
>            |                   eoACPwrAttributesTotalActivePower(7)
>
>     Change "Interger32" to Integer.
>
>     In the diagram for the eoACPwrAttributesEntry, after eoACPwrAttributes-
>     ThdAmpheres, the eoACPwrAttributesThdVoltage object is missing.
>
>     Also, here, and several places in the document, "Ampheres" is referenced.
>     Not sure if "Ampheres" is intentional, but "Amperes" might be preferable.
>     It might be good to do a case-insensitive search for "ampheres".
>
>
> Section 5, Page 9:
>
>            +-- eoACPwrAttributesDelPhaseEntry(1)
>            |     |   [entPhysicalIndex, eoPhaseIndex]
>            |     |
>            |     +-- r-n Integer32
>            |     |    eoACPwrAttributesDelPhaseToNextPhaseVoltage(1)
>            |     +-- r-n Integer32
>            |     | eoACPwrAttributesDelThdPhaseToNextPhaseVoltage(2)
>            |     +-- r-n Integer32
>                                   eoACPwrAttributesDelThdCurrent(3)
>
>     I think the column numbers 1, 2, and 3, are incorrect.  The actual column
>     numbers in the MIB are 2, 3, and 4.  Perhaps the MIB table should be fixed
>     so the column numbers are 1, 2, and 3.
>     
>
> Section 5, Page 12.
>
>     At the very top, in the diagram for the eoACPwrAttributesEntry, after
>     eoACPwrAttributesThdAmpheres, the eoACPwrAttributesThdVoltage object
>     is missing.
>
>
> Section 5.4, Page 15:
>
>     I suggest changing "powerAttributesMIB" to "POWER-ATTRIBUTES-MIB" for
>     for consistency with previous text that references the ENERGY-OBJECT-MIB
>     and POWER-ATTRIBUTES-MIB.
>
>
> Section 6, Page 20:
>
>     This section references the EMAN-MON-MIB.  I think this should be
>     ENERGY-OBJECT-MIB.
>
>
> Section 9, Page 27:
>
>     This section references the "energyObjectMIBObject".  Not sure what this
>     is.
>
>
> In the MIBs, adding eye-catching comments between the tables would ease
> visually identifying where each MIB table begins.
>
>
> Page 37, "eoPowerOperState" MIB object:
>
>     The second paragraph of the DESCRIPTION clause reads:  "Possible values
>     of eoPowerAdminState...".   I think "eoPowerAdminState" should be changed
>     to "eoPowerOperState."
>
>
> Page 38, "eoPowerStateTable":
>
>     The text reads:
>
>         This table has an expansion-dependent relationship on the
>         eoPowerTable, containing rows describing each Power State
>         for the corresponding Energy Object. For every Energy
>         Object in the eoPowerTable, there is a corresponding
>         entry in this table."
>
>     Not sure I correctly understand this table, but should the second sentence
>     read something like:  "For every Energy Object in the eoPowerTable, there
>     is a corresponding set of entries in this table, one entry for each power
>     state supported by the Energy Object."
>    
>
> Page 38, "eoPowerStateEntry":
>
>     In the DESCRIPTION clause, the state numbers and state names do not match
>     any documented enumerations for the IANAPowerStateSet textual convention.
>     Should the existing values be replaced with:
>
>             eman(1024),       -- indicates EMAN set
>             emanmechoff(1025),
>             emansoftoff(1026),
>             emanhibernate(1027),
>             emansleep(1028),
>             emanstandby(1029),
>             emanready(1030),
>             emanlowMinus(1031),
>             emanlow(1032),
>             emanmediumMinus(1033),
>             emanmedium(1034),
>             emanhighMinus(1035),
>             emanhigh(1036)
>
>
> Page 45, "eoEnergyStored":
>
>     The first sentence of the DESCRIPTION clause reads:
>
>     "This object indicates the resultant of the energy consumed..."
>
>     Perhaps the following text would be clearer:
>
>     "This object indicates the difference of the energy consumed..."
>                                ^^^^^^^^^^
>
>
> Page 46, "eoEnergyMaxConsumed":
>
>     The DESCRIPTION text reads:
>
>         "This object is the maximum energy ever observed in
>         eoEnergyConsumed since the monitoring started. This value
>         is specified in the common billing units of watt-hours
>         with the magnitude of watt-hours (kW-Hr,   MW-Hr, etc.)
>         indicated separately in eoEnergyUnitMultiplier."
>
>     Does this mean that the eoEnergyMaxConsumed value must be retained
>     across reboots, and thus must be stored in non-volatile memory?
>
>
> Page 48/49/54, where "entity4CRCompliance" is listed:
>
>     I believe the "entPhysicalClass" object must be added to the lists of
>     MIB objects (entPhysicalIndex,  entPhysicalName and entPhysicalUUID).
>
>
> Page 56, "eoACPwrAttributesAvgCurrent":
>
>     It might be niced to expand the DESCRIPTION clause so that it more
>     precisely specifies the significance of this, perhaps similar to the
>     DESCRIPTION clause for "eoACPwrAttributesAvgVoltage".
>
>
> Page 56, "eoACPwrAttributesFrequency":
>
>     The UNITS clause does not match the comment after the SYNTAX clause.
>
>          eoACPwrAttributesFrequency OBJECT-TYPE
>              SYNTAX          Integer32 (4500..6500) -- UNITS 0.01 Hertz
>              UNITS           "hertz"
>
>
> Would it be useful or helpful to have UnitsMultiplier companion objects for
> the following objects?
>
>     Page 58, "eoACPwrAttributesThdAmpheres":
>     Page 60, "eoACPwrAttributesPhaseAvgCurrent":
>     Page 62, "eoACPwrAttributesDelPhaseToNextPhaseVoltage":
>     Page 63, "eoACPwrAttributesWyePhaseToNeutralVoltage":
>     Page 64, "eoACPwrAttributesWyePhaseCurrent":
>                       
>
> Page 59, "eoACPwrAttributesPhaseEntry":
>     
>     Under the DESCRIPTION clause, it might be helpful to add a comment that
>     indicates the phase-to-phase voltages are in separate tables.
>
>     When I first encountered the "eoACPwrAttributesPhaseEntry", I wondered
>     about the voltages until I read further in the MIB and found the
>     eoACPwrAttributesDelPhaseTable and the eoACPwrAttributesWyePhaseTable.
>
>
>
> Page 60, "eoACPwrAttributesPhaseActivePower",
>           "eoACPwrAttributesPhaseReactivePower",
>           "eoACPwrAttributesPhaseApparentPower":
>
>     Expanding the DESCRIPTION clauses to note these are scaled by the
>     eoACPwrAttributesPowerUnitMultiplier might be helpful, but the
>     DESCRIPTION clause for the "eoACPwrAttributesPowerUnitMultiplier"
>     does note this scaling relationship.
>
>
> Page 61, "eoACPwrAttributesPhaseImpedance":
>
>     Are the UNITS of "volt-amperes" correct?
>
>
>
> Page 62/63, "eoACPwrAttributesDelPhaseToNextPhaseVoltage",
>              "eoACPwrAttributesDelThdPhaseToNextPhaseVoltage",
>              "eoACPwrAttributesDelThdCurrent":
>
>     The last SID values for these objects are 2, 3, and 4, which distinctly
>     does NOT match the values shown in the figure on Page 9, which shows
>     the last SID values as 1, 2, and 3.
>
>
> Page 74, Section 13:
>
>          [EMAN-AWARE-MIB] J. Parello, B. Claise and M. Chandramoili,
>                  "draft-ietf-eman-energy-aware-mib-11 ", work in
>                  progress, November 2013.
>
>     Does it matter, or should this reference draft 13,
>     draft-ietf-eman-energy-aware-mib-13?
>
>
> Regards,
> --Alan
>
>   ------------------------------------------------------------------------------
>   Alan Luchuk               SNMP Research, Inc.          Voice:  [phone]
>   Senior Software Engineer  3001 Kimberlin Heights Road  FAX:    [phone]
>   luchuk at snmp.com        Knoxville, TN  37920-9716    http://www.snmp.com/
>   ------------------------------------------------------------------------------
>
> _______________________________________________
> eman mailing list
> [e-mail]
> https://www.ietf.org/mailman/listinfo/eman
> .
>