In the original Open vSwitch LACP implementation, when no slaves
found a LACP partner, the LACP module would attach all of them.
This allowed the LACP bond to fall back to a standard bond when
partnered with a non-LACP switch. In practice, this has caused
confusion with marginal benefit, so this feature is removed with
this patch.
Signed-off-by: Ethan Jackson <ethan@nicira.com>
post-v1.5.0
------------------------
- bonding
post-v1.5.0
------------------------
- bonding
+ - LACP bonds no longer fall back to balance-slb when negotiations fail.
+ Instead they drop traffic.
- The default bond_mode changed from SLB to active-backup, to protect
unsuspecting users from the significant risks of SLB bonds (which are
documented in vswitchd/INTERNALS).
- The default bond_mode changed from SLB to active-backup, to protect
unsuspecting users from the significant risks of SLB bonds (which are
documented in vswitchd/INTERNALS).
#include "dynamic-string.h"
#include "flow.h"
#include "hmap.h"
#include "dynamic-string.h"
#include "flow.h"
#include "hmap.h"
#include "list.h"
#include "netdev.h"
#include "odp-util.h"
#include "list.h"
#include "netdev.h"
#include "odp-util.h"
struct bond_slave *active_slave;
tag_type no_slaves_tag; /* Tag for flows when all slaves disabled. */
int updelay, downdelay; /* Delay before slave goes up/down, in ms. */
struct bond_slave *active_slave;
tag_type no_slaves_tag; /* Tag for flows when all slaves disabled. */
int updelay, downdelay; /* Delay before slave goes up/down, in ms. */
- bool lacp_negotiated; /* LACP negotiations were successful. */
+ enum lacp_status lacp_status; /* Status of LACP negotiations. */
bool bond_revalidate; /* True if flows need revalidation. */
uint32_t basis; /* Basis for flow hash function. */
bool bond_revalidate; /* True if flows need revalidation. */
uint32_t basis; /* Basis for flow hash function. */
struct tag_set *);
static void bond_link_status_update(struct bond_slave *, struct tag_set *);
static void bond_choose_active_slave(struct bond *, struct tag_set *);
struct tag_set *);
static void bond_link_status_update(struct bond_slave *, struct tag_set *);
static void bond_choose_active_slave(struct bond *, struct tag_set *);
-static bool bond_is_tcp_hash(const struct bond *);
static unsigned int bond_hash_src(const uint8_t mac[ETH_ADDR_LEN],
uint16_t vlan, uint32_t basis);
static unsigned int bond_hash_tcp(const struct flow *, uint16_t vlan,
static unsigned int bond_hash_src(const uint8_t mac[ETH_ADDR_LEN],
uint16_t vlan, uint32_t basis);
static unsigned int bond_hash_tcp(const struct flow *, uint16_t vlan,
*
* The caller should check bond_should_send_learning_packets() afterward. */
void
*
* The caller should check bond_should_send_learning_packets() afterward. */
void
-bond_run(struct bond *bond, struct tag_set *tags, bool lacp_negotiated)
+bond_run(struct bond *bond, struct tag_set *tags, enum lacp_status lacp_status)
{
struct bond_slave *slave;
{
struct bond_slave *slave;
- bool is_tcp_hash = bond_is_tcp_hash(bond);
- if (bond->lacp_negotiated != lacp_negotiated) {
- bond->lacp_negotiated = lacp_negotiated;
+ if (bond->lacp_status != lacp_status) {
+ bond->lacp_status = lacp_status;
bond->bond_revalidate = true;
}
bond->bond_revalidate = true;
}
bond->next_fake_iface_update = time_msec() + 1000;
}
bond->next_fake_iface_update = time_msec() + 1000;
}
- if (is_tcp_hash != bond_is_tcp_hash(bond)) {
- bond->bond_revalidate = true;
- }
-
if (bond->bond_revalidate) {
bond->bond_revalidate = false;
if (bond->bond_revalidate) {
bond->bond_revalidate = false;
static bool
may_send_learning_packets(const struct bond *bond)
{
static bool
may_send_learning_packets(const struct bond *bond)
{
- return !bond->lacp_negotiated && bond->balance != BM_AB &&
- bond->active_slave;
+ return bond->lacp_status == LACP_DISABLED
+ && bond->balance != BM_AB
+ && bond->active_slave;
}
/* Returns true if 'bond' needs the client to send out packets to assist with
}
/* Returns true if 'bond' needs the client to send out packets to assist with
* assume the remote switch is aware of the bond and will "do the right
* thing". However, as a precaution we drop packets on disabled slaves
* because no correctly implemented partner switch should be sending
* assume the remote switch is aware of the bond and will "do the right
* thing". However, as a precaution we drop packets on disabled slaves
* because no correctly implemented partner switch should be sending
- * packets to them. */
- if (bond->lacp_negotiated) {
- return slave->enabled ? BV_ACCEPT : BV_DROP;
+ * packets to them.
+ *
+ * If LACP is configured, but LACP negotiations have been unsuccessful, we
+ * drop all incoming traffic. */
+ switch (bond->lacp_status) {
+ case LACP_NEGOTIATED: return slave->enabled ? BV_ACCEPT : BV_DROP;
+ case LACP_CONFIGURED: return BV_DROP;
+ case LACP_DISABLED: break;
}
/* Drop all multicast packets on inactive slaves. */
}
/* Drop all multicast packets on inactive slaves. */
return BV_ACCEPT;
case BM_TCP:
return BV_ACCEPT;
case BM_TCP:
- /* TCP balancing has degraded to SLB (otherwise the
- * bond->lacp_negotiated check above would have processed this).
- *
- * Fall through. */
+ /* TCP balanced bonds require successful LACP negotiated. Based on the
+ * above check, LACP is off on this bond. Therfore, we drop all
+ * incoming traffic. */
+ return BV_DROP;
+
case BM_SLB:
/* Drop all packets for which we have learned a different input port,
* because we probably sent the packet on one slave and got it back on
case BM_SLB:
/* Drop all packets for which we have learned a different input port,
* because we probably sent the packet on one slave and got it back on
ds_put_format(ds, "bond_mode: %s\n",
bond_mode_to_string(bond->balance));
ds_put_format(ds, "bond_mode: %s\n",
bond_mode_to_string(bond->balance));
- if (bond->balance != BM_AB) {
- ds_put_format(ds, "bond-hash-algorithm: %s\n",
- bond_is_tcp_hash(bond) ? "balance-tcp" : "balance-slb");
- }
-
ds_put_format(ds, "bond-hash-basis: %"PRIu32"\n", bond->basis);
ds_put_format(ds, "updelay: %d ms\n", bond->updelay);
ds_put_format(ds, "bond-hash-basis: %"PRIu32"\n", bond->basis);
ds_put_format(ds, "updelay: %d ms\n", bond->updelay);
bond->next_rebalance - time_msec());
}
bond->next_rebalance - time_msec());
}
- ds_put_format(ds, "lacp_negotiated: %s\n",
- bond->lacp_negotiated ? "true" : "false");
+ ds_put_cstr(ds, "lacp_status: ");
+ switch (bond->lacp_status) {
+ case LACP_NEGOTIATED:
+ ds_put_cstr(ds, "negotiated\n");
+ break;
+ case LACP_CONFIGURED:
+ ds_put_cstr(ds, "configured\n");
+ break;
+ case LACP_DISABLED:
+ ds_put_cstr(ds, "off\n");
+ break;
+ default:
+ ds_put_cstr(ds, "<unknown>\n");
+ break;
+ }
HMAP_FOR_EACH (slave, hmap_node, &bond->slaves) {
shash_add(&slave_shash, slave->name, slave);
HMAP_FOR_EACH (slave, hmap_node, &bond->slaves) {
shash_add(&slave_shash, slave->name, slave);
VLOG_INFO_RL(&rl, "interface %s: will not be %s",
slave->name, up ? "disabled" : "enabled");
} else {
VLOG_INFO_RL(&rl, "interface %s: will not be %s",
slave->name, up ? "disabled" : "enabled");
} else {
- int delay = (bond->lacp_negotiated ? 0
+ int delay = (bond->lacp_status != LACP_DISABLED ? 0
: up ? bond->updelay : bond->downdelay);
slave->delay_expires = time_msec() + delay;
if (delay) {
: up ? bond->updelay : bond->downdelay);
slave->delay_expires = time_msec() + delay;
if (delay) {
-static bool
-bond_is_tcp_hash(const struct bond *bond)
-{
- return (bond->balance == BM_TCP && bond->lacp_negotiated)
- || bond->balance == BM_STABLE;
-}
-
static unsigned int
bond_hash_src(const uint8_t mac[ETH_ADDR_LEN], uint16_t vlan, uint32_t basis)
{
static unsigned int
bond_hash_src(const uint8_t mac[ETH_ADDR_LEN], uint16_t vlan, uint32_t basis)
{
static unsigned int
bond_hash(const struct bond *bond, const struct flow *flow, uint16_t vlan)
{
static unsigned int
bond_hash(const struct bond *bond, const struct flow *flow, uint16_t vlan)
{
- assert(bond->balance != BM_AB);
+ assert(bond->balance == BM_TCP || bond->balance == BM_SLB);
- return (bond_is_tcp_hash(bond)
+ return (bond->balance == BM_TCP
? bond_hash_tcp(flow, vlan, bond->basis)
: bond_hash_src(flow->dl_src, vlan, bond->basis));
}
? bond_hash_tcp(flow, vlan, bond->basis)
: bond_hash_src(flow->dl_src, vlan, bond->basis));
}
best = NULL;
best_hash = 0;
best = NULL;
best_hash = 0;
- flow_hash = bond_hash(bond, flow, vlan);
+ flow_hash = bond_hash_tcp(flow, vlan, bond->basis);
HMAP_FOR_EACH (slave, hmap_node, &bond->slaves) {
if (slave->enabled) {
uint32_t hash;
HMAP_FOR_EACH (slave, hmap_node, &bond->slaves) {
if (slave->enabled) {
uint32_t hash;
+ if (bond->lacp_status == LACP_CONFIGURED) {
+ /* LACP has been configured on this bond but negotiations were
+ * unsuccussful. Drop all traffic. */
+ return NULL;
+ }
+
switch (bond->balance) {
case BM_AB:
return bond->active_slave;
case BM_STABLE:
return choose_stb_slave(bond, flow, vlan);
switch (bond->balance) {
case BM_AB:
return bond->active_slave;
case BM_STABLE:
return choose_stb_slave(bond, flow, vlan);
+ if (bond->lacp_status != LACP_NEGOTIATED) {
+ /* Must have LACP negotiations for TCP balanced bonds. */
+ return NULL;
+ }
+ /* Fall Through. */
+ case BM_SLB:
e = lookup_bond_entry(bond, flow, vlan);
if (!e->slave || !e->slave->enabled) {
e->slave = CONTAINER_OF(hmap_random_node(&bond->slaves),
e = lookup_bond_entry(bond, flow, vlan);
if (!e->slave || !e->slave->enabled) {
e->slave = CONTAINER_OF(hmap_random_node(&bond->slaves),
struct flow;
struct netdev;
struct ofpbuf;
struct flow;
struct netdev;
struct ofpbuf;
/* How flows are balanced among bond slaves. */
enum bond_mode {
/* How flows are balanced among bond slaves. */
enum bond_mode {
void bond_slave_set_netdev(struct bond *, void *slave_, struct netdev *);
void bond_slave_unregister(struct bond *, const void *slave);
void bond_slave_set_netdev(struct bond *, void *slave_, struct netdev *);
void bond_slave_unregister(struct bond *, const void *slave);
-void bond_run(struct bond *, struct tag_set *, bool lacp_negotiated);
+void bond_run(struct bond *, struct tag_set *, enum lacp_status);
void bond_wait(struct bond *);
void bond_slave_set_may_enable(struct bond *, void *slave_, bool may_enable);
void bond_wait(struct bond *);
void bond_slave_set_may_enable(struct bond *, void *slave_, bool may_enable);
-/* Returns true if 'lacp' has successfully negotiated with its partner. False
- * if 'lacp' is NULL. */
-bool
-lacp_negotiated(const struct lacp *lacp)
+/* Returns the lacp_status of the given 'lacp' object (which may be NULL). */
+enum lacp_status
+lacp_status(const struct lacp *lacp)
- return lacp ? lacp->negotiated : false;
+ if (!lacp) {
+ return LACP_DISABLED;
+ } else if (lacp->negotiated) {
+ return LACP_NEGOTIATED;
+ } else {
+ return LACP_CONFIGURED;
+ }
}
/* Registers 'slave_' as subordinate to 'lacp'. This should be called at least
}
/* Registers 'slave_' as subordinate to 'lacp'. This should be called at least
struct slave *slave = slave_lookup(lacp, slave_);
/* The slave may be enabled if it's attached to an aggregator and its
struct slave *slave = slave_lookup(lacp, slave_);
/* The slave may be enabled if it's attached to an aggregator and its
- * partner is synchronized. The only exception is defaulted slaves.
- * They are not required to have synchronized partners because they
- * have no partners at all. They will only be attached if negotiations
- * failed on all slaves in the bond. */
- return slave->attached && (slave->partner.state & LACP_STATE_SYNC
- || slave->status == LACP_DEFAULTED);
+ * partner is synchronized.*/
+ return slave->attached && (slave->partner.state & LACP_STATE_SYNC);
HMAP_FOR_EACH (slave, node, &lacp->slaves) {
struct lacp_info pri;
HMAP_FOR_EACH (slave, node, &lacp->slaves) {
struct lacp_info pri;
- slave->attached = true;
+ slave->attached = false;
/* XXX: In the future allow users to configure the expected system ID.
* For now just special case loopback. */
if (eth_addr_equals(slave->partner.sys_id, slave->lacp->sys_id)) {
VLOG_WARN_RL(&rl, "slave %s: Loopback detected. Slave is "
"connected to its own bond", slave->name);
/* XXX: In the future allow users to configure the expected system ID.
* For now just special case loopback. */
if (eth_addr_equals(slave->partner.sys_id, slave->lacp->sys_id)) {
VLOG_WARN_RL(&rl, "slave %s: Loopback detected. Slave is "
"connected to its own bond", slave->name);
- slave->attached = false;
+ slave->attached = true;
slave_get_priority(slave, &pri);
if (!lead || memcmp(&pri, &lead_pri, sizeof pri) < 0) {
slave_get_priority(slave, &pri);
if (!lead || memcmp(&pri, &lead_pri, sizeof pri) < 0) {
if (lead) {
HMAP_FOR_EACH (slave, node, &lacp->slaves) {
if (lead) {
HMAP_FOR_EACH (slave, node, &lacp->slaves) {
- if (slave->status == LACP_DEFAULTED
- || lead->partner.key != slave->partner.key
+ if (lead->partner.key != slave->partner.key
|| !eth_addr_equals(lead->partner.sys_id,
slave->partner.sys_id)) {
slave->attached = false;
|| !eth_addr_equals(lead->partner.sys_id,
slave->partner.sys_id)) {
slave->attached = false;
LACP_TIME_CUSTOM /* Nonstandard custom mode. */
};
LACP_TIME_CUSTOM /* Nonstandard custom mode. */
};
+enum lacp_status {
+ LACP_NEGOTIATED, /* Successful LACP negotations. */
+ LACP_CONFIGURED, /* LACP is enabled but not negotiated. */
+ LACP_DISABLED /* LACP is not enabled. */
+};
+
struct lacp_settings {
char *name; /* Name (for debugging). */
uint8_t id[ETH_ADDR_LEN]; /* System ID. Must be nonzero. */
struct lacp_settings {
char *name; /* Name (for debugging). */
uint8_t id[ETH_ADDR_LEN]; /* System ID. Must be nonzero. */
void lacp_process_packet(struct lacp *, const void *slave,
const struct ofpbuf *packet);
void lacp_process_packet(struct lacp *, const void *slave,
const struct ofpbuf *packet);
-bool lacp_negotiated(const struct lacp *);
+enum lacp_status lacp_status(const struct lacp *);
struct lacp_slave_settings {
char *name; /* Name (for debugging). */
struct lacp_slave_settings {
char *name; /* Name (for debugging). */
}
bond_run(bundle->bond, &bundle->ofproto->revalidate_set,
}
bond_run(bundle->bond, &bundle->ofproto->revalidate_set,
- lacp_negotiated(bundle->lacp));
+ lacp_status(bundle->lacp));
if (bond_should_send_learning_packets(bundle->bond)) {
bundle_send_learning_packets(bundle);
}
if (bond_should_send_learning_packets(bundle->bond)) {
bundle_send_learning_packets(bundle);
}
bond-hash-basis: 0
updelay: 0 ms
downdelay: 0 ms
bond-hash-basis: 0
updelay: 0 ms
downdelay: 0 ms
slave p1: disabled
may_enable: false
slave p1: disabled
may_enable: false
<p>
The following modes require the upstream switch to support 802.3ad with
<p>
The following modes require the upstream switch to support 802.3ad with
- successful LACP negotiation. If LACP negotiation fails then
- <code>balance-slb</code> style flow hashing is used as a fallback:
+ successful LACP negotiation:
connected to. <code>active</code> ports are allowed to initiate LACP
negotiations. <code>passive</code> ports are allowed to participate
in LACP negotiations initiated by a remote switch, but not allowed to
connected to. <code>active</code> ports are allowed to initiate LACP
negotiations. <code>passive</code> ports are allowed to participate
in LACP negotiations initiated by a remote switch, but not allowed to
- initiate such negotiations themselves. Defaults to <code>off</code>
- if unset.
+ initiate such negotiations themselves. If LACP is enabled on a port
+ whose partner switch does not support LACP, the bond will be
+ disabled. Defaults to <code>off</code> if unset.
</column>
<column name="other_config" key="lacp-system-id">
</column>
<column name="other_config" key="lacp-system-id">