It's kinda weird that route_table_get_ifindex() is a globally
accessible function which requires a mutex which is internal to the
module. I think the answer is to remove it from the header file and
make it static.
Other than that looks fine.
Acked-by: Ethan Jackson <et...@nicira.com>
On Thu, May 29, 2014 at 12:31 PM, Ryan Wilson <wr...@nicira.com> wrote:
> 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 <wr...@nicira.com>
> ---
> lib/route-table.c | 19 +++++++++++++++++++
> 1 file changed, 19 insertions(+)
>
> diff --git a/lib/route-table.c b/lib/route-table.c
> index 2986d3d..19bd414 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
> @@ -102,9 +103,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 +119,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;
> }
>
> @@ -130,6 +136,7 @@ route_table_get_name(ovs_be32 ip, char name[IFNAMSIZ])
> * Returns true if successful, otherwise false. */
> 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 +175,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 +195,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 +203,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 +218,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 +235,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
> --
> 1.7.9.5
>
> _______________________________________________
> dev mailing list
> dev@openvswitch.org
> http://openvswitch.org/mailman/listinfo/dev
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
