On Thu, Mar 1, 2018 at 12:26 PM, Guru Shetty <[email protected]> wrote: > > > > On 1 March 2018 at 12:21, Han Zhou <[email protected]> wrote: >> >> >> >> On Thu, Mar 1, 2018 at 12:13 PM, Guru Shetty <[email protected]> wrote: >> > >> > >> > >> > On 28 February 2018 at 19:37, Han Zhou <[email protected]> wrote: >> >> >> >> This patch enables using port group names in ACL match conditions. >> >> Users can create a port group in northbound DB Port_Group table, >> >> and then use the name of the port group in ACL match conditions >> >> for "inport" or "outport". It can help reduce the number of ACLs >> >> for CMS clients such as OpenStack Neutron, for the use cases >> >> where a group of logical ports share same ACL rules except the >> >> "inport"/"outport" part. Without this patch, the clients have to >> >> create N (N = number of lports) ACLs, and this patch helps achieve >> >> the same goal with only one ACL. E.g.: >> >> >> >> to-lport 1000 "outport == @port_group1 && ip4.src == {IP1, IP2, ...}" allow-related >> >> >> >> There was a similar attempt by Zong Kai Li in 2016 [1]. This patch >> >> takes a slightly different approach by using weak refs instead of >> >> strings, which requires a new table instead of reusing the address >> >> set table. This way it will also benefit for a follow up patch that >> >> enables generating address sets automatically from port groups to >> >> avoid a lot a trouble from client perspective [2]. >> >> >> >> [1] https://mail.openvswitch.org/pipermail/ovs-dev/2016-August/077118.html >> >> [2] https://mail.openvswitch.org/pipermail/ovs-discuss/2018-February/046260.html >> >> >> >> Reported-by: Daniel Alvarez Sanchez <[email protected]> >> >> Reported-at: https://mail.openvswitch.org/pipermail/ovs-discuss/2018-February/046166.html >> >> Signed-off-by: Han Zhou <[email protected]> >> > >> > >> > Wouldn't it be more complete and useful if we add the acl to a port group too? And then internally, you decide which switches you want to add the ACL to. >> > >> > For e.g: ovn-nbctl --port-group add-acl port_group1 to-lport 1000 "outport == @port_group1 && ip4.src == {IP1, IP2, ...}" allow-related >> > >> > This way, the client does not have to keep track of all the logical switches it needs to apply an ACL to. Thoughts? >> > >> Yes, this is a good idea. Since it is only about the ovn-nbctl tool improvement, it can be a follow up patch. > > > What if we have something like a acl column in the port_group table so that we just have one entry in OVN NB database? Logically, we apply a ACL to a security group instead of a logical switch. And then ovn-northd decided which logical switches to apply it to. Would that make difference in performance? It does reduce the size of the NB database. Any drawbacks? > Ok, I thought you were talking about ovn-nbctl tool only. Now I get your point. I think it is a good idea, since it is a common work for different clients: figuring out which lswitches are needed for each group of ACLs. So it makes sense to simplify clients implementation and support the feature in OVN. I think it would be better to have 2 columns for ACLs on port-groups, one for to-lports, the other for from-lports. And the match condition "outport/inport == @<port group name>" should be automatically added by northd when processing, instead of filling in the redundant information by clients. Would this sounds better?
This should be able to work without breaking the existing mechanism of specifying ACLs in lswitches. So existing ACL users should not be affected. Performance could be better in clients (networking-ovn, kubernetes-ovn, etc.), since there is no need to figuring out the lswitch list to apply ACLs, although not sure how much improvement it could be. Performance impact from ovn-northd perspective is not sure, because there are less data to process from OVN-NB, but more processing needed for the port-group attached ACLs handling. I can work on it as a follow up patch on top of the current implementation. _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
