RE: AD review of: draft-ietf-ipcdn-bpiplus-mib-12.txt - part 2

Very much appreciated

Thanks

Eduardo

-----Original Message-----
From: Wijnen, Bert (Bert) [mailto:[email protected]] 
Sent: Wednesday, July 14, 2004 2:06 PM
To: Ipcdn (E-mail)
Subject: [ipcdn] AD review of: draft-ietf-ipcdn-bpiplus-mib-12.txt -
part 2


OK, here is part 2. AT the bottom I also have repreated part 1, so you 
have it all in one email in case that is handy.

more or less serious (I continue with number 12):
12. I see:
       docsBpi2CmtsProvisionedCmCertStatus OBJECT-TYPE
            SYNTAX  RowStatus
            MAX-ACCESS read-create
            STATUS  current
            DESCRIPTION
                 "Standard RowStatus object except:
            a) if a row has ever been activated,
            a set to docsBpi2CmtsProvisionedCmCert need not succeed,
            b) inactive rows need not be timed out."

   So you are changing the rules of the RowStatus TC? Seems not allowed
to me.


nits/administrative/questions (continue with muber 2):

2. I see:
     docsBpi2CmtsCACertSubject OBJECT-TYPE
           SYNTAX         SnmpAdminString
           MAX-ACCESS     read-only
           STATUS         current
           DESCRIPTION
                "The subject name exactly as it is encoded in the
           X509 certificate.
           The organizationName portion of the certificate's subject
           name must be present.  All other fields are optional.  Any
           optional field present must be prepended with <CR>
           (carriage return) <LF> (line feed) ASCII characters.
           Ordering of fields present must conform to:

           organizationName <CR> <LF>
           countryName <CR> <LF>

    A 7-bit ASCII character (which CR and LF are) does get represented
exactly
    the same when UTF-8 encoded. But it is kind of weird to speak about
    ASCII characters when discussing the content of a UTF-8 based OCTET
STRING.
    I checked with our UTF-8 and Unicode expert (Patrik Faltstrom) and
he comes
    up with this suggestion:

    Replace sentence:
                                                           Any
           optional field present must be prepended with <CR>
           (carriage return) <LF> (line feed) ASCII characters.

    with:

                                                           Any
           optional field present must be prepended with <CR>
           (carriage return, U+000D) and <LF> (line feed, U+000A).

    You have this in a number of objects, pls check them all.

3. I see:
       docsBpi2CmDeviceCmCert   OBJECT-TYPE
            SYNTAX            DocsX509ASN1DEREncodedCertificate
            MAX-ACCESS             read-write
            STATUS              current
            DESCRIPTION
                 "The X509 DER-encoded cable modem certificate.
            Note:  This object can be set only when the value is the
            null string.  Once the object contains the certificate, its
            access MUST be read-only."

   Maybe this is just wording. 
   - First, I already discussed the "null string" issue. I think you
mean 
     a zero length string or maybe better "zero length certificate" or
     "zero length OCTET STRING" or "zero length value".
   - Now, it seems to me that if the requirement is that the object has
     a zero length value in order for a SET to be accepted, then, when
     someone tries a SET while there is already a value, that then the
     system ought to return a error that explains what is wrong. I.e.
     an error that would otherwise not occur. If you get a notWritable,
     then the management station does not necassarily know why that is,
     see bullet 2 page 20 of RFC3416 or point 9 on page 21.
     Maybe a better error would be a inconsistentValue, point 10 on page
     21 of RFC3416?? Does that not seem a better way to indicate this
     error?

4. I see:
       docsBpi2CmTEKDataEncryptAlg   OBJECT-TYPE
            SYNTAX         INTEGER {
                                      none(0),
                                   des56CbcMode(1),
                                   des40CbcMode(2)
                                   }
            MAX-ACCESS     read-only
   I understand that this is read-only and so can only report what is in
the 
   Cm. But I would not be surprised if this will cause questions from
the
   security folk. Is this the only envryption that is supported?

5. I see:
      docsBpi2CmtsDefaultSelfSignedManufCertTrust  OBJECT-TYPE
           SYNTAX    INTEGER {
                     trusted (1),
                     untrusted (2)
                     }
           MAX-ACCESS     read-write
           STATUS         current
           DESCRIPTION
                "This object determines the default trust of
           self-signed manufacturer certificate entries, contained in
           docsBpi2CmtsCACertTable, created after setting the object."

   I cannot say that I understand what "afetr setting the object" means.
   which object?


Thanks,
Bert 

> -----Original Message-----
> From: Wijnen, Bert (Bert) [mailto:[email protected]]
> Sent: woensdag 14 juli 2004 18:03
> To: Ipcdn (E-mail)
> Subject: [ipcdn] AD review of: draft-ietf-ipcdn-bpiplus-mib-12.txt - 
> part 1
> 
> 
> Sorry, it took too long  to finally get this done.
> 
> Not sure this document is ready for IETF Last Call.
> Actaully I think it is not.
> 
> This is part 1. Mainly serious things.
> I may have more nits/admin stuff later.
> 
> Here are my findings.
> 
> More or less serious:
> 1. I see various read-write and/or read-create objects and I do
>    not see any text in the DESCRIPTION clauses (non
> STorageType objects)
>    that tell me what the expected behaviour is w.r.t. 
> persistency of such
>    objects. SO what happens after a restart/reboot?
> 
> 2. I see a number of ZeroBasedCOunter32 objects that have text aka:
>    (for example docsBpi2CmAuthentInfos)
>             DESCRIPTION
>                  "The value of this object is the count of
> times the CM
>             has transmitted an Authentication Information message,
>             since reboot."
>    It is OK to tell us that a ZeroBasedCOunter32 object must 
> start with zero
>    at (re-)boot time or at row creation.
>    But from that point on, a ZeroBasedCounter32 behaves 
> exactly the same
>    as a Counter32, and so it is incorrect to say "count of X 
> since reboot"
>    because the Counter32 may have wrapped!. Possibly this is not gonna
>    happen in practice, but literally the claim is incorrect.
> 
> 3. For these ZeroBasedCounter32 objects, I also see no word
> about a possible
>    discontinuity timer. Why not. Can there NEVER be a 
> discontinuity? Of if
>    there is one does that mean ALL counters experience a 
> discontionuity?
>    The latter is what you basically state (or cause) by not 
> pointing to a
>    specific discontinuity timer. Because then by default it 
> is sysUpTime, and
>    so that means that when you DO experience a discontinuity, 
> then you MUST 
>    reset sysUpTime and that means that a discontinuity for 
> EVERYONE (object)
>    that assumes the default. If such is intended, then fine, 
> but it would be
>    good to then state that sysUpTime is the discontinuity timer.
> 
> 4. I see that for some objects you speak about a "null
> string" or "NULL string".
>    The base data type for such objects is OCTET STRING. In 
> all those cases
>    I suspect (but I am not sure) that you mean the 
> zero-length octet string.
>    Otherwise I do not understand what "null string" means.
>    Pls explain and fix.
> 
> 5. For docsBpi2CmtsAuthCmExpiresOld I see:
>             Note: For CMs running in BPI mode, implementation of this
>             object is optional and MAY vary."
>    Mmm... that sounds like a MODULE-COMPLIANCE aspect and I
> would rather
>    see such things in MODULE-COMPLIANCE and not in object 
> DESCRIPTION clauses.
> 
> 6. I see:
>       -- Note: the following object has been obsoleted
> 
>       docsBpi2CmtsAuthCmReset  OBJECT-TYPE
>            SYNTAX    INTEGER   {
>                                noResetRequested(1),
>                                invalidateAuth(2),
>                                sendAuthInvalid(3),
>                                invalidateTeks(4)
>                                }
>            MAX-ACCESS     read-write
>            STATUS         current
> 
>     So the --Note: is out of sync with the actual status!?
>     What is it? If it IS obsoleted, then status should sya so,
>     And DESCRIPTION clause should explain why it was obsoleted.
>     And the ASN.1 comment line then of course is no longer needed.
> 
> 7. I see:
>       docsBpi2CmtsAuthCACertIndexPtr    OBJECT-TYPE
>             SYNTAX         Integer32 (0..10000)
>    And find that a strange limit (range). And nowhere, not
> even in the 
>    docsBpi2CmtsCACertTable do I see an explanation why that 
> range makes
>    sense (assuming that it does).
> 
> 8. When I see:
>             docsBpi2CmtsIpMulticastAddressType      InetAddressType,
>             docsBpi2CmtsIpMulticastAddress          InetAddress,
>             docsBpi2CmtsIpMulticastMaskType         InetAddressType,
>             docsBpi2CmtsIpMulticastMask             InetAddress,
>    I wonder if (in the same row)
> docsBpi2CmtsIpMulticastMaskType will ever
>    have a different value then docsBpi2CmtsIpMulticastAddressType !??
>    It seems to me that should NOT be allowed, cause otherwise I am not
>    sure how the ANDing of the Mask is going to work/happen.
>    So the next question then is why you do not do:
>             docsBpi2CmtsIpMulticastAddressType      InetAddressType,
>             docsBpi2CmtsIpMulticastAddress          InetAddress,
>             docsBpi2CmtsIpMulticastMask             InetAddress,
>    And let docsBpi2CmtsIpMulticastAddressType be the 
> discriminator for both
>    InetAddresses. One less object, and less change for error/conflict.
> 
>    But thinking even further, Possibly the best thing to do is to use
>             docsBpi2CmtsIpMulticastAddressType      InetAddressType,
>             docsBpi2CmtsIpMulticastAddress          InetAddress,
>             docsBpi2CmtsIpMulticastPrefixLength     
> InetAddressPrefixLength,
>    Are not such masks always setup that they basically
> specify a prefix length?
>    If so, then this is the way to do it with the TCs from 
> INET-ADDRESS-MIB.
> 
> 9. I see various uses of InetAddress as for example here:
>        docsBpi2CmtsIpMulticastAddress          OBJECT-TYPE
>             SYNTAX         InetAddress
>             MAX-ACCESS     read-create
>             STATUS         current
>             DESCRIPTION
>                  "This object represents the IP multicast address
>             to be mapped, in conjunction with
>             docsBpi2CmtsIpMulticastMask."
>    The TC for InetAddress (in RFC3291 or its follow on)
> clearly state that
>    you MUST specify which InetAddressType controls the format 
> of this object
>    as per DESCRIPTION from InetAddress TC:
>          An InetAddress value is always interpreted within the context
>          of an InetAddressType value. Every usage of the InetAddress
>          textual convention is required to specify the InetAddressType
>          object which provides the context. ...
> 
> 10. I see:
>       docsBpi2CmtsIpMulticastMapControl  OBJECT-TYPE
>            SYNTAX         RowStatus
>            MAX-ACCESS     read-create
>            STATUS         current
>            DESCRIPTION
>                 "This object controls and reflects the IP multicast
>            address mapping entry.  There is no restriction on the
>            ability to change values in this row while the row is
>            active.  Inactive rows need not be timed out."
>     Mmm... that "need not be timed out" seems in conflict
> with the RowStatus
>     TC DESCRIPTION clause in RFC2579. Can you explain why this is? 
> 
>     Also, a RowSTatus object MUST specify in its DESCRIPTION
> clause under
>     which conditions 
>     - the row can be activated
>     - which columns (if any) can bve changed while in the 
> active state.
>     I am missing the first.
> 
> 11. You specify:
>        --
>        -- The BPI+ MIB Conformance Statements (with a placeholder for
>        -- notifications)
>        --
> 
>        docsBpi2Notification     OBJECT IDENTIFIER
>             ::= { docsBpi2MIB 2 }
>        docsBpi2Conformance OBJECT IDENTIFIER
>             ::= { docsBpi2MIB 3 }
>        docsBpi2Compliances OBJECT IDENTIFIER
>             ::= { docsBpi2Conformance 1 }
>        docsBpi2Groups      OBJECT IDENTIFIER
>             ::= { docsBpi2Conformance 2 }
> 
>    Why not be (more) consistent with other MIB modules and follow the
>    suggested OID subtrees as per MIB guidelines, 
>    draft-ietf-ops-mib-review-guidelines-03.txt, appendix D:
>         xxxMIB
>         |
>         +-- xxxNotifications(0)
>         +-- xxxObjects(1)
>         +-- xxxConformance(2)
>             |
>             +-- xxxCompliances(1)
>             +-- xxxGroups(2)
>    This is not mandatiory, but consistency is always useful/helpful
> 
> 
> Nits/administrativia:
> 
> 1. The RFC editor wants all references to have at least one citation
>    in the document. ALso for the normative references to RFCs from
>    which you import. See MIB review guidelines, sect 3.5
> 
>    You need to add a citation somehwere for [RFC3411], [RFC2021],
>    [RFC3291], [RFC2670]
> 
> Thanks,
> Bert
> 
> _______________________________________________
> IPCDN mailing list
> [email protected]
> https://www1.ietf.org/mailman/listinfo/ipcdn
> 

_______________________________________________
IPCDN mailing list
[email protected]
https://www1.ietf.org/mailman/listinfo/ipcdn