FW: draft-ietf-hubmib-efm-cu-mib-07 LC comments

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=20
Sent: Thursday, May 24, 2007 2:20
To: 'Alfred H=CEnes'
Cc: [email protected]; [email protected]
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.=20

> -----Original Message-----
> From: Alfred H=CEnes [mailto:[email protected]]
> Sent: Wednesday, May 23, 2007 22:39
> To: Edward Beili
> Cc: [email protected]; [email protected]
> Subject: draft-ietf-hubmib-efm-cu-mib-07 LC comments
>=20
> 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=20
> draft-ietf-hubmib-efm-cu-mib-07, plus the issues in Section 7 & up.
>=20
> A subsequent message will address the remaining textual flaws in=20
> Sections 1..6  I have observed.
>=20
> All but the last two items below deal with the MIB modules in the=20
> draft.  For brevity, I use the following shorthand notation to locate=20
> the affected part of the text there:
>=20
>       <object_name> D <n>
>                    /    \
>                   /      paragraph number; positive values count
>     DESCRIPTION clause   top-down, negative values count from the end
> or:
>       <object_name> <X>
>=20
> where the following single capital letters <X> are used:
>      S ... SYNTAX clause
>      O ... OBJECTS clause
>      R ... REFERENCES clause
>=20
> The items are listed in textual order.
> Issues spanning more than one object (etc.) will be noted for either=20
> place in the text via cross-references.
>=20
>=20
> (A)  =3D=3D=3D=3D  IF-CAP-STACK-MIB  =3D=3D=3D=3D
>=20
>=20
> (1)  ifCapStackTable D -2
>=20
> Please reconsider the trailing phrase,
>=20
>   "... for any existing value of x or y." .
>            ^^^^^^^^^^^^^^^^^^
>=20
> This might be misleading, since in the preceding text, the scenario of =

> EFM-Cu interfaces is only referred to by "e.g.".
>=20
> The IF-CAP-STACK-MIB obviously has been specified separately with=20
> other areas of applicability in mind, i.e., the ifCapStackTable (and=20
> its inverse) might be applicable for other scenarios of stacked=20
> interfaces too (although not specified yet).
> Currently these tables should only be applied to EFMCu interfaces; in=20
> all other cases where it would be conceptionally valid,=20
> ifCapStackTable rows should not be instantiated unless and until=20
> specified otherwise.
>
> Therefore, I recommend to replace the above phrase by:
>=20
>   "... for any ifIndex values x or y representing an EFMCu PME
>    or PCS, respectively."
>=20
> (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. =20
=20
> (B)  =3D=3D=3D=3D  EFM-CU-MIB  =3D=3D=3D=3D
>=20
>=20
> (2)  efmCuLowRateCrossing O
>=20
> The ASN.1 comment,
>=20
>   -- ifIndex is not needed here since we are under specific PCS
>                                      =20
> ^^^^^^^^^^^^^^^^^^^^^^^^^ is unclear, and in fact not needed at all:
> the ifIndex value occurs in the index part of all the OIDs of the=20
> objects included in this NOTIFICATION, anyway!
>=20
> I recomment to remove that ASN.1 comment entirely, or else to replace=20
> it by:
>=20
>   -- The ifIndex of the specific PCS is delivered in the index part
>      of the OIDs of the following objects' instances.
>=20
> (or similar).

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

> (3)  efmCuNumPMEs S
>=20
> The semantics of that object apparently perfectly fit the=20
> specification of the Gauge32 SYNTAX in SMIv2.
>=20
> 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
>=20
> The draft says:
>=20
>           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).
>=20
> It should say:
>=20
>           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
>=20
> The draft says:
>=20
>           The operational sub type of the PME can be configured via
>           efmCuPmeAdminSubType variable.
>=20
> This might be confusing.  It should perhaps better say:
>=20
>              vvvvvvvvv
> |         The desired operational sub type of the PME can be
> configured
> |         via the efmCuPmeAdminSubType variable.
>              ^^^^^

Ok.

> (6)  efmCuPme2BsModeRowStatus D 2
>=20
> The draft says:
>=20
>           If an 'active' entry is referenced via efmCuPme2BsMode, the
>           entry MUST remain 'active' until all references are removed.
>=20
> This is illogical and confusing.
> It should perhaps better (and even simpler) say:
>=20
>           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
>=20
> There are a couple of issues with the table of predefined=20
> efmCuPme10PProfileTable entries presented there.
>=20
> a)  The order of the columns does not exactly match the (OID)
>     order of the columnar objects specified subsequently.
>     This might raise some confusion.
>=20
>     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.
>=20
>     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.
>=20
>     But the efmCuPme10PUPBOReferenceProfile SYNTAX does not
>     admit the enumerated value 0, it only allows (1..9).
>=20
>     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'.
>=20
>     But the efmCuPme10PBandplanPSDMskProfile SYNTAX does not
>     admit the enumerated value 30, it only allows (1..29).
>=20
>     What's wrong there ???

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

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

Agree.

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

Agree.

> (9)  efmCuPme10PBandplanPSDMskProfile D 2  and  R
>=20
> 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.*' .
>=20
> 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=20
> efmCuPme10PUPBOReferenceProfile D, "ETSI TS 101 270-1", and in the=20
> efmCuPme10PBandNotchProfiles D, "TS101 270-1"
> is used for what perhaps denotes the same document.
>=20
> Using a unified notation in all three tables would be appreciated!

Agree.

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

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

Agree

> The draft says:
>=20
>           Any combination of profiles can be specified by ORing
>           individual profiles, for example value of 0x0622 selects
>           profiles 2,6,10 and 11.
>=20
> 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.]
>=20
> 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, ...)
>=20
> Assuming the set of profiles to be left unchanged, the hex value must=20
> be corrected to 0x2230 .
>=20
> Hence, the draft should say:
>=20
>           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
>=20
> The draft is not clear about the instantiation rules for rows in the=20
> efmCuPme10PStatusTable.
>=20
> From the D's of the columnar objects it can be concluded that such=20
> rows only make sense for 10PASS-TS PMEs, and that therefore the=20
> instantiation of efmCuPme10PStatusTable rows is not intended for=20
> 2-BASE-TL PMEs, and cannot be assumed to be mandatory in that case.
>=20
> The efmCuCompliance MODULE-COMPLIANCE statement contains the lines:
>=20
>           GROUP       efmCuPme10PStatusGroup
>           DESCRIPTION
>             "Support for this group is OPTIONAL for implementations
>             supporting 10PASS-TS Phy."
>=20
> Hence, even for 10PASS-TS PMEs, the instantiation of=20
> efmCuPme10PStatusTable rows is *not* mandatory.
>=20
> Contrary to that, the efmCuPme10PStatusEntry OBJECT-TYPE declaration=20
> contains the structural specification:
>=20
>         AUGMENTS { efmCuPmeStatusEntry }
>=20
> According to the SMIv2 (STD 58, RFC 2578, Section 7.8), this clause=20
> 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=20
> the corresponging efmCuPme10PStatusEntry.
>=20
> That makes no sense!
>=20
> IMHO, the above AUGMENTS clause has to be substituted by:
>=20
>         INDEX { ifIndex }

Agree.

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

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

Right.

> (17)  Sections 8, 10, and 11
>=20
> 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=20
> hastily preparation.
>=20
> If you like, please forward (parts of) this note to the hubmib  list=20
> if you think the issues above should be discussed there.
>=20
> Kind regards,
>   Alfred H=CEnes.
>=20
> --
>=20
> +------------------------+------------------------------------
> --------+
> | TR-Sys Alfred Hoenes   |  Alfred Hoenes   Dipl.-Math.,=20
> Dipl.-Phys.  |
> | Gerlinger Strasse 12   |  Phone: (+49)7156/9635-0, Fax: -18=20
>         |
> | D-71254  Ditzingen     |  E-Mail:  [email protected]            =20
>         |
> +------------------------+------------------------------------
> --------+
>=20