Subject: [Hubmib] FW: draft-ietf-hubmib-efm-cu-mib-07 LC comments

From: "Edward Beili" <[email-masked]>
Date: Fri, 25 May 2007 13:53:36 +0300

Dear members of the HUBMIB WG and IESG,
I have received some important LC comments w.r.t. draft-ietf-hubmib-efm-cu-mib-07 from Alfred Hoenes, which I'm forwarding together with my replies to the mailing lists.

Regards,
-E.

-----Original Message-----
From: Edward Beili 
Sent: Thursday, May 24, 2007 2:20
To: 'Alfred H־nes'
Cc: [email-masked]; [email-masked]
Subject: RE: draft-ietf-hubmib-efm-cu-mib-07 LC comments

Alfred,
Thank you very much for your comments, I fully appreciate the time and effort you put into checking the draft. I agree with almost all of your comments. See my rebuttal inline below.

Bert,
How do you want me to proceed - should I issue a new version of the draft?

Regards,
-E. 

> -----Original Message-----
> From: Alfred H־nes [mailto:[email-masked]]
> Sent: Wednesday, May 23, 2007 22:39
> To: Edward Beili
> Cc: [email-masked]; [email-masked]
> Subject: draft-ietf-hubmib-efm-cu-mib-07 LC comments
> 
> Hello,
> as recommended by Bert Wijnen yesterday, I've hurried up, and hereby I 
> submit my notes on the more-than-just-editorial issues I found in 
> draft-ietf-hubmib-efm-cu-mib-07, plus the issues in Section 7 & up.
> 
> A subsequent message will address the remaining textual flaws in 
> Sections 1..6  I have observed.
> 
> All but the last two items below deal with the MIB modules in the 
> draft.  For brevity, I use the following shorthand notation to locate 
> the affected part of the text there:
> 
>       <object_name> D <n>
>                    /    \
>                   /      paragraph number; positive values count
>     DESCRIPTION clause   top-down, negative values count from the end
> or:
>       <object_name> <X>
> 
> where the following single capital letters <X> are used:
>      S ... SYNTAX clause
>      O ... OBJECTS clause
>      R ... REFERENCES clause
> 
> The items are listed in textual order.
> Issues spanning more than one object (etc.) will be noted for either 
> place in the text via cross-references.
> 
> 
> (A)  ====  IF-CAP-STACK-MIB  ====
> 
> 
> (1)  ifCapStackTable D -2
> 
> Please reconsider the trailing phrase,
> 
>   "... for any existing value of x or y." .
>            ^^^^^^^^^^^^^^^^^^
> 
> This might be misleading, since in the preceding text, the scenario of 
> EFM-Cu interfaces is only referred to by "e.g.".
> 
> The IF-CAP-STACK-MIB obviously has been specified separately with 
> other areas of applicability in mind, i.e., the ifCapStackTable (and 
> its inverse) might be applicable for other scenarios of stacked 
> interfaces too (although not specified yet).
> Currently these tables should only be applied to EFMCu interfaces; in 
> all other cases where it would be conceptionally valid, 
> ifCapStackTable rows should not be instantiated unless and until 
> specified otherwise.
>
> Therefore, I recommend to replace the above phrase by:
> 
>   "... for any ifIndex values x or y representing an EFMCu PME
>    or PCS, respectively."
> 
> (or similar).

EFMCu interfaces with flexible cross-connect are just one example where IF-CAP-STACK-MIB can be used.
In fact ifCapStackTable (and its inverse) can be implemented for _any_ stacked interface. Of course in some cases it would not add any value, for example for interfaces without flexible cross-connect, the ifCapStackTable would be semantically identical to the ifStackTable - meaning that the sublayers which MAY be connected, as indicated by if CapStackTable, ARE connected, as indicated by ifStackTable.

The DESCRIPTION text clearly states that ifStackTable describes cross-connect capability of the devices with stacked interfaces. Apart from EFMCu there are G.Bond interfaces (their MIB modules are being drafted) which use this table, so specifying EFMCu here is incorrect. While there is no point in implementing this table for devices without cross-connect capability, it won't do any harm if implemented.

Therefore, I suggest leaving this text as it is.  
 
> (B)  ====  EFM-CU-MIB  ====
> 
> 
> (2)  efmCuLowRateCrossing O
> 
> The ASN.1 comment,
> 
>   -- ifIndex is not needed here since we are under specific PCS
>                                       
> ^^^^^^^^^^^^^^^^^^^^^^^^^ is unclear, and in fact not needed at all:
> the ifIndex value occurs in the index part of all the OIDs of the 
> objects included in this NOTIFICATION, anyway!
> 
> I recomment to remove that ASN.1 comment entirely, or else to replace 
> it by:
> 
>   -- The ifIndex of the specific PCS is delivered in the index part
>      of the OIDs of the following objects' instances.
> 
> (or similar).

You are right, I wrote this comment for myself, it should be removed.

> (3)  efmCuNumPMEs S
> 
> The semantics of that object apparently perfectly fit the 
> specification of the Gauge32 SYNTAX in SMIv2.
> 
> Are there specific reasons to *not* specify a subrange of
> Gauge32 for this object, but using Unsigned32 instead ?

My understanding of Gauge32 is it should be used for "latched" objects, that is, whose information being modeled can be greater than the maximum or smaller than the minimum.
Since the number of PMEs can never be negative or more than 32, I concluded that Gauge32 is inappropriate here. Please correct me if I'm wrong here.

> (4)  efmCuPmeThreshSnrMgn D 2
> 
> The draft says:
> 
>           This object is writable for the CO subtype PMEs
> |         (2BaseTL-O/10PassTS-R). It is read-only for the CPE subtype
>           (2BaseTL-R/10PassTS-R).
> 
> It should say:
> 
>           This object is writable for the CO subtype PMEs
> |         (2BaseTL-O/10PassTS-O). It is read-only for the CPE subtype
>           (2BaseTL-R/10PassTS-R).

You are right, it should be corrected.

> (5)  efmCuPmeOperSubType D 2
> 
> The draft says:
> 
>           The operational sub type of the PME can be configured via
>           efmCuPmeAdminSubType variable.
> 
> This might be confusing.  It should perhaps better say:
> 
>              vvvvvvvvv
> |         The desired operational sub type of the PME can be
> configured
> |         via the efmCuPmeAdminSubType variable.
>              ^^^^^

Ok.

> (6)  efmCuPme2BsModeRowStatus D 2
> 
> The draft says:
> 
>           If an 'active' entry is referenced via efmCuPme2BsMode, the
>           entry MUST remain 'active' until all references are removed.
> 
> This is illogical and confusing.
> It should perhaps better (and even simpler) say:
> 
>           If an 'active' entry is referenced via efmCuPme2BsMode, the
>           entry MUST remain 'active'.

I don't see a big difference between the two, but willing to go with your wording. I think though that it is important to remind the implementer that there can be multiple references to the same entry (from multiple PMEs). So I would suggest to say:

          If an 'active' entry is referenced via efmCuPme2BsMode, the
          entry MUST remain 'active'. Note that there can be multiple
	    references to the same entry.

> (7)  efmCuPme10PProfileTable D 3
> 
> There are a couple of issues with the table of predefined 
> efmCuPme10PProfileTable entries presented there.
> 
> a)  The order of the columns does not exactly match the (OID)
>     order of the columnar objects specified subsequently.
>     This might raise some confusion.
> 
>     I recommend to exchange the last two columns to achieve
>     a matching order.  (Changing the columnar object sequence
>     and the OIDs for efmCuPme10PPayloadURateProfile and
>     efmCuPme10PPayloadDRateProfile will perhaps not be feasible!)

I agree. Suggest to swap the order of the efmCuPme10PPayloadURateProfile and efmCuPme10PPayloadDRateProfile, since the IEEE standard defines Downstream rate before the Upstream.

> b)  In the first table row, the "(default)" is attached to the
>     value in the last column without any intervening white space.
> 
>     I strongly suspect that this tag conceptually is affixed to
>     the entire row, not to the last columnar valu there.
>     I that's right, to avoid confusion, I recommend to insert
>     at least one space character before the "(default)"

Agree, to further avoid confusion I suggest to add the word 'profile' after the 'default', i.e.
          1      1      3    2,6,10,11    20    20 (default profile)

> c)  From the 4th row down to the last row, in the third column
>     the value '0' (for UPBO p#) is specified.
> 
>     But the efmCuPme10PUPBOReferenceProfile SYNTAX does not
>     admit the enumerated value 0, it only allows (1..9).
> 
>     What's wrong there ???

Right, I added the value of profile(0) to efmCuPme10PUPBOReferenceProfile, with a description - no profile.

> d)  The last row (profile index 22) in its second row gives a
>     Bandplan PSDMask# value of '30'.
> 
>     But the efmCuPme10PBandplanPSDMskProfile SYNTAX does not
>     admit the enumerated value 30, it only allows (1..29).
> 
>     What's wrong there ???

Missed that one, I added the value 30 with description.

> (7')  efmCuPme10PProfileTable D -3 and D -2
> 
> Because of item (7) above, I include the following remarks on textual 
> flaws in the subsequent paragraphs as well:
>                            vvvv
> a) Change:  "... be created by during agent initialization ..."
> 
>    to say:  "... be created during agent initialization ..."

Agree.

> b) Delete the two commas from D -2.
> 
> 
> (8)  efmCuPme10PBandplanPSDMskProfile S
> 
> See item (7) d)  above!

Agree.

> (9)  efmCuPme10PBandplanPSDMskProfile D 2  and  R
> 
> a)
> The profile table uses short references for the "PSD Mask":
>    'T1.424/T-U P1', 'TS1 101 270-1', and 'G.993.1 F.1.2.*' .
> 
> These should be detailed in the REFERENCE clause of the object.

Agree.

> b)
> The ref. label "TS1 101 270-1" is used here, whereas in the 
> efmCuPme10PUPBOReferenceProfile D, "ETSI TS 101 270-1", and in the 
> efmCuPme10PBandNotchProfiles D, "TS101 270-1"
> is used for what perhaps denotes the same document.
> 
> Using a unified notation in all three tables would be appreciated!

Agree.

> (10)  efmCuPme10PUPBOReferenceProfile S
> 
> See item (7) c)  above!

Agree.
 
> (11)  efmCuPme10PUPBOReferenceProfile D 2  and  R
> 
> a)
> As in item (9) a)  above:
> 
> The profile table uses short references for the "PSD Mask":
>    'T1.424/T-U', and 'ETSI TS 101 270-1' .
> 
> These should be detailed in the REFERENCE clause of the object.
> 
> b)
> See item  (9) b)  above !
> 
> 
> (12)  efmCuPme10PBandNotchProfiles D 2  and  R
> 
> a)
> The profile table uses short references for the table heading
>    'G.991.3',  'T1.424/T-U', and 'TS101 270-1' .
> 
> These should be detailed in the REFERENCE clause of the object.
> 
> b)
> See item  (9) b)  above !
> 
> 
> (13)  efmCuPme10PBandNotchProfiles D -2

Agree

> The draft says:
> 
>           Any combination of profiles can be specified by ORing
>           individual profiles, for example value of 0x0622 selects
>           profiles 2,6,10 and 11.
> 
> The hex value given does not match the set of profiles listed.
> [See RFC 3416, Section 2.5, paragraph #2 for the mapping of BITS.]
> 
> 0x0622 corresponds to 5,6,10,14 !
> (The 1st nibble is for BITS 0,1,2,3, the 2nd for BIT 4,5,6,7, ...)
> 
> Assuming the set of profiles to be left unchanged, the hex value must 
> be corrected to 0x2230 .
> 
> Hence, the draft should say:
> 
>           Any combination of profiles can be specified by ORing
>           individual profiles, for example value of 0x2230 selects
>           profiles 2,6,10 and 11.

You are right. Must be corrected.

> (14)  efmCuPme10PStatusEntry D + other
> 
> The draft is not clear about the instantiation rules for rows in the 
> efmCuPme10PStatusTable.
> 
> From the D's of the columnar objects it can be concluded that such 
> rows only make sense for 10PASS-TS PMEs, and that therefore the 
> instantiation of efmCuPme10PStatusTable rows is not intended for 
> 2-BASE-TL PMEs, and cannot be assumed to be mandatory in that case.
> 
> The efmCuCompliance MODULE-COMPLIANCE statement contains the lines:
> 
>           GROUP       efmCuPme10PStatusGroup
>           DESCRIPTION
>             "Support for this group is OPTIONAL for implementations
>             supporting 10PASS-TS Phy."
> 
> Hence, even for 10PASS-TS PMEs, the instantiation of 
> efmCuPme10PStatusTable rows is *not* mandatory.
> 
> Contrary to that, the efmCuPme10PStatusEntry OBJECT-TYPE declaration 
> contains the structural specification:
> 
>         AUGMENTS { efmCuPmeStatusEntry }
> 
> According to the SMIv2 (STD 58, RFC 2578, Section 7.8), this clause 
> means that the efmCuPme10PStatusTable is a *dense* augmentation of the 
> efmCuPmeStatusTable, i.e. that the instantiation of any row
> (efmCuPmeStatusEntry) there immediately enforces the instantiation of 
> the corresponging efmCuPme10PStatusEntry.
> 
> That makes no sense!
> 
> IMHO, the above AUGMENTS clause has to be substituted by:
> 
>         INDEX { ifIndex }

Agree.

> Also, the instantiation rules should be made explicit in the 
> efmCuPme10PStatusTable D or the efmCuPme10PStatusEntry D .

Agree.
 
> (15)  efmCuNotificationGroup NOTIFICATION-GROUP
> 
> The draft says:
> 
>         NOTIFICATIONS {
>           efmCuLowRateCrossing,
>           efmCuPmeLineAtnCrossing,
>           efmCuPmeSnrMgnCrossing,
>           efmCuPmeDeviceFault,
>           efmCuPmeConfigInitFailure,
>           efmCuPmeProtocolInitFailure
> |  --       efmCuPmeDeviceFault,
> |  --       efmCuPmeLocalPowerLoss
>         }
> 
> It remains unclear what has been intended with the two lines tagged 
> above, written as ASN.1 comments.
> 
> a)
> 'efmCuPmeDeviceFault' is well defined in the MIB module.
> 
> I cannot see any immediate reason why it is getting excluded from the 
> conformance groups.
> 
> Perhaps this line should be re-activated !
>
> b)
> 'efmCuPmeLocalPowerLoss' does not appear anywhere else in the draft.
> 
> Perhaps this line should be dropped !
 
Both comments should be removed. efmCuPmeDeviceFault already appears in the confirmance group.
 
> (16)  Section 7
> 
> In the second paragraph below the bulleted list,
> change   "evasdrop"  -->  "eavesdrop" .

Right.

> (17)  Sections 8, 10, and 11
> 
> Hint:  To alleviate the job of the RFC-Editor and the IANA,
>        - RFC 4836 and RFC 4837 should be substituted for the
>          respective I-Ds;
>        - the first paragraph of Section 8 apparently is void
>          in the meantime (and hence should be deleted), by
>          publication of RFC 4836 and IANA action for that RFC;
>        - the text in Section 10 can be simplified;
>        - one Normative and one Informative Ref. can be updated
>          immediately from I-D to RFC.

Agree.

> I apologize for any errors introduced into this memo due to its 
> hastily preparation.
> 
> If you like, please forward (parts of) this note to the hubmib  list 
> if you think the issues above should be discussed there.
> 
> Kind regards,
>   Alfred H־nes.
> 
> --
> 
> +------------------------+------------------------------------
> --------+
> | TR-Sys Alfred Hoenes   |  Alfred Hoenes   Dipl.-Math., 
> Dipl.-Phys.  |
> | Gerlinger Strasse 12   |  Phone: (+49)7156/9635-0, Fax: -18 
>         |
> | D-71254  Ditzingen     |  E-Mail:  [email-masked]             
>         |
> +------------------------+------------------------------------
> --------+
> 

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

---

From: "Edward Beili" <[email-masked]>
Date: Fri, 25 May 2007 13:55:42 +0300

Continuation of the previous mail on the subject, below my second reply.
Regards,
-E. 

> Sent: Thursday, May 24, 2007 11:58
> To: 'Alfred H־nes'
> Cc: [email-masked]; Dan Romascanu ([email-masked])
> Thanks again. See my reply inline below.
> 
> Regards,
> -E.
> 
> > Sent: Thursday, May 24, 2007 10:48
> > To: Edward Beili
> > Cc: [email-masked]
> > Subject: Re: draft-ietf-hubmib-efm-cu-mib-07 LC comments
> > 
> > Edward,
> > thanks for your very quick and detailed response.
> > Please find inline below a few comments on selected open topics.
> > 
> > > Alfred,
> > > Thank you very much for your comments, I fully appreciate
> > the time and
> > > effort you put into checking the draft. I agree with 
> almost all of 
> > > your comments. See my rebuttal inline below.
> > > 
> > > Bert,
> > > How do you want me to proceed - should I issue a new 
> version of the 
> > > draft?
> > >
> > > ...
> > >
> > 'Strategic' consideration for the next steps:
> > 
> > I have myriads of textual issues, in particular 
> missing/wrong articles 
> > etc. to report.
> 
> BTW, last night when I was looking at the draft I realized 
> that some of the references, in particular various 10P 
> related profiles in Clause 30 are wrong, I was using an old 
> version of the draft at the time of writing and since then 
> didn't check it. My bad:(
> 
> > Taking altogether, that IMHO will deserve a new draft, for 
> clarity and 
> > for the ease of all parties involved.
> > 
> > From previous messages I conclude that you start from an XML source.
> > Perhaps it would be much more efficient if you could provide me the 
> > source, and I could make the proposed changes in a copy thereof
> > (-- this will not happen before Friday evening).  This 
> would save me a 
> > lot of time and decrease the amount of text to be produced, 
> and with 
> > that version at hand you could certainly derive an accepted version 
> > more efficiently -- thus speeding up the entire process.
> > 
> > If you agree, I propose to start from the version after the edits 
> > already discussed, and perhaps further changes as a result 
> from other 
> > comments.  For best timing, your then current intermediate source 
> > version on Friday evening (in your time zone) would be the best 
> > starting point.
> > 
> > Otherwise, if I do not obtain an updated version, I will start from 
> > the published -07.txt version, for my additional comments.
> 
> I agree, I'll try to implement all the changes and send you 
> the final xml file by Saturday night, I'm China now, flying 
> back home tonight, not sure I'll be able to do this by Friday 
> night. Since I'm going to check all the references again, I 
> might as well do it against 802.3-2005 instead of 
> 802.3ah-2004. That was a comment some time ago from Yakov 
> Stein, which we decided to decline. 
> 
> > >> This might be misleading, since in the preceding text, the
> > scenario
> > >> of EFM-Cu interfaces is only referred to by "e.g.".
> > >>
> > > In fact ifCapStackTable (and its inverse) can be
> > implemented for _any_
> > > stacked interface. Of course in some cases it would not add
> > any value,
> > > for example for interfaces without flexible cross-connect, the 
> > > ifCapStackTable would be semantically identical to the
> > ifStackTable -
> > > meaning that the sublayers which MAY be connected, as
> > indicated by if
> > > CapStackTable, ARE connected, as indicated by ifStackTable.
> > > 
> > > The DESCRIPTION text clearly states that ifStackTable describes 
> > > cross-connect capability of the devices with stacked
> > interfaces. Apart
> > > from EFMCu there are G.Bond interfaces (their MIB modules 
> are being
> > > drafted) which use this table, so specifying EFMCu here is
> > incorrect.
> > > While there is no point in implementing this table for
> > devices without
> > > cross-connect capability, it won't do any harm if implemented.
> > > 
> > I fully agree with almost all of your explanation.
> > That's what I reasonably had expected, and what the major 
> part of the 
> > DESCRIPTION clause says.
> > 
> > The ultimate reason for my note was that "any existing" 
> might well be 
> > misunderstood to extending the scope of the last sentence 
> there beyond 
> > whay is specified in this MIB module.
> > 
> > To propose a 'least impact' change:
> > What about  "... any such existing value ..."   ?
> >                     ^^^^^^
> 
> Agree.
> 
> > >> (3)  efmCuNumPMEs S
> > >>
> > > My understanding of Gauge32 is it should be used for "latched"
> > > objects, that is, whose information being modeled can be
> > greater than
> > > the maximum or smaller than the minimum.
> > > Since the number of PMEs can never be negative or more than 32, I 
> > > concluded that Gauge32 is inappropriate here.
> > > Please correct me if I'm wrong here.
> > 
> > The statement,
> >   "the number of PMEs can never be negative or more than 32"
> > IMHO perfectly represent the expected behavior of an object of
> > Gauge32 (0..32) type.  On page 23, RFC 2578 says:
> > 
> >  7.1.7.  Gauge32
> > 
> >    The Gauge32 type represents a non-negative integer, which may
> >    increase or decrease, but shall never exceed a maximum value, nor
> >    fall below a minimum value.  The maximum value can not be greater
> >    than 2^32-1 (4294967295 decimal), and the minimum value 
> can not be
> >    smaller than 0.
> > 
> > IMHO, this is a perfect match to the semantics intended for 
> > efmCuNumPMEs.
> 
> I'm still not convinced (I've read the Gague32 description, 
> it just sounds wrong to me to represent a number of apples as 
> a gauge; speed/throughput/attenuation/acceleration would be 
> perfect candidates for a gauge), but willing to go with your 
> suggestion. (Bert/Dan, your advice would be appreciated?) 
> 
> > > I don't see a big difference between the two, but willing
> > to go with
> > > your wording. I think though that it is important to remind the 
> > > implementer that there can be multiple references to the
> > same entry (from multiple PMEs).
> > > So I would suggest to say:
> > > 
> > A shorter variant might be:
> > 
> >             If an 'active' entry is referenced via some 
> > efmCuPme2BsMode
> >             instance(s), the entry MUST remain 'active'.
> 
> Agree.
>  
> > > I agree.
> > > Suggest to swap the order of the 
> efmCuPme10PPayloadURateProfile and 
> > > efmCuPme10PPayloadDRateProfile, since the IEEE standard defines 
> > > Downstream rate before the Upstream.
> > 
> > I do not know the state of initial implementation(s) and 
> whether such 
> > change would already negatively effect these; therefore I 
> had proposed 
> > the other way.
> > But if you do not see any obstacles, swapping the entries in the 
> > efmCuPme10PProfileEntry syntax (SEQUENCE statement), the order of 
> > description and the assigned OIDs, is clearly the better choice!
> 
> Yes, initial draft implementations do exist and I had a 
> similar concern a while ago on another change, but Bert 
> assured me that the existence of such implementations is not 
> a good excuse at all.
>  
> > > Agree, to further avoid confusion I suggest to add the word
> > 'profile'
> > > after the 'default', i.e.
> > > 
> > >           1      1      3    2,6,10,11    20    20 
> (default profile)
> > 
> > Even better!  I fully agree.
> > (But please verify that the 72 colunm margin really does 
> admit this!)
> 
> I verified, it's ok.
> 
> > >> ...
> > > Both comments should be removed.
> > 
> > O.k.
> > 
> > > efmCuPmeDeviceFault already appears in the conformance group.
> > 
> > Oooops, now that you say that ...  I really have overlooked that!
> >  
> >  
> > Kind regards, again,
> >    Alfred.
> > 
> > --
> >