[ovs-dev] [bondlib 10/19] bridge: Drop LACP configuration members from struct iface and struct port.
Ben Pfaff
blp at nicira.com
Fri Mar 25 17:35:43 UTC 2011
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.
---
lib/lacp.c | 8 ++++
lib/lacp.h | 3 +
vswitchd/bridge.c | 122 +++++++++++++++++++++++++----------------------------
3 files changed, 69 insertions(+), 64 deletions(-)
diff --git a/lib/lacp.c b/lib/lacp.c
index c01a567..094b036 100644
--- a/lib/lacp.c
+++ b/lib/lacp.c
@@ -141,6 +141,14 @@ lacp_configure(struct lacp *lacp, const char *name,
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(). */
diff --git a/lib/lacp.h b/lib/lacp.h
index 8d3bc78..c5f3c2f 100644
--- 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_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 *);
diff --git a/vswitchd/bridge.c b/vswitchd/bridge.c
index c9dc954..fea574d 100644
--- a/vswitchd/bridge.c
+++ b/vswitchd/bridge.c
@@ -113,9 +113,6 @@ struct iface {
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
@@ -183,9 +180,6 @@ struct port {
/* 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. */
@@ -3510,7 +3504,7 @@ bond_unixctl_show(struct unixctl_conn *conn,
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");
}
@@ -3970,7 +3964,7 @@ port_reconfigure(struct port *port, const struct ovsrec_port *cfg)
{
const char *detect_mode;
struct shash 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;
@@ -4068,57 +4062,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");
-
- 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;
- }
}
shash_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) {
@@ -4245,20 +4191,68 @@ port_lookup_iface(const struct port *port, const char *name)
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)
{
- 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);
}
}
--
1.7.1
More information about the dev
mailing list