[ovs-dev] [PATCHv4 2/4] ofproto-dpif: Don't poll ports when nothing changes

Joe Stringer joestringer at nicira.com
Wed Dec 11 17:10:53 UTC 2013


Currently, as part of ofproto-dpif run() processing, we loop through all
ports and poll corresponding devices for changes in carrier, cfm and bfd
status. This allows us to determine how it may affect bundles. For the
average case where devices are not going up or down constantly, this is
a large amount of unnecessary processing.

This patch gets the bfd, cfm, lacp and stp modules to update the global
netdev change_seq when changes in port/device status occur. We can then
use this global change_seq to check if anything has changed before
looping through the ports in ofproto-dpif. In a test environment of 5000
internal ports and 50 tunnel ports with bfd, this reduces CPU usage from
about 35% to about 25%.

Signed-off-by: Joe Stringer <joestringer at nicira.com>
---
v4: Ensure that bfd_forwarding__() is called each time through bfd_run().
---
 lib/bfd.c              |    6 +++++-
 lib/cfm.c              |   13 +++++++++++++
 lib/lacp.c             |    7 +++++++
 lib/stp.c              |    4 ++++
 ofproto/ofproto-dpif.c |   16 +++++++++++++---
 tests/test-stp.c       |    5 +++++
 6 files changed, 47 insertions(+), 4 deletions(-)

diff --git a/lib/bfd.c b/lib/bfd.c
index 1df5acd..2482d37 100644
--- a/lib/bfd.c
+++ b/lib/bfd.c
@@ -21,6 +21,7 @@
 #include <netinet/ip.h>
 
 #include "byte-order.h"
+#include "connectivity.h"
 #include "csum.h"
 #include "dpif.h"
 #include "dynamic-string.h"
@@ -505,8 +506,8 @@ bfd_run(struct bfd *bfd) OVS_EXCLUDED(mutex)
 
     if (bfd->state > STATE_DOWN && now >= bfd->detect_time) {
         bfd_set_state(bfd, STATE_DOWN, DIAG_EXPIRED);
-        bfd_forwarding__(bfd);
     }
+    bfd_forwarding__(bfd);
 
     /* Decay may only happen when state is STATE_UP, bfd->decay_min_rx is
      * configured, and decay_detect_time is reached. */
@@ -851,6 +852,7 @@ bfd_forwarding__(struct bfd *bfd) OVS_REQUIRES(mutex)
                             && bfd->rmt_diag != DIAG_RCPATH_DOWN;
     if (bfd->last_forwarding != last_forwarding) {
         bfd->flap_count++;
+        connectivity_seq_notify();
     }
     return bfd->last_forwarding;
 }
@@ -1052,6 +1054,8 @@ bfd_set_state(struct bfd *bfd, enum state state, enum diag diag)
         if (bfd->state == STATE_UP && bfd->decay_min_rx) {
             bfd_decay_update(bfd);
         }
+
+        connectivity_seq_notify();
     }
 }
 
diff --git a/lib/cfm.c b/lib/cfm.c
index 9c65b34..5d27c6f 100644
--- a/lib/cfm.c
+++ b/lib/cfm.c
@@ -22,6 +22,7 @@
 #include <string.h>
 
 #include "byte-order.h"
+#include "connectivity.h"
 #include "dynamic-string.h"
 #include "flow.h"
 #include "hash.h"
@@ -396,6 +397,7 @@ cfm_run(struct cfm *cfm) OVS_EXCLUDED(mutex)
         long long int interval = cfm_fault_interval(cfm);
         struct remote_mp *rmp, *rmp_next;
         bool old_cfm_fault = cfm->fault;
+        bool old_rmp_opup = cfm->remote_opup;
         bool demand_override;
         bool rmp_set_opup = false;
         bool rmp_set_opdown = false;
@@ -420,6 +422,7 @@ cfm_run(struct cfm *cfm) OVS_EXCLUDED(mutex)
                 cfm->health = 0;
             } else {
                 int exp_ccm_recvd;
+                int old_health = cfm->health;
 
                 rmp = CONTAINER_OF(hmap_first(&cfm->remote_mps),
                                    struct remote_mp, node);
@@ -434,6 +437,10 @@ cfm_run(struct cfm *cfm) OVS_EXCLUDED(mutex)
                 cfm->health = MIN(cfm->health, 100);
                 rmp->num_health_ccm = 0;
                 ovs_assert(cfm->health >= 0 && cfm->health <= 100);
+
+                if (cfm->health != old_health) {
+                    connectivity_seq_notify();
+                }
             }
             cfm->health_interval = 0;
         }
@@ -476,6 +483,10 @@ cfm_run(struct cfm *cfm) OVS_EXCLUDED(mutex)
             cfm->remote_opup = true;
         }
 
+        if (old_rmp_opup != cfm->remote_opup) {
+            connectivity_seq_notify();
+        }
+
         if (hmap_is_empty(&cfm->remote_mps)) {
             cfm->fault |= CFM_FAULT_RECV;
         }
@@ -497,6 +508,8 @@ cfm_run(struct cfm *cfm) OVS_EXCLUDED(mutex)
             if (old_cfm_fault == false || cfm->fault == false) {
                 cfm->flap_count++;
             }
+
+            connectivity_seq_notify();
         }
 
         cfm->booted = true;
diff --git a/lib/lacp.c b/lib/lacp.c
index fce65b3..bb6e940 100644
--- a/lib/lacp.c
+++ b/lib/lacp.c
@@ -18,6 +18,7 @@
 
 #include <stdlib.h>
 
+#include "connectivity.h"
 #include "dynamic-string.h"
 #include "hash.h"
 #include "hmap.h"
@@ -509,11 +510,16 @@ lacp_run(struct lacp *lacp, lacp_send_pdu *send_pdu) OVS_EXCLUDED(mutex)
     ovs_mutex_lock(&mutex);
     HMAP_FOR_EACH (slave, node, &lacp->slaves) {
         if (timer_expired(&slave->rx)) {
+            enum slave_status old_status = slave->status;
+
             if (slave->status == LACP_CURRENT) {
                 slave_set_expired(slave);
             } else if (slave->status == LACP_EXPIRED) {
                 slave_set_defaulted(slave);
             }
+            if (slave->status != old_status) {
+                connectivity_seq_notify();
+            }
         }
     }
 
@@ -544,6 +550,7 @@ lacp_run(struct lacp *lacp, lacp_send_pdu *send_pdu) OVS_EXCLUDED(mutex)
                         : LACP_SLOW_TIME_TX);
 
             timer_set_duration(&slave->tx, duration);
+            connectivity_seq_notify();
         }
     }
     ovs_mutex_unlock(&mutex);
diff --git a/lib/stp.c b/lib/stp.c
index 725bfff..05bf0ff 100644
--- a/lib/stp.c
+++ b/lib/stp.c
@@ -26,6 +26,7 @@
 #include <inttypes.h>
 #include <stdlib.h>
 #include "byte-order.h"
+#include "connectivity.h"
 #include "ofpbuf.h"
 #include "packets.h"
 #include "unixctl.h"
@@ -1127,6 +1128,7 @@ stp_configuration_update(struct stp *stp) OVS_REQUIRES(mutex)
 {
     stp_root_selection(stp);
     stp_designated_port_selection(stp);
+    connectivity_seq_notify();
 }
 
 static bool
@@ -1257,6 +1259,7 @@ stp_set_port_state(struct stp_port *p, enum stp_state state)
         if (p < p->stp->first_changed_port) {
             p->stp->first_changed_port = p;
         }
+        connectivity_seq_notify();
     }
     p->state = state;
 }
@@ -1275,6 +1278,7 @@ stp_topology_change_detection(struct stp *stp) OVS_REQUIRES(mutex)
     }
     stp->fdb_needs_flush = true;
     stp->topology_change_detected = true;
+    connectivity_seq_notify();
     VLOG_INFO_RL(&rl, "%s: detected topology change.", stp->name);
 }
 
diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c
index ff77903..a4334cc 100644
--- a/ofproto/ofproto-dpif.c
+++ b/ofproto/ofproto-dpif.c
@@ -25,6 +25,7 @@
 #include "bond.h"
 #include "bundle.h"
 #include "byte-order.h"
+#include "connectivity.h"
 #include "connmgr.h"
 #include "coverage.h"
 #include "cfm.h"
@@ -511,6 +512,7 @@ struct ofproto_dpif {
     struct sset ghost_ports;       /* Ports with no datapath port. */
     struct sset port_poll_set;     /* Queued names for port_poll() reply. */
     int port_poll_errno;           /* Last errno for port_poll() reply. */
+    uint64_t change_seq;           /* Connectivity status changes. */
 
     /* Per ofproto's dpif stats. */
     uint64_t n_hit;
@@ -1269,6 +1271,7 @@ construct(struct ofproto *ofproto_)
     sset_init(&ofproto->ghost_ports);
     sset_init(&ofproto->port_poll_set);
     ofproto->port_poll_errno = 0;
+    ofproto->change_seq = 0;
 
     SHASH_FOR_EACH_SAFE (node, next, &init_ofp_ports) {
         struct iface_hint *iface_hint = node->data;
@@ -1474,8 +1477,8 @@ static int
 run(struct ofproto *ofproto_)
 {
     struct ofproto_dpif *ofproto = ofproto_dpif_cast(ofproto_);
-    struct ofport_dpif *ofport;
     struct ofbundle *bundle;
+    uint64_t new_seq;
     int error;
 
     if (mbridge_need_revalidate(ofproto->mbridge)) {
@@ -1508,8 +1511,15 @@ run(struct ofproto *ofproto_)
         dpif_ipfix_run(ofproto->ipfix);
     }
 
-    HMAP_FOR_EACH (ofport, up.hmap_node, &ofproto->up.ports) {
-        port_run(ofport);
+    new_seq = connectivity_seq_read();
+    if (ofproto->change_seq != new_seq) {
+        struct ofport_dpif *ofport;
+
+        HMAP_FOR_EACH (ofport, up.hmap_node, &ofproto->up.ports) {
+            port_run(ofport);
+        }
+
+        ofproto->change_seq = new_seq;
     }
     HMAP_FOR_EACH (bundle, hmap_node, &ofproto->bundles) {
         bundle_run(bundle);
diff --git a/tests/test-stp.c b/tests/test-stp.c
index be1b395..a177c87 100644
--- a/tests/test-stp.c
+++ b/tests/test-stp.c
@@ -16,6 +16,7 @@
 
 #include <config.h>
 
+#include "connectivity.h"
 #include "stp.h"
 #include <assert.h>
 #include <ctype.h>
@@ -454,6 +455,8 @@ main(int argc, char *argv[])
         ovs_fatal(errno, "error opening \"%s\"", file_name);
     }
 
+    connectivity_seq_init();
+
     tc = new_test_case();
     for (i = 0; i < 26; i++) {
         char name[2];
@@ -654,6 +657,8 @@ main(int argc, char *argv[])
     }
     free(token);
 
+    connectivity_seq_destroy();
+
     for (i = 0; i < tc->n_lans; i++) {
         struct lan *lan = tc->lans[i];
         free(CONST_CAST(char *, lan->name));
-- 
1.7.9.5




More information about the dev mailing list