[ovs-dev] [lacp bridge 1/4] lacp: Remove enabled flag.

[Ethan Jackson]

The enabled flag in the LACP module was only used to set the
Collecting and Distributing flags in the LACP protocol.  It was
intended to be set by the bonding code to mimic its enabled flag.

The spec is relatively vague on the precise meaning of these flags,
and most implementations do something completely different with
them.  For these reasons, it seems acceptable to remove the enabled
flag for the sake of simplicity.  A slave is now Collecting and
Distributing if it is attached, or LACP couldn't be negotiated.
---
 lib/bond.c |    4 ----
 lib/lacp.c |   18 +++---------------
 lib/lacp.h |    1 -
 3 files changed, 3 insertions(+), 20 deletions(-)

diff --git a/lib/bond.c b/lib/bond.c
index c68b553..9f857e9 100644
--- a/lib/bond.c
+++ b/lib/bond.c
@@ -500,10 +500,6 @@ bond_run(struct bond *bond, struct tag_set *tags)
 
     /* Update LACP. */
     if (bond->lacp) {
-        HMAP_FOR_EACH (slave, hmap_node, &bond->slaves) {
-            lacp_slave_enable(bond->lacp, slave, slave->enabled);
-        }
-
         lacp_run(bond->lacp, bond_send_pdu_cb);
     }
 
diff --git a/lib/lacp.c b/lib/lacp.c
index 87514e3..ee79677 100644
--- a/lib/lacp.c
+++ b/lib/lacp.c
@@ -64,7 +64,6 @@ struct slave {
 
     enum slave_status status;     /* Slave status. */
     bool attached;                /* Attached. Traffic may flow. */
-    bool enabled;                 /* Enabled. Traffic is flowing. */
     struct lacp_info partner;     /* Partner information. */
     struct lacp_info ntt_actor;   /* Used to decide if we Need To Transmit. */
     struct timer tx;              /* Next message transmission timer. */
@@ -229,16 +228,6 @@ lacp_slave_unregister(struct lacp *lacp, const void *slave_)
     }
 }
 
-/* Should be called regularly to indicate whether 'slave_' is enabled.  An
- * enabled slave is allowed to send and receive traffic.  Generally a slave
- * should not be enabled if its carrier is down, or lacp_slave_may_enable()
- * indicates it should not be enabled. */
-void
-lacp_slave_enable(struct lacp *lacp, void *slave_, bool enabled)
-{
-   slave_lookup(lacp, slave_)->enabled = enabled;
-}
-
 /* This function should be called whenever the carrier status of 'slave_' has
  * changed. */
 void
@@ -467,7 +456,7 @@ slave_get_actor(struct slave *slave, struct lacp_info *actor)
         state |= LACP_STATE_AGG;
     }
 
-    if (slave->enabled) {
+    if (slave->attached || !slave->lacp->negotiated) {
         state |= LACP_STATE_COL | LACP_STATE_DIST;
     }
 
@@ -648,9 +637,8 @@ lacp_unixctl_show(struct unixctl_conn *conn,
             NOT_REACHED();
         }
 
-        ds_put_format(&ds, "\nslave: %s: %s %s %s\n", slave->name, status,
-                      slave->attached ? "attached" : "detached",
-                      slave->enabled ? "enabled" : "disabled");
+        ds_put_format(&ds, "\nslave: %s: %s %s\n", slave->name, status,
+                      slave->attached ? "attached" : "detached");
         ds_put_format(&ds, "\tport_id: %u\n", slave->port_id);
         ds_put_format(&ds, "\tport_priority: %u\n", slave->port_priority);
 
diff --git a/lib/lacp.h b/lib/lacp.h
index a486542..38c8f6e 100644
--- a/lib/lacp.h
+++ b/lib/lacp.h
@@ -49,7 +49,6 @@ struct lacp_slave_settings {
 void lacp_slave_register(struct lacp *, void *slave_,
                          const struct lacp_slave_settings *);
 void lacp_slave_unregister(struct lacp *, const void *slave);
-void lacp_slave_enable(struct lacp *lacp, void *slave_, bool enabled);
 void lacp_slave_carrier_changed(const struct lacp *, const void *slave);
 bool lacp_slave_may_enable(const struct lacp *, const void *slave);
 uint16_t lacp_slave_get_port_id(const struct lacp *, const void *slave);
-- 
1.7.4.2