[ovs-dev] [PATCH v3 5/6] ovn: l3ha, make is_chassis_active aware of gateway_chassis
Miguel Angel Ajo
majopela at redhat.com
Wed Jul 5 13:45:17 UTC 2017
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.
Signed-off-by: Miguel Angel Ajo <majopela at redhat.com>
---
ovn/controller/binding.c | 27 ++++----------
ovn/controller/binding.h | 3 +-
ovn/controller/gchassis.c | 47 ++++++++++++++++++++++-
ovn/controller/gchassis.h | 10 ++++-
ovn/controller/lflow.c | 51 +++++++++++++++++++------
ovn/controller/lflow.h | 6 ++-
ovn/controller/lport.c | 2 +-
ovn/controller/lport.h | 1 +
ovn/controller/ovn-controller.c | 10 +++--
tests/ovn.at | 82 ++++++++++++++++++++++++++++++++++++-----
10 files changed, 189 insertions(+), 50 deletions(-)
diff --git a/ovn/controller/binding.c b/ovn/controller/binding.c
index f8e501d..a835145 100644
--- a/ovn/controller/binding.c
+++ b/ovn/controller/binding.c
@@ -433,20 +433,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, our_chassis);
}
@@ -498,7 +488,8 @@ binding_run(struct controller_ctx *ctx, const struct ovsrec_bridge *br_int,
const struct ldatapath_index *ldatapaths,
const struct lport_index *lports,
const struct chassis_index *chassis_index,
- struct hmap *local_datapaths, struct sset *local_lports)
+ struct hmap *local_datapaths, struct sset *local_lports,
+ struct sset *active_tunnels)
{
if (!chassis_rec) {
return;
@@ -507,13 +498,12 @@ binding_run(struct controller_ctx *ctx, const struct ovsrec_bridge *br_int,
const struct sbrec_port_binding *binding_rec;
struct shash lport_to_iface = SHASH_INITIALIZER(&lport_to_iface);
struct sset egress_ifaces = SSET_INITIALIZER(&egress_ifaces);
- struct sset active_tunnels = SSET_INITIALIZER(&active_tunnels);
struct hmap qos_map;
hmap_init(&qos_map);
if (br_int) {
get_local_iface_ids(br_int, &lport_to_iface, local_lports,
- &egress_ifaces, &active_tunnels);
+ &egress_ifaces, active_tunnels);
}
/* Run through each binding record to see if it is resident on this
@@ -524,7 +514,7 @@ binding_run(struct controller_ctx *ctx, const struct ovsrec_bridge *br_int,
chassis_rec, binding_rec,
sset_is_empty(&egress_ifaces) ? NULL :
&qos_map, local_datapaths, &lport_to_iface,
- local_lports, &active_tunnels);
+ local_lports, active_tunnels);
}
if (!sset_is_empty(&egress_ifaces)
@@ -537,7 +527,6 @@ binding_run(struct controller_ctx *ctx, const struct ovsrec_bridge *br_int,
shash_destroy(&lport_to_iface);
sset_destroy(&egress_ifaces);
- sset_destroy(&active_tunnels);
hmap_destroy(&qos_map);
}
diff --git a/ovn/controller/binding.h b/ovn/controller/binding.h
index f3a2f37..7d0fec6 100644
--- a/ovn/controller/binding.h
+++ b/ovn/controller/binding.h
@@ -33,7 +33,8 @@ void binding_register_ovs_idl(struct ovsdb_idl *);
void binding_run(struct controller_ctx *, const struct ovsrec_bridge *br_int,
const struct sbrec_chassis *, const struct ldatapath_index *,
const struct lport_index *, const struct chassis_index *,
- struct hmap *local_datapaths, struct sset *all_lports);
+ struct hmap *local_datapaths, struct sset *all_lports,
+ struct sset *active_tunnels);
void bfd_run(struct controller_ctx *ctx, const struct ovsrec_bridge *br_int,
const struct sbrec_chassis *chassis_rec,
struct hmap *local_datapaths, const struct chassis_index *);
diff --git a/ovn/controller/gchassis.c b/ovn/controller/gchassis.c
index 27d8f66..af99635 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,47 @@ 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) {
+ 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_short(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 46d5e42..5735aec 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 if the local chassis is active based on tunnel status
+ * and a gateway_chassis list. This will always be true for a single
+ * 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 8cc5e7e..f868e1f 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 a23cde0..484502f 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 906fda2..28f5d77 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 8937ecb..9a5a5da 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 c136715..1418a15 100644
--- a/ovn/controller/ovn-controller.c
+++ b/ovn/controller/ovn-controller.c
@@ -623,6 +623,7 @@ main(int argc, char *argv[])
* l2gateway ports for which options:l2gateway-chassis designates the
* local hypervisor, and localnet ports. */
struct sset local_lports = SSET_INITIALIZER(&local_lports);
+ struct sset active_tunnels = SSET_INITIALIZER(&active_tunnels);
const struct ovsrec_bridge *br_int = get_br_int(&ctx);
const char *chassis_id = get_chassis_id(ctx.ovs_idl);
@@ -642,9 +643,9 @@ main(int argc, char *argv[])
chassis = chassis_run(&ctx, chassis_id, br_int);
encaps_run(&ctx, br_int, chassis_id);
binding_run(&ctx, br_int, chassis, &ldatapaths, &lports,
- &chassis_index, &local_datapaths, &local_lports);
+ &chassis_index, &local_datapaths, &local_lports,
+ &active_tunnels);
}
-
if (br_int && chassis) {
struct shash addr_sets = SHASH_INITIALIZER(&addr_sets);
addr_sets_init(&ctx, &addr_sets);
@@ -663,8 +664,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);
bfd_run(&ctx, br_int, chassis, &local_datapaths,
&chassis_index);
@@ -720,6 +721,7 @@ main(int argc, char *argv[])
ldatapath_index_destroy(&ldatapaths);
sset_destroy(&local_lports);
+ sset_destroy(&active_tunnels);
struct local_datapath *cur_node, *next_node;
HMAP_FOR_EACH_SAFE (cur_node, next_node, hmap_node, &local_datapaths) {
diff --git a/tests/ovn.at b/tests/ovn.at
index ae17b01..398afee 100644
--- a/tests/ovn.at
+++ b/tests/ovn.at
@@ -7836,17 +7836,25 @@ for chassis in gw1 gw2 hv1 hv2; do
echo "------ $chassis dump ----------"
ovs-ofctl show br-int
ovs-ofctl dump-flows br-int
- echo ""
- echo "BFD (from $chassis):"
- # dump BFD config and status to the other chassis
- for chassis2 in gw1 gw2 hv1 hv2; do
- if [[ "$chassis" != "$chassis2" ]]; then
- echo " -> $chassis2:"
- echo " $(ovs-vsctl --bare --columns bfd,bfd_status find Interface name=ovn-$chassis2-0)"
- fi
- done
echo "--------------------------"
done
+function bfd_dump() {
+ for chassis in gw1 gw2 hv1 hv2; do
+ as $chassis
+ echo "------ $chassis dump (BFD)----"
+ echo "BFD (from $chassis):"
+ # dump BFD config and status to the other chassis
+ for chassis2 in gw1 gw2 hv1 hv2; do
+ if [[ "$chassis" != "$chassis2" ]]; then
+ echo " -> $chassis2:"
+ echo " $(ovs-vsctl --bare --columns bfd,bfd_status find Interface name=ovn-$chassis2-0)"
+ fi
+ done
+ echo "--------------------------"
+ done
+}
+
+bfd_dump
hv1_gw1_ofport=$(as hv1 ovs-vsctl --bare --columns ofport find Interface name=ovn-gw1-0)
hv1_gw2_ofport=$(as hv1 ovs-vsctl --bare --columns ofport find Interface name=ovn-gw2-0)
@@ -7864,13 +7872,36 @@ as hv1 ovs-ofctl dump-flows br-int table=32
echo "--- hv2 ---"
as hv2 ovs-ofctl dump-flows br-int table=32
+gw1_chassis=$(ovn-sbctl --bare --columns=_uuid find Chassis name=gw1)
+gw2_chassis=$(ovn-sbctl --bare --columns=_uuid find Chassis name=gw2)
+
AT_CHECK([as hv1 ovs-ofctl dump-flows br-int table=32 | grep active_backup | grep slaves:$hv1_gw1_ofport,$hv1_gw2_ofport | wc -l], [0], [1
])
AT_CHECK([as hv2 ovs-ofctl dump-flows br-int table=32 | grep active_backup | grep slaves:$hv2_gw1_ofport,$hv2_gw2_ofport | wc -l], [0], [1
])
-# set higher priority to gw2 instead of gw1, and check for changes
+# make sure that flows for handling the outside router port reside on gw1
+AT_CHECK([as gw1 ovs-ofctl dump-flows br-int table=23 | grep 00:00:02:01:02:04 | wc -l], [0], [[1
+]])
+AT_CHECK([as gw2 ovs-ofctl dump-flows br-int table=23 | grep 00:00:02:01:02:04 | wc -l], [0], [[0
+]])
+
+# make sure ARP responder flows for outside router port reside on gw1 too
+AT_CHECK([as gw1 ovs-ofctl dump-flows br-int table=9 | grep arp_tpa=192.168.0.101 | wc -l], [0], [[1
+]])
+AT_CHECK([as gw2 ovs-ofctl dump-flows br-int table=9 | grep arp_tpa=192.168.0.101 | wc -l], [0], [[0
+]])
+
+
+
+# check that the chassis redirect port has been claimed by the gw1 chassis
+AT_CHECK([ovn-sbctl --columns chassis --bare find Port_Binding logical_port=cr-outside | grep $gw1_chassis | wc -l],
+ [0],[[1
+]])
+
+
+# at this point, we invert the priority of the gw chassis between gw1 and gw2
ovn-nbctl --id=@gc0 create Gateway_Chassis \
name=outside_gw1 chassis_name=gw1 priority=10 -- \
@@ -7878,15 +7909,21 @@ ovn-nbctl --id=@gc0 create Gateway_Chassis \
name=outside_gw2 chassis_name=gw2 priority=20 -- \
set Logical_Router_Port outside 'gateway_chassis=[@gc0, at gc1]'
+
# XXX: Let the change propagate down to the ovn-controllers
sleep 2
+# we make sure that the hypervisors noticed, and inverted the slave ports
AT_CHECK([as hv1 ovs-ofctl dump-flows br-int table=32 | grep active_backup | grep slaves:$hv1_gw2_ofport,$hv1_gw1_ofport | wc -l], [0], [1
])
AT_CHECK([as hv2 ovs-ofctl dump-flows br-int table=32 | grep active_backup | grep slaves:$hv2_gw2_ofport,$hv2_gw1_ofport | wc -l], [0], [1
])
+# check that the chassis redirect port has been reclaimed by the gw2 chassis
+AT_CHECK([ovn-sbctl --columns chassis --bare find Port_Binding logical_port=cr-outside | grep $gw2_chassis | wc -l],
+ [0],[[1
+]])
# check BFD enablement on tunnel ports from gw1 #########
as gw1
@@ -7934,6 +7971,31 @@ AT_CHECK([ovs-vsctl --bare --columns bfd find Interface name=ovn-hv1-0],[0],
[[enable=false
]])
+# make sure that flows for handling the outside router port reside on gw2 now
+AT_CHECK([as gw2 ovs-ofctl dump-flows br-int table=23 | grep 00:00:02:01:02:04 | wc -l], [0], [[1
+]])
+AT_CHECK([as gw1 ovs-ofctl dump-flows br-int table=23 | grep 00:00:02:01:02:04 | wc -l], [0], [[0
+]])
+
+# disconnect GW2 from the network, GW1 should take over
+as gw2
+port=${sandbox}_br-phys
+as main ovs-vsctl del-port n1 $port
+sleep 4
+
+bfd_dump
+
+# make sure that flows for handling the outside router port reside on gw2 now
+AT_CHECK([as gw1 ovs-ofctl dump-flows br-int table=23 | grep 00:00:02:01:02:04 | wc -l], [0], [[1
+]])
+AT_CHECK([as gw2 ovs-ofctl dump-flows br-int table=23 | grep 00:00:02:01:02:04 | wc -l], [0], [[0
+]])
+
+# check that the chassis redirect port has been reclaimed by the gw1 chassis
+AT_CHECK([ovn-sbctl --columns chassis --bare find Port_Binding logical_port=cr-outside | grep $gw1_chassis | wc -l],
+ [0],[[1
+]])
+
OVN_CLEANUP([gw1],[gw2],[hv1],[hv2])
AT_CLEANUP
--
1.8.3.1
More information about the dev
mailing list