Problem Statement: OVS flushes and subsequently repopulates its route cache whenever it receives a netlink notification about kernel interface change. At the same time the port addition triggers a revalidation of all datapath flow cache entries. The revalidation of egress tunnel flows depends on correct routing information and can fail during the rebuild of the route cache, which leads to temporary insertion of drop flows.
Solution: There is already a route_table_mutex that OVS seizes when it rebuilds the route cache. To ensure that flow revalidation cannot collide with route cache rebuild, seize that route_table_mutex also during revalidation. Since revalidation is multi-threaded, we seize the lock only on the leader thread. As a revalidation run can last hundreds of milliseconds, replace the lock() with trylock() in the route table code to avoid blocking the main OVS thread during revalidation. Signed-off-by: Cheekatamarla Eswara Venkata Pavan Kumar <[email protected]> --- lib/route-table.c | 34 ++++++++++++++++++++++++++++------ lib/route-table.h | 2 ++ ofproto/ofproto-dpif-upcall.c | 14 ++++++++++---- 3 files changed, 40 insertions(+), 10 deletions(-) diff --git a/lib/route-table.c b/lib/route-table.c index ac82cf262..e1b17f1b8 100644 --- a/lib/route-table.c +++ b/lib/route-table.c @@ -88,6 +88,21 @@ static void route_map_clear(void); static void name_table_init(void); static void name_table_change(const struct rtnetlink_change *, void *); +inline void route_table_lock(void) +{ + ovs_mutex_lock(&route_table_mutex); +} + +inline void route_table_unlock(void) +{ + ovs_mutex_unlock(&route_table_mutex); +} + +inline int route_table_trylock(void) +{ + return ovs_mutex_trylock(&route_table_mutex); +} + uint64_t route_table_get_change_seq(void) { @@ -100,7 +115,7 @@ void route_table_init(void) OVS_EXCLUDED(route_table_mutex) { - ovs_mutex_lock(&route_table_mutex); + route_table_lock(); ovs_assert(!nln); ovs_assert(!route_notifier); ovs_assert(!route6_notifier); @@ -119,7 +134,7 @@ route_table_init(void) route_table_reset(); name_table_init(); - ovs_mutex_unlock(&route_table_mutex); + route_table_unlock(); } /* Run periodically to update the locally maintained routing table. */ @@ -127,7 +142,11 @@ void route_table_run(void) OVS_EXCLUDED(route_table_mutex) { - ovs_mutex_lock(&route_table_mutex); + /* Skip route table updates when the table is locked. */ + if (route_table_trylock()) { + return; + } + if (nln) { rtnetlink_run(); nln_run(nln); @@ -136,7 +155,7 @@ route_table_run(void) route_table_reset(); } } - ovs_mutex_unlock(&route_table_mutex); + route_table_unlock(); } /* Causes poll_block() to wake up when route_table updates are required. */ @@ -144,12 +163,15 @@ void route_table_wait(void) OVS_EXCLUDED(route_table_mutex) { - ovs_mutex_lock(&route_table_mutex); + if (route_table_trylock()) { + return; + } + if (nln) { rtnetlink_wait(); nln_wait(nln); } - ovs_mutex_unlock(&route_table_mutex); + route_table_unlock(); } static int diff --git a/lib/route-table.h b/lib/route-table.h index 3a02d737a..58418bd3f 100644 --- a/lib/route-table.h +++ b/lib/route-table.h @@ -33,4 +33,6 @@ void route_table_wait(void); bool route_table_fallback_lookup(const struct in6_addr *ip6_dst, char name[], struct in6_addr *gw6); +void route_table_lock(void); +void route_table_unlock(void); #endif /* route-table.h */ diff --git a/ofproto/ofproto-dpif-upcall.c b/ofproto/ofproto-dpif-upcall.c index 1c9c720f0..ab2d20833 100644 --- a/ofproto/ofproto-dpif-upcall.c +++ b/ofproto/ofproto-dpif-upcall.c @@ -23,6 +23,7 @@ #include "coverage.h" #include "cmap.h" #include "lib/dpif-provider.h" +#include "lib/route-table.h" #include "dpif.h" #include "openvswitch/dynamic-string.h" #include "fail-open.h" @@ -587,7 +588,8 @@ udpif_start_threads(struct udpif *udpif, uint32_t n_handlers_, dpif_enable_upcall(udpif->dpif); ovs_barrier_init(&udpif->reval_barrier, udpif->n_revalidators); - ovs_barrier_init(&udpif->pause_barrier, udpif->n_revalidators + 1); + /* For main thread and leader*/ + ovs_barrier_init(&udpif->pause_barrier, 2); udpif->reval_exit = false; udpif->pause = false; udpif->offload_rebalance_time = time_msec(); @@ -953,6 +955,10 @@ udpif_revalidator(void *arg) * on the pause_barrier */ udpif->pause = latch_is_set(&udpif->pause_latch); + if (udpif->pause) { + revalidator_pause(revalidator); + } + /* Only the leader checks the exit latch to prevent a race where * some threads think it's true and exit and others think it's * false and block indefinitely on the reval_barrier */ @@ -965,14 +971,13 @@ udpif_revalidator(void *arg) terse_dump = udpif_use_ufid(udpif); udpif->dump = dpif_flow_dump_create(udpif->dpif, terse_dump, NULL); + /* Prevent reset of the routing table during revalidation */ + route_table_lock(); } } /* Wait for the leader to start the flow dump. */ ovs_barrier_block(&udpif->reval_barrier); - if (udpif->pause) { - revalidator_pause(revalidator); - } if (udpif->reval_exit) { break; @@ -990,6 +995,7 @@ udpif_revalidator(void *arg) unsigned int flow_limit; long long int duration; + route_table_unlock(); atomic_read_relaxed(&udpif->flow_limit, &flow_limit); dpif_flow_dump_destroy(udpif->dump); -- 2.25.1 _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
