[ovs-dev] [PATCH] Added handling of previously ignored cfm faults.

[Ethan Jackson]

Thanks for doing this. It basically looks like what we want.  One question,  shouldn't we calculate the new cfm_fault flag and then set then OR it into the old recv fault flag?  It looks like with this code the new fault reasons won't cause the fault flag to be triggered.

Ethan





On Apr 4, 2012, at 19:22, Mehak Mahajan <mmahajan at nicira.com> wrote:

> The CFM packets that are out of sequence or contain invalid cfm_interval were
> previously not ignored. The behavior is changed with this patch to not
> process those CFM frames.
> 
> Signed-off-by: Mehak Mahajan <mmahajan at nicira.com>
> ---
> lib/cfm.c            |    9 ++++++++-
> lib/cfm.h            |    4 +++-
> vswitchd/vswitch.xml |   10 ++++++++++
> 3 files changed, 21 insertions(+), 2 deletions(-)
> 
> diff --git a/lib/cfm.c b/lib/cfm.c
> index 8b9e5bc..3936e5a 100644
> --- a/lib/cfm.c
> +++ b/lib/cfm.c
> @@ -535,6 +535,7 @@ cfm_process_heartbeat(struct cfm *cfm, const struct ofpbuf *p)
>         uint64_t ccm_mpid;
>         uint32_t ccm_seq;
>         bool ccm_opdown;
> +        int cfm_fault = 0;
> 
>         if (cfm->extended) {
>             ccm_mpid = ntohll(ccm->mpid64);
> @@ -546,6 +547,7 @@ cfm_process_heartbeat(struct cfm *cfm, const struct ofpbuf *p)
>         ccm_seq = ntohl(ccm->seq);
> 
>         if (ccm_interval != cfm->ccm_interval) {
> +            cfm_fault |= CFM_FAULT_INTERVAL;
>             VLOG_WARN_RL(&rl, "%s: received a CCM with an invalid interval"
>                          " (%"PRIu8") from RMP %"PRIu64, cfm->name,
>                          ccm_interval, ccm_mpid);
> @@ -553,6 +555,7 @@ cfm_process_heartbeat(struct cfm *cfm, const struct ofpbuf *p)
> 
>         if (cfm->extended && ccm_interval == 0
>             && ccm_interval_ms_x != cfm->ccm_interval_ms) {
> +            cfm_fault |= CFM_FAULT_INTERVAL;
>             VLOG_WARN_RL(&rl, "%s: received a CCM with an invalid extended"
>                          " interval (%"PRIu16"ms) from RMP %"PRIu64, cfm->name,
>                          ccm_interval_ms_x, ccm_mpid);
> @@ -565,6 +568,7 @@ cfm_process_heartbeat(struct cfm *cfm, const struct ofpbuf *p)
>                 hmap_insert(&cfm->remote_mps, &rmp->node, hash_mpid(ccm_mpid));
>             } else {
>                 cfm->recv_fault |= CFM_FAULT_OVERFLOW;
> +                cfm_fault |= CFM_FAULT_OVERFLOW;
>                 VLOG_WARN_RL(&rl,
>                              "%s: dropped CCM with MPID %"PRIu64" from MAC "
>                              ETH_ADDR_FMT, cfm->name, ccm_mpid,
> @@ -578,6 +582,7 @@ cfm_process_heartbeat(struct cfm *cfm, const struct ofpbuf *p)
> 
>         if (rmp) {
>             if (rmp->seq && ccm_seq != (rmp->seq + 1)) {
> +                cfm_fault |= CFM_FAULT_SEQUENCE;
>                 VLOG_WARN_RL(&rl, "%s: (mpid %"PRIu64") detected sequence"
>                              " numbers which indicate possible connectivity"
>                              " problems (previous %"PRIu32") (current %"PRIu32
> @@ -585,7 +590,9 @@ cfm_process_heartbeat(struct cfm *cfm, const struct ofpbuf *p)
>             }
> 
>             rmp->mpid = ccm_mpid;
> -            rmp->recv = true;
> +            if (cfm_fault) {
> +                rmp->recv = true;
> +            }
>             rmp->seq = ccm_seq;
>             rmp->rdi = ccm_rdi;
>             rmp->opup = !ccm_opdown;
> diff --git a/lib/cfm.h b/lib/cfm.h
> index 2556a32..4a84b56 100644
> --- a/lib/cfm.h
> +++ b/lib/cfm.h
> @@ -32,7 +32,9 @@ struct ofpbuf;
>     CFM_FAULT_REASON(MAID, maid)           \
>     CFM_FAULT_REASON(LOOPBACK, loopback)   \
>     CFM_FAULT_REASON(OVERFLOW, overflow)   \
> -    CFM_FAULT_REASON(OVERRIDE, override)
> +    CFM_FAULT_REASON(OVERRIDE, override)   \
> +    CFM_FAULT_REASON(INTERVAL, interval)   \
> +    CFM_FAULT_REASON(SEQUENCE, sequence)
> 
> enum cfm_fault_bit_index {
> #define CFM_FAULT_REASON(NAME, STR) CFM_FAULT_INDEX_##NAME,
> diff --git a/vswitchd/vswitch.xml b/vswitchd/vswitch.xml
> index f3ea338..55ea84a 100644
> --- a/vswitchd/vswitch.xml
> +++ b/vswitchd/vswitch.xml
> @@ -1726,6 +1726,16 @@
>         an <code>ovs-appctl</code> command.
>       </column>
> 
> +      <column name="cfm_fault_status" key="interval">
> +        Indicates a CFM fault was triggered due to the reception of a CCM
> +        frame having an invalid interval.
> +      </column>
> +
> +      <column name="cfm_fault_status" key="sequence">
> +        Indicates a CFM fault was triggered because the CFM module received
> +        a CCM frame with a sequence number that it was not expecting.
> +      </column>
> +
>       <column name="cfm_remote_mpids">
>         When CFM is properly configured, Open vSwitch will occasionally
>         receive CCM broadcasts.  These broadcasts contain the MPID of the
> -- 
> 1.7.2.5
> 
> _______________________________________________
> dev mailing list
> dev at openvswitch.org
> http://openvswitch.org/mailman/listinfo/dev