[ovs-dev] [PATCH 1/3] cfm: Require 'name' field for 'cfm' objects.

[Ethan Jackson]

This commit also fixes a memory leak upon cfm_destroy() and
converts the 'all_cfms' list to a hash map.
---
 lib/cfm.c              |   31 +++++++++++++------------------
 lib/cfm.h              |    3 +--
 ofproto/ofproto-dpif.c |    2 +-
 vswitchd/bridge.c      |    1 -
 4 files changed, 15 insertions(+), 22 deletions(-)

diff --git a/lib/cfm.c b/lib/cfm.c
index f4c0e8e..8cd1cdc 100644
--- a/lib/cfm.c
+++ b/lib/cfm.c
@@ -61,12 +61,12 @@ struct ccm {
 BUILD_ASSERT_DECL(CCM_LEN == sizeof(struct ccm));
 
 struct cfm {
-    uint16_t mpid;
-    struct list list_node; /* Node in all_cfms list. */
+    char *name;                 /* Name of this CFM object. */
+    struct hmap_node hmap_node; /* Node in all_cfms list. */
 
+    uint16_t mpid;
     bool fault;            /* Indicates connectivity fault. */
     bool recv_fault;       /* Indicates an inability to receive CCMs. */
-    char *name;            /* Name of this CFM object. */
 
     uint32_t seq;          /* The sequence number of our last CCM. */
     uint8_t ccm_interval;  /* The CCM transmission interval. */
@@ -92,7 +92,7 @@ struct remote_mp {
 };
 
 static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(5, 20);
-static struct list all_cfms = LIST_INITIALIZER(&all_cfms);
+static struct hmap all_cfms = HMAP_INITIALIZER(&all_cfms);
 
 static void cfm_unixctl_show(struct unixctl_conn *, const char *args,
                              void *aux);
@@ -201,19 +201,18 @@ cfm_init(void)
     unixctl_command_register("cfm/show", cfm_unixctl_show, NULL);
 }
 
-/* Allocates a 'cfm' object.  This object should have its 'mpid', 'maid',
- * 'eth_src', and 'interval' filled out.  cfm_configure() should be called
- * whenever changes are made to 'cfm', and before cfm_run() is called for the
- * first time. */
+/* Allocates a 'cfm' object called 'name'.  'cfm' should be initialized by
+ * cfm_configure() before use. */
 struct cfm *
-cfm_create(void)
+cfm_create(const char *name)
 {
     struct cfm *cfm;
 
     cfm = xzalloc(sizeof *cfm);
+    cfm->name = xstrdup(name);
     hmap_init(&cfm->remote_mps);
     cfm_generate_maid(cfm);
-    list_push_back(&all_cfms, &cfm->list_node);
+    hmap_insert(&all_cfms, &cfm->hmap_node, hash_string(cfm->name, 0));
     return cfm;
 }
 
@@ -232,7 +231,8 @@ cfm_destroy(struct cfm *cfm)
     }
 
     hmap_destroy(&cfm->remote_mps);
-    list_remove(&cfm->list_node);
+    hmap_remove(&all_cfms, &cfm->hmap_node);
+    free(cfm->name);
     free(cfm);
 }
 
@@ -328,11 +328,6 @@ cfm_configure(struct cfm *cfm, const struct cfm_settings *s)
     cfm->mpid = s->mpid;
     interval = ms_to_ccm_interval(s->interval);
 
-    if (!cfm->name || strcmp(s->name, cfm->name)) {
-        free(cfm->name);
-        cfm->name = xstrdup(s->name);
-    }
-
     if (interval != cfm->ccm_interval) {
         cfm->ccm_interval = interval;
         cfm->ccm_interval_ms = ccm_interval_to_ms(interval);
@@ -456,8 +451,8 @@ cfm_find(const char *name)
 {
     struct cfm *cfm;
 
-    LIST_FOR_EACH (cfm, list_node, &all_cfms) {
-        if (cfm->name && !strcmp(cfm->name, name)) {
+    HMAP_FOR_EACH_WITH_HASH (cfm, hmap_node, hash_string(name, 0), &all_cfms) {
+        if (!strcmp(cfm->name, name)) {
             return cfm;
         }
     }
diff --git a/lib/cfm.h b/lib/cfm.h
index 85e9ddb..37e158f 100644
--- a/lib/cfm.h
+++ b/lib/cfm.h
@@ -27,14 +27,13 @@ struct ofpbuf;
 struct cfm_settings {
     uint16_t mpid;              /* The MPID of this CFM. */
     int interval;               /* The requested transmission interval. */
-    const char *name;           /* Name of this CFM object. */
 
     const uint16_t *remote_mpids; /* Array of remote MPIDs */
     size_t n_remote_mpids;        /* Number of MPIDs in 'remote_mpids'. */
 };
 
 void cfm_init(void);
-struct cfm *cfm_create(void);
+struct cfm *cfm_create(const char *name);
 void cfm_destroy(struct cfm *);
 void cfm_run(struct cfm *);
 bool cfm_should_send_ccm(struct cfm *);
diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c
index 63bc379..350358f 100644
--- a/ofproto/ofproto-dpif.c
+++ b/ofproto/ofproto-dpif.c
@@ -743,7 +743,7 @@ set_cfm(struct ofport *ofport_, const struct cfm_settings *s)
         error = 0;
     } else {
         if (!ofport->cfm) {
-            ofport->cfm = cfm_create();
+            ofport->cfm = cfm_create(netdev_get_name(ofport->up.netdev));
         }
 
         if (cfm_configure(ofport->cfm, s)) {
diff --git a/vswitchd/bridge.c b/vswitchd/bridge.c
index aead346..d150857 100644
--- a/vswitchd/bridge.c
+++ b/vswitchd/bridge.c
@@ -2493,7 +2493,6 @@ iface_configure_cfm(struct iface *iface)
         return;
     }
 
-    s.name = iface->name;
     s.mpid = *cfg->cfm_mpid;
     remote_mpid = *cfg->cfm_remote_mpid;
     s.remote_mpids = &remote_mpid;
-- 
1.7.4.4