Ethan Jackson | 1 Apr 2011 01:14

[PATCH] cfm: Fix appctl negative report.

When the cfm module has never received a bad CCM message, it would
report a negative time.
---
 lib/cfm.c |    7 +++++--
 1 files changed, 5 insertions(+), 2 deletions(-)

diff --git a/lib/cfm.c b/lib/cfm.c
index 4e5117d..fc7486e 100644
--- a/lib/cfm.c
+++ b/lib/cfm.c
 <at>  <at>  -399,11 +399,14  <at>  <at>  cfm_dump_ds(const struct cfm *cfm, struct ds *ds)

     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);

+    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) {
         ds_put_format(ds, "Remote MPID %"PRIu16": %s\n", rmp->mpid,
--

-- 
1.7.4.2

(Continue reading)

Ben Pfaff | 1 Apr 2011 01:16

Re: [PATCH] cfm: Fix appctl negative report.

On Thu, Mar 31, 2011 at 04:14:44PM -0700, Ethan Jackson wrote:
> When the cfm module has never received a bad CCM message, it would
> report a negative time.

Looks good, thank you.
Ethan Jackson | 1 Apr 2011 01:17

Re: [PATCH] cfm: Fix appctl negative report.

I noticed this while testing the timer changes. It was introduced back
when I implemented the extra cfm debugging stuff.

On Thu, Mar 31, 2011 at 4:14 PM, Ethan Jackson <ethan@...> wrote:
> When the cfm module has never received a bad CCM message, it would
> report a negative time.
> ---
>  lib/cfm.c |    7 +++++--
>  1 files changed, 5 insertions(+), 2 deletions(-)
>
> diff --git a/lib/cfm.c b/lib/cfm.c
> index 4e5117d..fc7486e 100644
> --- a/lib/cfm.c
> +++ b/lib/cfm.c
>  <at>  <at>  -399,11 +399,14  <at>  <at>  cfm_dump_ds(const struct cfm *cfm, struct ds *ds)
>
>     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);
>
> +    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) {
(Continue reading)

Ethan Jackson | 1 Apr 2011 01:22

Re: [timer 4/4] ofproto: Use new timer library.

Thanks for the reviews. I merged this series.

On Thu, Mar 31, 2011 at 3:46 PM, Ben Pfaff <blp@...> wrote:
> This looks good, thanks.
>
Ethan Jackson | 1 Apr 2011 01:27

[dead 1/3] netdev-linux: Remove dead assignments.

---
 lib/netdev-linux.c |    6 ++----
 1 files changed, 2 insertions(+), 4 deletions(-)

diff --git a/lib/netdev-linux.c b/lib/netdev-linux.c
index 02d8a4d..010f48c 100644
--- a/lib/netdev-linux.c
+++ b/lib/netdev-linux.c
 <at>  <at>  -2600,12 +2600,11  <at>  <at>  htb_tc_load(struct netdev *netdev, struct ofpbuf *nlmsg OVS_UNUSED)
     struct ofpbuf msg;
     struct nl_dump dump;
     struct htb_class hc;
-    struct htb *htb;

     /* Get qdisc options. */
     hc.max_rate = 0;
     htb_query_class__(netdev, tc_make_handle(1, 0xfffe), 0, &hc, NULL);
-    htb = htb_install__(netdev, hc.max_rate);
+    htb_install__(netdev, hc.max_rate);

     /* Get queues. */
     if (!start_queue_dump(netdev, &dump)) {
 <at>  <at>  -3100,13 +3099,12  <at>  <at>  static int
 hfsc_tc_load(struct netdev *netdev, struct ofpbuf *nlmsg OVS_UNUSED)
 {
     struct ofpbuf msg;
-    struct hfsc *hfsc;
     struct nl_dump dump;
     struct hfsc_class hc;

(Continue reading)

Ethan Jackson | 1 Apr 2011 01:27

[dead 3/3] learning-switch: Remove dead assignment.

---
 lib/learning-switch.c |    2 --
 1 files changed, 0 insertions(+), 2 deletions(-)

diff --git a/lib/learning-switch.c b/lib/learning-switch.c
index 97aaf9f..dc9af77 100644
--- a/lib/learning-switch.c
+++ b/lib/learning-switch.c
 <at>  <at>  -440,7 +440,6  <at>  <at>  process_packet_in(struct lswitch *sw, struct rconn *rconn,
     /* Send the packet, and possibly the whole flow, to the output port. */
     if (sw->max_idle >= 0 && (!sw->ml || out_port != OFPP_FLOOD)) {
         struct ofpbuf *buffer;
-        struct ofp_flow_mod *ofm;
         struct cls_rule rule;

         /* The output port is known, or we always flood everything, so add a
 <at>  <at>  -449,7 +448,6  <at>  <at>  process_packet_in(struct lswitch *sw, struct rconn *rconn,
         buffer = make_add_flow(&rule, ntohl(opi->buffer_id),
                                sw->max_idle, actions_len);
         ofpbuf_put(buffer, actions, actions_len);
-        ofm = buffer->data;
         queue_tx(sw, rconn, buffer);

         /* If the switch didn't buffer the packet, we need to send a copy. */
--

-- 
1.7.4.2

Ethan Jackson | 1 Apr 2011 01:27

[dead 2/3] ovs-ofctl: Remove dead assignment.

---
 utilities/ovs-ofctl.c |    6 ++----
 1 files changed, 2 insertions(+), 4 deletions(-)

diff --git a/utilities/ovs-ofctl.c b/utilities/ovs-ofctl.c
index 81ce1f7..4f2999e 100644
--- a/utilities/ovs-ofctl.c
+++ b/utilities/ovs-ofctl.c
 <at>  <at>  -1121,8 +1121,8  <at>  <at>  read_flows_from_switch(struct vconn *vconn, enum nx_flow_format flow_format,
                                         verbosity + 1));
             }

-            osr = reply->data;
-            if (!(osr->flags & htons(OFPSF_REPLY_MORE))) {
+            osr = ofpbuf_at(reply, 0, sizeof *osr);
+            if (!osr || !(osr->flags & htons(OFPSF_REPLY_MORE))) {
                 done = true;
             }

 <at>  <at>  -1151,8 +1151,6  <at>  <at>  read_flows_from_switch(struct vconn *vconn, enum nx_flow_format flow_format,

                 fte_insert(cls, &fs.rule, version, index);
             }
-
-            osr = ofpbuf_at(reply, 0, sizeof *osr);
         } else {
             VLOG_DBG("received reply with xid %08"PRIx32" "
                      "!= expected %08"PRIx32, recv_xid, send_xid);
--

-- 
1.7.4.2
(Continue reading)

Ben Pfaff | 1 Apr 2011 01:31

[daemon 01/10] stream-ssl: Use out_of_memory() to abort due to lack of memory.

This matches what xmalloc() does.  It will be handled better by a monitor
process (created with --monitor), which will restart the child instead of
exiting.
---
 lib/stream-ssl.c |    3 +--
 1 files changed, 1 insertions(+), 2 deletions(-)

diff --git a/lib/stream-ssl.c b/lib/stream-ssl.c
index 4874bbe..977c5ba 100644
--- a/lib/stream-ssl.c
+++ b/lib/stream-ssl.c
 <at>  <at>  -1044,8 +1044,7  <at>  <at>  tmp_dh_callback(SSL *ssl OVS_UNUSED, int is_export OVS_UNUSED, int keylength)
             if (!dh->dh) {
                 dh->dh = dh->constructor();
                 if (!dh->dh) {
-                    ovs_fatal(ENOMEM, "out of memory constructing "
-                              "Diffie-Hellman parameters");
+                    out_of_memory();
                 }
             }
             return dh->dh;
--

-- 
1.7.1

Ben Pfaff | 1 Apr 2011 01:31

[daemon 00/10] avoid races in daemonization

At Nicira we have an internal test suite that restarts the vswitch a
lot, by calling "/etc/init.d/openvswitch restart".  Once in a while,
we end up with zero copies of a daemon after this runs, or with two
copies of a daemon.  Obviously this isn't good.  I can't figure out
how it would happen.  We're not running restarts in parallel, so it
shouldn't be a race between two separate restarts, and there's nothing
very special going on on these boxes.

I don't know that this series fixes the problem, because I don't know
exactly what the problem is.  But it should at least nail down some
of the possible offenders, and I hope that at least the increased
logging added in various places makes the problem easier to track down
should it keep occurring.

Ben Pfaff (10):
  stream-ssl: Use out_of_memory() to abort due to lack of memory.
  vlog: Use PRINTF_FORMAT macro from compiler.h.
  Add a few more users for ovs_retval_to_string().
  type-props: New macro for estimating length of a decimal integer.
  signals: New function signal_name().
  util: New function ovs_fatal_valist().
  Log anything that could prevent a daemon from starting.
  daemon: Tolerate EINTR in fork_and_wait_for_startup().
  daemon: Integrate checking for an existing pidfile into
    daemonize_start().
  daemon: Avoid races on pidfile creation.

 debian/ovs-monitor-ipsec                           |    4 +-
 lib/command-line.c                                 |   19 +-
 lib/daemon.c                                       |  278 ++++++++++++--------
(Continue reading)

Ben Pfaff | 1 Apr 2011 01:31

[daemon 02/10] vlog: Use PRINTF_FORMAT macro from compiler.h.

PRINTF_FORMAT is more portable than raw __attribute__.  We use it
elsewhere, I don't know why it wasn't used here.
---
 lib/vlog.h |    7 ++++---
 1 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/lib/vlog.h b/lib/vlog.h
index 7411f76..00d324f 100644
--- a/lib/vlog.h
+++ b/lib/vlog.h
 <at>  <at>  -21,6 +21,7  <at>  <at> 
 #include <stdarg.h>
 #include <stdbool.h>
 #include <time.h>
+#include "compiler.h"
 #include "util.h"

 #ifdef  __cplusplus
 <at>  <at>  -152,13 +153,13  <at>  <at>  int vlog_reopen_log_file(void);
 void vlog_init(void);
 void vlog_exit(void);
 void vlog(const struct vlog_module *, enum vlog_level, const char *format, ...)
-    __attribute__((format(printf, 3, 4)));
+    PRINTF_FORMAT (3, 4);
 void vlog_valist(const struct vlog_module *, enum vlog_level,
                  const char *, va_list)
-    __attribute__((format(printf, 3, 0)));
+    PRINTF_FORMAT (3, 0);
 void vlog_rate_limit(const struct vlog_module *, enum vlog_level,
                      struct vlog_rate_limit *, const char *, ...)
(Continue reading)


Gmane