From 3c27dbe6b71e36c0d05e8299e811615697fc1bc0 Mon Sep 17 00:00:00 2001 From: Ryan Wilson Date: Thu, 29 May 2014 14:56:09 -0700 Subject: [PATCH] route-table: Make route-table module thread-safe. Due to patch fe83f8 (netdev: Remove netdev from global shash when the user is changing interface configuration), netdevs can be destructed and deallocated by revalidator and RCU threads. When netdevs with class vport are destroyed, the routing table is unregistered, possibly causing memory to be freed. This causes a race condition with the main thread which calls route_table_run periodically to check for routing table updates. This patch makes the route-table module thread-safe via a mutex. Bug #1258532 Signed-off-by: Ryan Wilson Acked-by: Ethan Jackson Signed-off-by: Alex Wang --- lib/route-table-stub.c | 7 ------- lib/route-table.c | 23 ++++++++++++++++++++++- lib/route-table.h | 1 - 3 files changed, 22 insertions(+), 9 deletions(-) diff --git a/lib/route-table-stub.c b/lib/route-table-stub.c index 9acc81c80..dab4fd2ee 100644 --- a/lib/route-table-stub.c +++ b/lib/route-table-stub.c @@ -24,13 +24,6 @@ route_table_get_name(ovs_be32 ip OVS_UNUSED, char name[IFNAMSIZ] OVS_UNUSED) return false; } -bool -route_table_get_ifindex(ovs_be32 ip OVS_UNUSED, int *ifindex) -{ - *ifindex = 0; - return false; -} - uint64_t route_table_get_change_seq(void) { diff --git a/lib/route-table.c b/lib/route-table.c index 2986d3d9a..9284924c5 100644 --- a/lib/route-table.c +++ b/lib/route-table.c @@ -63,6 +63,7 @@ struct name_node { char ifname[IFNAMSIZ]; /* Interface name. */ }; +static struct ovs_mutex route_table_mutex = OVS_MUTEX_INITIALIZER; static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(5, 20); /* Global change number for route-table, which should be incremented @@ -81,6 +82,8 @@ static struct hmap route_map; static struct hmap name_map; static int route_table_reset(void); +static bool route_table_get_ifindex(ovs_be32 ip, int *) + OVS_REQUIRES(route_table_mutex); static void route_table_handle_msg(const struct route_table_msg *); static bool route_table_parse(struct ofpbuf *, struct route_table_msg *); static void route_table_change(const struct route_table_msg *, void *); @@ -102,9 +105,12 @@ static struct name_node *name_node_lookup(int ifi_index); * Returns true if successful, otherwise false. */ bool route_table_get_name(ovs_be32 ip, char name[IFNAMSIZ]) + OVS_EXCLUDED(route_table_mutex) { int ifindex; + ovs_mutex_lock(&route_table_mutex); + if (!name_table_valid) { name_table_reset(); } @@ -115,10 +121,12 @@ route_table_get_name(ovs_be32 ip, char name[IFNAMSIZ]) nn = name_node_lookup(ifindex); if (nn) { ovs_strlcpy(name, nn->ifname, IFNAMSIZ); + ovs_mutex_unlock(&route_table_mutex); return true; } } + ovs_mutex_unlock(&route_table_mutex); return false; } @@ -128,8 +136,9 @@ route_table_get_name(ovs_be32 ip, char name[IFNAMSIZ]) * interface which is not physical (such as a bridge port). * * Returns true if successful, otherwise false. */ -bool +static bool route_table_get_ifindex(ovs_be32 ip_, int *ifindex) + OVS_REQUIRES(route_table_mutex) { struct route_node *rn; uint32_t ip = ntohl(ip_); @@ -168,7 +177,9 @@ route_table_get_change_seq(void) * function before making any other route_table function calls. */ void route_table_register(void) + OVS_EXCLUDED(route_table_mutex) { + ovs_mutex_lock(&route_table_mutex); if (!register_count) { ovs_assert(!nln); ovs_assert(!route_notifier); @@ -186,6 +197,7 @@ route_table_register(void) } register_count++; + ovs_mutex_unlock(&route_table_mutex); } /* Users of the route_table module should unregister themselves with this @@ -193,7 +205,9 @@ route_table_register(void) * calls. */ void route_table_unregister(void) + OVS_EXCLUDED(route_table_mutex) { + ovs_mutex_lock(&route_table_mutex); register_count--; if (!register_count) { @@ -206,12 +220,15 @@ route_table_unregister(void) hmap_destroy(&route_map); name_table_uninit(); } + ovs_mutex_unlock(&route_table_mutex); } /* Run periodically to update the locally maintained routing table. */ void route_table_run(void) + OVS_EXCLUDED(route_table_mutex) { + ovs_mutex_lock(&route_table_mutex); if (nln) { rtnetlink_link_run(); nln_run(nln); @@ -220,16 +237,20 @@ route_table_run(void) route_table_reset(); } } + ovs_mutex_unlock(&route_table_mutex); } /* Causes poll_block() to wake up when route_table updates are required. */ void route_table_wait(void) + OVS_EXCLUDED(route_table_mutex) { + ovs_mutex_lock(&route_table_mutex); if (nln) { rtnetlink_link_wait(); nln_wait(nln); } + ovs_mutex_unlock(&route_table_mutex); } static int diff --git a/lib/route-table.h b/lib/route-table.h index 2ed4b9143..2c5967e3d 100644 --- a/lib/route-table.h +++ b/lib/route-table.h @@ -25,7 +25,6 @@ #include "openvswitch/types.h" -bool route_table_get_ifindex(ovs_be32 ip, int *ifindex); bool route_table_get_name(ovs_be32 ip, char name[IFNAMSIZ]); uint64_t route_table_get_change_seq(void); void route_table_register(void); -- 2.20.1