cfm: cfm_run() return ccm instead of packet.

It doesn't really make sense for the CFM code to be composing
packets.  Its caller is better placed to compose the appropriate
L2 header.  This commit pulls that logic out of the CFM library.

diff --git a/lib/cfm.c b/lib/cfm.c
index 683cc71..1626514 100644 (file)
--- a/lib/cfm.c
+++ b/lib/cfm.c
@@ -110,34 +110,6 @@ lookup_remote_mp(const struct hmap *hmap, uint16_t mpid)
     return NULL;
 }
 
-static struct ofpbuf *
-compose_ccm(struct cfm_internal *cfmi)
-{
-    struct ccm *ccm;
-    struct ofpbuf *packet;
-    struct eth_header *eth;
-
-    packet = ofpbuf_new(ETH_HEADER_LEN + CCM_LEN + 2);
-
-    ofpbuf_reserve(packet, 2);
-
-    eth = ofpbuf_put_zeros(packet, ETH_HEADER_LEN);
-    ccm = ofpbuf_put_zeros(packet, CCM_LEN);
-
-    memcpy(eth->eth_dst, eth_addr_ccm, ETH_ADDR_LEN);
-    memcpy(eth->eth_src, cfmi->cfm.eth_src, sizeof eth->eth_src);
-    eth->eth_type = htons(ETH_TYPE_CFM);
-
-    ccm->mdlevel_version = 0;
-    ccm->opcode          = CCM_OPCODE;
-    ccm->tlv_offset      = 70;
-    ccm->seq             = htonl(++cfmi->seq);
-    ccm->mpid            = htons(cfmi->cfm.mpid);
-    ccm->flags           = cfmi->ccm_interval;
-    memcpy(ccm->maid, cfmi->cfm.maid, sizeof ccm->maid);
-    return packet;
-}
-
 /* Allocates a 'cfm' object.  This object should have its 'mpid', 'maid',
  * 'eth_src', and 'interval' filled out.  When changes are made to the 'cfm'
  * object, cfm_configure should be called before using it. */
@@ -187,10 +159,8 @@ cfm_destroy(struct cfm *cfm)
     free(cfm_to_internal(cfm));
 }
 
-/* Should be run periodically to update fault statistics and generate CCM
- * messages.  If necessary, returns a packet which the caller is responsible
- * for sending, un-initing, and deallocating.  Otherwise returns NULL. */
-struct ofpbuf *
+/* Should be run periodically to update fault statistics messages. */
+void
 cfm_run(struct cfm *cfm)
 {
     long long now = time_msec();
@@ -237,13 +207,34 @@ cfm_run(struct cfm *cfm)
         cfm->fault        = fault;
         cfmi->fault_check = now;
     }
+}
 
-    if (now >= cfmi->ccm_sent + cfmi->ccm_interval_ms) {
-        cfmi->ccm_sent = now;
-        return compose_ccm(cfmi);
-    }
+/* Should be run periodically to check if the CFM module has a CCM message it
+ * wishes to send. */
+bool
+cfm_should_send_ccm(struct cfm *cfm)
+{
+    struct cfm_internal *cfmi = cfm_to_internal(cfm);
 
-    return NULL;
+    return time_msec() >= cfmi->ccm_sent + cfmi->ccm_interval_ms;
+}
+
+/* Composes a CCM message into 'ccm'.  Messages generated with this function
+ * should be sent whenever cfm_should_send_ccm() indicates. */
+void
+cfm_compose_ccm(struct cfm *cfm, struct ccm *ccm)
+{
+    struct cfm_internal *cfmi = cfm_to_internal(cfm);
+
+    cfmi->ccm_sent = time_msec();
+
+    ccm->mdlevel_version = 0;
+    ccm->opcode = CCM_OPCODE;
+    ccm->tlv_offset = 70;
+    ccm->seq = htonl(++cfmi->seq);
+    ccm->mpid = htons(cfmi->cfm.mpid);
+    ccm->flags = cfmi->ccm_interval;
+    memcpy(ccm->maid, cfmi->cfm.maid, sizeof ccm->maid);
 }
 
 void

diff --git a/lib/cfm.h b/lib/cfm.h
index 71778b7..555e39b 100644 (file)
--- a/lib/cfm.h
+++ b/lib/cfm.h
@@ -36,7 +36,6 @@ struct cfm {
     uint16_t mpid;              /* The MPID of this CFM. */
     uint8_t maid[CCM_MAID_LEN]; /* The MAID of this CFM. */
     int interval;               /* The requested transmission interval. */
-    uint8_t eth_src[ETH_ADDR_LEN];
 
     /* Statistics. */
     struct hmap remote_mps;     /* Expected remote MPs. */
@@ -68,7 +67,11 @@ struct cfm *cfm_create(void);
 
 void cfm_destroy(struct cfm *);
 
-struct ofpbuf *cfm_run(struct cfm *);
+void cfm_run(struct cfm *);
+
+bool cfm_should_send_ccm(struct cfm *);
+
+void cfm_compose_ccm(struct cfm *, struct ccm *);
 
 void cfm_wait(struct cfm *);
 

diff --git a/ofproto/ofproto.c b/ofproto/ofproto.c
index 72ec8dd..e0715b9 100644 (file)
--- a/ofproto/ofproto.c
+++ b/ofproto/ofproto.c
@@ -991,7 +991,6 @@ ofproto_iface_set_cfm(struct ofproto *ofproto, uint32_t port_no,
 
     ofport->cfm->mpid = cfm->mpid;
     ofport->cfm->interval = cfm->interval;
-    memcpy(ofport->cfm->eth_src, cfm->eth_src, ETH_ADDR_LEN);
     memcpy(ofport->cfm->maid, cfm->maid, CCM_MAID_LEN);
 
     cfm_update_remote_mps(ofport->cfm, remote_mps, n_remote_mps);
@@ -1729,10 +1728,18 @@ static void
 ofport_run(struct ofproto *ofproto, struct ofport *ofport)
 {
     if (ofport->cfm) {
-        struct ofpbuf *packet = cfm_run(ofport->cfm);
-        if (packet) {
-            ofproto_send_packet(ofproto, ofport->odp_port, 0, packet);
-            ofpbuf_delete(packet);
+        cfm_run(ofport->cfm);
+
+        if (cfm_should_send_ccm(ofport->cfm)) {
+            struct ofpbuf packet;
+            struct ccm *ccm;
+
+            ofpbuf_init(&packet, 0);
+            ccm = compose_packet(&packet, eth_addr_ccm, ofport->opp.hw_addr,
+                                 ETH_TYPE_CFM,  sizeof *ccm);
+            cfm_compose_ccm(ofport->cfm, ccm);
+            ofproto_send_packet(ofproto, ofport->odp_port, 0, &packet);
+            ofpbuf_uninit(&packet);
         }
     }
 }

diff --git a/vswitchd/bridge.c b/vswitchd/bridge.c
index 455bc58..b57860f 100644 (file)
--- a/vswitchd/bridge.c
+++ b/vswitchd/bridge.c
@@ -4585,7 +4585,7 @@ iface_update_cfm(struct iface *iface)
     struct cfm cfm;
     uint16_t *remote_mps;
     struct ovsrec_monitor *mon;
-    uint8_t ea[ETH_ADDR_LEN], maid[CCM_MAID_LEN];
+    uint8_t maid[CCM_MAID_LEN];
 
     mon = iface->cfg->monitor;
 
@@ -4594,12 +4594,6 @@ iface_update_cfm(struct iface *iface)
         return;
     }
 
-    if (netdev_get_etheraddr(iface->netdev, ea)) {
-        VLOG_WARN("interface %s: Failed to get ethernet address. "
-                  "Skipping Monitor.", iface->name);
-        return;
-    }
-
     if (!cfm_generate_maid(mon->md_name, mon->ma_name, maid)) {
         VLOG_WARN("interface %s: Failed to generate MAID.", iface->name);
         return;
@@ -4608,7 +4602,6 @@ iface_update_cfm(struct iface *iface)
     cfm.mpid     = mon->mpid;
     cfm.interval = mon->interval ? *mon->interval : 1000;
 
-    memcpy(cfm.eth_src, ea, sizeof cfm.eth_src);
     memcpy(cfm.maid, maid, sizeof cfm.maid);
 
     remote_mps = xzalloc(mon->n_remote_mps * sizeof *remote_mps);