Thanks for fixing this bug Ryan,
For this round, high level comments:
> -static struct ovsdb_idl_txn *status_txn;
> +static struct ovsdb_idl_txn *update_txn;
> +enum ovsdb_idl_txn_status update_txn_status = TXN_UNCOMMITTED;
>
> /* When the status update transaction returns 'TXN_INCOMPLETE', should
> register a
> * timeout in 'STATUS_CHECK_AGAIN_MSEC' to check again. */
> @@ -1820,7 +1827,9 @@ 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)
> + && (update_txn_status == TXN_SUCCESS ||
> + update_txn_status == TXN_UNCHANGED)) {
> return;
> }
>
Instead of checking here, could we have a global boolean for forcing
status update?
Basically, we check the ovsdb_idl_txn_commit() return value, if the value
is not INCOMPLETE, SUCCESS or UNCHANGED, we set the boolean.
> @@ -2415,14 +2424,14 @@ bridge_run(void)
> stats_timer = time_msec() + stats_timer_interval;
> }
>
> - if (!status_txn) {
> + if (!update_txn) {
> uint64_t seq;
>
> /* Check the need to update status. */
> seq = seq_read(connectivity_seq_get());
> if (seq != connectivity_seqno) {
>
We need to use the flag here to force an update.
> connectivity_seqno = seq;
> - status_txn = ovsdb_idl_txn_create(idl);
> + update_txn = ovsdb_idl_txn_create(idl);
> HMAP_FOR_EACH (br, node, &all_bridges) {
> struct port *port;
>
> @@ -2440,15 +2449,13 @@ bridge_run(void)
> }
> }
>
> - if (status_txn) {
> - enum ovsdb_idl_txn_status status;
> -
> - status = ovsdb_idl_txn_commit(status_txn);
> - /* Do not destroy "status_txn" if the transaction is
> + if (update_txn) {
> + update_txn_status = ovsdb_idl_txn_commit(update_txn);
> + /* Do not destroy "update_txn" if the transaction is
> * "TXN_INCOMPLETE". */
> - if (status != TXN_INCOMPLETE) {
> - ovsdb_idl_txn_destroy(status_txn);
> - status_txn = NULL;
> + if (update_txn_status != TXN_INCOMPLETE) {
> + ovsdb_idl_txn_destroy(update_txn);
> + update_txn = NULL;
> }
> }
>
> @@ -2487,7 +2494,7 @@ bridge_wait(void)
> * 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 (update_txn) {
> 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
> [email protected]
> http://openvswitch.org/mailman/listinfo/dev
>
This line need to be removed,
diff --git a/vswitchd/bridge.c b/vswitchd/bridge.c
index 9764c1f..7fbf19a 100644
--- a/vswitchd/bridge.c
+++ b/vswitchd/bridge.c
@@ -1547,7 +1547,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)) {
Since we do another update transaction after the bridge_reconfigure()
execution, if this transaction fails, the initial status of netdev will not
be
updated to database until next global connectivity_seq change.
So, instead, we could set the force update flag and the update will be
handled in the status update.
_______________________________________________
dev mailing list
[email protected]
http://openvswitch.org/mailman/listinfo/dev
