Hey Ethan,
Could you review these two patches? Should be straightforward,
Thanks,
Alex Wang,
On Wed, May 7, 2014 at 12:01 AM, Alex Wang <al...@nicira.com> wrote:
> Commit 88bf179aa3 (bfd/cfm: Check status change before update
> status to database.) used a boolean flag to trigger cfm status
> update. However, the flag is not set on cfm creation and deletion.
> And this causes stale status in database which may confuse users.
>
> This commit fixes the issue by making cfm module trigger status
> update on creation and deletion.
>
> Bug #1245800
>
> Signed-off-by: Alex Wang <al...@nicira.com>
> ---
> lib/cfm.c | 18 +++++++++--------
> tests/cfm.at | 62
> ++++++++++++++++++++++++++++++++++++++++++++++------------
> 2 files changed, 59 insertions(+), 21 deletions(-)
>
> diff --git a/lib/cfm.c b/lib/cfm.c
> index 1b32625..c271e1e 100644
> --- a/lib/cfm.c
> +++ b/lib/cfm.c
> @@ -328,6 +328,14 @@ cfm_init(void)
> 1, 2, cfm_unixctl_set_fault, NULL);
> }
>
> +/* Records the status change and changes the global connectivity seq. */
> +static void
> +cfm_status_changed(struct cfm *cfm) OVS_REQUIRES(mutex)
> +{
> + seq_change(connectivity_seq_get());
> + cfm->status_changed = true;
> +}
> +
> /* Allocates a 'cfm' object called 'name'. 'cfm' should be initialized by
> * cfm_configure() before use. */
> struct cfm *
> @@ -349,6 +357,7 @@ cfm_create(const struct netdev *netdev)
> OVS_EXCLUDED(mutex)
> ovs_refcount_init(&cfm->ref_cnt);
>
> ovs_mutex_lock(&mutex);
> + cfm_status_changed(cfm);
> cfm_generate_maid(cfm);
> hmap_insert(all_cfms, &cfm->hmap_node, hash_string(cfm->name, 0));
> ovs_mutex_unlock(&mutex);
> @@ -370,6 +379,7 @@ cfm_unref(struct cfm *cfm) OVS_EXCLUDED(mutex)
> }
>
> ovs_mutex_lock(&mutex);
> + cfm_status_changed(cfm);
> hmap_remove(all_cfms, &cfm->hmap_node);
> ovs_mutex_unlock(&mutex);
>
> @@ -395,14 +405,6 @@ cfm_ref(const struct cfm *cfm_)
> return cfm;
> }
>
> -/* Records the status change and changes the global connectivity seq. */
> -static void
> -cfm_status_changed(struct cfm *cfm) OVS_REQUIRES(mutex)
> -{
> - seq_change(connectivity_seq_get());
> - cfm->status_changed = true;
> -}
> -
> /* Should be run periodically to update fault statistics messages. */
> void
> cfm_run(struct cfm *cfm) OVS_EXCLUDED(mutex)
> diff --git a/tests/cfm.at b/tests/cfm.at
> index fdca4ac..06cab90 100644
> --- a/tests/cfm.at
> +++ b/tests/cfm.at
> @@ -28,23 +28,54 @@ MPID $2: extended
> ])
>
> m4_define([CFM_VSCTL_LIST_IFACE], [
> -AT_CHECK([ovs-vsctl list interface $1 | sed -n '/$2/p'],[0],
> +AT_CHECK([ovs-vsctl list interface $1 | sed -n '/$2 /p'],[0],
> [dnl
> $3
> ])
> ])
>
> m4_define([CFM_CHECK_DB], [
> -CFM_VSCTL_LIST_IFACE([$1], [cfm_fault_status], [cfm_fault_status :
> [[$2]]])
> -CFM_VSCTL_LIST_IFACE([$1], [cfm_flap_count], [cfm_flap_count : $3])
> -CFM_VSCTL_LIST_IFACE([$1], [cfm_health], [cfm_health : [[$4]]])
> -CFM_VSCTL_LIST_IFACE([$1], [cfm_remote_mpids], [cfm_remote_mpids :
> [[$5]]])
> -CFM_VSCTL_LIST_IFACE([$1], [cfm_remote_opstate], [cfm_remote_opstate :
> $6])
> +CFM_VSCTL_LIST_IFACE([$1], [cfm_fault], [cfm_fault : $2])
> +CFM_VSCTL_LIST_IFACE([$1], [cfm_fault_status], [cfm_fault_status :
> [[$3]]])
> +CFM_VSCTL_LIST_IFACE([$1], [cfm_flap_count], [cfm_flap_count : $4])
> +CFM_VSCTL_LIST_IFACE([$1], [cfm_health], [cfm_health : [[$5]]])
> +CFM_VSCTL_LIST_IFACE([$1], [cfm_remote_mpids], [cfm_remote_mpids :
> [[$6]]])
> +CFM_VSCTL_LIST_IFACE([$1], [cfm_remote_opstate], [cfm_remote_opstate :
> $7])
> ])
>
> -# This test checks the update of cfm status to OVSDB at startup.
> -# The cfm status should be updated to OVSDB within 3.5 * cfm_interval.
> -AT_SETUP([cfm - check update ovsdb])
> +# These two tests check the update of cfm status at different scenarios.
> +
> +# Test cfm status update at startup and removal.
> +AT_SETUP([cfm - check update ovsdb 1])
> +OVS_VSWITCHD_START([add-port br0 p0 -- set Interface p0 type=gre \
> + options:remote_ip=1.2.3.4 -- \
> + set Interface p0 other_config:cfm_interval=300
> other_config:cfm_extended=true])
> +
> +ovs-appctl time/stop
> +
> +AT_CHECK([ovs-vsctl set Interface p0 cfm_mpid=1])
> +# at beginning, since the first fault check timeout is not reached
> +# cfm_fault should be false.
> +for i in `seq 0 4`; do
> + ovs-appctl time/warp 100
> + CFM_CHECK_DB([p0], [false], [], [0], [], [], [up])
> +done
> +
> +# advance clock to pass the fault check timeout and check cfm
> +# status update in OVSDB.
> +for i in `seq 0 14`; do ovs-appctl time/warp 100; done
> +CFM_CHECK_DB([p0], [true], [recv], [1], [], [], [up])
> +
> +# remove the cfm on p0 and status should be all empty.
> +AT_CHECK([ovs-vsctl remove int p0 cfm_mpid 1])
> +for i in `seq 0 4`; do ovs-appctl time/warp 100; done
> +CFM_CHECK_DB([p0], [[[]]], [], [[[]]], [], [], [[[]]])
> +
> +OVS_VSWITCHD_STOP
> +AT_CLEANUP
> +
> +# Test cfm status update in normal case.
> +AT_SETUP([cfm - check update ovsdb 2])
> #Create 2 bridges connected by patch ports and enable cfm
> OVS_VSWITCHD_START([add-br br1 -- \
> set bridge br1 datapath-type=dummy \
> @@ -61,13 +92,18 @@ ovs-appctl time/stop
> AT_CHECK([ovs-vsctl set Interface p0 cfm_mpid=1])
> # check cfm status update in OVSDB.
> for i in `seq 0 14`; do ovs-appctl time/warp 100; done
> -CFM_CHECK_DB([p0], [recv], [1], [], [], [up])
> +CFM_CHECK_DB([p0], [true], [recv], [1], [], [], [up])
>
> -# turn cfm on p1 off, should increment the cfm_flap_count on p0.
> +# turn cfm on p1 on, cfm status of p0 and p1 should all go up.
> AT_CHECK([ovs-vsctl set interface p1 cfm_mpid=2])
> for i in `seq 0 14`; do ovs-appctl time/warp 100; done
> -CFM_CHECK_DB([p0], [], [2], [], [2], [up])
> -CFM_CHECK_DB([p1], [], [0], [], [1], [up])
> +CFM_CHECK_DB([p0], [false], [], [2], [], [2], [up])
> +CFM_CHECK_DB([p1], [false], [], [0], [], [1], [up])
> +
> +# turn cfm on p1 off, cfm status of p0 should go down again.
> +AT_CHECK([ovs-vsctl remove int p1 cfm_mpid 2])
> +for i in `seq 0 14`; do ovs-appctl time/warp 100; done
> +CFM_CHECK_DB([p0], [true], [recv], [3], [], [], [up])
>
> OVS_VSWITCHD_STOP
> AT_CLEANUP
> --
> 1.7.9.5
>
>
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
