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 would have put the acl-name and acl-type first followed by the entries list. - 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). - 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. - 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? And should the interface reference not use a more specific type than 'string'? - 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? - In the example in section 4.3, does the CLI shown really help much? The syntax is pretty system specific. 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.) - 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 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. - 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. - Empty description statements or descriptions statements "(null)" need to be fixed. - 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 do not understand A.4 at all. Defining an identity does not make the choice work differently. - Has someone tried to implement this? Organization - Section 3 to a large extend is a repetition of section 1. Perhaps make section 1 shorter and leave the details to section 3? - Some empty lines in the YANG definition would have pleased my eyes. Editorial - there are a couple of missing articles throughout the text - sometimes singular/plural is at odds - s/this draft/this document/ - you may add another 'e' to my name ;-) /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
