[ovs-dev] [bug15171 4/4] ofproto: Merge all the CFM query functions into one.

Ben Pfaff blp at nicira.com
Wed Mar 6 00:28:22 UTC 2013


This eliminates several function calls and in particular several
hash table lookups to find structures corresponding to a port
number from iface_refresh_cfm_stats().

This does not seem to reduce CPU use, but the code is shorter
and more readable.

Signed-off-by: Ben Pfaff <blp at nicira.com>
---
 ofproto/ofproto-dpif.c     |   44 +++++++-------------------------
 ofproto/ofproto-provider.h |   42 ++----------------------------
 ofproto/ofproto.c          |   60 ++++---------------------------------------
 ofproto/ofproto.h          |   22 +++++++++++-----
 vswitchd/bridge.c          |   59 ++++++++++++++----------------------------
 5 files changed, 54 insertions(+), 173 deletions(-)

diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c
index 7035530..625429c 100644
--- a/ofproto/ofproto-dpif.c
+++ b/ofproto/ofproto-dpif.c
@@ -1778,43 +1778,22 @@ set_cfm(struct ofport *ofport_, const struct cfm_settings *s)
     return error;
 }
 
-static int
-get_cfm_fault(const struct ofport *ofport_)
-{
-    struct ofport_dpif *ofport = ofport_dpif_cast(ofport_);
-
-    return ofport->cfm ? cfm_get_fault(ofport->cfm) : -1;
-}
-
-static int
-get_cfm_opup(const struct ofport *ofport_)
-{
-    struct ofport_dpif *ofport = ofport_dpif_cast(ofport_);
-
-    return ofport->cfm ? cfm_get_opup(ofport->cfm) : -1;
-}
-
-static int
-get_cfm_remote_mpids(const struct ofport *ofport_, const uint64_t **rmps,
-                     size_t *n_rmps)
+static bool
+get_cfm_status(const struct ofport *ofport_,
+               struct ofproto_cfm_status *status)
 {
     struct ofport_dpif *ofport = ofport_dpif_cast(ofport_);
 
     if (ofport->cfm) {
-        cfm_get_remote_mpids(ofport->cfm, rmps, n_rmps);
-        return 0;
+        status->faults = cfm_get_fault(ofport->cfm);
+        status->remote_opstate = cfm_get_opup(ofport->cfm);
+        status->health = cfm_get_health(ofport->cfm);
+        cfm_get_remote_mpids(ofport->cfm, &status->rmps, &status->n_rmps);
+        return true;
     } else {
-        return -1;
+        return false;
     }
 }
-
-static int
-get_cfm_health(const struct ofport *ofport_)
-{
-    struct ofport_dpif *ofport = ofport_dpif_cast(ofport_);
-
-    return ofport->cfm ? cfm_get_health(ofport->cfm) : -1;
-}
 
 /* Spanning Tree. */
 
@@ -8370,10 +8349,7 @@ const struct ofproto_class ofproto_dpif_class = {
     get_netflow_ids,
     set_sflow,
     set_cfm,
-    get_cfm_fault,
-    get_cfm_opup,
-    get_cfm_remote_mpids,
-    get_cfm_health,
+    get_cfm_status,
     set_stp,
     get_stp_status,
     set_stp_port,
diff --git a/ofproto/ofproto-provider.h b/ofproto/ofproto-provider.h
index 95bda33..3b17a73 100644
--- a/ofproto/ofproto-provider.h
+++ b/ofproto/ofproto-provider.h
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2009, 2010, 2011, 2012 Nicira, Inc.
+ * 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.
@@ -1120,44 +1120,8 @@ struct ofproto_class {
      * support CFM, as does a null pointer. */
     int (*set_cfm)(struct ofport *ofport, const struct cfm_settings *s);
 
-    /* Checks the fault status of CFM configured on 'ofport'.  Returns a
-     * bitmask of 'cfm_fault_reason's to indicate a CFM fault (generally
-     * indicating a connectivity problem).  Returns zero if CFM is not faulted,
-     * and -1 if CFM is not enabled on 'port'.
-     *
-     * This function may be a null pointer if the ofproto implementation does
-     * not support CFM. */
-    int (*get_cfm_fault)(const struct ofport *ofport);
-
-    /* Check the operational status reported by the remote CFM endpoint of
-     * 'ofp_port'  Returns 1 if operationally up, 0 if operationally down, and
-     * -1 if CFM is not enabled on 'ofp_port' or does not support operational
-     * status.
-     *
-     * This function may be a null pointer if the ofproto implementation does
-     * not support CFM. */
-    int (*get_cfm_opup)(const struct ofport *ofport);
-
-    /* Gets the MPIDs of the remote maintenance points broadcasting to
-     * 'ofport'.  Populates 'rmps' with a provider owned array of MPIDs, and
-     * 'n_rmps' with the number of MPIDs in 'rmps'. Returns a number less than
-     * 0 if CFM is not enabled of 'ofport'.
-     *
-     * This function may be a null pointer if the ofproto implementation does
-     * not support CFM. */
-    int (*get_cfm_remote_mpids)(const struct ofport *ofport,
-                                const uint64_t **rmps, size_t *n_rmps);
-
-    /* Checks the health of CFM configured on 'ofport'.  Returns an integer
-     * to indicate the health percentage of the 'ofport' which is an average of
-     * the health of all the remote_mps.  Returns an integer between 0 and 100
-     * where 0 means that the 'ofport' is very unhealthy and 100 means the
-     * 'ofport' is perfectly healthy.  Returns -1 if CFM is not enabled on
-     * 'port' or if the number of remote_mpids is > 1.
-     *
-     * This function may be a null pointer if the ofproto implementation does
-     * not support CFM. */
-    int (*get_cfm_health)(const struct ofport *ofport);
+    bool (*get_cfm_status)(const struct ofport *ofport,
+                           struct ofproto_cfm_status *status);
 
     /* Configures spanning tree protocol (STP) on 'ofproto' using the
      * settings defined in 's'.
diff --git a/ofproto/ofproto.c b/ofproto/ofproto.c
index a9c7e76..bf27179 100644
--- a/ofproto/ofproto.c
+++ b/ofproto/ofproto.c
@@ -2843,62 +2843,14 @@ ofproto_get_netflow_ids(const struct ofproto *ofproto,
     ofproto->ofproto_class->get_netflow_ids(ofproto, engine_type, engine_id);
 }
 
-/* Checks the fault status of CFM for 'ofp_port' within 'ofproto'.  Returns a
- * bitmask of 'cfm_fault_reason's to indicate a CFM fault (generally
- * indicating a connectivity problem).  Returns zero if CFM is not faulted,
- * and -1 if CFM is not enabled on 'ofp_port'. */
-int
-ofproto_port_get_cfm_fault(const struct ofproto *ofproto, uint16_t ofp_port)
-{
-    struct ofport *ofport = ofproto_get_port(ofproto, ofp_port);
-    return (ofport && ofproto->ofproto_class->get_cfm_fault
-            ? ofproto->ofproto_class->get_cfm_fault(ofport)
-            : -1);
-}
-
-/* Checks the operational status reported by the remote CFM endpoint of
- * 'ofp_port'  Returns 1 if operationally up, 0 if operationally down, and -1
- * if CFM is not enabled on 'ofp_port' or does not support operational status.
- */
-int
-ofproto_port_get_cfm_opup(const struct ofproto *ofproto, uint16_t ofp_port)
-{
-    struct ofport *ofport = ofproto_get_port(ofproto, ofp_port);
-    return (ofport && ofproto->ofproto_class->get_cfm_opup
-            ? ofproto->ofproto_class->get_cfm_opup(ofport)
-            : -1);
-}
-
-/* Gets the MPIDs of the remote maintenance points broadcasting to 'ofp_port'
- * within 'ofproto'.  Populates 'rmps' with an array of MPIDs owned by
- * 'ofproto', and 'n_rmps' with the number of MPIDs in 'rmps'.  Returns a
- * number less than 0 if CFM is not enabled on 'ofp_port'. */
-int
-ofproto_port_get_cfm_remote_mpids(const struct ofproto *ofproto,
-                                  uint16_t ofp_port, const uint64_t **rmps,
-                                  size_t *n_rmps)
-{
-    struct ofport *ofport = ofproto_get_port(ofproto, ofp_port);
-
-    *rmps = NULL;
-    *n_rmps = 0;
-    return (ofport && ofproto->ofproto_class->get_cfm_remote_mpids
-            ? ofproto->ofproto_class->get_cfm_remote_mpids(ofport, rmps,
-                                                           n_rmps)
-            : -1);
-}
-
-/* Checks the health of the CFM for 'ofp_port' within 'ofproto'.  Returns an
- * integer value between 0 and 100 to indicate the health of the port as a
- * percentage which is the average of cfm health of all the remote_mpids or
- * returns -1 if CFM is not enabled on 'ofport'. */
-int
-ofproto_port_get_cfm_health(const struct ofproto *ofproto, uint16_t ofp_port)
+bool
+ofproto_port_get_cfm_status(const struct ofproto *ofproto, uint16_t ofp_port,
+                            struct ofproto_cfm_status *status)
 {
     struct ofport *ofport = ofproto_get_port(ofproto, ofp_port);
-    return (ofport && ofproto->ofproto_class->get_cfm_health
-            ? ofproto->ofproto_class->get_cfm_health(ofport)
-            : -1);
+    return (ofport
+            && ofproto->ofproto_class->get_cfm_status
+            && ofproto->ofproto_class->get_cfm_status(ofport, status));
 }
 
 static enum ofperr
diff --git a/ofproto/ofproto.h b/ofproto/ofproto.h
index a217e0a..f164b3f 100644
--- a/ofproto/ofproto.h
+++ b/ofproto/ofproto.h
@@ -355,15 +355,23 @@ void ofproto_get_snoops(const struct ofproto *, struct sset *);
 void ofproto_get_all_flows(struct ofproto *p, struct ds *);
 void ofproto_get_netflow_ids(const struct ofproto *,
                              uint8_t *engine_type, uint8_t *engine_id);
-int ofproto_port_get_cfm_fault(const struct ofproto *, uint16_t ofp_port);
-int ofproto_port_get_cfm_opup(const struct ofproto *, uint16_t ofp_port);
-int ofproto_port_get_cfm_remote_mpids(const struct ofproto *,
-                                      uint16_t ofp_port, const uint64_t **rmps,
-                                      size_t *n_rmps);
-int ofproto_port_get_cfm_health(const struct ofproto *ofproto,
-                                uint16_t ofp_port);
+
 void ofproto_get_ofproto_controller_info(const struct ofproto *, struct shash *);
 void ofproto_free_ofproto_controller_info(struct shash *);
+
+/* CFM status query. */
+struct ofproto_cfm_status {
+    enum cfm_fault_reason faults; /* 0 if not faulted. */
+    bool remote_opstate;          /* True if remote CFM endpoint is up.  */
+    int health;                   /* Health status in [0,100] range. */
+
+    /* MPIDs of remote maintenance points whose CCMs have been received. */
+    const uint64_t *rmps;
+    size_t n_rmps;
+};
+
+bool ofproto_port_get_cfm_status(const struct ofproto *, uint16_t ofp_port,
+                                 struct ofproto_cfm_status *);
 
 /* Linux VLAN device support (e.g. "eth0.10" for VLAN 10.)
  *
diff --git a/vswitchd/bridge.c b/vswitchd/bridge.c
index 993e4da..7bf169d 100644
--- a/vswitchd/bridge.c
+++ b/vswitchd/bridge.c
@@ -1742,57 +1742,38 @@ static void
 iface_refresh_cfm_stats(struct iface *iface)
 {
     const struct ovsrec_interface *cfg = iface->cfg;
-    int fault, opup, error;
-    const uint64_t *rmps;
-    size_t n_rmps;
-    int health;
-
-    fault = ofproto_port_get_cfm_fault(iface->port->bridge->ofproto,
-                                       iface->ofp_port);
-    if (fault >= 0) {
+    struct ofproto_cfm_status status;
+
+    if (!ofproto_port_get_cfm_status(iface->port->bridge->ofproto,
+                                    iface->ofp_port, &status)) {
+        ovsrec_interface_set_cfm_fault(cfg, NULL, 0);
+        ovsrec_interface_set_cfm_fault_status(cfg, NULL, 0);
+        ovsrec_interface_set_cfm_remote_opstate(cfg, NULL);
+        ovsrec_interface_set_cfm_health(cfg, NULL, 0);
+        ovsrec_interface_set_cfm_remote_mpids(cfg, NULL, 0);
+    } else {
         const char *reasons[CFM_FAULT_N_REASONS];
-        bool fault_bool = fault;
+        int64_t cfm_health = status.health;
+        bool faulted = status.faults != 0;
         size_t i, j;
 
+        ovsrec_interface_set_cfm_fault(cfg, &faulted, 1);
+
         j = 0;
         for (i = 0; i < CFM_FAULT_N_REASONS; i++) {
             int reason = 1 << i;
-            if (fault & reason) {
+            if (status.faults & reason) {
                 reasons[j++] = cfm_fault_reason_to_str(reason);
             }
         }
-
-        ovsrec_interface_set_cfm_fault(cfg, &fault_bool, 1);
         ovsrec_interface_set_cfm_fault_status(cfg, (char **) reasons, j);
-    } else {
-        ovsrec_interface_set_cfm_fault(cfg, NULL, 0);
-        ovsrec_interface_set_cfm_fault_status(cfg, NULL, 0);
-    }
-
-    opup = ofproto_port_get_cfm_opup(iface->port->bridge->ofproto,
-                                     iface->ofp_port);
-    if (opup >= 0) {
-        ovsrec_interface_set_cfm_remote_opstate(cfg, opup ? "up" : "down");
-    } else {
-        ovsrec_interface_set_cfm_remote_opstate(cfg, NULL);
-    }
 
-    error = ofproto_port_get_cfm_remote_mpids(iface->port->bridge->ofproto,
-                                              iface->ofp_port, &rmps, &n_rmps);
-    if (error >= 0) {
-        ovsrec_interface_set_cfm_remote_mpids(cfg, (const int64_t *)rmps,
-                                              n_rmps);
-    } else {
-        ovsrec_interface_set_cfm_remote_mpids(cfg, NULL, 0);
-    }
-
-    health = ofproto_port_get_cfm_health(iface->port->bridge->ofproto,
-                                        iface->ofp_port);
-    if (health >= 0) {
-        int64_t cfm_health = health;
+        ovsrec_interface_set_cfm_remote_opstate(cfg, (status.remote_opstate
+                                                      ? "up" : "down"));
+        ovsrec_interface_set_cfm_remote_mpids(cfg,
+                                              (const int64_t *)status.rmps,
+                                              status.n_rmps);
         ovsrec_interface_set_cfm_health(cfg, &cfm_health, 1);
-    } else {
-        ovsrec_interface_set_cfm_health(cfg, NULL, 0);
     }
 }
 
-- 
1.7.2.5




More information about the dev mailing list