bridge: Drop LACP configuration members from struct iface and struct port.

There's no reason that I can see to maintain this information in struct
port and struct iface.  It's redundant, since the lacp implementation
maintains the same information.

diff --git a/lib/lacp.c b/lib/lacp.c
index 005165f..c8fae62 100644 (file)
--- a/lib/lacp.c
+++ b/lib/lacp.c
@@ -142,6 +142,14 @@ lacp_configure(struct lacp *lacp, const char *name,
     lacp->fast = fast;
 }
 
     lacp->fast = fast;
 }
 

+/* Returns true if 'lacp' is configured in active mode, false if 'lacp' is
+ * configured for passive mode. */
+bool
+lacp_is_active(const struct lacp *lacp)
+{
+    return lacp->active;
+}
+

 /* Processes 'pdu', a parsed LACP packet received on 'slave_'.  This function
  * should be called on all packets received on 'slave_' with Ethernet Type
  * ETH_TYPE_LACP and parsable by parse_lacp_packet(). */
 /* Processes 'pdu', a parsed LACP packet received on 'slave_'.  This function
  * should be called on all packets received on 'slave_' with Ethernet Type
  * ETH_TYPE_LACP and parsable by parse_lacp_packet(). */

diff --git a/lib/lacp.h b/lib/lacp.h
index 8d3bc78..c5f3c2f 100644 (file)
--- a/lib/lacp.h
+++ b/lib/lacp.h
@@ -27,9 +27,12 @@ typedef void lacp_send_pdu(void *slave, const struct lacp_pdu *);
 void lacp_init(void);
 struct lacp *lacp_create(void);
 void lacp_destroy(struct lacp *);
 void lacp_init(void);
 struct lacp *lacp_create(void);
 void lacp_destroy(struct lacp *);

+

 void lacp_configure(struct lacp *, const char *name,
                     const uint8_t sys_id[ETH_ADDR_LEN],
                     uint16_t sys_priority, bool active, bool fast);
 void lacp_configure(struct lacp *, const char *name,
                     const uint8_t sys_id[ETH_ADDR_LEN],
                     uint16_t sys_priority, bool active, bool fast);

+bool lacp_is_active(const struct lacp *);
+

 void lacp_process_pdu(struct lacp *, const void *slave,
                       const struct lacp_pdu *);
 bool lacp_negotiated(const struct lacp *);
 void lacp_process_pdu(struct lacp *, const void *slave,
                       const struct lacp_pdu *);
 bool lacp_negotiated(const struct lacp *);

diff --git a/vswitchd/bridge.c b/vswitchd/bridge.c
index a174cad..b52fd8b 100644 (file)
--- a/vswitchd/bridge.c
+++ b/vswitchd/bridge.c
@@ -114,9 +114,6 @@ struct iface {
     bool up;                    /* Is the interface up? */
     const char *type;           /* Usually same as cfg->type. */
     const struct ovsrec_interface *cfg;
     bool up;                    /* Is the interface up? */
     const char *type;           /* Usually same as cfg->type. */
     const struct ovsrec_interface *cfg;

-
-    /* LACP information. */
-    uint16_t lacp_priority;     /* LACP port priority. */

 };
 
 #define BOND_MASK 0xff
 };
 
 #define BOND_MASK 0xff


@@ -184,9 +181,6 @@ struct port {
 
     /* LACP information. */
     struct lacp *lacp;          /* LACP object. NULL if LACP is disabled. */
 
     /* LACP information. */
     struct lacp *lacp;          /* LACP object. NULL if LACP is disabled. */

-    bool lacp_active;           /* True if LACP is active */
-    bool lacp_fast;             /* True if LACP is in fast mode. */
-    uint16_t lacp_priority;     /* LACP system priority. */

 
     /* SLB specific bonding info. */
     struct bond_entry *bond_hash; /* An array of (BOND_MASK + 1) elements. */
 
     /* SLB specific bonding info. */
     struct bond_entry *bond_hash; /* An array of (BOND_MASK + 1) elements. */


@@ -3529,7 +3523,7 @@ bond_unixctl_show(struct unixctl_conn *conn,
 
     if (port->lacp) {
         ds_put_format(&ds, "lacp: %s\n",
 
     if (port->lacp) {
         ds_put_format(&ds, "lacp: %s\n",

-                      port->lacp_active ? "active" : "passive");
+                      lacp_is_active(port->lacp) ? "active" : "passive");

     } else {
         ds_put_cstr(&ds, "lacp: off\n");
     }
     } else {
         ds_put_cstr(&ds, "lacp: off\n");
     }


@@ -3989,7 +3983,7 @@ port_reconfigure(struct port *port, const struct ovsrec_port *cfg)
 {
     const char *detect_mode;
     struct sset new_ifaces;
 {
     const char *detect_mode;
     struct sset new_ifaces;

-    long long int next_rebalance, miimon_next_update, lacp_priority;
+    long long int next_rebalance, miimon_next_update;

     bool need_flush = false;
     unsigned long *trunks;
     int vlan;
     bool need_flush = false;
     unsigned long *trunks;
     int vlan;


@@ -4087,57 +4081,9 @@ port_reconfigure(struct port *port, const struct ovsrec_port *cfg)
         iface->type = (!strcmp(if_cfg->name, port->bridge->name) ? "internal"
                        : if_cfg->type[0] ? if_cfg->type
                        : "system");
         iface->type = (!strcmp(if_cfg->name, port->bridge->name) ? "internal"
                        : if_cfg->type[0] ? if_cfg->type
                        : "system");

-
-        lacp_priority =
-            atoi(get_interface_other_config(if_cfg, "lacp-port-priority",
-                                            "0"));
-
-        if (lacp_priority <= 0 || lacp_priority > UINT16_MAX) {
-            iface->lacp_priority = UINT16_MAX;
-        } else {
-            iface->lacp_priority = lacp_priority;
-        }

     }
     sset_destroy(&new_ifaces);
 
     }
     sset_destroy(&new_ifaces);
 

-    port->lacp_fast = !strcmp(get_port_other_config(cfg, "lacp-time", "slow"),
-                             "fast");
-
-    lacp_priority =
-        atoi(get_port_other_config(cfg, "lacp-system-priority", "0"));
-
-    if (lacp_priority <= 0 || lacp_priority > UINT16_MAX) {
-        /* Prefer bondable links if unspecified. */
-        port->lacp_priority = port->n_ifaces > 1 ? UINT16_MAX - 1 : UINT16_MAX;
-    } else {
-        port->lacp_priority = lacp_priority;
-    }
-
-    if (!port->cfg->lacp) {
-        /* XXX when LACP implementation has been sufficiently tested, enable by
-         * default and make active on bonded ports. */
-        lacp_destroy(port->lacp);
-        port->lacp = NULL;
-    } else if (!strcmp(port->cfg->lacp, "off")) {
-        lacp_destroy(port->lacp);
-        port->lacp = NULL;
-    } else if (!strcmp(port->cfg->lacp, "active")) {
-        if (!port->lacp) {
-            port->lacp = lacp_create();
-        }
-        port->lacp_active = true;
-    } else if (!strcmp(port->cfg->lacp, "passive")) {
-        if (!port->lacp) {
-            port->lacp = lacp_create();
-        }
-        port->lacp_active = false;
-    } else {
-        VLOG_WARN("port %s: unknown LACP mode %s",
-                  port->name, port->cfg->lacp);
-        lacp_destroy(port->lacp);
-        port->lacp = NULL;
-    }
-

     /* Get VLAN tag. */
     vlan = -1;
     if (cfg->tag) {
     /* Get VLAN tag. */
     vlan = -1;
     if (cfg->tag) {


@@ -4264,20 +4210,68 @@ port_lookup_iface(const struct port *port, const char *name)
     return iface && iface->port == port ? iface : NULL;
 }
 
     return iface && iface->port == port ? iface : NULL;
 }
 

+static bool
+enable_lacp(struct port *port, bool *activep)
+{
+    if (!port->cfg->lacp) {
+        /* XXX when LACP implementation has been sufficiently tested, enable by
+         * default and make active on bonded ports. */
+        return false;
+    } else if (!strcmp(port->cfg->lacp, "off")) {
+        return false;
+    } else if (!strcmp(port->cfg->lacp, "active")) {
+        *activep = true;
+        return true;
+    } else if (!strcmp(port->cfg->lacp, "passive")) {
+        *activep = false;
+        return true;
+    } else {
+        VLOG_WARN("port %s: unknown LACP mode %s",
+                  port->name, port->cfg->lacp);
+        return false;
+    }
+}
+

 static void
 port_update_lacp(struct port *port)
 {
 static void
 port_update_lacp(struct port *port)
 {

-    if (port->lacp) {
-        struct iface *iface;
+    struct iface *iface;
+    int priority;
+    bool active;
+    bool fast;

 
 

-        lacp_configure(port->lacp, port->name,
-                       port->bridge->ea, port->lacp_priority,
-                       port->lacp_active, port->lacp_fast);
+    if (!enable_lacp(port, &active)) {
+        lacp_destroy(port->lacp);
+        port->lacp = NULL;
+        return;
+    }

 
 

-        LIST_FOR_EACH (iface, port_elem, &port->ifaces) {
-            lacp_slave_register(port->lacp, iface, iface->name,
-                                iface->dp_ifidx, iface->lacp_priority);
+    if (!port->lacp) {
+        port->lacp = lacp_create();
+    }
+
+    fast = !strcmp(get_port_other_config(port->cfg, "lacp-time", "slow"),
+                   "fast");
+
+    priority = atoi(get_port_other_config(port->cfg, "lacp-system-priority",
+                                          "0"));
+    if (priority <= 0 || priority > UINT16_MAX) {
+        /* Prefer bondable links if unspecified. */
+        priority = UINT16_MAX - (port->n_ifaces > 1);
+    }
+
+    lacp_configure(port->lacp, port->name, port->bridge->ea, priority,
+                   active, fast);
+
+    LIST_FOR_EACH (iface, port_elem, &port->ifaces) {
+        priority = atoi(get_interface_other_config(
+                            iface->cfg, "lacp-port-priority", "0"));
+        if (priority <= 0 || priority > UINT16_MAX) {
+            priority = UINT16_MAX;

         }
         }

+
+        lacp_slave_register(port->lacp, iface, iface->name,
+                            iface->dp_ifidx, priority);

     }
 }
 
     }
 }