In case there are many ACLs referencing the same address set or a port
group, or just listing the same IPs within a match, they will end up
referencing the same conjunctive OpenFlow rules and all end up on the
'references' list. If there are hundreds+ of these ACLs, the iteration
over this list becomes a performance bottleneck.
While merging conjunctive flows, we iterate over all the references just
to untrack their address sets, if any. However, only the first time
this operation is needed. While merging every other flow after the first
one, all the references are already untracked. So, this loop is a
complete waste of time in most cases.
Let's count how many references are actually currently tracking any
address sets and only try to untrack if there is something to untrack.
It should not be possible for flows to gain address set references after
they are already in the list of references.
In a setup with 3.5K ACLs referencing the same-ish 450 IP addresses and
a port group with ~20 ports, this change reduces the average full
recompute time from 48 to 22 seconds, which is a 2.2x speed up.
Fixes: eeb51fa20bc7 ("ovn-controller: Track individual IP information of
address set during lflow parsing.")
Reported-at: https://issues.redhat.com/browse/FDP-3319
Signed-off-by: Ilya Maximets <[email protected]>
---
controller/ofctrl.c | 44 ++++++++++++++++++++++++++++++++++++--------
1 file changed, 36 insertions(+), 8 deletions(-)
diff --git a/controller/ofctrl.c b/controller/ofctrl.c
index 81cc17ad7..4d53dd049 100644
--- a/controller/ofctrl.c
+++ b/controller/ofctrl.c
@@ -155,6 +155,10 @@ struct desired_flow {
* Key is the SB UUID. (There are cases that multiple SB entities share
* the same desired OpenFlow flow, e.g. when conjunction is used.) */
struct hmap references;
+ /* The number of flows in 'references' that are tracking some address
+ * sets, i.e., have their 'sb_flow_ref.as_ip_flow_list' node in the
+ * 'as_ip_to_flow_node.flows'. */
+ size_t referenced_as_ip_flows;
/* The corresponding flow in installed table. */
struct installed_flow *installed_flow;
@@ -1120,6 +1124,26 @@ sb_addrset_ref_size(const struct sb_addrset_ref *sar)
return sizeof *sar + strlen(sar->name) + 1;
}
+static void
+sb_flow_ref_as_ip_untrack(struct desired_flow *f, struct sb_flow_ref *sfr)
+{
+ if (ovs_list_is_empty(&sfr->as_ip_flow_list)) {
+ return;
+ }
+ ovs_list_remove(&sfr->as_ip_flow_list);
+ ovs_list_init(&sfr->as_ip_flow_list);
+ f->referenced_as_ip_flows--;
+}
+
+static void
+sb_flow_ref_as_ip_track(struct desired_flow *f,
+ struct as_ip_to_flow_node *itfn,
+ struct sb_flow_ref *sfr)
+{
+ ovs_list_insert(&itfn->flows, &sfr->as_ip_flow_list);
+ f->referenced_as_ip_flows++;
+}
+
static struct sb_to_flow *
sb_to_flow_find(struct hmap *uuid_flow_table, const struct uuid *sb_uuid)
{
@@ -1208,7 +1232,7 @@ link_flow_to_sb(struct ovn_desired_flow_table *flow_table,
hmap_insert(&sar->as_ip_to_flow_map, &itfn->hmap_node, hash);
}
- ovs_list_insert(&itfn->flows, &sfr->as_ip_flow_list);
+ sb_flow_ref_as_ip_track(f, itfn, sfr);
}
/* Flow table interfaces to the rest of ovn-controller. */
@@ -1399,10 +1423,12 @@ ofctrl_add_or_append_conj_flow(struct
ovn_desired_flow_table *desired_flows,
* lflows, but we need to remove the related conjunction from the
* actions properly when handle addrset ip deletion, instead of simply
* delete the flow. */
- struct sb_flow_ref *sfr;
- HMAP_FOR_EACH (sfr, sb_node, &f->references) {
- ovs_list_remove(&sfr->as_ip_flow_list);
- ovs_list_init(&sfr->as_ip_flow_list);
+ if (f->referenced_as_ip_flows) {
+ struct sb_flow_ref *sfr;
+ HMAP_FOR_EACH (sfr, sb_node, &f->references) {
+ sb_flow_ref_as_ip_untrack(f, sfr);
+ }
+ ovs_assert(!f->referenced_as_ip_flows);
}
if (flow_contains_sb_reference(f, sb_uuid)) {
@@ -1538,7 +1564,7 @@ ofctrl_remove_flows_for_as_ip(struct
ovn_desired_flow_table *flow_table,
hmap_remove(&f->references, &sfr->sb_node);
ovs_list_remove(&sfr->flow_list);
- ovs_list_remove(&sfr->as_ip_flow_list);
+ sb_flow_ref_as_ip_untrack(f, sfr);
mem_stats.sb_flow_ref_usage -= sb_flow_ref_size(sfr);
free(sfr);
@@ -1577,7 +1603,7 @@ remove_flows_from_sb_to_flow(struct
ovn_desired_flow_table *flow_table,
hmap_remove(&f->references, &sfr->sb_node);
ovs_list_remove(&sfr->flow_list);
- ovs_list_remove(&sfr->as_ip_flow_list);
+ sb_flow_ref_as_ip_untrack(f, sfr);
mem_stats.sb_flow_ref_usage -= sb_flow_ref_size(sfr);
free(sfr);
@@ -1624,7 +1650,7 @@ remove_flows_from_sb_to_flow(struct
ovn_desired_flow_table *flow_table,
ovs_assert(hmap_count(&f->references));
HMAP_FOR_EACH (sfr, sb_node, &f->references) {
ovs_list_remove(&sfr->flow_list);
- ovs_list_remove(&sfr->as_ip_flow_list);
+ sb_flow_ref_as_ip_untrack(f, sfr);
}
}
LIST_FOR_EACH_SAFE (f, list_node, &to_be_removed) {
@@ -1678,6 +1704,7 @@ desired_flow_alloc(uint8_t table_id, uint16_t priority,
uint64_t cookie,
{
struct desired_flow *f = xmalloc(sizeof *f);
hmap_init(&f->references);
+ f->referenced_as_ip_flows = 0;
ovs_list_init(&f->list_node);
ovs_list_init(&f->installed_ref_list_node);
ovs_list_init(&f->track_list_node);
@@ -1887,6 +1914,7 @@ desired_flow_destroy(struct desired_flow *f)
{
if (f) {
ovs_assert(!hmap_count(&f->references));
+ ovs_assert(!f->referenced_as_ip_flows);
ovs_assert(!f->installed_flow);
mem_stats.desired_flow_usage -= desired_flow_size(f);
hmap_destroy(&f->references);
--
2.53.0
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
