Err, for those without gmail context; I am referring to the bfd forwarding changes here:
http://git.openvswitch.org/cgi-bin/gitweb.cgi?p=openvswitch;a=commitdiff;h=a1aeea86475db086ce95679962fb6d03d0a645f3 On 10 December 2013 12:56, Joe Stringer <joestrin...@nicira.com> wrote: > The merged version of the above patchset doesn't quite address the > testsuite failure, for reasons described in that thread. Below is an > incremental to fix this issue. I'm happy to fold in and repost, but I > might as well wait until I get further feedback on the rest of this > set. > > diff --git a/lib/bfd.c b/lib/bfd.c > index 7f677ee..2482d37 100644 > --- a/lib/bfd.c > +++ b/lib/bfd.c > @@ -506,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. */ > > On 25 November 2013 14:32, Joe Stringer <joestrin...@nicira.com> wrote: >> The patch series posted below fixes the cause of these test failures:- >> >> http://openvswitch.org/pipermail/dev/2013-November/034359.html >> >> On 25 November 2013 13:44, 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, and cause >>> revalidation on bridges when necessary. 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 >>> connectivity_seq >>> when changes in port/device status occur. We can then use this 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 average CPU usage from around 35%->25%. >>> >>> Signed-off-by: Joe Stringer <joestrin...@nicira.com> >>> --- >>> v3: Rebase against bfd_forwarding__() changes >>> Track ofproto-dpif change separate from ofproto >>> Track cfm->health >>> Fix STP test failures >>> Track STP more comprehensively >>> Notify if slave status changes >>> v2: Fixed mutex safety in netdev_vport_changed() >>> Track STP changes >>> >>> NB: This introduces test failures for BFD. This is due to a subtle >>> interaction >>> between this patch and recent bfd changes. A forthcoming patch will fix this >>> issue. >>> --- >>> lib/bfd.c | 4 ++++ >>> 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, 46 insertions(+), 3 deletions(-) >>> >>> diff --git a/lib/bfd.c b/lib/bfd.c >>> index 4582f2e..c334e88 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" >>> @@ -831,6 +832,7 @@ bfd_forwarding__(struct bfd *bfd) OVS_REQUIRES(mutex) >>> && bfd->rmt_diag != DIAG_RCPATH_DOWN; >>> if (bfd->forwarding != forwarding_old) { >>> bfd->flap_count++; >>> + connectivity_seq_notify(); >>> } >>> return bfd->forwarding; >>> } >>> @@ -1032,6 +1034,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(); >>> } >>> >>> /* Updates the forwarding flag. */ >>> 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 6e1efd0..ecf37c1 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 6653049..c06dc20 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" >>> @@ -510,6 +511,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; >>> @@ -1268,6 +1270,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; >>> @@ -1473,8 +1476,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)) { >>> @@ -1507,8 +1510,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 28e9a6e..1bcd12f 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
