[ovs-dev] [PATCH] ofproto: Store time since last connect and disconnect in Controller table.

Andrew Evans aevans at nicira.com
Mon Mar 14 19:26:25 UTC 2011


On Mar 14, 2011, at 10:33 AM, Ben Pfaff wrote:
> In disconnect() you can use the value of 'now' that has already been
> initialized two lines up instead of calling time_now() again.

Changed, thanks. Must have had my blinders on.

> In the two instances of expressions like
> 	xasprintf("%ld", time_now() - rconn_get_last_connection(rconn))
> I would advise adding a cast to "long int" since there is no guarantee
> that time_t is compatible with long int.  It is probably int or long
> int, but it might be long long int or even short, and it's better not
> to guess.

Good point. Just curious: what is the reason for using "long int" rather than just "long"?

> As with the other patch I'm surprised to see an empty value in place
> of an omitted key-value pair.

Ok, I've changed that. Now those keys are set only if they have valid values.

Here is an incremental diff:

---cut here---
diff --git a/lib/rconn.c b/lib/rconn.c
index 03dbd76..6a86de6 100644
--- a/lib/rconn.c
+++ b/lib/rconn.c
@@ -990,7 +990,7 @@ disconnect(struct rconn *rc, int error)
         time_t now = time_now();
 
         if (rc->state & (S_CONNECTING | S_ACTIVE | S_IDLE)) {
-            rc->last_disconnected = time_now();
+            rc->last_disconnected = now;
             vconn_close(rc->vconn);
             rc->vconn = NULL;
             flush_queue(rc);
diff --git a/ofproto/ofproto.c b/ofproto/ofproto.c
index 89277ad..147ade3 100644
--- a/ofproto/ofproto.c
+++ b/ofproto/ofproto.c
@@ -1376,6 +1376,9 @@ ofproto_get_ofproto_controller_info(const struct ofproto *ofproto,
 
     HMAP_FOR_EACH (ofconn, hmap_node, &ofproto->controllers) {
         const struct rconn *rconn = ofconn->rconn;
+        time_t now = time_now();
+        time_t last_connection = rconn_get_last_connection(rconn);
+        time_t last_disconnect = rconn_get_last_disconnect(rconn);
         const int last_error = rconn_get_last_error(rconn);
         struct ofproto_controller_info *cinfo = xmalloc(sizeof *cinfo);
 
@@ -1396,15 +1399,17 @@ ofproto_get_ofproto_controller_info(const struct ofproto *ofproto,
         cinfo->pairs.values[cinfo->pairs.n++] =
             xstrdup(rconn_get_state(rconn));
 
-        cinfo->pairs.keys[cinfo->pairs.n] = "sec_since_connect";
-        cinfo->pairs.values[cinfo->pairs.n++] =
-            rconn_get_last_connection(rconn) == TIME_MIN ? xstrdup("")
-            : xasprintf("%ld", time_now() - rconn_get_last_connection(rconn));
+        if (last_connection != TIME_MIN) {
+            cinfo->pairs.keys[cinfo->pairs.n] = "sec_since_connect";
+            cinfo->pairs.values[cinfo->pairs.n++]
+                = xasprintf("%ld", (long int) (now - last_connection));
+        }
 
-        cinfo->pairs.keys[cinfo->pairs.n] = "sec_since_disconnect";
-        cinfo->pairs.values[cinfo->pairs.n++] =
-            rconn_get_last_disconnect(rconn) == TIME_MIN ? xstrdup("")
-            : xasprintf("%ld", time_now() - rconn_get_last_disconnect(rconn));
+        if (last_disconnect != TIME_MIN) {
+            cinfo->pairs.keys[cinfo->pairs.n] = "sec_since_disconnect";
+            cinfo->pairs.values[cinfo->pairs.n++]
+                = xasprintf("%ld", (long int) (now - last_disconnect));
+        }
     }
 }
 

---cut here---




More information about the dev mailing list