Good point! As discussed, I'll add another patch on top of the current ones to support adding ACLs to port groups.
On Mon, Mar 12, 2018 at 7:05 AM, Daniel Alvarez Sanchez <[email protected] > wrote: > FWIW, this [0] is making port deletion slow in OpenStack right now, which > basically is figuring out which ACL's belong to this particular Logical > Switch Port. > On a system with tons of ACL's this is expensive but it would be definitely > improved with this patch (as the # of ACL's will drop down) but we could > totally > get rid of it if we would add the ACLs to the Port Group table. > > [0] https://github.com/openstack/networking-ovn/blob/ > master/networking_ovn/ovsdb/commands.py#L460-#L463 > > > On Thu, Mar 1, 2018 at 9: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/p >> ipermail/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? >> _______________________________________________ >> dev mailing list >> [email protected] >> https://mail.openvswitch.org/mailman/listinfo/ovs-dev >> > > _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
