This commit adds call-back function to the tunnel monitoring protocal
bfd/cfm to allow the session to notify the ofproto-dpif whenever its
status changes. This helps eliminate the overhead of updating unchanged
session status to the database.
Signed-off-by: Alex Wang <al...@nicira.com>
---
lib/bfd.c | 24 ++++++++++----
lib/bfd.h | 5 ++-
lib/cfm.c | 22 +++++++++---
lib/cfm.h | 4 ++-
ofproto/ofproto-dpif.c | 79 ++++++++++++++++++++++++++++++++++++--------
ofproto/ofproto-provider.h | 19 ++++++-----
ofproto/ofproto.c | 18 +++++-----
ofproto/ofproto.h | 6 ++--
vswitchd/bridge.c | 16 ++++++---
9 files changed, 143 insertions(+), 50 deletions(-)
diff --git a/lib/bfd.c b/lib/bfd.c
index 8bfe385..e7c09f7 100644
--- a/lib/bfd.c
+++ b/lib/bfd.c
@@ -21,7 +21,6 @@
#include <netinet/ip.h>
#include "byte-order.h"
-#include "connectivity.h"
#include "csum.h"
#include "dpif.h"
#include "dynamic-string.h"
@@ -37,7 +36,6 @@
#include "packets.h"
#include "poll-loop.h"
#include "random.h"
-#include "seq.h"
#include "smap.h"
#include "timeval.h"
#include "unaligned.h"
@@ -213,6 +211,13 @@ struct bfd {
long long int decay_detect_time; /* Decay detection time. */
uint64_t flap_count; /* Counts bfd forwarding flaps. */
+
+ /* When the variables returned by bfd_get_status() are changed,
+ * the status_chang_cb() should be called to signal the upper
+ * level owner of the session. 'aux' serves as an identifier
+ * to the upper level. */
+ void (*status_change_cb)(void *aux);
+ void *aux;
};
static struct ovs_mutex mutex = OVS_MUTEX_INITIALIZER;
@@ -307,7 +312,8 @@ bfd_get_status(const struct bfd *bfd, struct smap *smap)
* Also returns NULL if cfg is NULL. */
struct bfd *
bfd_configure(struct bfd *bfd, const char *name, const struct smap *cfg,
- struct netdev *netdev) OVS_EXCLUDED(mutex)
+ struct netdev *netdev, void (*status_change_cb)(void *aux),
+ void *aux) OVS_EXCLUDED(mutex)
{
static struct ovsthread_once once = OVSTHREAD_ONCE_INITIALIZER;
static atomic_uint16_t udp_src = ATOMIC_VAR_INIT(0);
@@ -351,6 +357,8 @@ bfd_configure(struct bfd *bfd, const char *name, const
struct smap *cfg,
bfd->rx_packets = bfd_rx_packets(bfd);
bfd->in_decay = false;
bfd->flap_count = 0;
+ bfd->status_change_cb = status_change_cb;
+ bfd->aux = aux;
/* RFC 5881 section 4
* The source port MUST be in the range 49152 through 65535. The same
@@ -363,6 +371,7 @@ bfd_configure(struct bfd *bfd, const char *name, const
struct smap *cfg,
bfd_set_state(bfd, STATE_DOWN, DIAG_NONE);
memcpy(bfd->eth_dst, eth_addr_bfd, ETH_ADDR_LEN);
+ bfd->status_change_cb(bfd->aux);
}
atomic_store(&bfd->check_tnl_key,
@@ -467,6 +476,7 @@ bfd_unref(struct bfd *bfd) OVS_EXCLUDED(mutex)
{
if (bfd && ovs_refcount_unref(&bfd->ref_cnt) == 1) {
ovs_mutex_lock(&mutex);
+ bfd->status_change_cb(bfd->aux);
hmap_remove(all_bfds, &bfd->node);
netdev_close(bfd->netdev);
free(bfd->name);
@@ -749,7 +759,7 @@ bfd_process_packet(struct bfd *bfd, const struct flow *flow,
}
if (bfd->rmt_state != rmt_state) {
- seq_change(connectivity_seq_get());
+ bfd->status_change_cb(bfd->aux);
}
bfd->rmt_disc = ntohl(msg->my_disc);
@@ -872,7 +882,7 @@ bfd_forwarding__(struct bfd *bfd) OVS_REQUIRES(mutex)
&& bfd->rmt_diag != DIAG_RCPATH_DOWN;
if (bfd->last_forwarding != last_forwarding) {
bfd->flap_count++;
- seq_change(connectivity_seq_get());
+ bfd->status_change_cb(bfd->aux);
}
return bfd->last_forwarding;
}
@@ -1085,7 +1095,7 @@ bfd_set_state(struct bfd *bfd, enum state state, enum
diag diag)
bfd_decay_update(bfd);
}
- seq_change(connectivity_seq_get());
+ bfd->status_change_cb(bfd->aux);
}
}
@@ -1279,9 +1289,11 @@ bfd_unixctl_set_forwarding_override(struct unixctl_conn
*conn, int argc,
goto out;
}
bfd->forwarding_override = forwarding_override;
+ bfd->status_change_cb(bfd->aux);
} else {
HMAP_FOR_EACH (bfd, node, all_bfds) {
bfd->forwarding_override = forwarding_override;
+ bfd->status_change_cb(bfd->aux);
}
}
diff --git a/lib/bfd.h b/lib/bfd.h
index 4e7d4cb..a6c4d36 100644
--- a/lib/bfd.h
+++ b/lib/bfd.h
@@ -43,7 +43,10 @@ void bfd_process_packet(struct bfd *, const struct flow *,
struct bfd *bfd_configure(struct bfd *, const char *name,
const struct smap *smap,
- struct netdev *netdev);
+ struct netdev *netdev,
+ void (*status_change_cb)(void *aux),
+ void *aux);
+
struct bfd *bfd_ref(const struct bfd *);
void bfd_unref(struct bfd *);
diff --git a/lib/cfm.c b/lib/cfm.c
index ce0c471..f3bbc77 100644
--- a/lib/cfm.c
+++ b/lib/cfm.c
@@ -22,7 +22,6 @@
#include <string.h>
#include "byte-order.h"
-#include "connectivity.h"
#include "dynamic-string.h"
#include "flow.h"
#include "hash.h"
@@ -32,7 +31,6 @@
#include "packets.h"
#include "poll-loop.h"
#include "random.h"
-#include "seq.h"
#include "timer.h"
#include "timeval.h"
#include "unixctl.h"
@@ -133,6 +131,13 @@ struct cfm {
struct ovs_refcount ref_cnt;
uint64_t flap_count; /* Count the flaps since boot. */
+
+ /* When the variables returned by cfm_get_*() are changed,
+ * the status_chang_cb() should be called to signal the upper
+ * level owner of the session. 'aux' serves as an identifier
+ * to the upper level. */
+ void (*status_change_cb)(void *aux);
+ void *aux;
};
/* Remote MPs represent foreign network entities that are configured to have
@@ -322,7 +327,8 @@ cfm_init(void)
/* Allocates a 'cfm' object called 'name'. 'cfm' should be initialized by
* cfm_configure() before use. */
struct cfm *
-cfm_create(const struct netdev *netdev) OVS_EXCLUDED(mutex)
+cfm_create(const struct netdev *netdev, void (*status_change_cb)(void *aux),
+ void *aux) OVS_EXCLUDED(mutex)
{
struct cfm *cfm;
@@ -338,11 +344,15 @@ cfm_create(const struct netdev *netdev)
OVS_EXCLUDED(mutex)
atomic_init(&cfm->extended, false);
atomic_init(&cfm->check_tnl_key, false);
ovs_refcount_init(&cfm->ref_cnt);
+ cfm->status_change_cb = status_change_cb;
+ cfm->aux = aux;
ovs_mutex_lock(&mutex);
cfm_generate_maid(cfm);
hmap_insert(all_cfms, &cfm->hmap_node, hash_string(cfm->name, 0));
ovs_mutex_unlock(&mutex);
+
+ cfm->status_change_cb(cfm->aux);
return cfm;
}
@@ -368,6 +378,7 @@ cfm_unref(struct cfm *cfm) OVS_EXCLUDED(mutex)
free(rmp);
}
+ cfm->status_change_cb(cfm->aux);
hmap_destroy(&cfm->remote_mps);
netdev_close(cfm->netdev);
free(cfm->rmps_array);
@@ -510,7 +521,7 @@ cfm_run(struct cfm *cfm) OVS_EXCLUDED(mutex)
|| (old_rmps_array_len != cfm->rmps_array_len || old_rmps_deleted)
|| old_cfm_fault != cfm->fault
|| old_flap_count != cfm->flap_count) {
- seq_change(connectivity_seq_get());
+ cfm->status_change_cb(cfm->aux);
}
cfm->booted = true;
@@ -1029,13 +1040,14 @@ cfm_unixctl_set_fault(struct unixctl_conn *conn, int
argc, const char *argv[],
goto out;
}
cfm->fault_override = fault_override;
+ cfm->status_change_cb(cfm->aux);
} else {
HMAP_FOR_EACH (cfm, hmap_node, all_cfms) {
cfm->fault_override = fault_override;
+ cfm->status_change_cb(cfm->aux);
}
}
- seq_change(connectivity_seq_get());
unixctl_command_reply(conn, "OK");
out:
diff --git a/lib/cfm.h b/lib/cfm.h
index 4213eb5..3fda809 100644
--- a/lib/cfm.h
+++ b/lib/cfm.h
@@ -64,7 +64,9 @@ struct cfm_settings {
};
void cfm_init(void);
-struct cfm *cfm_create(const struct netdev *);
+struct cfm *cfm_create(const struct netdev *,
+ void (*status_change_cb)(void *aux),
+ void *aux);
struct cfm *cfm_ref(const struct cfm *);
void cfm_unref(struct cfm *);
void cfm_run(struct cfm *);
diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c
index f72d53e..f48bd58 100644
--- a/ofproto/ofproto-dpif.c
+++ b/ofproto/ofproto-dpif.c
@@ -145,11 +145,14 @@ struct ofport_dpif {
struct hmap_node odp_port_node; /* In dpif_backer's "odp_to_ofport_map". */
struct ofport up;
+ struct ovs_mutex status_mutex; /* Just protects the *_status_changed. */
odp_port_t odp_port;
struct ofbundle *bundle; /* Bundle that contains this port, if any. */
struct list bundle_node; /* In struct ofbundle's "ports" list. */
struct cfm *cfm; /* Connectivity Fault Management, if any. */
struct bfd *bfd; /* BFD, if any. */
+ bool cfm_status_changed; /* Set when cfm status needs to be updated. */
+ bool bfd_status_changed; /* Set when bfd status needs to be updated. */
bool may_enable; /* May be enabled in bonds. */
bool is_tunnel; /* This port is a tunnel. */
bool is_layer3; /* This is a layer 3 port. */
@@ -205,6 +208,8 @@ ofport_dpif_cast(const struct ofport *ofport)
}
static void port_run(struct ofport_dpif *);
+static void cfm_status_changed_cb(void *);
+static void bfd_status_changed_cb(void *);
static int set_bfd(struct ofport *, const struct smap *);
static int set_cfm(struct ofport *, const struct cfm_settings *);
static void ofport_update_peer(struct ofport_dpif *);
@@ -1465,9 +1470,12 @@ port_construct(struct ofport *port_)
int error;
ofproto->backer->need_revalidate = REV_RECONFIGURE;
+ ovs_mutex_init(&port->status_mutex);
port->bundle = NULL;
port->cfm = NULL;
port->bfd = NULL;
+ port->cfm_status_changed = false;
+ port->bfd_status_changed = false;
port->may_enable = true;
port->stp_port = NULL;
port->stp_state = STP_DISABLED;
@@ -1571,6 +1579,7 @@ port_destruct(struct ofport *port_)
bundle_remove(port_);
set_cfm(port_, NULL);
set_bfd(port_, NULL);
+ ovs_mutex_destroy(&port->status_mutex);
if (port->stp_port) {
stp_port_disable(port->stp_port);
}
@@ -1699,7 +1708,8 @@ set_cfm(struct ofport *ofport_, const struct cfm_settings
*s)
ofproto = ofproto_dpif_cast(ofport->up.ofproto);
ofproto->backer->need_revalidate = REV_RECONFIGURE;
- ofport->cfm = cfm_create(ofport->up.netdev);
+ ofport->cfm = cfm_create(ofport->up.netdev, cfm_status_changed_cb,
+ ofport);
}
if (cfm_configure(ofport->cfm, s)) {
@@ -1717,22 +1727,43 @@ out:
return error;
}
-static bool
+static void
+cfm_status_changed_cb(void *aux)
+{
+ struct ofport_dpif *port = aux;
+
+ ovs_mutex_lock(&port->status_mutex);
+ port->cfm_status_changed = true;
+ seq_change(connectivity_seq_get());
+ ovs_mutex_unlock(&port->status_mutex);
+}
+
+static int
get_cfm_status(const struct ofport *ofport_,
struct ofproto_cfm_status *status)
{
struct ofport_dpif *ofport = ofport_dpif_cast(ofport_);
+ int ret;
+ ovs_mutex_lock(&ofport->status_mutex);
if (ofport->cfm) {
- status->faults = cfm_get_fault(ofport->cfm);
- status->flap_count = cfm_get_flap_count(ofport->cfm);
- status->remote_opstate = cfm_get_opup(ofport->cfm);
- status->health = cfm_get_health(ofport->cfm);
- cfm_get_remote_mpids(ofport->cfm, &status->rmps, &status->n_rmps);
- return true;
+ if (ofport->cfm_status_changed) {
+ status->faults = cfm_get_fault(ofport->cfm);
+ status->flap_count = cfm_get_flap_count(ofport->cfm);
+ status->remote_opstate = cfm_get_opup(ofport->cfm);
+ status->health = cfm_get_health(ofport->cfm);
+ cfm_get_remote_mpids(ofport->cfm, &status->rmps, &status->n_rmps);
+ ofport->cfm_status_changed = false;
+ ret = 0;
+ } else {
+ ret = -1;
+ }
} else {
- return false;
+ ret = 1;
}
+ ovs_mutex_unlock(&ofport->status_mutex);
+
+ return ret;
}
static int
@@ -1744,7 +1775,8 @@ set_bfd(struct ofport *ofport_, const struct smap *cfg)
old = ofport->bfd;
ofport->bfd = bfd_configure(old, netdev_get_name(ofport->up.netdev),
- cfg, ofport->up.netdev);
+ cfg, ofport->up.netdev, bfd_status_changed_cb,
+ ofport);
if (ofport->bfd != old) {
ofproto->backer->need_revalidate = REV_RECONFIGURE;
}
@@ -1753,17 +1785,38 @@ set_bfd(struct ofport *ofport_, const struct smap *cfg)
return 0;
}
+static void
+bfd_status_changed_cb(void *aux)
+{
+ struct ofport_dpif *port = aux;
+
+ ovs_mutex_lock(&port->status_mutex);
+ port->bfd_status_changed = true;
+ seq_change(connectivity_seq_get());
+ ovs_mutex_unlock(&port->status_mutex);
+}
+
static int
get_bfd_status(struct ofport *ofport_, struct smap *smap)
{
struct ofport_dpif *ofport = ofport_dpif_cast(ofport_);
+ int ret;
+ ovs_mutex_lock(&ofport->status_mutex);
if (ofport->bfd) {
- bfd_get_status(ofport->bfd, smap);
- return 0;
+ if (ofport->bfd_status_changed) {
+ bfd_get_status(ofport->bfd, smap);
+ ofport->bfd_status_changed = false;
+ ret = 0;
+ } else {
+ ret = -1;
+ }
} else {
- return ENOENT;
+ ret = ENOENT;
}
+ ovs_mutex_unlock(&ofport->status_mutex);
+
+ return ret;
}
/* Spanning Tree. */
diff --git a/ofproto/ofproto-provider.h b/ofproto/ofproto-provider.h
index adf080f..a71c083 100644
--- a/ofproto/ofproto-provider.h
+++ b/ofproto/ofproto-provider.h
@@ -1459,15 +1459,16 @@ struct ofproto_class {
* support CFM, as does a null pointer. */
int (*set_cfm)(struct ofport *ofport, const struct cfm_settings *s);
- /* Checks the status of CFM configured on 'ofport'. Returns true if the
- * port's CFM status was successfully stored into '*status'. Returns false
- * if the port did not have CFM configured, in which case '*status' is
- * indeterminate.
+ /* Checks the status of CFM configured on 'ofport'. Returns 0 if the
+ * port's CFM status was successfully stored into '*status'. Returns
+ * positive number if the port did not have CFM configured, in which
+ * case '*status' is indeterminate. And returns negative number if there
+ * is no status change since last update.
*
* The caller must provide and owns '*status', but it does not own and must
* not modify or free the array returned in 'status->rmps'. */
- bool (*get_cfm_status)(const struct ofport *ofport,
- struct ofproto_cfm_status *status);
+ int (*get_cfm_status)(const struct ofport *ofport,
+ struct ofproto_cfm_status *status);
/* Configures BFD on 'ofport'.
*
@@ -1480,8 +1481,10 @@ struct ofproto_class {
int (*set_bfd)(struct ofport *ofport, const struct smap *cfg);
/* Populates 'smap' with the status of BFD on 'ofport'. Returns 0 on
- * success, or a positive errno. EOPNOTSUPP as a return value indicates
- * that this ofproto_class does not support BFD, as does a null pointer. */
+ * success. Returns a negative errno if there is no status change since
+ * last update. Returns a positive errno otherwise. EOPNOTSUPP as a
+ * return value indicates that this ofproto_class does not support BFD,
+ * as does a null pointer. */
int (*get_bfd_status)(struct ofport *ofport, struct smap *smap);
/* Configures spanning tree protocol (STP) on 'ofproto' using the
diff --git a/ofproto/ofproto.c b/ofproto/ofproto.c
index cd1ec27..ccd2f07 100644
--- a/ofproto/ofproto.c
+++ b/ofproto/ofproto.c
@@ -1017,7 +1017,7 @@ ofproto_port_set_bfd(struct ofproto *ofproto, ofp_port_t
ofp_port,
/* Populates 'status' with key value pairs indicating the status of the BFD
* session on 'ofp_port'. This information is intended to be populated in the
- * OVS database. Has no effect if 'ofp_port' is not na OpenFlow port in
+ * OVS database. Has no effect if 'ofp_port' is not an OpenFlow port in
* 'ofproto'. */
int
ofproto_port_get_bfd_status(struct ofproto *ofproto, ofp_port_t ofp_port,
@@ -3684,20 +3684,20 @@ ofproto_get_netflow_ids(const struct ofproto *ofproto,
ofproto->ofproto_class->get_netflow_ids(ofproto, engine_type, engine_id);
}
-/* Checks the status of CFM configured on 'ofp_port' within 'ofproto'. Returns
- * true if the port's CFM status was successfully stored into '*status'.
- * Returns false if the port did not have CFM configured, in which case
- * '*status' is indeterminate.
+/* Checks the status of CFM configured on 'ofp_port' within 'ofproto'.
+ * Returns 0 if the port's CFM status was successfully stored into
+ * '*status'. Returns positive number if the port did not have CFM
+ * configured, in which case '*status' is indeterminate. Returns
+ * negative number if there is no status change since last update.
*
* The caller must provide and owns '*status', and must free 'status->rmps'. */
-bool
+int
ofproto_port_get_cfm_status(const struct ofproto *ofproto, ofp_port_t ofp_port,
struct ofproto_cfm_status *status)
{
struct ofport *ofport = ofproto_get_port(ofproto, ofp_port);
- return (ofport
- && ofproto->ofproto_class->get_cfm_status
- && ofproto->ofproto_class->get_cfm_status(ofport, status));
+ return (ofport && ofproto->ofproto_class->get_cfm_status ?
+ ofproto->ofproto_class->get_cfm_status(ofport, status) : 1);
}
static enum ofperr
diff --git a/ofproto/ofproto.h b/ofproto/ofproto.h
index 3f3557c..fafb128 100644
--- a/ofproto/ofproto.h
+++ b/ofproto/ofproto.h
@@ -418,9 +418,9 @@ struct ofproto_cfm_status {
size_t n_rmps;
};
-bool ofproto_port_get_cfm_status(const struct ofproto *,
- ofp_port_t ofp_port,
- struct ofproto_cfm_status *);
+int ofproto_port_get_cfm_status(const struct ofproto *,
+ ofp_port_t ofp_port,
+ struct ofproto_cfm_status *);
/* Linux VLAN device support (e.g. "eth0.10" for VLAN 10.)
*
diff --git a/vswitchd/bridge.c b/vswitchd/bridge.c
index 24b3602..1842e49 100644
--- a/vswitchd/bridge.c
+++ b/vswitchd/bridge.c
@@ -1835,9 +1835,14 @@ iface_refresh_cfm_stats(struct iface *iface)
{
const struct ovsrec_interface *cfg = iface->cfg;
struct ofproto_cfm_status status;
+ int error;
- if (!ofproto_port_get_cfm_status(iface->port->bridge->ofproto,
- iface->ofp_port, &status)) {
+ error = ofproto_port_get_cfm_status(iface->port->bridge->ofproto,
+ iface->ofp_port, &status);
+ /* Do nothing if there is no status change since last update. */
+ if (error < 0) {
+ // PASS.
+ } else if (error > 0) {
ovsrec_interface_set_cfm_fault(cfg, NULL, 0);
ovsrec_interface_set_cfm_fault_status(cfg, NULL, 0);
ovsrec_interface_set_cfm_remote_opstate(cfg, NULL);
@@ -2245,8 +2250,11 @@ instant_stats_run(void)
iface_refresh_cfm_stats(iface);
smap_init(&smap);
- ofproto_port_get_bfd_status(br->ofproto, iface->ofp_port,
- &smap);
+ error = ofproto_port_get_bfd_status(br->ofproto,
iface->ofp_port,
+ &smap);
+ if (error < 0) {
+ continue;
+ }
ovsrec_interface_set_bfd_status(iface->cfg, &smap);
smap_destroy(&smap);
}
--
1.7.9.5
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
