Skip to content

Commit

Permalink
controller: Cleanup routes on stop.
Browse files Browse the repository at this point in the history 
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.

Signed-off-by: Felix Huettner <[email protected]>
Signed-off-by: 0-day Robot <[email protected]>
  • Loading branch information
@felixhuettner @ovsrobot
felixhuettner authored and ovsrobot committed Dec 19, 2024
1 parent ecffc6f commit 106a16e
Show file tree
Hide file tree
Showing 4 changed files with 177 additions and 1 deletion.
40 changes: 40 additions & 0 deletions controller/route-exchange-netlink.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -291,3 +291,43 @@ re_nl_sync_routes(uint32_t table_id,
}
}
}

static void
handle_route_msg_delete_all_our_routes(const struct route_table_msg *msg,
void *data OVS_UNUSED)
{
const struct route_data *rd = &msg->rd;
int err;

/* This route is not from us, so not interesting. */
if (rd->rtm_protocol != RTPROT_OVN) {
return;
}

err = re_nl_delete_route(rd->rta_table_id, &rd->rta_dst,
rd->rtm_dst_len, rd->rta_priority);
if (err) {
char addr_s[INET6_ADDRSTRLEN + 1];
VLOG_WARN_RL(&rl, "Delete route table_id=%"PRIu32" dst=%s plen=%d: %s",
rd->rta_table_id,
ipv6_string_mapped(
addr_s, &rd->rta_dst) ? addr_s : "(invalid)",
rd->rtm_dst_len,
ovs_strerror(err));
}
}

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 = NULL,
.learned_routes = NULL,
};
route_table_dump_one_table(table_id,
handle_route_msg_delete_all_our_routes,
&data);
}
2 changes: 2 additions & 0 deletions controller/route-exchange-netlink.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -50,4 +50,6 @@ void re_nl_sync_routes(uint32_t table_id,
const struct hmap *host_routes,
struct hmap *learned_routes);

void re_nl_cleanup_routes(uint32_t table_id);

#endif /* route-exchange-netlink.h */
67 changes: 67 additions & 0 deletions controller/route-exchange.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@

#include <errno.h>
#include <net/if.h>
#include <stdbool.h>

#include "openvswitch/vlog.h"

Expand All @@ -33,6 +34,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 {
Expand All @@ -47,6 +55,38 @@ struct route_entry {
bool stale;
};

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 = xzalloc(sizeof(*mrt));
mrt->table_id = table_id;
hmap_insert(&_maintained_route_tables, &mrt->node, hash);
}

static struct route_entry *
route_alloc_entry(struct hmap *routes,
const struct sbrec_datapath_binding *sb_db,
Expand Down Expand Up @@ -187,6 +227,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) {
Expand Down Expand Up @@ -214,6 +257,7 @@ route_exchange_run(struct route_exchange_ctx_in *r_ctx_in,
sset_find_and_delete(&old_maintained_vrfs, vrf_name);
}

maintained_route_table_add(ad->key);
re_nl_sync_routes(ad->key, &ad->routes,
&received_routes);

Expand All @@ -232,6 +276,17 @@ route_exchange_run(struct route_exchange_ctx_in *r_ctx_in,
re_nl_received_routes_destroy(&received_routes);
}

/* Remove routes in tables previousl maintained by us. */
struct maintained_route_table_entry *mrt;
HMAP_FOR_EACH_SAFE (mrt, node, &old_maintained_route_table) {
if (!maintained_route_table_contains(mrt->table_id)) {
re_nl_cleanup_routes(mrt->table_id);
}
hmap_remove(&old_maintained_route_table, &mrt->node);
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) {
Expand All @@ -246,6 +301,11 @@ route_exchange_run(struct route_exchange_ctx_in *r_ctx_in,
void
route_exchange_cleanup(void)
{
struct maintained_route_table_entry *mrt;
HMAP_FOR_EACH_SAFE (mrt, node, &_maintained_route_tables) {
re_nl_cleanup_routes(mrt->table_id);
}

const char *vrf_name;
SSET_FOR_EACH_SAFE (vrf_name, &_maintained_vrfs) {
re_nl_delete_vrf(vrf_name);
Expand All @@ -255,10 +315,17 @@ route_exchange_cleanup(void)
void
route_exchange_destroy(void)
{
struct maintained_route_table_entry *mrt;
HMAP_FOR_EACH_SAFE (mrt, node, &_maintained_route_tables) {
hmap_remove(&_maintained_route_tables, &mrt->node);
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);
}
69 changes: 68 additions & 1 deletion tests/system-ovn.at
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14952,8 +14952,40 @@ check ovn-nbctl --wait=hv set Logical_Router_Port internet-phys \
options:dynamic-routing-ifname=lo
check_row_count Learned_Route 1


# stopping the ovn-controller will clean up the route entries created by it
# we first need to unset maintain-vrf as otherwise it will delete the whole vrf
check ovn-nbctl --wait=hv set Logical_Router_Port internet-phys \
options: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
])

# starting it again will add the routes again
# the 2 sync commands ensure that we wait until the routes are actually
# installed. Otherwise this is racy
start_daemon ovn-controller
OVS_WAIT_UNTIL([test "$(ovn-appctl -t ovn-controller debug/status)" == "running"])
check ovn-nbctl --wait=hv sync
check ovn-nbctl --wait=hv sync
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])

# 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])

as ovn-sb
OVS_APP_EXIT_AND_WAIT([ovsdb-server])
Expand Down Expand Up @@ -15201,6 +15233,41 @@ check ovn-nbctl --wait=hv set Logical_Router_Port internet-phys \
options:dynamic-routing-ifname=lo
check_row_count Learned_Route 1

# stopping the ovn-controller will clean up the route entries created by it
# we first need to unset maintain-vrf as otherwise it will delete the whole vrf
check ovn-nbctl --wait=hv set Logical_Router_Port internet-phys \
options: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
])

# starting it again will add the routes again
# the 2 sync commands ensure that we wait until the routes are actually
# installed. Otherwise this is racy
start_daemon ovn-controller
OVS_WAIT_UNTIL([test "$(ovn-appctl -t ovn-controller debug/status)" == "running"])
check ovn-nbctl --wait=hv sync
check ovn-nbctl --wait=hv sync
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])

# 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])

as ovn-sb
OVS_APP_EXIT_AND_WAIT([ovsdb-server])

Expand Down

0 comments on commit 106a16e

Please sign in to comment.