When we stop ovn-controller without immediately restarting it we now
cleanup routes.
This allows the routing agents to stop advertising this chassis to the
fabric.
Acked-by: Dumitru Ceara <dce...@redhat.com>
Signed-off-by: Felix Huettner <felix.huettner@stackit.cloud>
---
v5->v6:
* addressed review comments
* fixed testcase to cleanup all vrfs
v3->v4:
- addressed review comments.
controller/route-exchange-netlink.c | 32 ++++++++---
controller/route-exchange-netlink.h | 2 +
controller/route-exchange.c | 67 ++++++++++++++++++++++-
tests/system-ovn.at | 84 +++++++++++++++++++++++++++++
4 files changed, 177 insertions(+), 8 deletions(-)
diff --git a/controller/route-exchange-netlink.c
b/controller/route-exchange-netlink.c
index 0cb07e39f..ad0c25989 100644
--- a/controller/route-exchange-netlink.c
+++ b/controller/route-exchange-netlink.c
@@ -214,6 +214,9 @@ handle_route_msg(const struct route_table_msg *msg, void
*data)
/* This route is not from us, so we learn it. */
if (rd->rtm_protocol != RTPROT_OVN) {
+ if (!handle_data->learned_routes) {
+ return;
+ }
if (prefix_is_link_local(&rd->rta_dst, rd->rtm_dst_len)) {
return;
}
@@ -236,13 +239,15 @@ handle_route_msg(const struct route_table_msg *msg, void
*data)
return;
}
- uint32_t hash = advertise_route_hash(&rd->rta_dst, rd->rtm_dst_len);
- HMAP_FOR_EACH_WITH_HASH (ar, node, hash, handle_data->routes) {
- if (ipv6_addr_equals(&ar->addr, &rd->rta_dst)
- && ar->plen == rd->rtm_dst_len
- && ar->priority == rd->rta_priority) {
- hmapx_find_and_delete(handle_data->routes_to_advertise, ar);
- return;
+ if (handle_data->routes_to_advertise) {
+ uint32_t hash = advertise_route_hash(&rd->rta_dst, rd->rtm_dst_len);
+ HMAP_FOR_EACH_WITH_HASH (ar, node, hash, handle_data->routes) {
+ if (ipv6_addr_equals(&ar->addr, &rd->rta_dst)
+ && ar->plen == rd->rtm_dst_len
+ && ar->priority == rd->rta_priority) {
+ hmapx_find_and_delete(handle_data->routes_to_advertise, ar);
+ return;
+ }
}
}
err = re_nl_delete_route(rd->rta_table_id, &rd->rta_dst,
@@ -302,3 +307,16 @@ re_nl_sync_routes(uint32_t table_id, const struct hmap
*routes,
}
hmapx_destroy(&routes_to_advertise);
}
+
+void
+re_nl_cleanup_routes(uint32_t table_id)
+{
+ /* Remove routes from the system that are not in the host_routes hmap and
+ * remove entries from host_routes hmap that match routes already installed
+ * in the system. */
+ struct route_msg_handle_data data = {
+ .routes_to_advertise = NULL,
+ .learned_routes = NULL,
+ };
+ route_table_dump_one_table(table_id, handle_route_msg, &data);
+}
diff --git a/controller/route-exchange-netlink.h
b/controller/route-exchange-netlink.h
index 08102edcf..1eadde491 100644
--- a/controller/route-exchange-netlink.h
+++ b/controller/route-exchange-netlink.h
@@ -57,4 +57,6 @@ void re_nl_sync_routes(uint32_t table_id, const struct hmap
*routes,
struct ovs_list *learned_routes,
const struct sbrec_datapath_binding *db);
+void re_nl_cleanup_routes(uint32_t table_id);
+
#endif /* route-exchange-netlink.h */
diff --git a/controller/route-exchange.c b/controller/route-exchange.c
index 8f740fa91..3c49c60c0 100644
--- a/controller/route-exchange.c
+++ b/controller/route-exchange.c
@@ -19,6 +19,7 @@
#include <errno.h>
#include <net/if.h>
+#include <stdbool.h>
#include "openvswitch/vlog.h"
#include "openvswitch/list.h"
@@ -36,6 +37,13 @@
VLOG_DEFINE_THIS_MODULE(route_exchange);
static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(5, 20);
+struct maintained_route_table_entry {
+ struct hmap_node node;
+ uint32_t table_id;
+};
+
+static struct hmap _maintained_route_tables =
+ HMAP_INITIALIZER(&_maintained_route_tables);
static struct sset _maintained_vrfs = SSET_INITIALIZER(&_maintained_vrfs);
struct route_entry {
@@ -44,6 +52,38 @@ struct route_entry {
const struct sbrec_learned_route *sb_route;
};
+static uint32_t
+maintained_route_table_hash(uint32_t table_id)
+{
+ return hash_int(table_id, 0);
+}
+
+static bool
+maintained_route_table_contains(uint32_t table_id)
+{
+ uint32_t hash = maintained_route_table_hash(table_id);
+ struct maintained_route_table_entry *mrt;
+ HMAP_FOR_EACH_WITH_HASH (mrt, node, hash,
+ &_maintained_route_tables) {
+ if (mrt->table_id == table_id) {
+ return true;
+ }
+ }
+ return false;
+}
+
+static void
+maintained_route_table_add(uint32_t table_id)
+{
+ if (maintained_route_table_contains(table_id)) {
+ return;
+ }
+ uint32_t hash = maintained_route_table_hash(table_id);
+ struct maintained_route_table_entry *mrt = xmalloc(sizeof *mrt);
+ mrt->table_id = table_id;
+ hmap_insert(&_maintained_route_tables, &mrt->node, hash);
+}
+
static void
route_add_entry(struct hmap *routes,
const struct sbrec_learned_route *sb_route)
@@ -171,6 +211,9 @@ route_exchange_run(struct route_exchange_ctx_in *r_ctx_in,
{
struct sset old_maintained_vrfs = SSET_INITIALIZER(&old_maintained_vrfs);
sset_swap(&_maintained_vrfs, &old_maintained_vrfs);
+ struct hmap old_maintained_route_table =
+ HMAP_INITIALIZER(&old_maintained_route_table);
+ hmap_swap(&_maintained_route_tables, &old_maintained_route_table);
const struct advertise_datapath_entry *ad;
HMAP_FOR_EACH (ad, node, r_ctx_in->announce_routes) {
@@ -198,9 +241,10 @@ route_exchange_run(struct route_exchange_ctx_in *r_ctx_in,
sset_find_and_delete(&old_maintained_vrfs, vrf_name);
}
+ maintained_route_table_add(table_id);
+
struct ovs_list received_routes = OVS_LIST_INITIALIZER(
&received_routes);
-
re_nl_sync_routes(ad->db->tunnel_key, &ad->routes,
&received_routes, ad->db);
@@ -215,6 +259,16 @@ route_exchange_run(struct route_exchange_ctx_in *r_ctx_in,
re_nl_learned_routes_destroy(&received_routes);
}
+ /* Remove routes in tables previously maintained by us. */
+ struct maintained_route_table_entry *mrt;
+ HMAP_FOR_EACH_POP (mrt, node, &old_maintained_route_table) {
+ if (!maintained_route_table_contains(mrt->table_id)) {
+ re_nl_cleanup_routes(mrt->table_id);
+ }
+ free(mrt);
+ }
+ hmap_destroy(&old_maintained_route_table);
+
/* Remove VRFs previously maintained by us not found in the above loop. */
const char *vrf_name;
SSET_FOR_EACH_SAFE (vrf_name, &old_maintained_vrfs) {
@@ -229,6 +283,11 @@ route_exchange_run(struct route_exchange_ctx_in *r_ctx_in,
void
route_exchange_cleanup_vrfs(void)
{
+ struct maintained_route_table_entry *mrt;
+ HMAP_FOR_EACH (mrt, node, &_maintained_route_tables) {
+ re_nl_cleanup_routes(mrt->table_id);
+ }
+
const char *vrf_name;
SSET_FOR_EACH (vrf_name, &_maintained_vrfs) {
re_nl_delete_vrf(vrf_name);
@@ -238,10 +297,16 @@ route_exchange_cleanup_vrfs(void)
void
route_exchange_destroy(void)
{
+ struct maintained_route_table_entry *mrt;
+ HMAP_FOR_EACH_POP (mrt, node, &_maintained_route_tables) {
+ free(mrt);
+ }
+
const char *vrf_name;
SSET_FOR_EACH_SAFE (vrf_name, &_maintained_vrfs) {
sset_delete(&_maintained_vrfs, SSET_NODE_FROM_NAME(vrf_name));
}
sset_destroy(&_maintained_vrfs);
+ hmap_destroy(&_maintained_route_tables);
}
diff --git a/tests/system-ovn.at b/tests/system-ovn.at
index 10fbcc8e5..62fd77cdb 100644
--- a/tests/system-ovn.at
+++ b/tests/system-ovn.at
@@ -16172,7 +16172,49 @@ check ip route add 233.253.0.0/24 via 192.168.20.20
dev hv1-mll onlink vrf ovnvr
check ovn-nbctl --wait=hv sync
check_row_count Learned_Route 1 ip_prefix=233.253.0.0/24 nexthop=192.168.20.20
+# Stopping the ovn-controller will clean up the route entries created by it.
+# We first need to unset dynamic-routing-maintain-vrf as otherwise it will
+# delete the whole vrf.
+check ovn-nbctl --wait=hv set Logical_Router_Port internet-phys \
+ options:dynamic-routing-maintain-vrf=false
OVS_APP_EXIT_AND_WAIT([ovn-controller])
+AT_CHECK([ip route list vrf ovnvrf1337 | awk '{$1=$1};1'], [0], [dnl
+233.252.0.0/24 via 192.168.10.10 dev lo onlink
+233.253.0.0/24 via 192.168.20.20 dev hv1-mll onlink
+])
+
+# Starting it again will add the routes again.
+start_daemon ovn-controller
+OVS_WAIT_UNTIL([test "$(ovn-appctl -t ovn-controller debug/status)" ==
"running"])
+OVS_WAIT_UNTIL_EQUAL([ip route list vrf ovnvrf1337 | awk '{$1=$1};1'], [dnl
+blackhole 192.0.2.1 proto 84 metric 1000
+blackhole 192.0.2.2 proto 84 metric 100
+blackhole 192.0.2.3 proto 84 metric 100
+blackhole 192.0.2.10 proto 84 metric 100
+blackhole 198.51.100.0/24 proto 84 metric 1000
+233.252.0.0/24 via 192.168.10.10 dev lo onlink
+233.253.0.0/24 via 192.168.20.20 dev hv1-mll onlink])
+
+# Stoping with --restart will not touch the routes.
+check ovn-appctl -t ovn-controller exit --restart
+OVS_WAIT_UNTIL([test "$(ovn-appctl -t ovn-controller debug/status)" !=
"running"])
+OVS_WAIT_UNTIL_EQUAL([ip route list vrf ovnvrf1337 | awk '{$1=$1};1'], [dnl
+blackhole 192.0.2.1 proto 84 metric 1000
+blackhole 192.0.2.2 proto 84 metric 100
+blackhole 192.0.2.3 proto 84 metric 100
+blackhole 192.0.2.10 proto 84 metric 100
+blackhole 198.51.100.0/24 proto 84 metric 1000
+233.252.0.0/24 via 192.168.10.10 dev lo onlink
+233.253.0.0/24 via 192.168.20.20 dev hv1-mll onlink])
+
+# Now we set maintain-vrf again and stop the ovn-controller.
+# It will then remove the VRF.
+start_daemon ovn-controller
+OVS_WAIT_UNTIL([test "$(ovn-appctl -t ovn-controller debug/status)" ==
"running"])
+check ovn-nbctl --wait=hv set Logical_Router_Port internet-phys \
+ options:dynamic-routing-maintain-vrf=true
+OVS_APP_EXIT_AND_WAIT([ovn-controller])
+AT_CHECK([ip vrf | grep -q ovnvrf1337], [1], [])
as ovn-sb
OVS_APP_EXIT_AND_WAIT([ovsdb-server])
@@ -16446,7 +16488,49 @@ check ip route add 233.253.0.0/24 via 192.168.20.20
dev hv1-mll onlink vrf ovnvr
check ovn-nbctl --wait=hv sync
check_row_count Learned_Route 1 ip_prefix=233.253.0.0/24 nexthop=192.168.20.20
+# Stopping the ovn-controller will clean up the route entries created by it.
+# We first need to unset dynamic-routing-maintain-vrf as otherwise it will
+# delete the whole vrf
+check ovn-nbctl --wait=hv set Logical_Router_Port internet-phys \
+ options:dynamic-routing-maintain-vrf=false
OVS_APP_EXIT_AND_WAIT([ovn-controller])
+AT_CHECK([ip route list vrf ovnvrf1337 | awk '{$1=$1};1'], [0], [dnl
+233.252.0.0/24 via 192.168.10.10 dev lo onlink
+233.253.0.0/24 via 192.168.20.20 dev hv1-mll onlink
+])
+
+# Starting it again will add the routes again.
+start_daemon ovn-controller
+OVS_WAIT_UNTIL([test "$(ovn-appctl -t ovn-controller debug/status)" ==
"running"])
+OVS_WAIT_UNTIL_EQUAL([ip route list vrf ovnvrf1337 | awk '{$1=$1};1'], [dnl
+blackhole 192.0.2.1 proto 84 metric 100
+blackhole 192.0.2.2 proto 84 metric 100
+blackhole 192.0.2.3 proto 84 metric 100
+blackhole 192.0.2.10 proto 84 metric 100
+blackhole 198.51.100.0/24 proto 84 metric 1000
+233.252.0.0/24 via 192.168.10.10 dev lo onlink
+233.253.0.0/24 via 192.168.20.20 dev hv1-mll onlink])
+
+# Stoping with --restart will not touch the routes.
+check ovn-appctl -t ovn-controller exit --restart
+OVS_WAIT_UNTIL([test "$(ovn-appctl -t ovn-controller debug/status)" !=
"running"])
+OVS_WAIT_UNTIL_EQUAL([ip route list vrf ovnvrf1337 | awk '{$1=$1};1'], [dnl
+blackhole 192.0.2.1 proto 84 metric 100
+blackhole 192.0.2.2 proto 84 metric 100
+blackhole 192.0.2.3 proto 84 metric 100
+blackhole 192.0.2.10 proto 84 metric 100
+blackhole 198.51.100.0/24 proto 84 metric 1000
+233.252.0.0/24 via 192.168.10.10 dev lo onlink
+233.253.0.0/24 via 192.168.20.20 dev hv1-mll onlink])
+
+# Now we set maintain-vrf again and stop the ovn-controller.
+# It will then remove the VRF.
+start_daemon ovn-controller
+OVS_WAIT_UNTIL([test "$(ovn-appctl -t ovn-controller debug/status)" ==
"running"])
+check ovn-nbctl --wait=hv set Logical_Router_Port internet-phys \
+ options:dynamic-routing-maintain-vrf=true
+OVS_APP_EXIT_AND_WAIT([ovn-controller])
+AT_CHECK([ip vrf | grep -q ovnvrf1337], [1], [])
as ovn-sb
OVS_APP_EXIT_AND_WAIT([ovsdb-server])
--
2.47.1
_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
