Re: [netmod] I-D Action: draft-ietf-netmod-acl-model-17.txt

[Kent Watsen <[e-mail]>]

[To all those that said this draft was ready, really?]


Hi Mahesh,

Thanks for the update.  I found some more issues.  Some must be fixed, 
others are nits, and might be caught by the RFC Editor.  But I think
that it's embarrassing to receive comments for such things from the 
IESG, as is recently the case for the syslog draft, so please see 
what you can do.

Thanks,
Kent


From Idnits:

  ** There are 6 instances of too long lines in the document, the longest one
     being 7 characters in excess of 72.

  You wrote before that it was "Fixed", but it's still here?  Note: "**" is
  an error (idnits label)

  -- The document has examples using IPv4 documentation addresses according
     to RFC6890, but does not use any IPv6 documentation addresses.  Maybe
     there should be IPv6 examples, too?

  I don't feel strongly about this, but if it's easy enough to do...

In the Abstract:
  - I think the word "an" is missing (e.g., an ACL)

In the Introduction:
  - should "ordered-by-user" be "ordered-by user" to avoid confusion, or perhaps say it another way?
  - what does "a tuple of" mean?  Can this be restated?
  - s/In case vendor supports it/In case a vendor supports it/ ?
  - "The list of X is endless depending on...".  Is "endless" the right word, perhaps restate?
  - same sentence as above, should "networked devices" be "network" or "networking" devices?

In Section 3:
  - "A network system usually have a list of ACLs"  (s/system/systems/ or s/have/has/?)
  - "The match criteria consist of packet header matching" - is consist the right word?
  - "It as also possible for ACE to match on metadata"  s/as/is/ and s/ACE/an ACE/
  - "When applied to interfaces of a networked device, the ACL is applied in a direction
     which indicates if it should be applied to packet entering (input) or leaving the
     device (output)."  - restate to talk about "ingress" and "egress"?
  - "An example in the appendix shows how to express it in YANG model." - either this
    is not true, or the sentence should not be at the end of this paragraph

In Section 3.1:
  - s/and must statements/and 'must' statements/
  - s/define new "matches" choice/define a new "matches" choice/ ?

In Section 4.1:
  - "ietf-access-control-list" is the standard top level module for access lists
      - what does this mean?
  - The "access-lists" container stores a list of "acl". - s/stores/has or contains?/ 
  - "...that can be used to determine which rule was matched upon" - not sure if this
    part is needed, or maybe better restated ", which can later be used to determine..."?
  - s/ability for ACL's to be/ability for ACLs to be/

In Section 4.1 (in the YANG module):
  - A number of identities read "ACL that primarily matches...".  Is "primarily"
    an accurate word? - if so, then do we need to say anything about when it's
    not the case?  Separately, s/ACL/an ACL/?
  - A number of features read "Device can support..." - s/Device/The device/?
  - "It can have one or more Access Control Lists" - lists should be singular.
  - "An Access Control List(ACL)" - put a space before (ACL)
  - " Indicates the primary intended" - here's that word "primary" again...
  - s/a list of access-list-entries(ACE)/ a list of access-list-entry nodes (ACE)/?
  - s/List of access list entries(ACE)/List of access list entry nodes (ACE)/?
      - there is more than one instance of this in the model
  - "../../../../type" - still some long relative XPaths
  - " or referring to a group of source ports" - this isn't there yet.  I think you
    want to say something like "this is a choice so as to support future 'case'
    statements, such as one enabling a group of source ports to be referenced"
  - ditto for "or referring to a group of destination ports."
  - ditto on both of the above for the "udp" container
  - is it possible for both "egress-interface" and "ingress-interface" leafs to 
    be specified at the same time?  - if not, should there a 'must' statement to
    prevent that possibility? - or an explanation for what happens if it occurs?
  - s/The ACL's applied/The ACLs applied/   (this happens more than once in model)

In Section 4.2:
  - references them by "uses" --> references them by 'uses' statements  ???
  - not all your 'reference' statements have the title of the referenced document.
  - "then the datagram must be destroyed" - s/destroyed/dropped/?
  - "or referring to a group of ..."  - same comments as for previous module
  - "ece" is missing a 'reference' statement?  - 
  - "Indicates that the Urgent pointer field is significant" - urgent is
    capitalized, but there's no context as for why.  Perhaps missing a
    reference statement too?
  - in "window-size" leaf description, remove parentheses

In Section 4.3:
  - the text says that it drops traffic from X to Y, but the example seems to do
    the reverse.

In Section 4.4:
  - The "With the follow XML example:" <EXAMPLE> "This represents..." is 
    difficult to read.  How about just having "The following XML example ...:"?
  - does the second example provide any value of the first? - seems the same to me...
  - seems like example 3 could also be expressed as "<lower-port>21</lower-port>",
    right?  - the text at the beginning of the section says this construct is
    possible, but there is no example for it.  Maybe this makes a better ex #2?

In all your YANG modules:
  - replace "NETMOD (NETCONF Data Modeling Language)" with "NETMOD (Network 
    Modeling) Working Group"

In Section ??:
  In the examples, why did you add the "<?xml version="1.0" encoding="UTF-8"?>"
  line and the "config" element?  - the examples validate equally well when
  these are removed.

In Section 6:
  - s/three YANG module/three YANG modules/

In Section 6.1:
  - The first paragraph says "three URI", but it should be "three URIs"

In Section A.1:
  - "The following figure is the tree structure" - should say "tree diagram" and
    should reference the tree-diagrams draft, or else have a draft-wide "Tree
    Diagram Notation" section in the Introduction.
  - s/In other example/In another example/?
  - s/with new choice of actions/with a new choice of actions/?

In Section A.3;
  - some 'reference' statements are missing titles
  - some 'description' statements might benefit from a 'reference' statement
  - "The uint16 type placeholder type..." - is this a typo?


Kent


===== original message ======

This version of the draft addresses comments raised during LC, shepherd review and other comments received during that period.

> On Mar 3, 2018, at 2:13 PM, [e-mail] wrote:
> 
> 
> A New Internet-Draft is available from the on-line Internet-Drafts directories.
> This draft is a work item of the Network Modeling WG of the IETF.
> 
>        Title           : Network Access Control List (ACL) YANG Data Model
>        Authors         : Mahesh Jethanandani
>                          Lisa Huang
>                          Sonal Agarwal
>                          Dana Blair
> 	Filename        : draft-ietf-netmod-acl-model-17.txt
> 	Pages           : 57
> 	Date            : 2018-03-03
> 
> Abstract:
>   This document defines a data model for Access Control List (ACL).
>   ACL is a ordered-by-user set of rules, used to configure the
>   forwarding behavior in device.  Each rule is used to find a match on
>   a packet, and define actions that will be performed on the packet.
> 
> 
> The IETF datatracker status page for this draft is:
> https://urldefense.proofpoint.com/v2/url?u=https-3A__datatracker.ietf.org_doc_draft-2Dietf-2Dnetmod-2Dacl-2Dmodel_&d=DwICAg&c=HAkYuh63rsuhr6Scbfh0UjBXeMK-ndb3voDTXcWzoCI&r=9zkP0xnJUvZGJ9EPoOH7Yhqn2gsBYaGTvjISlaJdcZo&m=huBe-BRKk8B5XCRf7lG_gWUwZNr67SAA8GHfgYnyZoc&s=44GJlqxqB0YK5G9gb1TUzAobugMHxDWflaPCZ3IYpKA&e=
> 
> There are also htmlized versions available at:
> https://urldefense.proofpoint.com/v2/url?u=https-3A__tools.ietf.org_html_draft-2Dietf-2Dnetmod-2Dacl-2Dmodel-2D17&d=DwICAg&c=HAkYuh63rsuhr6Scbfh0UjBXeMK-ndb3voDTXcWzoCI&r=9zkP0xnJUvZGJ9EPoOH7Yhqn2gsBYaGTvjISlaJdcZo&m=huBe-BRKk8B5XCRf7lG_gWUwZNr67SAA8GHfgYnyZoc&s=rbm91SSJ_0sxFxb692d0FH0G-dbBTAUCf2KRySyztJQ&e=
> https://urldefense.proofpoint.com/v2/url?u=https-3A__datatracker.ietf.org_doc_html_draft-2Dietf-2Dnetmod-2Dacl-2Dmodel-2D17&d=DwICAg&c=HAkYuh63rsuhr6Scbfh0UjBXeMK-ndb3voDTXcWzoCI&r=9zkP0xnJUvZGJ9EPoOH7Yhqn2gsBYaGTvjISlaJdcZo&m=huBe-BRKk8B5XCRf7lG_gWUwZNr67SAA8GHfgYnyZoc&s=siypyBn3F8o6bsB3Z3E5qS0uaSq2EUGUPwirx_a_KDw&e=
> 
> A diff from the previous version is available at:
> https://urldefense.proofpoint.com/v2/url?u=https-3A__www.ietf.org_rfcdiff-3Furl2-3Ddraft-2Dietf-2Dnetmod-2Dacl-2Dmodel-2D17&d=DwICAg&c=HAkYuh63rsuhr6Scbfh0UjBXeMK-ndb3voDTXcWzoCI&r=9zkP0xnJUvZGJ9EPoOH7Yhqn2gsBYaGTvjISlaJdcZo&m=huBe-BRKk8B5XCRf7lG_gWUwZNr67SAA8GHfgYnyZoc&s=t2lpzSSW72BvQK1VjPoxX0ADxhb9ZD0fp3fXqcd80g8&e=
> 
> 
> Please note that it may take a couple of minutes from the time of submission
> until the htmlized version and diff are available at tools.ietf.org.
> 
> Internet-Drafts are also available by anonymous FTP at:
> https://urldefense.proofpoint.com/v2/url?u=ftp-3A__ftp.ietf.org_internet-2Ddrafts_&d=DwICAg&c=HAkYuh63rsuhr6Scbfh0UjBXeMK-ndb3voDTXcWzoCI&r=9zkP0xnJUvZGJ9EPoOH7Yhqn2gsBYaGTvjISlaJdcZo&m=huBe-BRKk8B5XCRf7lG_gWUwZNr67SAA8GHfgYnyZoc&s=xP7z9VxUgtOtSVIgqPF5RKIqTOi6wj-HEXvZKBRTiUw&e=
> 
> _______________________________________________
> netmod mailing list
> [e-mail]
> https://urldefense.proofpoint.com/v2/url?u=https-3A__www.ietf.org_mailman_listinfo_netmod&d=DwICAg&c=HAkYuh63rsuhr6Scbfh0UjBXeMK-ndb3voDTXcWzoCI&r=9zkP0xnJUvZGJ9EPoOH7Yhqn2gsBYaGTvjISlaJdcZo&m=huBe-BRKk8B5XCRf7lG_gWUwZNr67SAA8GHfgYnyZoc&s=OKIVLXLo0Rsrf1DSoLWSyHj97DuE6vuaJ4Cqk_oi1HA&e=

Mahesh Jethanandani
[e-mail]

_______________________________________________
netmod mailing list
[e-mail]
https://urldefense.proofpoint.com/v2/url?u=https-3A__www.ietf.org_mailman_listinfo_netmod&d=DwICAg&c=HAkYuh63rsuhr6Scbfh0UjBXeMK-ndb3voDTXcWzoCI&r=9zkP0xnJUvZGJ9EPoOH7Yhqn2gsBYaGTvjISlaJdcZo&m=huBe-BRKk8B5XCRf7lG_gWUwZNr67SAA8GHfgYnyZoc&s=OKIVLXLo0Rsrf1DSoLWSyHj97DuE6vuaJ4Cqk_oi1HA&e=