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