Juergen, Please see answers inline
Dean > On Dec 11, 2015, at 12:31 PM, Juergen Schoenwaelder > <[email protected]> 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. > > - 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? > > - 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. > > - 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. > > - 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 > 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. > > - 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. > > - 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. > 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. 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. container source-port-range { presence "Enables setting source port range"; description "Inclusive range representing source ports to be used. When only lower-port is present, it represents a single port."; leaf lower-port { type inet:port-number; mandatory true; description "Lower boundary for port."; } leaf upper-port { type inet:port-number; must ". >= ../lower-port" { error-message "The upper-port must be greater than or equal to lower-port"; } description "Upper boundary for port . If existing, the upper port must be greater or equal to lower-port."; } } Same is done for the 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. > > - 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. > > - 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. pyang --ietf --strict ietf-packet-fields.yang ietf-packet-fields.yang:67: warning: RFC 6087: 4.12: statement "must" should have a "description" substatement ietf-packet-fields.yang:89: warning: RFC 6087: 4.12: statement "must" should have a "description” substatement The code is correct, just the compiler is reporting an error. error-message "The upper-port must be greater than or equal to lower-port"; } description "Upper boundary for port . If existing, the upper port must be greater or equal to lower-port."; } > > - 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 > > - 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. > 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? OK > > - Some empty lines in the YANG definition would have pleased my eyes. OK > > 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 ;-) or ü Will fix those > > /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/> > > _______________________________________________ > Rtg-dt-yang-arch mailing list > [email protected] > https://www.ietf.org/mailman/listinfo/rtg-dt-yang-arch _______________________________________________ netmod mailing list [email protected] https://www.ietf.org/mailman/listinfo/netmod
