ovn: l3ha, make is_chassis_active aware of gateway_chassis

is_chassis_active now is only true for redirect-chassis ports
in the case of the specific lport being active on the
local chassis.

This will naturally make the ARP responder / redirection openflow
rules automatically inserted/removed when a router goes active/backup
in a way that BACKUP routers won't respond to ARP on gateway ports,
and they won't route packets that arrive on the wrong gateway
chassis (that can happen until all hypervisors converge in the
new MASTER thanks to the BFD monitoring of the tunnel endpoints).

Signed-off-by: Miguel Angel Ajo <majopela@redhat.com>
Signed-off-by: Russell Bryant <russell@ovn.org>

10 files changed, 117 insertions, 44 deletions

diff --git a/ovn/controller/binding.c b/ovn/controller/binding.c
index 4744cf0a4..a80671aa9 100644
--- a/ovn/controller/binding.c
+++ b/ovn/controller/binding.c
@@ -411,20 +411,10 @@ consider_local_datapath(struct controller_ctx *ctx,
                                                        chassis_index);
         if (gateway_chassis &&
             gateway_chassis_contains(gateway_chassis, chassis_rec)) {
-            struct gateway_chassis *gwc;
-            LIST_FOR_EACH (gwc, node, gateway_chassis) {
-                if (!gwc->db->chassis) {
-                    continue;
-                }
-                if (!strcmp(gwc->db->chassis->name, chassis_rec->name)) {
-                    /* sb_rec_port_binding->chassis should reflect master */
-                    our_chassis = true;
-                    break;
-                }
-                if (sset_contains(active_tunnels, gwc->db->chassis->name)) {
-                    break;
-                }
-            }
+
+            our_chassis = gateway_chassis_is_active(
+                gateway_chassis, chassis_rec, active_tunnels);
+
             add_local_datapath(ldatapaths, lports, binding_rec->datapath,
                                false, local_datapaths);
         }
diff --git a/ovn/controller/gchassis.c b/ovn/controller/gchassis.c
index 5edcd7f3e..fedb4329c 100644
--- a/ovn/controller/gchassis.c
+++ b/ovn/controller/gchassis.c
@@ -17,6 +17,7 @@
 
 #include "gchassis.h"
 #include "lport.h"
+#include "lib/sset.h"
 #include "openvswitch/vlog.h"
 #include "ovn/lib/chassis-index.h"
 #include "ovn/lib/ovn-sb-idl.h"
@@ -110,7 +111,7 @@ gateway_chassis_get_ordered(const struct sbrec_port_binding *binding,
 }
 
 bool
-gateway_chassis_contains(struct ovs_list *gateway_chassis,
+gateway_chassis_contains(const struct ovs_list *gateway_chassis,
                          const struct sbrec_chassis *chassis) {
     struct gateway_chassis *chassis_item;
     if (gateway_chassis) {
@@ -174,3 +175,48 @@ gateway_chassis_in_pb_contains(const struct sbrec_port_binding *binding,
 
     return false;
 }
+
+bool
+gateway_chassis_is_active(const struct ovs_list *gateway_chassis,
+                          const struct sbrec_chassis *local_chassis,
+                          const struct sset *active_tunnels)
+{
+    struct gateway_chassis *gwc;
+
+    if (!gateway_chassis
+        || (gateway_chassis && ovs_list_is_empty(gateway_chassis))) {
+        return false;
+    }
+    /* if there's only one chassis, and local chassis is on the list
+     * it's not HA and it's the equivalent of being active */
+    if (ovs_list_is_singleton(gateway_chassis) &&
+        gateway_chassis_contains(gateway_chassis, local_chassis)) {
+        return true;
+    }
+
+    /* if there are no other tunnels active, we assume that the
+     * connection providing tunneling is down, hence we're down */
+    if (sset_is_empty(active_tunnels)) {
+        return false;
+    }
+
+    /* gateway_chassis is an ordered list, by priority, of chassis
+     * hosting the redirect of the port */
+    LIST_FOR_EACH (gwc, node, gateway_chassis) {
+        if (!gwc->db->chassis) {
+            continue;
+        }
+        /* if we found the chassis on the list, and we didn't exit before
+         * on the active_tunnels check for other higher priority chassis
+         * being active, then this chassis is master. */
+        if (!strcmp(gwc->db->chassis->name, local_chassis->name)) {
+            return true;
+        }
+        /* if we find this specific chassis on the list to have an active
+         * tunnel, then 'local_chassis' is not master */
+        if (sset_contains(active_tunnels, gwc->db->chassis->name)) {
+            return false;
+        }
+    }
+    return false;
+}
diff --git a/ovn/controller/gchassis.h b/ovn/controller/gchassis.h
index 46d5e4294..c16529e74 100644
--- a/ovn/controller/gchassis.h
+++ b/ovn/controller/gchassis.h
@@ -26,6 +26,7 @@ struct ovsdb_idl;
 struct sbrec_chassis;
 struct sbrec_gateway_chassis;
 struct sbrec_port_binding;
+struct sset;
 
 
 /* Gateway_Chassis management
@@ -48,7 +49,7 @@ struct ovs_list *gateway_chassis_get_ordered(
 
 /* Checks if an specific chassis is contained in the gateway_chassis
  * list */
-bool gateway_chassis_contains(struct ovs_list *gateway_chassis,
+bool gateway_chassis_contains(const struct ovs_list *gateway_chassis,
                               const struct sbrec_chassis *chassis);
 
 /* Destroy a gateway_chassis list from memory */
@@ -60,4 +61,11 @@ bool gateway_chassis_in_pb_contains(
         const struct sbrec_port_binding *binding,
         const struct sbrec_chassis *chassis);
 
+/* Returns true if the local chassis is the active gateway among a set
+ * of gateway_chassis.  Return false if the local chassis is currently a
+ * backup in a set of multiple gateway_chassis. */
+bool gateway_chassis_is_active(
+        const struct ovs_list *gateway_chassis,
+        const struct sbrec_chassis *local_chassis,
+        const struct sset *active_tunnels);
 #endif /* ovn/controller/gchassis.h */
diff --git a/ovn/controller/lflow.c b/ovn/controller/lflow.c
index 8cc5e7e2a..f868e1fe5 100644
--- a/ovn/controller/lflow.c
+++ b/ovn/controller/lflow.c
@@ -54,10 +54,13 @@ struct lookup_port_aux {
 struct condition_aux {
     const struct lport_index *lports;
     const struct sbrec_chassis *chassis;
+    const struct sset *active_tunnels;
+    const struct chassis_index *chassis_index;
 };
 
 static void consider_logical_flow(const struct lport_index *lports,
                                   const struct mcgroup_index *mcgroups,
+                                  const struct chassis_index *chassis_index,
                                   const struct sbrec_logical_flow *lflow,
                                   const struct hmap *local_datapaths,
                                   struct group_table *group_table,
@@ -66,7 +69,8 @@ static void consider_logical_flow(const struct lport_index *lports,
                                   struct hmap *dhcpv6_opts,
                                   uint32_t *conj_id_ofs,
                                   const struct shash *addr_sets,
-                                  struct hmap *flow_table);
+                                  struct hmap *flow_table,
+                                  struct sset *active_tunnels);
 
 static bool
 lookup_port_cb(const void *aux_, const char *port_name, unsigned int *portp)
@@ -97,10 +101,24 @@ is_chassis_resident_cb(const void *c_aux_, const char *port_name)
 
     const struct sbrec_port_binding *pb
         = lport_lookup_by_name(c_aux->lports, port_name);
-    if (pb && pb->chassis && pb->chassis == c_aux->chassis) {
-        return true;
+    if (!pb) {
+        return false;
+    }
+    if (strcmp(pb->type, "chassisredirect")) {
+        /* for non-chassisredirect ports */
+        return pb->chassis && pb->chassis == c_aux->chassis;
     } else {
-        return gateway_chassis_in_pb_contains(pb, c_aux->chassis);
+        struct ovs_list *gateway_chassis;
+        gateway_chassis = gateway_chassis_get_ordered(pb,
+                                                      c_aux->chassis_index);
+        if (gateway_chassis) {
+            bool active = gateway_chassis_is_active(gateway_chassis,
+                                                    c_aux->chassis,
+                                                    c_aux->active_tunnels);
+            gateway_chassis_destroy(gateway_chassis);
+            return active;
+        }
+        return false;
     }
 }
 
@@ -124,11 +142,13 @@ is_gateway_router(const struct sbrec_datapath_binding *ldp,
 static void
 add_logical_flows(struct controller_ctx *ctx, const struct lport_index *lports,
                   const struct mcgroup_index *mcgroups,
+                  const struct chassis_index *chassis_index,
                   const struct hmap *local_datapaths,
                   struct group_table *group_table,
                   const struct sbrec_chassis *chassis,
                   const struct shash *addr_sets,
-                  struct hmap *flow_table)
+                  struct hmap *flow_table,
+                  struct sset *active_tunnels)
 {
     uint32_t conj_id_ofs = 1;
     const struct sbrec_logical_flow *lflow;
@@ -149,10 +169,11 @@ add_logical_flows(struct controller_ctx *ctx, const struct lport_index *lports,
     }
 
     SBREC_LOGICAL_FLOW_FOR_EACH (lflow, ctx->ovnsb_idl) {
-        consider_logical_flow(lports, mcgroups, lflow, local_datapaths,
+        consider_logical_flow(lports, mcgroups, chassis_index,
+                              lflow, local_datapaths,
                               group_table, chassis,
                               &dhcp_opts, &dhcpv6_opts, &conj_id_ofs,
-                              addr_sets, flow_table);
+                              addr_sets, flow_table, active_tunnels);
     }
 
     dhcp_opts_destroy(&dhcp_opts);
@@ -162,6 +183,7 @@ add_logical_flows(struct controller_ctx *ctx, const struct lport_index *lports,
 static void
 consider_logical_flow(const struct lport_index *lports,
                       const struct mcgroup_index *mcgroups,
+                      const struct chassis_index *chassis_index,
                       const struct sbrec_logical_flow *lflow,
                       const struct hmap *local_datapaths,
                       struct group_table *group_table,
@@ -170,7 +192,8 @@ consider_logical_flow(const struct lport_index *lports,
                       struct hmap *dhcpv6_opts,
                       uint32_t *conj_id_ofs,
                       const struct shash *addr_sets,
-                      struct hmap *flow_table)
+                      struct hmap *flow_table,
+                      struct sset *active_tunnels)
 {
     /* Determine translation of logical table IDs to physical table IDs. */
     bool ingress = !strcmp(lflow->pipeline, "ingress");
@@ -267,7 +290,8 @@ consider_logical_flow(const struct lport_index *lports,
         return;
     }
 
-    struct condition_aux cond_aux = { lports, chassis };
+    struct condition_aux cond_aux = { lports, chassis, active_tunnels,
+                                      chassis_index};
     expr = expr_simplify(expr, is_chassis_resident_cb, &cond_aux);
     expr = expr_normalize(expr);
     uint32_t n_conjs = expr_to_matches(expr, lookup_port_cb, &aux,
@@ -393,13 +417,16 @@ lflow_run(struct controller_ctx *ctx,
           const struct sbrec_chassis *chassis,
           const struct lport_index *lports,
           const struct mcgroup_index *mcgroups,
+          const struct chassis_index *chassis_index,
           const struct hmap *local_datapaths,
           struct group_table *group_table,
           const struct shash *addr_sets,
-          struct hmap *flow_table)
+          struct hmap *flow_table,
+          struct sset *active_tunnels)
 {
-    add_logical_flows(ctx, lports, mcgroups, local_datapaths, group_table,
-                      chassis, addr_sets, flow_table);
+    add_logical_flows(ctx, lports, mcgroups, chassis_index, local_datapaths,
+                      group_table, chassis, addr_sets, flow_table,
+                      active_tunnels);
     add_neighbor_flows(ctx, lports, flow_table);
 }
 
diff --git a/ovn/controller/lflow.h b/ovn/controller/lflow.h
index a23cde099..484502f4f 100644
--- a/ovn/controller/lflow.h
+++ b/ovn/controller/lflow.h
@@ -35,6 +35,7 @@
 
 #include <stdint.h>
 
+struct chassis_index;
 struct controller_ctx;
 struct group_table;
 struct hmap;
@@ -42,6 +43,7 @@ struct lport_index;
 struct mcgroup_index;
 struct sbrec_chassis;
 struct simap;
+struct sset;
 struct uuid;
 
 /* OpenFlow table numbers.
@@ -66,10 +68,12 @@ void lflow_run(struct controller_ctx *,
                const struct sbrec_chassis *chassis,
                const struct lport_index *,
                const struct mcgroup_index *,
+               const struct chassis_index *,
                const struct hmap *local_datapaths,
                struct group_table *group_table,
                const struct shash *addr_sets,
-               struct hmap *flow_table);
+               struct hmap *flow_table,
+               struct sset *active_tunnels);
 void lflow_destroy(void);
 
 #endif /* ovn/lflow.h */
diff --git a/ovn/controller/lport.c b/ovn/controller/lport.c
index 906fda222..28f5d7726 100644
--- a/ovn/controller/lport.c
+++ b/ovn/controller/lport.c
@@ -15,11 +15,11 @@
 
 #include <config.h>
 
+#include "lib/sset.h"
 #include "lport.h"
 #include "hash.h"
 #include "openvswitch/vlog.h"
 #include "ovn/lib/ovn-sb-idl.h"
-
 VLOG_DEFINE_THIS_MODULE(lport);
 
 static struct ldatapath *ldatapath_lookup_by_key__(
diff --git a/ovn/controller/lport.h b/ovn/controller/lport.h
index 8937ecb6c..9a5a5dad8 100644
--- a/ovn/controller/lport.h
+++ b/ovn/controller/lport.h
@@ -26,6 +26,7 @@ struct sbrec_chassis;
 struct sbrec_datapath_binding;
 struct sbrec_port_binding;
 
+
 /* Database indexes.
  * =================
  *
diff --git a/ovn/controller/ovn-controller.c b/ovn/controller/ovn-controller.c
index 97153718e..02e6ef536 100644
--- a/ovn/controller/ovn-controller.c
+++ b/ovn/controller/ovn-controller.c
@@ -649,7 +649,6 @@ main(int argc, char *argv[])
                         &chassis_index, &active_tunnels, &local_datapaths,
                         &local_lports);
         }
-
         if (br_int && chassis) {
             struct shash addr_sets = SHASH_INITIALIZER(&addr_sets);
             addr_sets_init(&ctx, &addr_sets);
@@ -668,8 +667,8 @@ main(int argc, char *argv[])
 
                     struct hmap flow_table = HMAP_INITIALIZER(&flow_table);
                     lflow_run(&ctx, chassis, &lports, &mcgroups,
-                              &local_datapaths, &group_table,
-                              &addr_sets, &flow_table);
+                              &chassis_index, &local_datapaths, &group_table,
+                              &addr_sets, &flow_table, &active_tunnels);
 
                     if (chassis_id) {
                         bfd_run(&ctx, br_int, chassis, &local_datapaths,
@@ -678,7 +677,7 @@ main(int argc, char *argv[])
                     physical_run(&ctx, mff_ovn_geneve,
                                  br_int, chassis, &ct_zones, &lports,
                                  &flow_table, &local_datapaths, &local_lports,
-                                 &chassis_index);
+                                 &chassis_index, &active_tunnels);
 
                     ofctrl_put(&flow_table, &pending_ct_zones,
                                get_nb_cfg(ctx.ovnsb_idl));
diff --git a/ovn/controller/physical.c b/ovn/controller/physical.c
index 0588d0034..1d0fe7fc9 100644
--- a/ovn/controller/physical.c
+++ b/ovn/controller/physical.c
@@ -295,6 +295,7 @@ consider_port_binding(enum mf_field_id mff_ovn_geneve,
                       const struct simap *ct_zones,
                       const struct lport_index *lports,
                       const struct chassis_index *chassis_index,
+                      struct sset *active_tunnels,
                       struct hmap *local_datapaths,
                       const struct sbrec_port_binding *binding,
                       const struct sbrec_chassis *chassis,
@@ -362,15 +363,10 @@ consider_port_binding(enum mf_field_id mff_ovn_geneve,
     struct ovs_list *gateway_chassis
         = gateway_chassis_get_ordered(binding, chassis_index);
 
-    /* XXX: later in the series we should insert the next flows only
-     * on the active chassis, and not on all of them. This is useful to
-     * check that the BFD implementation on following patches has
-     * an effect and routes packet by the chassis which is responding,
-     * but later on we should not create those flows on all the
-     * chassis of the gateway_chassis list */
     if (!strcmp(binding->type, "chassisredirect")
         && (binding->chassis == chassis
-            || gateway_chassis_contains(gateway_chassis, chassis))) {
+            || gateway_chassis_is_active(gateway_chassis, chassis,
+                                         active_tunnels))) {
 
         /* Table 33, priority 100.
          * =======================
@@ -865,7 +861,8 @@ physical_run(struct controller_ctx *ctx, enum mf_field_id mff_ovn_geneve,
              const struct simap *ct_zones, struct lport_index *lports,
              struct hmap *flow_table, struct hmap *local_datapaths,
              const struct sset *local_lports,
-             struct chassis_index *chassis_index)
+             struct chassis_index *chassis_index,
+             struct sset *active_tunnels)
 {
 
     /* This bool tracks physical mapping changes. */
@@ -987,7 +984,7 @@ physical_run(struct controller_ctx *ctx, enum mf_field_id mff_ovn_geneve,
     const struct sbrec_port_binding *binding;
     SBREC_PORT_BINDING_FOR_EACH (binding, ctx->ovnsb_idl) {
         consider_port_binding(mff_ovn_geneve, ct_zones, lports,
-                              chassis_index,
+                              chassis_index, active_tunnels,
                               local_datapaths, binding, chassis,
                               &ofpacts, flow_table);
     }
diff --git a/ovn/controller/physical.h b/ovn/controller/physical.h
index 901962189..3dfc64c43 100644
--- a/ovn/controller/physical.h
+++ b/ovn/controller/physical.h
@@ -49,6 +49,7 @@ void physical_run(struct controller_ctx *, enum mf_field_id mff_ovn_geneve,
                   const struct simap *ct_zones, struct lport_index *,
                   struct hmap *flow_table, struct hmap *local_datapaths,
                   const struct sset *local_lports,
-                  struct chassis_index *chassis_index);
+                  struct chassis_index *chassis_index,
+                  struct sset *active_tunnels);
 
 #endif /* ovn/physical.h */