ofproto: Consistently force off OFPPS_LIVE if port or link is down.

It doesn't make sense for a port that is down to be "live" from OpenFlow's
point of view, but this could happen in OVS.

Signed-off-by: Ben Pfaff <blp@ovn.org>
Acked-by: Numan Siddique <nusididq@redhat.com>

1 files changed, 33 insertions, 10 deletions

diff --git a/ofproto/ofproto.c b/ofproto/ofproto.c
index 32e9c4852..8b2e3ca97 100644
--- a/ofproto/ofproto.c
+++ b/ofproto/ofproto.c
@@ -2457,11 +2457,34 @@ ofport_remove_with_name(struct ofproto *ofproto, const char *name)
     }
 }
 
+static enum ofputil_port_state
+normalize_state(enum ofputil_port_config config,
+                enum ofputil_port_state state,
+                bool may_enable)
+{
+    return (config & OFPUTIL_PC_PORT_DOWN
+            || state & OFPUTIL_PS_LINK_DOWN
+            || !may_enable
+            ? state & ~OFPUTIL_PS_LIVE
+            : state | OFPUTIL_PS_LIVE);
+}
+
+void
+ofproto_port_set_enable(struct ofport *port, bool enable)
+{
+    if (enable != port->may_enable) {
+        port->may_enable = enable;
+        ofproto_port_set_state(port, normalize_state(port->pp.config,
+                                                     port->pp.state,
+                                                     port->may_enable));
+    }
+}
 
 /* Update OpenFlow 'state' in 'port' and notify controller. */
 void
 ofproto_port_set_state(struct ofport *port, enum ofputil_port_state state)
 {
+    state = normalize_state(port->pp.config, state, port->may_enable);
     if (port->pp.state != state) {
         struct ofputil_phy_port old_pp = port->pp;
         port->pp.state = state;
@@ -2611,16 +2634,18 @@ update_port(struct ofproto *ofproto, const char *name)
         port = ofproto_get_port(ofproto, ofproto_port.ofp_port);
         if (port && !strcmp(netdev_get_name(port->netdev), name)) {
             struct netdev *old_netdev = port->netdev;
-            struct ofputil_phy_port old_pp = port->pp;
 
             /* ofport_open() only sets OFPUTIL_PC_PORT_DOWN and
-             * OFPUTIL_PS_LINK_DOWN.  Keep the other config and state bits. */
+             * OFPUTIL_PS_LINK_DOWN.  Keep the other config and state bits (but
+             * a port that is down cannot be live). */
             pp.config |= port->pp.config & ~OFPUTIL_PC_PORT_DOWN;
             pp.state |= port->pp.state & ~OFPUTIL_PS_LINK_DOWN;
+            pp.state = normalize_state(pp.config, pp.state, port->may_enable);
 
             /* 'name' hasn't changed location.  Any properties changed? */
-            bool port_changed = !ofport_equal(&port->pp, &pp);
-            if (port_changed) {
+            if (!ofport_equal(&port->pp, &pp)) {
+                connmgr_send_port_status(port->ofproto->connmgr, NULL,
+                                         &port->pp, &pp, OFPPR_MODIFY);
                 port->pp = pp;
             }
 
@@ -2636,12 +2661,6 @@ update_port(struct ofproto *ofproto, const char *name)
                 port->ofproto->ofproto_class->port_modified(port);
             }
 
-            /* Send status update, if any port property changed */
-            if (port_changed) {
-                connmgr_send_port_status(port->ofproto->connmgr, NULL,
-                                         &old_pp, &port->pp, OFPPR_MODIFY);
-            }
-
             netdev_close(old_netdev);
         } else {
             /* If 'port' is nonnull then its name differs from 'name' and thus
@@ -3636,7 +3655,11 @@ update_port_config(struct ofconn *ofconn, struct ofport *port,
 
     if (toggle) {
         struct ofputil_phy_port old_pp = port->pp;
+
         port->pp.config ^= toggle;
+        port->pp.state = normalize_state(port->pp.config, port->pp.state,
+                                         port->may_enable);
+
         port->ofproto->ofproto_class->port_reconfigured(port, old_pp.config);
         connmgr_send_port_status(port->ofproto->connmgr, ofconn, &old_pp,
                                  &port->pp, OFPPR_MODIFY);