cfm: Fix broken fault logic.

[sliver-openvswitch.git] / lib / cfm.c

diff --git a/lib/cfm.c b/lib/cfm.c
index 4e5117d..a504714 100644 (file)
--- a/lib/cfm.c
+++ b/lib/cfm.c
@@ -28,6 +28,7 @@
 #include "ofpbuf.h"
 #include "packets.h"
 #include "poll-loop.h"
+#include "timer.h"
 #include "timeval.h"
 #include "vlog.h"
 
@@ -42,8 +43,8 @@ struct cfm_internal {
     uint8_t ccm_interval;  /* The CCM transmission interval. */
     int ccm_interval_ms;   /* 'ccm_interval' in milliseconds. */
 
-    long long ccm_sent;    /* The time we last sent a CCM. */
-    long long fault_check; /* The time we last checked for faults. */
+    struct timer tx_timer;    /* Send CCM when expired. */
+    struct timer fault_timer; /* Check for faults when expired. */
 
     long long x_recv_time;
 };
@@ -66,6 +67,20 @@ ccm_interval_to_ms(uint8_t interval)
     NOT_REACHED();
 }
 
+static long long int
+cfm_fault_interval(struct cfm_internal *cfmi)
+{
+    /* According to the 802.1ag specification we should assume every other MP
+     * with the same MAID has the same transmission interval that we have.  If
+     * an MP has a different interval, cfm_process_heartbeat will register it
+     * as a fault (likely due to a configuration error).  Thus we can check all
+     * MPs at once making this quite a bit simpler.
+     *
+     * According to the specification we should check when (ccm_interval_ms *
+     * 3.5)ms have passed. */
+    return (cfmi->ccm_interval_ms * 7) / 2;
+}
+
 static uint8_t
 ms_to_ccm_interval(int interval_ms)
 {
@@ -155,30 +170,28 @@ cfm_run(struct cfm *cfm)
 {
     long long now = time_msec();
     struct cfm_internal *cfmi = cfm_to_internal(cfm);
-    long long fault_interval;
 
-    /* According to the 802.1ag specification we should assume every other MP
-     * with the same MAID has the same transmission interval that we have.  If
-     * an MP has a different interval, cfm_process_heartbeat will register it
-     * as a fault (likely due to a configuration error).  Thus we can check all
-     * MPs at once making this quite a bit simpler.
-     *
-     * According to the specification we should check when (ccm_interval_ms *
-     * 3.5)ms have passed. */
-    fault_interval = (cfmi->ccm_interval_ms * 7) / 2;
-    if (now >= cfmi->fault_check + fault_interval) {
+    if (timer_expired(&cfmi->fault_timer)) {
         bool fault;
         struct remote_mp *rmp;
+        long long int interval;
 
-        fault = now < cfmi->x_recv_time + fault_interval;
+        interval = cfm_fault_interval(cfmi);
+        fault = now < cfmi->x_recv_time + interval;
 
         HMAP_FOR_EACH (rmp, node, &cfm->remote_mps) {
-            rmp->fault = rmp->fault || cfmi->fault_check > rmp->recv_time;
-            fault      = rmp->fault || fault;
+            if (rmp->recv_time < timer_enabled_at(&cfmi->fault_timer, interval)
+                || timer_expired_at(&cfmi->fault_timer, rmp->recv_time)) {
+                rmp->fault = true;
+            }
+
+            if (rmp->fault) {
+                fault = true;
+            }
         }
 
-        cfm->fault        = fault;
-        cfmi->fault_check = now;
+        cfm->fault = fault;
+        timer_set_duration(&cfmi->fault_timer, interval);
     }
 }
 
@@ -189,7 +202,7 @@ cfm_should_send_ccm(struct cfm *cfm)
 {
     struct cfm_internal *cfmi = cfm_to_internal(cfm);
 
-    return time_msec() >= cfmi->ccm_sent + cfmi->ccm_interval_ms;
+    return timer_expired(&cfmi->tx_timer);
 }
 
 /* Composes a CCM message into 'ccm'.  Messages generated with this function
@@ -199,7 +212,7 @@ cfm_compose_ccm(struct cfm *cfm, struct ccm *ccm)
 {
     struct cfm_internal *cfmi = cfm_to_internal(cfm);
 
-    cfmi->ccm_sent = time_msec();
+    timer_set_duration(&cfmi->tx_timer, cfmi->ccm_interval_ms);
 
     ccm->mdlevel_version = 0;
     ccm->opcode = CCM_OPCODE;
@@ -213,12 +226,10 @@ cfm_compose_ccm(struct cfm *cfm, struct ccm *ccm)
 void
 cfm_wait(struct cfm *cfm)
 {
-    long long wait;
     struct cfm_internal *cfmi = cfm_to_internal(cfm);
 
-    wait = MIN(cfmi->ccm_sent + cfmi->ccm_interval_ms,
-               cfmi->fault_check + cfmi->ccm_interval_ms * 4);
-    poll_timer_wait_until(wait);
+    timer_wait(&cfmi->tx_timer);
+    timer_wait(&cfmi->fault_timer);
 }
 
 /* Should be called whenever a client of the cfm library changes the internals
@@ -226,19 +237,23 @@ cfm_wait(struct cfm *cfm)
 bool
 cfm_configure(struct cfm *cfm)
 {
-    struct cfm_internal *cfmi;
+    struct cfm_internal *cfmi = cfm_to_internal(cfm);
+    uint8_t interval;
 
     if (!cfm_is_valid_mpid(cfm->mpid) || !cfm->interval) {
         return false;
     }
 
-    cfmi                  = cfm_to_internal(cfm);
-    cfmi->ccm_interval    = ms_to_ccm_interval(cfm->interval);
-    cfmi->ccm_interval_ms = ccm_interval_to_ms(cfmi->ccm_interval);
+    interval = ms_to_ccm_interval(cfm->interval);
+
+    if (interval != cfmi->ccm_interval) {
+        cfmi->ccm_interval = interval;
+        cfmi->ccm_interval_ms = ccm_interval_to_ms(interval);
+
+        timer_set_expired(&cfmi->tx_timer);
+        timer_set_duration(&cfmi->fault_timer, cfm_fault_interval(cfmi));
+    }
 
-    /* Force a resend and check in case anything changed. */
-    cfmi->ccm_sent    = 0;
-    cfmi->fault_check = 0;
     return true;
 }
 
@@ -398,11 +413,15 @@ cfm_dump_ds(const struct cfm *cfm, struct ds *ds)
                   cfm->fault ? "fault" : "");
 
     ds_put_format(ds, "\tinterval: %dms\n", cfmi->ccm_interval_ms);
-    ds_put_format(ds, "\ttime since CCM tx: %lldms\n", now - cfmi->ccm_sent);
-    ds_put_format(ds, "\ttime since bad CCM rx: %lldms\n",
-                  now - cfmi->x_recv_time);
-    ds_put_format(ds, "\ttime since fault check: %lldms\n",
-                  now - cfmi->fault_check);
+    ds_put_format(ds, "\tnext CCM tx: %lldms\n",
+                  timer_msecs_until_expired(&cfmi->tx_timer));
+    ds_put_format(ds, "\tnext fault check: %lldms\n",
+                  timer_msecs_until_expired(&cfmi->fault_timer));
+
+    if (cfmi->x_recv_time != LLONG_MIN) {
+        ds_put_format(ds, "\ttime since bad CCM rx: %lldms\n",
+                      now - cfmi->x_recv_time);
+    }
 
     ds_put_cstr(ds, "\n");
     HMAP_FOR_EACH (rmp, node, &cfm->remote_mps) {