[ovs-dev] [PATCH 07/11] cfm: Remove cfm_remote_mpid configuration.

Ethan Jackson ethan at nicira.com
Wed Sep 7 02:32:30 UTC 2011


According to the 802.1ag specification, users should be able to
configure the CFM module with a list of remote endpoints with which
the local endpoint should have connectivity.  Commit 93b8df3853
"cfm: Remove Maintenance_Point and Monitor tables." changed the
behavior so that only one remote endpoint could be specified.  This
commit takes it further, by disallowing specification of any
remote endpoints.

Due to this change, the semantics of the fault flag are slightly
different.  Before, a fault was triggered if any of the configured
remote endpoints were unreachable (or with RDI), or if any
unconfigured remote endpoints were reachable.  Now a fault is
triggered if no remote endpoints are reachable at all, or if
reachable endpoints have set their RDI.

Bug #7014.
---
 lib/cfm.c                  |  111 +++++++++++++++++---------------------------
 lib/cfm.h                  |    5 +-
 vswitchd/bridge.c          |    7 +--
 vswitchd/vswitch.ovsschema |    9 +---
 vswitchd/vswitch.xml       |    7 ---
 5 files changed, 48 insertions(+), 91 deletions(-)

diff --git a/lib/cfm.c b/lib/cfm.c
index fc62079..bee8c61 100644
--- a/lib/cfm.c
+++ b/lib/cfm.c
@@ -66,7 +66,6 @@ struct cfm {
 
     uint16_t mpid;
     bool fault;            /* Indicates connectivity fault. */
-    bool recv_fault;       /* Indicates an inability to receive CCMs. */
     bool xrecv;            /* Received an unexpected CCM. */
 
     uint32_t seq;          /* The sequence number of our last CCM. */
@@ -77,7 +76,7 @@ struct cfm {
     struct timer tx_timer;    /* Send CCM when expired. */
     struct timer fault_timer; /* Check for faults when expired. */
 
-    struct hmap remote_mps; /* Expected remote MPs. */
+    struct hmap remote_mps;   /* Remote MPs. */
 };
 
 /* Remote MPs represent foreign network entities that are configured to have
@@ -87,7 +86,6 @@ struct remote_mp {
     struct hmap_node node; /* Node in 'remote_mps' map. */
 
     bool recv;           /* CCM was received since last fault check. */
-    bool fault;          /* Indicates a connectivity fault. */
     bool rdi;            /* Remote Defect Indicator. Indicates remote_mp isn't
                             receiving CCMs that it's expecting to. */
 };
@@ -183,11 +181,11 @@ cfm_is_valid_mpid(uint32_t mpid)
 }
 
 static struct remote_mp *
-lookup_remote_mp(const struct hmap *hmap, uint16_t mpid)
+lookup_remote_mp(const struct cfm *cfm, uint16_t mpid)
 {
     struct remote_mp *rmp;
 
-    HMAP_FOR_EACH_IN_BUCKET (rmp, node, hash_mpid(mpid), hmap) {
+    HMAP_FOR_EACH_IN_BUCKET (rmp, node, hash_mpid(mpid), &cfm->remote_mps) {
         if (rmp->mpid == mpid) {
             return rmp;
         }
@@ -243,32 +241,36 @@ cfm_run(struct cfm *cfm)
 {
     if (timer_expired(&cfm->fault_timer)) {
         long long int interval = cfm_fault_interval(cfm);
-        struct remote_mp *rmp;
+        struct remote_mp *rmp, *rmp_next;
 
         cfm->fault = cfm->xrecv;
-        cfm->recv_fault = false;
         cfm->xrecv = false;
+        HMAP_FOR_EACH_SAFE (rmp, rmp_next, node, &cfm->remote_mps) {
 
-        HMAP_FOR_EACH (rmp, node, &cfm->remote_mps) {
-            rmp->fault = !rmp->recv;
-            rmp->recv = false;
-
-            if (rmp->fault) {
-                cfm->recv_fault = true;
+            if (!rmp->recv) {
                 VLOG_DBG("%s: No CCM from RMP %"PRIu16" in the last %lldms",
                          cfm->name, rmp->mpid, interval);
-            } else if (rmp->rdi) {
-                cfm->fault = true;
-                VLOG_DBG("%s: RDI bit flagged from RMP %"PRIu16, cfm->name,
-                         rmp->mpid);
+                hmap_remove(&cfm->remote_mps, &rmp->node);
+                free(rmp);
+            } else {
+                rmp->recv = false;
+
+                if (rmp->mpid == cfm->mpid) {
+                    VLOG_WARN_RL(&rl, "%s: Received CCM with local MPID (%d)",
+                                 cfm->name, rmp->mpid);
+                    cfm->fault = true;
+                }
+
+                if (rmp->rdi) {
+                    VLOG_DBG("%s: RDI bit flagged from RMP %"PRIu16, cfm->name,
+                             rmp->mpid);
+                    cfm->fault = true;
+                }
             }
         }
 
-        if (cfm->recv_fault) {
+        if (hmap_is_empty(&cfm->remote_mps)) {
             cfm->fault = true;
-        } else {
-            VLOG_DBG("%s: All RMPs received CCMs in the last %lldms",
-                     cfm->name, interval);
         }
 
         timer_set_duration(&cfm->fault_timer, interval);
@@ -304,7 +306,7 @@ cfm_compose_ccm(struct cfm *cfm, struct ofpbuf *packet,
     memcpy(ccm->maid, cfm->maid, sizeof ccm->maid);
     memset(ccm->zero, 0, sizeof ccm->zero);
 
-    if (cfm->recv_fault) {
+    if (hmap_is_empty(&cfm->remote_mps)) {
         ccm->flags |= CCM_RDI_MASK;
     }
 }
@@ -321,13 +323,9 @@ cfm_wait(struct cfm *cfm)
 bool
 cfm_configure(struct cfm *cfm, const struct cfm_settings *s)
 {
-    size_t i;
     uint8_t interval;
-    struct hmap new_rmps;
-    struct remote_mp *rmp, *rmp_next;
 
-    if (!cfm_is_valid_mpid(s->mpid) || s->interval <= 0
-        || s->n_remote_mpids <= 0) {
+    if (!cfm_is_valid_mpid(s->mpid) || s->interval <= 0) {
         return false;
     }
 
@@ -342,32 +340,6 @@ cfm_configure(struct cfm *cfm, const struct cfm_settings *s)
         timer_set_duration(&cfm->fault_timer, cfm_fault_interval(cfm));
     }
 
-    hmap_init(&new_rmps);
-    for (i = 0; i < s->n_remote_mpids; i++) {
-        uint16_t mpid = s->remote_mpids[i];
-
-        if (!cfm_is_valid_mpid(mpid)
-            || lookup_remote_mp(&new_rmps, mpid)) {
-            continue;
-        }
-
-        if ((rmp = lookup_remote_mp(&cfm->remote_mps, mpid))) {
-            hmap_remove(&cfm->remote_mps, &rmp->node);
-        } else {
-            rmp = xzalloc(sizeof *rmp);
-            rmp->mpid = mpid;
-        }
-
-        hmap_insert(&new_rmps, &rmp->node, hash_mpid(mpid));
-    }
-
-    hmap_swap(&new_rmps, &cfm->remote_mps);
-    HMAP_FOR_EACH_SAFE (rmp, rmp_next, node, &new_rmps) {
-        hmap_remove(&new_rmps, &rmp->node);
-        free(rmp);
-    }
-    hmap_destroy(&new_rmps);
-
     return true;
 }
 
@@ -425,21 +397,25 @@ cfm_process_heartbeat(struct cfm *cfm, const struct ofpbuf *p)
         ccm_interval = ccm->flags & 0x7;
         ccm_rdi = ccm->flags & CCM_RDI_MASK;
 
-        rmp = lookup_remote_mp(&cfm->remote_mps, ccm_mpid);
+        if (ccm_interval != cfm->ccm_interval) {
+            VLOG_WARN_RL(&rl, "%s: received a CCM with an invalid interval"
+                         " (%"PRIu8") from RMP %"PRIu16, cfm->name,
+                         ccm_interval, ccm_mpid);
+        }
 
+        rmp = lookup_remote_mp(cfm, ccm_mpid);
         if (rmp) {
             rmp->recv = true;
             rmp->rdi = ccm_rdi;
-
-            if (ccm_interval != cfm->ccm_interval) {
-                VLOG_WARN_RL(&rl, "%s: received a CCM with an invalid interval"
-                             " (%"PRIu8") from RMP %"PRIu16, cfm->name,
-                             ccm_interval, rmp->mpid);
-            }
+        } else if (hmap_count(&cfm->remote_mps) < CFM_MAX_RMPS) {
+            rmp = xmalloc(sizeof *rmp);
+            rmp->mpid = ccm_mpid;
+            rmp->recv = true;
+            rmp->rdi = ccm_rdi;
+            hmap_insert(&cfm->remote_mps, &rmp->node, hash_mpid(rmp->mpid));
         } else {
-            cfm->xrecv = true;
-            VLOG_WARN_RL(&rl, "%s: Received unexpected remote MPID %d from"
-                         " MAC " ETH_ADDR_FMT, cfm->name, ccm_mpid,
+            VLOG_WARN_RL(&rl, "%s: Dropped CCM with MPID %d from MAC "
+                         ETH_ADDR_FMT, cfm->name, ccm_mpid,
                          ETH_ADDR_ARGS(eth->eth_src));
         }
 
@@ -484,10 +460,9 @@ cfm_unixctl_show(struct unixctl_conn *conn,
         return;
     }
 
-    ds_put_format(&ds, "MPID %"PRIu16":%s%s%s\n", cfm->mpid,
+    ds_put_format(&ds, "MPID %"PRIu16":%s%s\n", cfm->mpid,
                   cfm->fault ? " fault" : "",
-                  cfm->xrecv ? " xrecv" : "",
-                  cfm->recv_fault ? " recv_fault" : "");
+                  cfm->xrecv ? " xrecv" : "");
 
     ds_put_format(&ds, "\tinterval: %dms\n", cfm->ccm_interval_ms);
     ds_put_format(&ds, "\tnext CCM tx: %lldms\n",
@@ -497,8 +472,8 @@ cfm_unixctl_show(struct unixctl_conn *conn,
 
     ds_put_cstr(&ds, "\n");
     HMAP_FOR_EACH (rmp, node, &cfm->remote_mps) {
-        ds_put_format(&ds, "Remote MPID %"PRIu16": %s\n", rmp->mpid,
-                      rmp->fault ? "fault" : "");
+        ds_put_format(&ds, "Remote MPID %"PRIu16":%s\n", rmp->mpid,
+                      rmp->rdi ? " rdi" : "");
         ds_put_format(&ds, "\trecv since check: %s",
                       rmp->recv ? "true" : "false");
     }
diff --git a/lib/cfm.h b/lib/cfm.h
index 37e158f..f1ab638 100644
--- a/lib/cfm.h
+++ b/lib/cfm.h
@@ -21,15 +21,14 @@
 #include "hmap.h"
 #include "openvswitch/types.h"
 
+#define CFM_MAX_RMPS 256
+
 struct flow;
 struct ofpbuf;
 
 struct cfm_settings {
     uint16_t mpid;              /* The MPID of this CFM. */
     int interval;               /* The requested transmission interval. */
-
-    const uint16_t *remote_mpids; /* Array of remote MPIDs */
-    size_t n_remote_mpids;        /* Number of MPIDs in 'remote_mpids'. */
 };
 
 void cfm_init(void);
diff --git a/vswitchd/bridge.c b/vswitchd/bridge.c
index a348e15..adfc3b6 100644
--- a/vswitchd/bridge.c
+++ b/vswitchd/bridge.c
@@ -2591,18 +2591,13 @@ iface_configure_cfm(struct iface *iface)
 {
     const struct ovsrec_interface *cfg = iface->cfg;
     struct cfm_settings s;
-    uint16_t remote_mpid;
 
-    if (!cfg->n_cfm_mpid || !cfg->n_cfm_remote_mpid) {
+    if (!cfg->n_cfm_mpid) {
         ofproto_port_clear_cfm(iface->port->bridge->ofproto, iface->ofp_port);
         return;
     }
 
     s.mpid = *cfg->cfm_mpid;
-    remote_mpid = *cfg->cfm_remote_mpid;
-    s.remote_mpids = &remote_mpid;
-    s.n_remote_mpids = 1;
-
     s.interval = atoi(get_interface_other_config(iface->cfg, "cfm_interval",
                                                  "0"));
     if (s.interval <= 0) {
diff --git a/vswitchd/vswitch.ovsschema b/vswitchd/vswitch.ovsschema
index ca61a2c..43ff95d 100644
--- a/vswitchd/vswitch.ovsschema
+++ b/vswitchd/vswitch.ovsschema
@@ -1,6 +1,6 @@
 {"name": "Open_vSwitch",
- "version": "5.2.0",
- "cksum": "434778864 14545",
+ "version": "6.0.0",
+ "cksum": "1100213054 14376",
  "tables": {
    "Open_vSwitch": {
      "columns": {
@@ -171,11 +171,6 @@
            "key": {"type": "integer", "minInteger": 1, "maxInteger": 8191},
            "min": 0,
            "max": 1}},
-       "cfm_remote_mpid": {
-         "type" : {
-           "key": { "type": "integer", "minInteger": 1, "maxInteger": 8191},
-           "min": 0,
-           "max": 1}},
        "cfm_fault": {
          "type": {
            "key": { "type": "boolean"},
diff --git a/vswitchd/vswitch.xml b/vswitchd/vswitch.xml
index c058721..8a74bfa 100644
--- a/vswitchd/vswitch.xml
+++ b/vswitchd/vswitch.xml
@@ -1340,13 +1340,6 @@
         CFM on this <ref table="Interface"/>.
       </column>
 
-      <column name="cfm_remote_mpid">
-        The MPID of the remote endpoint being monitored.  If this
-        <ref table="Interface"/> does not have connectivity to an endpoint
-        advertising the configured MPID, a fault is signalled.  Must be
-        configured to enable CFM on this <ref table="Interface"/>
-      </column>
-
       <column name="cfm_fault">
         Indicates a connectivity fault triggered by an inability to receive
         heartbeats from the remote endpoint.  When a fault is triggered on
-- 
1.7.6.1




More information about the dev mailing list