Re: midcom revised mib review

Dear all,

For your convenience, please find the new version -07 at
<ftp://195.37.70.21/pub/midcom/draft-ietf-midcom-mib-07.txt>

and a diff to the previous version -06 at
<ftp://195.37.70.21/pub/midcom/diff-06-07.html>

    Juergen Q.


--On 07.06.2006 15:38 Uhr +0200 Juergen Quittek wrote:

> Hi Juergen,
>
> Many thanks for the review!
>
> Martin and I went through your comments and fixed the draft.
> Please find our replies inline below.
> Please have a look at issues h), m), n), and s).
>
> Thanks,
>
>     Juergen Q.
>
> --On 31.05.2006 15:08 Uhr +0200 Juergen Schoenwaelder wrote:
>
>> 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"
>
> fixed.  We chose "MIDCOM clients".
>
>> b) p13: "for request message and reply message" -> "for the request
>>         and the reply message"
>
> fixed.
>> c) p17: "For example, it can" -> "For example, they can"
>
> fixed.
>
>> d) p22: "The values provide" -> "The values provided"
>
> fixed.
>
>> e) p26: "the idle or" -> "the idle time"
>
> fixed.
>
>> f) p27: "MIDCOm" -> "MIDCOM"
>
> fixed.
>
>> g) p29: "For some application" -> "For some applications"
>
> fixed.
>
>> h) p29: I am wondering why the document does not suggest to use
>>         snmpSetSerialNo to solve the idempotency problem.
>
> Actually, we did not find any MIB module using this feature.
> Therefore, we do not expect big support form implementations.
>
> Also, its DESCRIPTION clause
>
>            "An advisory lock used to allow several cooperating
>            command generator applications to coordinate their
>            use of the SNMP set operation. ...
>
> says that it is to be used for coordinating between multiple
> command generators.  Our idempotency problem is rather related
> to re-sent requests from a single command generator.
>
>> i) p30: "below recommended" -> "below are recommended"
>
> fixed.
>
>> 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.
>
> done for all of them.
>
>> 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.
>
> fixed.
>
>> 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).
>
> done.
>
>> m) p42: Should 0 not be excluded from midcomRuleIndex? You have done
>>         this for the midcomGroupIndex.
>
> In an earlier version we had a special semantics for references to groups
> with index 0.  therefore, we excluded 0 from the set of values for
> midcomGroupIndex.  When we removed this feature, we forgot to include value
> 0 again.
>
> Consequently, the suggestion is allowing 0 also for midcomGroupIndex.
>
> Would there be a problem with using index 0 for midcomRuleIndex and
> 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.
>
> We added value notSet(3) to this object and the following paragraph to
> the DESCRIPTION clause.
>
>          When created an midcomRuleEntry is created without
>          explicitly setting this object, its value will be notSet(3).
>          However, a set request can only set this object to either
>          reserve(1) or enable(2).  Attempts to set this object to
>          notSet(3) will always fail with an inconsistentValue error.
>
>> 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.
>
> fixed.
>
>> p) p46: "rule is expired" -> "rule has expired"
>
> fixed.
>
>> 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...
>
> Thank you.
> We do not see a straight way to supporting localization here.
>
>> 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.
>
> replaced
>     SYNTAX Unsigned32 (0..128)
> with
>     SYNTAX InetAddressPrefixLength
> for both objects.
>
>> 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".
>
> In all discussions of related issues on the mailing list, the term
> "persistent" was used instead of "nonVolatile".  Therefore, we used
> it for naming this object.  If you insist we can change it to
> midcomConfigNonVolatileRules, but we prefer the current name.
>
> Anyway, we appended the folowing paragraph to the DESCRIPTION clause.
>
>          A value of true(1) indicates that there may be
>          entries in the midcomRuleTable with object
>          midcomRuleStorageType set to value nonVolatile(3).
>
>> t) p63: First sentence in the DESCRIPTION is missing some words.
>
> fixed.  Just one word missing.
>
>> u) p67: "inteface" -> "interface"
>
> fixed.
>
>> v) p73: "request. (" -> "request ("
>
> fixed.
>
>> w) p74: "request. (" -> "request ("
>
> fixed.
>
>> x) p81: "to managed object" -> "to the managed object"
>
> fixed.
>
>> /js
>>
>> --
>> Juergen Schoenwaelder		    International University Bremen
>> <http://www.eecs.iu-bremen.de/>	    P.O. Box 750 561, 28725 Bremen, Germany
>
>