Re: [netmod] WG Last Call: draft-ietf-netmod-acl-model-01.txt

[Juergen Schoenwaelder <[e-mail]>]

On Fri, Feb 06, 2015 at 09:15:46PM +0100, Juergen Schoenwaelder wrote:
> On Fri, Feb 06, 2015 at 01:59:30PM -0500, Thomas D. Nadeau wrote:
> > 
> > This commences a NETMOD WG Last call for draft-ietf-netmod-acl-model-01.txt.  Please send comments to the list by 20-FEB-2015 by 9AM EST.
> >
> 
> Before I read this in detail already a few quick comments:
> 
> - The module name 'packet-fields' should follow RFC 6020 guidelines.
> 
> - Non-normative example modules should be in an appendix marked as
>   non-normative.
> 
> - The security considerations include a TBD action item.
> 
> - If ietf-route-filter.yang is an example, I would probably give it a
>   different name that has less chance to be confused with an official
>   module.
>

More comments:

- Why do you use the prefix 'ietf' for ietf-yang-types? The default
  would have been 'yang'.

- How is the packet-fields module updated? IETF process? Is this a
  candidate for IANA maintenance?

- Do we need a copyright notice in the module descriptions?

- The description of the revision does not make much sense, and the
  reference even less. I suggest to follow the YANG guidelines,
  RFC 6087.

- Is layer 2 always ethernet or is this

    identity eth-acl {
       base "acl:acl-base";
       description "layer 2 ACL type";
     }

  just a misnomer? Perhaps the description of ip-acl also should be
  "IP layer ACL type" to be precise. But then, the associated grouping
  also includes layer 4 elements (port number ranges).

- Since access-list/acl-type is optional, what is the meaning if it is
  not present?

- I have no clue what to expect in the targets strings in
  acl-oper-data/targets.

- The description of access-list talks about sequence numbers - I
  could not find them. Perhaps the text is outdated?

- I personally would not inline the operational state nodes but this
  may be personal preference.

- Is 'entry' the same as 'rule'? If so, why do we use two terms if one
  would be sufficient?

- I do not understand this:

      The time range is identified by a name
      and then referenced by a function, so that those
      time restrictions are imposed on the function itself.

  I was surprised to find this since the introductory text never
  mentions time filters, it only talks about about packet filters and
  metadata filters. Well, this is filtering on the meta data when a
  packet is received but perhaps this deserves to be mentioned
  somewhere earlier.

- You will be asked later on anyway to use the reserved example IP
  addresses in the examples, so make the changes now.

- In the example in section 4.4, I suggest to remove all the
  edit-config stuff and to show only the config itself. I am also not
  sure the example is correct, has this been validated against the
  schema?

- The explanation given at the beginning of section 4.5 belongs into
  the data model definition. What happens if I configure only an
  upper-port? What if I leave both ports out? What happens if the
  lower-port is larger than the upper-port?

- I generally suggest to use the prefixed defined in an imported
  module unless there is a name clash that makes this impossible.  If
  we always write inet:ipv4-prefix (where possible), it becomes
  simpler for human readers to read modules.

- reference " "; avoids a warning but is otherwise not useful

- The biggest thing that is unclear to me is how these generic acls
  are used in certain contexts. In the operational state, there is a
  list of opaque target strings but I do not know what they contain or
  how they are configured. So how do I use these generic acls?
  Suppose I want to use these acls to control access to my NC
  server. How would I have to extend the NC server config model to do
  that?

- The IANA considerations are incomplete since I think you have two
  modules that need registration.

- Since Section 10 is empty, remove it.

/js

-- 
Juergen Schoenwaelder           Jacobs University Bremen gGmbH
Phone: [phone]         Campus Ring 1 | 28759 Bremen | Germany
Fax:   [phone]         <http://www.jacobs-university.de/>