Thanks, merged with some minor tweaks.
Ethan
On Thu, Sep 19, 2013 at 3:13 PM, Alex Wang <al...@nicira.com> wrote:
> This commit prevents cfm from raising 'interval' fault when demand
> mode is only enabled on one end of link.
>
> Signed-off-by: Alex Wang <al...@nicira.com>
>
> ---
>
> v1 -> v2:
> - simplify the code.
>
> ---
> lib/cfm.c | 12 +++++++----
> tests/automake.mk | 1 +
> tests/cfm.at | 57
> ++++++++++++++++++++++++++++++++++++++++++++++++++
> tests/testsuite.at | 1 +
> vswitchd/vswitch.xml | 5 +++--
> 5 files changed, 70 insertions(+), 6 deletions(-)
> create mode 100644 tests/cfm.at
>
> diff --git a/lib/cfm.c b/lib/cfm.c
> index 4a46c05..b496d81 100644
> --- a/lib/cfm.c
> +++ b/lib/cfm.c
> @@ -255,9 +255,14 @@ cfm_fault_interval(struct cfm *cfm) OVS_REQUIRES(mutex)
> * as a fault (likely due to a configuration error). Thus we can check
> all
> * MPs at once making this quite a bit simpler.
> *
> - * According to the specification we should check when (ccm_interval_ms *
> - * 3.5)ms have passed. */
> - return (cfm->ccm_interval_ms * 7) / 2;
> + * When cfm is not in demand mode, we check when (ccm_interval_ms * 3.5)
> ms
> + * have passed.
> + * When cfm is in demand mode, we check when
> + * (MAX(ccm_interval_ms, 500) * 3.5) ms have passed. This is to ensure
> + * that ovs-vswitchd has enough time to pull statistics from the
> datapath. */
> +
> + return (MAX(cfm->ccm_interval_ms, cfm->demand ? 500 :
> cfm->ccm_interval_ms)
> + * 7) / 2;
> }
>
> static uint8_t
> @@ -611,7 +616,6 @@ cfm_configure(struct cfm *cfm, const struct cfm_settings
> *s)
> }
>
> if (s->extended && s->demand) {
> - interval_ms = MAX(interval_ms, 500);
> if (!cfm->demand) {
> cfm->demand = true;
> cfm->rx_packets = cfm_rx_packets(cfm);
> diff --git a/tests/automake.mk b/tests/automake.mk
> index 60d3640..8f51a65 100644
> --- a/tests/automake.mk
> +++ b/tests/automake.mk
> @@ -22,6 +22,7 @@ TESTSUITE_AT = \
> tests/odp.at \
> tests/multipath.at \
> tests/bfd.at \
> + tests/cfm.at \
> tests/lacp.at \
> tests/learn.at \
> tests/vconn.at \
> diff --git a/tests/cfm.at b/tests/cfm.at
> new file mode 100644
> index 0000000..638e03f
> --- /dev/null
> +++ b/tests/cfm.at
> @@ -0,0 +1,57 @@
> +AT_BANNER([cfm])
> +
> +m4_define([CFM_CHECK_EXTENDED], [
> +AT_CHECK([ovs-appctl cfm/show $1 | sed -e '/next CCM tx:/d' | sed -e '/next
> fault check:/d' | sed -e '/recv since check:/d'],[0],
> +[dnl
> +---- $1 ----
> +MPID $2: extended
> + average health: $3
> + opstate: $4
> + remote_opstate: $5
> + interval: $6
> +Remote MPID $7
> + opstate: $8
> +])
> +])
> +
> +# test cfm under demand mode.
> +AT_SETUP([cfm - demand mode])
> +#Create 2 bridges connected by patch ports and enable BFD
> +OVS_VSWITCHD_START([add-br br1 -- \
> + set bridge br1 datapath-type=dummy \
> + other-config:hwaddr=aa:55:aa:56:00:00 -- \
> + add-port br1 p1 -- set Interface p1 type=patch \
> + options:peer=p0 -- \
> + add-port br0 p0 -- set Interface p0 type=patch \
> + options:peer=p1 -- \
> + set Interface p0 cfm_mpid=1
> other_config:cfm_interval=300 other_config:cfm_extended=true -- \
> + set Interface p1 cfm_mpid=2
> other_config:cfm_interval=300 other_config:cfm_extended=true ])
> +
> +ovs-appctl time/stop
> +# wait for a while to stablize cfm.
> +for i in `seq 0 100`; do ovs-appctl time/warp 100; done
> +CFM_CHECK_EXTENDED([p0], [1], [100], [up], [up], [300ms], [2], [up])
> +CFM_CHECK_EXTENDED([p1], [2], [100], [up], [up], [300ms], [1], [up])
> +
> +# turn on demand mode on one end.
> +AT_CHECK([ovs-vsctl set interface p0 other_config:cfm_demand=true])
> +
> +# cfm should never go down.
> +for i in `seq 0 100`
> +do
> + ovs-appctl time/warp 100
> + CFM_CHECK_EXTENDED([p0], [1], [100], [up], [up], [300ms], [2], [up])
> + CFM_CHECK_EXTENDED([p1], [2], [100], [up], [up], [300ms], [1], [up])
> +done
> +
> +# turn on demand mode on the other end.
> +AT_CHECK([ovs-vsctl set interface p1 other_config:cfm_demand=true])
> +for i in `seq 0 100`
> +do
> + ovs-appctl time/warp 100
> + CFM_CHECK_EXTENDED([p0], [1], [100], [up], [up], [300ms], [2], [up])
> + CFM_CHECK_EXTENDED([p1], [2], [100], [up], [up], [300ms], [1], [up])
> +done
> +
> +OVS_VSWITCHD_STOP
> +AT_CLEANUP
> diff --git a/tests/testsuite.at b/tests/testsuite.at
> index 43efc09..9e16f94 100644
> --- a/tests/testsuite.at
> +++ b/tests/testsuite.at
> @@ -68,6 +68,7 @@ m4_include([tests/ovsdb-macros.at])
> m4_include([tests/ofproto-macros.at])
>
> m4_include([tests/bfd.at])
> +m4_include([tests/cfm.at])
> m4_include([tests/lacp.at])
> m4_include([tests/library.at])
> m4_include([tests/heap.at])
> diff --git a/vswitchd/vswitch.xml b/vswitchd/vswitch.xml
> index 4b42ec7..5fd5b3b 100644
> --- a/vswitchd/vswitch.xml
> +++ b/vswitchd/vswitch.xml
> @@ -2142,8 +2142,9 @@
> <ul>
> <li>
> To ensure that ovs-vswitchd has enough time to pull statistics
> - from the datapath, the minimum
> - <ref column="other_config" key="cfm_interval"/> is 500ms.
> + from the datapath, the fault detection interval is set to
> + 3.5 * MAX(<ref column="other_config" key="cfm_interval"/>, 500)
> + ms.
> </li>
>
> <li>
> --
> 1.7.9.5
>
> _______________________________________________
> dev mailing list
> dev@openvswitch.org
> http://openvswitch.org/mailman/listinfo/dev
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
