On 9/10/20 10:33 PM, Mark Michelson wrote: > Acked-by: Mark Michelson <[email protected]> >
Thanks for the review! > Ideally, I'd like it more if when we detected conflicts we just didn't > install ANY of the conflicts, just because it's not always possible to > know what the intention of the admin was. This sort of behavior > encourages admins to leave in ACL conflicts instead of fixing the damn > things :) > > However, I also understand that from an admin's perspective, it may not > be clear that ACLs are actually in conflict. And in the majority of > cases, ACLs will be created in this way where a less restrictive deny > should be installed instead of the more restrictive allows. This means > we're more likely to deny traffic than allow it, and that's better than > allowing traffic that is expected to be denied. > > At least this change logs the conflicting ACLs so that the admin has the > option to fix it if it's not doing what's expected. > Maybe we can think of an option to help out admins pinpoint such conflicting ACLs "offline". E.g., run a checker tool on the NB database. Nevertheless, I sent a v3 of the patch because I had forgotten to add the last part that actually checks the RX pcap after the less restrictive ACL is removed. As this is just a minor change of the test and not of the fix itself I've also added your ack. v3: http://patchwork.ozlabs.org/project/ovn/patch/[email protected]/ Thanks, Dumitru _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
