Thanks Ethan,
I really should do that.
On Thu, Sep 19, 2013 at 1:27 PM, Ethan Jackson <et...@nicira.com> wrote:
> Why not just modify cfm_fault_interval() to check if demand mode is
> enabled and if so return MAX(cfm->ccm_interval, 500) * 7 / 2?
>
> Ethan
>
> On Thu, Sep 19, 2013 at 11:39 AM, 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>
> > ---
> > lib/cfm.c | 18 +++++++++-------
> > tests/automake.mk | 1 +
> > tests/cfm.at | 57
> ++++++++++++++++++++++++++++++++++++++++++++++++++
> > tests/testsuite.at | 1 +
> > vswitchd/vswitch.xml | 5 +++--
> > 5 files changed, 73 insertions(+), 9 deletions(-)
> > create mode 100644 tests/cfm.at
> >
> > diff --git a/lib/cfm.c b/lib/cfm.c
> > index 4a46c05..03dd92d 100644
> > --- a/lib/cfm.c
> > +++ b/lib/cfm.c
> > @@ -105,6 +105,8 @@ struct cfm {
> > uint32_t seq; /* The sequence number of our last CCM. */
> > uint8_t ccm_interval; /* The CCM transmission interval. */
> > int ccm_interval_ms; /* 'ccm_interval' in milliseconds. */
> > + int ccm_interval_fault_ms;/* Used to compute the fault interval, */
> > + /* in milliseconds. */
> > uint16_t ccm_vlan; /* Vlan tag of CCM PDUs. CFM_RANDOM_VLAN if
> > random. */
> > uint8_t ccm_pcp; /* Priority of CCM PDUs. */
> > @@ -255,9 +257,8 @@ 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;
> > + * We should check when (ccm_interval_fault_ms * 3.5)ms have
> passed. */
> > + return (cfm->ccm_interval_fault_ms * 7) / 2;
> > }
> >
> > static uint8_t
> > @@ -589,6 +590,7 @@ cfm_configure(struct cfm *cfm, const struct
> cfm_settings *s)
> > {
> > uint8_t interval;
> > int interval_ms;
> > + int interval_fault_ms;
> >
> > if (!cfm_is_valid_mpid(s->extended, s->mpid) || s->interval <= 0) {
> > return false;
> > @@ -598,7 +600,7 @@ cfm_configure(struct cfm *cfm, const struct
> cfm_settings *s)
> > cfm->mpid = s->mpid;
> > cfm->opup = s->opup;
> > interval = ms_to_ccm_interval(s->interval);
> > - interval_ms = ccm_interval_to_ms(interval);
> > + interval_fault_ms = interval_ms = ccm_interval_to_ms(interval);
> >
> > atomic_store(&cfm->check_tnl_key, s->check_tnl_key);
> > atomic_store(&cfm->extended, s->extended);
> > @@ -607,11 +609,11 @@ cfm_configure(struct cfm *cfm, const struct
> cfm_settings *s)
> > cfm->ccm_pcp = s->ccm_pcp & (VLAN_PCP_MASK >> VLAN_PCP_SHIFT);
> > if (s->extended && interval_ms != s->interval) {
> > interval = 0;
> > - interval_ms = MIN(s->interval, UINT16_MAX);
> > + interval_fault_ms = interval_ms = MIN(s->interval, UINT16_MAX);
> > }
> >
> > if (s->extended && s->demand) {
> > - interval_ms = MAX(interval_ms, 500);
> > + interval_fault_ms = MAX(interval_ms, 500);
> > if (!cfm->demand) {
> > cfm->demand = true;
> > cfm->rx_packets = cfm_rx_packets(cfm);
> > @@ -620,9 +622,11 @@ cfm_configure(struct cfm *cfm, const struct
> cfm_settings *s)
> > cfm->demand = false;
> > }
> >
> > - if (interval != cfm->ccm_interval || interval_ms !=
> cfm->ccm_interval_ms) {
> > + if (interval != cfm->ccm_interval || interval_ms !=
> cfm->ccm_interval_ms
> > + || interval_fault_ms != cfm->ccm_interval_fault_ms) {
> > cfm->ccm_interval = interval;
> > cfm->ccm_interval_ms = interval_ms;
> > + cfm->ccm_interval_fault_ms = interval_fault_ms;
> >
> > timer_set_expired(&cfm->tx_timer);
> > timer_set_duration(&cfm->fault_timer, cfm_fault_interval(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
