ofproto: Take responsibility for connectivity fault management (CFM).

This takes one step toward the larger goal of getting rid of all of the
hooks from ofproto back into vswitchd, by eliminating one of the reasons
that they are required.

diff --git a/ofproto/ofproto.c b/ofproto/ofproto.c
index 580f45d..e914bbf 100644 (file)
--- a/ofproto/ofproto.c
+++ b/ofproto/ofproto.c
@@ -25,6 +25,7 @@
 #include <stdbool.h>
 #include <stdlib.h>
 #include "byte-order.h"
+#include "cfm.h"
 #include "classifier.h"
 #include "coverage.h"
 #include "dpif.h"
@@ -100,9 +101,12 @@ struct ofport {
     struct netdev *netdev;
     struct ofp_phy_port opp;    /* In host byte order. */
     uint16_t odp_port;
+    struct cfm *cfm;            /* Connectivity Fault Management, if any. */
 };
 
 static void ofport_free(struct ofport *);
+static void ofport_run(struct ofproto *, struct ofport *);
+static void ofport_wait(struct ofport *);
 static void hton_ofp_phy_port(struct ofp_phy_port *);
 
 struct action_xlate_ctx {
@@ -947,7 +951,71 @@ ofproto_set_sflow(struct ofproto *ofproto,
         ofproto->sflow = NULL;
     }
 }
+\f
+/* Connectivity Fault Management configuration. */
+
+/* Clears the CFM configuration from 'port_no' on 'ofproto'. */
+void
+ofproto_iface_clear_cfm(struct ofproto *ofproto, uint32_t port_no)
+{
+    struct ofport *ofport = get_port(ofproto, port_no);
+    if (ofport && ofport->cfm){
+        cfm_destroy(ofport->cfm);
+        ofport->cfm = NULL;
+    }
+}
 
+/* Configures connectivity fault management on 'port_no' in 'ofproto'.  Takes
+ * basic configuration from the configuration members in 'cfm', and the set of
+ * remote maintenance points from the 'n_remote_mps' elements in 'remote_mps'.
+ * Ignores the statistics members of 'cfm'.
+ *
+ * This function has no effect if 'ofproto' does not have a port 'port_no'. */
+void
+ofproto_iface_set_cfm(struct ofproto *ofproto, uint32_t port_no,
+                      const struct cfm *cfm,
+                      const uint16_t *remote_mps, size_t n_remote_mps)
+{
+    struct ofport *ofport;
+
+    ofport = get_port(ofproto, port_no);
+    if (!ofport) {
+        VLOG_WARN("%s: cannot configure CFM on nonexistent port %"PRIu32,
+                  dpif_name(ofproto->dpif), port_no);
+        return;
+    }
+
+    if (!ofport->cfm) {
+        ofport->cfm = cfm_create();
+    }
+
+    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);
+
+    if (!cfm_configure(ofport->cfm)) {
+        VLOG_WARN("%s: CFM configuration on port %"PRIu32" (%s) failed",
+                  dpif_name(ofproto->dpif), port_no,
+                  netdev_get_name(ofport->netdev));
+        cfm_destroy(ofport->cfm);
+        ofport->cfm = NULL;
+    }
+}
+
+/* Returns the connectivity fault management object associated with 'port_no'
+ * within 'ofproto', or a null pointer if 'ofproto' does not have a port
+ * 'port_no' or if that port does not have CFM configured.  The caller must not
+ * modify or destroy the returned object. */
+const struct cfm *
+ofproto_iface_get_cfm(struct ofproto *ofproto, uint32_t port_no)
+{
+    struct ofport *ofport = get_port(ofproto, port_no);
+    return ofport ? ofport->cfm : NULL;
+}
+\f
 uint64_t
 ofproto_get_datapath_id(const struct ofproto *ofproto)
 {
@@ -1110,6 +1178,7 @@ ofproto_run1(struct ofproto *p)
 {
     struct ofconn *ofconn, *next_ofconn;
     struct ofservice *ofservice;
+    struct ofport *ofport;
     char *devname;
     int error;
     int i;
@@ -1146,6 +1215,10 @@ ofproto_run1(struct ofproto *p)
         process_port_change(p, error, devname);
     }
 
+    HMAP_FOR_EACH (ofport, hmap_node, &p->ports) {
+        ofport_run(p, ofport);
+    }
+
     if (p->in_band) {
         if (time_msec() >= p->next_in_band_update) {
             update_in_band_remotes(p);
@@ -1246,11 +1319,15 @@ ofproto_wait(struct ofproto *p)
 {
     struct ofservice *ofservice;
     struct ofconn *ofconn;
+    struct ofport *ofport;
     size_t i;
 
     dpif_recv_wait(p->dpif);
     dpif_port_poll_wait(p->dpif);
     netdev_monitor_poll_wait(p->netdev_monitor);
+    HMAP_FOR_EACH (ofport, hmap_node, &p->ports) {
+        ofport_wait(ofport);
+    }
     LIST_FOR_EACH (ofconn, node, &p->all_conns) {
         ofconn_wait(ofconn);
     }
@@ -1648,10 +1725,31 @@ ofport_remove(struct ofproto *p, struct ofport *ofport)
     }
 }
 
+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);
+        }
+    }
+}
+
+static void
+ofport_wait(struct ofport *ofport)
+{
+    if (ofport->cfm) {
+        cfm_wait(ofport->cfm);
+    }
+}
+
 static void
 ofport_free(struct ofport *ofport)
 {
     if (ofport) {
+        cfm_destroy(ofport->cfm);
         netdev_close(ofport->netdev);
         free(ofport);
     }
@@ -3081,6 +3179,18 @@ action_xlate_ctx_init(struct action_xlate_ctx *ctx,
     ctx->check_special = true;
 }
 
+static void
+ofproto_process_cfm(struct ofproto *ofproto, const struct flow *flow,
+                    const struct ofpbuf *packet)
+{
+    struct ofport *ofport;
+
+    ofport = get_port(ofproto, flow->in_port);
+    if (ofport && ofport->cfm) {
+        cfm_process_heartbeat(ofport->cfm, packet);
+    }
+}
+
 static struct ofpbuf *
 xlate_actions(struct action_xlate_ctx *ctx,
               const union ofp_action *in, size_t n_in)
@@ -3094,13 +3204,18 @@ xlate_actions(struct action_xlate_ctx *ctx,
     ctx->recurse = 0;
     ctx->last_pop_priority = -1;
 
-    if (!ctx->check_special
-        || !ctx->ofproto->ofhooks->special_cb
-        || ctx->ofproto->ofhooks->special_cb(&ctx->flow, ctx->packet,
-                                             ctx->ofproto->aux)) {
-        do_xlate_actions(in, n_in, ctx);
-    } else {
+    if (ctx->check_special && cfm_should_process_flow(&ctx->flow)) {
+        if (ctx->packet) {
+            ofproto_process_cfm(ctx->ofproto, &ctx->flow, ctx->packet);
+        }
         ctx->may_set_up_flow = false;
+    } else if (ctx->check_special
+               && ctx->ofproto->ofhooks->special_cb
+               && !ctx->ofproto->ofhooks->special_cb(&ctx->flow, ctx->packet,
+                                                     ctx->ofproto->aux)) {
+        ctx->may_set_up_flow = false;
+    } else {
+        do_xlate_actions(in, n_in, ctx);
     }
 
     remove_pop_action(ctx);
@@ -4375,8 +4490,12 @@ handle_miss_upcall(struct ofproto *p, struct dpif_upcall *upcall)
     /* Set header pointers in 'flow'. */
     flow_extract(upcall->packet, flow.tun_id, flow.in_port, &flow);
 
-    if (p->ofhooks->special_cb
-        && !p->ofhooks->special_cb(&flow, upcall->packet, p->aux)) {
+    if (cfm_should_process_flow(&flow)) {
+        ofproto_process_cfm(p, &flow, upcall->packet);
+        ofpbuf_delete(upcall->packet);
+        return;
+    } else if (p->ofhooks->special_cb
+               && !p->ofhooks->special_cb(&flow, upcall->packet, p->aux)) {
         ofpbuf_delete(upcall->packet);
         return;
     }

diff --git a/ofproto/ofproto.h b/ofproto/ofproto.h
index 4baf99f..b049fe1 100644 (file)
--- a/ofproto/ofproto.h
+++ b/ofproto/ofproto.h
@@ -105,7 +105,7 @@ bool ofproto_is_alive(const struct ofproto *);
 int ofproto_port_del(struct ofproto *, uint16_t odp_port);
 bool ofproto_port_is_floodable(struct ofproto *, uint16_t odp_port);
 
-/* Configuration. */
+/* Top-level configuration. */
 void ofproto_set_datapath_id(struct ofproto *, uint64_t datapath_id);
 void ofproto_set_controllers(struct ofproto *,
                              const struct ofproto_controller *, size_t n);
@@ -123,6 +123,15 @@ int ofproto_set_netflow(struct ofproto *,
                         const struct netflow_options *nf_options);
 void ofproto_set_sflow(struct ofproto *, const struct ofproto_sflow_options *);
 
+/* Configuration of individual interfaces. */
+struct cfm;
+
+void ofproto_iface_clear_cfm(struct ofproto *, uint32_t port_no);
+void ofproto_iface_set_cfm(struct ofproto *, uint32_t port_no,
+                           const struct cfm *,
+                           const uint16_t *remote_mps, size_t n_remote_mps);
+const struct cfm *ofproto_iface_get_cfm(struct ofproto *, uint32_t port_no);
+
 /* Configuration querying. */
 uint64_t ofproto_get_datapath_id(const struct ofproto *);
 bool ofproto_has_primary_controller(const struct ofproto *);

diff --git a/vswitchd/bridge.c b/vswitchd/bridge.c
index 4663463..c082e5d 100644 (file)
--- a/vswitchd/bridge.c
+++ b/vswitchd/bridge.c
@@ -112,7 +112,6 @@ struct iface {
     bool enabled;               /* May be chosen for flows? */
     bool up;                    /* Is the interface up? */
     const char *type;           /* Usually same as cfg->type. */
-    struct cfm *cfm;            /* Connectivity Fault Management */
     const struct ovsrec_interface *cfg;
 
     /* LACP information. */
@@ -1208,12 +1207,12 @@ iface_refresh_status(struct iface *iface)
 static void
 iface_refresh_cfm_stats(struct iface *iface)
 {
-    size_t i;
-    struct cfm *cfm;
     const struct ovsrec_monitor *mon;
+    const struct cfm *cfm;
+    size_t i;
 
     mon = iface->cfg->monitor;
-    cfm = iface->cfm;
+    cfm = ofproto_iface_get_cfm(iface->port->bridge->ofproto, iface->dp_ifidx);
 
     if (!cfm || !mon) {
         return;
@@ -2994,14 +2993,7 @@ bridge_special_ofhook_cb(const struct flow *flow,
 
     iface = iface_from_dp_ifidx(br, flow->in_port);
 
-    if (cfm_should_process_flow(flow)) {
-
-        if (iface && packet && iface->cfm) {
-            COVERAGE_INC(bridge_process_cfm);
-            cfm_process_heartbeat(iface->cfm, packet);
-        }
-        return false;
-    } else if (flow->dl_type == htons(ETH_TYPE_LACP)) {
+    if (flow->dl_type == htons(ETH_TYPE_LACP)) {
 
         if (iface && iface->port->lacp && packet) {
             const struct lacp_pdu *pdu = parse_lacp_packet(packet);
@@ -3854,8 +3846,6 @@ lacp_send_pdu_cb(void *aux, const struct lacp_pdu *pdu)
 static void
 port_run(struct port *port)
 {
-    size_t i;
-
     if (port->monitor) {
         char *devname;
 
@@ -3870,6 +3860,7 @@ port_run(struct port *port)
             free(devname);
         }
     } else if (time_msec() >= port->miimon_next_update) {
+        size_t i;
 
         for (i = 0; i < port->n_ifaces; i++) {
             struct iface *iface = port->ifaces[i];
@@ -3879,6 +3870,8 @@ port_run(struct port *port)
     }
 
     if (port->lacp) {
+        size_t i;
+
         for (i = 0; i < port->n_ifaces; i++) {
             struct iface *iface = port->ifaces[i];
             lacp_slave_enable(port->lacp, iface, iface->enabled);
@@ -3888,27 +3881,11 @@ port_run(struct port *port)
     }
 
     bond_run(port);
-
-    for (i = 0; i < port->n_ifaces; i++) {
-        struct iface *iface = port->ifaces[i];
-
-        if (iface->cfm) {
-            struct ofpbuf *packet = cfm_run(iface->cfm);
-            if (packet) {
-                ofproto_send_packet(port->bridge->ofproto, iface->dp_ifidx,
-                                    0, packet);
-                ofpbuf_uninit(packet);
-                free(packet);
-            }
-        }
-    }
 }
 
 static void
 port_wait(struct port *port)
 {
-    size_t i;
-
     if (port->monitor) {
         netdev_monitor_poll_wait(port->monitor);
     } else {
@@ -3920,13 +3897,6 @@ port_wait(struct port *port)
     }
 
     bond_wait(port);
-
-    for (i = 0; i < port->n_ifaces; i++) {
-        struct iface *iface = port->ifaces[i];
-        if (iface->cfm) {
-            cfm_wait(iface->cfm);
-        }
-    }
 }
 
 static struct port *
@@ -4402,8 +4372,6 @@ iface_destroy(struct iface *iface)
             bond_send_learning_packets(port);
         }
 
-        cfm_destroy(iface->cfm);
-
         free(iface->name);
         free(iface);
 
@@ -4606,7 +4574,7 @@ static void
 iface_update_cfm(struct iface *iface)
 {
     size_t i;
-    struct cfm *cfm;
+    struct cfm cfm;
     uint16_t *remote_mps;
     struct ovsrec_monitor *mon;
     uint8_t ea[ETH_ADDR_LEN], maid[CCM_MAID_LEN];
@@ -4614,8 +4582,7 @@ iface_update_cfm(struct iface *iface)
     mon = iface->cfg->monitor;
 
     if (!mon) {
-        cfm_destroy(iface->cfm);
-        iface->cfm = NULL;
+        ofproto_iface_clear_cfm(iface->port->bridge->ofproto, iface->dp_ifidx);
         return;
     }
 
@@ -4630,28 +4597,20 @@ iface_update_cfm(struct iface *iface)
         return;
     }
 
-    if (!iface->cfm) {
-        iface->cfm = cfm_create();
-    }
-
-    cfm           = iface->cfm;
-    cfm->mpid     = mon->mpid;
-    cfm->interval = mon->interval ? *mon->interval : 1000;
+    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);
+    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);
     for(i = 0; i < mon->n_remote_mps; i++) {
         remote_mps[i] = mon->remote_mps[i]->mpid;
     }
-    cfm_update_remote_mps(cfm, remote_mps, mon->n_remote_mps);
-    free(remote_mps);
 
-    if (!cfm_configure(iface->cfm)) {
-        cfm_destroy(iface->cfm);
-        iface->cfm = NULL;
-    }
+    ofproto_iface_set_cfm(iface->port->bridge->ofproto, iface->dp_ifidx,
+                          &cfm, remote_mps, mon->n_remote_mps);
+    free(remote_mps);
 }
 
 /* Read carrier or miimon status directly from 'iface''s netdev, according to