[ovs-dev,3/3] rconn: Increase precision of timers.

Message ID 20190301235448.3168-3-blp@ovn.org
State New
Headers show
Series
  • [ovs-dev,1/3] sat-math: Add functions for saturating arithmetic on "long long int".
Related show

Commit Message

Ben Pfaff March 1, 2019, 11:54 p.m.
Until now, the rconn timers have been precise only to the nearest second.
This increases them to millisecond precision, which seems cleaner these
days.

Signed-off-by: Ben Pfaff <blp@ovn.org>
---
 include/openvswitch/rconn.h |   7 +-
 lib/rconn.c                 | 138 ++++++++++++++++++------------------
 ofproto/connmgr.c           |  16 ++---
 3 files changed, 80 insertions(+), 81 deletions(-)

Comments

0-day Robot March 2, 2019, 1:04 a.m. | #1
Bleep bloop.  Greetings Ben Pfaff, I am a robot and I have tried out your patch.
Thanks for your contribution.

I encountered some error that I wasn't expecting.  See the details below.


checkpatch:
WARNING: Line is 82 characters long (recommended limit is 79)
#456 FILE: ofproto/connmgr.c:2:
 * Copyright (c) 2009, 2010, 2011, 2012, 2013, 2014, 2015, 2016, 2019 Nicira, Inc.

Lines checked: 495, Warnings: 1, Errors: 0


Please check this out.  If you feel there has been an error, please email aconole@bytheb.org

Thanks,
0-day Robot

Patch

diff --git a/include/openvswitch/rconn.h b/include/openvswitch/rconn.h
index fd60a6ce1dea..25c18f97e405 100644
--- a/include/openvswitch/rconn.h
+++ b/include/openvswitch/rconn.h
@@ -1,5 +1,5 @@ 
 /*
- * Copyright (c) 2008, 2009, 2010, 2011, 2012, 2013, 2015 Nicira, Inc.
+ * Copyright (c) 2008, 2009, 2010, 2011, 2012, 2013, 2015, 2019 Nicira, Inc.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -19,7 +19,6 @@ 
 
 #include <stdbool.h>
 #include <stdint.h>
-#include <time.h>
 #include "openvswitch/types.h"
 
 /* A wrapper around vconn that provides queuing and optionally reliability.
@@ -88,8 +87,8 @@  int rconn_failure_duration(const struct rconn *);
 int rconn_get_version(const struct rconn *);
 
 const char *rconn_get_state(const struct rconn *);
-time_t rconn_get_last_connection(const struct rconn *);
-time_t rconn_get_last_disconnect(const struct rconn *);
+long long int rconn_get_last_connection(const struct rconn *);
+long long int rconn_get_last_disconnect(const struct rconn *);
 unsigned int rconn_get_connection_seqno(const struct rconn *);
 int rconn_get_last_error(const struct rconn *);
 unsigned int rconn_count_txqlen(const struct rconn *);
diff --git a/lib/rconn.c b/lib/rconn.c
index ba95bb3a1a61..9fab33e2e59a 100644
--- a/lib/rconn.c
+++ b/lib/rconn.c
@@ -84,13 +84,16 @@  state_name(enum state state)
 }
 
 /* A reliable connection to an OpenFlow switch or controller.
+ *
+ * Members of type 'long long int' are times in milliseconds on the monotonic
+ * clock, as returned by time_msec().  Other times are durations in seconds.
  *
  * See the large comment in rconn.h for more information. */
 struct rconn {
     struct ovs_mutex mutex;
 
     enum state state;
-    time_t state_entered;
+    long long int state_entered;
 
     struct vconn *vconn;
     char *name;                 /* Human-readable descriptive name. */
@@ -99,11 +102,11 @@  struct rconn {
 
     struct ovs_list txq;        /* Contains "struct ofpbuf"s. */
 
-    int backoff;
-    int max_backoff;
-    time_t backoff_deadline;
-    time_t last_connected;
-    time_t last_disconnected;
+    int backoff;                     /* Current backoff, in seconds. */
+    int max_backoff;                 /* Limit for backoff, In seconds. */
+    long long int backoff_deadline;
+    long long int last_connected;
+    long long int last_disconnected;
     unsigned int seqno;
     int last_error;
 
@@ -116,7 +119,7 @@  struct rconn {
      * last_admitted reports the last time we believe such a positive admission
      * control decision was made. */
     bool probably_admitted;
-    time_t last_admitted;
+    long long int last_admitted; /* Milliseconds on monotonic clock. */
 
     /* Throughout this file, "probe" is shorthand for "inactivity probe".  When
      * no activity has been observed from the peer for a while, we send out an
@@ -126,7 +129,7 @@  struct rconn {
      * "Activity" is defined as either receiving an OpenFlow message from the
      * peer or successfully sending a message that had been in 'txq'. */
     int probe_interval;         /* Secs of inactivity before sending probe. */
-    time_t last_activity;       /* Last time we saw some activity. */
+    long long int last_activity;       /* Last time we saw some activity. */
 
     uint8_t dscp;
 
@@ -152,9 +155,9 @@  uint32_t rconn_get_allowed_versions(const struct rconn *rconn)
     return rconn->allowed_versions;
 }
 
-static unsigned int elapsed_in_this_state(const struct rconn *rc)
+static long long int elapsed_in_this_state(const struct rconn *rc)
     OVS_REQUIRES(rc->mutex);
-static unsigned int timeout(const struct rconn *rc) OVS_REQUIRES(rc->mutex);
+static long long int timeout(const struct rconn *rc) OVS_REQUIRES(rc->mutex);
 static bool timed_out(const struct rconn *rc) OVS_REQUIRES(rc->mutex);
 static void state_transition(struct rconn *rc, enum state)
     OVS_REQUIRES(rc->mutex);
@@ -247,7 +250,7 @@  rconn_create(int probe_interval, int max_backoff, uint8_t dscp,
     ovs_mutex_init(&rc->mutex);
 
     rc->state = S_VOID;
-    rc->state_entered = time_now();
+    rc->state_entered = time_msec();
 
     rc->vconn = NULL;
     rc->name = xstrdup("void");
@@ -258,15 +261,15 @@  rconn_create(int probe_interval, int max_backoff, uint8_t dscp,
 
     rc->backoff = 0;
     rc->max_backoff = max_backoff ? max_backoff : 8;
-    rc->backoff_deadline = TIME_MIN;
-    rc->last_connected = TIME_MIN;
-    rc->last_disconnected = TIME_MIN;
+    rc->backoff_deadline = LLONG_MIN;
+    rc->last_connected = LLONG_MIN;
+    rc->last_disconnected = LLONG_MIN;
     rc->seqno = 0;
 
     rc->probably_admitted = false;
-    rc->last_admitted = time_now();
+    rc->last_admitted = time_msec();
 
-    rc->last_activity = time_now();
+    rc->last_activity = time_msec();
 
     rconn_set_probe_interval(rc, probe_interval);
     rconn_set_dscp(rc, dscp);
@@ -287,8 +290,11 @@  rconn_set_max_backoff(struct rconn *rc, int max_backoff)
     rc->max_backoff = MAX(1, max_backoff);
     if (rc->state == S_BACKOFF && rc->backoff > max_backoff) {
         rc->backoff = max_backoff;
-        if (rc->backoff_deadline > time_now() + max_backoff) {
-            rc->backoff_deadline = time_now() + max_backoff;
+
+        long long int max_deadline = llsat_add(time_msec(),
+                                               llsat_mul(1000, max_backoff));
+        if (rc->backoff_deadline > max_deadline) {
+            rc->backoff_deadline = max_deadline;
         }
     }
     ovs_mutex_unlock(&rc->mutex);
@@ -396,7 +402,7 @@  rconn_disconnect__(struct rconn *rc)
         rc->reliable = false;
 
         rc->backoff = 0;
-        rc->backoff_deadline = TIME_MIN;
+        rc->backoff_deadline = LLONG_MIN;
 
         state_transition(rc, S_VOID);
     }
@@ -434,11 +440,11 @@  rconn_destroy(struct rconn *rc)
     }
 }
 
-static unsigned int
+static long long int
 timeout_VOID(const struct rconn *rc OVS_UNUSED)
     OVS_REQUIRES(rc->mutex)
 {
-    return UINT_MAX;
+    return LLONG_MAX;
 }
 
 static void
@@ -460,21 +466,22 @@  reconnect(struct rconn *rc)
     retval = vconn_open(rc->target, rc->allowed_versions, rc->dscp,
                         &rc->vconn);
     if (!retval) {
-        rc->backoff_deadline = time_now() + rc->backoff;
+        rc->backoff_deadline = llsat_add(time_msec(),
+                                         llsat_mul(1000, rc->backoff));
         state_transition(rc, S_CONNECTING);
     } else {
         VLOG_WARN("%s: connection failed (%s)",
                   rc->name, ovs_strerror(retval));
-        rc->backoff_deadline = TIME_MAX; /* Prevent resetting backoff. */
+        rc->backoff_deadline = LLONG_MAX; /* Prevent resetting backoff. */
         disconnect(rc, retval);
     }
 }
 
-static unsigned int
+static long long int
 timeout_BACKOFF(const struct rconn *rc)
     OVS_REQUIRES(rc->mutex)
 {
-    return rc->backoff;
+    return llsat_mul(1000, rc->backoff);
 }
 
 static void
@@ -486,11 +493,11 @@  run_BACKOFF(struct rconn *rc)
     }
 }
 
-static unsigned int
+static long long int
 timeout_CONNECTING(const struct rconn *rc)
     OVS_REQUIRES(rc->mutex)
 {
-    return MAX(2, rc->backoff);
+    return llsat_mul(1000, MAX(1, rc->backoff));
 }
 
 static void
@@ -513,7 +520,7 @@  run_CONNECTING(struct rconn *rc)
         if (rconn_logging_connection_attempts__(rc)) {
             VLOG_INFO("%s: connection timed out", rc->name);
         }
-        rc->backoff_deadline = TIME_MAX; /* Prevent resetting backoff. */
+        rc->backoff_deadline = LLONG_MAX; /* Prevent resetting backoff. */
         disconnect(rc, ETIMEDOUT);
     }
 }
@@ -530,23 +537,23 @@  do_tx_work(struct rconn *rc)
         if (error) {
             break;
         }
-        rc->last_activity = time_now();
+        rc->last_activity = time_msec();
     }
     if (ovs_list_is_empty(&rc->txq)) {
         poll_immediate_wake();
     }
 }
 
-static unsigned int
+static long long int
 timeout_ACTIVE(const struct rconn *rc)
     OVS_REQUIRES(rc->mutex)
 {
     if (rc->probe_interval) {
-        unsigned int base = MAX(rc->last_activity, rc->state_entered);
-        unsigned int arg = base + rc->probe_interval - rc->state_entered;
-        return arg;
+        long long int base = MAX(rc->last_activity, rc->state_entered);
+        long long int probe = llsat_mul(rc->probe_interval, 1000);
+        return llsat_sub(llsat_add(base, probe), rc->state_entered);
     }
-    return UINT_MAX;
+    return LLONG_MAX;
 }
 
 static void
@@ -554,9 +561,9 @@  run_ACTIVE(struct rconn *rc)
     OVS_REQUIRES(rc->mutex)
 {
     if (timed_out(rc)) {
-        unsigned int base = MAX(rc->last_activity, rc->state_entered);
-        VLOG_DBG("%s: idle %u seconds, sending inactivity probe",
-                 rc->name, (unsigned int) (time_now() - base));
+        long long int base = MAX(rc->last_activity, rc->state_entered);
+        VLOG_DBG("%s: idle %lld seconds, sending inactivity probe",
+                 rc->name, (time_msec() - base) / 1000);
 
         /* Ordering is important here: rconn_send() can transition to BACKOFF,
          * and we don't want to transition back to IDLE if so, because then we
@@ -572,11 +579,11 @@  run_ACTIVE(struct rconn *rc)
     do_tx_work(rc);
 }
 
-static unsigned int
+static long long int
 timeout_IDLE(const struct rconn *rc)
     OVS_REQUIRES(rc->mutex)
 {
-    return rc->probe_interval;
+    return llsat_mul(rc->probe_interval, 1000);
 }
 
 static void
@@ -584,20 +591,20 @@  run_IDLE(struct rconn *rc)
     OVS_REQUIRES(rc->mutex)
 {
     if (timed_out(rc)) {
-        VLOG_ERR("%s: no response to inactivity probe after %u "
+        VLOG_ERR("%s: no response to inactivity probe after %lld "
                  "seconds, disconnecting",
-                 rc->name, elapsed_in_this_state(rc));
+                 rc->name, elapsed_in_this_state(rc) / 1000);
         disconnect(rc, ETIMEDOUT);
     } else {
         do_tx_work(rc);
     }
 }
 
-static unsigned int
+static long long int
 timeout_DISCONNECTED(const struct rconn *rc OVS_UNUSED)
     OVS_REQUIRES(rc->mutex)
 {
-    return UINT_MAX;
+    return LLONG_MAX;
 }
 
 static void
@@ -665,9 +672,6 @@  void
 rconn_run_wait(struct rconn *rc)
     OVS_EXCLUDED(rc->mutex)
 {
-    unsigned int timeo;
-    size_t i;
-
     ovs_mutex_lock(&rc->mutex);
     if (rc->vconn) {
         vconn_run_wait(rc->vconn);
@@ -675,16 +679,12 @@  rconn_run_wait(struct rconn *rc)
             vconn_wait(rc->vconn, WAIT_SEND);
         }
     }
-    for (i = 0; i < rc->n_monitors; i++) {
+    for (size_t i = 0; i < rc->n_monitors; i++) {
         vconn_run_wait(rc->monitors[i]);
         vconn_recv_wait(rc->monitors[i]);
     }
 
-    timeo = timeout(rc);
-    if (timeo != UINT_MAX) {
-        long long int expires = sat_add(rc->state_entered, timeo);
-        poll_timer_wait_until(expires * 1000);
-    }
+    poll_timer_wait_until(llsat_add(rc->state_entered, timeout(rc)));
     ovs_mutex_unlock(&rc->mutex);
 }
 
@@ -703,11 +703,11 @@  rconn_recv(struct rconn *rc)
         if (!error) {
             copy_to_monitor(rc, buffer);
             if (rc->probably_admitted || is_admitted_msg(buffer)
-                || time_now() - rc->last_connected >= 30) {
+                || time_msec() - rc->last_connected >= 30 * 1000) {
                 rc->probably_admitted = true;
-                rc->last_admitted = time_now();
+                rc->last_admitted = time_msec();
             }
-            rc->last_activity = time_now();
+            rc->last_activity = time_msec();
             if (rc->state == S_IDLE) {
                 state_transition(rc, S_ACTIVE);
             }
@@ -927,7 +927,7 @@  rconn_failure_duration(const struct rconn *rconn)
     ovs_mutex_lock(&rconn->mutex);
     duration = (rconn_is_admitted__(rconn)
                 ? 0
-                : time_now() - rconn->last_admitted);
+                : (time_msec() - rconn->last_admitted) / 1000);
     ovs_mutex_unlock(&rconn->mutex);
 
     return duration;
@@ -960,16 +960,16 @@  rconn_get_state(const struct rconn *rc)
 }
 
 /* Returns the time at which the last successful connection was made by
- * 'rc'. Returns TIME_MIN if never connected. */
-time_t
+ * 'rc'. Returns LLONG_MIN if never connected. */
+long long int
 rconn_get_last_connection(const struct rconn *rc)
 {
     return rc->last_connected;
 }
 
-/* Returns the time at which 'rc' was last disconnected. Returns TIME_MIN
+/* Returns the time at which 'rc' was last disconnected. Returns LLONG_MIN
  * if never disconnected. */
-time_t
+long long int
 rconn_get_last_disconnect(const struct rconn *rc)
 {
     return rc->last_disconnected;
@@ -1187,9 +1187,9 @@  disconnect(struct rconn *rc, int error)
         vconn_close(rc->vconn);
         rc->vconn = NULL;
     }
-    if (rc->reliable) {
-        time_t now = time_now();
 
+    long long int now = time_msec();
+    if (rc->reliable) {
         if (rc->state & (S_CONNECTING | S_ACTIVE | S_IDLE)) {
             rc->last_disconnected = now;
             flush_queue(rc);
@@ -1209,10 +1209,10 @@  disconnect(struct rconn *rc, int error)
             }
             rc->backoff = rc->max_backoff;
         }
-        rc->backoff_deadline = now + rc->backoff;
+        rc->backoff_deadline = llsat_add(now, llsat_mul(1000, rc->backoff));
         state_transition(rc, S_BACKOFF);
     } else {
-        rc->last_disconnected = time_now();
+        rc->last_disconnected = now;
         state_transition(rc, S_DISCONNECTED);
     }
 }
@@ -1238,14 +1238,14 @@  flush_queue(struct rconn *rc)
     poll_immediate_wake();
 }
 
-static unsigned int
+static long long int
 elapsed_in_this_state(const struct rconn *rc)
     OVS_REQUIRES(rc->mutex)
 {
-    return time_now() - rc->state_entered;
+    return time_msec() - rc->state_entered;
 }
 
-static unsigned int
+static long long int
 timeout(const struct rconn *rc)
     OVS_REQUIRES(rc->mutex)
 {
@@ -1262,7 +1262,7 @@  static bool
 timed_out(const struct rconn *rc)
     OVS_REQUIRES(rc->mutex)
 {
-    return time_now() >= sat_add(rc->state_entered, timeout(rc));
+    return time_msec() >= llsat_add(rc->state_entered, timeout(rc));
 }
 
 static void
@@ -1275,7 +1275,7 @@  state_transition(struct rconn *rc, enum state state)
     }
     VLOG_DBG("%s: entering %s", rc->name, state_name(state));
     rc->state = state;
-    rc->state_entered = time_now();
+    rc->state_entered = time_msec();
 }
 
 static void
diff --git a/ofproto/connmgr.c b/ofproto/connmgr.c
index 80dedd849b52..d975a532bf2d 100644
--- a/ofproto/connmgr.c
+++ b/ofproto/connmgr.c
@@ -1,5 +1,5 @@ 
 /*
- * Copyright (c) 2009, 2010, 2011, 2012, 2013, 2014, 2015, 2016 Nicira, Inc.
+ * Copyright (c) 2009, 2010, 2011, 2012, 2013, 2014, 2015, 2016, 2019 Nicira, Inc.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -501,9 +501,9 @@  connmgr_get_controller_info(struct connmgr *mgr, struct shash *info)
         if (!shash_find(info, target)) {
             struct ofconn *ofconn = ofservice_first_conn(ofservice);
             struct ofproto_controller_info *cinfo = xmalloc(sizeof *cinfo);
-            time_t now = time_now();
-            time_t last_connection = rconn_get_last_connection(rconn);
-            time_t last_disconnect = rconn_get_last_disconnect(rconn);
+            long long int now = time_msec();
+            long long int last_connection = rconn_get_last_connection(rconn);
+            long long int last_disconnect = rconn_get_last_disconnect(rconn);
             int last_error = rconn_get_last_error(rconn);
             int i;
 
@@ -520,14 +520,14 @@  connmgr_get_controller_info(struct connmgr *mgr, struct shash *info)
 
             smap_add(&cinfo->pairs, "state", rconn_get_state(rconn));
 
-            if (last_connection != TIME_MIN) {
+            if (last_connection != LLONG_MIN) {
                 smap_add_format(&cinfo->pairs, "sec_since_connect",
-                                "%ld", (long int) (now - last_connection));
+                                "%lld", (now - last_connection) / 1000);
             }
 
-            if (last_disconnect != TIME_MIN) {
+            if (last_disconnect != LLONG_MIN) {
                 smap_add_format(&cinfo->pairs, "sec_since_disconnect",
-                                "%ld", (long int) (now - last_disconnect));
+                                "%lld", (now - last_disconnect) / 1000);
             }
 
             for (i = 0; i < N_SCHEDULERS; i++) {