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

["Wijnen, Bert (Bert)" <[e-mail]>]

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.
- 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.
- 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
  - 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
[e-mail]
https://www1.ietf.org/mailman/listinfo/ipcdn