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
