[ovs-dev] [PATCH v2] bond: Use active-backup mode on LACP failure.
Ethan Jackson
ethan at nicira.com
Tue Nov 12 22:25:56 UTC 2013
Sorry it took me so long to get to this. I've merged the patch to
master with some minor style tweaks.
Thanks,
Ethan
On Mon, Nov 4, 2013 at 3:07 PM, Ravi Kondamuru
<Ravi.Kondamuru at citrix.com> wrote:
> Commit bdebeece5 (lacp: Require successful LACP negotiations when
> configured.) makes successful LACP negotiation mandatory for the
> bond to come UP. This patch provides a configuration option to
> bring up the bond by falling back to active-backup mode on LACP
> negotiation failure.
>
> Several of the physical switches that support LACP block all traffic
> for ports that are configured to use LACP, until LACP is negotiated
> with the host. When configuring a LACP bond on a OVS host
> (eg: XenServer), this means that there will be an interruption of the
> network connectivity between the time the ports on the physical
> switch and the bond on the OVS host are configured. The interruption
> may be relatively long, if different people are responsible for
> managing the switches and the OVS host.
>
> Such network connectivity failure can be avoided if LACP can be
> configured on the OVS host before configuring the physical switch,
> and having the OVS host fall back to a bond mode (active-backup) till
> the physical switch LACP configuration is complete. An option
> "lacp-fallback-ab" is introduced with this patch to provide such
> behavior on openvswitch.
>
> Signed-off-by: Ravi Kondamuru <Ravi.Kondamuru at citrix.com>
> Signed-off-by: Dominic Curran <Dominic.Curran at citrix.com>
> ---
> lib/bond.c | 46 ++++++++++++++++++++++++++++++++--------------
> lib/bond.h | 1 +
> lib/lacp.c | 17 +++++++++++++++--
> lib/lacp.h | 1 +
> vswitchd/INTERNALS | 14 ++++++++++++++
> vswitchd/bridge.c | 18 ++++++++++++++++++
> vswitchd/vswitch.xml | 19 +++++++++++++++++--
> 7 files changed, 98 insertions(+), 18 deletions(-)
>
> diff --git a/lib/bond.c b/lib/bond.c
> index 5be1bae..831f9df 100644
> --- a/lib/bond.c
> +++ b/lib/bond.c
> @@ -99,6 +99,7 @@ struct bond {
>
> /* Legacy compatibility. */
> long long int next_fake_iface_update; /* LLONG_MAX if disabled. */
> + bool lacp_fallback_ab; /* Fallback to active-backup on LACP failure. */
>
> atomic_int ref_cnt;
> };
> @@ -258,6 +259,11 @@ bond_reconfigure(struct bond *bond, const struct bond_settings *s)
> bond->updelay = s->up_delay;
> bond->downdelay = s->down_delay;
>
> + if (bond->lacp_fallback_ab != s->lacp_fallback_ab_cfg) {
> + bond->lacp_fallback_ab = s->lacp_fallback_ab_cfg;
> + revalidate = true;
> + }
> +
> if (bond->rebalance_interval != s->rebalance_interval) {
> bond->rebalance_interval = s->rebalance_interval;
> revalidate = true;
> @@ -491,8 +497,9 @@ bond_wait(struct bond *bond)
> static bool
> may_send_learning_packets(const struct bond *bond)
> {
> - return bond->lacp_status == LACP_DISABLED
> - && (bond->balance == BM_SLB || bond->balance == BM_AB)
> + return ((bond->lacp_status == LACP_DISABLED
> + && (bond->balance == BM_SLB || bond->balance == BM_AB))
> + || (bond->lacp_fallback_ab && bond->lacp_status == LACP_CONFIGURED))
> && bond->active_slave;
> }
>
> @@ -585,13 +592,15 @@ bond_check_admissibility(struct bond *bond, const void *slave_,
> * packets to them.
> *
> * If LACP is configured, but LACP negotiations have been unsuccessful, we
> - * drop all incoming traffic. */
> + * drop all incoming traffic except if lacp_fallback_ab is enabled. */
> switch (bond->lacp_status) {
> case LACP_NEGOTIATED:
> verdict = slave->enabled ? BV_ACCEPT : BV_DROP;
> goto out;
> case LACP_CONFIGURED:
> - goto out;
> + if (!bond->lacp_fallback_ab) {
> + goto out;
> + }
> case LACP_DISABLED:
> break;
> }
> @@ -604,6 +613,15 @@ bond_check_admissibility(struct bond *bond, const void *slave_,
> }
>
> switch (bond->balance) {
> + case BM_TCP:
> + /* TCP balanced bonds require successful LACP negotiations. Based on the
> + * above check, LACP is off or lacp_fallback_ab is true on this bond.
> + * If lacp_fallback_ab is true fall through to BM_AB case else, we
> + * drop all incoming traffic. */
> + if (!bond->lacp_fallback_ab) {
> + goto out;
> + }
> +
> case BM_AB:
> /* Drop all packets which arrive on backup slaves. This is similar to
> * how Linux bonding handles active-backup bonds. */
> @@ -618,12 +636,6 @@ bond_check_admissibility(struct bond *bond, const void *slave_,
> verdict = BV_ACCEPT;
> goto out;
>
> - case BM_TCP:
> - /* TCP balanced bonds require successful LACP negotiated. Based on the
> - * above check, LACP is off on this bond. Therfore, we drop all
> - * incoming traffic. */
> - goto out;
> -
> 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
> @@ -1416,14 +1428,20 @@ choose_output_slave(const struct bond *bond, const struct flow *flow,
> struct flow_wildcards *wc, uint16_t vlan)
> {
> struct bond_entry *e;
> -
> + int balance;
> +
> + balance = bond->balance;
> if (bond->lacp_status == LACP_CONFIGURED) {
> /* LACP has been configured on this bond but negotiations were
> - * unsuccussful. Drop all traffic. */
> - return NULL;
> + * unsuccussful. If lacp_fallback_ab is enabled use active-
> + * backup mode else drop all traffic. */
> + if (!bond->lacp_fallback_ab) {
> + return NULL;
> + }
> + balance = BM_AB;
> }
>
> - switch (bond->balance) {
> + switch (balance) {
> case BM_AB:
> return bond->active_slave;
>
> diff --git a/lib/bond.h b/lib/bond.h
> index f80fead..5b3814e 100644
> --- a/lib/bond.h
> +++ b/lib/bond.h
> @@ -53,6 +53,7 @@ struct bond_settings {
>
> /* Legacy compatibility. */
> bool fake_iface; /* Update fake stats for netdev 'name'? */
> + bool lacp_fallback_ab_cfg; /* Fallback to active-backup on LACP failure. */
> };
>
> /* Program startup. */
> diff --git a/lib/lacp.c b/lib/lacp.c
> index 5421e2a..fce65b3 100644
> --- a/lib/lacp.c
> +++ b/lib/lacp.c
> @@ -102,6 +102,7 @@ struct lacp {
> bool fast; /* True if using fast probe interval. */
> bool negotiated; /* True if LACP negotiations were successful. */
> bool update; /* True if lacp_update() needs to be called. */
> + bool fallback_ab; /* True if fallback to active-backup on LACP failure. */
>
> atomic_int ref_cnt;
> };
> @@ -283,6 +284,12 @@ lacp_configure(struct lacp *lacp, const struct lacp_settings *s)
>
> lacp->active = s->active;
> lacp->fast = s->fast;
> +
> + if (lacp->fallback_ab != s->fallback_ab_cfg) {
> + lacp->fallback_ab = s->fallback_ab_cfg;
> + lacp->update = true;
> + }
> +
> ovs_mutex_unlock(&mutex);
> }
>
> @@ -450,7 +457,9 @@ slave_may_enable__(struct slave *slave) OVS_REQUIRES(mutex)
> {
> /* The slave may be enabled if it's attached to an aggregator and its
> * partner is synchronized.*/
> - return slave->attached && (slave->partner.state & LACP_STATE_SYNC);
> + return slave->attached && (slave->partner.state & LACP_STATE_SYNC
> + || (slave->lacp && slave->lacp->fallback_ab
> + && slave->status == LACP_DEFAULTED));
> }
>
> /* This function should be called before enabling 'slave_' to send or receive
> @@ -587,6 +596,9 @@ lacp_update_attached(struct lacp *lacp) OVS_REQUIRES(mutex)
> }
>
> if (slave->status == LACP_DEFAULTED) {
> + if (lacp->fallback_ab) {
> + slave->attached = true;
> + }
> continue;
> }
>
> @@ -603,7 +615,8 @@ lacp_update_attached(struct lacp *lacp) OVS_REQUIRES(mutex)
>
> if (lead) {
> HMAP_FOR_EACH (slave, node, &lacp->slaves) {
> - if (lead->partner.key != slave->partner.key
> + if ((lacp->fallback_ab && slave->status == LACP_DEFAULTED)
> + || lead->partner.key != slave->partner.key
> || !eth_addr_equals(lead->partner.sys_id,
> slave->partner.sys_id)) {
> slave->attached = false;
> diff --git a/lib/lacp.h b/lib/lacp.h
> index 89b0e0a..593b80d 100644
> --- a/lib/lacp.h
> +++ b/lib/lacp.h
> @@ -35,6 +35,7 @@ struct lacp_settings {
> uint16_t priority; /* System priority. */
> bool active; /* Active or passive mode? */
> bool fast; /* Fast or slow probe interval. */
> + bool fallback_ab_cfg; /* Fallback to BM_SLB on LACP failure. */
> };
>
> void lacp_init(void);
> diff --git a/vswitchd/INTERNALS b/vswitchd/INTERNALS
> index 2b48c9b..994353d 100644
> --- a/vswitchd/INTERNALS
> +++ b/vswitchd/INTERNALS
> @@ -143,6 +143,20 @@ LACP bonding requires the remote switch to implement LACP, but it is
> otherwise very simple in that, after LACP negotiation is complete,
> there is no need for special handling of received packets.
>
> +Several of the physical switches that support LACP block all traffic
> +for ports that are configured to use LACP, until LACP is negotiated with
> +the host. When configuring a LACP bond on a OVS host (eg: XenServer),
> +this means that there will be an interruption of the network connectivity
> +between the time the ports on the physical switch and the bond on the OVS
> +host are configured. The interruption may be relatively long, if different
> +people are responsible for managing the switches and the OVS host.
> +
> +Such network connectivity failure can be avoided if LACP can be configured
> +on the OVS host before configuring the physical switch, and having
> +the OVS host fall back to a bond mode (active-backup) till the physical
> +switch LACP configuration is complete. An option "lacp-fallback-ab" exists to
> +provide such behavior on openvswitch.
> +
> Active Backup Bonding
> ---------------------
>
> diff --git a/vswitchd/bridge.c b/vswitchd/bridge.c
> index ec3633c..7d5f87d 100644
> --- a/vswitchd/bridge.c
> +++ b/vswitchd/bridge.c
> @@ -235,6 +235,7 @@ static struct lacp_settings *port_configure_lacp(struct port *,
> struct lacp_settings *);
> static void port_configure_bond(struct port *, struct bond_settings *);
> static bool port_is_synthetic(const struct port *);
> +static bool set_lacp_fallback_ab_cfg(struct port *);
>
> static void reconfigure_system_stats(const struct ovsrec_open_vswitch *);
> static void run_system_stats(void);
> @@ -3283,6 +3284,18 @@ enable_lacp(struct port *port, bool *activep)
> }
> }
>
> +static bool
> +set_lacp_fallback_ab_cfg(struct port *port)
> +{
> + const char *lacp_fallback_s;
> +
> + lacp_fallback_s = smap_get(&port->cfg->other_config, "lacp-fallback-ab");
> + if (lacp_fallback_s && !strcmp(lacp_fallback_s, "true"))
> + return true;
> +
> + return false;
> +}
> +
> static struct lacp_settings *
> port_configure_lacp(struct port *port, struct lacp_settings *s)
> {
> @@ -3321,6 +3334,9 @@ port_configure_lacp(struct port *port, struct lacp_settings *s)
>
> lacp_time = smap_get(&port->cfg->other_config, "lacp-time");
> s->fast = lacp_time && !strcasecmp(lacp_time, "fast");
> +
> + s->fallback_ab_cfg = set_lacp_fallback_ab_cfg(port);
> +
> return s;
> }
>
> @@ -3409,6 +3425,8 @@ port_configure_bond(struct port *port, struct bond_settings *s)
>
> s->fake_iface = port->cfg->bond_fake_iface;
>
> + s->lacp_fallback_ab_cfg = set_lacp_fallback_ab_cfg(port);
> +
> LIST_FOR_EACH (iface, port_elem, &port->ifaces) {
> netdev_set_miimon_interval(iface->netdev, miimon_interval);
> }
> diff --git a/vswitchd/vswitch.xml b/vswitchd/vswitch.xml
> index 4ad0d63..f2a1b1e 100644
> --- a/vswitchd/vswitch.xml
> +++ b/vswitchd/vswitch.xml
> @@ -941,7 +941,9 @@
>
> <p>
> The following modes require the upstream switch to support 802.3ad with
> - successful LACP negotiation:
> + successful LACP negotiation. If LACP negotiation fails and
> + other-config:lacp-fallback-ab is true, then <code>active-backup</code>
> + mode is used:
> </p>
>
> <dl>
> @@ -1031,7 +1033,8 @@
> in LACP negotiations initiated by a remote switch, but not allowed to
> 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.
> + disabled, unless other-config:lacp-fallback-ab is set to true.
> + Defaults to <code>off</code> if unset.
> </column>
>
> <column name="other_config" key="lacp-system-id">
> @@ -1059,6 +1062,18 @@
> rate of once every 30 seconds.
> </p>
> </column>
> +
> + <column name="other_config" key="lacp-fallback-ab"
> + type='{"type": "boolean"}'>
> + <p>
> + Determines the behavior of openvswitch bond in LACP mode. If
> + the partner switch does not support LACP, setting this option
> + to <code>true</code> allows openvswitch to fallback to
> + active-backup. If the option is set to <code>false</code>, the
> + bond will be disabled. In both the cases, once the partner switch
> + is configured to LACP mode, the bond will use LACP.
> + </p>
> + </column>
> </group>
>
> <group title="Rebalancing Configuration">
> --
> 1.7.10.4
>
> _______________________________________________
> dev mailing list
> dev at openvswitch.org
> http://openvswitch.org/mailman/listinfo/dev
More information about the dev
mailing list