We are currently maintaining a state for _maintained_route_tables
hashmap and _maintained_vrfs sset between route_exchange_run()
consecutive runs. This approach does not take into account possible
netlink failures. As a result OVN and kernel state can be not aligned.
Fix the problem retrying the failed VRF or route update in the next
route_exchange_run() iteration.
Moreover report operation result in re_nl_sync_routes.
Fixes: faf4df563f1d ("controller: Announce routes via route-exchange.")
Fixes: b344a5341da8 ("controller: Cleanup routes on stop.")
Acked-By: Felix Huettner <felix.huettner@stackit.cloud>
Signed-off-by: Lorenzo Bianconi <lorenzo.bianc...@redhat.com>
---
controller/route-exchange-netlink.c | 18 ++++++++++++++++--
controller/route-exchange-netlink.h | 8 ++++----
controller/route-exchange-stub.c | 3 ++-
controller/route-exchange.c | 27 +++++++++++++++++++++------
controller/route-exchange.h | 4 ++--
5 files changed, 45 insertions(+), 15 deletions(-)
diff --git a/controller/route-exchange-netlink.c
b/controller/route-exchange-netlink.c
index 83deb13e6..7ee0d0829 100644
--- a/controller/route-exchange-netlink.c
+++ b/controller/route-exchange-netlink.c
@@ -194,6 +194,7 @@ struct route_msg_handle_data {
struct hmapx *routes_to_advertise;
struct vector *learned_routes;
const struct hmap *routes;
+ int ret;
};
static void
@@ -247,6 +248,9 @@ handle_route_msg(const struct route_table_msg *msg, void
*data)
}
err = re_nl_delete_route(rd->rta_table_id, &rd->rta_dst,
rd->rtm_dst_len, rd->rta_priority);
+ /* Report return value and number of deleted routes to the caller. */
+ handle_data->ret |= err;
+
if (err) {
char addr_s[INET6_ADDRSTRLEN + 1];
static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(5, 20);
@@ -259,13 +263,14 @@ handle_route_msg(const struct route_table_msg *msg, void
*data)
}
}
-void
+int
re_nl_sync_routes(uint32_t table_id, const struct hmap *routes,
struct vector *learned_routes,
const struct sbrec_datapath_binding *db)
{
struct hmapx routes_to_advertise = HMAPX_INITIALIZER(&routes_to_advertise);
struct advertise_route_entry *ar;
+ int ret;
HMAP_FOR_EACH (ar, node, routes) {
hmapx_add(&routes_to_advertise, ar);
@@ -281,6 +286,7 @@ re_nl_sync_routes(uint32_t table_id, const struct hmap
*routes,
.db = db,
};
route_table_dump_one_table(table_id, handle_route_msg, &data);
+ ret = data.ret;
/* Add any remaining routes in the routes_to_advertise hmapx to the
* system routing table. */
@@ -298,12 +304,18 @@ re_nl_sync_routes(uint32_t table_id, const struct hmap
*routes,
addr_s, &ar->addr) ? addr_s : "(invalid)",
ar->plen,
ovs_strerror(err));
+ if (err != EEXIST) {
+ ret = err;
+ break;
+ }
}
}
hmapx_destroy(&routes_to_advertise);
+
+ return ret;
}
-void
+int
re_nl_cleanup_routes(uint32_t table_id)
{
/* Remove routes from the system that are not in the host_routes hmap and
@@ -314,4 +326,6 @@ re_nl_cleanup_routes(uint32_t table_id)
.learned_routes = NULL,
};
route_table_dump_one_table(table_id, handle_route_msg, &data);
+
+ return data.ret;
}
diff --git a/controller/route-exchange-netlink.h
b/controller/route-exchange-netlink.h
index e37cfe711..e6142c1ff 100644
--- a/controller/route-exchange-netlink.h
+++ b/controller/route-exchange-netlink.h
@@ -51,10 +51,10 @@ int re_nl_delete_route(uint32_t table_id, const struct
in6_addr *dst,
void re_nl_dump(uint32_t table_id);
-void re_nl_sync_routes(uint32_t table_id, const struct hmap *routes,
- struct vector *learned_routes,
- const struct sbrec_datapath_binding *db);
+int re_nl_sync_routes(uint32_t table_id, const struct hmap *routes,
+ struct vector *learned_routes,
+ const struct sbrec_datapath_binding *db);
-void re_nl_cleanup_routes(uint32_t table_id);
+int re_nl_cleanup_routes(uint32_t table_id);
#endif /* route-exchange-netlink.h */
diff --git a/controller/route-exchange-stub.c b/controller/route-exchange-stub.c
index 27827df1d..5dff0c0fb 100644
--- a/controller/route-exchange-stub.c
+++ b/controller/route-exchange-stub.c
@@ -20,10 +20,11 @@
#include "openvswitch/compiler.h"
#include "route-exchange.h"
-void
+int
route_exchange_run(const struct route_exchange_ctx_in *r_ctx_in OVS_UNUSED,
struct route_exchange_ctx_out *r_ctx_out OVS_UNUSED)
{
+ return 0;
}
void
diff --git a/controller/route-exchange.c b/controller/route-exchange.c
index 79a046f60..f5b0a2527 100644
--- a/controller/route-exchange.c
+++ b/controller/route-exchange.c
@@ -204,7 +204,7 @@ sb_sync_learned_routes(const struct vector *learned_routes,
hmap_destroy(&sync_routes);
}
-void
+int
route_exchange_run(const struct route_exchange_ctx_in *r_ctx_in,
struct route_exchange_ctx_out *r_ctx_out)
{
@@ -213,6 +213,7 @@ route_exchange_run(const struct route_exchange_ctx_in
*r_ctx_in,
struct hmap old_maintained_route_table =
HMAP_INITIALIZER(&old_maintained_route_table);
hmap_swap(&_maintained_route_tables, &old_maintained_route_table);
+ int error, ret = 0;
const struct advertise_datapath_entry *ad;
HMAP_FOR_EACH (ad, node, r_ctx_in->announce_routes) {
@@ -220,13 +221,14 @@ route_exchange_run(const struct route_exchange_ctx_in
*r_ctx_in,
if (ad->maintain_vrf) {
if (!sset_contains(&old_maintained_vrfs, ad->vrf_name)) {
- int error = re_nl_create_vrf(ad->vrf_name, table_id);
+ error = re_nl_create_vrf(ad->vrf_name, table_id);
if (error && error != EEXIST) {
VLOG_WARN_RL(&rl,
"Unable to create VRF %s for datapath "
"%"PRIi32": %s.",
ad->vrf_name, table_id,
ovs_strerror(error));
+ ret = ret ? ret : error;
continue;
}
}
@@ -243,8 +245,9 @@ route_exchange_run(const struct route_exchange_ctx_in
*r_ctx_in,
struct vector received_routes =
VECTOR_EMPTY_INITIALIZER(struct re_nl_received_route_node);
- re_nl_sync_routes(ad->db->tunnel_key, &ad->routes,
- &received_routes, ad->db);
+ error = re_nl_sync_routes(ad->db->tunnel_key, &ad->routes,
+ &received_routes, ad->db);
+ ret = ret ? ret : error;
sb_sync_learned_routes(&received_routes, ad->db,
&ad->bound_ports, r_ctx_in->ovnsb_idl_txn,
@@ -261,7 +264,12 @@ route_exchange_run(const struct route_exchange_ctx_in
*r_ctx_in,
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);
+ error = re_nl_cleanup_routes(mrt->table_id);
+ if (error) {
+ /* If netlink transaction fails, we will retry next time. */
+ maintained_route_table_add(mrt->table_id);
+ ret = ret ? ret : error;
+ }
}
free(mrt);
}
@@ -271,11 +279,18 @@ route_exchange_run(const struct route_exchange_ctx_in
*r_ctx_in,
const char *vrf_name;
SSET_FOR_EACH_SAFE (vrf_name, &old_maintained_vrfs) {
if (!sset_contains(&_maintained_vrfs, vrf_name)) {
- re_nl_delete_vrf(vrf_name);
+ error = re_nl_delete_vrf(vrf_name);
+ if (error) {
+ /* If netlink transaction fails, we will retry next time. */
+ sset_add(&_maintained_vrfs, vrf_name);
+ ret = ret ? ret : error;
+ }
}
sset_delete(&old_maintained_vrfs, SSET_NODE_FROM_NAME(vrf_name));
}
sset_destroy(&old_maintained_vrfs);
+
+ return ret;
}
void
diff --git a/controller/route-exchange.h b/controller/route-exchange.h
index 0d6772df5..7adae0adb 100644
--- a/controller/route-exchange.h
+++ b/controller/route-exchange.h
@@ -33,8 +33,8 @@ struct route_exchange_ctx_out {
struct hmap route_table_watches;
};
-void route_exchange_run(const struct route_exchange_ctx_in *,
- struct route_exchange_ctx_out *);
+int route_exchange_run(const struct route_exchange_ctx_in *,
+ struct route_exchange_ctx_out *);
void route_exchange_cleanup_vrfs(void);
void route_exchange_destroy(void);
--
2.49.0
_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
