Hi Mahesh, Thank you for the review.
Samier kindly prepared an updated version that addresses most of your comments. The diff can be seen here: https://author-tools.ietf.org/api/iddiff?url_1=https://netmod-wg.github.io/enhanced-acl-netmod/draft-ietf-netmod-acl-extensions.txt&url_2=https://netmod-wg.github.io/enhanced-acl-netmod/Mahesh-comments/draft-ietf-netmod-acl-extensions.txt Please see inline. Cheers, Med De : Mahesh Jethanandani <mjethanand...@gmail.com> Envoyé : vendredi 3 janvier 2025 06:32 À : draft-ietf-netmod-acl-extensi...@ietf.org Cc : NETMOD Working Group <netmod@ietf.org>; Per Andersson <per.i...@ionio.se> Objet : AD review of draft-ietf-netmod-acl-extensions Hi Authors of draft-ietf-netmod-acl-extensions, Happy New Year! One expert SECDIR review is still pending on the document, which I hope will be coming soon. Thanks to Per for providing the YANG Doctors review, to Tim for providing the INTDIR, and to David for the TSVART review. In the meantime, please find my AD review comments on the document. I hope these comments help to improve the document. COMMENT: "Abstract", paragraph 2 > RFC 8519 defines a YANG data model for Access Control Lists (ACLs). > This document discusses a set of extensions that fix many of the > limitations of the ACL model as initially defined in RFC 8519. > > The document also defines IANA-maintained modules for ICMP types and > IPv6 extension headers. This document provided important extensions to the base ACL YANG model defined in RFC8519. However, nowhere in the document does it mention that these extensions augment the base model, except in the model itself. Reading the abstract and the introduction, a reader would be left wondering whether this model is replacing RFC 8519 or augmenting it. Can that be made clear? [Med] A new sentence was added to the abstract. Section 2, paragraph 4 > Defined set: Refers to reusable description of one or multiple > information elements (e.g., IP address, IP prefix, port number, or > ICMP type). Is it just a description? A better way to define it would be to say: Elements in a defined set typically share a logical purpose or function, such as IP address, IP prefixes, port number, or ICMP type. Similarly definition of 'payload-based filtering' could be added to define the term or referenced if defined somewhere else. [Med] Done. Section 3.5, paragraph 1 > The augmented ACL structure (Figure 1) includes new leafs > 'ipv4-fragment' and 'ipv6-fragment' to better handle fragments. The document comments that RFC8519 does not do a good job of handling fragments and that this document addresses that issue. It would help to explain how the model defined in the document allows for better handling of fragments. For example, describe how the combination of operator and fragment type eases fragment handling. [Med] We do explain this in https://datatracker.ietf.org/doc/html/draft-ietf-netmod-acl-extensions-13#name-partial-or-lack-of-ipv4-ipv and then illustrate the use of bitmask in https://datatracker.ietf.org/doc/html/draft-ietf-netmod-acl-extensions-13#name-fragments-handling-2. Do you think we need to say more? Section 4, paragraph 1 > This model imports types from [RFC6991], [RFC8519], and [RFC8294]. Thanks first of all to Per for providing YANG doctor review comments. I also tend to agree with him that as a practice reference to RFC 9293, 4443, 3032, 792, and IEEE 802.1Q, IEEE 802.1ah need to be added here, even if they are referenced or mentioned elsewhere. [Med] I'm afraid that saying this module references REF1, REF2, etc. isn't that useful. These refs are cited in their context. "Appendix E.", paragraph 1 > This section provides a few examples to illustrate the use of the > enhanced ACL module ("ietf-acl-enh"). Thank you for providing examples along with the module. The added feature in this module is payload-based filtering. An example configuration reflecting that, say for HTTP headers, would greatly enhance the understanding of this module. [Med] Will consider adding an example to illustrate this. No reference entries found for these items, which were mentioned in the text: [RFC3692]. [Med] ACK. ------------------------------------------------------------------------------- NIT ------------------------------------------------------------------------------- All comments below are about very minor potential issues that you may choose to address in some way - or ignore - as you see fit. Some were flagged by automated tools (via https://github.com/larseggert/ietf-reviewtool), so there will likely be some false positives. There is no need to let me know what you did with these suggestions. Section 3.6, paragraph 2 > A new feature, called 'match-on-payload', is defined in the document. > This can be used, for example, for QUIC [RFC9000] or for tunneling > protocols. This feature requires configuring a data offset, a > length, and a binary pattern to macth data against using a specified > operator. s/macth/match/ [Med] ACK Section 6.3.1, paragraph 15 > IANA is requested to updated the "Reference" in the "ICMP Type > Numbers" registry as follows: s/updated/update/ [Med] ACK Section 6.3.2, paragraph 15 > IANA is requested to updated the "Reference" in the "ICMPv6 "type" > Numbers" registry as follows: s/updated/update/ [Med] ACK Section 6.3.3, paragraph 15 > IANA is requested to updated the "Reference" in the "IPv6 Extension > Header Types" registry as follows: s/updated/update/ [Med] ACK Section 1.1, paragraph 10 > ixes, port number, or ICMP type. Similarly definition of 'payload-based filt > ^^^^^^^^^ A comma may be missing after the conjunctive/linking adverb "Similarly". [Med] Don't see where this one is used in the doc. Section 4, paragraph 3 > { base tcp-flag; description "Acknowledgment TCP flag bit."; reference "RFC > ^^^^^^^^^^^^^^ Do not mix variants of the same word ("acknowledgment" and "acknowledgement") within a single text. [Med] ACK Thanks. Mahesh Jethanandani mjethanand...@gmail.com<mailto:mjethanand...@gmail.com> ____________________________________________________________________________________________________________ Ce message et ses pieces jointes peuvent contenir des informations confidentielles ou privilegiees et ne doivent donc pas etre diffuses, exploites ou copies sans autorisation. Si vous avez recu ce message par erreur, veuillez le signaler a l'expediteur et le detruire ainsi que les pieces jointes. Les messages electroniques etant susceptibles d'alteration, Orange decline toute responsabilite si ce message a ete altere, deforme ou falsifie. Merci. This message and its attachments may contain confidential or privileged information that may be protected by law; they should not be distributed, used or copied without authorisation. If you have received this email in error, please notify the sender and delete this message and its attachments. As emails may be altered, Orange is not liable for messages that have been modified, changed or falsified. Thank you.
_______________________________________________ netmod mailing list -- netmod@ietf.org To unsubscribe send an email to netmod-le...@ietf.org
