Acked-by: Ethan Jackson <et...@nicira.com>
Could you fold my acked by into the commit message so when you resend
rebased on the new version of the first patch, I know not to review it
again?
Thanks
On Wed, Dec 11, 2013 at 9:10 AM, Joe Stringer <joestrin...@nicira.com> wrote:
> 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 <joestrin...@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
>
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
