draft-ietf-ipsp-config-policy-model comments

[Wes Hardaker <wes@xxxxxxxxxxxxx>]

(Some of these could have been brought up before, I didn't check the archives.)

1) The definitions for SARule, SAAction, SAStaticAction,
   SANegotiationAction and IPsecAction contain the following sentence:

     "Although the class is concrete, is MUST not be instantiated."

   I suspect you might as well use the fully-capitalized wording for
   "must not":

     "Although the class is concrete, is MUST NOT be instantiated."
                                              ^^^

2) Regarding the above, why not just make the classes abstract to
   ensure they're not instantiated?

3) Some of the objects (EG, IKERuleOverridePoint) contain high to low
   precedence (higher numbers have a larger precedence) and others
   (EG, IPsecPolicyGroupInPolicyGroup's "Precedence" attribute) have
   low to high precedence where lower numbers have a larger
   precedence.  Although not important from a technical prospective,
   it would make more sense to have this be consistent across the
   entire document unless there is a reason its done like it is.

4) Section 5.12 specifies the property of FQDNFilterEntry as "Name"
   but section 5.12.1 which describes the property has a name of
   "Address" instead of "Name".

5) Section 6.2 which describes "The Class SAStaticAction" says that it
   "serves as the base class for IKE and IPsec actions that
   do not require negotiation.
   ^^^^^^^^^^^^^^^^^^^^^^^^^^

   But yet, section 6.2.1 "The property LifetimeSeconds" describes the
   property value with "A non-zero value is typically used in
   conjunction with fallback actions performed when there is a
   negotiation failure of some sort."
   ^^^^^^^^^^^^^^^^^^^

   To lead away from possible confusion, I'd suggest rewording the
   above to something like: "A non-zero value is typically used in
   conjunction with fallback action from a failed SANegotiationAction
   to an SAStaticAction after a negotiation failure of some sort has
   taken place."

6) section 6.7.3, 6.7.4 and 6.11.1 say that "A random value may be
   added...".  I think I'd suggest making that "may" be a "SHOULD" and
   if not that then possibly a "MAY".

-- 
Wes Hardaker
NAI Labs
Network Associates