AD review of draft-ietf-adslmib-vdsl2-06.txt

This is the AD review of draft-ietf-adslmib-vdsl2-06.txt. The document
is in quite a good shape, and it is visible that a lot of work was
invested. However a number of issues need to be clarified and resolved
before the document goes into IETF Last Call. 

MIB Doctors are invited to jump in with further comments. I am not an
expert in ADSL technology, and I did little on checking the accuracy of
many ADSL-related technology claims, beyond checking consistency with
some of the referred documents. So any ADSL-related comments from
knowledgeable people are also welcome. 

One meta-comment is that this document and the MIB module it describes
is huge. This will put challenges in front of people who will wish to
implement it. I have suggested in my review below a few places where
some duplicated text can be taken out, but I recommend the authors to
have another look and make all possible to shorten and make the document
more accessible. 

My comments are divided in Technical and Editorial. 

Technical Comments

T1 - (this is a mix of an editorial and technical comment, but because
of the high impact I prefer to list it as technical) - I believe that it
I s not enough to speak about the persistence of object in a section out
of the MIB module. It is good that such a section exists (although it Is
not mandatory) but this should not prevent writing specifically in the
MIB module, in the DESCRIPTION clauses of the MIB tables what is the
persistence policy relative to the objects in the table. 

T2 - I have strong reservations about indexing tables that describe
profiles and templates by names objects with a Syntax of SnmpAdminString
as described in Section 2.8.3. Such an indexation is very resource
consuming, as each object in the table must carry the name in the index
for each MIB operation. What is the reason these tables cannot be
indexed by a running index, and the name objects be just columns? 

T3 - page 33

   Notifications, other than the threshold notifications listed above,
   SHOULD be rate-limited (throttled) such that there is an
   implementation-specific gap between the generation of consecutive
   notifications of the same event.  When notifications are rate-
   limited, they are dropped and not queued for sending at a future
   time.  This is intended to be a general rate-limiting statement for
   notifications that otherwise have no explicit rate limiting
   assertions in this document.

This should be in my opinion visibly externally. It Is not clear from
the text whether this capability is configurable in any way, it Is not
by SNMP as I do not see ay objects in the MIB module to do it, but at
least objects that allow a manager to know that rate limitation is in
place should be provided. 

T4 - page 37 - what is the reason for reserving bits in TCs like
Xdsl2TransmissionModeType? If this is because if the need to be
compatible with ITU-T or other standards, it would be good to mention
this reason and provide a reference. If this is not the case, reconsider
aligning values to use all bits. 

T5 - Some of the Textual Conventions with enumerated INTEGER syntax
start from zero. This is not forbidden but is not recommended according
to RFC2578. What are the reasons for doing this? For example -
Xdsl2InitResult, Xdsl2ConfPmsForce, Xdsl2LineLdsf

T6 - It is not clear why for a number of Boolean objects new TCs were
invented instead of just using TruthValue - Xdsl2LineLdsf,
Xdsl2LineCeFlag, Xdsl2LineSnrMode, Xdsl2LineForceInp

T7 - a number of objects use RowStatus only to delete rows. The behavior
is however incompletely described. How does the agent respond if a
manager tries to create a row using this object? Such objects are -
xdsl2LineSegmentrowStatus, xdsl2SCStatusAttainableRate

T8 - why are new objects invented for inventory information in
Xdsl2LineInventoryEntry instead of implementing in the agent the
entPhysicalTable in RFC4133? 

T9 - another variant of the standard implementation of RowStatus can be
found in objects like xdsl2ConfProfRowStatus,
xdsl2LConfProfModeSpecRowStatus, xdsl2LConfProfModeSpecBandUsRowStatus,
xdsl2ChConfProfRowStatus, xdsl2LAlarmConfTempRowStatus,
xdsl2LineAlarmConfProfileRowStatus  . What does 'the system will
validate the profile' mean? What happens with the row if the profile is
not validated? What does 'it MUST be unreferenced from all associated
templates' mean? Please explain this in terms of rows that would be
deleted for example, or actions to be taken by the manager if this is
the case. 

T10. A am missing an explanation about how the thresholds in the
xdsl2ChAlarmConfProfileTable are being applied. Are the counters
measured and compared only once in the given interval? Can't the TCs
from RFC3705 be used here?: 

T11. I was expecting the MODULE-COMPLIANCE to provide indication what an
agent implements for each of the flavors of ADSL. Is the case that an
agent implementation must implement everything, as line cards of
different types may be added? 



Editorial Comments

E1 page 3: 

This document does not obsolete RFC 2662 [RFC2662], or RFC 4706
   [RFC4706] but rather provides a more comprehensive management model
   that addresses the VDSL2 technology per G.993.2 ([G.993.2]) as well
   as ADSL, ADSL2 and ADSL2+ technologies.

s/ a more comprehensive/a complementary/

E2 - page 6, section 2.3.1 - please make a more attentive search to
determine what is missing from the list of abbreviations. A few
instances I discovered - POTS, ISDN, TCM-ISDN, DMT. 

E3 - page 7, page 2.3.2 - there is no reason for this section which to a
large extent duplicates text from the TC-MIB module itself. I suggest to
take it out and replace it by a list of TCs defined in this document and
a reference to the TC-MIB module. 

E4 - page 28 - s/This MIB allows supporting/This MIB module allows
supporting 

E5 - page 31 - s/ Network Elements may /Network Elements MAY/ and s/The
xTU-C should / The xTU-C SHOULD/

E6 - some of the DESCRIPTION clauses in the TC-MIB use the expression
'Attributes with this syntax ...' In SMIv2 we do not usually speak about
attributes but about objects. It would be better to correct this term
(appears several times). 

E7. The text in the DESCRIPTION clause of the MIB module duplicates text
in the Overview section. No need for both, actually here I think that
the better place is in the overview, but the main thing in my view is to
eliminate one of them to reduce the length of the text 

E8. A number of tables mention 'an ifType of vdsl(xxx)'. It is good to
add at least at the first occurrence a note to the RFC Editor to be
deleted at publication that mentions that xxx needs to be replaced by
the value that will be assigned by IANA for vdls2

E9. The DESCRIPTION clauses of Valid Intervals and Invalid Intervals are
almost void. 

E10. Section 4 would benefit from one or two examples of configuration
templates (did I say the document is too big? :-))

Thanks and Regards,

Dan