[ovs-dev] [PATCH 4/5] ofproto-dpif-monitor: Add ofproto-dpif-monitor module.

Alex Wang alexw at nicira.com
Tue Oct 1 20:36:14 UTC 2013


This commit adds a new module ofproto-dpif-monitor in ofproto
directory.  This module is in charge of executing the periodic
functions of monitoring code (e.g. bfd and cfm).

Signed-off-by: Alex Wang <alexw at nicira.com>
---
 ofproto/automake.mk            |    2 +
 ofproto/ofproto-dpif-monitor.c |  203 ++++++++++++++++++++++++++++++++++++++++
 ofproto/ofproto-dpif-monitor.h |   33 +++++++
 ofproto/ofproto-dpif-xlate.c   |   20 ++++
 ofproto/ofproto-dpif-xlate.h   |    2 +
 ofproto/ofproto-dpif.c         |   56 ++---------
 tests/bfd.at                   |   30 ++++--
 7 files changed, 286 insertions(+), 60 deletions(-)
 create mode 100644 ofproto/ofproto-dpif-monitor.c
 create mode 100644 ofproto/ofproto-dpif-monitor.h

diff --git a/ofproto/automake.mk b/ofproto/automake.mk
index 47ca1b8..432f083 100644
--- a/ofproto/automake.mk
+++ b/ofproto/automake.mk
@@ -28,6 +28,8 @@ ofproto_libofproto_a_SOURCES = \
 	ofproto/ofproto-dpif-ipfix.h \
 	ofproto/ofproto-dpif-mirror.c \
 	ofproto/ofproto-dpif-mirror.h \
+	ofproto/ofproto-dpif-monitor.c \
+	ofproto/ofproto-dpif-monitor.h \
 	ofproto/ofproto-dpif-sflow.c \
 	ofproto/ofproto-dpif-sflow.h \
 	ofproto/ofproto-dpif-upcall.c \
diff --git a/ofproto/ofproto-dpif-monitor.c b/ofproto/ofproto-dpif-monitor.c
new file mode 100644
index 0000000..49210fe
--- /dev/null
+++ b/ofproto/ofproto-dpif-monitor.c
@@ -0,0 +1,203 @@
+/*
+ * Copyright (c) 2009, 2010, 2011, 2012, 2013 Nicira, Inc.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at:
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#include <config.h>
+#include "ofproto-dpif-monitor.h"
+
+#include <string.h>
+
+#include "bfd.h"
+#include "cfm.h"
+#include "hash.h"
+#include "hmap.h"
+#include "ofpbuf.h"
+#include "ofproto-dpif.h"
+#include "util.h"
+#include "vlog.h"
+
+/* Monitored port.  It contains references to ofport, bfd, cfm structs. */
+struct mport {
+    struct hmap_node hmap_node;       /* In monitor's hmap. */
+    const struct ofport_dpif *ofport; /* The corresponding ofport. */
+
+    struct cfm *cfm;                  /* Reference to cfm. */
+    struct bfd *bfd;                  /* Reference to bfd. */
+    uint8_t hw_addr[OFP_ETH_ALEN];    /* Hardware address. */
+};
+
+/* hmap that contains all monitored port. */
+static struct hmap monitor_hmap = HMAP_INITIALIZER(&monitor_hmap);
+
+static struct ovs_rwlock monitor_rwlock = OVS_RWLOCK_INITIALIZER;
+
+static void mport_register(const struct ofport_dpif *, struct bfd *,
+                           struct cfm *, uint8_t[ETH_ADDR_LEN])
+    OVS_REQ_WRLOCK(monitor_rwlock);
+static void mport_unregister(const struct ofport_dpif *)
+    OVS_REQ_WRLOCK(monitor_rwlock);
+static void mport_update(struct mport *, struct bfd *, struct cfm *,
+                         uint8_t[ETH_ADDR_LEN]) OVS_REQ_WRLOCK(monitor_rwlock);
+static struct mport *mport_find(const struct ofport_dpif *)
+    OVS_REQ_WRLOCK(monitor_rwlock);
+
+/* Tries finding and returning the 'mport' from the monitor_hmap.
+ * If there is no such 'mport', returns NULL. */
+static struct mport *
+mport_find(const struct ofport_dpif *ofport) OVS_REQ_WRLOCK(monitor_rwlock)
+{
+    struct mport *node;
+
+    HMAP_FOR_EACH_WITH_HASH (node, hmap_node, hash_pointer(ofport, 0),
+                             &monitor_hmap) {
+        if (node->ofport == ofport) {
+            return node;
+        }
+    }
+    return NULL;
+}
+
+/* Creates a new mport and inserts it into monitor_hmap, if it doesn't exist.
+ * Otherwise, just updates its fields. */
+static void
+mport_register(const struct ofport_dpif *ofport, struct bfd *bfd,
+               struct cfm *cfm, uint8_t *hw_addr)
+    OVS_REQ_WRLOCK(monitor_rwlock)
+{
+    struct mport *mport = mport_find(ofport);
+
+    if (!mport) {
+        mport = xzalloc(sizeof *mport);
+        mport->ofport = ofport;
+        hmap_insert(&monitor_hmap, &mport->hmap_node, hash_pointer(ofport, 0));
+    }
+    mport_update(mport, bfd, cfm, hw_addr);
+}
+
+/* Removes mport from monitor_hmap and frees it. */
+static void
+mport_unregister(const struct ofport_dpif *ofport)
+    OVS_REQ_WRLOCK(monitor_rwlock)
+{
+    struct mport *mport = mport_find(ofport);
+
+    if (mport) {
+        mport_update(mport, NULL, NULL, NULL);
+        hmap_remove(&monitor_hmap, &mport->hmap_node);
+        free(mport);
+    }
+}
+
+/* Updates the fields of an existing mport struct. */
+static void
+mport_update(struct mport *mport, struct bfd *bfd, struct cfm *cfm,
+             uint8_t hw_addr[ETH_ADDR_LEN]) OVS_REQ_WRLOCK(monitor_rwlock)
+{
+    ovs_assert(mport);
+
+    if (mport->cfm != cfm) {
+        cfm_unref(mport->cfm);
+        mport->cfm = cfm_ref(cfm);
+    }
+    if (mport->bfd != bfd) {
+        bfd_unref(mport->bfd);
+        mport->bfd = bfd_ref(bfd);
+    }
+    if (hw_addr && memcmp(mport->hw_addr, hw_addr, ETH_ADDR_LEN)) {
+        memcpy(mport->hw_addr, hw_addr, ETH_ADDR_LEN);
+    }
+}
+
+
+/* Creates the mport in monitor module if either bfd or cfm
+ * is configured.  Otherwise, deletes the mport. */
+void
+ofproto_dpif_monitor_port_update(const struct ofport_dpif *ofport,
+                                 struct bfd *bfd, struct cfm *cfm,
+                                 uint8_t hw_addr[ETH_ADDR_LEN])
+{
+    ovs_rwlock_wrlock(&monitor_rwlock);
+    if (!cfm && !bfd) {
+        mport_unregister(ofport);
+    } else {
+        mport_register(ofport, bfd, cfm, hw_addr);
+    }
+    ovs_rwlock_unlock(&monitor_rwlock);
+}
+
+/* Checks the sending of control packets on all mports.  Sends the control
+ * packets if needed. */
+void
+ofproto_dpif_monitor_run_fast(void)
+{
+    struct mport *mport;
+
+    ovs_rwlock_rdlock(&monitor_rwlock);
+    HMAP_FOR_EACH (mport, hmap_node, &monitor_hmap) {
+        if (mport->cfm && cfm_should_send_ccm(mport->cfm)) {
+            struct ofpbuf packet;
+
+            ofpbuf_init(&packet, 0);
+            cfm_compose_ccm(mport->cfm, &packet, mport->hw_addr);
+            ofproto_dpif_send_packet(mport->ofport, &packet);
+            ofpbuf_uninit(&packet);
+        }
+        if (mport->bfd && bfd_should_send_packet(mport->bfd)) {
+            struct ofpbuf packet;
+
+            ofpbuf_init(&packet, 0);
+            bfd_put_packet(mport->bfd, &packet, mport->hw_addr);
+            ofproto_dpif_send_packet(mport->ofport, &packet);
+            ofpbuf_uninit(&packet);
+        }
+    }
+    ovs_rwlock_unlock(&monitor_rwlock);
+}
+
+/* Executes bfd_run(), cfm_run() on all mports. */
+void
+ofproto_dpif_monitor_run(void)
+{
+    struct mport *mport;
+
+    ovs_rwlock_rdlock(&monitor_rwlock);
+    HMAP_FOR_EACH (mport, hmap_node, &monitor_hmap) {
+        if (mport->cfm) {
+            cfm_run(mport->cfm);
+        }
+        if (mport->bfd) {
+            bfd_run(mport->bfd);
+        }
+    }
+    ovs_rwlock_unlock(&monitor_rwlock);
+}
+
+/* Executes the bfd_wait() and cfm_wait() functions on all mports. */
+void
+ofproto_dpif_monitor_wait(void)
+{
+    struct mport *mport;
+
+    ovs_rwlock_rdlock(&monitor_rwlock);
+    HMAP_FOR_EACH (mport, hmap_node, &monitor_hmap) {
+        if (mport->cfm) {
+            cfm_wait(mport->cfm);
+        }
+        if (mport->bfd) {
+            bfd_wait(mport->bfd);
+        }
+    }
+    ovs_rwlock_unlock(&monitor_rwlock);
+}
diff --git a/ofproto/ofproto-dpif-monitor.h b/ofproto/ofproto-dpif-monitor.h
new file mode 100644
index 0000000..0f27ba5
--- /dev/null
+++ b/ofproto/ofproto-dpif-monitor.h
@@ -0,0 +1,33 @@
+/* Copyright (c) 2009, 2010, 2011, 2012, 2013 Nicira, Inc.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at:
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License. */
+
+#ifndef OFPROTO_DPIF_MONITOR_H
+#define OFPROTO_DPIF_MONITOR_H 1
+
+#include <stdint.h>
+
+#include "packets.h"
+
+struct bfd;
+struct cfm;
+struct ofport_dpif;
+
+void ofproto_dpif_monitor_run(void);
+void ofproto_dpif_monitor_run_fast(void);
+void ofproto_dpif_monitor_wait(void);
+
+void ofproto_dpif_monitor_port_update(const struct ofport_dpif *,
+                                      struct bfd *, struct cfm *,
+                                      uint8_t[OFP_ETH_ALEN]);
+#endif /* ofproto-dpif-monitor.h */
diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c
index 67a3298..59f6eb3 100644
--- a/ofproto/ofproto-dpif-xlate.c
+++ b/ofproto/ofproto-dpif-xlate.c
@@ -42,6 +42,7 @@
 #include "ofp-actions.h"
 #include "ofproto/ofproto-dpif-ipfix.h"
 #include "ofproto/ofproto-dpif-mirror.h"
+#include "ofproto/ofproto-dpif-monitor.h"
 #include "ofproto/ofproto-dpif-sflow.h"
 #include "ofproto/ofproto-dpif.h"
 #include "ofproto/ofproto-provider.h"
@@ -131,6 +132,8 @@ struct xport {
 
     struct cfm *cfm;                 /* CFM handle or null. */
     struct bfd *bfd;                 /* BFD handle or null. */
+
+    uint8_t hw_addr[OFP_ETH_ALEN];   /* Hardware address. */
 };
 
 struct xlate_ctx {
@@ -384,7 +387,9 @@ xlate_ofport_set(struct ofproto_dpif *ofproto, struct ofbundle *ofbundle,
                  bool may_enable)
 {
     struct xport *xport = xport_lookup(ofport);
+    uint8_t hw_addr_tmp[OFP_ETH_ALEN];
     size_t i;
+    bool update_monitor = false;
 
     if (!xport) {
         xport = xzalloc(sizeof *xport);
@@ -414,11 +419,19 @@ xlate_ofport_set(struct ofproto_dpif *ofproto, struct ofbundle *ofbundle,
     if (xport->cfm != cfm) {
         cfm_unref(xport->cfm);
         xport->cfm = cfm_ref(cfm);
+        update_monitor = true;
     }
 
     if (xport->bfd != bfd) {
         bfd_unref(xport->bfd);
         xport->bfd = bfd_ref(bfd);
+        update_monitor = true;
+    }
+
+    netdev_get_etheraddr(netdev, hw_addr_tmp);
+    if (memcmp(xport->hw_addr, hw_addr_tmp, ETH_ADDR_LEN)) {
+        memcpy(xport->hw_addr, hw_addr_tmp, ETH_ADDR_LEN);
+        update_monitor = true;
     }
 
     if (xport->peer) {
@@ -437,6 +450,11 @@ xlate_ofport_set(struct ofproto_dpif *ofproto, struct ofbundle *ofbundle,
         list_insert(&xport->xbundle->xports, &xport->bundle_node);
     }
 
+    /* Updates the monitor hmap. */
+    if (update_monitor) {
+        ofproto_dpif_monitor_port_update(xport->ofport, xport->bfd,
+                                         xport->cfm, xport->hw_addr);
+    }
     clear_skb_priorities(xport);
     for (i = 0; i < n_qdscp; i++) {
         struct skb_priority_to_dscp *pdscp;
@@ -482,6 +500,8 @@ xlate_ofport_remove(struct ofport_dpif *ofport)
     netdev_close(xport->netdev);
     cfm_unref(xport->cfm);
     bfd_unref(xport->bfd);
+    /* Remove xport from monitor hmap. */
+    ofproto_dpif_monitor_port_update(xport->ofport, NULL, NULL, NULL);
     free(xport);
 }
 
diff --git a/ofproto/ofproto-dpif-xlate.h b/ofproto/ofproto-dpif-xlate.h
index e231f94..8fd8702 100644
--- a/ofproto/ofproto-dpif-xlate.h
+++ b/ofproto/ofproto-dpif-xlate.h
@@ -15,6 +15,8 @@
 #ifndef OFPROTO_DPIF_XLATE_H
 #define OFPROTO_DPIF_XLATE_H 1
 
+#include <stdint.h>
+
 #include "flow.h"
 #include "meta-flow.h"
 #include "odp-util.h"
diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c
index 15c4dd3..fde8943 100644
--- a/ofproto/ofproto-dpif.c
+++ b/ofproto/ofproto-dpif.c
@@ -52,6 +52,7 @@
 #include "ofproto-dpif-governor.h"
 #include "ofproto-dpif-ipfix.h"
 #include "ofproto-dpif-mirror.h"
+#include "ofproto-dpif-monitor.h"
 #include "ofproto-dpif-sflow.h"
 #include "ofproto-dpif-upcall.h"
 #include "ofproto-dpif-xlate.h"
@@ -357,8 +358,6 @@ ofport_dpif_cast(const struct ofport *ofport)
 }
 
 static void port_run(struct ofport_dpif *);
-static void port_run_fast(struct ofport_dpif *);
-static void port_wait(struct ofport_dpif *);
 static int set_bfd(struct ofport *, const struct smap *);
 static int set_cfm(struct ofport *, const struct cfm_settings *);
 static void ofport_update_peer(struct ofport_dpif *);
@@ -1449,7 +1448,6 @@ run_fast(struct ofproto *ofproto_)
 {
     struct ofproto_dpif *ofproto = ofproto_dpif_cast(ofproto_);
     struct ofputil_packet_in *pin, *next_pin;
-    struct ofport_dpif *ofport;
     struct list pins;
 
     /* Do not perform any periodic activity required by 'ofproto' while
@@ -1466,10 +1464,7 @@ run_fast(struct ofproto *ofproto_)
         free(pin);
     }
 
-    HMAP_FOR_EACH (ofport, up.hmap_node, &ofproto->up.ports) {
-        port_run_fast(ofport);
-    }
-
+    ofproto_dpif_monitor_run_fast();
     return 0;
 }
 
@@ -1511,6 +1506,9 @@ run(struct ofproto *ofproto_)
         dpif_ipfix_run(ofproto->ipfix);
     }
 
+    ofproto_dpif_monitor_run_fast();
+    ofproto_dpif_monitor_run();
+
     HMAP_FOR_EACH (ofport, up.hmap_node, &ofproto->up.ports) {
         port_run(ofport);
     }
@@ -1555,7 +1553,6 @@ static void
 wait(struct ofproto *ofproto_)
 {
     struct ofproto_dpif *ofproto = ofproto_dpif_cast(ofproto_);
-    struct ofport_dpif *ofport;
     struct ofbundle *bundle;
 
     if (ofproto_get_flow_restore_wait()) {
@@ -1568,9 +1565,7 @@ wait(struct ofproto *ofproto_)
     if (ofproto->ipfix) {
         dpif_ipfix_wait(ofproto->ipfix);
     }
-    HMAP_FOR_EACH (ofport, up.hmap_node, &ofproto->up.ports) {
-        port_wait(ofport);
-    }
+    ofproto_dpif_monitor_wait();
     HMAP_FOR_EACH (bundle, hmap_node, &ofproto->bundles) {
         bundle_wait(bundle);
     }
@@ -1977,7 +1972,6 @@ set_bfd(struct ofport *ofport_, const struct smap *cfg)
     if (ofport->bfd != old) {
         ofproto->backer->need_revalidate = REV_RECONFIGURE;
     }
-
     return 0;
 }
 
@@ -2863,28 +2857,6 @@ ofport_update_peer(struct ofport_dpif *ofport)
 }
 
 static void
-port_run_fast(struct ofport_dpif *ofport)
-{
-    if (ofport->cfm && cfm_should_send_ccm(ofport->cfm)) {
-        struct ofpbuf packet;
-
-        ofpbuf_init(&packet, 0);
-        cfm_compose_ccm(ofport->cfm, &packet, ofport->up.pp.hw_addr);
-        ofproto_dpif_send_packet(ofport, &packet);
-        ofpbuf_uninit(&packet);
-    }
-
-    if (ofport->bfd && bfd_should_send_packet(ofport->bfd)) {
-        struct ofpbuf packet;
-
-        ofpbuf_init(&packet, 0);
-        bfd_put_packet(ofport->bfd, &packet, ofport->up.pp.hw_addr);
-        ofproto_dpif_send_packet(ofport, &packet);
-        ofpbuf_uninit(&packet);
-    }
-}
-
-static void
 port_run(struct ofport_dpif *ofport)
 {
     long long int carrier_seq = netdev_get_carrier_resets(ofport->up.netdev);
@@ -2895,12 +2867,9 @@ port_run(struct ofport_dpif *ofport)
 
     ofport->carrier_seq = carrier_seq;
 
-    port_run_fast(ofport);
-
     if (ofport->cfm) {
         int cfm_opup = cfm_get_opup(ofport->cfm);
 
-        cfm_run(ofport->cfm);
         cfm_enable = !cfm_get_fault(ofport->cfm);
 
         if (cfm_opup >= 0) {
@@ -2909,7 +2878,6 @@ port_run(struct ofport_dpif *ofport)
     }
 
     if (ofport->bfd) {
-        bfd_run(ofport->bfd);
         bfd_enable = bfd_forwarding(ofport->bfd);
     }
 
@@ -2932,18 +2900,6 @@ port_run(struct ofport_dpif *ofport)
     ofport->may_enable = enable;
 }
 
-static void
-port_wait(struct ofport_dpif *ofport)
-{
-    if (ofport->cfm) {
-        cfm_wait(ofport->cfm);
-    }
-
-    if (ofport->bfd) {
-        bfd_wait(ofport->bfd);
-    }
-}
-
 static int
 port_query_by_name(const struct ofproto *ofproto_, const char *devname,
                    struct ofproto_port *ofproto_port)
diff --git a/tests/bfd.at b/tests/bfd.at
index 0b2b7cc..b9dd7d8 100644
--- a/tests/bfd.at
+++ b/tests/bfd.at
@@ -260,10 +260,23 @@ OVS_VSWITCHD_START([add-br br1 -- set bridge br1 datapath-type=dummy -- \
 
 ovs-appctl time/stop
 
-# wait for local session state to go from down to up.
-for i in `seq 0 1`; do ovs-appctl time/warp 500; done
-BFD_CHECK([p0], [true], [false], [none], [up], [No Diagnostic], [none], [init], [No Diagnostic])
-
+# figuring out which port initiates the bfd session is important,
+# since this whole unit test is based on exact timing sequence.
+# for example, if p0 starts the bfd session, the p0 should have gone
+# [up] now, and it will decay after 3000ms. if p1 starts the bfd
+# session, we should wait for another 1000ms for p0 to go [up], and
+# then 3000ms for it to decay.
+
+# check which port sends the first bfd control packet.
+if [ ovs-appctl bfd/show p0 | grep "Remote Session State: init" ]
+then
+# if p0 sends first, it should have gone up already.
+    BFD_CHECK([p0], [true], [false], [none], [up], [No Diagnostic], [none], [init], [No Diagnostic])
+else
+# if p1 sends first, wait 1000ms for p0 to go up.
+    BFD_CHECK([p0], [false], [false], [none], [init], [No Diagnostic], [none], [down], [No Diagnostic])
+    for i in `seq 0 1`; do ovs-appctl time/warp 500; done
+fi
 
 # Test-1 BFD decay: decay to decay_min_rx
 # bfd:decay_min_rx is set to 3000ms after the local state of p0 goes up,
@@ -450,7 +463,7 @@ done
 AT_CHECK([ovs-vsctl set Interface p0 bfd:decay_min_rx=3000 -- set interface p1 bfd:min_tx=5000])
 # there will be poll sequences from both sides. and it is hard to determine the
 # order. so just skip 10000ms and check the RX/TX. at that time, p0 should be in decay already.
-for i in `seq 0 19`; do echo $i; ovs-appctl bfd/show; ovs-appctl time/warp 500; done
+for i in `seq 0 19`; do ovs-appctl time/warp 500; done
 BFD_CHECK([p0], [true], [false], [none], [up], [No Diagnostic], [none], [up], [No Diagnostic])
 BFD_CHECK([p1], [true], [false], [none], [up], [No Diagnostic], [none], [up], [No Diagnostic])
 BFD_CHECK_TX([p0], [500ms], [300ms], [5000ms])
@@ -493,15 +506,12 @@ BFD_CHECK_RX([p0], [300ms], [300ms], [1ms])
 
 # resume the bfd on p1. the bfd should not go to decay mode direclty.
 AT_CHECK([ovs-vsctl set Interface p1 bfd:enable=true])
-for i in `seq 0 1`; do ovs-appctl time/warp 500; done
-BFD_CHECK([p0], [true], [false], [none], [up], [Control Detection Time Expired], [none], [up], [No Diagnostic])
+for i in `seq 0 3`; do ovs-appctl time/warp 500; done
 BFD_CHECK_TX([p0], [500ms], [300ms], [500ms])
 BFD_CHECK_RX([p0], [500ms], [300ms], [500ms])
 
-# since the decay_min_rx is still 3000ms, so after 3000ms, there should be the decay and poll sequence.
+# since the decay_min_rx is still 3000ms, so after 3000ms, there should be the decay.
 for i in `seq 0 5`; do ovs-appctl time/warp 500; done
-BFD_CHECK([p0], [true], [false], [none], [up], [Control Detection Time Expired], [final], [up], [No Diagnostic])
-BFD_CHECK([p1], [true], [false], [none], [up], [No Diagnostic], [poll], [up], [Control Detection Time Expired])
 BFD_CHECK_TX([p0], [500ms], [300ms], [500ms])
 BFD_CHECK_RX([p0], [3000ms], [3000ms], [500ms])
 # End of Test-8 ################################################################
-- 
1.7.9.5




More information about the dev mailing list