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

From: "Wijnen, Bert \(Bert\)" <[email-masked]>
Date: Wed, 21 Feb 2007 15:51:14 +0100

Looks good now. One thing (I should have seen yesterday too)
is that you need to move a few informative refrences to 
the normative references and vice versa

- I think RFC3410 is informative (it is also an informational
  RFC).

- RFC2863 and RFC2864 are normative, because we IMPORT from those.

- Since we use them in REFERENCE clauses or we use profiles
  from (as listed in DESCRIPTION clauses), I think that also
  ANFP< but certainly 991.2 and 992.1 are normative, no?

- Since we state:
   3.4.  Relation to Ethernet-Like and MAU MIB modules

   The implementation of EtherLike-MIB [RFC3635] and MAU-MIB
   [I-D.ietf-hubmib-rfc3636bis] is REQUIRED for the EFMCu interfaces.

  We probably also better make RFC3635 a normative ref.

With that I think we would be ready.

Further, I would like to react to a few of Ed's rebuttals:

> > - 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.
> 
> [EB] ifStackGroup2 is already imported, I've fixed that in 
> the version I sent before.
> 

My appology, the error is in RFC2864, not in the EFM-CU-MIB.

> > > >- 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.
> > 
> > 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.
> 
> [EB] ifCapStack already stands for "Interface Capability 
> Stack" - appending "Capability" would make it "Interface 
> Capability Stack Capability". How about: ifCapStackAbility ?
> Or we can leave it ifCapStackStatus, to emphasize its 
> similarity with IfStackTable
> 

Your argument for consistency with ifCapStackStatus makes sense.
And as I said, I am not hung up on it.
So I am OK now.

> [EB] I've found only one table without the persistency definition
> (efmCuPme10PStatusTable) and corrected it - now all tables 
> contain the persistency behavior definition in the 
> DESCRIPTION clause for the table.
> Basically only the Status tables are non-persistent.
> Would that be satisfactory?
> 

Yep.

I think we made good progress.

Pls correct the references (as stated at the top of this email)
and then you can submit to internet-drafts as far as I am
concenrned.

Next step is then (another) W Last Call to givbe anyone a chance
to look at the latest changes.

Bert

_______________________________________________
Hubmib mailing list
[email-masked]
https://www1.ietf.org/mailman/listinfo/hubmib

---

From: "Edward Beili" <[email-masked]>
Date: Wed, 21 Feb 2007 15:46:25 +0200

Bert,
Thanks for such a quick reply.

I've attached the latest version of the draft and the MIB modules.

See my rebuttal below (there are two questions for you):

Regards,
-E.

> -----Original Message-----
> From: Wijnen, Bert (Bert) [mailto:[email-masked]] 
> Sent: Tuesday, February 20, 2007 18:14
> To: Edward Beili
> Cc: Dan Romascanu (E-mail); Hub Mib
> Subject: RE: [Hubmib] My review of: 
> draft-ietf-hubmib-efm-cu-mib-06.txt
> 
> 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.

[EB] I agree - removed the lines marked by !.

>   E: f(rfc2864.mi2), (168,26) Item "ifStackGroup2" should be IMPORTed
>   

[EB] ifStackGroup2 is already imported, I've fixed that in the version
I sent before.

> - 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.
>  
> > > 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
> > 
> ifCapStackStatus to ifCapStackCapability to better represent 
> its purpose. Same for possibly renaming ifInvCapStackStatus 
> into ifInvCapStackCapability.
> 

[EB] ifCapStack already stands for "Interface Capability Stack" -
appending "Capability" would make it "Interface Capability Stack
Capability". How about: ifCapStackAbility ?
Or we can leave it ifCapStackStatus, to emphasize its similarity with
IfStackTable

> > >- 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.
> 
> Would it not be wise to also add 45.2.6.8 and 61A.2 in the
> REFERENCE clause?

[EB] Added

> > >- 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'
> 
> 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"

[EB] Replaced "unsupported" or "not supported" with "irrelevant" for:
efmCuAdminProfile, efmCuPmeAdminProfile, efmCuPmePeerSnrMgn,
efmCuPmePeerLineAtn
In all cases specified which value shall be returned on an attempt to
read and write, if allowed.
I've also added a zero length value to EfmProfileIndexList TC.

> > >- 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.
> 
> 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.

[EB] Agree. Replaced "ignored" with "rejected"
 
> You also changed the SYNTAX from
> 
>   SYNTAX      PhysAddress
> 
> to
> 
>   SYNTAX      PhysAddress (SIZE(6))
> 
> 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.
> 
>           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.
> 
> 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] Agree. Modified the syntax to include 0 length for both
efmCuPAFDiscoveryCode and efmCuPAFRemoteDiscoveryCode.
For efmCuPAFDiscoveryCode reworded the behavior for non-PAF ports as
follows:
       PCS ports incapable of supporting PAF SHALL return a zero
       length octet string on an attempt to read this object. An
       attempt to write to this object SHALL be rejected for such
       ports.

> > >- 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.
> 
> 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] I added the following paragraph to all RowStatus DESCRIPTION
clauses:

       An 'active' entry SHALL NOT be modified. In order to modify an
       existing entry it MUST be taken out of service (by setting
       this object to 'notInService'), modified and set 'active'
       again.

> > >  Same question for efmCuPme2BsModeRowStatus, although 
> there a change
> 
> > > is probably not disruptive.
> > >  Pls check all occurences of RowStatus
> > 
> > [EB] Same answer as above.
> > 
> Same comment as above.

[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.
> 
> 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] OK.
 
> > >- 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)
> 
> 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.
> 
> > - 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
> 
> 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] I'm going to wait here.

> > >- 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.
> > 
> 
> 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

[EB] I've found only one table without the persistency definition
(efmCuPme10PStatusTable) and corrected it - now all tables contain the
persistency behavior definition in the DESCRIPTION clause for the
table.
Basically only the Status tables are non-persistent.
Would that be satisfactory?
_______________________________________________
Hubmib mailing list
[email-masked]
https://www1.ietf.org/mailman/listinfo/hubmib