> On Dec 21, 2015, at 4:33 PM, Juergen Schoenwaelder 
> <j.schoenwael...@jacobs-university.de> wrote:
> 
> On Sat, Dec 19, 2015 at 07:50:58AM -0500, Dean Bogdanovic wrote:
>> Juergen,
>> 
>> Please see answers inline
>> 
>> Dean
>> 
>>> On Dec 11, 2015, at 12:31 PM, Juergen Schoenwaelder 
>>> <j.schoenwael...@jacobs-university.de> wrote:
>>> 
>>> On Wed, Dec 09, 2015 at 08:27:04AM -0800, Nadeau Thomas wrote:
>>>> 
>>>>    This email initiates a NETMOD WG Last call for 
>>>> draft-ietf-netmod-acl-model-06. 
>>>> Please review the draft and make any comments to the list by Wednesday, 16 
>>>> December, 2015
>>>> by 8AM EST.
>>>> 
>>> 
>>> Technical
>>> 
>>> - I am not sure I personally like the acl-oper-data and ace-oper-data
>>> containers in the middle of the config true tree. I understand that
>>> operational state in this case is likely tightly bound to the
>>> lifetime of the config state but I also generally prefer to have a
>>> clean separation of config from state. But since I am not
>>> implementing this, I am happy to leave the decision to those who
>>> write the code.
>>> 
>>> - I would probably have used src-ipv4-prefix and dst-ipv4-prefix
>>> instead of source-ipv4-network and destination-ipv4-network and
>>> I would probably have used src-mac-address and src-mac-address-mask
>>> and dst-mac-address and dst-mac-address-mask. Generally simple
>>> src instead source and dst instead destination - lines up all
>>> much more nicely.
>> 
>> I know that src and dst are pretty standard abbreviations, but have 
>> preference to use full words.
>> 
> 
> My eyes prefer the shorter versions but it might be subjective where
> the line between arconyms and words is drawn. For me, things are not
> done consistently according to my personal preferences. ;-)

As many people, so many different preferences :)

> 
>>> - I would have put the acl-name and acl-type first followed by the
>>> entries list.
>> 
>> Can you please expand on this? Is there any major difference? OR just design 
>> choice?
> 
> Technically you can define key leafs anywhere but I think there are some
> benefits of defining them at the beginning of a list:
> 
> a) I find it nice if the leafs listed in the key statement follow the
>   key statement and not N pages down the document.
> b) If you put them at the end and you have to add additional definitions,
>   you will end up with leafs somewhere in the middle.
> c) The XML encoding requires to ship key leafs first and it might be
>   simpler if the key leafs are also defined first in the data model
>   (so if you use the same order, you actually get things correct).
> 
> Now you tell me what the advantages are to put them at the end…

I think this ended up being an edit oversight. In the draft 02, the stated 
leafs were at the top

module: ietf-acl
+--rw access-lists
+--rw access-list* [access-control-list-name]
+--rw access-control-list-name         string
+--rw access-control-list-type?        access-control-list-type
+--ro access-control-list-oper-data
|  +--ro (targets)?
|     +--:(interface-name)
|        +--ro interface-name*   string
+--rw access-list-entries
+--rw access-list-entry* [rule-name]

I don’t remember if there was any specific reason to push the acl-name and 
acl-type to the end. Will bring it back up.
> 
>>> - I also wonder why we use both the term 'entry' and 'rule', why not
>>> pick one of them? You have for example rule-name (which is the key
>>> of the list but again comes last).
>> This is a bit of compromise between Cisco and Juniper terminology. In Cisco 
>> it is ACE and in Juniper term. But in the yang model and in the text we are 
>> using consistently one or the other word.
> 
> For the sake of clarity, I personally would prefer to have a single
> term. I think Linux packet filters call things rules. I think BSD
> packet filtering calls things rules. Wikipedia seem say that packet
> filters consist of filtering rules... So I guess I have a preference
> but I will not get a sleepless night over this.

Authors are Cisco/Juniper people, so were using that terminology and I believe 
that CSCO and JNPR are more used in networking then Linux :)

> 
>>> - Should there be features for ace-eth and ace-ipv4 and ace-ipv6 or do
>>> we expect that all implementations will always support all three?
>>> Another option would be to have the generic model completely without
>>> any protocol specific elements and to have separate models to
>>> augment in ipv4, ipv6, and ethernet specific ace types. I actually
>>> think this would make much more sense since you would not have a
>>> module 'packet fields' but instead a clear extension of the
>>> ietf-access-control-list module. You could also drop the examples
>>> how to extend the core model since the ip and ethernet extensions
>>> would already serve the purpose. You also would not need features
>>> since an implementation advertises the extension modules.
>> 
>> We started early with such design, but then the WG feedback moved us to the 
>> current  design.
> 
> Can you provide pointers to minutes etc? I think the routing model
> also has a core model where even unicast routing is augmented in. This
> seems to make the boundary between the generic infrastructure and the
> addins very clear.

I haven’t seen a network device with filters without supporting L2 and L3 
types. This is how we started originally, but then comments came in on the 
mailing list and wanted exact typing. Please see the thread

http://www.ietf.org/mail-archive/web/netmod/current/msg12144.html

Model was changed for IETF 94 and at the WG meeting nobody raised the issue.

> 
>>> - The 'uses packet-fields:metadata' resolves to a leaf
>>> 'input-interface' which is of type string. Is this the only metadata
>>> field we ever envision to be there? If so, why not make it part of
>>> the base model? If not, what is the extensibility model here?  
>> 
>> It can be used for route filtering
> 
> Yeah, but the questions were:
> 
> - Is this the only metadata field we ever envision to be there?
No, it can be timestamp, RIB, VRF, etc.

> - If so, why not make it part of the base model?

We are early in the modeling and people were asking to remove time range from 
the base model and create separate draft, as many other features are using 
time-range.
> - If not, what is the extensibility model here?
> 
>>> And
>>> should the interface reference not use a more specific type than
>>> 'string’?
>> 
>> Interface references can be many things, from standard naming we are 
>> familiar, e.g. ge-1/0/0.1 to a numerical value like 13276. Leaving it as 
>> string gives us most flexibility in that regards.
> 
> I disagree that the goal here is most flexibility. We do have an
> interfaces data model in the IETF. Why are we avoiding to refer to it
> here?

There is discussion in separate thread on this. And it seems we are in 
agreement to move this model to YANG 1.1 and use Martin B proposed solution.
> 
>>> - Some implementations support the action to jump or goto other
>>> acls. Is this not common enough to include it as a base action,
>>> perhaps protected by a feature?
>> 
>> Yes among vendors, but it can be very specific. Example, in Juniper 
>> implementation there are two types of filters, classical and Fast Update 
>> Filters (FUF). Classic filters support (on specific HW platforms) filter as 
>> action, but FUF does not. Not all terms and not all actions are supported on 
>> FUFs. So you have to see what filter type and what platform it is, in order 
>> to such a specific action.
> 
> What speaks against making jump and/or call actions a feature?

jump is not supported on all platforms and all types of filters.
> 
>>> - In the example in section 4.3, does the CLI shown really help much?
>>> The syntax is pretty system specific.
>> 
>> OTH, it is widely understood and self explanatory.
> 
> The question was whether it is needed to show system specific CLI.
> BTW, I note that the textual description says "deny ... from
> _leaving_".  How is the 'from leaving' translated into the XML? Is
> there any distinction between input and output filters (or forwarding
> filters or whatever your engine supports)?

We can improve the text description. Why not show system specific CLI? It is 
something widely understood and it is an example. 

This is what is being translated into XML
               access-list ip sample-ip-acl
               deny tcp host 10.10.10.1 host 10.10.10.255

And usually people understand real example better, then just text description.

> 
>>> In the XML shown, can you not
>>> leave out all the fields that are not set? This would remove a lot
>>> of noise. I do not understand what having both actions deny and
>>> permit at the same time means. Did you validate the example against
>>> the data model? (I also find the keys at the end somewhat strange
>>> and not that NETCONF XML serialization actually requires the keys to
>>> be sent first.)
> 
>> We used pyang sample xml skeleton to create the xml example.
> 
> Whatever, the noise does not really help and the example might even
> mislead people to believe they have to write down all unused leafs.

We could edit the empty fields out, but from personal experience working with 
customers, I was getting questions that the compiler output and the examples 
are not matching (it was vendor data modeling language).
> 
>> Leaving both actions in the document is my fault. In the model, action is a 
>> choice, and it was a bad copy paste job. Didn’t choose :)
>>> 
>>> - The clarification whether the boundary port numbers are included in
>>> a port range should be in the YANG model. The YANG model should also
>>> say what it means if one of the ranges is not present. We should not
>>> define the semantics by examples.
>> I’m loosing you here. We are stating in the model if the boundary port 
>> numbers are included in a port rang.
> 
> Ack, I found it in the description of container source-port-range (and
> previously I looked at lower-port and upper-port).
> 
>>> 
>>> - I do not understand section 5. It shows some nftables syntax but
>>> does not really shed a light on what the example shown does or how
>>> that maps to the YANG data model. So I am left somewhat clueless.
>> 
>> Just wanted to point out that the basic structure of iptables is similar to 
>> ACL yang model, so a translation from one to the other should be a  simple 
>> process. If someone wants to make it compliant.
> 
> It escapes me how the iptables fragment shown in page 18 maps to the
> YANG model. What is the substance behind the statement "It should be
> fairly easy to do translation between ACL YANG model described in this
> draft and Linux nftables."? Has someone done a mapping? It is not
> obvious to me how this works.

No, the mapping between nftables and ACL YANG model  has not been finished. 
> 
>>> - Can I trigger multiple actions from an ace? Or do I have to
>>> replicate the ace to do that? In general, there is extension of
>>> a choice (means one of several) and there are extension of a
>>> choice already present.
>> Again, this is very platform dependent, so didn’t want to include it in the 
>> base model.
>> 
> 
> So what would I do if I want to 'log' and 'deny'? Even if the base
> model does not do that, how can I augment this in? The base model must
> provide the necessary extensibility hooks.

Logging/counting and another action (permit/deny) is possible with most 
vendors, but other multiple actions are not by most vendors, example to jump to 
another ACE or ACL. You have to replicate the ACE to do that. So to make it 
simple, this was the choice we made.
> 
>>> - Empty description statements or descriptions statements "(null)"
>>> need to be fixed.
>> 
>> There are no empty descriptions. You are probably referring to 
>> ietf-packet-fields.yang model. There is a bug in pyang which was reported to 
>> Martin and fixed since.
> 
> I am talking about this (page 25):
> 
>      description "(null)”;

OK, got it. 
> 
>>> - I am a bit surprised that we do not define how to attach an ACL to
>>> an interfaces or a set of interfaces given that we do have an
>>> interfaces YANG data model. The example given in A.3 makes me wonder
>>> why there should be a counter in the interfaces config tree. The
>>> targets/interface-name back-reference seems to indicate that I can
>>> attach an ACL only to a single interface.
>> 
>> I wasn’t the author of that section
> 
> This is a WG document so this needs to be resolved.

You are right. 
> 
>>> - I do not understand A.4 at all. Defining an identity does not make
>>> the choice work differently.
>>> 
>>> - Has someone tried to implement this?
>> 
>> Was working on implementation while at my previous employer, but am not 
>> aware of any other implementation.
> 
> I believe it will help a lot to have prototype implementations of this
> data model in order to make sure the model does actually work and
> provides the extensibility hooks that are necessary. In particular, I
> would feel much more convinced if it would be clear (as a proof of
> concept) how the model maps to say Linux packet filters. I would
> actually not mind if mapping examples from the data model to various
> filtering engines would be in the appendix. This would give evidence
> that people have actually worked through a couple of concrete
> examples.

In this case, I can’t help at the moment, as have left the previous company 
where I was working on an implementation. 
> 
> /js
> 
> -- 
> Juergen Schoenwaelder           Jacobs University Bremen gGmbH
> Phone: +49 421 200 3587         Campus Ring 1 | 28759 Bremen | Germany
> Fax:   +49 421 200 3103         <http://www.jacobs-university.de/>

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

Reply via email to