Commit (9cdc68a1 bfd: Add forwarding flag to struct bfd.) allows
several functions to call bfd_forwarding__() and update the
forwarding flag.  When forwarding_if_rx feature is enabled, this
introduces a race condition among threads calling these functions
and trying to update the flag.  And this may cause forwarding flag
flapping.

This commit fixes the above issue by limiting that only bfd_run()
and bfd_process_packet() can update the forwarding flag.  And when
forwarding_if_rx is enabled, only bfd_run() can update the forwarding
flag.  Other functions can only read the value of the forwarding flag.

Reported-by: Joe Stringer <joestrin...@nicira.com>
Signed-off-by: Alex Wang <al...@nicira.com>
---
 lib/bfd.c |   34 +++++++++++++++++++++++-----------
 1 file changed, 23 insertions(+), 11 deletions(-)

diff --git a/lib/bfd.c b/lib/bfd.c
index 4582f2e..2b2cad8 100644
--- a/lib/bfd.c
+++ b/lib/bfd.c
@@ -215,7 +215,7 @@ static struct ovs_mutex mutex = OVS_MUTEX_INITIALIZER;
 static struct hmap all_bfds__ = HMAP_INITIALIZER(&all_bfds__);
 static struct hmap *const all_bfds OVS_GUARDED_BY(mutex) = &all_bfds__;
 
-static bool bfd_forwarding__(struct bfd *) OVS_REQUIRES(mutex);
+static bool bfd_update_forwarding(struct bfd *) OVS_REQUIRES(mutex);
 static bool bfd_in_poll(const struct bfd *) OVS_REQUIRES(mutex);
 static void bfd_poll(struct bfd *bfd) OVS_REQUIRES(mutex);
 static const char *bfd_diag_str(enum diag) OVS_REQUIRES(mutex);
@@ -246,15 +246,16 @@ static void log_msg(enum vlog_level, const struct msg *, 
const char *message,
 
 static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(20, 20);
 
-/* Returns true if the interface on which 'bfd' is running may be used to
- * forward traffic according to the BFD session state. */
+/* Returns the forwarding flag value.  The flag is true if the interface on
+ * which 'bfd' is running may be used to forward traffic according to the BFD
+ * session state. */
 bool
 bfd_forwarding(struct bfd *bfd) OVS_EXCLUDED(mutex)
 {
     bool ret;
 
     ovs_mutex_lock(&mutex);
-    ret = bfd_forwarding__(bfd);
+    ret = bfd->forwarding;
     ovs_mutex_unlock(&mutex);
     return ret;
 }
@@ -507,6 +508,10 @@ bfd_run(struct bfd *bfd) OVS_EXCLUDED(mutex)
         || bfd->in_decay != old_in_decay) {
         bfd_poll(bfd);
     }
+
+    /* Updates the forwarding flag. */
+    bfd_update_forwarding(bfd);
+
     ovs_mutex_unlock(&mutex);
 }
 
@@ -786,6 +791,11 @@ bfd_process_packet(struct bfd *bfd, const struct flow 
*flow,
     }
     /* XXX: RFC 5880 Section 6.8.6 Demand mode related calculations here. */
 
+    /* Updates the forwarding flag value if not in demand mode. */
+    if (!bfd->forwarding_if_rx) {
+        bfd_update_forwarding(bfd);
+    }
+
 out:
     ovs_mutex_unlock(&mutex);
 }
@@ -812,9 +822,15 @@ bfd_set_netdev(struct bfd *bfd, const struct netdev 
*netdev)
 
 
 /* Updates the forwarding flag.  If override is not configured and
- * the forwarding flag value changes, increments the flap count. */
+ * the forwarding flag value changes, increments the flap count.
+ *
+ * When forwarding_if_rx is disabled, this function can be called
+ * by bfd_process_packet() and bfd_run().  When forwarding_if_rx
+ * is enabled, this function can only be called by bfd_run().
+ * This is to prevent the forwarding flag flapping due to race
+ * between calling threads. */
 static bool
-bfd_forwarding__(struct bfd *bfd) OVS_REQUIRES(mutex)
+bfd_update_forwarding(struct bfd *bfd) OVS_REQUIRES(mutex)
 {
     long long int time;
     bool forwarding_old = bfd->forwarding;
@@ -1033,9 +1049,6 @@ bfd_set_state(struct bfd *bfd, enum state state, enum 
diag diag)
             bfd_decay_update(bfd);
         }
     }
-
-    /* Updates the forwarding flag. */
-    bfd_forwarding__(bfd);
 }
 
 static uint64_t
@@ -1100,13 +1113,12 @@ bfd_check_rx(struct bfd *bfd) OVS_REQUIRES(mutex)
     }
 }
 
-/* Updates the forwarding_if_rx_detect_time and the forwarding flag. */
+/* Updates the forwarding_if_rx_detect_time. */
 static void
 bfd_forwarding_if_rx_update(struct bfd *bfd) OVS_REQUIRES(mutex)
 {
     int64_t incr = bfd_rx_interval(bfd) * bfd->mult;
     bfd->forwarding_if_rx_detect_time = MAX(incr, 2000) + time_msec();
-    bfd_forwarding__(bfd);
 }
 
 static uint32_t
-- 
1.7.9.5

_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Reply via email to