[midcom] midcom revised mib review

[Juergen Schoenwaelder <[e-mail]>]

Hi,

first of all, I have to apologize for the delay (I will spare you the
reasons you might not be interested in anyway).

I have read <draft-ietf-midcom-mib-06.txt> once again carefully. I
think it is generally of high quality and I do not have major problems
with it. Still, I have a few comments I like to share with you and
perhaps some of the things can still be improved without much further
delay.

a) p6: "sending notification to MIDCOM client" - either "a MIDCOM
       client" or "MIDCOM clients"

b) p13: "for request message and reply message" -> "for the request
        and the reply message"

c) p17: "For example, it can" -> "For example, they can"

d) p22: "The values provide" -> "The values provided"

e) p26: "the idle or" -> "the idle time"

f) p27: "MIDCOm" -> "MIDCOM"

g) p29: "For some application" -> "For some applications"

h) p29: I am wondering why the document does not suggest to use
        snmpSetSerialNo to solve the idempotency problem.

i) p30: "below recommended" -> "below are recommended"

j) p31ff: I am still somewhat concerned about the fact that the text
          encourages implementors to potentially do the wrong thing.
          You can't rely on notifications. While step 7. in section
          7.3 says the right thing, I would prefer if the text would
          actually be moved where it belonts, namely in step 4. So
          here is what I propose how step 4 should be written:

   4. The MIDCOM client awaits a midcomSolicitedRuleEvent notification
      concerning the new policy rule in the midcomRuleTable.  Waiting
      for the notification is timed out after a pre-selected maximum
      waiting time. In case of a timeout while waiting for the
      notification or if the client does not use notifications, the
      MIDCOM client retrieves the status of the midcomRuleEntry by one
      or more SNMP get operation.

          By moving the text into this step, you can get rid of step 7
	  and make it clearer that you have to write code to poll for
	  the completion of the operation anyways.

	  Note that this change also affects subsequent sections,
	  namely 7.4, 7.5, 7.6, and 7.10.

k) p33: Step 5 in 7.5 refers to step 5 in 7.3 but I think it should be
        step 4 in 7.3.

l) p42: The description of midcomRuleOwner is a bit confusing. It says:

            This object SHOULD uniquely identify an authenticated
            MIDCOM client.  It is of type SnmpAdminString, a textual
            convention that allows for use of the SNMPv3 View-Based
            Access Control Model (RFC 3415, VACM) and allows an
            management application to identify its entries."

        This sounds like the SnmpAdminString TC has something special
        concerning VACM which is not true. Perhaps less is more:

            This object SHOULD uniquely identify an authenticated
            MIDCOM client. This object is part of the table index to
            allow for the use of the SNMPv3 View-Based Access Control
            Model (RFC 3415, VACM).

m) p42: Should 0 not be excluded from midcomRuleIndex? You have done
        this for the midcomGroupIndex.

n) p44: The description of midcomRuleOperStatus says that setting(2)
        indicates that no request was made. I am not sure how this can
        actually happen since midcomRuleAdminStatus is either
        reserve(1) or enable(2) and hence during row creation it has
        to take on one of these values. Perhaps a cleaner way to deal
        with this would be to add another state off(3) or whatever you
        want to call it which can be used when a row is created
        without already setting reserve(1) or enable(2). The other
        option would be to be very clear that a midcom client has to
        set either reserve(1) or enable(2) while creating a row in
        which case the state setting(2) does not make much sense to
        me.

o) p45: "can be retrieved" -> "are meaningful" (twice on that page).
	My understanding is that you can always retrieve all columns
	but depending of the midcomRuleAdminStatus only some of them
	are meaningful.

p) p46: "rule is expired" -> "rule has expired"

q) p48: midcomRuleError does not support localization and this seems
        to be a string that is likely to be shown to human users. I am
        just mentioning this...

r) p54: Is there a reason why you do not use the
        InetAddressPrefixLength TC for
        midcomRuleInternalIpPrefixLength and
        midcomRuleExternalIpPrefixLength? In case you use
        InetAddressPrefixLength, make sure you allocate the max value
        for the "no wildcarding" semantics and not just 128.

s) p63: How does midcomConfigPersistentRules interact with the
        StorageType objects? In the light of StorageType, the term
        "persistent" is probably wrong and should be "nonVolatile".

t) p63: First sentence in the DESCRIPTION is missing some words.

u) p67: "inteface" -> "interface"

v) p73: "request. (" -> "request ("

w) p74: "request. (" -> "request ("

x) p81: "to managed object" -> "to the managed object"

/js

-- 
Juergen Schoenwaelder		    International University Bremen
<http://www.eecs.iu-bremen.de/>	    P.O. Box 750 561, 28725 Bremen, Germany

_______________________________________________
midcom mailing list
[e-mail]
https://www1.ietf.org/mailman/listinfo/midcom