We were removing routes while still iterating over them, while it might work it's not a good practice. Postpone the removal after the dump is done.
Reported-at: https://issues.redhat.com/browse/FDP-1594 Signed-off-by: Ales Musil <[email protected]> --- controller/route-exchange-netlink.c | 70 ++++++++++++++++++++--------- 1 file changed, 50 insertions(+), 20 deletions(-) diff --git a/controller/route-exchange-netlink.c b/controller/route-exchange-netlink.c index cec4b1ec3..91f059492 100644 --- a/controller/route-exchange-netlink.c +++ b/controller/route-exchange-netlink.c @@ -200,9 +200,9 @@ struct route_msg_handle_data { const struct sbrec_datapath_binding *db; struct hmapx *routes_to_advertise; struct vector *learned_routes; + struct vector *stale_routes; const struct hmap *routes; uint32_t table_id; /* requested table id. */ - int ret; }; static void @@ -211,7 +211,6 @@ handle_route_msg(const struct route_table_msg *msg, void *data) struct route_msg_handle_data *handle_data = data; const struct route_data *rd = &msg->rd; struct advertise_route_entry *ar; - int err; if (handle_data->table_id != rd->rta_table_id) { /* We do not have the NLM_F_DUMP_FILTERED info here, so check if the @@ -261,23 +260,37 @@ 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); - if (err) { - char addr_s[INET6_ADDRSTRLEN + 1]; - static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(5, 20); - VLOG_WARN_RL(&rl, "Delete route table_id=%"PRIu32" dst=%s plen=%d " - "failed: %s", rd->rta_table_id, - ipv6_string_mapped( - addr_s, &rd->rta_dst) ? addr_s : "(invalid)", - rd->rtm_dst_len, - ovs_strerror(err)); - - if (!handle_data->ret) { - /* Report the first error value to the caller. */ - handle_data->ret = err; + + if (handle_data->stale_routes) { + vector_push(handle_data->stale_routes, rd); + } +} + +static int +re_nl_delete_stale_routes(const struct vector *stale_routes) +{ + int ret = 0; + + const struct route_data *rd; + VECTOR_FOR_EACH_PTR (stale_routes, rd) { + int 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]; + static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(5, 20); + VLOG_WARN_RL(&rl, "Delete route table_id=%"PRIu32" dst=%s plen=%d " + "failed: %s", rd->rta_table_id, + ipv6_string_mapped( + addr_s, &rd->rta_dst) ? addr_s : "(invalid)", + rd->rtm_dst_len, + ovs_strerror(err)); + if (!ret) { + ret = err; + } } } + + return ret; } int @@ -286,8 +299,9 @@ re_nl_sync_routes(uint32_t table_id, const struct hmap *routes, const struct sbrec_datapath_binding *db) { struct hmapx routes_to_advertise = HMAPX_INITIALIZER(&routes_to_advertise); + struct vector stale_routes = VECTOR_EMPTY_INITIALIZER(struct route_data); struct advertise_route_entry *ar; - int ret; + int ret = 0; HMAP_FOR_EACH (ar, node, routes) { hmapx_add(&routes_to_advertise, ar); @@ -300,11 +314,11 @@ re_nl_sync_routes(uint32_t table_id, const struct hmap *routes, .routes = routes, .routes_to_advertise = &routes_to_advertise, .learned_routes = learned_routes, + .stale_routes = &stale_routes, .db = db, .table_id = table_id, }; 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. */ @@ -328,7 +342,14 @@ re_nl_sync_routes(uint32_t table_id, const struct hmap *routes, } } } + + int err = re_nl_delete_stale_routes(&stale_routes); + if (!ret) { + ret = err; + } + hmapx_destroy(&routes_to_advertise); + vector_destroy(&stale_routes); return ret; } @@ -336,15 +357,24 @@ re_nl_sync_routes(uint32_t table_id, const struct hmap *routes, int re_nl_cleanup_routes(uint32_t table_id) { + int ret = 0; + struct vector stale_routes = VECTOR_EMPTY_INITIALIZER(struct route_data); /* 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, + .stale_routes = &stale_routes, .table_id = table_id, }; route_table_dump_one_table(table_id, handle_route_msg, &data); - return data.ret; + int err = re_nl_delete_stale_routes(&stale_routes); + if (!ret) { + ret = err; + } + vector_destroy(&stale_routes); + + return ret; } -- 2.51.0 _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
