Re: [netmod] WG Last Call: draft-ietf-netmod-acl-extensions-03

[Sonal Agarwal <[e-mail]>]

Hi authors,

In the Introduction you say:

"When managing ACLs, it is common for network operators to group match
elements in pre-defined sets. The consolidation into group matches allows
for reducing the number of rules, especially in large scale networks. If,
for example, it is needed to find a match against 100 IP addresses (or
prefixes), a single rule will suffice rather than creating individual
Access Control Entries (ACEs) for each IP address (or prefix). In doing so,
implementations would optimize the performance of matching lists vs
multiple rules matching.¶
<https://www.ietf.org/archive/id/draft-ietf-netmod-acl-extensions-06.html#section-1-2>

The enhanced ACL structure (Section 4
<https://www.ietf.org/archive/id/draft-ietf-netmod-acl-extensions-06.html#sec-module>)
is also meant to facilitate the management of network operators. Instead of
entering the IP address or port number literals, using user-named lists
decouples the creation of the rule from the management of the sets. Hence,
it is possible to remove/add entries to the list without redefining the
(parent) ACL rule."

It should be noted that even though changing the IP address/port etc is
trivial from a manageability POV, it is complicated in data plane. Most
data plane architectures (that I have come across) deploy "make before
break" i.e. the entire new ACL will be programmed in HW prior to destroying
the old version. This helps ensure minimal traffic loss. There is no
optimization being done for the group vs the non group rules.

Regards,
Sonal.



On Mon, Mar 25, 2024 at 6:26 PM Mahesh Jethanandani <[e-mail]>
wrote:

> Hi Joe,
>
> Your name popped up on this thread. See below.
>
> Mahesh Jethanandani
> [e-mail]
>
> On Feb 8, 2024, at 4:01 AM, [e-mail] wrote:
>
> 
>
> Hi Mahesh, all,
>
>
>
> FWIW, we submitted an updated version of the draft to address the pending
> points from your reviews. A diff to track the changes vs. -04 can be seen
> at:
> https://author-tools.ietf.org/iddiff?url1=draft-ietf-netmod-acl-extensions-04&url2=draft-ietf-netmod-acl-extensions-06&difftype=--html
> .
>
>
>
> Cheers,
>
> Med
>
>
>
> *De :* BOUCADAIR Mohamed INNOV/NET
> *Envoyé :* mardi 23 janvier 2024 16:57
> *À :* 'Mahesh Jethanandani' <[e-mail]>
> *Cc :* Lou Berger <[e-mail]>; NETMOD Group <[e-mail]>;
> NetMod WG Chairs <[e-mail]>
> *Objet :* RE: [netmod] WG Last Call: draft-ietf-netmod-acl-extensions-03
>
>
>
> Hi Mahesh,
>
>
>
> Thanks for the follow-up. Made some changes as you can see at
> https://boucadair.github.io/enhanced-acl-netmod/#go.draft-ietf-netmod-acl-extensions.diff
> .
>
>
>
> Please see inline for more context.
>
>
>
> Cheers,
>
> Med
>
>
>
>
>
> Orange Restricted
>
> *De :* Mahesh Jethanandani <[e-mail]>
> *Envoyé :* mercredi 20 décembre 2023 18:20
> *À :* BOUCADAIR Mohamed INNOV/NET <[e-mail]>
> *Cc :* Lou Berger <[e-mail]>; NETMOD Group <[e-mail]>;
> NetMod WG Chairs <[e-mail]>
> *Objet :* Re: [netmod] WG Last Call: draft-ietf-netmod-acl-extensions-03
>
>
>
> Hi Med,
>
>
>
> Thanks for addressing some of my comments. Please see inline.
>
>
>
> On Dec 19, 2023, at 12:09 AM, [e-mail] wrote:
>
>
>
> Hi Mahesh, all,
>
>
>
> Thank you for the review and comments. We just posed
> draft-ietf-netmod-acl-extensions-04.
>
>
>
> Please see more context inline.
>
>
>
> Cheers,
>
> Med
>
>
>
> *De :* netmod <[e-mail]> *De la part de* Mahesh
> Jethanandani
> *Envoyé :* mardi 5 décembre 2023 23:09
> *À :* Lou Berger <[e-mail]>
> *Cc :* NETMOD Group <[e-mail]>; NetMod WG Chairs <
> [e-mail]>
> *Objet :* Re: [netmod] WG Last Call: draft-ietf-netmod-acl-extensions-03
>
>
>
> Hi,
>
>
>
> I do support this work, as it is much needed, and would like to see it
> progress. However, I do believe that the document needs to undergo a
> revision to qualify for LC. Some of the comments are editorial or minor,
> and can be addressed easily, but others are not. They should all be
> addressed for the WG to call the document ready.
>
>
>
> - The Security Considerations section has both the read/write nodes and
> the read-only nodes as empty (or marked as TBC, which I imagine stands for
> To Be Completed). This needs to be filled out, or if no nodes are worth any
> security considerations, it should be stated so, and why.
>
>
>
> *[Med] ACK. We don’t repeat what is already in 8519 but focus on key
> additions in the
> spec: https://github.com/boucadair/enhanced-acl-netmod/pull/65/files
> <https://github.com/boucadair/enhanced-acl-netmod/pull/65/files>*
>
>
>
> [mj] Thanks for updating the section.
>
>
>
> s/setf/set/
>
> s/Simialr/Similar/
>
>
>
> and in other place
>
> s/modelled/modeled/
>
>
>
> *[Med] Thanks. Fixed.*
>
>
>
>
>
> - Isn’t the YANG model normative portion of the document? Isn’t what this
> document all about? Why is it then in the Appendix?
>
>
>
> *[Med] We are using a script to generate the IANA modules + we are
> actually following this part from the 8407bis:*
>
>
>
>    It is RECOMMENDED to include the URL from where to retrieve the
>
>    recent version of the module.  When a script is used, the Internet-
>
>    Draft that defines an IANA-maintained module SHOULD include an
>
>    appendix with the initial full version of the module.  Including such
>
>    an appendix in pre-RFC versions is meant to assess the correctness of
>
>    the outcome of the supplied script.  The authors MUST include a note
>
>    to the RFC Editor requesting that the appendix be removed before
>
>    publication as RFC and that RFC IIII is replaced with the RFC number
>
>    that is assigned to the document.  Initial versions of IANA-
>
>    maintained modules that are published in RFCs may be misused despite
>
>    the appropriate language to refer to the IANA registry to retrieve
>
>    the up-to-date module.
>
>
>
> [mj] I am not clear on what happens to the IANA module once the draft is
> published as an RFC based on what you cite from 8407bis.
>
> *[Med] It will be removed as per the note: *
>
>
>
> *(2) The modules are provided in {{iana-icmp}}, {{iana-icmpv6}}, and
> {{iana-ipv6-ext}} for the users convenience before publication as RFC.
> Please remove these appendices from the final RFC.*
>
>
>
> The document states that the reference to “RFC IIII” is replaced with the
> actual RFC number, but  it also says that the Appendix be removed. What
> happens to the initial version of the module itself? Is it removed if the
> Appendix is removed?
>
> *[Med] It will be removed as per the note above. Please note that this
> practice is already followed in rfc9108, for example.*
>
>
>
> Or does it remain in the Appendix as an initial version, with language
> that indicates that the IANA registry should be used to retrieve the most
> up-to-date model? The language in Section 1.1 item (2) does not help.
>
>
>
> The above text from 8407bis needs to be explicit on what happens to the
> initial version of the module as part of the RFC publication.
>
> *[Med] Please feel free to propose changes to this part of the bis for
> better clarity:*
>
>
>
>    The authors MUST include a note
>
>    to the RFC Editor requesting that the appendix be removed before
>
>    publication as RFC and that RFC IIII is replaced with the RFC number
>
>    that is assigned to the document.
>
>
>
>
>
> - Why is the Section titled "Initial Version of the The ICMPv4 Types
> IANA-Maintained Module”, when the model in question is "
> [e-mail]”?
>
> *[Med] This was a typo. Fixed.*
>
>
>
> [mj] You fixed it another location. However, I still see the following in
> the -04 version of the document.
>
> *[Med] Thanks for catching this. Fixed.*
>
>
>
> B.2. Initial Version of the The ICMPv4 Types IANA-Maintained Module
>
> <CODE BEGINS> file "[e-mail]"
>
>
>
>
>
> module iana-icmpv6-types {
>
>
>
>
>
> - ‘defined-sets’ and ‘aliases’ have been defined in a the separate model
> ‘ietf-acl-enh’. Are these sets and aliases defined to be used outside of
> ACL? If that is the case then having them outside the
> ‘ietf-access-control-list’ model makes sense. Otherwise, almost everything
> in the ‘ietf-acl-enh’ is an augmentation of the model defined in RFC 8519,
> as stated in the Introduction of the document
>
>
>
> *[Med] These are defined to be consumed for ACL policies.*
>
>
>
>
>
> "The YANG module in this document is solely based on augmentations to the
> ACL YANG module defined in [RFC8519].”
>
>
>
> *[Med] The intent was to highlight that we are not using a bis approach.
> Tweaked the paragraph that includes that text for better clarity.*
>
>
>
> [mj] I think it already clear that this model an augmentation and not a
> bis. A bis is when you take the original document and edit it for updates,
> and this is clearly not that.
>
>
>
> I actually agree with your above statement in the Introduction that you
> had, about the module being solely an enhancement of the ACL YANG model,
> and was surprised to see it taken out. The point I was making was that just
> like what you have done with augmenting
> "/acl:acls/acl:acl/acl:aces/acl:ace/acl:matches” to add ‘choice payload’,
> ‘choice alias’ etc, you could have augmented “/acl:acls” to add
> ‘defined-sets’ and ‘aliases’.
>
> Right now, as is, the ietf-acl-enh module sits on the root of the config
> tree, with no relation to the ACL model, other than references to it from
> within the ACL model. If the definitions in ietf-acl-enh are to be
> consumed by the ACL model only, why not augment the ACL model (as shown
> below) to add them in the ACL tree?
>
>
>
> *[Med] This is fair. Now that I managed to refresh the context in my mind
> I confirm that we have done that in a previous version of the spec, but the
> feedback we received from the WG was to move those upper in the hierarchy
> (because there might be other cases). See for example
> https://datatracker.ietf.org/doc/minutes-115-netmod-202211080930/
> <https://datatracker.ietf.org/doc/minutes-115-netmod-202211080930/>:*
>
>
>
> *==*
>
> Joe Clarke: It would be nice in a standalone container (i.e. groupings
> that could be imported). I see some other use cases for these defined
> groupings besides just ACLs.
>
> *==*
>
>
> I know that routing policy uses defined-sets, but I believe they have
> already defined their own. Which other models do you foresee using these
> groupings?
>
>
>
>
>
> If that is the case I see no reason why those containers should not be
> augmentations into the same model, as in
>
>
>
> augment “/acl:acls” {
>
>   container defined-sets {
>
>   ….
>
>   }
>
>
>
>   container aliases {
>
>      …
>
>   }
>
> }
>
>
>
>
>
> - I just pulled down the latest version (-03) of the draft, and ran into
> this error.
>
>
>
> $ pyang [e-mail]
>
> [e-mail]:1: error: unexpected latest revision
> "2023-04-28" in [e-mail], should be "2020-09-25”.
>
>
>
> *[Med] Fixed. Thanks.*
>
>
>
> - Section 3.4. TCP Flags Handling. The document states that.
>
>
>
> "Clients that support both 'flags-bitmask' and 'flags' matching
> fields MUST NOT set these fields in the same request.”.
>
>
>
> Can the model have a must statement to prevent this from being configured
> inadvertently?
>
>
>
> *[Med] We don’t see how to do that with a must statement, hence the
> normative language in the narrative text.*
>
>
>
> [mj] How about something like
>
>
>
> must
> “not(/acl:acls/acl:acl/acl:aces/acl:ace/acl:matches/acl:l4/acl:tcp/acl:flags)”
>  {
>
>   error-message
>
>     “Either flags or flags-bitmask should be configured, but not both.”;
>
> }
>
>
>
> under ‘flags-bitmask’?
>
> *[Med] Thanks. *
>
>
>
> If you are feeling adventurous, you could add a deviation add statement to
> add a similar must statement under tcp/flags also :-).
>
>
>
>
>
> Same for Section 3.5 Fragments Handling
>
> *[Med] Same answer :-)*
>
>
>
> - There should be clear direction to the RFC Editor on what should be done
> with revision dates. The same is true for other placeholder text. For
> example, what is the RFC Editor to do with text "RFC XXXX"?
>
> *[Med]
> Done: https://github.com/boucadair/enhanced-acl-netmod/pull/59/files
> <https://github.com/boucadair/enhanced-acl-netmod/pull/59/files>*
>
>
>
> [mj] Thanks.
>
>
>
>
>
> - References in the YANG model should be expanded to include the title of
> the RFC.
>
>
>
> *[Med] We are echoing references as listed in an IANA registry, so we do
> not have control over that reference.*
>
>
>
> - Examples are always good. Not only can they be used to validate the
> model, but users get to understand how it can be used. See other models
> such as BGP, TCP, BFD on how an example can be added.
>
>
>
> *[Med] We do already have many in the core document. Will consider adding
> more if needed.*
>
>
>
> [mj] I am referring to the example as stated in Section 3.12 of RFC 8407
> <https://datatracker.ietf.org/doc/html/rfc8407#section-3.12>. If by core
> you are referring to RFC 8519, then unfortunately, we the authors missed it
> too -:( But here is a module usage example from another draft.
>
> *[Med] I’m referring to examples such as those in
> https://datatracker.ietf.org/doc/html/draft-ietf-netmod-acl-extensions-04#name-tcp-flags-handling
> <https://datatracker.ietf.org/doc/html/draft-ietf-netmod-acl-extensions-04#name-tcp-flags-handling>.
> *
>
>
>
>
> https://datatracker.ietf.org/doc/html/draft-ietf-idr-bgp-model-17#name-creating-bgp-instance
>
>
>
>
>
>
>
> - How is this a reference?
>
>         reference
>
>           "- Bill Simpson <mailto:Bill.Simpson&um.cc.umich.edu>
>
>
>
> *[Med] We are echoing a reference as cited in an IANA registry, so we do
> not have control over that reference.*
>
>
>
> [mj] Regardless, and I am repeating the question, how is this a reference?
>
> *[Med] That’s a reference as per the IANA registry:*
>
>
>
> *33  IPv6 Where-Are-You (Deprecated) [Simpson][RFC6918]*
>
> *34  IPv6 I-Am-Here (Deprecated)     [Simpson][RFC6918]*
>
> *35  Mobile Registration Request (Deprecated)   [Simpson][RFC6918]*
>
> *36  Mobile Registration Reply (Deprecated)     [Simpson][RFC6918]*
>
>
>
> I think having RFC 6918 as a reference is good enough.
>
> *[Med] but this will deviate from what is in the IANA registries.*
>
>
>
> And that brings up another point. The sections that contain the YANG
> models need to list out all the references cited in the model at the
> beginning of the section. For example, Section 4 needs to list RFC 9293,
> 3032, 792, 4443 etc. at the beginning of the section, such that they are
> included in the Normative list of references. See Section 3.9 of RFC 8407
> <https://datatracker.ietf.org/doc/html/rfc8407#section-3.9>.
>
>
>
> *[Med] We are familiar with that part. The point here is that we don’t
> cite them in the main text because the IANA modules will be removed from
> the final RFC as per the comment above.*
>
>
>
> Thanks.
>
>
>
>
>
> Thanks.
>
> *[Med] Thanks for the review. Much appreciated.*
>
>
>
>
>
> Thank you.
>
>
>
>
> Mahesh Jethanandani
>
> [e-mail]
>
>
>
>
>
>
>
>
>
>
>
> ____________________________________________________________________________________________________________
> 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
> [e-mail]
> https://www.ietf.org/mailman/listinfo/netmod
>