Bridge, port and interface status changes are not sent to the
database if the connectivity and netdev sequence numbers have not
changed. However, if the previous database transaction fails, then
status changes will not be updated in the database until the
connectivity and netdev sequence numbers change again. This could
leave the database in an incorrect state for a long period of time.
This patch always sends status changes to the database if the last
transaction was not successful.
Bug #1256577
Signed-off-by: Ryan Wilson <wr...@nicira.com>
---
v2: Addressed Alex's comments, edited commit message to be more
accurate
v3: Remove iface_refresh_netdev_status() from iface_create() upon
further discussion with Alex
v4: Changed force_status_update to force_status_commit since it now
is set to true when status == 'TXN_INCOMPLETE'. Status txn also
needs to be run every STATUS_CHECK_AGAIN_MSEC if last transaction
was not successful.
v5: Add static identifier to force_status_commit.Add force update
BFD and CFM status.
---
ofproto/ofproto-dpif.c | 8 ++++----
ofproto/ofproto-provider.h | 25 ++++++++++++++++---------
ofproto/ofproto.c | 31 ++++++++++++++++++-------------
ofproto/ofproto.h | 5 +++--
vswitchd/bridge.c | 41 ++++++++++++++++++++++++++++++-----------
5 files changed, 71 insertions(+), 39 deletions(-)
diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c
index 06be234..4b73158 100644
--- a/ofproto/ofproto-dpif.c
+++ b/ofproto/ofproto-dpif.c
@@ -1821,13 +1821,13 @@ out:
static int
get_cfm_status(const struct ofport *ofport_,
- struct ofproto_cfm_status *status)
+ struct ofproto_cfm_status *status, bool force)
{
struct ofport_dpif *ofport = ofport_dpif_cast(ofport_);
int ret = 0;
if (ofport->cfm) {
- if (cfm_check_status_change(ofport->cfm)) {
+ if (cfm_check_status_change(ofport->cfm) || force) {
status->faults = cfm_get_fault(ofport->cfm);
status->flap_count = cfm_get_flap_count(ofport->cfm);
status->remote_opstate = cfm_get_opup(ofport->cfm);
@@ -1862,13 +1862,13 @@ set_bfd(struct ofport *ofport_, const struct smap *cfg)
}
static int
-get_bfd_status(struct ofport *ofport_, struct smap *smap)
+get_bfd_status(struct ofport *ofport_, struct smap *smap, bool force)
{
struct ofport_dpif *ofport = ofport_dpif_cast(ofport_);
int ret = 0;
if (ofport->bfd) {
- if (bfd_check_status_change(ofport->bfd)) {
+ if (bfd_check_status_change(ofport->bfd) || force) {
bfd_get_status(ofport->bfd, smap);
} else {
ret = NO_STATUS_CHANGE;
diff --git a/ofproto/ofproto-provider.h b/ofproto/ofproto-provider.h
index ff539b9..212ed24 100644
--- a/ofproto/ofproto-provider.h
+++ b/ofproto/ofproto-provider.h
@@ -1442,10 +1442,13 @@ 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 0 if the
- * port's CFM status was successfully stored into '*status'. Returns
- * negative number if there is no status change since last update.
- * Returns positive errno otherwise.
+ /* Checks the status of CFM configured on 'ofport' and stores port's CFM
+ * status in '*status'. If 'force' is set to true, status will be returned
+ * even if there is no status change since last update.
+ *
+ * Returns 0 on success. Returns a negative number if there is no status
+ * change since last update and 'force' is set to false. Returns positive
+ * errno otherwise.
*
* EOPNOTSUPP as a return value indicates that this ofproto_class does not
* support CFM, as does a null pointer.
@@ -1454,7 +1457,7 @@ struct ofproto_class {
* returned in 'status->rmps'. '*status' is indeterminate if the return
* value is non-zero. */
int (*get_cfm_status)(const struct ofport *ofport,
- struct ofproto_cfm_status *status);
+ struct ofproto_cfm_status *status, bool force);
/* Configures BFD on 'ofport'.
*
@@ -1466,13 +1469,17 @@ struct ofproto_class {
* support BFD, as does a null pointer. */
int (*set_bfd)(struct ofport *ofport, const struct smap *cfg);
- /* Populates 'smap' with the status of BFD on 'ofport'. Returns 0 on
- * success. Returns a negative number if there is no status change since
- * last update. Returns a positive errno otherwise.
+ /* Populates 'smap' with the status of BFD on 'ofport'. If 'force' is set
+ * to true, status will be returned even if there is no status change since
+ * last update.
+ *
+ * Returns 0 on success. Returns a negative number if there is no status
+ * change since last update and 'force' is set to false. 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);
+ int (*get_bfd_status)(struct ofport *ofport, struct smap *smap, bool
force);
/* Configures spanning tree protocol (STP) on 'ofproto' using the
* settings defined in 's'.
diff --git a/ofproto/ofproto.c b/ofproto/ofproto.c
index 24a709b..2b872b7 100644
--- a/ofproto/ofproto.c
+++ b/ofproto/ofproto.c
@@ -1020,19 +1020,22 @@ ofproto_port_set_bfd(struct ofproto *ofproto,
ofp_port_t ofp_port,
}
}
-/* Populates 'status' with the status of BFD on 'ofport'. Returns 0 on
- * success. Returns a negative number if there is no status change since
- * last update. Returns a positive errno otherwise. Has no effect if
- * 'ofp_port' is not an OpenFlow port in 'ofproto'.
+/* Populates 'status' with the status of BFD on 'ofport'. If 'force' is set to
+ * true, status will be returned even if there is no status change since last
+ * update.
+ *
+ * Returns 0 on success. Returns a negative number if there is no status
change
+ * since last update and 'force' is set to false. Returns a positive errno
+ * otherwise. Has no effect if 'ofp_port' is not an OpenFlow port in
'ofproto'.
*
* The caller must provide and own '*status'. */
int
ofproto_port_get_bfd_status(struct ofproto *ofproto, ofp_port_t ofp_port,
- struct smap *status)
+ struct smap *status, bool force)
{
struct ofport *ofport = ofproto_get_port(ofproto, ofp_port);
return (ofport && ofproto->ofproto_class->get_bfd_status
- ? ofproto->ofproto_class->get_bfd_status(ofport, status)
+ ? ofproto->ofproto_class->get_bfd_status(ofport, status, force)
: EOPNOTSUPP);
}
@@ -3738,21 +3741,23 @@ 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 0 if the port's CFM status was successfully stored into
- * '*status'. Returns positive errno if the port did not have CFM
- * configured. Returns negative number if there is no status change
- * since last update.
+/* Checks the status of CFM configured on 'ofp_port' within 'ofproto' and
stores
+ * the port's CFM status in '*status'. If 'force' is set to true, status will
+ * be returned even if there is no status change since last update.
+ *
+ * Returns 0 on success. Returns a negative number if there is no status
+ * change since last update and 'force' is set to false. Returns positive
errno
+ * if the port did not have CFM configured.
*
* The caller must provide and own '*status', and must free 'status->rmps'.
* '*status' is indeterminate if the return value is non-zero. */
int
ofproto_port_get_cfm_status(const struct ofproto *ofproto, ofp_port_t ofp_port,
- struct ofproto_cfm_status *status)
+ struct ofproto_cfm_status *status, bool force)
{
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)
+ ? ofproto->ofproto_class->get_cfm_status(ofport, status, force)
: EOPNOTSUPP);
}
diff --git a/ofproto/ofproto.h b/ofproto/ofproto.h
index de078b7..aa7b95d 100644
--- a/ofproto/ofproto.h
+++ b/ofproto/ofproto.h
@@ -265,7 +265,7 @@ void ofproto_port_set_cfm(struct ofproto *, ofp_port_t
ofp_port,
void ofproto_port_set_bfd(struct ofproto *, ofp_port_t ofp_port,
const struct smap *cfg);
int ofproto_port_get_bfd_status(struct ofproto *, ofp_port_t ofp_port,
- struct smap *);
+ struct smap *, bool force);
int ofproto_port_is_lacp_current(struct ofproto *, ofp_port_t ofp_port);
int ofproto_port_set_stp(struct ofproto *, ofp_port_t ofp_port,
const struct ofproto_port_stp_settings *);
@@ -424,7 +424,8 @@ struct ofproto_cfm_status {
int ofproto_port_get_cfm_status(const struct ofproto *,
ofp_port_t ofp_port,
- struct ofproto_cfm_status *);
+ struct ofproto_cfm_status *,
+ bool force);
/* Linux VLAN device support (e.g. "eth0.10" for VLAN 10.)
*
diff --git a/vswitchd/bridge.c b/vswitchd/bridge.c
index 9764c1f..a20e950 100644
--- a/vswitchd/bridge.c
+++ b/vswitchd/bridge.c
@@ -172,9 +172,15 @@ static uint64_t connectivity_seqno = LLONG_MIN;
* we check the return status of each update transaction and do not start new
* update if the previous transaction status is 'TXN_INCOMPLETE'.
*
- * 'statux_txn' is NULL if there is no ongoing status update.
+ * 'status_txn' is NULL if there is no ongoing status update.
+ *
+ * If the previous database transaction was incomplete or failed (is not
+ * 'TXN_SUCCESS' or 'TXN_UNCHANGED'), 'force_status_commit' is set to true.
+ * This means that 'status_txn' must be committed next iteration of
bridge_run()
+ * even if the connectivity or netdev sequence numbers do not change.
*/
static struct ovsdb_idl_txn *status_txn;
+static bool force_status_commit = true;
/* When the status update transaction returns 'TXN_INCOMPLETE', should
register a
* timeout in 'STATUS_CHECK_AGAIN_MSEC' to check again. */
@@ -1547,7 +1553,6 @@ iface_create(struct bridge *br, const struct
ovsrec_interface *iface_cfg,
/* Populate initial status in database. */
iface_refresh_stats(iface);
- iface_refresh_netdev_status(iface);
/* Add bond fake iface if necessary. */
if (port_is_bond_fake_iface(port)) {
@@ -1820,7 +1825,8 @@ iface_refresh_netdev_status(struct iface *iface)
return;
}
- if (iface->change_seq == netdev_get_change_seq(iface->netdev)) {
+ if (iface->change_seq == netdev_get_change_seq(iface->netdev)
+ && !force_status_commit) {
return;
}
@@ -1913,7 +1919,8 @@ iface_refresh_ofproto_status(struct iface *iface)
smap_init(&smap);
error = ofproto_port_get_bfd_status(iface->port->bridge->ofproto,
- iface->ofp_port, &smap);
+ iface->ofp_port, &smap,
+ force_status_commit);
if (error >= 0) {
ovsrec_interface_set_bfd_status(iface->cfg, &smap);
}
@@ -1930,7 +1937,8 @@ iface_refresh_cfm_stats(struct iface *iface)
int error;
error = ofproto_port_get_cfm_status(iface->port->bridge->ofproto,
- iface->ofp_port, &status);
+ iface->ofp_port, &status,
+ force_status_commit);
if (error < 0) {
/* Do nothing if there is no status change since last update. */
} else if (error > 0) {
@@ -2420,7 +2428,7 @@ bridge_run(void)
/* Check the need to update status. */
seq = seq_read(connectivity_seq_get());
- if (seq != connectivity_seqno) {
+ if (seq != connectivity_seqno || force_status_commit) {
connectivity_seqno = seq;
status_txn = ovsdb_idl_txn_create(idl);
HMAP_FOR_EACH (br, node, &all_bridges) {
@@ -2444,6 +2452,17 @@ bridge_run(void)
enum ovsdb_idl_txn_status status;
status = ovsdb_idl_txn_commit(status_txn);
+
+ /* If the transaction is incomplete or fails, 'status_txn'
+ * needs to be committed next iteration of bridge_run() even if
+ * connectivity or netdev sequence numbers do not change. */
+ if (status == TXN_SUCCESS || status == TXN_UNCHANGED)
+ {
+ force_status_commit = false;
+ } else {
+ force_status_commit = true;
+ }
+
/* Do not destroy "status_txn" if the transaction is
* "TXN_INCOMPLETE". */
if (status != TXN_INCOMPLETE) {
@@ -2483,11 +2502,11 @@ bridge_wait(void)
poll_timer_wait_until(stats_timer);
}
- /* If the status database transaction is 'TXN_INCOMPLETE' in this run,
- * register a timeout in 'STATUS_CHECK_AGAIN_MSEC'. Else, wait on the
- * global connectivity sequence number. Note, this also helps batch
- * multiple status changes into one transaction. */
- if (status_txn) {
+ /* If the status database transaction is 'TXN_INCOMPLETE' or is
+ * unsuccessful, register a timeout in 'STATUS_CHECK_AGAIN_MSEC'. Else,
+ * wait on the global connectivity sequence number. Note, this also helps
+ * batch multiple status changes into one transaction. */
+ if (force_status_commit) {
poll_timer_wait_until(time_msec() + STATUS_CHECK_AGAIN_MSEC);
} else {
seq_wait(connectivity_seq_get(), connectivity_seqno);
--
1.7.9.5
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
