Re: [netmod] ACL draft issues found during shepherd writeup

Kent Watsen <kwatsen@juniper.net> Fri, 23 February 2018 20:12 UTC

Return-Path: <kwatsen@juniper.net>
X-Original-To: netmod@ietfa.amsl.com
Delivered-To: netmod@ietfa.amsl.com
Received: from localhost (localhost [127.0.0.1]) by ietfa.amsl.com (Postfix) with ESMTP id C7292124B18; Fri, 23 Feb 2018 12:12:49 -0800 (PST)
X-Virus-Scanned: amavisd-new at amsl.com
X-Spam-Flag: NO
X-Spam-Score: -2.7
X-Spam-Level:
X-Spam-Status: No, score=-2.7 tagged_above=-999 required=5 tests=[BAYES_00=-1.9, DKIM_SIGNED=0.1, DKIM_VALID=-0.1, DKIM_VALID_AU=-0.1, HTML_MESSAGE=0.001, RCVD_IN_DNSWL_LOW=-0.7, SPF_PASS=-0.001] autolearn=ham autolearn_force=no
Authentication-Results: ietfa.amsl.com (amavisd-new); dkim=pass (2048-bit key) header.d=juniper.net
Received: from mail.ietf.org ([4.31.198.44]) by localhost (ietfa.amsl.com [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id jcavjm6LA531; Fri, 23 Feb 2018 12:12:46 -0800 (PST)
Received: from mx0a-00273201.pphosted.com (mx0a-00273201.pphosted.com [208.84.65.16]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ietfa.amsl.com (Postfix) with ESMTPS id 133AD1201F8; Fri, 23 Feb 2018 12:12:46 -0800 (PST)
Received: from pps.filterd (m0108159.ppops.net [127.0.0.1]) by mx0a-00273201.pphosted.com (8.16.0.22/8.16.0.22) with SMTP id w1NK9pGX016997; Fri, 23 Feb 2018 12:12:44 -0800
DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=juniper.net; h=from : to : cc : subject : date : message-id : references : in-reply-to : content-type : mime-version; s=PPS1017; bh=rcF21n89/m1KcDtZfSnFgpkCfPbOt5BIGD//KQibvH8=; b=oj09KY3M+6dDx218yCmFYOAXSzBQOH1zgNiIkbCzteB1hTGM/4aLqOGYJbCZUyWcCaOF jCqS7IWDyQxR5lJh5jiTBafyOcPCqRwSzT/C90iL/4FovmN2/49hIYg79FX5mWwArvAK ui1wZQ6oOP8y1aHgCZws+vYKMOujI76/0AfOw2d3j8oD/vzVCBzVkBCB0F4SjF6RQGoR xXJ3ZLbx/BtbulydfV++vVuvnY3K1Z/w01iX8SSPm2Dz51GzXMSOI1rBGsXC7iGbhpjg hc79WnoiYl9VcT7GFsjzzQNfyyOz4FgPBt233s2l90/+qI/wma2K+I2tBRnIvP8KrgRQ Rw==
Received: from nam02-bl2-obe.outbound.protection.outlook.com (mail-bl2nam02lp0084.outbound.protection.outlook.com [207.46.163.84]) by mx0a-00273201.pphosted.com with ESMTP id 2gaser019f-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-SHA384 bits=256 verify=NOT); Fri, 23 Feb 2018 12:12:43 -0800
Received: from DM5PR05MB3484.namprd05.prod.outlook.com (10.174.240.147) by DM5PR05MB3306.namprd05.prod.outlook.com (10.174.191.33) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.548.6; Fri, 23 Feb 2018 20:12:39 +0000
Received: from DM5PR05MB3484.namprd05.prod.outlook.com ([fe80::507:f464:b89a:c64b]) by DM5PR05MB3484.namprd05.prod.outlook.com ([fe80::507:f464:b89a:c64b%3]) with mapi id 15.20.0548.005; Fri, 23 Feb 2018 20:12:39 +0000
From: Kent Watsen <kwatsen@juniper.net>
To: Mahesh Jethanandani <mjethanandani@gmail.com>
CC: "draft-ietf-netmod-acl-model@ietf.org" <draft-ietf-netmod-acl-model@ietf.org>, "netmod@ietf.org" <netmod@ietf.org>
Thread-Topic: [netmod] ACL draft issues found during shepherd writeup
Thread-Index: AQHTpRpSR0BIQpp1FUa5qmqsmhR9+aOpZLiAgAjCjgA=
Date: Fri, 23 Feb 2018 20:12:39 +0000
Message-ID: <8D3773A8-ECA6-406A-B28D-6DD44F951F10@juniper.net>
References: <14BA9086-69D4-4BAF-A7C7-0EB1F3F400BB@juniper.net> <2864E0CF-D038-4FDA-B69C-FD43F486BF17@gmail.com>
In-Reply-To: <2864E0CF-D038-4FDA-B69C-FD43F486BF17@gmail.com>
Accept-Language: en-US
Content-Language: en-US
X-MS-Has-Attach:
X-MS-TNEF-Correlator:
user-agent: Microsoft-MacOutlook/f.20.0.170309
x-originating-ip: [66.129.241.14]
x-ms-publictraffictype: Email
x-microsoft-exchange-diagnostics: 1; DM5PR05MB3306; 7:/7abxAN2RlyBIVKfs+aveP74brTFE2ym5rcCBDYz0wq2/mkWxTWWI/M2VDLpE+E+GsD4erpP8zzEhstCfRG8uEtx3Sl3jGMgHd57ksRIn5PycliYoH68kDqXNqvVfJmxNwMfrdzSneWhOk6KES8san0p9uALUxXDqcqnC6a0/Uv0HOFffWhjIo6is225BIXilO9Vkv3LJa1i07OhTwV00QNBrg8SCIk49J4AC6uOp3vqInGYi7sXQRMCgfUrhFiz
x-ms-exchange-antispam-srfa-diagnostics: SSOS;
x-ms-office365-filtering-ht: Tenant
x-ms-office365-filtering-correlation-id: 4c7d8fdb-3911-40d7-fa06-08d57af9c9bb
x-microsoft-antispam: UriScan:; BCL:0; PCL:0; RULEID:(7020095)(4652020)(4534165)(4627221)(201703031133081)(201702281549075)(48565401081)(5600026)(4604075)(3008032)(2017052603307)(7153060)(7193020); SRVR:DM5PR05MB3306;
x-ms-traffictypediagnostic: DM5PR05MB3306:
x-microsoft-antispam-prvs: <DM5PR05MB330662898ABA70743DA14137A5CC0@DM5PR05MB3306.namprd05.prod.outlook.com>
x-exchange-antispam-report-test: UriScan:(28532068793085)(10436049006162)(192374486261705)(138986009662008)(85827821059158)(788757137089)(100405760836317)(21748063052155);
x-exchange-antispam-report-cfa-test: BCL:0; PCL:0; RULEID:(8211001083)(6040501)(2401047)(5005006)(8121501046)(3231101)(944501161)(3002001)(93006095)(93001095)(10201501046)(6055026)(6041288)(201703131423095)(201702281528075)(20161123555045)(201703061421075)(201703061406153)(20161123564045)(20161123562045)(20161123558120)(20161123560045)(6072148)(201708071742011); SRVR:DM5PR05MB3306; BCL:0; PCL:0; RULEID:; SRVR:DM5PR05MB3306;
x-forefront-prvs: 0592A9FDE6
x-forefront-antispam-report: SFV:NSPM; SFS:(10019020)(39860400002)(39380400002)(366004)(346002)(376002)(396003)(85644002)(377424004)(199004)(189003)(478600001)(8676002)(6512007)(2950100002)(33656002)(81156014)(26005)(4326008)(6916009)(1411001)(9326002)(6506007)(53546011)(102836004)(106356001)(7736002)(14454004)(59450400001)(81166006)(2906002)(82746002)(3660700001)(6436002)(99286004)(8936002)(83716003)(966005)(105586002)(39060400002)(76176011)(6246003)(5660300001)(53376002)(25786009)(68736007)(6116002)(3846002)(316002)(54896002)(6306002)(6486002)(3280700002)(58126008)(236005)(2900100001)(36756003)(606006)(53936002)(97736004)(5250100002)(86362001)(229853002)(575784001)(186003)(54906003)(66066001); DIR:OUT; SFP:1102; SCL:1; SRVR:DM5PR05MB3306; H:DM5PR05MB3484.namprd05.prod.outlook.com; FPR:; SPF:None; PTR:InfoNoRecords; A:1; MX:1; LANG:en;
received-spf: None (protection.outlook.com: juniper.net does not designate permitted sender hosts)
x-microsoft-antispam-message-info: LNJjXei3vtlJeaRkZrGuQxfQiRIRcFCCUujJsOBonNRDGhT7+ePLf+AoDbFr0Q4J3HwRASXk2pOtEcwJjVMp3W68n8Wfp0631d9Cs+keaHUWJAFJfMWwxcK8hR3gkyTt/1J08fylWbFLxsOnn67c/Rv1p+yNKC4ivUuOJpsNUUc=
spamdiagnosticoutput: 1:99
spamdiagnosticmetadata: NSPM
Content-Type: multipart/alternative; boundary="_000_8D3773A8ECA6406AB28D6DD44F951F10junipernet_"
MIME-Version: 1.0
X-OriginatorOrg: juniper.net
X-MS-Exchange-CrossTenant-Network-Message-Id: 4c7d8fdb-3911-40d7-fa06-08d57af9c9bb
X-MS-Exchange-CrossTenant-originalarrivaltime: 23 Feb 2018 20:12:39.1977 (UTC)
X-MS-Exchange-CrossTenant-fromentityheader: Hosted
X-MS-Exchange-CrossTenant-id: bea78b3c-4cdb-4130-854a-1d193232e5f4
X-MS-Exchange-Transport-CrossTenantHeadersStamped: DM5PR05MB3306
X-Proofpoint-Virus-Version: vendor=fsecure engine=2.50.10432:, , definitions=2018-02-23_07:, , signatures=0
X-Proofpoint-Spam-Details: rule=outbound_spam_notspam policy=outbound_spam score=0 priorityscore=1501 malwarescore=0 suspectscore=0 phishscore=0 bulkscore=0 spamscore=0 clxscore=1011 lowpriorityscore=0 mlxscore=0 impostorscore=0 mlxlogscore=999 adultscore=0 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.0.1-1711220000 definitions=main-1802230246
Archived-At: <https://mailarchive.ietf.org/arch/msg/netmod/qi4DvMJhlQwqpTjXqsbSEcl7zfI>
Subject: Re: [netmod] ACL draft issues found during shepherd writeup
X-BeenThere: netmod@ietf.org
X-Mailman-Version: 2.1.22
Precedence: list
List-Id: NETMOD WG list <netmod.ietf.org>
List-Unsubscribe: <https://www.ietf.org/mailman/options/netmod>, <mailto:netmod-request@ietf.org?subject=unsubscribe>
List-Archive: <https://mailarchive.ietf.org/arch/browse/netmod/>
List-Post: <mailto:netmod@ietf.org>
List-Help: <mailto:netmod-request@ietf.org?subject=help>
List-Subscribe: <https://www.ietf.org/mailman/listinfo/netmod>, <mailto:netmod-request@ietf.org?subject=subscribe>
X-List-Received-Date: Fri, 23 Feb 2018 20:12:50 -0000

Hi Mahesh,

Please search for <KENT> below (6 instances)

Thanks,
Kent // shepherd


On 2/17/18, 8:26 PM, "Mahesh Jethanandani" <mjethanandani@gmail.com<mailto:mjethanandani@gmail.com>> wrote:

Kent,

Thanks for a detailed review. See inline.


On Feb 13, 2018, at 2:30 PM, Kent Watsen <kwatsen@juniper.net<mailto:kwatsen@juniper.net>> wrote:

[sorry, wrong WG, moving netconf to BCC!]


ACL Authors,

Below are some issues I found while looking at doing the Shepherd
write-up today.  Please take a look.

Also, with regards to the request for those having Last Call comments
to please verify that their comments were addressed, I only saw one
response from Kristian, but should we be expecting respeonses from
others too, perhaps Einar or Elliot?

Eliot can confirm if he feels his issues have been addressed.




1 IDNITS

 - some issues found by idnits
 - using https://urldefense.proofpoint.com/v2/url?u=https-3A__www.ietf.org_tools_idnits_&d=DwICAg&c=HAkYuh63rsuhr6Scbfh0UjBXeMK-ndb3voDTXcWzoCI&r=9zkP0xnJUvZGJ9EPoOH7Yhqn2gsBYaGTvjISlaJdcZo&m=7Bx3hgofSFxvNRV7Xz7FuaJcKKfzEB0sBJzN_KOCtSg&s=_5f-lxCoJW2TidcrjW_KbDkdPhfxLoL67kn1A2okgs0&e=
 - without selecting "verbose output"


1.1

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

Fixed.




 This "**" is being flagged as an "error".
 Idnits label, not mine.  Please fix.


1.2

 == There are 7 instances of lines with non-RFC6890-compliant IPv4 addresses
    in the document.  If these are example addresses, they should be changed.

 This is just a warning, but given that there are seven occurrences, it
 might be a good idea to fix.  Please see Section 3, point #6 in this
 document for details: https://urldefense.proofpoint.com/v2/url?u=https-3A__www.ietf.org_id-2Dinfo_checklist&d=DwICAg&c=HAkYuh63rsuhr6Scbfh0UjBXeMK-ndb3voDTXcWzoCI&r=9zkP0xnJUvZGJ9EPoOH7Yhqn2gsBYaGTvjISlaJdcZo&m=7Bx3hgofSFxvNRV7Xz7FuaJcKKfzEB0sBJzN_KOCtSg&s=AYo8ZHPY4SAHMqcy1qV9yr7BjoxGy_C9zcJ_ZbwXBT4&e=xGy_C9zcJ_ZbwXBT4&e=.

Fixed.




1.3

 ** The document seems to lack a both a reference to RFC 2119 and the
    recommended RFC 2119 boilerplate, even if it appears to use RFC 2119
    keywords.

    RFC 2119 keyword, line 797: '...s-list. A device MAY restrict the leng...'


 There needs to be a section that looks like RFC 8174, paragraph 11:

    The key words "MUST", "MUST NOT", "REQUIRED", "SHALL", "SHALL NOT",
    "SHOULD", "SHOULD NOT", "RECOMMENDED", "NOT RECOMMENDED", "MAY",
    and "OPTIONAL" in this document are to be interpreted as
    described in BCP 14 [RFC2119] [RFC8174] when, and only when, they
    appear in all capitals, as shown here.

Added.




1.4.

 -- The document date (February 2, 2018) is 11 days in the past.  Is this
    intentional?

 This is fine, ignore it.

1.5

 ** Obsolete normative reference: RFC 2460

 This needs to be fixed.

Updated the reference to RFC 8200.



1.6

 ** Downref: Normative reference to an Historic RFC: RFC 3540

 Hmmmm, another HISTORIC document, but this time not due to an IESG
 action.  The question is how important this reference is, is this
 "ns" bit (ECN-nonce concealment protection) commonly used in the
 industry?

I do not know enough to know it is not used. If the consensus is that we do not use it, I can drop it from the model.

<KENT> As shepherd, I would like the normative reference to a historic RFC removed from this draft.   My recommendation is to remove it.  As chair, if anyone wants to make a case for keeping the "ns" bit, *now* is
your time to say something.


1.7

 == Outdated reference: A later version (-06) exists of
    draft-ietf-netmod-yang-tree-diagrams-04

 Please update to -06

This might be because the draft was last published when -04 was around. I do not reference any particular version. My reference is to
<?rfc include='reference.I-D.ietf-netmod-yang-tree-diagrams’?>. The tool pulls in the latest when it generates the draft.


1.8

 -- Obsolete informational reference (is this intentional?): RFC 5101
    (Obsoleted by RFC 7011)

 Please update to RFC 7011

Done.





2  YANG VALIDATION

2.1 Normative Modules

 All of the following passed:

   pyang --ietf ietf-access-control-list\@2018-02-02.yang
   pyang --ietf ietf-packet-fields\@2018-02-02.yang
   pyang --ietf ietf-ethertypes\@2018-02-02.yang

   yanglint -s ietf-access-control-list\@2018-02-02.yang
   yanglint -s ietf-packet-fields\@2018-02-02.yang
   yanglint -s ietf-ethertypes\@2018-02-02.yang

2.2 Example Module

 Example module passed `yanglint -s`, but not `pyang --lint`:

   yanglint -s example-newco-acl.yang
   pyang --lint example-newco-acl.yang

   example-newco-acl.yang:78: error: keyword "description" not in
   canonical order, expected "type", (See RFC 6020, Section 12)

   example-newco-acl.yang:79: error: keyword "type" not in
   canonical order, (See RFC 6020, Section 12)

   example-newco-acl.yang:82: error: keyword "default" not in
   canonical order, (See RFC 6020, Section 12)

 Please fix.

Fixed.




2.3 XML Examples from Section 4.3

 yanglint didn't find any issues:

   yanglint ietf-access-control-list\@2018-02-02.yang ex-4.3.xml


2.4 Examples from Section 4.4

 I had to stitch these into the 4.3 example.  It found one issue, a typo
 in the last closing tag in the first example in this section:

   yanglint ietf-access-control-list\@2018-02-02.yang ex-4.4++.xml
   err : Invalid (mixed names) opening (source-port-range-or-operator) and closing (tcp) element tags. (/data/access-lists/acl/aces/ace/matches/l4/tcp/source-port-range-or-operator/source-port-range-or-operator)

 Please fix.

Made them complete examples so you do not have to stitch them anymore. And made sure yanglint validated the examples before it includes it in the draft.




 PS: And this is not a shepherd directive, but I found the whole
     "source-port-range-or-operator" syntax clumsy.  I'm surprised
     it didn't look something like:

         OLD
               <source-port-range-or-operator>
                  <port-range-or-operator>
                    <range>
                      <lower-port>16384</lower-port>
                      <upper-port>65535</upper-port>
                    </range>
                  </port-range-or-operator>
               </source-port-range-or-operator>

               <source-port-range-or-operator>
                 <port-range-or-operator>
                   <operator>
                     <operator>eq</operator>
                     <port>21</port>
                   </operator>
                 </port-range-or-operator>
               </source-port-range-or-operator>

         NEW

               <source-port>
                 <range>
                   <lower>16384</lower>
                   <upper>65535</upper>
                 </range>
               </source-port>

               <source-port>
                 <operator>
                   <operator>eq</operator>
                   <port>21</port>
                 </operator>
               </source-port>

Did you try making the change in the model to see if it work? It will complain that <range> is already used within the container and that it cannot be repeated (for destination-port).

<KENT> No, I did not, nor do I intend to get that deep into it.  But I recall that Kristian made the same comment before, and was making pull requests before, so maybe he can suggest something?


3 Key Draft Sections


3.1 Abstract

 First, I'm unsure if that first "sentence" is properly worded, but I
 definitely think that it is a bit too much on the terse side.  Can you
 embellish it a little?

How about this:

OLD:

  This document describes a data model of Access Control List (ACL)

  basic building blocks.
NEW:

  This document describes 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.

<KENT> good.


 Second, am I reading it correct? - is the "Editorial Note" in the
 Abstract section.  I strongly advise moving

Moved it to Introduction section.

<KENT> was it before in a <note> element?   It may have just been a rendering issue that made it look like it was par of the abstract.  If it was a <note> before, then that is actually a common use of the <note> element.  It doesn't really matter, the RFC Editor will remove the section anyway.


3.2 RFC Editor Note

 There is no request to replace "I-D.ietf-netmod-yang-tree-diagrams" with
 the final RFC assignment.

Added.



 You might want to add what the current date value used in the draft is
 (i.e., 2018-02-02).   PS: my draft build tools, which I think you're
 using, should set the value for you automatically if you put YYYY-MM-DD
 into the text.

Added text to replace the revision date in the model with the date the draft gets published.



3.3 Import statements missing references

 All import statements in all modules are missing reference statements
    - why wasn't this caught by the tools?!

 Please see rfc6087bis Section 4.7.

Adding reference implies import by revision, which we want to avoid, specially since we do not want to import by revision. Right?

<KENT> I wrote "reference" (not "revision").  A reference statement just specifies which RFC the imported module is defined in.


3.4 Security Considerations

 Please reformat the last paragraph so the "aces" path is more pronounced.
 Perhaps use hangText.

What is hangText? I italicized it.

<KENT> https://xml2rfc.tools.ietf.org/xml2rfcFAQ.html#q_hanging_lists



3.5 IANA Considerations

 This section is hard to read.

 Consideration breaking up the "XML" and the "YANG Module Names" registry
 requests into two subsections.

 Consider making the registration entry requests themselves artwork so
 they're line-spaced and indented as such.

 The first paragraph of the "XML" registry request says "a URI", but it
 should be "two URIs"

 The first paragraph of the "YANG Module Names" registry request says
 "a YANG module", but it should be "two YANG modules”

Split into two sections and upped the count of URIs and YANG models to three (was missing the ietf-ethertypes module).




3.6 References

 I haven't checked yet, but please verify that all the references are
 properly sorted as to being Normative or Informative.


3.7 Appendix A

 It took me awhile to figure out what I was looking at.  The tree-diagram
 is poorly indented and there is no text preceding the example module.

I have moved the example module after the first paragraph, that describes the module. Let me know if that looks ok.



 I recommend you fold the lines of your tree diagram at a certain column
 whilst adding a '\' character.  I've since added this ability to my draft
 build tools, let me know if interested in an update.  You might also want
 to look at draft-wu-netmod-yang-xml-doc-conventions.

Shortened the prefix so the augment statement fits within 72 columns.

BTW, I use 'pyang  -f tree —tree-line-length=69' to generate the tree. Plus I use fold -w 71 to fold the diagram, but I guess it does not work for augment statement.



 Also, please fix the example module's namespace per the end of
 rfc6087bis Section 4.9.

Updated the namespace to “http://example.com/ns/example-newco-acl<https://urldefense.proofpoint.com/v2/url?u=http-3A__example.com_ns_example-2Dnewco-2Dacl&d=DwMFaQ&c=HAkYuh63rsuhr6Scbfh0UjBXeMK-ndb3voDTXcWzoCI&r=9zkP0xnJUvZGJ9EPoOH7Yhqn2gsBYaGTvjISlaJdcZo&m=yWaNP3COBT8TgmVD5YCM5MT3RKbupJRm6BWCzPyiC4k&s=LcLSD4FXu5W7vqtE66AfFokTq8N66aosAOuBuO3G73I&e=>”

Cheers.





Thanks,
Kent




_______________________________________________
Netconf mailing list
Netconf@ietf.org<mailto:Netconf@ietf.org>
https://urldefense.proofpoint.com/v2/url?u=https-3A__www.ietf.org_mailman_listinfo_netconf&d=DwICAg&c=HAkYuh63rsuhr6Scbfh0UjBXeMK-ndb3voDTXcWzoCI&r=9zkP0xnJUvZGJ9EPoOH7Yhqn2gsBYaGTvjISlaJdcZo&m=7Bx3hgofSFxvNRV7Xz7FuaJcKKfzEB0sBJzN_KOCtSg&s=XknLqgAu3Z9t1ME6FO-_mZY2oCN61867VB0ubLeiv3Q&e=


_______________________________________________
netmod mailing list
netmod@ietf.org
https://www.ietf.org/mailman/listinfo/netmod

Mahesh Jethanandani
mjethanandani@gmail.com<mailto:mjethanandani@gmail.com>