AD review of: draft-ietf-adslmib-vdsl-ext-scm-05.txt

"Wijnen, Bert (Bert)" <[email protected]>
Newsgroups gmane.ietf.adslmib
Message-ID <7D5D48D2CAA3D84C813F5B154F43B1550526AF42@nl0006exch001u.nl.lucent.com>
OK here are my other review comments:

Serious issues

1. The SMICng compilation/syntax issues I reported earlier.
   - must use INTEGER for enumartions, not Unsigned32.
   - must use Unsigned32, not unsigned32
   - index item "vdslLineSCMConfProfileBandId" must be not-accessible

2. I see:
   vdslLineSCMConfProfileBandUsage OBJECT-TYPE
        SYNTAX       Unsigned32 
        MAX-ACCESS  read-create
        STATUS  current 
        DESCRIPTION 
          "Indicates whether this band is in use.
           Specified as an Unsigned32, the two
           possible values are:
           Unused(1),
           InUse(2)"
        ::= { vdslLineSCMConfProfileBandEntry 2 }
   Seems to me you would rather do that as an enumeration, using INTEGER
   don't you think so?

3. I see:
    vdslLineSCMConfProfileBandRowStatus OBJECT-TYPE
        SYNTAX       RowStatus
        MAX-ACCESS   read-create
        STATUS       current
        DESCRIPTION
           "This object is used to create a new row or modify or
           delete an existing row in this table.

           A profile activated by setting this object to `active'.
   s/profile/profile is/
           When `active' is set, the system will validate the profile.

           Before a profile can be deleted or taken out of
           service, (by setting this object to `destroy' or
           `outOfService') it must be first unreferenced
   s/outOfService/notInService/
           from all associated lines."
       ::= { vdslLineSCMConfProfileBandEntry 7 }
   
   The RowStatus TC does not have a value of outOfService but it does
   have a notInService value.
   You MUST describe in this DESCRIPTIOON clause if any coluns in the
   row can or cannot be changed while the row is in active state.

4. For table: vdslLineSCMConfProfileBandTable 
   I see not text about the persistency of that table.
   Either there needs to be a StorageType column or you must describe
   in the DESCRIPTION clause of the Table or Entry what the persistence
   behaviour is. I see that in sect 2.4 you state that the entries MUST
   be persistent. So This issue can be fixed by putting similar text
   in the DESCRIPTION clause of vdslLineSCMConfProfileBandEntry

5. vdslLineSCMPhysBandUsage 
   should probably be an enumeration (INTEGER as base type).
   WOuld it make sense to do a TC cause you are using it twice?

6. I have (in a spearate email) already expressed my concerns over the
   way you assign the OID to the MODULE-IDENTITY. If you want to do it
   this way, we need IANA instructions on how to administer that namespace,
   see my other email.

nits:
1. vdslLineSCMConfProfileBandId 
     ....
        DESCRIPTION
          "The BandId for this entry, which specifies which band
           is being referred to.  Specified as an Unsigned32, the five
           possible values are:
   and then you go on to list 7 instead of 5.
   How about s/five//
   I already pointed out in an earlier email that enumerations must
   be done with a bse type of INTEGER, so also s/Unsigned32/INTEGER/
   or may be better: s/Unsigned32/enumeration/

2. I also mentioned that the 2 enumerations for BandId might be better
   done as a TC. And I also wonder if BandId is a good name.
   to me it sounds more like a BandType, But this may be just personal
   taste, so telling me so and to shut up is fine.

3. vdslLineSCMPhysBandTable 
   DESCRIPTION clause talks about 5 bands again while you have enumerated 7.

4. I hope you are aware that the MODULE-COMPLIANCE statement you have defined
   mandates that everyone MUST implement the first table as read-create table.
   That means, a read-only implementation cannot claim compliance.
   Such is fine, as long as the WG has consensus on that and is aware that 
   that is what you have documented.

5. It seems that reference [RFC3593] can/should be removed. It is not cited
   anywhere and I do not understand why it is here.

6. It seems there are no citations to RFC3411 and RFC3418, so probably there
    do not need to be references to them either.

admin notes:

when you do a ner revision, pls replace front page boilerplate text:
   This document is an Internet-Draft and is in full conformance with
   all provisions of Section 10 of RFC2026.
with the new RFC3667/8 boilerplate. I.e. this:
   This document is an Internet-Draft and is subject to all provisions
   of section 3 of RFC 3667.  By submitting this Internet-Draft, each
   author represents that any applicable patent or other IPR claims of
   which he or she is aware have been or will be disclosed, and any of
   which he or she become aware will be disclosed, in accordance with
   RFC 3668.
mmm... I see you sort of have that already. Oh well... I think the
above is what will soon become the requirement.

Please add an IANA COnsiderations section, see www.ietf.org/ID-Checklist.html


Thanks, Bert
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.