Hi Mahesh,

Please look for <KENT> below.


On 3/8/18, 7:40 PM, "Mahesh Jethanandani" 
<mjethanand...@gmail.com<mailto:mjethanand...@gmail.com>> wrote:


On Mar 7, 2018, at 1:55 PM, Kent Watsen 
<kwat...@juniper.net<mailto:kwat...@juniper.net>> wrote:

[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.


From Idnits:

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

Hmm. The idnits at submission time did not complain. Will apply the new script 
that you provided to make sure I wrap them around.

 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?

How about this in both the Abstract and the Introduction.


ACL is a ordered-by-user set of rules

An ACL is a set of rules, in an order set by the user

<KENT>or how about "An ACL is a user-ordered set of rules"?

 - what does "a tuple of" mean?  Can this be restated?

How about this?


The match criteria consist of a tuple of packet header match criteria and can 
have metadata match criteria as well.


The match criteria consist of packet header matches, and or or metadata as 
described below:

<KENT>or how about "The match criteria can be a multiplicity of criteria, all 
of which must be true for the match to occur.   The match criteria may match 
against values in the packet header or against vendor-specific metadata about 
the packet."?   - or something in between?

 - 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?

The list of potential actions is endless


The list of potential actions is limitless

<KENT> or maybe "unbounded"?

 - same sentence as above, should "networked devices" be "network" or 
"networking" devices?

Will change “networked devices” to “networking devices”.

In Section 3:
 - "A network system usually have a list of ACLs"  (s/system/systems/ or 


 - "The match criteria consist of packet header matching" - is consist the 
right word?

How would you restate it? (After I have s/consist/consists/)

<KENT> see above (my comment before last, it is the same sentence, right?)

 - "It as also possible for ACE to match on metadata"  s/as/is/ and s/ACE/an 


 - "When applied to interfaces of a networked device, the ACL is applied in a 
    which indicates if it should be applied to packet entering (input) or 
leaving the
    device (output)."  - restate to talk about "ingress" and "egress”?

How about:

When applied to interfaces of a networked device, the ACL is applied in a 
direction which indicates if it should be applied to ingress interface (input) 
or egress interface (output).

<KENT>or maybe "When applied to interfaces of a networked device, distinct ACLs 
are defined for the ingress (input) and egress (output) directions."

 - "An example in the appendix shows how to express it in YANG model." - either 
   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/

Done and s/if-feature/‘if-feature’/

 - 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?

"ietf-access-control-list" is the standard top level module for access lists

"ietf-access-control-list" is the top level module for access lists

<KENT> it's more than the word "standard".  Maybe something like this:  The 
module defines a container called "access-list"  - what do you think?

<KENT>BTW, why is the container called "access-lists" and not e.g., "acls".  I 
thought that there was a node-naming idiom along the lines of "/widgets/widget" 
for when a list is a descendent of a container.

 - The "access-lists" container stores a list of "acl". - s/stores/has or 


 - "...that can be used to determine which rule was matched upon" - not sure if 
   part is needed, or maybe better restated ", which can later be used to 


 - 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?

As one of the text says. It primarily matches IPv4, and does not match either 
ethernet or IPv6 headers. Such ACL types are different from the mixed ACL types 
that might match on a combination of ethernet and IPv4 headers etc.

<KENT>But my comment is more that "primarily" seems wishy-washy.  It seems like 
it doesn't *primarily* do something, it actually does it.   If there is a grey 
area, where it might match something else, if possible, maybe it would help to 
call that out?

 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.

Really? English grammar says that if a sentence has both a singular and a 
plural, the one nearest to the subject is the one you select.

<KENT>actually, I'm just going off the fact that the list node is call "acl", 
which is singular.  Perhaps even better would be to say:  It can contain one or 
more "acl" nodes - thoughts?

 - "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 


 - 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 
   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”

How about:

Choice of source port definition using range/operator or referring to a group 
of source ports, to be added as a future 'case' statement.

<KENT>I like my framing better because 1) it is less committal about the future 
and 2) it doesn't limit there to being just one 'case' statement that might be 
added in the future.

 - 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?

Let me discuss this with my co-authors.

<KENT>any update on this?

 - s/The ACL's applied/The ACLs applied/   (this happens more than once in 


In Section 4.2:
 - references them by "uses" --> references them by 'uses' statements  ???


 - not all your 'reference' statements have the title of the referenced 


 - "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?

Added a reference statement.

 - 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 ...:”?


<KENT> BTW, I missed it before, but I think the 4.4 section title should be
plural: "Port Range Usage Examples"

 - does the second example provide any value of the first? - seems the same to 

Will change the example.

<KENT> was it changed per the next item below, or something else?

 - seems like example 3 could also be expressed as 
   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?

Have changed the language in the beginning of the section to say:

"When only a port is present, it represents a port, with the operator 
specifying the range."

That is because, it now a choice between specifying a range or specifying a 
single port with an operator.

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.

The examples can then be cut and pasted into any client such as ncclient which 
takes an entire <rpc>.

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.

Added a section in the Introduction.

 - s/In other example/In another example/?
 - s/with new choice of actions/with a new choice of actions/?

Both fixed.

In Section A.3;
 - some 'reference' statements are missing titles


 - some 'description' statements might benefit from a 'reference’ statement

I have added references that I could find.

 - "The uint16 type placeholder type..." - is this a typo?

Dropped the second “type”.



Kent // shepherd

===== 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, 
internet-dra...@ietf.org<mailto:internet-dra...@ietf.org> 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

 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:

There are also htmlized versions available at:

A diff from the previous version is available at:

Please note that it may take a couple of minutes from the time of submission
until the htmlized version and diff are available at 

Internet-Drafts are also available by anonymous FTP at:

netmod mailing list

Mahesh Jethanandani

netmod mailing list

Mahesh Jethanandani

netmod mailing list

Reply via email to