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 <[email protected]> Signed-off-by: Russell Bryant <[email protected]>
lzg19 · Jul 16, 2017 · 508b7f9 · 508b7f9
1 parent 3475695
commit 508b7f9
Show file tree

Hide file tree

Showing 11 changed files with 193 additions and 54 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -35,13 +35,15 @@
 
 #include <stdint.h>
 
+struct chassis_index;
 struct controller_ctx;
 struct group_table;
 struct hmap;
 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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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 */