[ovs-dev] [PATCH 4/6] ofproto-dpif: Refactor port_run().

[Ben Pfaff]

This makes port_run() easier to understand but should not change its
behavior.

Signed-off-by: Ben Pfaff <blp at ovn.org>
---
 ofproto/ofproto-dpif.c | 62 ++++++++++++++++++++++++++------------------------
 1 file changed, 32 insertions(+), 30 deletions(-)

diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c
index 643c2cf325f0..868c728c0c14 100644
--- a/ofproto/ofproto-dpif.c
+++ b/ofproto/ofproto-dpif.c
@@ -3569,45 +3569,49 @@ ofport_update_peer(struct ofport_dpif *ofport)
     free(peer_name);
 }
 
-static void
-port_run(struct ofport_dpif *ofport)
+static bool
+may_enable_port(struct ofport_dpif *ofport)
 {
-    long long int carrier_seq = netdev_get_carrier_resets(ofport->up.netdev);
-    bool carrier_changed = carrier_seq != ofport->carrier_seq;
-    bool enable = netdev_get_carrier(ofport->up.netdev);
-    bool cfm_enable = false;
-    bool bfd_enable = false;
-
-    ofport->carrier_seq = carrier_seq;
-
-    if (ofport->cfm) {
-        int cfm_opup = cfm_get_opup(ofport->cfm);
-
-        cfm_enable = !cfm_get_fault(ofport->cfm);
-
-        if (cfm_opup >= 0) {
-            cfm_enable = cfm_enable && cfm_opup;
-        }
+    /* Carrier must be up. */
+    if (!netdev_get_carrier(ofport->up.netdev)) {
+        return false;
     }
 
-    if (ofport->bfd) {
-        bfd_enable = bfd_forwarding(ofport->bfd);
+    /* If CFM or BFD is enabled, then at least one of them must report that the
+     * port is up. */
+    if ((ofport->bfd || ofport->cfm)
+        && !(ofport->cfm
+             && !cfm_get_fault(ofport->cfm)
+             && cfm_get_opup(ofport->cfm) != 0)
+        && !(ofport->bfd
+             && bfd_forwarding(ofport->bfd))) {
+        return false;
     }
 
-    if (ofport->bfd || ofport->cfm) {
-        enable = enable && (cfm_enable || bfd_enable);
+    /* If LACP is enabled, it must report that the link is enabled. */
+    if (ofport->bundle
+        && !lacp_slave_may_enable(ofport->bundle->lacp, ofport)) {
+        return false;
     }
 
-    if (ofport->bundle) {
-        enable = enable && lacp_slave_may_enable(ofport->bundle->lacp, ofport);
-        if (carrier_changed) {
-            lacp_slave_carrier_changed(ofport->bundle->lacp, ofport);
-        }
+    return true;
+}
+
+static void
+port_run(struct ofport_dpif *ofport)
+{
+    long long int carrier_seq = netdev_get_carrier_resets(ofport->up.netdev);
+    bool carrier_changed = carrier_seq != ofport->carrier_seq;
+    ofport->carrier_seq = carrier_seq;
+    if (carrier_changed && ofport->bundle) {
+        lacp_slave_carrier_changed(ofport->bundle->lacp, ofport);
     }
 
+    bool enable = may_enable_port(ofport);
     if (ofport->up.may_enable != enable) {
-        struct ofproto_dpif *ofproto = ofproto_dpif_cast(ofport->up.ofproto);
+        ofport->up.may_enable = enable;
 
+        struct ofproto_dpif *ofproto = ofproto_dpif_cast(ofport->up.ofproto);
         ofproto->backer->need_revalidate = REV_PORT_TOGGLED;
 
         if (ofport->rstp_port) {
@@ -3627,8 +3631,6 @@ port_run(struct ofport_dpif *ofport)
             ofproto_port_set_state(&ofport->up, of_state);
         }
     }
-
-    ofport->up.may_enable = enable;
 }
 
 static int
-- 
2.16.1