Re: [netmod] WG Last Call: draft-ietf-netmod-acl-model-14

[Eliot Lear <[e-mail]>]

Presuming this issue is behind us, are all WGLC comments addressed?  Can
we expect to see a new draft soon?

Eliot


On 18.12.17 13:31, Eliot Lear 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 <[e-mail]
>>> <mailto:[e-mail]>> 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)
>>>>> <[e-mail] <mailto:[e-mail]>> wrote:
>>>>>
>>>>>
>>>>>
>>>>>> On 14 Dec 2017, at 08:21, Sonal Agarwal <[e-mail]
>>>>>> <mailto:[e-mail]>> 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
>>>>>> <[e-mail] <mailto:[e-mail]>> 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)
>>>>>>>     <[e-mail] <mailto:[e-mail]>> 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 <[e-mail]
>>>>>>>>     <mailto:[e-mail]>> 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
>>>>>>>>     [e-mail] <mailto:[e-mail]>
>>>>>>>>     https://www.ietf.org/mailman/listinfo/netmod
>>>>>>>>     <https://www.ietf.org/mailman/listinfo/netmod>
>>>>>>>
>>>>>>
>>>>>>     Mahesh Jethanandani
>>>>>>     [e-mail] <mailto:[e-mail]>
>>>>>>
>>>>>>
>>>>>>     _______________________________________________
>>>>>>     netmod mailing list
>>>>>>     [e-mail] <mailto:[e-mail]>
>>>>>>     https://www.ietf.org/mailman/listinfo/netmod
>>>>>>     <https://www.ietf.org/mailman/listinfo/netmod>
>>>>>>
>>>>>>
>>>>>
>>>>> _______________________________________________
>>>>> netmod mailing list
>>>>> [e-mail] <mailto:[e-mail]>
>>>>> https://www.ietf.org/mailman/listinfo/netmod
>>>>
>>>>
>>>>
>>>> _______________________________________________
>>>> netmod mailing list
>>>> [e-mail]
>>>> https://www.ietf.org/mailman/listinfo/netmod
>>>
>>
>
>
>
> _______________________________________________
> netmod mailing list
> [e-mail]
> https://www.ietf.org/mailman/listinfo/netmod