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.kondam...@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.kondam...@citrix.com>
> Signed-off-by: Dominic Curran <dominic.cur...@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@openvswitch.org
> http://openvswitch.org/mailman/listinfo/dev
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
