Subject: RE: [Hubmib] My review of: draft-ietf-hubmib-efm-cu-mib-06.txt

From: "Edward Beili" <[email-masked]>
Date: Mon, 19 Feb 2007 04:10:50 +0200

Bert,

I've attached the next version of the efm-cu-mib draft, together with the extracted MIB modules.
This version implements most of your comments, see below for the full list of changes.

Regards,
-E.

> From: Wijnen, Bert (Bert) 
> [mailto:[email-masked]]
> Sent: Thursday, January 18, 2007 21:45
> To: Edward Beili
> Cc: Dan Romascanu (E-mail)
> Subject: RE: [Hubmib] My review of: draft-ietf-hubmib-efm-cu-mib-06.txt

>My appology, "this week" took a bit longer than I had expected. 
>
>So here is my review as WG chair.
>
>- SMICng:
>  W: f(EFM-CU-MIB.mi2), (300,21) Item "efmCuPAFDiscoveryCode" should have SIZE specified
>  W: f(EFM-CU-MIB.mi2), (1178,21) Item "efmCuPAFRemoteDiscoveryCode"
should have SIZE specified
>  W: f(EFM-CU-MIB.mi2), (2925,23) For "efmCuPmeSubTypesSupported", syntax is identical
>
>  for the first two, I think: SIZE(6)
>  for the 3rd one I am OK to let the warning go., i.e. ignore it.

[EB] Corrected as suggested.

>  W: f(IF-CAP-STACK-MIB.mi2), (195,17) Row "ifInvCapStackEntry" does not
>     have a consistent indexing scheme - index items must be in same order
>     as used in INDEX clause for "base row" ifStackEntry
>  E: f(IF-CAP-STACK-MIB.mi2), (291,13) Item "ifInvStackGroup" should be IMPORTed

>  Warning is OK I think, but pls fix error.

[EB] Corrected as suggested.

>- You may want to add a note-to-rfcs-editor to:
>  [I-D.ietf-hubmib-efm-mib]
>             Squire, M., "Definitions and Managed Objects for OAM
>             Functions on Ethernet Like Interfaces",
>             draft-ietf-hubmib-efm-mib-04 (work in progress),
>             March 2006.
>
>  [I-D.ietf-hubmib-rfc3636bis]
>             Beili, E., "Definitions of Managed Objects for IEEE 802.3
>             Medium Attachment Units (MAUs)",
>             draft-ietf-hubmib-rfc3636bis-05 (work in progress),
>             July 2006.
>
>  in which you ask them to replace with actual RFC numbers if those
>  are available at time of publication.

[EB] Done.

>- note that if you do a revision, you must use new copyright
>  and no longer 
>   Copyright (C) The Internet Society (2006).
>  See my earlier post to the hubmib list

[EB] Done.

>- Did we resolve the use of Rowstatus for the ifCapStackTable
>  and ifInvCapStackTable? In any event, pls re-check the
>  feedback we've got on that. I do not think that what we
>  currently have in the MIB module is acceptable.

[EB] Replaced with TruthValue.

>- In order to avoid any possible future name clashed, I would
>  prefer it if we rename Textual Convnetions:
>
>     ProfileIndex ::= TEXTUAL-CONVENTION
>     ProfileIndexOrZero ::= TEXTUAL-CONVENTION
>     ProfileIndexList ::= TEXTUAL-CONVENTION
>     TruthValueOrUnknown ::= TEXTUAL-CONVENTION
>
>  such that they are prefixed with Efm, so I would name them:
>
>     EfmProfileIndex ::= TEXTUAL-CONVENTION
>     EfmProfileIndexOrZero ::= TEXTUAL-CONVENTION
>     EfmProfileIndexList ::= TEXTUAL-CONVENTION
>     EfmTruthValueOrUnknown ::= TEXTUAL-CONVENTION
>
>  this is also common practice in most (if not all) other
>  IETF MIB modules these days

[EB] Done.

>- for efmCuPAFAdminState you describe a few operations that
>  should be ignore when tried. I am not sure how to inerpret
>  that and what would happen when a SNMP SET is received.
>  I would think it is better to say the need to be rejected.
>  (that is, cause an error on an SNMP SET).

[EB] Agreed. All 'ignored' are replaced with 'rejected'

>- for 
>     efmCuPAFDiscoveryCode  OBJECT-TYPE
>       SYNTAX      PhysAddress
>       MAX-ACCESS  read-write
>       STATUS      current
>       DESCRIPTION
>         "PAF Discovery Code of the EFMCu port (PCS).
>         A unique 6 Byte long code used by the Discovery function, when
>         PAF is supported.
>         PCS ports incapable of supporting PAF SHALL return a value of
>         all zeroes. Attempts to change this object SHALL be ignored in
>         this case.
>         This object MUST be instantiated for the -O subtype PCS before
>         writing operations on the efmCuPAFRemoteDiscoveryCode
>         (Set_if_Clear and Clear_if_Same) are performed by PMEs
>         associated with the PCS.
>         The value of this object is read-only for -R port subtypes.
>         The initial value of this object for -R ports after reset
>         is 0. This value may be changed as a result of writing
>         operation on efmCuPAFRemoteDiscoveryCode variable of remote
>         PME of -O subtype, connected to one of the local PMEs
>         associated with the PCS.
>
>         Discovery MUST be performed when the link is Down.
>         Attempts to change this object MUST be rejected with the error
>         inconsistentValue if the link is Up or Initializing.
>
>         The PAF Discovery code maps to the local Discovery code
>         variable in PAF (note that it does not have a corresponding
>         Clause 45 register)"
>       REFERENCE
>         "[802.3ah] 61.2.2.8.3, 61.2.2.8.4, 45.2.6.6.1"
>       ::= { efmCuPortConfEntry 2 }
>
>  I am trying to figure out what made you choose PhysAddress as the
>  SYNTAX for the discovery code. Can you explain, or point me to
>  the 802.3ah clause that explains/justifies that?

[EB] The remote discovery code is defined by 802.3ah as a 6-octets (48-bit)
value, see 45.2.6.8. Clause 61A.2 actually suggests to use a MAC
address for the discovery operation, in order to ensure its 'local uniqueness'.
Since PhysAddress represents media- or physical-level addresses it was
a natural choice. I've added the size attribute to indicate it's length. 

>  I wonder what you mean with "the value of this object is read-only"
>  The value has to be a PhysAddress ( 6 octets) value, no?
>  If you mean that for -R port subtypes the object cannot allow write
>  access, then I would say it in terms aka:
>
>         The initial value of this object for -R port subtypes after
>         reset is all zeroes. For such -R ports, the value of this
>         object cannot be changed directly. The value may be changed
>         as a result of a writing operation on the 
>         efmCuPAFRemoteDiscoveryCode object of a remote PME of 
>         -O subtype, connected to one of the local PMEs
>         associated with the PCS.
>
>  I hope I understood the intention and reworded it properly.

[EB] Corrected as suggested.

>  These day we prefer to not hard-wire SNMP error codes in DESCRIPTION
>  clause, or if we do, then as an example (A MIB could be used by non
>  SNMP protocols). SO I suggest 
>  
>         Discovery MUST be performed when the link is Down.
>         Attempts to change this object MUST be rejected (in case of
>         SNMP with the error  inconsistentValue) if the link is Up 
>         or Initializing.

[EB] Corrected - inserted '(in case of SNMP' to all occurrences of
'with the error'.

>- For efmCuAdminProfile I read in DESCRIPTION clause:
>
>          This object is writable and readable for the -O subtype
>          (2BaseTL-O or 10PassTS-O) EFMCu ports. It is unavailable for
>          the -R  subtype (2BaseTL-R or 10PassTS-R) ports.
>
>  what does it mean that this object is "unavailable"
>  Do you not want it instatntiated in that case, or do you want
>  its value to be ignored/irrelevant? Pls be specific.
>
>  You use that at various other places as well. Pls check and fix.

[EB] Replaced 'unavailable' with 'unsupported'

>- For
>     efmCuPAFRemoteDiscoveryCode  OBJECT-TYPE
>       SYNTAX      PhysAddress
>       MAX-ACCESS  read-write
>       STATUS      current
>       DESCRIPTION
>         "PAF Remote Discovery Code of the PME port at CO.
>         A 6 Byte long Discovery Code of the peer PCS connected via
>         the PME.
>         Reading this object results in a Discovery Get operation.
>         Writing a zero to this object results in a Discovery
>         Clear_if_Same operation (the value of efmCuPAFDiscoveryCode
>         at the peer PCS SHALL be the same as efmCuPAFDiscoveryCode of
>         the local PCS associated with the PME for the operation to
>         succeed).
>         Writing a non-zero value to this object results in a
>         Discovery Set_if_Clear operation.
>         This object does not exist in CPE port subtypes. A zero length
>         octet string SHALL be returned for CPE port subtypes and also
>         when PAF aggregation is not enabled.
>
>  What is "writing a zero value" ??
>  Is that 6 octets, all zeroes?
>  Or one octet conatining zero,
>  or the zero length octet-string?
>  What means "does not exist" is it not instanctiated? I guess so.
>  pls be clear. You do get gaps in the tables if some objects are
>  not instantiated. Might be easier (on NM apps) if there was
>  a value (like all zeros, or zerolength string) that can be ignored.

[EB] Reworded as follows:
       This object is irrelevant in CPE port (-R) subtypes: in this
       case a zero length octet string SHALL be returned on an
       attempt to read this object, writing to this object SHALL
       be ignored.

>- I believe I have seen this SYNTAX
>       SYNTAX      INTEGER {
>         ieee2BaseTLO(1),
>         ieee2BaseTLR(2),
>         ieee10PassTSO(3),
>         ieee10PassTSR(4)
>       }
>  several times. Candidate for a Textual Convention?

[EB] No actually all occurences are different- there are:
efmCuPmeAdminSubType with 7 enums,
efmCuPmeSubTypesSupported with 4 bitmap positions and
efmCuPmeOperSubType with 4 enums.

>- for efmCuPme2BRegion I wonder if we ever (soon?) expect more
>  regions? If so, maybe we ought to make this a TC?
>  How will regions be added?

[EB] Regions for G.SHDSL are defined by the ITU-T in G.991.2 recommendation.
The last revision of this document was done in 2003, and this version
was used as a reference for the IEEE 802.3ah 2BASE-TL specification.
There's no reason to believe that a new region would ever be defined
(unless Japanese decide to play hard and revive this issue:), in any
case even if this happens it would require a re-issue of 802.3 as the
regions are hard-coded in a Clause 45 register. 

>- For efmCuPme2BProfileRowStatus I wonder if any (all?) of the objects
>  in a row can be changed while the row is active? Would that not
>  be disruptive? In any event, pls specify if any (which( objects
>  can be changed or stat eif none can be change in active state.

[EB] I believe that 'read-create' in MAX-ACCESS clause of all the
objects in a row (except for the 'not-accessible' index) already
indicates that in order to change an object in a particular profile,
one would have to create a new one, change the reference(s) from the
old profile to a new one and remove the old (inactive) profile if not
needed anymore.

>  Same question for efmCuPme2BsModeRowStatus, although there a change
>  is probably not disruptive.
>  Pls check all occurences of RowStatus

[EB] Same answer as above.

>- I really wonder if we are doing a smart thing by give the same labels
>  to the various enums in
>          efmCuPme10PBandplanPSDMskProfile  INTEGER,
>          efmCuPme10PUPBOReferenceProfile   INTEGER,
>          efmCuPme10PBandNotchProfiles      BITS,
>          efmCuPme10PPayloadURateProfile    INTEGER,
>          efmCuPme10PPayloadDRateProfile    INTEGER,
>  They have different meanings, don't they? So would it not be better
>  to differentiate between the labels of the neumerations?

[EB] If we take it to extreme why not turning all enums to TCs?
Personally I prefer inline enums (if they are not repeated of course
which would make they perfectly good candidates for TCs) as they
list the values and their meaning in the description for the object -
less jumps between the object and a TC when reading the MIB.

>- For OBBJECT-GROUP definitions, one is NOT supposed to state requirements
>  or optional attributes. Such things (if a group if required or
>optional)
>  are stated in the MODULE-COMPLIANCE.
>  The OBJECT0GROUP macro is ONLY intended to group objects that logically
>  fit together in order to model something.
>  Sof (for example):
>      efmCuBasicGroup OBJECT-GROUP
>        OBJECTS {
>          efmCuPAFSupported,
>          efmCuAdminProfile,
>          efmCuTargetDataRate,
>          efmCuTargetSnrMgn,
>          efmCuAdaptiveSpectra,
>          efmCuPortSide,
>          efmCuFltStatus
>        }
>        STATUS      current
>        DESCRIPTION
>          "A collection of objects required for all of EFMCu ports."
>        ::= { efmCuGroups 1 }
>
>  Sould NOT state that these objects are "required". I would phrase
>  it as:
>          "A collection of objects reperesenting management information
>           that is common for all of EFMCu ports."
>
>  The fact that is is REQUIRED for all EFMCu ports is expressed in
>      efmCuCompliance MODULE-COMPLIANCE
>        STATUS      current
>        DESCRIPTION
>           ... snip ...
>        MODULE  -- this module
>          MANDATORY-GROUPS {
>            efmCuBasicGroup,
>  That is, here it is listed in the MANDATORY-GROUPS clause and so that
>  inidicated that it is required.
>  
>  Pls try to rephrase for all OBJECT-GROUPs

[EB] Done.

>- With regard to naming, I am somewhat worried about the conventions that
>  are used (or maybe those that are not used). I always find it smarter
>  to have all columnar objects in a table prefixed with a string that
>  makes it clear to which table the object belongs. For example for
>  the efmCuPortConfTable:
>
>     EfmCuPortConfEntry ::=
>       SEQUENCE {
>         efmCuPAFAdminState               INTEGER,
>         efmCuPAFDiscoveryCode            PhysAddress,
>         efmCuAdminProfile                ProfileIndexList,
>         efmCuTargetDataRate              Unsigned32,
>         efmCuTargetSnrMgn                Unsigned32,
>         efmCuAdaptiveSpectra             TruthValue,
>         efmCuThreshLowRate               Unsigned32,
>         efmCuLowRateCrossingEnable       TruthValue
>       }
>  
>  I would personally prefer the columns to be alll prefixed with
>  efmCuPortConf, so I would have named them as follows:
>
>     EfmCuPortConfEntry ::=
>       SEQUENCE {
>         efmCuPortConfPAFAdminState               INTEGER,
>         efmCuPortConfPAFDiscoveryCode            PhysAddress,
>         efmCuPortConfAdminProfile                ProfileIndexList,
>         efmCuPortConfTargetDataRate              Unsigned32,
>         efmCuPortConfTargetSnrMgn                Unsigned32,
>         efmCuPortConfAdaptiveSpectra             TruthValue,
>         efmCuPortConfThreshLowRate               Unsigned32,
>         efmCuPortConfLowRateCrossingEnable       TruthValue
>       }
>
>  This also holds true for most (all of) the other tables.
>  My suggested naming convention above has (in my view) 2 advantages:
>  
>    - less change for conflicting names. 
>      I will admit, that this is not too big a risk, since this is
>      all within the same MIB module. Nevertheless, I think it just
>      makes it easier if any additions need to be made in the future.
>    - easier for people to see which objects belong to which tables.
>      I think this is a strong point. But I also understand it is
>      somewhat subjective.
>
>   I can accept if the WG and/or editor/author tells me that I am far
>   too late with this type of comment. It of course requires a quite 
>   massive editorial change. So, although I think it would improve
>   the human-friendlyness of the MIB module, I will not insist on
>   this change.

[EB] While I tend to agree with the advantages of the naming scheme
you proposed, I would like to ask to be forgiven and leave the names
as they are now, due to the following rasons:
- It is a big change (considering also that there are
at least 2 experimental implementations of this draft that I know of)
- Some time ago I was asked to stick with the RFC 2578/2579
recommendation of 32 char restriction - the proposed change would make
some names to be longer than 32 chars. (Yes, I just read RFC 4181 view
on that - the 32 character restriction recommendation of SMIv2 SHOULD
be set aside in favor of promoting clarity and uniqueness). However it
could be argued that it is easier to remember
efmCuLowRateCrossingEnable vs. efmCuPortConfLowRateCrossingEnable

>NITS:
>
>- the RFC editor probably wants you to expand acronyms when they are
>  used for the first time. Pls check you do it for all acronyms.
>  I found for example VDSL, DMT, SHDSL acronyms which have not
>  been expanded.
>  There may be others. Pls check.

[EB] All acronyms are expanded.

>- For:
>      ifCapStackEntry  OBJECT-TYPE
>        SYNTAX      IfCapStackEntry
>        MAX-ACCESS  not-accessible
>        STATUS      current
>        DESCRIPTION
>          "Information on a particular relationship between two
>          sub-layers, specifying that one sub-layer runs on 'top' of the
>          other sub-layer. Each sub-layer corresponds to a conceptual
>
>  I wonder if we should: s/runs on top/can run on 'top'/
>  After all, it is a capability, which not necessarily is used, right?

[EB] Right, replaced to "MAY possibly run on top".

>- for efmCuPAFDiscoveryCode you speak about a "6 Byte code"
>  If this is common 802.3 terminology, then I guess it is OK.
>  I know that some people in the IETF prefer to use 'octet' instead
>  of 'byte'.

[EB] Corrected to "octets".

>- This table
>     efmCuPmeStatusTable OBJECT-TYPE
>       SYNTAX      SEQUENCE OF EfmCuPmeStatusEntry
>       MAX-ACCESS  not-accessible
>       STATUS      current
>       DESCRIPTION
>         "This table provides common status information of EFMCu
>         2BASE-TL/10PASS-TS PME ports. Status information specific
>         to 10PASS-TS PME is represented in efmCuPme10PStatusTable.
>
>         This table contains live data from the equipment. As such,
>         it is NOT persistent."
>       ::= { efmCuPme 3 }
>
>  Is cmposed of just read-only objects. So the last sentence of the
>  DESCRIPTION clause is not needed. In general people expect (I think)
>  writable objects when they see such a ststament.

[EB] You are right. However when one is looking at the description
clause of a 'not-accessible' table, the only way to determine if it is
composed of read-only objects is to check the Max access value of
each and every object in the table. I believe that the 'NOT persistent'
sentence helps reader to understand the nature of a table quicker, so
I would leave it as is.

>- For efmCuPme2BProfileDescr I think I would change the uppercase MAY
>  to a lowercase may.
>  Same for efmCuPme10PProfileDescr.
>  There may be other places. Pls check RFC2119 to understand how/when
>  capilaized terms are needed. They are for COMPLIANCE, the two
>  objects above have no special compliance requirments for content
>  except for adhering to the SYNTAX, right?

[EB] right, changed to lowercase 'may', Also changed in
efmCuPme2BsModeDescr.

>TYPOs:
>- typo in table 1:
>  | ifIndex       | Interface index. Note that each PME and each PCS  |
>  |               | in the EFMCu PHY MUST have a unique index, as     |
>  |               | there some PCS and PME specific attributes        |
>  |               | accessible only on the PCS or PME level.          |
> 
>  s/there some/there are some/ I think

[EB] Corrected.

>- 3rd para of sect 4.3:
>   A specific configuration or administrative profile is assigned to a
>   specific PME via efmCuPmeAdminProfile object.  If
>   efmCuPmeAdminProfile is zero, then efmCuAdminProfile object of the
>   PCS port, connected to the PME, determines the configuration profile
>   (or a list of possible profiles) for that PME.  This mechanism allows
>   to specify a common profile(s) for all PMEs connected to the PCS
>   port, with an ability to change individual PME profiles by setting
>   efmCuPmeAdminProfile object, which overwrites profile set by
>   efmCuAdminProfile.
>
>  s/overwrites profile set by/overwrites the profile set by/
>  I think.

[EB] Corrected. 

> -----Original Message-----
> From: Wijnen, Bert (Bert) [mailto:[email-masked]]
> Sent: Wednesday, October 18, 2006 5:45 PM
> To: Hubmib Mailing List (E-mail)
> Subject: [Hubmib] My review of: draft-ietf-hubmib-efm-cu-mib-06.txt
> 
> My WG LC review comments are here.
> For the EFM-U-MIB itself I will do a separate posting to the list.
> 
> - Abstract 3rd line
> 
>     This document proposes an extension to the Ethernet-like 
> Interfaces
> 
>   By the time we get published as RFC, we no longer "propose".
>   So how about:
> 
>     This document describes extensions to the Ethernet-like
> Interfaces

[EB] Corrected.

> - Page 7, a nit:
>            // pottentially be connected to the PCS
>   s/pottentially/potentially/
>            // for PCS[i] and there room for another PME in the
>   s/there room/there is room/ ??

[EB] Corrected
 
> - A nit/typo on page 10:
>    | ifIndex       | Interface index. Note that each PME and 
> each PCS  |
>    |               | in the EFMCu PHY MUST have a unique 
> index, as     |
>    |               | there some PCS and PME specific 
> attributes        |
>    |               | accessible only on the PCS or PME level. 
>          |
> 
>   s/as there some/as there are some/ ??

[EB] Corrected

> - a type in 2nd para in sect 4.2
> 
>    The PME profiles are defined in efmCuPme2BProfileTable and
>    efmCu10PProfileTable for 2BASE-TL and 10PASS-TS PMEs respectively.
> 
>   I think: s/efmCu10PProfileTable/efmCuPme10PProfileTable/
>   So insert "Pme"

[EB] Corrected.

> - SMICng IF-CAP-STACK-MIB tells me:
> 
>     W: f(IF-CAP-STACK-MIB.mi2), (195,17) Row "ifInvCapStackEntry"
>        does not have a consistent indexing scheme - index items must
>        be in same order as used in INDEX clause for "base row"
>        ifStackEntry
> 
>   The above is OK, it is an INVERTED table.
>   So we can ignore the warning.
> 
>     E: f(IF-CAP-STACK-MIB.mi2), (291,13) Item "ifInvStackGroup"
>        should be IMPORTed
> 
>   I think that error should be fixed and we shoul dimport that group.

[EB] Already done.
 
> - SMICNg EFM-CU-MIB tells me:
> 
>     W: f(EFM-CU-MIB.mi2), (300,21) Item "efmCuPAFDiscoveryCode" should
>        have SIZE specified
> 
>    Do we know a reasonable size for this? In the pseudo code on 
> pages 7/8
>    it speaks about a 6-byte (octet) code. And so does the DESCRIPTION
>    clause. Is it always fixed to 6 octets?
>    Now and in future? If so, I would suggest to use
> 
>         SYNTAX      PhysAddress (SIZE(6))
> 
>     W: f(EFM-CU-MIB.mi2), (1178,21) Item "efmCuPAFRemoteDiscoveryCode"
>        should have SIZE specified
> 
>    Same story for the above.

[EB] Already done.

>     W: f(EFM-CU-MIB.mi2), (2925,23) For "efmCuPmeSubTypesSupported",
>        syntax is identical
> 
>    This is probably OK, although it looks a bit weird:
>           OBJECT      efmCuPmeSubTypesSupported
>           SYNTAX      BITS {
>             ieee2BaseTLO(0),
>             ieee2BaseTLR(1),
>             ieee10PassTSO(2),
>             ieee10PassTSR(3)
>           }
>           DESCRIPTION
>             "Support for all subtypes is not required. 
> However at least
>             one value SHALL be supported"

[EB] Left as is.

> - W.r.t. the SYNTAX of objects ifCapStackStatus and 
> ifInvCapStackStatus
>   I think it would be much better to use a SYNTAX of TruthValue.
>   See my separate posting on this topic to the HubMIB WG list.

[EB] Agreed to use TruthValue. Added a note that the value of false
 (theoreticallypossible but temporary out of service), should not be
confused with a non-existent row (absolutely impossible).

> - I believe that instead of
> 
>       ifCapStackConformance OBJECT IDENTIFIER
>       ::= { ifCapStackObjects 3 }
> 
>   It would be better to adhere to the strcuture suggested by
> RFC4181 page 38
>   and so use instead:
> 
>      ifCapStackConformance  OBJECT IDENTIFIER ::= { ifCapStackMIB 2 }

[EB] Agreed, restructured according to RFC4181, Appendix D: Suggested OID Layout

> - In the Security Considerations section I see capitalized MAY
>   which I think is not what RFC2119 intended. Unless you can 
>   explain to me why this capilaized MAY makes sense, I would
>   prefer if we change it to just lowercase "may" for all
>   occurences in the Security Considerations section.

[EB] All capitalized MAY in the "Security Considerations" section are
changed to lowercase "may"

> - 2nd para on page 83 (a NIT):
>     Even if the network itself is secure (for example by using IPSec),
>   pls change "IPSec" into "IPsec", that is a lower case "s".
>   The current MIB security template has it fixed. I know that the
>   Security ADs want/prefer the proper spelling.

[EB] Done.

> - IANA Considerations.
>   Since you state that some values SHALL be defined in the
>   IANA-MAU-MIB, I think that you make rfc3636bis a normative
>   reference, while it is now listed under informative. And by making
>   that document normative, you/we automagically make sure that this
>   efmCuMIB will not get published before 3636bis.
> 
>   You must also request/ask (in IANA COnsiderations section) that 
>   IANA assigns two new OID branches for
> 
>      ifCapStackMIB ::= { mib-2 ZZZ }
> 
>      efmCuMIB  ::= { mib-2 YYY }

[EB] Corrected

> - If/wehen we do a revision, we must adhere to new boilerplate.
>   That means we must change all occurences of:
> 
>      Copyright (C) The Internet Society (2006).
> 
>   into
> 
>      Copyright (C) The IETF Trust (2006).
> 
>   If you are using xml2rfc, thsi can be achieved with:
> 
>         ipr="full3978update"
> 
>   You can/could not yet know this. I know that this starts on Nov 1st
>   (because I was asked to update the IDChecklist.html.

[EB] Corrected

[EB] Additional changes:
- Replaced Dan Romascanu with Bert Wijnen as the WG chair
- Fixed a typo in the compliance statement for efmCuPmeAdminSubType:
ieee2BaseTSR -> ieee10PassTSO

----------------------------------------------------------------------------------
The following email exchange is provided here for historical purposes
 
> -----Original Message-----
> From: Wijnen, Bert (Bert) [mailto:[email-masked]] 
> Sent: Wednesday, October 18, 2006 17:45
> To: Edward Beili
> Cc: [email-masked]; Keith McCloghrie; [email-masked]
> Subject: RE: [Hubmib] WGLC for: draft-ietf-hubmib-efm-cu-mib-06.txt
> 
> Having looked at the MIB itself now, it seems to me that the object
>       ifCapStackStatus  OBJECT-TYPE
>         SYNTAX      RowStatus
> would be much better represented by 
>       ifCapStackPossible  OBJECT-TYPE
>         SYNTAX      TruthValue
> where 'true' means it CAN be stacked this way and 'false' 
> means it cannot. That way you re-use an existing TC, and you 
> can exactly represent what you need, no?
> 
> Same for ifInvCapStackStatus
> 
> Bert
> > -----Original Message-----
> > From: Keith McCloghrie [mailto:[email-masked]]
> > Sent: Wednesday, October 18, 2006 16:41
> > To: [email-masked]
> > Cc: [email-masked]; [email-masked]; [email-masked]; 
> > [email-masked]
> > Subject: Re: [Hubmib] WGLC for: draft-ietf-hubmib-efm-cu-mib-06.txt
> > 
> > 
> > > The reason I changed the syntax of ifCapStackStatus and 
> > > ifInvCapStackStatus objects from a new enum to the 
> RowStatus was to 
> > > emphasize the similarity between the
> > ifCapStackTable/ifInvCapStackTable
> > > and ifStackTable/ifInvStackTable as well as to try to reuse
> > an existing
> > > textual convention instead of inventing yet another enum.
> >  
> > The motivation is good but it's only valid if the semantics match.
> > 
> > > I don't think making the 
> ifCapStackStatus/ifInvCapStackStatus to be 
> > > read-only is illegal or problematic - the ifInvStackStatus
> > object from
> > > IF-INV-STACK-MIB with a RowStatus syntax is read-only as well.
> >  
> > The ifInvStackTable table has exactly the same structure 
> and content, 
> > and exactly the same number of rows as the ifStackTable.
> > Semantically,
> > ifInvStackStatus *could* be read-create, but if it were, a 
> SetRequest 
> > to ifStackStatus and a SetRequest to ifInvStackStatus would have 
> > exactly the same result, i.e., to make it read-create would be 
> > redundant because it would provide two means of achieving the same 
> > behaviour.  So, making it read-only is a simplification.
> > 
> > > I would also argue that overloading of NotInService value
> > is ok - this
> > > value is defined as:
> > > "- 'NotInService', which indicates that the conceptual row
> > exists in the
> > > agent, but is unavailable for use by the managed device".
> > 
> > 'notInService' is "unavailable for use by the managed 
> device" because 
> > it normally requires an SNMP operation on a RowStatus 
> object to make 
> > it available.  Normally, the necessary operation is on the same 
> > RowStatus object but in the case of ifInvStackStatus (see above 
> > comment on simplification), the operation needs to be on 
> > ifStackStatus.  In contrast,
> > 
> >              notInService(2) - the sub-layer interfaces cannot be
> >                               connected temporarily due to
> >                               unavailability of the interface(s),
> > 
> > The semantics are different here.  It is not an SNMP operation on a 
> > RowStatus object which prevents this row from being 
> "available for use 
> > by the managed device".
> > 
> > > Since these tables are static (read-only) there is no question of 
> > > resource consumption - these rows are never deleted and
> > thus can stay in
> > > NotInService state indefinitely. RFC2579 states that "It is the 
> > > responsibility of the DESCRIPTION clause of the status column to 
> > > indicate what an abnormally long period of time would be."
> > I can add a
> > > sentence in the DESCRIPTION clause saying that an 
> > > ifCapStackStatus/ifInvCapStackStatus may be in the
> > NotInService state
> > > indefinitely, formally complying with RFC2579.
> >  
> > ifInvStackStatus *could* semantically be read-create, but 
> is read-only 
> > so as to remove redundancy.  In contrast, it is the semantics of 
> > ifCapStackStatus and ifInvCapStackStatus which prevent them 
> from being 
> > read-create.
> > 
> > Keith.
> > 
> > > However I'm not bent on staying with RowStatus - I'll move
> > back to the
> > > enum if the group feels it is best.
> > > 
> > > Regards,
> > > -E.
> > > 
> > >   _____
> > > 
> > > > From: Clay Sikes [mailto:[email-masked]]
> > > > Sent: Tuesday, October 17, 2006 23:11
> > > > To: [email-masked]
> > > > Cc: [email-masked]
> > > > Subject: Re: [Hubmib] WGLC for: 
> > draft-ietf-hubmib-efm-cu-mib-06.txt
> > > 
> > > 
> > > > Hi,
> > > 
> > > > I have run into an issue with the ifCapStackStatus and
> > > ifInvCapStackStatus objects.  Although the ifCapStackMIB
> > passes smilint,
> > > I have seen compilation issues with these objects.  It
> > seems that the
> > > root cause is the combination RowStatus  syntax and the read-only 
> > > max-access (A RowStatus that is read-only seems a bit
> > strange to me).  
> > > The compiler does not generate all the code that is
> > required.  Although
> > > there may or may not be an issue with the compiler, I would
> > beg that the
> > > objects be changed from a RowStatus syntax to a enumeration
> > like what
> > > was in the -05 version of the ID.  This may resolve Keith's
> > issue as
> > > well.
> > > 
> > > > Thoughts?
> > > 
> > > > Thanks,
> > > > Clay
> > > 
> > > > On 10/17/2006 12:09 PM, Keith McCloghrie wrote:
> > > 
> > > > > > This is a formal WG Last Call for
> > > > > >   
> http://www.ietf.org/internet-drafts/draft-ietf-hubmib-efm-cu-m
ib-06.txt
> > > >    
> 
> > > 
> > >       ifCapStackStatus  OBJECT-TYPE
> > >         SYNTAX      RowStatus
> > >         MAX-ACCESS  read-only
> > >         STATUS      current
> > >         DESCRIPTION
> > >           "The status of the 'cross-connect capability' relationship
> > >           between two sub-layers. The following values can be returned:
> > >             active(1)       - indicates that the sub-layer interface,
> > >                               identified by the ifStackLowerLayer MAY
> > >                               be connected to run 'below' the sub-layer
> > >                               interface, identified by the
> > >                               ifStackHigherLayer index.
> > >             notInService(2) - the sub-layer interfaces cannot be
> > >                               connected temporarily due to
> > >                               unavailability of the interface(s), e.g.
> > >                               one of the interfaces is located on a
> > >                               pluggable module which is absent.
> > 
> > > I suggest this is an ill-advised overloading of 'notInService'.
> > > RFC 2579 says:
> > 
> > >             If the management station is prevented from setting the
> > >             status column to `active' (e.g., due to management station
> > >             or network failure) the conceptual row will be left in the
> > >             `notInService' or `notReady' state, consuming resources
> > >             indefinitely.  The agent must detect conceptual rows that
> > >             have been in either state for an abnormally long period of
> > >             time and remove them.  It is the responsibility of the
> > >             DESCRIPTION clause of the status column to indicate what an
> > >             abnormally long period of time would be.  This period of
> > >             time should be long enough to allow for human response time
> > >             (including `think time') between the creation of the
> > >             conceptual row and the setting of the status to `active'.
> > >             In the absence of such information in the DESCRIPTION
> > >             clause, it is suggested that this period be approximately 5
> > >             minutes in length.  This removal action applies not only to
> > >             newly-created rows, but also to previously active rows which
> > >             are set to, and left in, the notInService state for a
> > >             prolonged period exceeding that which is considered normal
> > >             for such a conceptual row.
> > 
> > > In other words, the 'notInService' of RowStatus is for a temporary 
> > > delay in a management station setting it to `active'.  The "temporary"
> > > situation of a "pluggable module which is absent" is liable to be 
> > > much longer than the 5 minutes before the agent is required to 
> > > delete the row.
> > 
> > > Keith.
_______________________________________________
Hubmib mailing list
[email-masked]
https://www1.ietf.org/mailman/listinfo/hubmib

---

From: "Wijnen, Bert \(Bert\)" <[email-masked]>
Date: Tue, 20 Feb 2007 17:14:04 +0100

Thanks for the revision Ed,

I still have some comments though.

- Not sure if I mentioned this before. But anyway.
  I see:

      ifCapStackCompliance MODULE-COMPLIANCE
        STATUS      current
        DESCRIPTION
          "The compliance statement for SNMP entities, which provide
          information on the cross-connect capability of multi-layer
          (stacked) network interfaces, with flexible cross-connect
          between the sub-layers.
!          Compliance with the following external compliance statements
!          is REQUIRED:
!
!          MIB Module             Compliance Statement
!          ----------             --------------------
!          IF-MIB                 ifCompliance3
!          IF-INVERTED-STACK-MIB  ifInvCompliance"

        MODULE  -- this module
          MANDATORY-GROUPS {
            ifCapStackGroup
          }

          OBJECT       ifCapStackStatus
          SYNTAX       TruthValue { true(1) }
          DESCRIPTION
            "Support for the false(2) value is OPTIONAL for
            implementations supporting pluggable interfaces."

          OBJECT       ifInvCapStackStatus
          SYNTAX       TruthValue { true(1) }
          DESCRIPTION
            "Support for the false(2) value is OPTIONAL for
            implementations supporting pluggable interfaces."

        MODULE  IF-MIB
          MANDATORY-GROUPS {

            ifStackGroup2
          }

 @       MODULE  IF-INVERTED-STACK-MIB
 @         MANDATORY-GROUPS {
 @           ifInvStackGroup
 @         }

        ::= { ifCapStackCompliances 1 }

  My concerns are these:
  - I wonder if we want to keep (all) the lines that I marked with !
  - part of it is already coverd with the lines I marked with @
    So I would at least remove the line
         IF-INVERTED-STACK-MIB  ifInvCompliance
    to avoid duplication. 

  At the other hand, I could also live with what you have.

- But I do want you to fix SMICng reported error:

  E: f(rfc2864.mi2), (168,26) Item "ifStackGroup2" should be IMPORTed
  
  since you do list that as a mandatory group.

- see below answers from me for resolutions that I am not
  agreeing to. I removed all the items we agree on. Thanks
  for fixing or answering/explaining.

> [EB] Replaced with TruthValue.

This is much better.
I wonder if it would now be better to rename the object from
ifCapStackStatus to ifCapStackCapability to better represent
its purpose. Same for possibly renaming ifInvCapStackStatus
into ifInvCapStackCapability.

I am not hung up on it though.

> > SYNTAX for the discovery code. Can you explain, or point me to  the 
> > 802.3ah clause that explains/justifies that?
> 
> [EB] The remote discovery code is defined by 802.3ah as a 
> 6-octets (48-bit) value, see 45.2.6.8. Clause 61A.2 actually 
> suggests to use a MAC address for the discovery operation, in 
> order to ensure its 'local uniqueness'.
> Since PhysAddress represents media- or physical-level 
> addresses it was a natural choice. I've added the size 
> attribute to indicate it's length. 
> 

Would it not be wise to also add 45.2.6.8 and 61A.2 in the
REFERENCE clause?

> >  Do you not want it instatntiated in that case, or do you want  its 
> >  value to be ignored/irrelevant? Pls be specific.
> >

But I keep wondering what that means? Does the object get instantiated
with some value that means "unsupported"? What is a SNMP SET occurs,
that one gets rejected I guess (probably with inconsistentValue for
SNMP)?
Or is the object not instantiated?
Or if read, does it return a zero-length OCTET STRING?
Oh no, that is not possible, cause the TC has a SIZE (1..6)

Maybe we need a TC EfmProfileIndexListOrNone, which allows a 
SIZE (0..6), and requries that an OBJECT of that SYNTAX specifies in
the DESCRIPTION clause what the zero length means. And in this case
it could mean "unsupported".

Similar questions/comments for other uses of "unsupported"

> >  pls be clear. You do get gaps in the tables if some objects are
not 
> > instantiated. Might be easier (on NM apps) if there was a value
(like 
> > all zeros, or zerolength string) that can be ignored.
> 

MMM... remember I do not understand what "writing to this object SHALL
be ignored" means. You had replaced "ignored" with "rejected" in other
objects. Maybe you better do that here as wel;l.

You also changed the SYNTAX from

  SYNTAX      PhysAddress

to

Such was needed for INDEX objects, but not necessarily for this object.
I do find it wise though to also be specific as to what the SIZE is,
specifically since the DESCRIPTION clause does state that it is a
6-octet
value. However, that makes it invalid to state:

          A zero-length octet string SHALL be returned on an attempt to
          read this object when PAF aggregation is not enabled.

Because the SYNTAX states that it CANNOT be zero length. To cover this,
you
can use a SYNTAX of:

  SYNTAX      PhysAddress (SIZE(0 | 6))

> [EB] I believe that 'read-create' in MAX-ACCESS clause of all 
> the objects in a row (except for the 'not-accessible' index) 
> already indicates that in order to change an object in a 
> particular profile, one would have to create a new one, 
> change the reference(s) from the old profile to a new one and 
> remove the old (inactive) profile if not needed anymore.
> 

Mmm... a MAX-ACCESS read-create does not imply that semantic.
So I would be more specific and state this somewhere. 
the DESCRIPTION clause of the RowStatus object might be a good 
place for that.

I could imagine that efmCuPme2BProfileDescr might be changed even
for an active row. At least from the DESCRIPTION clause of that
object, I do not get the impression that it controls/prescribes
operational attributes, does it?

I can see that for the other objects, they probably cannot be 
changed while the row is active and referenced. But maybe one
could put the row in notInService, changed the attributes and
use it again?

I hope I am now explaining better why I wondered (and still wonder).

In any event, this is not clear from the current definitions.
If you want to prescirbe that 
    in order to change an object in a particular profile, 
    one would have to create a new one, change the reference(s)
    from the old profile to a new one and remove the old 
    (inactive) profile if not needed anymore.

then pls be explict and put it in a DESCRIPTION clause.

> [EB] Same answer as above.
Same comment as above.

> Personally I prefer inline enums (if they are not repeated of 
> course which would make they perfectly good candidates for 
> TCs) as they list the values and their meaning in the 
> description for the object - less jumps between the object 
> and a TC when reading the MIB.
> 

As I said. I wondered if it is smart.
If no one else wonders, and thereby (implicitly, by being silent)
agrees with you. Then I am OK with what you have.

> [EB] While I tend to agree with the advantages of the naming 
> scheme you proposed, I would like to ask to be forgiven and 
> leave the names as they are now, due to the following rasons:
> - It is a big change (considering also that there are at 
> least 2 experimental implementations of this draft that I know of)

I know it is a big change, and so I am willing to accept what you 
have because of that.

The fact that there are 2 experimental implementations is somewhat
of a bad argument. They should (in order to avoid conflicts
with whatever we finally produce as a standards track MIB module)
have taken an approach whereby:

- they have rooted the MIB module under their own enterprise OID
  tree
- renamed the MIB mdoule and all objects so they won't conflict 
  with our (soon to be) standard MIB module. Normally one then
  prefixes all object names with the company name or some such.

> recommendation of 32 char restriction - the proposed change 
> would make some names to be longer than 32 chars. (Yes, I 
> just read RFC 4181 view on that - the 32 character 
> restriction recommendation of SMIv2 SHOULD be set aside in 
> favor of promoting clarity and uniqueness). However it could 
> be argued that it is easier to remember 

Not so strong argument, but I understand what you are saying.

So based on the "it is a BIG change and it is late in the process"
I will not require the change to be made. Hope Dan agrees as well,
and I assume that WG members also agree. 

WG members/paritcipants: Speak up if not!

> [EB] You are right. However when one is looking at the 
> description clause of a 'not-accessible' table, the only way 
> to determine if it is composed of read-only objects is to 
> check the Max access value of each and every object in the 
> table. I believe that the 'NOT persistent'
> sentence helps reader to understand the nature of a table 
> quicker, so I would leave it as is.
> 

Mmmm... also read-write objects can be NON-persistent (volatile).
So I do not follow your logic ??

I believe I still see other objects that are read-write or read-create
and for which it is not clear what the persistency behaviour is 
supposed to be. Can you pls check and make sure that that is specified
for all writable objects. If the behaviour is the same for all
objects in a row, you can describe it in the xxxEntry DESCRITPION
clause for that row.

Bert