bhouse-nexthop commented on PR #14199: URL: https://github.com/apache/cloudstack/pull/14199#issuecomment-6173454489
Yes, I measured it. I fed the rules `AclDevice` generates through the unmodified `CsNetfilters.compare()`, against real iptables (1.8.10, nf_tables backend) in a throwaway network namespace. The test ACL has one TCP rule with N CIDRs, followed by a single-CIDR allow and a deny. Times are wall clock, with the number of `iptables` processes run: | N | before, first apply | before, re-apply | after, first apply | after, re-apply | |---|---|---|---|---| | 100 | 0.02s / 8 | 1.5s / 106 | 0.3s / 107 | 2.0s / 205 | | 300 | 0.03s / 8 | 5.1s / 306 | 1.0s / 307 | 6.6s / 605 | | 1000 | rule not applied (`iptables: Message too long`) | - | 9.8s / 1007 | 43.9s / 2005 | Generating the rules in Python takes about 1ms either way. Most of this cost was already there before the PR. `has_rule()` never matches a rule that has a position, so on every apply each ACL rule is inserted again and its old copy is deleted as unseen. Those old copies were already one rule per CIDR, because iptables had expanded `-s a,b,c`. So a re-apply was already 1 insert + N deletes. With this PR it is N inserts + N deletes. The deletes are the slow part: about 17ms each at N=300, against about 3ms per insert, because each call reads the whole ruleset. At N=300 this PR adds roughly 1 to 1.5s to an apply that already took 5s. That is also exactly what the current workaround costs, since N single-CIDR rules produce the same iptables calls. The real fix is to batch the changes through `iptables-restore --noflush`. That way, 300 inserts plus 300 deletes take about 0.02s. It would change how every iptables rule on the VR is applied, though: - Failure handling changes: one bad line fails the whole batch. - It needs chunking: a 1000-rule batch also fails with `Message too long`. So I'd rather keep it out of this bug fix and do it as a separate PR. I'm happy to open one. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
