Hi Mahesh, Thanks for this work. I think this is okay. In the case of MUD we simply won't have the other container. Can I please ask that you get the draft out quickly as draft-ietf-opsawg-mud has been waiting quite some time for this work to complete.
Eliot On 10.01.18 04:08, Mahesh Jethanandani wrote: > I have pulled in the changes as they relate to: > > - moving “interface-acl” under the container “attachment-points” > making it local to that container. > - reverting “acl-type” to “type” > - removed “interface-all-aggregate” feature > - simplified source port and destination port definition > > The pull request for the changes can be found here. > > https://github.com/netmod-wg/acl-model/pull/20 > > After discussing with some of the original contributors, decided not > to include the change as it relates to augmenting ietf-interfaces. We > did not find that the change had a particular advantage over the > current implementation. Even if we do not completely understand how > ACLs might be attached “globally” or on something that is not an > interface, having the flexibility to attach them to other attachment > points is important. Keeping it as interface-ref gives us that > flexibility. > > Cheers. > >> On Dec 18, 2017, at 4:31 AM, Eliot Lear <[email protected] >> <mailto:[email protected]>> wrote: >> >> So long as nobody expects an interface construct in a MUD file, I'm >> happy. >> >> >> On 17.12.17 15:34, Einar Nilsen-Nygaard (einarnn) wrote: >>> Eliot, >>> >>> Nothing can force an implementation to have to implement either >>> the ietf-interfaces model or the augmentation in the >>> ietf-access-control-list model. I appreciate your desire for >>> modularity and cohesiveness, but I would resist #1, because I feel >>> that the majority of users will be targeting interface-based >>> attachment over time. I’ve adde back in use of the >>> “interface-attachment” feature (which I took out as part of >>> refactoring interface attachment). Part of: >>> >>> https://github.com/netmod-wg/acl-model/pull/21 >>> >>> >>> The augments part of the tree now looks like: >>> >>> augment /if:interfaces/if:interface: >>> +--rw acls *{interface-attachment}*? >>> +--rw ingress >>> | +--rw acl-sets >>> | +--rw acl-set* [name] >>> | +--rw name -> /access-lists/acl/name >>> | +--rw type? -> /access-lists/acl/type >>> | +--ro ace-statistics* [name] {interface-stats}? >>> | +--ro name -> >>> /access-lists/acl/aces/ace/name >>> | +--ro matched-packets? yang:counter64 >>> | +--ro matched-octets? yang:counter64 >>> +--rw egress >>> +--rw acl-sets >>> +--rw acl-set* [name] >>> +--rw name -> /access-lists/acl/name >>> +--rw type? -> /access-lists/acl/type >>> +--ro ace-statistics* [name] {interface-stats}? >>> +--ro name -> >>> /access-lists/acl/aces/ace/name >>> +--ro matched-packets? yang:counter64 >>> +--ro matched-octets? yang:counter64 >>> >>> Cheers, >>> >>> Einar >>> >>> >>>> On 17 Dec 2017, at 11:29, Eliot Lear <[email protected] >>>> <mailto:[email protected]>> wrote: >>>> >>>> Einar, >>>> >>>> I think this change is fine, with one exception. I would rather >>>> the augment to the interface not be required for implementations >>>> that don't actually have interfaces. I understand that there may >>>> be two ways to go about this: >>>> >>>> 1. Separate out the augment into a separate module (same doc is >>>> fine); or >>>> 2. Somehow "feature-ize" the augment. >>>> >>>> I don't know how to do (2) but if you do, that's okay by me. >>>> >>>> Eliot >>>> >>>> >>>> On 16.12.17 14:19, Einar Nilsen-Nygaard (einarnn) wrote: >>>>> All, >>>>> >>>>> After a series of discussions on- and off-list, I have a candidate >>>>> PR that includes the changes in the PR Mahesh sent out plus some >>>>> more edits. Please see consolidated PR here: >>>>> >>>>> https://github.com/netmod-wg/acl-model/pull/21 >>>>> >>>>> >>>>> Main changes in addition to Mahesh’s PR are: >>>>> >>>>> * Moved interface attachment to be via an interface augmentation. >>>>> * Restructured port matches slightly under both IPv4 and IPv6 >>>>> containers. >>>>> * Removed unnecessary identity 'interface-acl-aggregate’. >>>>> * Removed action ‘icmp-off’, can be augmented later. >>>>> >>>>> >>>>> For reference, here is the current YANG tree plus “--ietf” logs: >>>>> >>>>> 13:12 $ pyang --ietf --lint -f tree ietf-access-control-list.yang >>>>> ietf-access-control-list.yang:51: error: bad value "YYYY-MM-DD" >>>>> (should be date) >>>>> module: ietf-access-control-list >>>>> +--rw access-lists >>>>> +--rw acl* [name] >>>>> +--rw name string >>>>> +--rw type? acl-type >>>>> +--rw aces >>>>> +--rw ace* [name] >>>>> +--rw name string >>>>> +--rw matches >>>>> | +--rw (l2)? >>>>> | | +--:(eth) >>>>> | | +--rw eth {match-on-eth}? >>>>> | | +--rw destination-mac-address? >>>>> yang:mac-address >>>>> | | +--rw destination-mac-address-mask? >>>>> yang:mac-address >>>>> | | +--rw source-mac-address? >>>>> yang:mac-address >>>>> | | +--rw source-mac-address-mask? >>>>> yang:mac-address >>>>> | | +--rw ethertype? >>>>> eth:ethertype >>>>> | +--rw (l3)? >>>>> | | +--:(ipv4) >>>>> | | | +--rw ipv4 {match-on-ipv4}? >>>>> | | | +--rw dscp? >>>>> inet:dscp >>>>> | | | +--rw ecn? uint8 >>>>> | | | +--rw length? uint16 >>>>> | | | +--rw ttl? uint8 >>>>> | | | +--rw protocol? uint8 >>>>> | | | +--rw (source-port-range-or-operator)? >>>>> | | | | +--:(range) >>>>> | | | | | +--rw source-port-lower >>>>> inet:port-number >>>>> | | | | | +--rw source-port-upper >>>>> inet:port-number >>>>> | | | | +--:(operator) >>>>> | | | | +--rw source-operator >>>>> operator >>>>> | | | | +--rw source-port >>>>> inet:port-number >>>>> | | | +--rw >>>>> (destination-port-range-or-operator)? >>>>> | | | | +--:(range) >>>>> | | | | | +--rw destination-port-lower >>>>> inet:port-number >>>>> | | | | | +--rw destination-port-upper >>>>> inet:port-number >>>>> | | | | +--:(operator) >>>>> | | | | +--rw destination-operator >>>>> operator >>>>> | | | | +--rw destination-port >>>>> inet:port-number >>>>> | | | +--rw ihl? uint8 >>>>> | | | +--rw flags? bits >>>>> | | | +--rw offset? uint16 >>>>> | | | +--rw identification? uint16 >>>>> | | | +--rw destination-ipv4-network? >>>>> inet:ipv4-prefix >>>>> | | | +--rw source-ipv4-network? >>>>> inet:ipv4-prefix >>>>> | | +--:(ipv6) >>>>> | | +--rw ipv6 {match-on-ipv6}? >>>>> | | +--rw dscp? >>>>> inet:dscp >>>>> | | +--rw ecn? uint8 >>>>> | | +--rw length? uint16 >>>>> | | +--rw ttl? uint8 >>>>> | | +--rw protocol? uint8 >>>>> | | +--rw (source-port-range-or-operator)? >>>>> | | | +--:(range) >>>>> | | | | +--rw source-port-lower >>>>> inet:port-number >>>>> | | | | +--rw source-port-upper >>>>> inet:port-number >>>>> | | | +--:(operator) >>>>> | | | +--rw source-operator >>>>> operator >>>>> | | | +--rw source-port >>>>> inet:port-number >>>>> | | +--rw >>>>> (destination-port-range-or-operator)? >>>>> | | | +--:(range) >>>>> | | | | +--rw destination-port-lower >>>>> inet:port-number >>>>> | | | | +--rw destination-port-upper >>>>> inet:port-number >>>>> | | | +--:(operator) >>>>> | | | +--rw destination-operator >>>>> operator >>>>> | | | +--rw destination-port >>>>> inet:port-number >>>>> | | +--rw destination-ipv6-network? >>>>> inet:ipv6-prefix >>>>> | | +--rw source-ipv6-network? >>>>> inet:ipv6-prefix >>>>> | | +--rw flow-label? >>>>> inet:ipv6-flow-label >>>>> | +--rw (l4)? >>>>> | | +--:(tcp) >>>>> | | | +--rw tcp {match-on-tcp}? >>>>> | | | +--rw sequence-number? uint32 >>>>> | | | +--rw acknowledgement-number? uint32 >>>>> | | | +--rw data-offset? uint8 >>>>> | | | +--rw reserved? uint8 >>>>> | | | +--rw flags? bits >>>>> | | | +--rw window-size? uint16 >>>>> | | | +--rw urgent-pointer? uint16 >>>>> | | | +--rw options? uint32 >>>>> | | +--:(udp) >>>>> | | | +--rw udp {match-on-udp}? >>>>> | | | +--rw length? uint16 >>>>> | | +--:(icmp) >>>>> | | +--rw icmp {match-on-icmp}? >>>>> | | +--rw type? uint8 >>>>> | | +--rw code? uint8 >>>>> | | +--rw rest-of-header? uint32 >>>>> | +--rw egress-interface? if:interface-ref >>>>> | +--rw ingress-interface? if:interface-ref >>>>> +--rw actions >>>>> | +--rw forwarding identityref >>>>> | +--rw logging? identityref >>>>> +--ro statistics {acl-aggregate-stats}? >>>>> +--ro matched-packets? yang:counter64 >>>>> +--ro matched-octets? yang:counter64 >>>>> >>>>> augment /if:interfaces/if:interface: >>>>> +--rw acls >>>>> +--rw ingress >>>>> | +--rw acl-sets >>>>> | +--rw acl-set* [name] >>>>> | +--rw name -> /access-lists/acl/name >>>>> | +--rw type? -> /access-lists/acl/type >>>>> | +--ro ace-statistics* [name] {interface-stats}? >>>>> | +--ro name -> >>>>> /access-lists/acl/aces/ace/name >>>>> | +--ro matched-packets? yang:counter64 >>>>> | +--ro matched-octets? yang:counter64 >>>>> +--rw egress >>>>> +--rw acl-sets >>>>> +--rw acl-set* [name] >>>>> +--rw name -> /access-lists/acl/name >>>>> +--rw type? -> /access-lists/acl/type >>>>> +--ro ace-statistics* [name] {interface-stats}? >>>>> +--ro name -> >>>>> /access-lists/acl/aces/ace/name >>>>> +--ro matched-packets? yang:counter64 >>>>> +--ro matched-octets? yang:counter64 >>>>> >>>>> Comments welcome! >>>>> >>>>> Cheers, >>>>> >>>>> Einar >>>>> >>>>> >>>>> >>>>>> On 14 Dec 2017, at 18:50, Einar Nilsen-Nygaard (einarnn) >>>>>> <[email protected] <mailto:[email protected]>> wrote: >>>>>> >>>>>> >>>>>> >>>>>>> On 14 Dec 2017, at 08:21, Sonal Agarwal <[email protected] >>>>>>> <mailto:[email protected]>> wrote: >>>>>>> >>>>>>> Hi Einar, >>>>>>> >>>>>>> You had 3 questions for me on all the several e-mail threads. >>>>>>> 1. Global attachment point >>>>>>> 2. icmp-off >>>>>>> 3. acl-aggregate-interface stats. >>>>>>> >>>>>>> For (1), my first preference is to have the model define >>>>>>> attachment point for interfaces only. >>>>>> >>>>>> einarnn> I have some diffs, layered on top of Mahesh’s PR to >>>>>> netmod-wg/acl-model that do this. Nearly like the augmentation I >>>>>> have below. Feel free to take a look at: >>>>>> >>>>>> https://github.com/mjethanandani/acl-model/pull/3 >>>>>> >>>>>> >>>>>>> However, Kristian wants the global attachment point as well so >>>>>>> that he can add the ACL to the linux tables. >>>>>> >>>>>> einarnn> I think Kristian doesn’t feel a global attachment point >>>>>> needs to be in this first revision. But he can confirm. >>>>>> >>>>>>> If an ACL is attached globally, does this mean it is per >>>>>>> direction or does it mean it is across directions? >>>>>> >>>>>> einarnn> I don’t know right now :-) >>>>>> >>>>>>> This global ACL may not be applicable to any of Cisco's service >>>>>>> provider routers as I don't see any platform actually >>>>>>> replicating the ACL to all line cards and attaching it in >>>>>>> ingress and egress directions across all interfaces. >>>>>> >>>>>> einarnn> Per other emails, I don’t think we understand this >>>>>> enough yet to specify it, so I suggest we just leave it out for >>>>>> now. Nothing in the model prevents a “global attachment point” >>>>>> being added later once we understand what it really means. >>>>>> >>>>>>> For (2), I am ok with removing icmp-off. >>>>>> >>>>>> einarnn> Done in my PR above. >>>>>> >>>>>>> For (3), this would have to be a combination of ACL stats across >>>>>>> all interfaces for all ACL's. Something like this is possible on >>>>>>> an XR box where ACES have counter names associated with it. >>>>>>> Let's chat about this offline tomorrow. >>>>>> >>>>>> einarnn> I’ll ping you to clarify, and we can bring any >>>>>> conclusion back to the list. >>>>>> >>>>>> Cheers, >>>>>> >>>>>> Einar >>>>>> >>>>>> >>>>>> >>>>>>> Sonal. >>>>>>> >>>>>>> >>>>>>> On Wed, Dec 13, 2017 at 12:10 PM, Mahesh Jethanandani >>>>>>> <[email protected] <mailto:[email protected]>> wrote: >>>>>>> >>>>>>> We want to support “global” attachment point down the line, >>>>>>> and that “global” attachment point will be one of the >>>>>>> choices (the other being the interface), what would this >>>>>>> augment look like. Note, as far as I know, you cannot >>>>>>> augment inside a choice node. >>>>>>> >>>>>>>> On Dec 13, 2017, at 6:57 AM, Einar Nilsen-Nygaard (einarnn) >>>>>>>> <[email protected] <mailto:[email protected]>> wrote: >>>>>>>> >>>>>>>> Perhaps like this, as an augmentation to the interface: >>>>>>>> >>>>>>>> augment /if:interfaces/if:interface: >>>>>>>> +--rw ingress-acls >>>>>>>> | +--rw acl-sets >>>>>>>> | +--rw acl-set* [name] >>>>>>>> | +--rw name -> >>>>>>>> /access-lists/acl/name >>>>>>>> | +--rw type? -> >>>>>>>> /access-lists/acl/type >>>>>>>> | +--ro ace-statistics* [name] >>>>>>>> {interface-stats}? >>>>>>>> | +--ro name -> >>>>>>>> /access-lists/acl/aces/ace/name >>>>>>>> | +--ro matched-packets? yang:counter64 >>>>>>>> | +--ro matched-octets? yang:counter64 >>>>>>>> +--rw egress-acls >>>>>>>> +--rw acl-sets >>>>>>>> +--rw acl-set* [name] >>>>>>>> +--rw name -> >>>>>>>> /access-lists/acl/name >>>>>>>> +--rw type? -> >>>>>>>> /access-lists/acl/type >>>>>>>> +--ro ace-statistics* [name] >>>>>>>> {interface-stats}? >>>>>>>> +--ro name -> >>>>>>>> /access-lists/acl/aces/ace/name >>>>>>>> +--ro matched-packets? yang:counter64 >>>>>>>> +--ro matched-octets? yang:counter64 >>>>>>>> >>>>>>>> >>>>>>>> Could also put an “aces” container above both these & >>>>>>>> rename “ingress-acls" to “ingress”, etc. to give a single >>>>>>>> root for the augmentation if preferred. >>>>>>>> >>>>>>>> Cheers, >>>>>>>> >>>>>>>> Einar >>>>>>>> >>>>>>>> >>>>>>>>> On 6 Dec 2017, at 19:43, Eliot Lear <[email protected] >>>>>>>>> <mailto:[email protected]>> wrote: >>>>>>>>> >>>>>>>>> >>>>>>>>> >>>>>>>>> On 12/6/17 7:23 PM, Mahesh Jethanandani wrote: >>>>>>>>>> How does one move the interface attachment point, >>>>>>>>>> currently an >>>>>>>>>> 'interface-ref', to an augmentation of the >>>>>>>>>> if:interfaces/interface, >>>>>>>>>> inside of the ‘acl’ container? Down the line we might >>>>>>>>>> need to have an >>>>>>>>>> container for "attachment points" to accommodate the >>>>>>>>>> possibility of >>>>>>>>>> attaching an ACL either to an interface or “globally”. >>>>>>>>>> >>>>>>>>> >>>>>>>>> Keeping in mind that one use is that an ACL doesn't attach >>>>>>>>> to an >>>>>>>>> interface at all. >>>>>>>>> >>>>>>>>> _______________________________________________ >>>>>>>>> netmod mailing list >>>>>>>>> [email protected] <mailto:[email protected]> >>>>>>>>> https://www.ietf.org/mailman/listinfo/netmod >>>>>>>>> <https://www.ietf.org/mailman/listinfo/netmod> >>>>>>>> >>>>>>> >>>>>>> Mahesh Jethanandani >>>>>>> [email protected] <mailto:[email protected]> >>>>>>> >>>>>>> >>>>>>> _______________________________________________ >>>>>>> netmod mailing list >>>>>>> [email protected] <mailto:[email protected]> >>>>>>> https://www.ietf.org/mailman/listinfo/netmod >>>>>>> <https://www.ietf.org/mailman/listinfo/netmod> >>>>>>> >>>>>>> >>>>>> >>>>>> _______________________________________________ >>>>>> netmod mailing list >>>>>> [email protected] <mailto:[email protected]> >>>>>> https://www.ietf.org/mailman/listinfo/netmod >>>>> >>>>> >>>>> >>>>> _______________________________________________ >>>>> netmod mailing list >>>>> [email protected] >>>>> https://www.ietf.org/mailman/listinfo/netmod >>>> >>> >> >> _______________________________________________ >> netmod mailing list >> [email protected] <mailto:[email protected]> >> https://www.ietf.org/mailman/listinfo/netmod > > Mahesh Jethanandani > [email protected] <mailto:[email protected]> > > > > _______________________________________________ > netmod mailing list > [email protected] > https://www.ietf.org/mailman/listinfo/netmod
signature.asc
Description: OpenPGP digital signature
_______________________________________________ netmod mailing list [email protected] https://www.ietf.org/mailman/listinfo/netmod
