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

base on my earlier comments on rev 06

> -----Original Message-----
> From: Wijnen, Bert (Bert) 
> Sent: dinsdag 1 oktober 2002 17:28
> To: wsawyer@ieee.org; ipcdn@ietf.org
> Cc: Thomas Narten (E-mail); Erik Nordmark (E-mail); Bert 
> Wijnen (E-mail)
> Subject: MIB Doctor review of: draft-ietf-ipcdn-subscriber-mib-06.txt
> 
> 
> 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.

I see text is added, also had some more input from the 
authors/editors. I have suggested to potentially add some more
of the explanations that were given to me to the document
in this section.
> - 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.

docsSubMgtCmFilterTable now uses AUGMENTS.
However docsSubMgtCpeIpTable does not. so it seems it rather
extends than augments?

> - 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.

I don't see any changes... but I won't hold you up on this

> - 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)

I had some private emails from authors/editors that I have 
now responded to. I see that this is now handled much better
(or at least in my opinion). I guess the WG agrees.

> - 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

fixed, thanks

> - 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

fixed, thanks

> - 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 saw comment from Mike. I checked with MIB doctors team.
here are some comments I got:

  - As long as there is 1) a time stamp, 2) doing this multiple times
    in a row doesn't mess anything up, and 3) it cannot fail,
    then it should be fine.
  - If you don't get a response back, how do you determine
    if the action was done? One way is a time stamp,
    another is a counter.
  - What happens when one writes a "false" into one of these?
    Why is it necessary for the object to have two possible
    values if only one is ever observed?

> - 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.

Mike reacted to the above that the first bullet is covered by RFC2578.
I know that. I intended the 2 bullets to go together.
I may not have bean clear. The warning in the description I feel
should explain that you are adding WRITABLE objects to a READ-ONLY
base table. That is something to be aware of I think.
I see that such has been done, thanks

> - 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 don;t see any additional explanation.
I guess all is clear to the WG and people who need to implement?

>   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?

Much better now, thanks

> - 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'

Fixed, thanks you

> - 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 no change. I will assume it is all clear to the WG and the
people who need to implement

> - I see:
>        docsSubMgtTcpUdpSrcPort     Integer32,
>        docsSubMgtTcpUdpDstPort     Integer32,
>   These probably should be of type InetPortNumber instead, see RFC3291

Fixed, thanks.

> - 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.

fixed, thanks

> - 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.

I see this has been fixed

> - you have no Notifications, so there seems to be no need to define
>     docsSubMgtNotification OBJECT IDENTIFIER  ::= { docsSubMgt 2 }

fixed, thanks

> - 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.

Fixed, thanks

>   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?

I leave that to Thomas and Erik

> - 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.
> 
Looks much better already... but see below

> 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.
> 

Look much better now.

Now ... since this all took a while... new SNMPv3 RFCs have come out.
That means a new MIB boiler plate as per
  http://www.ops.ietf.org/mib-boilerplate.html
And also new security considerations guidelines as per
  http://www.ops.ietf.org/security.html
The good news is they are much shorter and have far less
references. So I think it would be easy to pick up.
If not, then RFC-Editor will probably have to do it (or at
least update the current references to new RFC numbers).
While you're editing, I think this is easy to pick up.

We laso have another thing to add to the MODULE-DESCRIPTION clause
as per: http://www.ietf.org/IESG/STATEMENTS/MIB-COPYRIGHT.txt

It is easy to do too I think. If not done, then RFC-Editor will
add it as you can read. However, reducing load on RFC-Editor will
speed up things in the whole process.

Hope this helps,
Bert
> Thanks,
> Bert 
> 
_______________________________________________
IPCDN mailing list
IPCDN@ietf.org
https://www1.ietf.org/mailman/listinfo/ipcdn