> On Mar 13, 2018, at 3:23 PM, Kent Watsen <[email protected]> wrote: > > Hi Mahesh, > > Please look for <KENT> below. > > Thanks, > Kent > > > On 3/8/18, 7:40 PM, "Mahesh Jethanandani" <[email protected] > <mailto:[email protected]>> wrote: > > Kent, > > >> On Mar 7, 2018, at 1:55 PM, Kent Watsen <[email protected] >> <mailto:[email protected]>> 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. >> >> 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. > > 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) > > Added. > > >> >> 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. > > OLD: > ACL is a ordered-by-user set of rules > > NEW: > 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”?
Ok. > > >> - what does "a tuple of" mean? Can this be restated? > > How about this? > > OLD: > The match criteria consist of a tuple of packet header match criteria and can > have metadata match criteria as well. > > NEW: > 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? Or simply as: “The match criteria allows for definition of packet headers and metadata, all of which must be true for the match to occur." > > > >> - s/In case vendor supports it/In case a vendor supports it/ ? > > Ok. > > >> - "The list of X is endless depending on...". Is "endless" the right word, >> perhaps restate? > OLD: > The list of potential actions is endless > > NEW: > The list of potential actions is limitless > > <KENT> or maybe "unbounded”? Ok. > > >> - 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 >> s/have/has/?) > > s/have/has/. > > >> - "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?) Once we agree on the above comment, I will replicate it. > > >> - "It as also possible for ACE to match on metadata" s/as/is/ and s/ACE/an >> ACE/ > > Ok > > >> - "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”? > > 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.” Ok. > > >> - "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 > > Removed. > > >> >> 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/ ? > > Done. > > >> >> In Section 4.1: >> - "ietf-access-control-list" is the standard top level module for access >> lists >> - what does this mean? > > OLD: > "ietf-access-control-list" is the standard top level module for access lists > > NEW: > "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 > "ietf-access-control-list" > module defines a container called "access-list" - what do you think? Ok. > > <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. History. When we inherited the draft, it was named access-lists. I can change it to “acls”. > > > >> - The "access-lists" container stores a list of "acl". - s/stores/has or >> contains?/ > > s/stores/has/ > > >> - "...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…"? > > Ok. > > >> - s/ability for ACL's to be/ability for ACLs to be/ > > Ok. > > >> >> 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? Ok. Will drop the word “primarily”. > > >> Separately, s/ACL/an ACL/? > > Ok. > > >> - A number of features read "Device can support..." - s/Device/The device/? > > Ok. > > >> - "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? But even nodes is a plural. So what would be the difference between “acls” and “acl nodes”. I would rather have the RFC editor deal with this. > > >> - "An Access Control List(ACL)" - put a space before (ACL) > > Ok. > > >> - " 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)/? > > Ok. > > >> - 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 > > Fixed. > > >> - "../../../../type" - still some long relative XPaths > > Fixed. > > >> - " 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” > > 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. Ok. > > >> - 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? Yes, it is possible for both the “egress-interface” and “ingress-interface” to be specified at the same time. > >> - s/The ACL's applied/The ACLs applied/ (this happens more than once in >> model) > > Fixed. > >> >> In Section 4.2: >> - references them by "uses" --> references them by 'uses' statements ??? > > Ok. > > >> - not all your 'reference' statements have the title of the referenced >> document. > > Fixed. > > >> - "then the datagram must be destroyed" - s/destroyed/dropped/? > > Ok. > > >> - "or referring to a group of ..." - same comments as for previous module >> - "ece" is missing a 'reference' statement? - > > Added. > > >> - "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 > > Ok. > > >> >> In Section 4.3: >> - the text says that it drops traffic from X to Y, but the example seems to >> do >> the reverse. > > Fixed. > > >> >> 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 >> ...:”? > > Fixed. > > > <KENT> BTW, I missed it before, but I think the 4.4 section title should be > plural: "Port Range Usage Examples” The title now reads: "Port Range Usage and Other Examples”. See below. > > >> - does the second example provide any value of the first? - seems the same >> to me… > > Will change the example. > > <KENT> was it changed per the next item below, or something else? Changed the example to an ACE entry that drops all ping requests. > >> - 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? > > 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” > > Ok. > > >> >> 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/ > > Fixed. > > >> >> In Section 6.1: >> - The first paragraph says "three URI", but it should be "three URIs” > > Fixed. > > >> >> 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 > > Added. > > >> - 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”. > > Thanks. > > <KENT>np > > > > 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, [email protected] >>> <mailto:[email protected]> 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= >>> >>> <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__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= >>> >>> <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= >>> >>> <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 >>> <https://urldefense.proofpoint.com/v2/url?u=http-3A__tools.ietf.org_&d=DwMFaQ&c=HAkYuh63rsuhr6Scbfh0UjBXeMK-ndb3voDTXcWzoCI&r=9zkP0xnJUvZGJ9EPoOH7Yhqn2gsBYaGTvjISlaJdcZo&m=BgyjnfSrZfswWrTMiW-PdKEJUEl3IXtwCSo1PQyVUaA&s=4FjGOld49GwI0moZ7h6ltluv0RXN1rPmGp0d-8mjDmA&e=>. >>> >>> 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= >>> >>> <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 >>> [email protected] <mailto:[email protected]> >>> 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= >>> >>> <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 >> [email protected] <mailto:[email protected]> >> >> _______________________________________________ >> netmod mailing list >> [email protected] <mailto:[email protected]> >> 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= >> >> <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 > [email protected] <mailto:[email protected]> > Mahesh Jethanandani [email protected]
_______________________________________________ netmod mailing list [email protected] https://www.ietf.org/mailman/listinfo/netmod
