From 9d095799aa11c555bf7d5f8e86fee1be1a16c67b Mon Sep 17 00:00:00 2001
From: Dumitru Ceara <dceara@redhat.com>
Date: Sun, 11 Oct 2020 14:05:31 +0200
Subject: [PATCH 3/7] ofctrl.c: Only merge actions for conjunctive flows.
In ofctrl_add_or_append_flow() when merging flow actions make sure we only
do that for conjunctive flows. All other actions can not be merged with
action "conjunction".
CC: Mark Michelson <mmichels@redhat.com>
Fixes: e659bab31a91 ("Combine conjunctions with identical matches into one flow.")
Signed-off-by: Dumitru Ceara <dceara@redhat.com>
Signed-off-by: Han Zhou <hzhou@ovn.org>
(cherry picked from upstream commit dadae4f800ccb1f2759378f0bd804dd002e31605)
Change-Id: I6fc4091b3110ae595615c7f9006c3f5e53ebc39a
---
controller/ofctrl.c | 124 +++++++++++++++++++++++++++++++++++++++++-----------
1 file changed, 99 insertions(+), 25 deletions(-)
diff --git a/controller/ofctrl.c b/controller/ofctrl.c
index 4425d98..24b55fc 100644
--- a/controller/ofctrl.c
+++ b/controller/ofctrl.c
@@ -206,6 +206,9 @@ struct installed_flow {
struct desired_flow *desired_flow;
};
+typedef bool
+(*desired_flow_match_cb)(const struct desired_flow *candidate,
+ const void *arg);
static struct desired_flow *desired_flow_alloc(
uint8_t table_id,
uint16_t priority,
@@ -214,8 +217,14 @@ static struct desired_flow *desired_flow_alloc(
const struct ofpbuf *actions);
static struct desired_flow *desired_flow_lookup(
struct ovn_desired_flow_table *,
+ const struct ovn_flow *target);
+static struct desired_flow *desired_flow_lookup_check_uuid(
+ struct ovn_desired_flow_table *,
const struct ovn_flow *target,
- const struct uuid *sb_uuid);
+ const struct uuid *);
+static struct desired_flow *desired_flow_lookup_conjunctive(
+ struct ovn_desired_flow_table *,
+ const struct ovn_flow *target);
static void desired_flow_destroy(struct desired_flow *);
static struct installed_flow *installed_flow_lookup(
@@ -806,6 +815,19 @@ desired_flow_set_active(struct desired_flow *d)
d->installed_flow->desired_flow = d;
}
+static bool
+flow_action_has_conj(const struct ovn_flow *f)
+{
+ const struct ofpact *a = NULL;
+
+ OFPACT_FOR_EACH (a, f->ofpacts, f->ofpacts_len) {
+ if (a->type == OFPACT_CONJUNCTION) {
+ return true;
+ }
+ }
+ return false;
+}
+
/* Adds the desired flow to the list of desired flows that have same match
* conditions as the installed flow.
*
@@ -962,7 +984,7 @@ ofctrl_check_and_add_flow(struct ovn_desired_flow_table *flow_table,
struct desired_flow *f = desired_flow_alloc(table_id, priority, cookie,
match, actions);
- if (desired_flow_lookup(flow_table, &f->flow, sb_uuid)) {
+ if (desired_flow_lookup_check_uuid(flow_table, &f->flow, sb_uuid)) {
if (log_duplicate_flow) {
static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(5, 5);
if (!VLOG_DROP_DBG(&rl)) {
@@ -1002,14 +1024,15 @@ ofctrl_add_or_append_flow(struct ovn_desired_flow_table *desired_flows,
const struct ofpbuf *actions,
const struct uuid *sb_uuid)
{
- struct desired_flow *f = desired_flow_alloc(table_id, priority, cookie,
- match, actions);
-
struct desired_flow *existing;
- existing = desired_flow_lookup(desired_flows, &f->flow, NULL);
+ struct desired_flow *f;
+
+ f = desired_flow_alloc(table_id, priority, cookie, match, actions);
+ existing = desired_flow_lookup_conjunctive(desired_flows, &f->flow);
if (existing) {
- /* There's already a flow with this particular match. Append the
- * action to that flow rather than adding a new flow
+ /* There's already a flow with this particular match and action
+ * 'conjunction'. Append the action to that flow rather than
+ * adding a new flow.
*/
uint64_t compound_stub[64 / 8];
struct ofpbuf compound;
@@ -1248,15 +1271,11 @@ installed_flow_dup(struct desired_flow *src)
return dst;
}
-/* Finds and returns a desired_flow in 'flow_table' whose key is identical to
- * 'target''s key, or NULL if there is none.
- *
- * If sb_uuid is not NULL, the function will also check if the found flow is
- * referenced by the sb_uuid. */
static struct desired_flow *
-desired_flow_lookup(struct ovn_desired_flow_table *flow_table,
- const struct ovn_flow *target,
- const struct uuid *sb_uuid)
+desired_flow_lookup__(struct ovn_desired_flow_table *flow_table,
+ const struct ovn_flow *target,
+ desired_flow_match_cb match_cb,
+ const void *arg)
{
struct desired_flow *d;
HMAP_FOR_EACH_WITH_HASH (d, match_hmap_node, target->hash,
@@ -1265,20 +1284,76 @@ desired_flow_lookup(struct ovn_desired_flow_table *flow_table,
if (f->table_id == target->table_id
&& f->priority == target->priority
&& minimatch_equal(&f->match, &target->match)) {
- if (!sb_uuid) {
+
+ if (!match_cb || match_cb(d, arg)) {
return d;
}
- struct sb_flow_ref *sfr;
- LIST_FOR_EACH (sfr, sb_list, &d->references) {
- if (uuid_equals(sb_uuid, &sfr->sb_uuid)) {
- return d;
- }
- }
}
}
return NULL;
}
+/* Finds and returns a desired_flow in 'flow_table' whose key is identical to
+ * 'target''s key, or NULL if there is none.
+ */
+static struct desired_flow *
+desired_flow_lookup(struct ovn_desired_flow_table *flow_table,
+ const struct ovn_flow *target)
+{
+ return desired_flow_lookup__(flow_table, target, NULL, NULL);
+}
+
+static bool
+flow_lookup_match_uuid_cb(const struct desired_flow *candidate,
+ const void *arg)
+{
+ const struct uuid *sb_uuid = arg;
+ struct sb_flow_ref *sfr;
+
+ LIST_FOR_EACH (sfr, sb_list, &candidate->references) {
+ if (uuid_equals(sb_uuid, &sfr->sb_uuid)) {
+ return true;
+ }
+ }
+ return false;
+}
+
+/* Finds and returns a desired_flow in 'flow_table' whose key is identical to
+ * 'target''s key, or NULL if there is none.
+ *
+ * The function will also check if the found flow is referenced by the
+ * 'sb_uuid'.
+ */
+static struct desired_flow *
+desired_flow_lookup_check_uuid(struct ovn_desired_flow_table *flow_table,
+ const struct ovn_flow *target,
+ const struct uuid *sb_uuid)
+{
+ return desired_flow_lookup__(flow_table, target, flow_lookup_match_uuid_cb,
+ sb_uuid);
+}
+
+static bool
+flow_lookup_match_conj_cb(const struct desired_flow *candidate,
+ const void *arg OVS_UNUSED)
+{
+ return flow_action_has_conj(&candidate->flow);
+}
+
+/* Finds and returns a desired_flow in 'flow_table' whose key is identical to
+ * 'target''s key, or NULL if there is none.
+ *
+ * The function will only return a matching flow if it contains action
+ * 'conjunction'.
+ */
+static struct desired_flow *
+desired_flow_lookup_conjunctive(struct ovn_desired_flow_table *flow_table,
+ const struct ovn_flow *target)
+{
+ return desired_flow_lookup__(flow_table, target, flow_lookup_match_conj_cb,
+ NULL);
+}
+
/* Finds and returns an installed_flow in installed_flows whose key is
* identical to 'target''s key, or NULL if there is none. */
static struct installed_flow *
@@ -1676,8 +1751,7 @@ update_installed_flows_by_compare(struct ovn_desired_flow_table *flow_table,
struct installed_flow *i, *next;
HMAP_FOR_EACH_SAFE (i, next, match_hmap_node, &installed_flows) {
unlink_all_refs_for_installed_flow(i);
- struct desired_flow *d =
- desired_flow_lookup(flow_table, &i->flow, NULL);
+ struct desired_flow *d = desired_flow_lookup(flow_table, &i->flow);
if (!d) {
/* Installed flow is no longer desirable. Delete it from the
* switch and from installed_flows. */
--
1.8.3.1