[ovs-dev] [PATCH 3/6] cfm: Don't report unexpected remote endpoints.

Ethan Jackson ethan at nicira.com
Fri Mar 25 23:39:26 UTC 2011


Before this patch, CFM would report unexpected remote maintenance
points in the database.  This commit no longer exposes this
information.

Information about precisely why a link is faulty is more interesting
to a system administrator debugging a problem than a controller
which will generally only care about whether or not a link is
faulty.  For simplicity sake, this commit removes this information
from the database where it was somewhat awkwardly placed.  In the
future it may be valuable to report the information through
ovs-appctl commands for debugging purposes.
---
 lib/cfm.c                  |   47 +++++++++++++++++++++++------------------
 lib/cfm.h                  |    2 -
 vswitchd/bridge.c          |   49 --------------------------------------------
 vswitchd/vswitch.ovsschema |   16 +------------
 vswitchd/vswitch.xml       |   16 --------------
 5 files changed, 28 insertions(+), 102 deletions(-)

diff --git a/lib/cfm.c b/lib/cfm.c
index 5c969e4..71594e2 100644
--- a/lib/cfm.c
+++ b/lib/cfm.c
@@ -43,6 +43,9 @@ struct cfm_internal {
 
     long long ccm_sent;    /* The time we last sent a CCM. */
     long long fault_check; /* The time we last checked for faults. */
+
+    struct hmap x_remote_mps;   /* Unexpected remote MPs. */
+    struct hmap x_remote_maids; /* Unexpected remote MAIDs. */
 };
 
 static int
@@ -137,14 +140,15 @@ cfm_create(void)
     cfm  = &cfmi->cfm;
 
     hmap_init(&cfm->remote_mps);
-    hmap_init(&cfm->x_remote_mps);
-    hmap_init(&cfm->x_remote_maids);
+    hmap_init(&cfmi->x_remote_mps);
+    hmap_init(&cfmi->x_remote_maids);
     return cfm;
 }
 
 void
 cfm_destroy(struct cfm *cfm)
 {
+    struct cfm_internal *cfmi = cfm_to_internal(cfm);
     struct remote_mp *rmp, *rmp_next;
     struct remote_maid *rmaid, *rmaid_next;
 
@@ -157,20 +161,20 @@ cfm_destroy(struct cfm *cfm)
         free(rmp);
     }
 
-    HMAP_FOR_EACH_SAFE (rmp, rmp_next, node, &cfm->x_remote_mps) {
-        hmap_remove(&cfm->x_remote_mps, &rmp->node);
+    HMAP_FOR_EACH_SAFE (rmp, rmp_next, node, &cfmi->x_remote_mps) {
+        hmap_remove(&cfmi->x_remote_mps, &rmp->node);
         free(rmp);
     }
 
-    HMAP_FOR_EACH_SAFE (rmaid, rmaid_next, node, &cfm->x_remote_maids) {
-        hmap_remove(&cfm->x_remote_maids, &rmaid->node);
+    HMAP_FOR_EACH_SAFE (rmaid, rmaid_next, node, &cfmi->x_remote_maids) {
+        hmap_remove(&cfmi->x_remote_maids, &rmaid->node);
         free(rmaid);
     }
 
     hmap_destroy(&cfm->remote_mps);
-    hmap_destroy(&cfm->x_remote_mps);
-    hmap_destroy(&cfm->x_remote_maids);
-    free(cfm_to_internal(cfm));
+    hmap_destroy(&cfmi->x_remote_mps);
+    hmap_destroy(&cfmi->x_remote_maids);
+    free(cfmi);
 }
 
 /* Should be run periodically to update fault statistics messages. */
@@ -201,22 +205,22 @@ cfm_run(struct cfm *cfm)
             fault      = rmp->fault || fault;
         }
 
-        HMAP_FOR_EACH_SAFE (rmp, rmp_next, node, &cfm->x_remote_mps) {
+        HMAP_FOR_EACH_SAFE (rmp, rmp_next, node, &cfmi->x_remote_mps) {
             if (cfmi->fault_check > rmp->recv_time) {
-                hmap_remove(&cfm->x_remote_mps, &rmp->node);
+                hmap_remove(&cfmi->x_remote_mps, &rmp->node);
                 free(rmp);
             }
         }
 
-        HMAP_FOR_EACH_SAFE (rmaid, rmaid_next, node, &cfm->x_remote_maids) {
+        HMAP_FOR_EACH_SAFE (rmaid, rmaid_next, node, &cfmi->x_remote_maids) {
             if (cfmi->fault_check > rmaid->recv_time) {
-                hmap_remove(&cfm->x_remote_maids, &rmaid->node);
+                hmap_remove(&cfmi->x_remote_maids, &rmaid->node);
                 free(rmaid);
             }
         }
 
-        fault = (fault || !hmap_is_empty(&cfm->x_remote_mps)
-                 || !hmap_is_empty(&cfm->x_remote_maids));
+        fault = (fault || !hmap_is_empty(&cfmi->x_remote_mps)
+                 || !hmap_is_empty(&cfmi->x_remote_maids));
 
         cfm->fault        = fault;
         cfmi->fault_check = now;
@@ -288,6 +292,7 @@ cfm_configure(struct cfm *cfm)
 void
 cfm_update_remote_mps(struct cfm *cfm, const uint16_t *mpids, size_t n_mpids)
 {
+    struct cfm_internal *cfmi = cfm_to_internal(cfm);
     size_t i;
     struct hmap new_rmps;
     struct remote_mp *rmp, *rmp_next;
@@ -304,8 +309,8 @@ cfm_update_remote_mps(struct cfm *cfm, const uint16_t *mpids, size_t n_mpids)
 
         if ((rmp = lookup_remote_mp(&cfm->remote_mps, mpid))) {
             hmap_remove(&cfm->remote_mps, &rmp->node);
-        } else if ((rmp = lookup_remote_mp(&cfm->x_remote_mps, mpid))) {
-            hmap_remove(&cfm->x_remote_mps, &rmp->node);
+        } else if ((rmp = lookup_remote_mp(&cfmi->x_remote_mps, mpid))) {
+            hmap_remove(&cfmi->x_remote_mps, &rmp->node);
         } else {
             rmp = xzalloc(sizeof *rmp);
             rmp->mpid = mpid;
@@ -408,14 +413,14 @@ cfm_process_heartbeat(struct cfm *cfm, const struct ofpbuf *p)
     if (memcmp(ccm->maid, cfm->maid, sizeof ccm->maid)) {
         struct remote_maid *rmaid;
 
-        rmaid = lookup_remote_maid(&cfm->x_remote_maids, ccm->maid);
+        rmaid = lookup_remote_maid(&cfmi->x_remote_maids, ccm->maid);
         if (rmaid) {
             rmaid->recv_time = time_msec();
         } else {
             rmaid = xzalloc(sizeof *rmaid);
             rmaid->recv_time = time_msec();
             memcpy(rmaid->maid, ccm->maid, sizeof rmaid->maid);
-            hmap_insert(&cfm->x_remote_maids, &rmaid->node,
+            hmap_insert(&cfmi->x_remote_maids, &rmaid->node,
                         hash_bytes(ccm->maid, CCM_MAID_LEN, 0));
         }
         cfm->fault = true;
@@ -427,13 +432,13 @@ cfm_process_heartbeat(struct cfm *cfm, const struct ofpbuf *p)
         rmp = lookup_remote_mp(&cfm->remote_mps, ccm_mpid);
 
         if (!rmp) {
-            rmp = lookup_remote_mp(&cfm->x_remote_mps, ccm_mpid);
+            rmp = lookup_remote_mp(&cfmi->x_remote_mps, ccm_mpid);
         }
 
         if (!rmp) {
             rmp = xzalloc(sizeof *rmp);
             rmp->mpid = ccm_mpid;
-            hmap_insert(&cfm->x_remote_mps, &rmp->node, hash_mpid(ccm_mpid));
+            hmap_insert(&cfmi->x_remote_mps, &rmp->node, hash_mpid(ccm_mpid));
             rmp->fault = true;
         }
 
diff --git a/lib/cfm.h b/lib/cfm.h
index 651b1c4..e4bb6df 100644
--- a/lib/cfm.h
+++ b/lib/cfm.h
@@ -59,8 +59,6 @@ struct cfm {
 
     /* Statistics. */
     struct hmap remote_mps;     /* Expected remote MPs. */
-    struct hmap x_remote_mps;   /* Unexpected remote MPs. */
-    struct hmap x_remote_maids; /* Unexpected remote MAIDs. */
     bool fault;                 /* Indicates connectivity vaults. */
 };
 
diff --git a/vswitchd/bridge.c b/vswitchd/bridge.c
index b57860f..96e4cf1 100644
--- a/vswitchd/bridge.c
+++ b/vswitchd/bridge.c
@@ -1220,55 +1220,6 @@ iface_refresh_cfm_stats(struct iface *iface)
         ovsrec_maintenance_point_set_fault(mp, &rmp->fault, 1);
     }
 
-    if (hmap_is_empty(&cfm->x_remote_mps)) {
-        ovsrec_monitor_set_unexpected_remote_mpids(mon, NULL, 0);
-    } else {
-        size_t length;
-        struct remote_mp *rmp;
-        int64_t *x_remote_mps;
-
-        length = hmap_count(&cfm->x_remote_mps);
-        x_remote_mps = xzalloc(length * sizeof *x_remote_mps);
-
-        i = 0;
-        HMAP_FOR_EACH (rmp, node, &cfm->x_remote_mps) {
-            x_remote_mps[i++] = rmp->mpid;
-        }
-
-        ovsrec_monitor_set_unexpected_remote_mpids(mon, x_remote_mps, length);
-        free(x_remote_mps);
-    }
-
-    if (hmap_is_empty(&cfm->x_remote_maids)) {
-        ovsrec_monitor_set_unexpected_remote_maids(mon, NULL, 0);
-    } else {
-        size_t length;
-        char **x_remote_maids;
-        struct remote_maid *rmaid;
-
-        length = hmap_count(&cfm->x_remote_maids);
-        x_remote_maids = xzalloc(length * sizeof *x_remote_maids);
-
-        i = 0;
-        HMAP_FOR_EACH (rmaid, node, &cfm->x_remote_maids) {
-            size_t j;
-
-            x_remote_maids[i] = xzalloc(CCM_MAID_LEN * 2 + 1);
-
-            for (j = 0; j < CCM_MAID_LEN; j++) {
-                 snprintf(&x_remote_maids[i][j * 2], 3, "%02hhx",
-                          rmaid->maid[j]);
-            }
-            i++;
-        }
-        ovsrec_monitor_set_unexpected_remote_maids(mon, x_remote_maids, length);
-
-        for (i = 0; i < length; i++) {
-            free(x_remote_maids[i]);
-        }
-        free(x_remote_maids);
-    }
-
     ovsrec_monitor_set_fault(mon, &cfm->fault, 1);
 }
 
diff --git a/vswitchd/vswitch.ovsschema b/vswitchd/vswitch.ovsschema
index 98e4a10..5afb606 100644
--- a/vswitchd/vswitch.ovsschema
+++ b/vswitchd/vswitch.ovsschema
@@ -1,6 +1,6 @@
 {"name": "Open_vSwitch",
- "version": "3.0.1",
- "cksum": "1940448373 15497",
+ "version": "3.1.0",
+ "cksum": "2572059147 15155",
  "tables": {
    "Open_vSwitch": {
      "columns": {
@@ -221,18 +221,6 @@
            "key": { "type": "uuid", "refTable": "Maintenance_Point"},
            "min": 0,
            "max": "unlimited"}},
-       "unexpected_remote_mpids": {
-         "type": {
-           "key": { "type": "integer"},
-           "min": 0,
-           "max": "unlimited"},
-         "ephemeral": true},
-       "unexpected_remote_maids": {
-         "type": {
-           "key": "string",
-           "min": 0,
-           "max": "unlimited"},
-         "ephemeral": true},
        "fault": {
          "type": {
            "key": { "type": "boolean"},
diff --git a/vswitchd/vswitch.xml b/vswitchd/vswitch.xml
index 36d08a4..b74c55c 100644
--- a/vswitchd/vswitch.xml
+++ b/vswitchd/vswitch.xml
@@ -1482,22 +1482,6 @@
     </group>
 
     <group title="Monitor Status">
-      <column name="unexpected_remote_mpids">
-        A set of MPIDs representing MPs to which this <ref table="Monitor"/>
-        has detected connectivity that are not in the
-        <ref column="remote_mps"/> set.  This <ref table="Monitor"/> should not
-        have connectivity to any MPs not listed in <ref column="remote_mps"/>.
-        Thus, if this set is non-empty a fault is indicated.
-      </column>
-
-      <column name="unexpected_remote_maids">
-        A set of MAIDs representing foreign Maintenance Associations (MAs)
-        which this <ref table="Monitor"/> has detected connectivity to. A
-        <ref table="Monitor"/> should not have connectivity to a Maintenance
-        Association other than its own.  Thus, if this set is non-empty a fault
-        is indicated.
-      </column>
-
       <column name="fault">
         Indicates a Connectivity Fault caused by a configuration error, a down
         remote MP, or unexpected connectivity to a remote MAID or remote MP.
-- 
1.7.4.1




More information about the dev mailing list