Re: [ipcdn] MIB Doctor review of: draft-ietf-ipcdn-subscriber-mib-06.txt

At 05:27 PM 10/1/2002 +0200, Wijnen, Bert (Bert) wrote:
>Took a bit longer than I had hoped. Oh well, here we go
>
>More or less serious issues/concerns:
>- 2nd para in Section 2 starts off with:
>     "Much of this MIB duplicates capabilities found in
>      the DOCSIS Cable Device MIB [17]."
>   So I immediately wonder why we need to duplicate any of
>   that other stuff, even without looking at the details.
>   Pls explain/justify.
>   The other praragraphs in this section try to explain some
>   of it I think. But what is not clear is: will this MIB
>   then obsolete the one in reference [17].
>   If it only partially obsoletes/replaces it, then maybe
>   that should be made clear and the [17] may need a
>   revision to deprecate the objects being replaced.

These objects were added to permit policy control to occur two places - in 
the CMTS and in the cable modem.  The original model (that described in the 
cable device mib) assumed that CMs were owned and controlled by the cable 
operator and that it would be pretty hard to hack them.  The new model 
makes the assumption that some cable modems might be the equivalent of 
WINMODEMS - e.g. a little bit of hardware to do modulation, with software 
to do all the rest of the functions - like filtering.  Since a filtering 
capability is critical for operator control of the service, this MIB was 
created to describe the functionality required in the CMTS where it 
wouldn't be subject to customer hacking of a software modem.  This is a 
seperate MIB from the other MIB, it mostly applies to a different device 
(the CMTS) - the other MIB will not necessarily be deprecated and you 
shouldn't consider whether it will or not in your comments.

>- In sect 2.1 you write:
>      "The docsSubMgtCpeControlTable, docsSubMgtCpeIpTable,
>       and docsSubMgtCmFilterTable augment the
>       docsIfCmtsCmStatusTable from [18]."
>   I do see this is true for docsSubMgtCpeControlTable but
>   for docsSubMgtCmFilterTable you are not using the AUGMENTS
>   clause. So maybe it is only a sparse augmentation? If so,
>   the better say so. If it is a real AUGMENT then use the
>   AUGMENTS clause.
>- Mmm... sect 2.2 starts to talk about the use of TFTTTP...
>   what does that have to do with the MIB? Maybe explain that
>   a bit (better). FOr example, why such is needed. And maybe
>   you can add a reference where people can find info about it.
>   The good news is that you do mention possible security aspects
>   (with references) in the Security Considerations section. Hope
>   it is acceptable to Security ADs.
>- Sect 2.3
>   I really wonder if this is smart. Why not give a much larger range
>   to the index objects and then specify via the MODULE COMPLIANCE
>   that they only need to support up to 1024. That way you do not
>   have to change the MIB in the future if you ever need more
>   than 1024. You then add another compliance statement (which
>   can actually be done in a different module/document)
>- The MIB MODULE-IDENTITY also talks about experimental.
>   You want to removethat if you go for STDs track
>- In the MIB you must use 4-digit year notation, so
>   change
>            LAST-UPDATED    "0202280000Z" -- February 28, 2002
>   into
>            LAST-UPDATED    "200202280000Z" -- February 28, 2002
>   and also
>            REVISION "0202280000Z" -- February 28, 2002
>   into
>            REVISION "200202280000Z" -- February 28, 2002
>- Normally, for a new RFC, we only have one REVISION clause. Whatever
>   changes were made during the WG process are not recorded in revision
>   clauses, so you can remove the 2nd revision clause.
>   In fact the one and only revision clause should say something like:
>
>            REVISION "200202280000Z" -- February 28, 2002
>            DESCRIPTION "Initial version, published as RFC xxxx."
>            -- RFC Editor to assign xxxx
>- I am not comfortable with you putting this MIB under { docsDev 5 }
>   How does DCOSIS keep track as to hwo makes assignments under
>   the docsDev OID subtree?
>   Why can this not be another assignment under mib-2 ??
>   Certainly it should not have been docsDev 5 while you were making
>   all sorts of changes during the WG process or while it was so
>   called "experimental" ...
>- I see:
>     docsSubMgtCpeControlReset OBJECT-TYPE
>        SYNTAX  TruthValue
>        MAX-ACCESS read-write
>        STATUS  current
>        DESCRIPTION
>            "This object always returns false on read.  If this object is
>        set to true, the rows with  'learned' addresses in
>        docsSubMgtCpeIpTable for this CM are deleted from that table."
>   Mmm.. so if I SET the object to true and do a GET afterwards then it is
>   back in false? I need to check if SNMP/MIB people are really happy with
>   this.

Bert -  This is the appropriate IDIOM for a "push to reset" toggle since 
the first mib was written and has been used in a number of books as an 
example of using a mib to control things.  What would you expect it to 
return?  "true if it was ever manually reset"?

>- I am kind of wondering about the AUGEMENTS for
>           AUGMENTS { docsIfCmtsCmStatusEntry }
>   Are all the WG members and potential implementers aware that you are
>   adding read-write objects to an otherwise read-only table?
>   I guess it would be good to spell out in the DESCRIPTION clause
>   of this table:
>   - that it AUGEMENTS (instead of extends) the docsIfCmtsCmStatusTable

Bert - directly from the SMIv2 - this needs no further description here 
about the meaning of AUGMENTS - this applies to the item above as well:

7.8.1.  Relation between INDEX and AUGMENTS clauses

    When defining instance identification information for a conceptual
    table:

(1)  If there is a one-to-one correspondence between the conceptual rows
      of this table and an existing table, then the AUGMENTS clause
      should be used.

(2)  Otherwise, if there is a sparse relationship between the conceptual
      rows of this table and an existing table, then an INDEX clause
      should be used which is identical to that in the existing table.
      For example, the relationship between RFC 2233's ifTable and a
      media-specific MIB which extends the ifTable for a specific media
      (e.g., the dot3Table in RFC 2358), is a sparse relationship.

(3)  Otherwise, if no existing objects have the required syntax and
      semantics, then auxiliary objects should be defined within the
      conceptual row for the new table, and those objects should be used
      within the INDEX clause for the conceptual row.

>   - that it adds 4 WRITEable objects.
>- the following 3 objects that define defaults. Can you explain how
>   that works and when they come into play. I see that hey do when the
>   registration fails. So is it that when a cable modem registers, then
>   this MIB-table (at a device in the provider premises?) gets populated
>   with whatever the user registers? And if he does not register you still
>   add entries to this MIB-table and the defaults get picked up in that case?
>   I may be confused, because neither in this document
>   nor in RFC2669/2670 did I find where this MIB and the others reside.
>   Would be good to have a picture of the devices involved, where they
>   are located adn what their function is.
>- table docsSubMgtPktFilterTable is a read-create table.
>   I see no StorageType object and no words at all about the persistency
>   of that table. We need to understand the behaviour for persistency.
>   This comment applies to all your read-create tables. In fact it also
>   applies (I think) to read-write values of other objects and other
>   tables as well.
>- I see:
>        docsSubMgtPktFilterGroup        Integer32,
>        docsSubMgtPktFilterIndex        Integer32,
>        docsSubMgtPktFilterSrcAddrType  InetAddressType,
>        docsSubMgtPktFilterSrcAddr      InetAddress,
>        docsSubMgtPktFilterSrcMaskType  InetAddressType,
>        docsSubMgtPktFilterSrcMask      InetAddress,
>        docsSubMgtPktFilterDstAddrType  InetAddressType,
>        docsSubMgtPktFilterDstAddr      InetAddress,
>        docsSubMgtPktFilterDstMaskType  InetAddressType,
>        docsSubMgtPktFilterDstMask      InetAddress,
>   A few remarks/questions
>   - that would give me the impression that the addressType for
>     address and mask could be different.
>   - it also gives me the impression that addressType for source
>     and destination can be different.
>   - From your COMPLIANCE section that seems not to be the case
>     for current implementers of this MIB.
>     Not clear if it will never be the case in the future.
>   - Anyway, if addressType is the same, then only one such object
>     is needed. Maybe an old version of the INET-ADDRESS-MIB did
>     suggest that you needed multiple. But the current version
>     as per RFC 3291.
>   - This new version also suggest to use InetAddressPrefixLength
>     instead of a mask. Pls take a look at that.
>- if the DEFVAL for    docsSubMgtPktFilterSrcAddrType is ipv4,
>   then why is the DEFVAL for    docsSubMgtPktFilterSrcAddr not 0.0.0.0?
>- same for Destination address
>- object docsSubMgtPktFilterUlp says:
>        DESCRIPTION
>        "Upper level protocol to match.  If this value is 256,
>        matches ALL ULP values.  Otherwise, this matches the specific
>        protocol value.  Note that if the packet ULP is either 6 (tcp) or
>        17 (udp), then docsSubMgtPktTcpUdpFilterTable must also be
>        consulted (if its entry exists) to see if this entry matches.
>        If this value is neither tcp(6) nor udp(17), then that
>        table is not consulted."
>   while object    docsSubMgtTcpUdpFilterTable says:
>        DESCRIPTION ".... This table
>             is not consulted unless the upper-layer protocol is TCP,
>             UDP, or 'any'."
>   So that seems to conflict with each other. (I would also add (256)
>   after 'any' so it is easier to make the connection.
>   In the first object you say it is onlu consulted for tcp and udp
>   and in the table itself you say that it is also consulted for 'any'
>- I see:
>    docsSubMgtPktFilterAction OBJECT-TYPE
>        SYNTAX      INTEGER
>                       {
>                       accept(1),
>                       drop(2)
>                       }
>        MAX-ACCESS  read-create
>        STATUS      current
>        DESCRIPTION
>            "The action to take upon this filter matching.  Accept means
>        to accept the packet for further processing.  Drop means to drop
>        the packet."
>    And I wonder: what does "further processing" mean? Will it be checked
>    against the next filter... or will it be forwarded, or what.
>    "drop" seems clear in that the packet gets dropped.
>- I see:
>        docsSubMgtTcpUdpSrcPort     Integer32,
>        docsSubMgtTcpUdpDstPort     Integer32,
>   These probably should be of type InetPortNumber instead, see RFC3291
>- object    docsSubMgtTcpFlagValues
>   says: ..................... An attempt to violate this constraint
>        returns an inconsistentValue error for an SNMPv2 or v3 agent
>        and a badValue error for an SNMPv1 agent.
>   We prefer that you just talk about the inconsistentValue error and
>   not mention SNMPvx at all. RFC2576 has common procedures to translate
>   such errors between different versions of SNMP.
>- For rowStatus objects, you MUST also indicate which objects must have
>   proper values before the row can be made active. See RowStatus TC
>   in RFC2579. SO this comment applies to all your rowStatus objects.
>- you have no Notifications, so there seems to be no need to define
>     docsSubMgtNotification OBJECT IDENTIFIER  ::= { docsSubMgt 2 }
>- Why have you commented out (in the MODULE COMPLIANCE)
>           -- SYNTAX InetAddressType { ipv4(1) }
>   because if you only requite support for IPv4, then that is
>   exactly how you specify it in the MIB. I know that some older
>   version of SMICng complians about it... but you can ignore that
>   error/warning. See for example RFC3289 that uses it too.
>   Now ... are cable modems not supporting IPv6 and will they not
>   do so in the (near) future?
>   Cuase. what might be a problem though is that in principle the
>   IETF requires support for IPv4 and IPv6 these days.
>   Thomas or Erik, any comment on that?
>- In the security section, you MUST list the objects that are
>   sensitive (read or read-write or read-create) and then for
>   each of them explain why they are sensitive and need protection.
>
>Administrative/editorial nits:
>- In the abstract, it talks about "experimental portion".
>   Should be just "portion" if you're heading for PS.
>- the RFC-Editor does not want unfamiliar acronyms in the
>   title. See http://www.rfc-editor.org/policy.html
>   I think MIB is OK, but DOSCIS is probably not OK.
>   Same comment for abstract.
>- 2nd para in abstract is not needed. That is already part
>   of the MIB boiler plate as you have it in sect 1.
>- references need to be split in normative and informative.
>   Again see: http://www.rfc-editor.org/policy.html
>   For the references in the MIB boiler-plate, a good sample
>   of the split is in draft-ietf-rmonmib-hc-alarm-mib-02.txt
>- object  docsSubMgtCpeIpAddr has a decription clause that
>   talks about "wiretapping". Not sure you want to use that term.
>
>Thanks,
>Bert
>_______________________________________________
>IPCDN mailing list
>IPCDN@ietf.org
>https://www1.ietf.org/mailman/listinfo/ipcdn

_______________________________________________
IPCDN mailing list
IPCDN@ietf.org
https://www1.ietf.org/mailman/listinfo/ipcdn