Message ID | 20200625210334.12081-1-jsquyres@cisco.com |
---|---|
State | Changes Requested |
Headers | show |
Series | [ovs-dev,v9] AB bonding: Add "primary" interface concept | expand |
Gentle nudge. Any further feedback on this one? Thank you! > On Jun 25, 2020, at 5:03 PM, Jeff Squyres <jsquyres@cisco.com> wrote: > > In AB bonding, if the current active slave becomes disabled, a > replacement slave is arbitrarily picked from the remaining set of > enabled slaves. This commit adds the concept of a "primary" slave: an > interface that will always be (or become) the current active slave if > it is enabled. > > The rationale for this functionality is to allow the designation of a > preferred interface for a given bond. For example: > > 1. Bond is created with interfaces p1 (primary) and p2, both enabled. > 2. p1 becomes the current active slave (because it was designated as > the primary). > 3. Later, p1 fails/becomes disabled. > 4. p2 is chosen to become the current active slave. > 5. Later, p1 becomes re-enabled. > 6. p1 is chosen to become the current active slave (because it was > designated as the primary) > > Note that p1 becomes the active slave once it becomes re-enabled, even > if nothing has happened to p2. > > This "primary" concept exists in Linux kernel network interface > bonding, but did not previously exist in OVS bonding. > > Only one primary slave inteface is supported per bond, and is only > supported for active/backup bonding. > > The primary slave interface is designated via > "other_config:bond-primary" when creating a bond. > > Also, while adding tests for the "primary" concept, make a few small > improvements to the non-primary AB bonding test. > > Signed-off-by: Jeff Squyres <jsquyres@cisco.com> > Reviewed-by: Aaron Conole <aconole@redhat.com> > Tested-by: Greg Rose <gvrose8192@gmail.com> > Acked-by: Greg Rose <gvrose8192@gmail.com> > --- > v9: > - Removed stale code that should have been removed in v8. > - Fixed comment punctuation. > > v8: > - Many style and simplification suggestions from Ilya Maximets > - Rename "primary:" to "active-backup primary:" in ovs-appctl > bond/show output, and make it always appear (vs. only appearing in > explicit AB bonding scenarios). > - Properly handle fallback-to-AB-bonding cases > - Use time/warp and OVS_WAIT_UNTIL in the bonding tests to prevent > races, and a few other improvements to the test quality > - After rebasing patch to head of tree, adjust test output as result > of changes from other commits. > > v7: > - After rebasing patch to head of tree, adjust test output as result > of changes from 81f71381f. > > v6: > - Style: bleep bloop. Fix use of {}. > > v5: > - Handle when bond is reconfigured to remove "bond-primary" config. > - Fix potential flapping between remaining slaves. > - Add a test to re-add a removed primary interface and make sure the > bond reflects that properly. > - Add a test to remove "bond-primary" config and make sure the bond > reflects that properly. > > v4: > - Style: bleep bloop. Trim line length below 79 characters. > > v3: > - Adjusted print logic for when the primary slave is not attached > > v2: > - Added "ovs-vsctl bond/show" label when primary interface is no > longer enslaved > - Fixed strcmp() usage nits > - Moved "other_config:bond-primary" docs to the "Bonding > Configuration" group > - Expanded commit message > - Added more test cases (including one for when the primary interface > is no longer enslaved) > > ofproto/bond.c | 54 ++++++++-- > ofproto/bond.h | 1 + > tests/lacp.at | 9 ++ > tests/ofproto-dpif.at | 224 +++++++++++++++++++++++++++++++++++++++--- > vswitchd/bridge.c | 5 + > vswitchd/vswitch.xml | 8 ++ > 6 files changed, 280 insertions(+), 21 deletions(-) > > diff --git a/ofproto/bond.c b/ofproto/bond.c > index 9947e7531..58ee67beb 100644 > --- a/ofproto/bond.c > +++ b/ofproto/bond.c > @@ -89,6 +89,7 @@ struct bond_slave { > /* Link status. */ > bool enabled; /* May be chosen for flows? */ > bool may_enable; /* Client considers this slave bondable. */ > + bool is_primary; /* This slave is preferred over others. */ > long long delay_expires; /* Time after which 'enabled' may change. */ > > /* Rebalancing info. Used only by bond_rebalance(). */ > @@ -124,6 +125,7 @@ struct bond { > uint32_t basis; /* Basis for flow hash function. */ > bool use_lb_output_action; /* Use lb_output action to avoid recirculation. > Applicable only for Balance TCP mode. */ > + char *primary; /* Name of the primary slave interface. */ > > /* SLB specific bonding info. */ > struct bond_entry *hash; /* An array of BOND_BUCKETS elements. */ > @@ -241,6 +243,7 @@ bond_create(const struct bond_settings *s, struct ofproto_dpif *ofproto) > > bond->active_slave_mac = eth_addr_zero; > bond->active_slave_changed = false; > + bond->primary = NULL; > > bond_reconfigure(bond, s); > return bond; > @@ -294,6 +297,7 @@ bond_unref(struct bond *bond) > update_recirc_rules__(bond); > > hmap_destroy(&bond->pr_rule_ops); > + free(bond->primary); > free(bond->name); > free(bond); > } > @@ -471,6 +475,12 @@ bond_reconfigure(struct bond *bond, const struct bond_settings *s) > bond->bond_revalidate = false; > } > > + if (!nullable_string_is_equal(bond->primary, s->primary)) { > + free(bond->primary); > + bond->primary = nullable_xstrdup(s->primary); > + revalidate = true; > + } > + > if (bond->balance != BM_AB) { > if (!bond->recirc_id) { > bond->recirc_id = recirc_alloc_id(bond->ofproto); > @@ -575,6 +585,12 @@ bond_slave_register(struct bond *bond, void *slave_, > slave->name = xstrdup(netdev_get_name(netdev)); > bond->bond_revalidate = true; > > + if (bond->primary && !strcmp(bond->primary, slave->name)) { > + slave->is_primary = true; > + } else { > + slave->is_primary = false; > + } > + > slave->enabled = false; > bond_enable_slave(slave, netdev_get_carrier(netdev)); > } > @@ -668,7 +684,7 @@ bond_slave_set_may_enable(struct bond *bond, void *slave_, bool may_enable) > bool > bond_run(struct bond *bond, enum lacp_status lacp_status) > { > - struct bond_slave *slave; > + struct bond_slave *slave, *primary; > bool revalidate; > > ovs_rwlock_wrlock(&rwlock); > @@ -685,11 +701,19 @@ bond_run(struct bond *bond, enum lacp_status lacp_status) > } > > /* Enable slaves based on link status and LACP feedback. */ > + primary = NULL; > HMAP_FOR_EACH (slave, hmap_node, &bond->slaves) { > bond_link_status_update(slave); > slave->change_seq = seq_read(connectivity_seq_get()); > + > + /* Discover if there is an active slave marked 'primary'. */ > + if (bond->balance == BM_AB && slave->is_primary && slave->enabled) { > + primary = slave; > + } > } > - if (!bond->active_slave || !bond->active_slave->enabled) { > + > + if (!bond->active_slave || !bond->active_slave->enabled || > + (primary && bond->active_slave != primary)) { > bond_choose_active_slave(bond); > } > > @@ -1437,16 +1461,25 @@ bond_print_details(struct ds *ds, const struct bond *bond) > ds_put_format(ds, "lacp_fallback_ab: %s\n", > bond->lacp_fallback_ab ? "true" : "false"); > > - ds_put_cstr(ds, "active slave mac: "); > - ds_put_format(ds, ETH_ADDR_FMT, ETH_ADDR_ARGS(bond->active_slave_mac)); > + bool found_primary = false; > slave = bond_find_slave_by_mac(bond, bond->active_slave_mac); > - ds_put_format(ds,"(%s)\n", slave ? slave->name : "none"); > - > HMAP_FOR_EACH (slave, hmap_node, &bond->slaves) { > + if (slave->is_primary) { > + found_primary = true; > + } > shash_add(&slave_shash, slave->name, slave); > } > - sorted_slaves = shash_sort(&slave_shash); > > + ds_put_format(ds, "active-backup primary: %s%s\n", > + bond->primary ? bond->primary : "<none>", > + (!found_primary && bond->primary) > + ? " (no such slave)" : ""); > + > + ds_put_cstr(ds, "active slave mac: "); > + ds_put_format(ds, ETH_ADDR_FMT, ETH_ADDR_ARGS(bond->active_slave_mac)); > + ds_put_format(ds,"(%s)\n", slave ? slave->name : "none"); > + > + sorted_slaves = shash_sort(&slave_shash); > for (i = 0; i < shash_count(&slave_shash); i++) { > struct bond_entry *be; > > @@ -1906,6 +1939,13 @@ bond_choose_slave(const struct bond *bond) > { > struct bond_slave *slave, *best; > > + /* If there's a primary and it's active, return that. */ > + HMAP_FOR_EACH (slave, hmap_node, &bond->slaves) { > + if (slave->is_primary && slave->enabled) { > + return slave; > + } > + } > + > /* Find the last active slave. */ > slave = bond_find_slave_by_mac(bond, bond->active_slave_mac); > if (slave && slave->enabled) { > diff --git a/ofproto/bond.h b/ofproto/bond.h > index 40c3258dc..1a42a37c9 100644 > --- a/ofproto/bond.h > +++ b/ofproto/bond.h > @@ -45,6 +45,7 @@ struct bond_settings { > > /* Balancing configuration. */ > enum bond_mode balance; > + const char *primary; /* For AB balance, primary interface name. */ > int rebalance_interval; /* Milliseconds between rebalances. > Zero to disable rebalancing. */ > > diff --git a/tests/lacp.at b/tests/lacp.at > index df1691731..5257f0cce 100644 > --- a/tests/lacp.at > +++ b/tests/lacp.at > @@ -126,6 +126,7 @@ updelay: 0 ms > downdelay: 0 ms > lacp_status: negotiated > lacp_fallback_ab: false > +active-backup primary: <none> > active slave mac: 00:00:00:00:00:00(none) > > slave p1: disabled > @@ -292,6 +293,7 @@ updelay: 0 ms > downdelay: 0 ms > lacp_status: negotiated > lacp_fallback_ab: false > +active-backup primary: <none> > > slave p0: enabled > may_enable: true > @@ -308,6 +310,7 @@ updelay: 0 ms > downdelay: 0 ms > lacp_status: negotiated > lacp_fallback_ab: false > +active-backup primary: <none> > > slave p2: enabled > may_enable: true > @@ -431,6 +434,7 @@ updelay: 0 ms > downdelay: 0 ms > lacp_status: negotiated > lacp_fallback_ab: false > +active-backup primary: <none> > <active slave mac del> > > slave p0: disabled > @@ -449,6 +453,7 @@ updelay: 0 ms > downdelay: 0 ms > lacp_status: negotiated > lacp_fallback_ab: false > +active-backup primary: <none> > <active slave mac del> > > slave p2: disabled > @@ -565,6 +570,7 @@ updelay: 0 ms > downdelay: 0 ms > lacp_status: negotiated > lacp_fallback_ab: false > +active-backup primary: <none> > <active slave mac del> > > slave p0: disabled > @@ -583,6 +589,7 @@ updelay: 0 ms > downdelay: 0 ms > lacp_status: negotiated > lacp_fallback_ab: false > +active-backup primary: <none> > <active slave mac del> > > slave p2: disabled > @@ -704,6 +711,7 @@ updelay: 0 ms > downdelay: 0 ms > lacp_status: negotiated > lacp_fallback_ab: false > +active-backup primary: <none> > <active slave mac del> > > slave p0: enabled > @@ -722,6 +730,7 @@ updelay: 0 ms > downdelay: 0 ms > lacp_status: negotiated > lacp_fallback_ab: false > +active-backup primary: <none> > <active slave mac del> > > slave p2: enabled > diff --git a/tests/ofproto-dpif.at b/tests/ofproto-dpif.at > index e3402e7b8..808200800 100644 > --- a/tests/ofproto-dpif.at > +++ b/tests/ofproto-dpif.at > @@ -29,12 +29,16 @@ AT_CHECK([ovs-appctl revalidator/wait]) > OVS_VSWITCHD_STOP > AT_CLEANUP > > -AT_SETUP([ofproto-dpif - active-backup bonding]) > -# Create br0 with interfaces p1, p2 and p7, creating bond0 with p1 and p2 > -# and br1 with interfaces p3, p4 and p8. > -# toggle p1,p2 of bond0 up and down to test bonding in active-backup mode. > +AT_SETUP([ofproto-dpif - active-backup bonding (with primary)]) > + > +dnl Create br0 with interfaces p1, p2 and p7, creating bond0 with p1 and > +dnl p2 (p1 as primary) and br1 with interfaces p3, p4 and p8. > +dnl toggle p1,p2 of bond0 up and down to test bonding in active-backup mode. > +dnl With p1 down and p2 up/active, bring p1 back up. Since p1 is the primary, > +dnl it should become active. > OVS_VSWITCHD_START( > - [add-bond br0 bond0 p1 p2 bond_mode=active-backup --\ > + [add-bond br0 bond0 p1 p2 bond_mode=active-backup \ > + other_config:bond-primary=p1 -- \ > set interface p1 type=dummy options:pstream=punix:$OVS_RUNDIR/p1.sock ofport_request=1 -- \ > set interface p2 type=dummy options:pstream=punix:$OVS_RUNDIR/p2.sock ofport_request=2 -- \ > add-port br0 p7 -- set interface p7 ofport_request=7 type=dummy -- \ > @@ -45,9 +49,202 @@ OVS_VSWITCHD_START( > add-port br1 p3 -- set interface p3 type=dummy options:stream=unix:$OVS_RUNDIR/p1.sock ofport_request=3 -- \ > add-port br1 p4 -- set interface p4 type=dummy options:stream=unix:$OVS_RUNDIR/p2.sock ofport_request=4 -- \ > add-port br1 p8 -- set interface p8 ofport_request=8 type=dummy --]) > +AT_CHECK([ovs-appctl vlog/set dpif:dbg dpif_netdev:dbg]) > WAIT_FOR_DUMMY_PORTS([p3], [p4]) > +OVS_WAIT_UNTIL([test -n "`ovs-appctl bond/show | grep 'active-backup primary: p1'`"]) > + > + > +AT_CHECK([ovs-ofctl add-flow br0 action=normal]) > +AT_CHECK([ovs-ofctl add-flow br1 action=normal]) > +ovs-appctl netdev-dummy/set-admin-state up > +ovs-appctl time/warp 100 > +ovs-appctl netdev-dummy/set-admin-state p2 down > +ovs-appctl time/stop > +ovs-appctl time/warp 100 > +AT_CHECK([ovs-appctl netdev-dummy/receive p7 'in_port(7),eth(src=50:54:00:00:00:09,dst=50:54:00:00:00:0a),eth_type(0x0800),ipv4(src=10.0.0.2,dst=10.0.0.1,proto=1,tos=0,ttl=64,frag=no),icmp(type=8,code=0)']) > +AT_CHECK([ovs-appctl netdev-dummy/receive p7 'in_port(7),eth(src=50:54:00:00:00:0b,dst=50:54:00:00:00:0c),eth_type(0x0800),ipv4(src=10.0.0.3,dst=10.0.0.4,proto=1,tos=0,ttl=64,frag=no),icmp(type=8,code=0)']) > +ovs-appctl time/warp 100 > +ovs-appctl netdev-dummy/set-admin-state p2 up > +ovs-appctl netdev-dummy/set-admin-state p1 down > +ovs-appctl time/warp 100 > +AT_CHECK([ovs-appctl netdev-dummy/receive p7 'in_port(7),eth(src=50:54:00:00:00:09,dst=50:54:00:00:00:0d),eth_type(0x0800),ipv4(src=10.0.0.5,dst=10.0.0.1,proto=1,tos=0,ttl=64,frag=no),icmp(type=8,code=0)']) > +AT_CHECK([ovs-appctl netdev-dummy/receive p7 'in_port(7),eth(src=50:54:00:00:00:09,dst=50:54:00:00:00:0e),eth_type(0x0800),ipv4(src=10.0.0.6,dst=10.0.0.1,proto=1,tos=0,ttl=64,frag=no),icmp(type=8,code=0)']) > +ovs-appctl time/warp 2000 100 > +AT_CHECK([ovs-appctl dpctl/dump-flows | grep 'in_port([[348]])' | strip_xout], [0], [dnl > +recirc_id(0),in_port(3),packet_type(ns=0,id=0),eth(src=50:54:00:00:00:09,dst=50:54:00:00:00:0a),eth_type(0x0800),ipv4(frag=no), packets:0, bytes:0, used:never, actions: <del> > +recirc_id(0),in_port(3),packet_type(ns=0,id=0),eth(src=50:54:00:00:00:0b,dst=50:54:00:00:00:0c),eth_type(0x0800),ipv4(frag=no), packets:0, bytes:0, used:never, actions: <del> > +recirc_id(0),in_port(4),packet_type(ns=0,id=0),eth(src=50:54:00:00:00:09,dst=50:54:00:00:00:0d),eth_type(0x0800),ipv4(frag=no), packets:0, bytes:0, used:never, actions: <del> > +recirc_id(0),in_port(4),packet_type(ns=0,id=0),eth(src=50:54:00:00:00:09,dst=50:54:00:00:00:0e),eth_type(0x0800),ipv4(frag=no), packets:0, bytes:0, used:never, actions: <del> > +recirc_id(0),in_port(4),packet_type(ns=0,id=0),eth(src=50:54:00:00:00:09,dst=ff:ff:ff:ff:ff:ff),eth_type(0x8035), packets:0, bytes:0, used:never, actions: <del> > +recirc_id(0),in_port(4),packet_type(ns=0,id=0),eth(src=50:54:00:00:00:0b,dst=ff:ff:ff:ff:ff:ff),eth_type(0x8035), packets:0, bytes:0, used:never, actions: <del> > +]) > + > +ovs-appctl netdev-dummy/set-admin-state p1 up > +ovs-appctl time/warp 100 > +OVS_WAIT_UNTIL([ovs-appctl bond/show | STRIP_RECIRC_ID | STRIP_ACTIVE_SLAVE_MAC], [0], [dnl > +---- bond0 ---- > +bond_mode: active-backup > +bond may use recirculation: no, <del> > +bond-hash-basis: 0 > +updelay: 0 ms > +downdelay: 0 ms > +lacp_status: off > +lacp_fallback_ab: false > +active-backup primary: p1 > +<active slave mac del> > + > +slave p1: enabled > + active slave > + may_enable: true > + > +slave p2: enabled > + may_enable: true > + > +]) > + > +OVS_VSWITCHD_STOP > +AT_CLEANUP > + > +AT_SETUP([ofproto-dpif - active-backup bonding (primary validation)]) > +dnl Make a switch with 3 ports in a bond, so that when we delete one of > +dnl the ports from the bond, there are still 2 ports left and the bond > +dnl remains functional. > +OVS_VSWITCHD_START( > + [add-bond br0 bond0 p1 p2 p3 bond_mode=active-backup \ > + other_config:bond-primary=p1 -- \ > + set interface p1 type=dummy options:pstream=punix:$OVS_RUNDIR/p1.sock ofport_request=1 -- \ > + set interface p2 type=dummy options:pstream=punix:$OVS_RUNDIR/p2.sock ofport_request=2 -- \ > + set interface p3 type=dummy options:pstream=punix:$OVS_RUNDIR/p3.sock ofport_request=3 -- \ > + add-port br0 p7 -- set interface p7 ofport_request=7 type=dummy --]) > AT_CHECK([ovs-appctl vlog/set dpif:dbg dpif_netdev:dbg]) > > +dnl Make sure the initial primary interface is set > +OVS_WAIT_UNTIL([test -n "`ovs-appctl bond/show | grep 'active-backup primary: p1'`"]) > + > +dnl Down the primary interface and verify that we switched. Then > +dnl bring the primary back and verify that we switched back to the > +dnl primary. > +ovs-appctl netdev-dummy/set-admin-state p1 down > +ovs-appctl time/warp 100 > +OVS_WAIT_UNTIL([test -n "`ovs-appctl bond/show | fgrep 'slave p1: disabled'`"]) > +ovs-appctl netdev-dummy/set-admin-state p1 up > +ovs-appctl time/warp 100 > +OVS_WAIT_UNTIL([ovs-appctl bond/show | STRIP_RECIRC_ID | STRIP_ACTIVE_SLAVE_MAC], [0], [dnl > +---- bond0 ---- > +bond_mode: active-backup > +bond may use recirculation: no, <del> > +bond-hash-basis: 0 > +updelay: 0 ms > +downdelay: 0 ms > +lacp_status: off > +lacp_fallback_ab: false > +active-backup primary: p1 > +<active slave mac del> > + > +slave p1: enabled > + active slave > + may_enable: true > + > +slave p2: enabled > + may_enable: true > + > +slave p3: enabled > + may_enable: true > + > +]) > + > +dnl Now delete the primary and verify that the output shows that the > +dnl primary is no longer enslaved > +ovs-vsctl --id=@p1 get Interface p1 -- remove Port bond0 interfaces @p1 > +ovs-appctl time/warp 100 > +OVS_WAIT_UNTIL([test -n "`ovs-appctl bond/show | fgrep 'active-backup primary: p1 (no such slave)'`"]) > + > +dnl Now re-add the primary and verify that the output shows that the > +dnl primary is available again. > +dnl > +dnl First, get the UUIDs of the interfaces that exist on bond0. > +dnl Strip the trailing ] so that we can add a new UUID to the end. > +uuids=`ovs-vsctl get Port bond0 interfaces | sed -e 's/]//'` > +dnl Create a new port "p1" and add its UUID to the set of interfaces > +dnl on bond0. > +ovs-vsctl \ > + --id=@p1 create Interface name=p1 type=dummy options:pstream=punix:$OVS_RUNDIR/p1.sock ofport_request=1 -- \ > + set Port bond0 interfaces="$uuids, @p1]" > +ovs-appctl time/warp 100 > +OVS_WAIT_UNTIL([ovs-appctl bond/show | STRIP_RECIRC_ID | STRIP_ACTIVE_SLAVE_MAC], [0], [dnl > +---- bond0 ---- > +bond_mode: active-backup > +bond may use recirculation: no, <del> > +bond-hash-basis: 0 > +updelay: 0 ms > +downdelay: 0 ms > +lacp_status: off > +lacp_fallback_ab: false > +active-backup primary: p1 > +<active slave mac del> > + > +slave p1: enabled > + active slave > + may_enable: true > + > +slave p2: enabled > + may_enable: true > + > +slave p3: enabled > + may_enable: true > + > +]) > + > +dnl Remove the "bond-primary" config directive from the bond. > +AT_CHECK([ovs-vsctl remove Port bond0 other_config bond-primary]) > +ovs-appctl time/warp 100 > +OVS_WAIT_UNTIL([ovs-appctl bond/show | STRIP_RECIRC_ID | STRIP_ACTIVE_SLAVE_MAC], [0], [dnl > +---- bond0 ---- > +bond_mode: active-backup > +bond may use recirculation: no, <del> > +bond-hash-basis: 0 > +updelay: 0 ms > +downdelay: 0 ms > +lacp_status: off > +lacp_fallback_ab: false > +active-backup primary: <none> > +<active slave mac del> > + > +slave p1: enabled > + active slave > + may_enable: true > + > +slave p2: enabled > + may_enable: true > + > +slave p3: enabled > + may_enable: true > + > +]) > + > +OVS_VSWITCHD_STOP > +AT_CLEANUP > + > +AT_SETUP([ofproto-dpif - active-backup bonding (without primary)]) > +dnl Create br0 with interfaces p1, p2 and p7, creating bond0 with p1 and p2 > +dnl and br1 with interfaces p3, p4 and p8. > +dnl toggle p1,p2 of bond0 up and down to test bonding in active-backup mode. > +OVS_VSWITCHD_START( > + [add-bond br0 bond0 p1 p2 bond_mode=active-backup --\ > + set interface p1 type=dummy options:pstream=punix:$OVS_RUNDIR/p1.sock ofport_request=1 -- \ > + set interface p2 type=dummy options:pstream=punix:$OVS_RUNDIR/p2.sock ofport_request=2 -- \ > + add-port br0 p7 -- set interface p7 ofport_request=7 type=dummy -- \ > + add-br br1 -- \ > + set bridge br1 other-config:hwaddr=aa:66:aa:66:00:00 -- \ > + set bridge br1 datapath-type=dummy other-config:datapath-id=1234 \ > + fail-mode=secure -- \ > + add-port br1 p3 -- set interface p3 type=dummy options:stream=unix:$OVS_RUNDIR/p1.sock ofport_request=3 -- \ > + add-port br1 p4 -- set interface p4 type=dummy options:stream=unix:$OVS_RUNDIR/p2.sock ofport_request=4 -- \ > + add-port br1 p8 -- set interface p8 ofport_request=8 type=dummy --]) > +AT_CHECK([ovs-appctl vlog/set dpif:dbg dpif_netdev:dbg]) > +WAIT_FOR_DUMMY_PORTS([p3], [p4]) > +OVS_WAIT_UNTIL([test -n "`ovs-appctl bond/show | grep 'active-backup primary: <none>'`"]) > + > AT_CHECK([ovs-ofctl add-flow br0 action=normal]) > AT_CHECK([ovs-ofctl add-flow br1 action=normal]) > ovs-appctl netdev-dummy/set-admin-state up > @@ -63,15 +260,14 @@ ovs-appctl netdev-dummy/set-admin-state p1 down > ovs-appctl time/warp 100 > AT_CHECK([ovs-appctl netdev-dummy/receive p7 'in_port(7),eth(src=50:54:00:00:00:09,dst=50:54:00:00:00:0d),eth_type(0x0800),ipv4(src=10.0.0.5,dst=10.0.0.1,proto=1,tos=0,ttl=64,frag=no),icmp(type=8,code=0)']) > AT_CHECK([ovs-appctl netdev-dummy/receive p7 'in_port(7),eth(src=50:54:00:00:00:09,dst=50:54:00:00:00:0e),eth_type(0x0800),ipv4(src=10.0.0.6,dst=10.0.0.1,proto=1,tos=0,ttl=64,frag=no),icmp(type=8,code=0)']) > -ovs-appctl time/warp 200 100 > -sleep 1 > -AT_CHECK([grep 'in_port([[348]])' ovs-vswitchd.log | filter_flow_install | strip_xout], [0], [dnl > -recirc_id(0),in_port(3),packet_type(ns=0,id=0),eth(src=50:54:00:00:00:09,dst=50:54:00:00:00:0a),eth_type(0x0800),ipv4(frag=no), actions: <del> > -recirc_id(0),in_port(3),packet_type(ns=0,id=0),eth(src=50:54:00:00:00:0b,dst=50:54:00:00:00:0c),eth_type(0x0800),ipv4(frag=no), actions: <del> > -recirc_id(0),in_port(4),packet_type(ns=0,id=0),eth(src=50:54:00:00:00:09,dst=50:54:00:00:00:0d),eth_type(0x0800),ipv4(frag=no), actions: <del> > -recirc_id(0),in_port(4),packet_type(ns=0,id=0),eth(src=50:54:00:00:00:09,dst=50:54:00:00:00:0e),eth_type(0x0800),ipv4(frag=no), actions: <del> > -recirc_id(0),in_port(4),packet_type(ns=0,id=0),eth(src=50:54:00:00:00:09,dst=ff:ff:ff:ff:ff:ff),eth_type(0x8035), actions: <del> > -recirc_id(0),in_port(4),packet_type(ns=0,id=0),eth(src=50:54:00:00:00:0b,dst=ff:ff:ff:ff:ff:ff),eth_type(0x8035), actions: <del> > +ovs-appctl time/warp 2000 100 > +AT_CHECK([ovs-appctl dpctl/dump-flows | grep 'in_port([[348]])' | strip_xout], [0], [dnl > +recirc_id(0),in_port(3),packet_type(ns=0,id=0),eth(src=50:54:00:00:00:09,dst=50:54:00:00:00:0a),eth_type(0x0800),ipv4(frag=no), packets:0, bytes:0, used:never, actions: <del> > +recirc_id(0),in_port(3),packet_type(ns=0,id=0),eth(src=50:54:00:00:00:0b,dst=50:54:00:00:00:0c),eth_type(0x0800),ipv4(frag=no), packets:0, bytes:0, used:never, actions: <del> > +recirc_id(0),in_port(4),packet_type(ns=0,id=0),eth(src=50:54:00:00:00:09,dst=50:54:00:00:00:0d),eth_type(0x0800),ipv4(frag=no), packets:0, bytes:0, used:never, actions: <del> > +recirc_id(0),in_port(4),packet_type(ns=0,id=0),eth(src=50:54:00:00:00:09,dst=50:54:00:00:00:0e),eth_type(0x0800),ipv4(frag=no), packets:0, bytes:0, used:never, actions: <del> > +recirc_id(0),in_port(4),packet_type(ns=0,id=0),eth(src=50:54:00:00:00:09,dst=ff:ff:ff:ff:ff:ff),eth_type(0x8035), packets:0, bytes:0, used:never, actions: <del> > +recirc_id(0),in_port(4),packet_type(ns=0,id=0),eth(src=50:54:00:00:00:0b,dst=ff:ff:ff:ff:ff:ff),eth_type(0x8035), packets:0, bytes:0, used:never, actions: <del> > ]) > OVS_VSWITCHD_STOP > AT_CLEANUP > diff --git a/vswitchd/bridge.c b/vswitchd/bridge.c > index f312efd8e..7111fbac3 100644 > --- a/vswitchd/bridge.c > +++ b/vswitchd/bridge.c > @@ -4563,6 +4563,11 @@ port_configure_bond(struct port *port, struct bond_settings *s) > port->name); > } > > + s->primary = NULL; > + if (s->balance == BM_AB || s->lacp_fallback_ab_cfg) { > + s->primary = smap_get(&port->cfg->other_config, "bond-primary"); > + } > + > miimon_interval = smap_get_int(&port->cfg->other_config, > "bond-miimon-interval", 0); > if (miimon_interval <= 0) { > diff --git a/vswitchd/vswitch.xml b/vswitchd/vswitch.xml > index b6acb34ca..944708ed6 100644 > --- a/vswitchd/vswitch.xml > +++ b/vswitchd/vswitch.xml > @@ -2003,6 +2003,14 @@ > This knob does not affect other balancing modes. > </column> > > + <column name="other_config" key="bond-primary" > + type='{"type": "string"}'> > + If a slave interface with this name exists in the bond and > + is up, it will be made active. Relevant only when <ref > + column="other_config" key="bond-mode"/> is > + <code>active-backup</code>. > + </column> > + > <group title="Link Failure Detection"> > <p> > An important part of link bonding is detecting that links are down so > -- > 2.26.2 >
Hi Jeff, Thanks for following up. I have a couple comments inline. On Thu, Jun 25, 2020 at 02:03:34PM -0700, Jeff Squyres via dev wrote: > In AB bonding, if the current active slave becomes disabled, a > replacement slave is arbitrarily picked from the remaining set of > enabled slaves. This commit adds the concept of a "primary" slave: an > interface that will always be (or become) the current active slave if > it is enabled. > > The rationale for this functionality is to allow the designation of a > preferred interface for a given bond. For example: > > 1. Bond is created with interfaces p1 (primary) and p2, both enabled. > 2. p1 becomes the current active slave (because it was designated as > the primary). > 3. Later, p1 fails/becomes disabled. > 4. p2 is chosen to become the current active slave. > 5. Later, p1 becomes re-enabled. > 6. p1 is chosen to become the current active slave (because it was > designated as the primary) > > Note that p1 becomes the active slave once it becomes re-enabled, even > if nothing has happened to p2. > > This "primary" concept exists in Linux kernel network interface > bonding, but did not previously exist in OVS bonding. > > Only one primary slave inteface is supported per bond, and is only > supported for active/backup bonding. > > The primary slave interface is designated via > "other_config:bond-primary" when creating a bond. > > Also, while adding tests for the "primary" concept, make a few small > improvements to the non-primary AB bonding test. > > Signed-off-by: Jeff Squyres <jsquyres@cisco.com> > Reviewed-by: Aaron Conole <aconole@redhat.com> > Tested-by: Greg Rose <gvrose8192@gmail.com> > Acked-by: Greg Rose <gvrose8192@gmail.com> > --- > v9: > - Removed stale code that should have been removed in v8. > - Fixed comment punctuation. > > v8: > - Many style and simplification suggestions from Ilya Maximets > - Rename "primary:" to "active-backup primary:" in ovs-appctl > bond/show output, and make it always appear (vs. only appearing in > explicit AB bonding scenarios). > - Properly handle fallback-to-AB-bonding cases > - Use time/warp and OVS_WAIT_UNTIL in the bonding tests to prevent > races, and a few other improvements to the test quality > - After rebasing patch to head of tree, adjust test output as result > of changes from other commits. > > v7: > - After rebasing patch to head of tree, adjust test output as result > of changes from 81f71381f. > > v6: > - Style: bleep bloop. Fix use of {}. > > v5: > - Handle when bond is reconfigured to remove "bond-primary" config. > - Fix potential flapping between remaining slaves. > - Add a test to re-add a removed primary interface and make sure the > bond reflects that properly. > - Add a test to remove "bond-primary" config and make sure the bond > reflects that properly. > > v4: > - Style: bleep bloop. Trim line length below 79 characters. > > v3: > - Adjusted print logic for when the primary slave is not attached > > v2: > - Added "ovs-vsctl bond/show" label when primary interface is no > longer enslaved > - Fixed strcmp() usage nits > - Moved "other_config:bond-primary" docs to the "Bonding > Configuration" group > - Expanded commit message > - Added more test cases (including one for when the primary interface > is no longer enslaved) > > ofproto/bond.c | 54 ++++++++-- > ofproto/bond.h | 1 + > tests/lacp.at | 9 ++ > tests/ofproto-dpif.at | 224 +++++++++++++++++++++++++++++++++++++++--- > vswitchd/bridge.c | 5 + > vswitchd/vswitch.xml | 8 ++ > 6 files changed, 280 insertions(+), 21 deletions(-) > > diff --git a/ofproto/bond.c b/ofproto/bond.c > index 9947e7531..58ee67beb 100644 > --- a/ofproto/bond.c > +++ b/ofproto/bond.c > @@ -89,6 +89,7 @@ struct bond_slave { > /* Link status. */ > bool enabled; /* May be chosen for flows? */ > bool may_enable; /* Client considers this slave bondable. */ > + bool is_primary; /* This slave is preferred over others. */ > long long delay_expires; /* Time after which 'enabled' may change. */ > > /* Rebalancing info. Used only by bond_rebalance(). */ > @@ -124,6 +125,7 @@ struct bond { > uint32_t basis; /* Basis for flow hash function. */ > bool use_lb_output_action; /* Use lb_output action to avoid recirculation. > Applicable only for Balance TCP mode. */ > + char *primary; /* Name of the primary slave interface. */ > > /* SLB specific bonding info. */ > struct bond_entry *hash; /* An array of BOND_BUCKETS elements. */ > @@ -241,6 +243,7 @@ bond_create(const struct bond_settings *s, struct ofproto_dpif *ofproto) > > bond->active_slave_mac = eth_addr_zero; > bond->active_slave_changed = false; > + bond->primary = NULL; > > bond_reconfigure(bond, s); > return bond; > @@ -294,6 +297,7 @@ bond_unref(struct bond *bond) > update_recirc_rules__(bond); > > hmap_destroy(&bond->pr_rule_ops); > + free(bond->primary); > free(bond->name); > free(bond); > } > @@ -471,6 +475,12 @@ bond_reconfigure(struct bond *bond, const struct bond_settings *s) > bond->bond_revalidate = false; > } > > + if (!nullable_string_is_equal(bond->primary, s->primary)) { > + free(bond->primary); > + bond->primary = nullable_xstrdup(s->primary); > + revalidate = true; > + } > + > if (bond->balance != BM_AB) { > if (!bond->recirc_id) { > bond->recirc_id = recirc_alloc_id(bond->ofproto); > @@ -575,6 +585,12 @@ bond_slave_register(struct bond *bond, void *slave_, > slave->name = xstrdup(netdev_get_name(netdev)); > bond->bond_revalidate = true; > > + if (bond->primary && !strcmp(bond->primary, slave->name)) { > + slave->is_primary = true; > + } else { > + slave->is_primary = false; > + } > + > slave->enabled = false; > bond_enable_slave(slave, netdev_get_carrier(netdev)); > } > @@ -668,7 +684,7 @@ bond_slave_set_may_enable(struct bond *bond, void *slave_, bool may_enable) > bool > bond_run(struct bond *bond, enum lacp_status lacp_status) > { > - struct bond_slave *slave; > + struct bond_slave *slave, *primary; > bool revalidate; > > ovs_rwlock_wrlock(&rwlock); > @@ -685,11 +701,19 @@ bond_run(struct bond *bond, enum lacp_status lacp_status) > } > > /* Enable slaves based on link status and LACP feedback. */ > + primary = NULL; > HMAP_FOR_EACH (slave, hmap_node, &bond->slaves) { > bond_link_status_update(slave); > slave->change_seq = seq_read(connectivity_seq_get()); > + > + /* Discover if there is an active slave marked 'primary'. */ > + if (bond->balance == BM_AB && slave->is_primary && slave->enabled) { > + primary = slave; > + } > } > - if (!bond->active_slave || !bond->active_slave->enabled) { > + > + if (!bond->active_slave || !bond->active_slave->enabled || > + (primary && bond->active_slave != primary)) { > bond_choose_active_slave(bond); > } > > @@ -1437,16 +1461,25 @@ bond_print_details(struct ds *ds, const struct bond *bond) > ds_put_format(ds, "lacp_fallback_ab: %s\n", > bond->lacp_fallback_ab ? "true" : "false"); > > - ds_put_cstr(ds, "active slave mac: "); > - ds_put_format(ds, ETH_ADDR_FMT, ETH_ADDR_ARGS(bond->active_slave_mac)); > + bool found_primary = false; > slave = bond_find_slave_by_mac(bond, bond->active_slave_mac); ^^^^^^ > - ds_put_format(ds,"(%s)\n", slave ? slave->name : "none"); > - > HMAP_FOR_EACH (slave, hmap_node, &bond->slaves) { ^^^^^^ So, the active slave is lost here. > + if (slave->is_primary) { > + found_primary = true; > + } > shash_add(&slave_shash, slave->name, slave); > } > - sorted_slaves = shash_sort(&slave_shash); > > + ds_put_format(ds, "active-backup primary: %s%s\n", > + bond->primary ? bond->primary : "<none>", > + (!found_primary && bond->primary) > + ? " (no such slave)" : ""); Perhaps it should here: slave = bond_find_slave_by_mac(bond, bond->active_slave_mac); > + > + ds_put_cstr(ds, "active slave mac: "); > + ds_put_format(ds, ETH_ADDR_FMT, ETH_ADDR_ARGS(bond->active_slave_mac)); > + ds_put_format(ds,"(%s)\n", slave ? slave->name : "none"); > + > + sorted_slaves = shash_sort(&slave_shash); > for (i = 0; i < shash_count(&slave_shash); i++) { > struct bond_entry *be; > > @@ -1906,6 +1939,13 @@ bond_choose_slave(const struct bond *bond) > { > struct bond_slave *slave, *best; > > + /* If there's a primary and it's active, return that. */ > + HMAP_FOR_EACH (slave, hmap_node, &bond->slaves) { > + if (slave->is_primary && slave->enabled) { > + return slave; > + } > + } > + > /* Find the last active slave. */ > slave = bond_find_slave_by_mac(bond, bond->active_slave_mac); > if (slave && slave->enabled) { > diff --git a/ofproto/bond.h b/ofproto/bond.h > index 40c3258dc..1a42a37c9 100644 > --- a/ofproto/bond.h > +++ b/ofproto/bond.h > @@ -45,6 +45,7 @@ struct bond_settings { > > /* Balancing configuration. */ > enum bond_mode balance; > + const char *primary; /* For AB balance, primary interface name. */ > int rebalance_interval; /* Milliseconds between rebalances. > Zero to disable rebalancing. */ > > diff --git a/tests/lacp.at b/tests/lacp.at > index df1691731..5257f0cce 100644 > --- a/tests/lacp.at > +++ b/tests/lacp.at > @@ -126,6 +126,7 @@ updelay: 0 ms > downdelay: 0 ms > lacp_status: negotiated > lacp_fallback_ab: false > +active-backup primary: <none> > active slave mac: 00:00:00:00:00:00(none) > > slave p1: disabled > @@ -292,6 +293,7 @@ updelay: 0 ms > downdelay: 0 ms > lacp_status: negotiated > lacp_fallback_ab: false > +active-backup primary: <none> > > slave p0: enabled > may_enable: true > @@ -308,6 +310,7 @@ updelay: 0 ms > downdelay: 0 ms > lacp_status: negotiated > lacp_fallback_ab: false > +active-backup primary: <none> > > slave p2: enabled > may_enable: true > @@ -431,6 +434,7 @@ updelay: 0 ms > downdelay: 0 ms > lacp_status: negotiated > lacp_fallback_ab: false > +active-backup primary: <none> > <active slave mac del> > > slave p0: disabled > @@ -449,6 +453,7 @@ updelay: 0 ms > downdelay: 0 ms > lacp_status: negotiated > lacp_fallback_ab: false > +active-backup primary: <none> > <active slave mac del> > > slave p2: disabled > @@ -565,6 +570,7 @@ updelay: 0 ms > downdelay: 0 ms > lacp_status: negotiated > lacp_fallback_ab: false > +active-backup primary: <none> > <active slave mac del> > > slave p0: disabled > @@ -583,6 +589,7 @@ updelay: 0 ms > downdelay: 0 ms > lacp_status: negotiated > lacp_fallback_ab: false > +active-backup primary: <none> > <active slave mac del> > > slave p2: disabled > @@ -704,6 +711,7 @@ updelay: 0 ms > downdelay: 0 ms > lacp_status: negotiated > lacp_fallback_ab: false > +active-backup primary: <none> > <active slave mac del> > > slave p0: enabled > @@ -722,6 +730,7 @@ updelay: 0 ms > downdelay: 0 ms > lacp_status: negotiated > lacp_fallback_ab: false > +active-backup primary: <none> > <active slave mac del> > > slave p2: enabled > diff --git a/tests/ofproto-dpif.at b/tests/ofproto-dpif.at > index e3402e7b8..808200800 100644 > --- a/tests/ofproto-dpif.at > +++ b/tests/ofproto-dpif.at > @@ -29,12 +29,16 @@ AT_CHECK([ovs-appctl revalidator/wait]) > OVS_VSWITCHD_STOP > AT_CLEANUP > > -AT_SETUP([ofproto-dpif - active-backup bonding]) > -# Create br0 with interfaces p1, p2 and p7, creating bond0 with p1 and p2 > -# and br1 with interfaces p3, p4 and p8. > -# toggle p1,p2 of bond0 up and down to test bonding in active-backup mode. > +AT_SETUP([ofproto-dpif - active-backup bonding (with primary)]) > + > +dnl Create br0 with interfaces p1, p2 and p7, creating bond0 with p1 and > +dnl p2 (p1 as primary) and br1 with interfaces p3, p4 and p8. > +dnl toggle p1,p2 of bond0 up and down to test bonding in active-backup mode. > +dnl With p1 down and p2 up/active, bring p1 back up. Since p1 is the primary, > +dnl it should become active. > OVS_VSWITCHD_START( > - [add-bond br0 bond0 p1 p2 bond_mode=active-backup --\ > + [add-bond br0 bond0 p1 p2 bond_mode=active-backup \ > + other_config:bond-primary=p1 -- \ > set interface p1 type=dummy options:pstream=punix:$OVS_RUNDIR/p1.sock ofport_request=1 -- \ > set interface p2 type=dummy options:pstream=punix:$OVS_RUNDIR/p2.sock ofport_request=2 -- \ > add-port br0 p7 -- set interface p7 ofport_request=7 type=dummy -- \ > @@ -45,9 +49,202 @@ OVS_VSWITCHD_START( > add-port br1 p3 -- set interface p3 type=dummy options:stream=unix:$OVS_RUNDIR/p1.sock ofport_request=3 -- \ > add-port br1 p4 -- set interface p4 type=dummy options:stream=unix:$OVS_RUNDIR/p2.sock ofport_request=4 -- \ > add-port br1 p8 -- set interface p8 ofport_request=8 type=dummy --]) > +AT_CHECK([ovs-appctl vlog/set dpif:dbg dpif_netdev:dbg]) > WAIT_FOR_DUMMY_PORTS([p3], [p4]) > +OVS_WAIT_UNTIL([test -n "`ovs-appctl bond/show | grep 'active-backup primary: p1'`"]) > + > + > +AT_CHECK([ovs-ofctl add-flow br0 action=normal]) > +AT_CHECK([ovs-ofctl add-flow br1 action=normal]) > +ovs-appctl netdev-dummy/set-admin-state up Missing the interface name? > +ovs-appctl time/warp 100 > +ovs-appctl netdev-dummy/set-admin-state p2 down > +ovs-appctl time/stop > +ovs-appctl time/warp 100 > +AT_CHECK([ovs-appctl netdev-dummy/receive p7 'in_port(7),eth(src=50:54:00:00:00:09,dst=50:54:00:00:00:0a),eth_type(0x0800),ipv4(src=10.0.0.2,dst=10.0.0.1,proto=1,tos=0,ttl=64,frag=no),icmp(type=8,code=0)']) > +AT_CHECK([ovs-appctl netdev-dummy/receive p7 'in_port(7),eth(src=50:54:00:00:00:0b,dst=50:54:00:00:00:0c),eth_type(0x0800),ipv4(src=10.0.0.3,dst=10.0.0.4,proto=1,tos=0,ttl=64,frag=no),icmp(type=8,code=0)']) > +ovs-appctl time/warp 100 > +ovs-appctl netdev-dummy/set-admin-state p2 up > +ovs-appctl netdev-dummy/set-admin-state p1 down > +ovs-appctl time/warp 100 > +AT_CHECK([ovs-appctl netdev-dummy/receive p7 'in_port(7),eth(src=50:54:00:00:00:09,dst=50:54:00:00:00:0d),eth_type(0x0800),ipv4(src=10.0.0.5,dst=10.0.0.1,proto=1,tos=0,ttl=64,frag=no),icmp(type=8,code=0)']) > +AT_CHECK([ovs-appctl netdev-dummy/receive p7 'in_port(7),eth(src=50:54:00:00:00:09,dst=50:54:00:00:00:0e),eth_type(0x0800),ipv4(src=10.0.0.6,dst=10.0.0.1,proto=1,tos=0,ttl=64,frag=no),icmp(type=8,code=0)']) > +ovs-appctl time/warp 2000 100 > +AT_CHECK([ovs-appctl dpctl/dump-flows | grep 'in_port([[348]])' | strip_xout], [0], [dnl > +recirc_id(0),in_port(3),packet_type(ns=0,id=0),eth(src=50:54:00:00:00:09,dst=50:54:00:00:00:0a),eth_type(0x0800),ipv4(frag=no), packets:0, bytes:0, used:never, actions: <del> > +recirc_id(0),in_port(3),packet_type(ns=0,id=0),eth(src=50:54:00:00:00:0b,dst=50:54:00:00:00:0c),eth_type(0x0800),ipv4(frag=no), packets:0, bytes:0, used:never, actions: <del> > +recirc_id(0),in_port(4),packet_type(ns=0,id=0),eth(src=50:54:00:00:00:09,dst=50:54:00:00:00:0d),eth_type(0x0800),ipv4(frag=no), packets:0, bytes:0, used:never, actions: <del> > +recirc_id(0),in_port(4),packet_type(ns=0,id=0),eth(src=50:54:00:00:00:09,dst=50:54:00:00:00:0e),eth_type(0x0800),ipv4(frag=no), packets:0, bytes:0, used:never, actions: <del> > +recirc_id(0),in_port(4),packet_type(ns=0,id=0),eth(src=50:54:00:00:00:09,dst=ff:ff:ff:ff:ff:ff),eth_type(0x8035), packets:0, bytes:0, used:never, actions: <del> > +recirc_id(0),in_port(4),packet_type(ns=0,id=0),eth(src=50:54:00:00:00:0b,dst=ff:ff:ff:ff:ff:ff),eth_type(0x8035), packets:0, bytes:0, used:never, actions: <del> > +]) > + > +ovs-appctl netdev-dummy/set-admin-state p1 up > +ovs-appctl time/warp 100 > +OVS_WAIT_UNTIL([ovs-appctl bond/show | STRIP_RECIRC_ID | STRIP_ACTIVE_SLAVE_MAC], [0], [dnl > +---- bond0 ---- > +bond_mode: active-backup > +bond may use recirculation: no, <del> > +bond-hash-basis: 0 > +updelay: 0 ms > +downdelay: 0 ms > +lacp_status: off > +lacp_fallback_ab: false > +active-backup primary: p1 > +<active slave mac del> > + > +slave p1: enabled > + active slave > + may_enable: true > + > +slave p2: enabled > + may_enable: true > + > +]) > + > +OVS_VSWITCHD_STOP > +AT_CLEANUP > + > +AT_SETUP([ofproto-dpif - active-backup bonding (primary validation)]) > +dnl Make a switch with 3 ports in a bond, so that when we delete one of > +dnl the ports from the bond, there are still 2 ports left and the bond > +dnl remains functional. > +OVS_VSWITCHD_START( > + [add-bond br0 bond0 p1 p2 p3 bond_mode=active-backup \ > + other_config:bond-primary=p1 -- \ > + set interface p1 type=dummy options:pstream=punix:$OVS_RUNDIR/p1.sock ofport_request=1 -- \ > + set interface p2 type=dummy options:pstream=punix:$OVS_RUNDIR/p2.sock ofport_request=2 -- \ > + set interface p3 type=dummy options:pstream=punix:$OVS_RUNDIR/p3.sock ofport_request=3 -- \ > + add-port br0 p7 -- set interface p7 ofport_request=7 type=dummy --]) > AT_CHECK([ovs-appctl vlog/set dpif:dbg dpif_netdev:dbg]) > > +dnl Make sure the initial primary interface is set > +OVS_WAIT_UNTIL([test -n "`ovs-appctl bond/show | grep 'active-backup primary: p1'`"]) > + > +dnl Down the primary interface and verify that we switched. Then > +dnl bring the primary back and verify that we switched back to the > +dnl primary. > +ovs-appctl netdev-dummy/set-admin-state p1 down > +ovs-appctl time/warp 100 > +OVS_WAIT_UNTIL([test -n "`ovs-appctl bond/show | fgrep 'slave p1: disabled'`"]) > +ovs-appctl netdev-dummy/set-admin-state p1 up > +ovs-appctl time/warp 100 > +OVS_WAIT_UNTIL([ovs-appctl bond/show | STRIP_RECIRC_ID | STRIP_ACTIVE_SLAVE_MAC], [0], [dnl > +---- bond0 ---- > +bond_mode: active-backup > +bond may use recirculation: no, <del> > +bond-hash-basis: 0 > +updelay: 0 ms > +downdelay: 0 ms > +lacp_status: off > +lacp_fallback_ab: false > +active-backup primary: p1 > +<active slave mac del> > + > +slave p1: enabled > + active slave > + may_enable: true > + > +slave p2: enabled > + may_enable: true > + > +slave p3: enabled > + may_enable: true > + > +]) > + > +dnl Now delete the primary and verify that the output shows that the > +dnl primary is no longer enslaved > +ovs-vsctl --id=@p1 get Interface p1 -- remove Port bond0 interfaces @p1 > +ovs-appctl time/warp 100 > +OVS_WAIT_UNTIL([test -n "`ovs-appctl bond/show | fgrep 'active-backup primary: p1 (no such slave)'`"]) > + > +dnl Now re-add the primary and verify that the output shows that the > +dnl primary is available again. > +dnl > +dnl First, get the UUIDs of the interfaces that exist on bond0. > +dnl Strip the trailing ] so that we can add a new UUID to the end. > +uuids=`ovs-vsctl get Port bond0 interfaces | sed -e 's/]//'` > +dnl Create a new port "p1" and add its UUID to the set of interfaces > +dnl on bond0. > +ovs-vsctl \ > + --id=@p1 create Interface name=p1 type=dummy options:pstream=punix:$OVS_RUNDIR/p1.sock ofport_request=1 -- \ > + set Port bond0 interfaces="$uuids, @p1]" > +ovs-appctl time/warp 100 > +OVS_WAIT_UNTIL([ovs-appctl bond/show | STRIP_RECIRC_ID | STRIP_ACTIVE_SLAVE_MAC], [0], [dnl > +---- bond0 ---- > +bond_mode: active-backup > +bond may use recirculation: no, <del> > +bond-hash-basis: 0 > +updelay: 0 ms > +downdelay: 0 ms > +lacp_status: off > +lacp_fallback_ab: false > +active-backup primary: p1 > +<active slave mac del> > + > +slave p1: enabled > + active slave > + may_enable: true > + > +slave p2: enabled > + may_enable: true > + > +slave p3: enabled > + may_enable: true > + > +]) > + > +dnl Remove the "bond-primary" config directive from the bond. > +AT_CHECK([ovs-vsctl remove Port bond0 other_config bond-primary]) > +ovs-appctl time/warp 100 > +OVS_WAIT_UNTIL([ovs-appctl bond/show | STRIP_RECIRC_ID | STRIP_ACTIVE_SLAVE_MAC], [0], [dnl > +---- bond0 ---- > +bond_mode: active-backup > +bond may use recirculation: no, <del> > +bond-hash-basis: 0 > +updelay: 0 ms > +downdelay: 0 ms > +lacp_status: off > +lacp_fallback_ab: false > +active-backup primary: <none> > +<active slave mac del> > + > +slave p1: enabled > + active slave > + may_enable: true > + > +slave p2: enabled > + may_enable: true > + > +slave p3: enabled > + may_enable: true > + > +]) > + > +OVS_VSWITCHD_STOP > +AT_CLEANUP > + > +AT_SETUP([ofproto-dpif - active-backup bonding (without primary)]) > +dnl Create br0 with interfaces p1, p2 and p7, creating bond0 with p1 and p2 > +dnl and br1 with interfaces p3, p4 and p8. > +dnl toggle p1,p2 of bond0 up and down to test bonding in active-backup mode. > +OVS_VSWITCHD_START( > + [add-bond br0 bond0 p1 p2 bond_mode=active-backup --\ > + set interface p1 type=dummy options:pstream=punix:$OVS_RUNDIR/p1.sock ofport_request=1 -- \ > + set interface p2 type=dummy options:pstream=punix:$OVS_RUNDIR/p2.sock ofport_request=2 -- \ > + add-port br0 p7 -- set interface p7 ofport_request=7 type=dummy -- \ > + add-br br1 -- \ > + set bridge br1 other-config:hwaddr=aa:66:aa:66:00:00 -- \ > + set bridge br1 datapath-type=dummy other-config:datapath-id=1234 \ > + fail-mode=secure -- \ > + add-port br1 p3 -- set interface p3 type=dummy options:stream=unix:$OVS_RUNDIR/p1.sock ofport_request=3 -- \ > + add-port br1 p4 -- set interface p4 type=dummy options:stream=unix:$OVS_RUNDIR/p2.sock ofport_request=4 -- \ > + add-port br1 p8 -- set interface p8 ofport_request=8 type=dummy --]) > +AT_CHECK([ovs-appctl vlog/set dpif:dbg dpif_netdev:dbg]) > +WAIT_FOR_DUMMY_PORTS([p3], [p4]) > +OVS_WAIT_UNTIL([test -n "`ovs-appctl bond/show | grep 'active-backup primary: <none>'`"]) > + > AT_CHECK([ovs-ofctl add-flow br0 action=normal]) > AT_CHECK([ovs-ofctl add-flow br1 action=normal]) > ovs-appctl netdev-dummy/set-admin-state up > @@ -63,15 +260,14 @@ ovs-appctl netdev-dummy/set-admin-state p1 down > ovs-appctl time/warp 100 > AT_CHECK([ovs-appctl netdev-dummy/receive p7 'in_port(7),eth(src=50:54:00:00:00:09,dst=50:54:00:00:00:0d),eth_type(0x0800),ipv4(src=10.0.0.5,dst=10.0.0.1,proto=1,tos=0,ttl=64,frag=no),icmp(type=8,code=0)']) > AT_CHECK([ovs-appctl netdev-dummy/receive p7 'in_port(7),eth(src=50:54:00:00:00:09,dst=50:54:00:00:00:0e),eth_type(0x0800),ipv4(src=10.0.0.6,dst=10.0.0.1,proto=1,tos=0,ttl=64,frag=no),icmp(type=8,code=0)']) > -ovs-appctl time/warp 200 100 > -sleep 1 > -AT_CHECK([grep 'in_port([[348]])' ovs-vswitchd.log | filter_flow_install | strip_xout], [0], [dnl > -recirc_id(0),in_port(3),packet_type(ns=0,id=0),eth(src=50:54:00:00:00:09,dst=50:54:00:00:00:0a),eth_type(0x0800),ipv4(frag=no), actions: <del> > -recirc_id(0),in_port(3),packet_type(ns=0,id=0),eth(src=50:54:00:00:00:0b,dst=50:54:00:00:00:0c),eth_type(0x0800),ipv4(frag=no), actions: <del> > -recirc_id(0),in_port(4),packet_type(ns=0,id=0),eth(src=50:54:00:00:00:09,dst=50:54:00:00:00:0d),eth_type(0x0800),ipv4(frag=no), actions: <del> > -recirc_id(0),in_port(4),packet_type(ns=0,id=0),eth(src=50:54:00:00:00:09,dst=50:54:00:00:00:0e),eth_type(0x0800),ipv4(frag=no), actions: <del> > -recirc_id(0),in_port(4),packet_type(ns=0,id=0),eth(src=50:54:00:00:00:09,dst=ff:ff:ff:ff:ff:ff),eth_type(0x8035), actions: <del> > -recirc_id(0),in_port(4),packet_type(ns=0,id=0),eth(src=50:54:00:00:00:0b,dst=ff:ff:ff:ff:ff:ff),eth_type(0x8035), actions: <del> > +ovs-appctl time/warp 2000 100 > +AT_CHECK([ovs-appctl dpctl/dump-flows | grep 'in_port([[348]])' | strip_xout], [0], [dnl > +recirc_id(0),in_port(3),packet_type(ns=0,id=0),eth(src=50:54:00:00:00:09,dst=50:54:00:00:00:0a),eth_type(0x0800),ipv4(frag=no), packets:0, bytes:0, used:never, actions: <del> > +recirc_id(0),in_port(3),packet_type(ns=0,id=0),eth(src=50:54:00:00:00:0b,dst=50:54:00:00:00:0c),eth_type(0x0800),ipv4(frag=no), packets:0, bytes:0, used:never, actions: <del> > +recirc_id(0),in_port(4),packet_type(ns=0,id=0),eth(src=50:54:00:00:00:09,dst=50:54:00:00:00:0d),eth_type(0x0800),ipv4(frag=no), packets:0, bytes:0, used:never, actions: <del> > +recirc_id(0),in_port(4),packet_type(ns=0,id=0),eth(src=50:54:00:00:00:09,dst=50:54:00:00:00:0e),eth_type(0x0800),ipv4(frag=no), packets:0, bytes:0, used:never, actions: <del> > +recirc_id(0),in_port(4),packet_type(ns=0,id=0),eth(src=50:54:00:00:00:09,dst=ff:ff:ff:ff:ff:ff),eth_type(0x8035), packets:0, bytes:0, used:never, actions: <del> > +recirc_id(0),in_port(4),packet_type(ns=0,id=0),eth(src=50:54:00:00:00:0b,dst=ff:ff:ff:ff:ff:ff),eth_type(0x8035), packets:0, bytes:0, used:never, actions: <del> > ]) > OVS_VSWITCHD_STOP > AT_CLEANUP > diff --git a/vswitchd/bridge.c b/vswitchd/bridge.c > index f312efd8e..7111fbac3 100644 > --- a/vswitchd/bridge.c > +++ b/vswitchd/bridge.c > @@ -4563,6 +4563,11 @@ port_configure_bond(struct port *port, struct bond_settings *s) > port->name); > } > > + s->primary = NULL; > + if (s->balance == BM_AB || s->lacp_fallback_ab_cfg) { > + s->primary = smap_get(&port->cfg->other_config, "bond-primary"); > + } > + > miimon_interval = smap_get_int(&port->cfg->other_config, > "bond-miimon-interval", 0); > if (miimon_interval <= 0) { > diff --git a/vswitchd/vswitch.xml b/vswitchd/vswitch.xml > index b6acb34ca..944708ed6 100644 > --- a/vswitchd/vswitch.xml > +++ b/vswitchd/vswitch.xml > @@ -2003,6 +2003,14 @@ > This knob does not affect other balancing modes. > </column> > > + <column name="other_config" key="bond-primary" > + type='{"type": "string"}'> > + If a slave interface with this name exists in the bond and > + is up, it will be made active. Relevant only when <ref > + column="other_config" key="bond-mode"/> is I think it is bond_mode. > + <code>active-backup</code>. > + </column> > + > <group title="Link Failure Detection"> > <p> > An important part of link bonding is detecting that links are down so > -- > 2.26.2 > > _______________________________________________ > dev mailing list > dev@openvswitch.org > https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Another thing, consider a small note in NEWS. On Thu, Jul 02, 2020 at 05:17:59PM -0300, Flavio Leitner wrote: > > Hi Jeff, > > Thanks for following up. > I have a couple comments inline. > > > On Thu, Jun 25, 2020 at 02:03:34PM -0700, Jeff Squyres via dev wrote: > > In AB bonding, if the current active slave becomes disabled, a > > replacement slave is arbitrarily picked from the remaining set of > > enabled slaves. This commit adds the concept of a "primary" slave: an > > interface that will always be (or become) the current active slave if > > it is enabled. > > > > The rationale for this functionality is to allow the designation of a > > preferred interface for a given bond. For example: > > > > 1. Bond is created with interfaces p1 (primary) and p2, both enabled. > > 2. p1 becomes the current active slave (because it was designated as > > the primary). > > 3. Later, p1 fails/becomes disabled. > > 4. p2 is chosen to become the current active slave. > > 5. Later, p1 becomes re-enabled. > > 6. p1 is chosen to become the current active slave (because it was > > designated as the primary) > > > > Note that p1 becomes the active slave once it becomes re-enabled, even > > if nothing has happened to p2. > > > > This "primary" concept exists in Linux kernel network interface > > bonding, but did not previously exist in OVS bonding. > > > > Only one primary slave inteface is supported per bond, and is only > > supported for active/backup bonding. > > > > The primary slave interface is designated via > > "other_config:bond-primary" when creating a bond. > > > > Also, while adding tests for the "primary" concept, make a few small > > improvements to the non-primary AB bonding test. > > > > Signed-off-by: Jeff Squyres <jsquyres@cisco.com> > > Reviewed-by: Aaron Conole <aconole@redhat.com> > > Tested-by: Greg Rose <gvrose8192@gmail.com> > > Acked-by: Greg Rose <gvrose8192@gmail.com> > > --- > > v9: > > - Removed stale code that should have been removed in v8. > > - Fixed comment punctuation. > > > > v8: > > - Many style and simplification suggestions from Ilya Maximets > > - Rename "primary:" to "active-backup primary:" in ovs-appctl > > bond/show output, and make it always appear (vs. only appearing in > > explicit AB bonding scenarios). > > - Properly handle fallback-to-AB-bonding cases > > - Use time/warp and OVS_WAIT_UNTIL in the bonding tests to prevent > > races, and a few other improvements to the test quality > > - After rebasing patch to head of tree, adjust test output as result > > of changes from other commits. > > > > v7: > > - After rebasing patch to head of tree, adjust test output as result > > of changes from 81f71381f. > > > > v6: > > - Style: bleep bloop. Fix use of {}. > > > > v5: > > - Handle when bond is reconfigured to remove "bond-primary" config. > > - Fix potential flapping between remaining slaves. > > - Add a test to re-add a removed primary interface and make sure the > > bond reflects that properly. > > - Add a test to remove "bond-primary" config and make sure the bond > > reflects that properly. > > > > v4: > > - Style: bleep bloop. Trim line length below 79 characters. > > > > v3: > > - Adjusted print logic for when the primary slave is not attached > > > > v2: > > - Added "ovs-vsctl bond/show" label when primary interface is no > > longer enslaved > > - Fixed strcmp() usage nits > > - Moved "other_config:bond-primary" docs to the "Bonding > > Configuration" group > > - Expanded commit message > > - Added more test cases (including one for when the primary interface > > is no longer enslaved) > > > > ofproto/bond.c | 54 ++++++++-- > > ofproto/bond.h | 1 + > > tests/lacp.at | 9 ++ > > tests/ofproto-dpif.at | 224 +++++++++++++++++++++++++++++++++++++++--- > > vswitchd/bridge.c | 5 + > > vswitchd/vswitch.xml | 8 ++ > > 6 files changed, 280 insertions(+), 21 deletions(-) > > > > diff --git a/ofproto/bond.c b/ofproto/bond.c > > index 9947e7531..58ee67beb 100644 > > --- a/ofproto/bond.c > > +++ b/ofproto/bond.c > > @@ -89,6 +89,7 @@ struct bond_slave { > > /* Link status. */ > > bool enabled; /* May be chosen for flows? */ > > bool may_enable; /* Client considers this slave bondable. */ > > + bool is_primary; /* This slave is preferred over others. */ > > long long delay_expires; /* Time after which 'enabled' may change. */ > > > > /* Rebalancing info. Used only by bond_rebalance(). */ > > @@ -124,6 +125,7 @@ struct bond { > > uint32_t basis; /* Basis for flow hash function. */ > > bool use_lb_output_action; /* Use lb_output action to avoid recirculation. > > Applicable only for Balance TCP mode. */ > > + char *primary; /* Name of the primary slave interface. */ > > > > /* SLB specific bonding info. */ > > struct bond_entry *hash; /* An array of BOND_BUCKETS elements. */ > > @@ -241,6 +243,7 @@ bond_create(const struct bond_settings *s, struct ofproto_dpif *ofproto) > > > > bond->active_slave_mac = eth_addr_zero; > > bond->active_slave_changed = false; > > + bond->primary = NULL; > > > > bond_reconfigure(bond, s); > > return bond; > > @@ -294,6 +297,7 @@ bond_unref(struct bond *bond) > > update_recirc_rules__(bond); > > > > hmap_destroy(&bond->pr_rule_ops); > > + free(bond->primary); > > free(bond->name); > > free(bond); > > } > > @@ -471,6 +475,12 @@ bond_reconfigure(struct bond *bond, const struct bond_settings *s) > > bond->bond_revalidate = false; > > } > > > > + if (!nullable_string_is_equal(bond->primary, s->primary)) { > > + free(bond->primary); > > + bond->primary = nullable_xstrdup(s->primary); > > + revalidate = true; > > + } > > + > > if (bond->balance != BM_AB) { > > if (!bond->recirc_id) { > > bond->recirc_id = recirc_alloc_id(bond->ofproto); > > @@ -575,6 +585,12 @@ bond_slave_register(struct bond *bond, void *slave_, > > slave->name = xstrdup(netdev_get_name(netdev)); > > bond->bond_revalidate = true; > > > > + if (bond->primary && !strcmp(bond->primary, slave->name)) { > > + slave->is_primary = true; > > + } else { > > + slave->is_primary = false; > > + } > > + > > slave->enabled = false; > > bond_enable_slave(slave, netdev_get_carrier(netdev)); > > } > > @@ -668,7 +684,7 @@ bond_slave_set_may_enable(struct bond *bond, void *slave_, bool may_enable) > > bool > > bond_run(struct bond *bond, enum lacp_status lacp_status) > > { > > - struct bond_slave *slave; > > + struct bond_slave *slave, *primary; > > bool revalidate; > > > > ovs_rwlock_wrlock(&rwlock); > > @@ -685,11 +701,19 @@ bond_run(struct bond *bond, enum lacp_status lacp_status) > > } > > > > /* Enable slaves based on link status and LACP feedback. */ > > + primary = NULL; > > HMAP_FOR_EACH (slave, hmap_node, &bond->slaves) { > > bond_link_status_update(slave); > > slave->change_seq = seq_read(connectivity_seq_get()); > > + > > + /* Discover if there is an active slave marked 'primary'. */ > > + if (bond->balance == BM_AB && slave->is_primary && slave->enabled) { > > + primary = slave; > > + } > > } > > - if (!bond->active_slave || !bond->active_slave->enabled) { > > + > > + if (!bond->active_slave || !bond->active_slave->enabled || > > + (primary && bond->active_slave != primary)) { > > bond_choose_active_slave(bond); > > } > > > > @@ -1437,16 +1461,25 @@ bond_print_details(struct ds *ds, const struct bond *bond) > > ds_put_format(ds, "lacp_fallback_ab: %s\n", > > bond->lacp_fallback_ab ? "true" : "false"); > > > > - ds_put_cstr(ds, "active slave mac: "); > > - ds_put_format(ds, ETH_ADDR_FMT, ETH_ADDR_ARGS(bond->active_slave_mac)); > > + bool found_primary = false; > > slave = bond_find_slave_by_mac(bond, bond->active_slave_mac); > ^^^^^^ > > > > - ds_put_format(ds,"(%s)\n", slave ? slave->name : "none"); > > - > > HMAP_FOR_EACH (slave, hmap_node, &bond->slaves) { > ^^^^^^ > So, the active slave is lost here. > > > > + if (slave->is_primary) { > > + found_primary = true; > > + } > > shash_add(&slave_shash, slave->name, slave); > > } > > - sorted_slaves = shash_sort(&slave_shash); > > > > + ds_put_format(ds, "active-backup primary: %s%s\n", > > + bond->primary ? bond->primary : "<none>", > > + (!found_primary && bond->primary) > > + ? " (no such slave)" : ""); > > Perhaps it should here: > slave = bond_find_slave_by_mac(bond, bond->active_slave_mac); > > > + > > + ds_put_cstr(ds, "active slave mac: "); > > + ds_put_format(ds, ETH_ADDR_FMT, ETH_ADDR_ARGS(bond->active_slave_mac)); > > + ds_put_format(ds,"(%s)\n", slave ? slave->name : "none"); > > + > > + sorted_slaves = shash_sort(&slave_shash); > > for (i = 0; i < shash_count(&slave_shash); i++) { > > struct bond_entry *be; > > > > @@ -1906,6 +1939,13 @@ bond_choose_slave(const struct bond *bond) > > { > > struct bond_slave *slave, *best; > > > > + /* If there's a primary and it's active, return that. */ > > + HMAP_FOR_EACH (slave, hmap_node, &bond->slaves) { > > + if (slave->is_primary && slave->enabled) { > > + return slave; > > + } > > + } > > + > > /* Find the last active slave. */ > > slave = bond_find_slave_by_mac(bond, bond->active_slave_mac); > > if (slave && slave->enabled) { > > diff --git a/ofproto/bond.h b/ofproto/bond.h > > index 40c3258dc..1a42a37c9 100644 > > --- a/ofproto/bond.h > > +++ b/ofproto/bond.h > > @@ -45,6 +45,7 @@ struct bond_settings { > > > > /* Balancing configuration. */ > > enum bond_mode balance; > > + const char *primary; /* For AB balance, primary interface name. */ > > int rebalance_interval; /* Milliseconds between rebalances. > > Zero to disable rebalancing. */ > > > > diff --git a/tests/lacp.at b/tests/lacp.at > > index df1691731..5257f0cce 100644 > > --- a/tests/lacp.at > > +++ b/tests/lacp.at > > @@ -126,6 +126,7 @@ updelay: 0 ms > > downdelay: 0 ms > > lacp_status: negotiated > > lacp_fallback_ab: false > > +active-backup primary: <none> > > active slave mac: 00:00:00:00:00:00(none) > > > > slave p1: disabled > > @@ -292,6 +293,7 @@ updelay: 0 ms > > downdelay: 0 ms > > lacp_status: negotiated > > lacp_fallback_ab: false > > +active-backup primary: <none> > > > > slave p0: enabled > > may_enable: true > > @@ -308,6 +310,7 @@ updelay: 0 ms > > downdelay: 0 ms > > lacp_status: negotiated > > lacp_fallback_ab: false > > +active-backup primary: <none> > > > > slave p2: enabled > > may_enable: true > > @@ -431,6 +434,7 @@ updelay: 0 ms > > downdelay: 0 ms > > lacp_status: negotiated > > lacp_fallback_ab: false > > +active-backup primary: <none> > > <active slave mac del> > > > > slave p0: disabled > > @@ -449,6 +453,7 @@ updelay: 0 ms > > downdelay: 0 ms > > lacp_status: negotiated > > lacp_fallback_ab: false > > +active-backup primary: <none> > > <active slave mac del> > > > > slave p2: disabled > > @@ -565,6 +570,7 @@ updelay: 0 ms > > downdelay: 0 ms > > lacp_status: negotiated > > lacp_fallback_ab: false > > +active-backup primary: <none> > > <active slave mac del> > > > > slave p0: disabled > > @@ -583,6 +589,7 @@ updelay: 0 ms > > downdelay: 0 ms > > lacp_status: negotiated > > lacp_fallback_ab: false > > +active-backup primary: <none> > > <active slave mac del> > > > > slave p2: disabled > > @@ -704,6 +711,7 @@ updelay: 0 ms > > downdelay: 0 ms > > lacp_status: negotiated > > lacp_fallback_ab: false > > +active-backup primary: <none> > > <active slave mac del> > > > > slave p0: enabled > > @@ -722,6 +730,7 @@ updelay: 0 ms > > downdelay: 0 ms > > lacp_status: negotiated > > lacp_fallback_ab: false > > +active-backup primary: <none> > > <active slave mac del> > > > > slave p2: enabled > > diff --git a/tests/ofproto-dpif.at b/tests/ofproto-dpif.at > > index e3402e7b8..808200800 100644 > > --- a/tests/ofproto-dpif.at > > +++ b/tests/ofproto-dpif.at > > @@ -29,12 +29,16 @@ AT_CHECK([ovs-appctl revalidator/wait]) > > OVS_VSWITCHD_STOP > > AT_CLEANUP > > > > -AT_SETUP([ofproto-dpif - active-backup bonding]) > > -# Create br0 with interfaces p1, p2 and p7, creating bond0 with p1 and p2 > > -# and br1 with interfaces p3, p4 and p8. > > -# toggle p1,p2 of bond0 up and down to test bonding in active-backup mode. > > +AT_SETUP([ofproto-dpif - active-backup bonding (with primary)]) > > + > > +dnl Create br0 with interfaces p1, p2 and p7, creating bond0 with p1 and > > +dnl p2 (p1 as primary) and br1 with interfaces p3, p4 and p8. > > +dnl toggle p1,p2 of bond0 up and down to test bonding in active-backup mode. > > +dnl With p1 down and p2 up/active, bring p1 back up. Since p1 is the primary, > > +dnl it should become active. > > OVS_VSWITCHD_START( > > - [add-bond br0 bond0 p1 p2 bond_mode=active-backup --\ > > + [add-bond br0 bond0 p1 p2 bond_mode=active-backup \ > > + other_config:bond-primary=p1 -- \ > > set interface p1 type=dummy options:pstream=punix:$OVS_RUNDIR/p1.sock ofport_request=1 -- \ > > set interface p2 type=dummy options:pstream=punix:$OVS_RUNDIR/p2.sock ofport_request=2 -- \ > > add-port br0 p7 -- set interface p7 ofport_request=7 type=dummy -- \ > > @@ -45,9 +49,202 @@ OVS_VSWITCHD_START( > > add-port br1 p3 -- set interface p3 type=dummy options:stream=unix:$OVS_RUNDIR/p1.sock ofport_request=3 -- \ > > add-port br1 p4 -- set interface p4 type=dummy options:stream=unix:$OVS_RUNDIR/p2.sock ofport_request=4 -- \ > > add-port br1 p8 -- set interface p8 ofport_request=8 type=dummy --]) > > +AT_CHECK([ovs-appctl vlog/set dpif:dbg dpif_netdev:dbg]) > > WAIT_FOR_DUMMY_PORTS([p3], [p4]) > > +OVS_WAIT_UNTIL([test -n "`ovs-appctl bond/show | grep 'active-backup primary: p1'`"]) > > + > > + > > +AT_CHECK([ovs-ofctl add-flow br0 action=normal]) > > +AT_CHECK([ovs-ofctl add-flow br1 action=normal]) > > +ovs-appctl netdev-dummy/set-admin-state up > > Missing the interface name? > > > > +ovs-appctl time/warp 100 > > +ovs-appctl netdev-dummy/set-admin-state p2 down > > +ovs-appctl time/stop > > +ovs-appctl time/warp 100 > > +AT_CHECK([ovs-appctl netdev-dummy/receive p7 'in_port(7),eth(src=50:54:00:00:00:09,dst=50:54:00:00:00:0a),eth_type(0x0800),ipv4(src=10.0.0.2,dst=10.0.0.1,proto=1,tos=0,ttl=64,frag=no),icmp(type=8,code=0)']) > > +AT_CHECK([ovs-appctl netdev-dummy/receive p7 'in_port(7),eth(src=50:54:00:00:00:0b,dst=50:54:00:00:00:0c),eth_type(0x0800),ipv4(src=10.0.0.3,dst=10.0.0.4,proto=1,tos=0,ttl=64,frag=no),icmp(type=8,code=0)']) > > +ovs-appctl time/warp 100 > > +ovs-appctl netdev-dummy/set-admin-state p2 up > > +ovs-appctl netdev-dummy/set-admin-state p1 down > > +ovs-appctl time/warp 100 > > +AT_CHECK([ovs-appctl netdev-dummy/receive p7 'in_port(7),eth(src=50:54:00:00:00:09,dst=50:54:00:00:00:0d),eth_type(0x0800),ipv4(src=10.0.0.5,dst=10.0.0.1,proto=1,tos=0,ttl=64,frag=no),icmp(type=8,code=0)']) > > +AT_CHECK([ovs-appctl netdev-dummy/receive p7 'in_port(7),eth(src=50:54:00:00:00:09,dst=50:54:00:00:00:0e),eth_type(0x0800),ipv4(src=10.0.0.6,dst=10.0.0.1,proto=1,tos=0,ttl=64,frag=no),icmp(type=8,code=0)']) > > +ovs-appctl time/warp 2000 100 > > +AT_CHECK([ovs-appctl dpctl/dump-flows | grep 'in_port([[348]])' | strip_xout], [0], [dnl > > +recirc_id(0),in_port(3),packet_type(ns=0,id=0),eth(src=50:54:00:00:00:09,dst=50:54:00:00:00:0a),eth_type(0x0800),ipv4(frag=no), packets:0, bytes:0, used:never, actions: <del> > > +recirc_id(0),in_port(3),packet_type(ns=0,id=0),eth(src=50:54:00:00:00:0b,dst=50:54:00:00:00:0c),eth_type(0x0800),ipv4(frag=no), packets:0, bytes:0, used:never, actions: <del> > > +recirc_id(0),in_port(4),packet_type(ns=0,id=0),eth(src=50:54:00:00:00:09,dst=50:54:00:00:00:0d),eth_type(0x0800),ipv4(frag=no), packets:0, bytes:0, used:never, actions: <del> > > +recirc_id(0),in_port(4),packet_type(ns=0,id=0),eth(src=50:54:00:00:00:09,dst=50:54:00:00:00:0e),eth_type(0x0800),ipv4(frag=no), packets:0, bytes:0, used:never, actions: <del> > > +recirc_id(0),in_port(4),packet_type(ns=0,id=0),eth(src=50:54:00:00:00:09,dst=ff:ff:ff:ff:ff:ff),eth_type(0x8035), packets:0, bytes:0, used:never, actions: <del> > > +recirc_id(0),in_port(4),packet_type(ns=0,id=0),eth(src=50:54:00:00:00:0b,dst=ff:ff:ff:ff:ff:ff),eth_type(0x8035), packets:0, bytes:0, used:never, actions: <del> > > +]) > > + > > +ovs-appctl netdev-dummy/set-admin-state p1 up > > +ovs-appctl time/warp 100 > > +OVS_WAIT_UNTIL([ovs-appctl bond/show | STRIP_RECIRC_ID | STRIP_ACTIVE_SLAVE_MAC], [0], [dnl > > +---- bond0 ---- > > +bond_mode: active-backup > > +bond may use recirculation: no, <del> > > +bond-hash-basis: 0 > > +updelay: 0 ms > > +downdelay: 0 ms > > +lacp_status: off > > +lacp_fallback_ab: false > > +active-backup primary: p1 > > +<active slave mac del> > > + > > +slave p1: enabled > > + active slave > > + may_enable: true > > + > > +slave p2: enabled > > + may_enable: true > > + > > +]) > > + > > +OVS_VSWITCHD_STOP > > +AT_CLEANUP > > + > > +AT_SETUP([ofproto-dpif - active-backup bonding (primary validation)]) > > +dnl Make a switch with 3 ports in a bond, so that when we delete one of > > +dnl the ports from the bond, there are still 2 ports left and the bond > > +dnl remains functional. > > +OVS_VSWITCHD_START( > > + [add-bond br0 bond0 p1 p2 p3 bond_mode=active-backup \ > > + other_config:bond-primary=p1 -- \ > > + set interface p1 type=dummy options:pstream=punix:$OVS_RUNDIR/p1.sock ofport_request=1 -- \ > > + set interface p2 type=dummy options:pstream=punix:$OVS_RUNDIR/p2.sock ofport_request=2 -- \ > > + set interface p3 type=dummy options:pstream=punix:$OVS_RUNDIR/p3.sock ofport_request=3 -- \ > > + add-port br0 p7 -- set interface p7 ofport_request=7 type=dummy --]) > > AT_CHECK([ovs-appctl vlog/set dpif:dbg dpif_netdev:dbg]) > > > > +dnl Make sure the initial primary interface is set > > +OVS_WAIT_UNTIL([test -n "`ovs-appctl bond/show | grep 'active-backup primary: p1'`"]) > > + > > +dnl Down the primary interface and verify that we switched. Then > > +dnl bring the primary back and verify that we switched back to the > > +dnl primary. > > +ovs-appctl netdev-dummy/set-admin-state p1 down > > +ovs-appctl time/warp 100 > > +OVS_WAIT_UNTIL([test -n "`ovs-appctl bond/show | fgrep 'slave p1: disabled'`"]) > > +ovs-appctl netdev-dummy/set-admin-state p1 up > > +ovs-appctl time/warp 100 > > +OVS_WAIT_UNTIL([ovs-appctl bond/show | STRIP_RECIRC_ID | STRIP_ACTIVE_SLAVE_MAC], [0], [dnl > > +---- bond0 ---- > > +bond_mode: active-backup > > +bond may use recirculation: no, <del> > > +bond-hash-basis: 0 > > +updelay: 0 ms > > +downdelay: 0 ms > > +lacp_status: off > > +lacp_fallback_ab: false > > +active-backup primary: p1 > > +<active slave mac del> > > + > > +slave p1: enabled > > + active slave > > + may_enable: true > > + > > +slave p2: enabled > > + may_enable: true > > + > > +slave p3: enabled > > + may_enable: true > > + > > +]) > > + > > +dnl Now delete the primary and verify that the output shows that the > > +dnl primary is no longer enslaved > > +ovs-vsctl --id=@p1 get Interface p1 -- remove Port bond0 interfaces @p1 > > +ovs-appctl time/warp 100 > > +OVS_WAIT_UNTIL([test -n "`ovs-appctl bond/show | fgrep 'active-backup primary: p1 (no such slave)'`"]) > > + > > +dnl Now re-add the primary and verify that the output shows that the > > +dnl primary is available again. > > +dnl > > +dnl First, get the UUIDs of the interfaces that exist on bond0. > > +dnl Strip the trailing ] so that we can add a new UUID to the end. > > +uuids=`ovs-vsctl get Port bond0 interfaces | sed -e 's/]//'` > > +dnl Create a new port "p1" and add its UUID to the set of interfaces > > +dnl on bond0. > > +ovs-vsctl \ > > + --id=@p1 create Interface name=p1 type=dummy options:pstream=punix:$OVS_RUNDIR/p1.sock ofport_request=1 -- \ > > + set Port bond0 interfaces="$uuids, @p1]" > > +ovs-appctl time/warp 100 > > +OVS_WAIT_UNTIL([ovs-appctl bond/show | STRIP_RECIRC_ID | STRIP_ACTIVE_SLAVE_MAC], [0], [dnl > > +---- bond0 ---- > > +bond_mode: active-backup > > +bond may use recirculation: no, <del> > > +bond-hash-basis: 0 > > +updelay: 0 ms > > +downdelay: 0 ms > > +lacp_status: off > > +lacp_fallback_ab: false > > +active-backup primary: p1 > > +<active slave mac del> > > + > > +slave p1: enabled > > + active slave > > + may_enable: true > > + > > +slave p2: enabled > > + may_enable: true > > + > > +slave p3: enabled > > + may_enable: true > > + > > +]) > > + > > +dnl Remove the "bond-primary" config directive from the bond. > > +AT_CHECK([ovs-vsctl remove Port bond0 other_config bond-primary]) > > +ovs-appctl time/warp 100 > > +OVS_WAIT_UNTIL([ovs-appctl bond/show | STRIP_RECIRC_ID | STRIP_ACTIVE_SLAVE_MAC], [0], [dnl > > +---- bond0 ---- > > +bond_mode: active-backup > > +bond may use recirculation: no, <del> > > +bond-hash-basis: 0 > > +updelay: 0 ms > > +downdelay: 0 ms > > +lacp_status: off > > +lacp_fallback_ab: false > > +active-backup primary: <none> > > +<active slave mac del> > > + > > +slave p1: enabled > > + active slave > > + may_enable: true > > + > > +slave p2: enabled > > + may_enable: true > > + > > +slave p3: enabled > > + may_enable: true > > + > > +]) > > + > > +OVS_VSWITCHD_STOP > > +AT_CLEANUP > > + > > +AT_SETUP([ofproto-dpif - active-backup bonding (without primary)]) > > +dnl Create br0 with interfaces p1, p2 and p7, creating bond0 with p1 and p2 > > +dnl and br1 with interfaces p3, p4 and p8. > > +dnl toggle p1,p2 of bond0 up and down to test bonding in active-backup mode. > > +OVS_VSWITCHD_START( > > + [add-bond br0 bond0 p1 p2 bond_mode=active-backup --\ > > + set interface p1 type=dummy options:pstream=punix:$OVS_RUNDIR/p1.sock ofport_request=1 -- \ > > + set interface p2 type=dummy options:pstream=punix:$OVS_RUNDIR/p2.sock ofport_request=2 -- \ > > + add-port br0 p7 -- set interface p7 ofport_request=7 type=dummy -- \ > > + add-br br1 -- \ > > + set bridge br1 other-config:hwaddr=aa:66:aa:66:00:00 -- \ > > + set bridge br1 datapath-type=dummy other-config:datapath-id=1234 \ > > + fail-mode=secure -- \ > > + add-port br1 p3 -- set interface p3 type=dummy options:stream=unix:$OVS_RUNDIR/p1.sock ofport_request=3 -- \ > > + add-port br1 p4 -- set interface p4 type=dummy options:stream=unix:$OVS_RUNDIR/p2.sock ofport_request=4 -- \ > > + add-port br1 p8 -- set interface p8 ofport_request=8 type=dummy --]) > > +AT_CHECK([ovs-appctl vlog/set dpif:dbg dpif_netdev:dbg]) > > +WAIT_FOR_DUMMY_PORTS([p3], [p4]) > > +OVS_WAIT_UNTIL([test -n "`ovs-appctl bond/show | grep 'active-backup primary: <none>'`"]) > > + > > AT_CHECK([ovs-ofctl add-flow br0 action=normal]) > > AT_CHECK([ovs-ofctl add-flow br1 action=normal]) > > ovs-appctl netdev-dummy/set-admin-state up > > @@ -63,15 +260,14 @@ ovs-appctl netdev-dummy/set-admin-state p1 down > > ovs-appctl time/warp 100 > > AT_CHECK([ovs-appctl netdev-dummy/receive p7 'in_port(7),eth(src=50:54:00:00:00:09,dst=50:54:00:00:00:0d),eth_type(0x0800),ipv4(src=10.0.0.5,dst=10.0.0.1,proto=1,tos=0,ttl=64,frag=no),icmp(type=8,code=0)']) > > AT_CHECK([ovs-appctl netdev-dummy/receive p7 'in_port(7),eth(src=50:54:00:00:00:09,dst=50:54:00:00:00:0e),eth_type(0x0800),ipv4(src=10.0.0.6,dst=10.0.0.1,proto=1,tos=0,ttl=64,frag=no),icmp(type=8,code=0)']) > > -ovs-appctl time/warp 200 100 > > -sleep 1 > > -AT_CHECK([grep 'in_port([[348]])' ovs-vswitchd.log | filter_flow_install | strip_xout], [0], [dnl > > -recirc_id(0),in_port(3),packet_type(ns=0,id=0),eth(src=50:54:00:00:00:09,dst=50:54:00:00:00:0a),eth_type(0x0800),ipv4(frag=no), actions: <del> > > -recirc_id(0),in_port(3),packet_type(ns=0,id=0),eth(src=50:54:00:00:00:0b,dst=50:54:00:00:00:0c),eth_type(0x0800),ipv4(frag=no), actions: <del> > > -recirc_id(0),in_port(4),packet_type(ns=0,id=0),eth(src=50:54:00:00:00:09,dst=50:54:00:00:00:0d),eth_type(0x0800),ipv4(frag=no), actions: <del> > > -recirc_id(0),in_port(4),packet_type(ns=0,id=0),eth(src=50:54:00:00:00:09,dst=50:54:00:00:00:0e),eth_type(0x0800),ipv4(frag=no), actions: <del> > > -recirc_id(0),in_port(4),packet_type(ns=0,id=0),eth(src=50:54:00:00:00:09,dst=ff:ff:ff:ff:ff:ff),eth_type(0x8035), actions: <del> > > -recirc_id(0),in_port(4),packet_type(ns=0,id=0),eth(src=50:54:00:00:00:0b,dst=ff:ff:ff:ff:ff:ff),eth_type(0x8035), actions: <del> > > +ovs-appctl time/warp 2000 100 > > +AT_CHECK([ovs-appctl dpctl/dump-flows | grep 'in_port([[348]])' | strip_xout], [0], [dnl > > +recirc_id(0),in_port(3),packet_type(ns=0,id=0),eth(src=50:54:00:00:00:09,dst=50:54:00:00:00:0a),eth_type(0x0800),ipv4(frag=no), packets:0, bytes:0, used:never, actions: <del> > > +recirc_id(0),in_port(3),packet_type(ns=0,id=0),eth(src=50:54:00:00:00:0b,dst=50:54:00:00:00:0c),eth_type(0x0800),ipv4(frag=no), packets:0, bytes:0, used:never, actions: <del> > > +recirc_id(0),in_port(4),packet_type(ns=0,id=0),eth(src=50:54:00:00:00:09,dst=50:54:00:00:00:0d),eth_type(0x0800),ipv4(frag=no), packets:0, bytes:0, used:never, actions: <del> > > +recirc_id(0),in_port(4),packet_type(ns=0,id=0),eth(src=50:54:00:00:00:09,dst=50:54:00:00:00:0e),eth_type(0x0800),ipv4(frag=no), packets:0, bytes:0, used:never, actions: <del> > > +recirc_id(0),in_port(4),packet_type(ns=0,id=0),eth(src=50:54:00:00:00:09,dst=ff:ff:ff:ff:ff:ff),eth_type(0x8035), packets:0, bytes:0, used:never, actions: <del> > > +recirc_id(0),in_port(4),packet_type(ns=0,id=0),eth(src=50:54:00:00:00:0b,dst=ff:ff:ff:ff:ff:ff),eth_type(0x8035), packets:0, bytes:0, used:never, actions: <del> > > ]) > > OVS_VSWITCHD_STOP > > AT_CLEANUP > > diff --git a/vswitchd/bridge.c b/vswitchd/bridge.c > > index f312efd8e..7111fbac3 100644 > > --- a/vswitchd/bridge.c > > +++ b/vswitchd/bridge.c > > @@ -4563,6 +4563,11 @@ port_configure_bond(struct port *port, struct bond_settings *s) > > port->name); > > } > > > > + s->primary = NULL; > > + if (s->balance == BM_AB || s->lacp_fallback_ab_cfg) { > > + s->primary = smap_get(&port->cfg->other_config, "bond-primary"); > > + } > > + > > miimon_interval = smap_get_int(&port->cfg->other_config, > > "bond-miimon-interval", 0); > > if (miimon_interval <= 0) { > > diff --git a/vswitchd/vswitch.xml b/vswitchd/vswitch.xml > > index b6acb34ca..944708ed6 100644 > > --- a/vswitchd/vswitch.xml > > +++ b/vswitchd/vswitch.xml > > @@ -2003,6 +2003,14 @@ > > This knob does not affect other balancing modes. > > </column> > > > > + <column name="other_config" key="bond-primary" > > + type='{"type": "string"}'> > > + If a slave interface with this name exists in the bond and > > + is up, it will be made active. Relevant only when <ref > > + column="other_config" key="bond-mode"/> is > > I think it is bond_mode. > > > > + <code>active-backup</code>. > > + </column> > > + > > <group title="Link Failure Detection"> > > <p> > > An important part of link bonding is detecting that links are down so > > -- > > 2.26.2 > > > > _______________________________________________ > > dev mailing list > > dev@openvswitch.org > > https://mail.openvswitch.org/mailman/listinfo/ovs-dev > > -- > fbl > _______________________________________________ > dev mailing list > dev@openvswitch.org > https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Jeff Squyres <jsquyres@cisco.com> writes: > In AB bonding, if the current active slave becomes disabled, a > replacement slave is arbitrarily picked from the remaining set of > enabled slaves. This commit adds the concept of a "primary" slave: an > interface that will always be (or become) the current active slave if > it is enabled. > > The rationale for this functionality is to allow the designation of a > preferred interface for a given bond. For example: > > 1. Bond is created with interfaces p1 (primary) and p2, both enabled. > 2. p1 becomes the current active slave (because it was designated as > the primary). > 3. Later, p1 fails/becomes disabled. > 4. p2 is chosen to become the current active slave. > 5. Later, p1 becomes re-enabled. > 6. p1 is chosen to become the current active slave (because it was > designated as the primary) > > Note that p1 becomes the active slave once it becomes re-enabled, even > if nothing has happened to p2. > > This "primary" concept exists in Linux kernel network interface > bonding, but did not previously exist in OVS bonding. > > Only one primary slave inteface is supported per bond, and is only > supported for active/backup bonding. > > The primary slave interface is designated via > "other_config:bond-primary" when creating a bond. > > Also, while adding tests for the "primary" concept, make a few small > improvements to the non-primary AB bonding test. > > Signed-off-by: Jeff Squyres <jsquyres@cisco.com> > Reviewed-by: Aaron Conole <aconole@redhat.com> > Tested-by: Greg Rose <gvrose8192@gmail.com> > Acked-by: Greg Rose <gvrose8192@gmail.com> > --- Still looks good to me. > v9: > - Removed stale code that should have been removed in v8. > - Fixed comment punctuation. > > v8: > - Many style and simplification suggestions from Ilya Maximets > - Rename "primary:" to "active-backup primary:" in ovs-appctl > bond/show output, and make it always appear (vs. only appearing in > explicit AB bonding scenarios). > - Properly handle fallback-to-AB-bonding cases > - Use time/warp and OVS_WAIT_UNTIL in the bonding tests to prevent > races, and a few other improvements to the test quality > - After rebasing patch to head of tree, adjust test output as result > of changes from other commits. > > v7: > - After rebasing patch to head of tree, adjust test output as result > of changes from 81f71381f. > > v6: > - Style: bleep bloop. Fix use of {}. > > v5: > - Handle when bond is reconfigured to remove "bond-primary" config. > - Fix potential flapping between remaining slaves. > - Add a test to re-add a removed primary interface and make sure the > bond reflects that properly. > - Add a test to remove "bond-primary" config and make sure the bond > reflects that properly. > > v4: > - Style: bleep bloop. Trim line length below 79 characters. > > v3: > - Adjusted print logic for when the primary slave is not attached > > v2: > - Added "ovs-vsctl bond/show" label when primary interface is no > longer enslaved > - Fixed strcmp() usage nits > - Moved "other_config:bond-primary" docs to the "Bonding > Configuration" group > - Expanded commit message > - Added more test cases (including one for when the primary interface > is no longer enslaved) > > ofproto/bond.c | 54 ++++++++-- > ofproto/bond.h | 1 + > tests/lacp.at | 9 ++ > tests/ofproto-dpif.at | 224 +++++++++++++++++++++++++++++++++++++++--- > vswitchd/bridge.c | 5 + > vswitchd/vswitch.xml | 8 ++ > 6 files changed, 280 insertions(+), 21 deletions(-) > > diff --git a/ofproto/bond.c b/ofproto/bond.c > index 9947e7531..58ee67beb 100644 > --- a/ofproto/bond.c > +++ b/ofproto/bond.c > @@ -89,6 +89,7 @@ struct bond_slave { > /* Link status. */ > bool enabled; /* May be chosen for flows? */ > bool may_enable; /* Client considers this slave bondable. */ > + bool is_primary; /* This slave is preferred over others. */ > long long delay_expires; /* Time after which 'enabled' may change. */ > > /* Rebalancing info. Used only by bond_rebalance(). */ > @@ -124,6 +125,7 @@ struct bond { > uint32_t basis; /* Basis for flow hash function. */ > bool use_lb_output_action; /* Use lb_output action to avoid recirculation. > Applicable only for Balance TCP mode. */ > + char *primary; /* Name of the primary slave interface. */ > > /* SLB specific bonding info. */ > struct bond_entry *hash; /* An array of BOND_BUCKETS elements. */ > @@ -241,6 +243,7 @@ bond_create(const struct bond_settings *s, struct ofproto_dpif *ofproto) > > bond->active_slave_mac = eth_addr_zero; > bond->active_slave_changed = false; > + bond->primary = NULL; > > bond_reconfigure(bond, s); > return bond; > @@ -294,6 +297,7 @@ bond_unref(struct bond *bond) > update_recirc_rules__(bond); > > hmap_destroy(&bond->pr_rule_ops); > + free(bond->primary); > free(bond->name); > free(bond); > } > @@ -471,6 +475,12 @@ bond_reconfigure(struct bond *bond, const struct bond_settings *s) > bond->bond_revalidate = false; > } > > + if (!nullable_string_is_equal(bond->primary, s->primary)) { > + free(bond->primary); > + bond->primary = nullable_xstrdup(s->primary); > + revalidate = true; > + } > + > if (bond->balance != BM_AB) { > if (!bond->recirc_id) { > bond->recirc_id = recirc_alloc_id(bond->ofproto); > @@ -575,6 +585,12 @@ bond_slave_register(struct bond *bond, void *slave_, > slave->name = xstrdup(netdev_get_name(netdev)); > bond->bond_revalidate = true; > > + if (bond->primary && !strcmp(bond->primary, slave->name)) { > + slave->is_primary = true; > + } else { > + slave->is_primary = false; > + } > + > slave->enabled = false; > bond_enable_slave(slave, netdev_get_carrier(netdev)); > } > @@ -668,7 +684,7 @@ bond_slave_set_may_enable(struct bond *bond, void *slave_, bool may_enable) > bool > bond_run(struct bond *bond, enum lacp_status lacp_status) > { > - struct bond_slave *slave; > + struct bond_slave *slave, *primary; > bool revalidate; > > ovs_rwlock_wrlock(&rwlock); > @@ -685,11 +701,19 @@ bond_run(struct bond *bond, enum lacp_status lacp_status) > } > > /* Enable slaves based on link status and LACP feedback. */ > + primary = NULL; > HMAP_FOR_EACH (slave, hmap_node, &bond->slaves) { > bond_link_status_update(slave); > slave->change_seq = seq_read(connectivity_seq_get()); > + > + /* Discover if there is an active slave marked 'primary'. */ > + if (bond->balance == BM_AB && slave->is_primary && slave->enabled) { > + primary = slave; > + } > } > - if (!bond->active_slave || !bond->active_slave->enabled) { > + > + if (!bond->active_slave || !bond->active_slave->enabled || > + (primary && bond->active_slave != primary)) { > bond_choose_active_slave(bond); > } > > @@ -1437,16 +1461,25 @@ bond_print_details(struct ds *ds, const struct bond *bond) > ds_put_format(ds, "lacp_fallback_ab: %s\n", > bond->lacp_fallback_ab ? "true" : "false"); > > - ds_put_cstr(ds, "active slave mac: "); > - ds_put_format(ds, ETH_ADDR_FMT, ETH_ADDR_ARGS(bond->active_slave_mac)); > + bool found_primary = false; > slave = bond_find_slave_by_mac(bond, bond->active_slave_mac); > - ds_put_format(ds,"(%s)\n", slave ? slave->name : "none"); > - > HMAP_FOR_EACH (slave, hmap_node, &bond->slaves) { > + if (slave->is_primary) { > + found_primary = true; > + } > shash_add(&slave_shash, slave->name, slave); > } > - sorted_slaves = shash_sort(&slave_shash); > > + ds_put_format(ds, "active-backup primary: %s%s\n", > + bond->primary ? bond->primary : "<none>", > + (!found_primary && bond->primary) > + ? " (no such slave)" : ""); > + > + ds_put_cstr(ds, "active slave mac: "); > + ds_put_format(ds, ETH_ADDR_FMT, ETH_ADDR_ARGS(bond->active_slave_mac)); > + ds_put_format(ds,"(%s)\n", slave ? slave->name : "none"); > + > + sorted_slaves = shash_sort(&slave_shash); > for (i = 0; i < shash_count(&slave_shash); i++) { > struct bond_entry *be; > > @@ -1906,6 +1939,13 @@ bond_choose_slave(const struct bond *bond) > { > struct bond_slave *slave, *best; > > + /* If there's a primary and it's active, return that. */ > + HMAP_FOR_EACH (slave, hmap_node, &bond->slaves) { > + if (slave->is_primary && slave->enabled) { > + return slave; > + } > + } > + > /* Find the last active slave. */ > slave = bond_find_slave_by_mac(bond, bond->active_slave_mac); > if (slave && slave->enabled) { > diff --git a/ofproto/bond.h b/ofproto/bond.h > index 40c3258dc..1a42a37c9 100644 > --- a/ofproto/bond.h > +++ b/ofproto/bond.h > @@ -45,6 +45,7 @@ struct bond_settings { > > /* Balancing configuration. */ > enum bond_mode balance; > + const char *primary; /* For AB balance, primary interface name. */ > int rebalance_interval; /* Milliseconds between rebalances. > Zero to disable rebalancing. */ > > diff --git a/tests/lacp.at b/tests/lacp.at > index df1691731..5257f0cce 100644 > --- a/tests/lacp.at > +++ b/tests/lacp.at > @@ -126,6 +126,7 @@ updelay: 0 ms > downdelay: 0 ms > lacp_status: negotiated > lacp_fallback_ab: false > +active-backup primary: <none> > active slave mac: 00:00:00:00:00:00(none) > > slave p1: disabled > @@ -292,6 +293,7 @@ updelay: 0 ms > downdelay: 0 ms > lacp_status: negotiated > lacp_fallback_ab: false > +active-backup primary: <none> > > slave p0: enabled > may_enable: true > @@ -308,6 +310,7 @@ updelay: 0 ms > downdelay: 0 ms > lacp_status: negotiated > lacp_fallback_ab: false > +active-backup primary: <none> > > slave p2: enabled > may_enable: true > @@ -431,6 +434,7 @@ updelay: 0 ms > downdelay: 0 ms > lacp_status: negotiated > lacp_fallback_ab: false > +active-backup primary: <none> > <active slave mac del> > > slave p0: disabled > @@ -449,6 +453,7 @@ updelay: 0 ms > downdelay: 0 ms > lacp_status: negotiated > lacp_fallback_ab: false > +active-backup primary: <none> > <active slave mac del> > > slave p2: disabled > @@ -565,6 +570,7 @@ updelay: 0 ms > downdelay: 0 ms > lacp_status: negotiated > lacp_fallback_ab: false > +active-backup primary: <none> > <active slave mac del> > > slave p0: disabled > @@ -583,6 +589,7 @@ updelay: 0 ms > downdelay: 0 ms > lacp_status: negotiated > lacp_fallback_ab: false > +active-backup primary: <none> > <active slave mac del> > > slave p2: disabled > @@ -704,6 +711,7 @@ updelay: 0 ms > downdelay: 0 ms > lacp_status: negotiated > lacp_fallback_ab: false > +active-backup primary: <none> > <active slave mac del> > > slave p0: enabled > @@ -722,6 +730,7 @@ updelay: 0 ms > downdelay: 0 ms > lacp_status: negotiated > lacp_fallback_ab: false > +active-backup primary: <none> > <active slave mac del> > > slave p2: enabled > diff --git a/tests/ofproto-dpif.at b/tests/ofproto-dpif.at > index e3402e7b8..808200800 100644 > --- a/tests/ofproto-dpif.at > +++ b/tests/ofproto-dpif.at > @@ -29,12 +29,16 @@ AT_CHECK([ovs-appctl revalidator/wait]) > OVS_VSWITCHD_STOP > AT_CLEANUP > > -AT_SETUP([ofproto-dpif - active-backup bonding]) > -# Create br0 with interfaces p1, p2 and p7, creating bond0 with p1 and p2 > -# and br1 with interfaces p3, p4 and p8. > -# toggle p1,p2 of bond0 up and down to test bonding in active-backup mode. > +AT_SETUP([ofproto-dpif - active-backup bonding (with primary)]) > + > +dnl Create br0 with interfaces p1, p2 and p7, creating bond0 with p1 and > +dnl p2 (p1 as primary) and br1 with interfaces p3, p4 and p8. > +dnl toggle p1,p2 of bond0 up and down to test bonding in active-backup mode. > +dnl With p1 down and p2 up/active, bring p1 back up. Since p1 is the primary, > +dnl it should become active. > OVS_VSWITCHD_START( > - [add-bond br0 bond0 p1 p2 bond_mode=active-backup --\ > + [add-bond br0 bond0 p1 p2 bond_mode=active-backup \ > + other_config:bond-primary=p1 -- \ > set interface p1 type=dummy options:pstream=punix:$OVS_RUNDIR/p1.sock ofport_request=1 -- \ > set interface p2 type=dummy options:pstream=punix:$OVS_RUNDIR/p2.sock ofport_request=2 -- \ > add-port br0 p7 -- set interface p7 ofport_request=7 type=dummy -- \ > @@ -45,9 +49,202 @@ OVS_VSWITCHD_START( > add-port br1 p3 -- set interface p3 type=dummy options:stream=unix:$OVS_RUNDIR/p1.sock ofport_request=3 -- \ > add-port br1 p4 -- set interface p4 type=dummy options:stream=unix:$OVS_RUNDIR/p2.sock ofport_request=4 -- \ > add-port br1 p8 -- set interface p8 ofport_request=8 type=dummy --]) > +AT_CHECK([ovs-appctl vlog/set dpif:dbg dpif_netdev:dbg]) > WAIT_FOR_DUMMY_PORTS([p3], [p4]) > +OVS_WAIT_UNTIL([test -n "`ovs-appctl bond/show | grep 'active-backup primary: p1'`"]) > + > + > +AT_CHECK([ovs-ofctl add-flow br0 action=normal]) > +AT_CHECK([ovs-ofctl add-flow br1 action=normal]) > +ovs-appctl netdev-dummy/set-admin-state up > +ovs-appctl time/warp 100 > +ovs-appctl netdev-dummy/set-admin-state p2 down > +ovs-appctl time/stop > +ovs-appctl time/warp 100 > +AT_CHECK([ovs-appctl netdev-dummy/receive p7 'in_port(7),eth(src=50:54:00:00:00:09,dst=50:54:00:00:00:0a),eth_type(0x0800),ipv4(src=10.0.0.2,dst=10.0.0.1,proto=1,tos=0,ttl=64,frag=no),icmp(type=8,code=0)']) > +AT_CHECK([ovs-appctl netdev-dummy/receive p7 'in_port(7),eth(src=50:54:00:00:00:0b,dst=50:54:00:00:00:0c),eth_type(0x0800),ipv4(src=10.0.0.3,dst=10.0.0.4,proto=1,tos=0,ttl=64,frag=no),icmp(type=8,code=0)']) > +ovs-appctl time/warp 100 > +ovs-appctl netdev-dummy/set-admin-state p2 up > +ovs-appctl netdev-dummy/set-admin-state p1 down > +ovs-appctl time/warp 100 > +AT_CHECK([ovs-appctl netdev-dummy/receive p7 'in_port(7),eth(src=50:54:00:00:00:09,dst=50:54:00:00:00:0d),eth_type(0x0800),ipv4(src=10.0.0.5,dst=10.0.0.1,proto=1,tos=0,ttl=64,frag=no),icmp(type=8,code=0)']) > +AT_CHECK([ovs-appctl netdev-dummy/receive p7 'in_port(7),eth(src=50:54:00:00:00:09,dst=50:54:00:00:00:0e),eth_type(0x0800),ipv4(src=10.0.0.6,dst=10.0.0.1,proto=1,tos=0,ttl=64,frag=no),icmp(type=8,code=0)']) > +ovs-appctl time/warp 2000 100 > +AT_CHECK([ovs-appctl dpctl/dump-flows | grep 'in_port([[348]])' | strip_xout], [0], [dnl > +recirc_id(0),in_port(3),packet_type(ns=0,id=0),eth(src=50:54:00:00:00:09,dst=50:54:00:00:00:0a),eth_type(0x0800),ipv4(frag=no), packets:0, bytes:0, used:never, actions: <del> > +recirc_id(0),in_port(3),packet_type(ns=0,id=0),eth(src=50:54:00:00:00:0b,dst=50:54:00:00:00:0c),eth_type(0x0800),ipv4(frag=no), packets:0, bytes:0, used:never, actions: <del> > +recirc_id(0),in_port(4),packet_type(ns=0,id=0),eth(src=50:54:00:00:00:09,dst=50:54:00:00:00:0d),eth_type(0x0800),ipv4(frag=no), packets:0, bytes:0, used:never, actions: <del> > +recirc_id(0),in_port(4),packet_type(ns=0,id=0),eth(src=50:54:00:00:00:09,dst=50:54:00:00:00:0e),eth_type(0x0800),ipv4(frag=no), packets:0, bytes:0, used:never, actions: <del> > +recirc_id(0),in_port(4),packet_type(ns=0,id=0),eth(src=50:54:00:00:00:09,dst=ff:ff:ff:ff:ff:ff),eth_type(0x8035), packets:0, bytes:0, used:never, actions: <del> > +recirc_id(0),in_port(4),packet_type(ns=0,id=0),eth(src=50:54:00:00:00:0b,dst=ff:ff:ff:ff:ff:ff),eth_type(0x8035), packets:0, bytes:0, used:never, actions: <del> > +]) > + > +ovs-appctl netdev-dummy/set-admin-state p1 up > +ovs-appctl time/warp 100 > +OVS_WAIT_UNTIL([ovs-appctl bond/show | STRIP_RECIRC_ID | STRIP_ACTIVE_SLAVE_MAC], [0], [dnl > +---- bond0 ---- > +bond_mode: active-backup > +bond may use recirculation: no, <del> > +bond-hash-basis: 0 > +updelay: 0 ms > +downdelay: 0 ms > +lacp_status: off > +lacp_fallback_ab: false > +active-backup primary: p1 > +<active slave mac del> > + > +slave p1: enabled > + active slave > + may_enable: true > + > +slave p2: enabled > + may_enable: true > + > +]) > + > +OVS_VSWITCHD_STOP > +AT_CLEANUP > + > +AT_SETUP([ofproto-dpif - active-backup bonding (primary validation)]) > +dnl Make a switch with 3 ports in a bond, so that when we delete one of > +dnl the ports from the bond, there are still 2 ports left and the bond > +dnl remains functional. > +OVS_VSWITCHD_START( > + [add-bond br0 bond0 p1 p2 p3 bond_mode=active-backup \ > + other_config:bond-primary=p1 -- \ > + set interface p1 type=dummy options:pstream=punix:$OVS_RUNDIR/p1.sock ofport_request=1 -- \ > + set interface p2 type=dummy options:pstream=punix:$OVS_RUNDIR/p2.sock ofport_request=2 -- \ > + set interface p3 type=dummy options:pstream=punix:$OVS_RUNDIR/p3.sock ofport_request=3 -- \ > + add-port br0 p7 -- set interface p7 ofport_request=7 type=dummy --]) > AT_CHECK([ovs-appctl vlog/set dpif:dbg dpif_netdev:dbg]) > > +dnl Make sure the initial primary interface is set > +OVS_WAIT_UNTIL([test -n "`ovs-appctl bond/show | grep 'active-backup primary: p1'`"]) > + > +dnl Down the primary interface and verify that we switched. Then > +dnl bring the primary back and verify that we switched back to the > +dnl primary. > +ovs-appctl netdev-dummy/set-admin-state p1 down > +ovs-appctl time/warp 100 > +OVS_WAIT_UNTIL([test -n "`ovs-appctl bond/show | fgrep 'slave p1: disabled'`"]) > +ovs-appctl netdev-dummy/set-admin-state p1 up > +ovs-appctl time/warp 100 > +OVS_WAIT_UNTIL([ovs-appctl bond/show | STRIP_RECIRC_ID | STRIP_ACTIVE_SLAVE_MAC], [0], [dnl > +---- bond0 ---- > +bond_mode: active-backup > +bond may use recirculation: no, <del> > +bond-hash-basis: 0 > +updelay: 0 ms > +downdelay: 0 ms > +lacp_status: off > +lacp_fallback_ab: false > +active-backup primary: p1 > +<active slave mac del> > + > +slave p1: enabled > + active slave > + may_enable: true > + > +slave p2: enabled > + may_enable: true > + > +slave p3: enabled > + may_enable: true > + > +]) > + > +dnl Now delete the primary and verify that the output shows that the > +dnl primary is no longer enslaved > +ovs-vsctl --id=@p1 get Interface p1 -- remove Port bond0 interfaces @p1 > +ovs-appctl time/warp 100 > +OVS_WAIT_UNTIL([test -n "`ovs-appctl bond/show | fgrep 'active-backup primary: p1 (no such slave)'`"]) > + > +dnl Now re-add the primary and verify that the output shows that the > +dnl primary is available again. > +dnl > +dnl First, get the UUIDs of the interfaces that exist on bond0. > +dnl Strip the trailing ] so that we can add a new UUID to the end. > +uuids=`ovs-vsctl get Port bond0 interfaces | sed -e 's/]//'` > +dnl Create a new port "p1" and add its UUID to the set of interfaces > +dnl on bond0. > +ovs-vsctl \ > + --id=@p1 create Interface name=p1 type=dummy options:pstream=punix:$OVS_RUNDIR/p1.sock ofport_request=1 -- \ > + set Port bond0 interfaces="$uuids, @p1]" > +ovs-appctl time/warp 100 > +OVS_WAIT_UNTIL([ovs-appctl bond/show | STRIP_RECIRC_ID | STRIP_ACTIVE_SLAVE_MAC], [0], [dnl > +---- bond0 ---- > +bond_mode: active-backup > +bond may use recirculation: no, <del> > +bond-hash-basis: 0 > +updelay: 0 ms > +downdelay: 0 ms > +lacp_status: off > +lacp_fallback_ab: false > +active-backup primary: p1 > +<active slave mac del> > + > +slave p1: enabled > + active slave > + may_enable: true > + > +slave p2: enabled > + may_enable: true > + > +slave p3: enabled > + may_enable: true > + > +]) > + > +dnl Remove the "bond-primary" config directive from the bond. > +AT_CHECK([ovs-vsctl remove Port bond0 other_config bond-primary]) > +ovs-appctl time/warp 100 > +OVS_WAIT_UNTIL([ovs-appctl bond/show | STRIP_RECIRC_ID | STRIP_ACTIVE_SLAVE_MAC], [0], [dnl > +---- bond0 ---- > +bond_mode: active-backup > +bond may use recirculation: no, <del> > +bond-hash-basis: 0 > +updelay: 0 ms > +downdelay: 0 ms > +lacp_status: off > +lacp_fallback_ab: false > +active-backup primary: <none> > +<active slave mac del> > + > +slave p1: enabled > + active slave > + may_enable: true > + > +slave p2: enabled > + may_enable: true > + > +slave p3: enabled > + may_enable: true > + > +]) > + > +OVS_VSWITCHD_STOP > +AT_CLEANUP > + > +AT_SETUP([ofproto-dpif - active-backup bonding (without primary)]) > +dnl Create br0 with interfaces p1, p2 and p7, creating bond0 with p1 and p2 > +dnl and br1 with interfaces p3, p4 and p8. > +dnl toggle p1,p2 of bond0 up and down to test bonding in active-backup mode. > +OVS_VSWITCHD_START( > + [add-bond br0 bond0 p1 p2 bond_mode=active-backup --\ > + set interface p1 type=dummy options:pstream=punix:$OVS_RUNDIR/p1.sock ofport_request=1 -- \ > + set interface p2 type=dummy options:pstream=punix:$OVS_RUNDIR/p2.sock ofport_request=2 -- \ > + add-port br0 p7 -- set interface p7 ofport_request=7 type=dummy -- \ > + add-br br1 -- \ > + set bridge br1 other-config:hwaddr=aa:66:aa:66:00:00 -- \ > + set bridge br1 datapath-type=dummy other-config:datapath-id=1234 \ > + fail-mode=secure -- \ > + add-port br1 p3 -- set interface p3 type=dummy options:stream=unix:$OVS_RUNDIR/p1.sock ofport_request=3 -- \ > + add-port br1 p4 -- set interface p4 type=dummy options:stream=unix:$OVS_RUNDIR/p2.sock ofport_request=4 -- \ > + add-port br1 p8 -- set interface p8 ofport_request=8 type=dummy --]) > +AT_CHECK([ovs-appctl vlog/set dpif:dbg dpif_netdev:dbg]) > +WAIT_FOR_DUMMY_PORTS([p3], [p4]) > +OVS_WAIT_UNTIL([test -n "`ovs-appctl bond/show | grep 'active-backup primary: <none>'`"]) > + > AT_CHECK([ovs-ofctl add-flow br0 action=normal]) > AT_CHECK([ovs-ofctl add-flow br1 action=normal]) > ovs-appctl netdev-dummy/set-admin-state up > @@ -63,15 +260,14 @@ ovs-appctl netdev-dummy/set-admin-state p1 down > ovs-appctl time/warp 100 > AT_CHECK([ovs-appctl netdev-dummy/receive p7 'in_port(7),eth(src=50:54:00:00:00:09,dst=50:54:00:00:00:0d),eth_type(0x0800),ipv4(src=10.0.0.5,dst=10.0.0.1,proto=1,tos=0,ttl=64,frag=no),icmp(type=8,code=0)']) > AT_CHECK([ovs-appctl netdev-dummy/receive p7 'in_port(7),eth(src=50:54:00:00:00:09,dst=50:54:00:00:00:0e),eth_type(0x0800),ipv4(src=10.0.0.6,dst=10.0.0.1,proto=1,tos=0,ttl=64,frag=no),icmp(type=8,code=0)']) > -ovs-appctl time/warp 200 100 > -sleep 1 > -AT_CHECK([grep 'in_port([[348]])' ovs-vswitchd.log | filter_flow_install | strip_xout], [0], [dnl > -recirc_id(0),in_port(3),packet_type(ns=0,id=0),eth(src=50:54:00:00:00:09,dst=50:54:00:00:00:0a),eth_type(0x0800),ipv4(frag=no), actions: <del> > -recirc_id(0),in_port(3),packet_type(ns=0,id=0),eth(src=50:54:00:00:00:0b,dst=50:54:00:00:00:0c),eth_type(0x0800),ipv4(frag=no), actions: <del> > -recirc_id(0),in_port(4),packet_type(ns=0,id=0),eth(src=50:54:00:00:00:09,dst=50:54:00:00:00:0d),eth_type(0x0800),ipv4(frag=no), actions: <del> > -recirc_id(0),in_port(4),packet_type(ns=0,id=0),eth(src=50:54:00:00:00:09,dst=50:54:00:00:00:0e),eth_type(0x0800),ipv4(frag=no), actions: <del> > -recirc_id(0),in_port(4),packet_type(ns=0,id=0),eth(src=50:54:00:00:00:09,dst=ff:ff:ff:ff:ff:ff),eth_type(0x8035), actions: <del> > -recirc_id(0),in_port(4),packet_type(ns=0,id=0),eth(src=50:54:00:00:00:0b,dst=ff:ff:ff:ff:ff:ff),eth_type(0x8035), actions: <del> > +ovs-appctl time/warp 2000 100 > +AT_CHECK([ovs-appctl dpctl/dump-flows | grep 'in_port([[348]])' | strip_xout], [0], [dnl > +recirc_id(0),in_port(3),packet_type(ns=0,id=0),eth(src=50:54:00:00:00:09,dst=50:54:00:00:00:0a),eth_type(0x0800),ipv4(frag=no), packets:0, bytes:0, used:never, actions: <del> > +recirc_id(0),in_port(3),packet_type(ns=0,id=0),eth(src=50:54:00:00:00:0b,dst=50:54:00:00:00:0c),eth_type(0x0800),ipv4(frag=no), packets:0, bytes:0, used:never, actions: <del> > +recirc_id(0),in_port(4),packet_type(ns=0,id=0),eth(src=50:54:00:00:00:09,dst=50:54:00:00:00:0d),eth_type(0x0800),ipv4(frag=no), packets:0, bytes:0, used:never, actions: <del> > +recirc_id(0),in_port(4),packet_type(ns=0,id=0),eth(src=50:54:00:00:00:09,dst=50:54:00:00:00:0e),eth_type(0x0800),ipv4(frag=no), packets:0, bytes:0, used:never, actions: <del> > +recirc_id(0),in_port(4),packet_type(ns=0,id=0),eth(src=50:54:00:00:00:09,dst=ff:ff:ff:ff:ff:ff),eth_type(0x8035), packets:0, bytes:0, used:never, actions: <del> > +recirc_id(0),in_port(4),packet_type(ns=0,id=0),eth(src=50:54:00:00:00:0b,dst=ff:ff:ff:ff:ff:ff),eth_type(0x8035), packets:0, bytes:0, used:never, actions: <del> > ]) > OVS_VSWITCHD_STOP > AT_CLEANUP > diff --git a/vswitchd/bridge.c b/vswitchd/bridge.c > index f312efd8e..7111fbac3 100644 > --- a/vswitchd/bridge.c > +++ b/vswitchd/bridge.c > @@ -4563,6 +4563,11 @@ port_configure_bond(struct port *port, struct bond_settings *s) > port->name); > } > > + s->primary = NULL; > + if (s->balance == BM_AB || s->lacp_fallback_ab_cfg) { > + s->primary = smap_get(&port->cfg->other_config, "bond-primary"); > + } > + > miimon_interval = smap_get_int(&port->cfg->other_config, > "bond-miimon-interval", 0); > if (miimon_interval <= 0) { > diff --git a/vswitchd/vswitch.xml b/vswitchd/vswitch.xml > index b6acb34ca..944708ed6 100644 > --- a/vswitchd/vswitch.xml > +++ b/vswitchd/vswitch.xml > @@ -2003,6 +2003,14 @@ > This knob does not affect other balancing modes. > </column> > > + <column name="other_config" key="bond-primary" > + type='{"type": "string"}'> > + If a slave interface with this name exists in the bond and > + is up, it will be made active. Relevant only when <ref > + column="other_config" key="bond-mode"/> is > + <code>active-backup</code>. > + </column> > + > <group title="Link Failure Detection"> > <p> > An important part of link bonding is detecting that links are down so
On Jul 2, 2020, at 4:17 PM, Flavio Leitner <fbl@sysclose.org> wrote: > > > Hi Jeff, > > Thanks for following up. > I have a couple comments inline. > > > On Thu, Jun 25, 2020 at 02:03:34PM -0700, Jeff Squyres via dev wrote: >> In AB bonding, if the current active slave becomes disabled, a >> replacement slave is arbitrarily picked from the remaining set of >> enabled slaves. This commit adds the concept of a "primary" slave: an >> interface that will always be (or become) the current active slave if >> it is enabled. >> >> The rationale for this functionality is to allow the designation of a >> preferred interface for a given bond. For example: >> >> 1. Bond is created with interfaces p1 (primary) and p2, both enabled. >> 2. p1 becomes the current active slave (because it was designated as >> the primary). >> 3. Later, p1 fails/becomes disabled. >> 4. p2 is chosen to become the current active slave. >> 5. Later, p1 becomes re-enabled. >> 6. p1 is chosen to become the current active slave (because it was >> designated as the primary) >> >> Note that p1 becomes the active slave once it becomes re-enabled, even >> if nothing has happened to p2. >> >> This "primary" concept exists in Linux kernel network interface >> bonding, but did not previously exist in OVS bonding. >> >> Only one primary slave inteface is supported per bond, and is only >> supported for active/backup bonding. >> >> The primary slave interface is designated via >> "other_config:bond-primary" when creating a bond. >> >> Also, while adding tests for the "primary" concept, make a few small >> improvements to the non-primary AB bonding test. >> >> Signed-off-by: Jeff Squyres <jsquyres@cisco.com> >> Reviewed-by: Aaron Conole <aconole@redhat.com> >> Tested-by: Greg Rose <gvrose8192@gmail.com> >> Acked-by: Greg Rose <gvrose8192@gmail.com> >> --- >> v9: >> - Removed stale code that should have been removed in v8. >> - Fixed comment punctuation. >> >> v8: >> - Many style and simplification suggestions from Ilya Maximets >> - Rename "primary:" to "active-backup primary:" in ovs-appctl >> bond/show output, and make it always appear (vs. only appearing in >> explicit AB bonding scenarios). >> - Properly handle fallback-to-AB-bonding cases >> - Use time/warp and OVS_WAIT_UNTIL in the bonding tests to prevent >> races, and a few other improvements to the test quality >> - After rebasing patch to head of tree, adjust test output as result >> of changes from other commits. >> >> v7: >> - After rebasing patch to head of tree, adjust test output as result >> of changes from 81f71381f. >> >> v6: >> - Style: bleep bloop. Fix use of {}. >> >> v5: >> - Handle when bond is reconfigured to remove "bond-primary" config. >> - Fix potential flapping between remaining slaves. >> - Add a test to re-add a removed primary interface and make sure the >> bond reflects that properly. >> - Add a test to remove "bond-primary" config and make sure the bond >> reflects that properly. >> >> v4: >> - Style: bleep bloop. Trim line length below 79 characters. >> >> v3: >> - Adjusted print logic for when the primary slave is not attached >> >> v2: >> - Added "ovs-vsctl bond/show" label when primary interface is no >> longer enslaved >> - Fixed strcmp() usage nits >> - Moved "other_config:bond-primary" docs to the "Bonding >> Configuration" group >> - Expanded commit message >> - Added more test cases (including one for when the primary interface >> is no longer enslaved) >> >> ofproto/bond.c | 54 ++++++++-- >> ofproto/bond.h | 1 + >> tests/lacp.at | 9 ++ >> tests/ofproto-dpif.at | 224 +++++++++++++++++++++++++++++++++++++++--- >> vswitchd/bridge.c | 5 + >> vswitchd/vswitch.xml | 8 ++ >> 6 files changed, 280 insertions(+), 21 deletions(-) >> >> diff --git a/ofproto/bond.c b/ofproto/bond.c >> index 9947e7531..58ee67beb 100644 >> --- a/ofproto/bond.c >> +++ b/ofproto/bond.c >> @@ -89,6 +89,7 @@ struct bond_slave { >> /* Link status. */ >> bool enabled; /* May be chosen for flows? */ >> bool may_enable; /* Client considers this slave bondable. */ >> + bool is_primary; /* This slave is preferred over others. */ >> long long delay_expires; /* Time after which 'enabled' may change. */ >> >> /* Rebalancing info. Used only by bond_rebalance(). */ >> @@ -124,6 +125,7 @@ struct bond { >> uint32_t basis; /* Basis for flow hash function. */ >> bool use_lb_output_action; /* Use lb_output action to avoid recirculation. >> Applicable only for Balance TCP mode. */ >> + char *primary; /* Name of the primary slave interface. */ >> >> /* SLB specific bonding info. */ >> struct bond_entry *hash; /* An array of BOND_BUCKETS elements. */ >> @@ -241,6 +243,7 @@ bond_create(const struct bond_settings *s, struct ofproto_dpif *ofproto) >> >> bond->active_slave_mac = eth_addr_zero; >> bond->active_slave_changed = false; >> + bond->primary = NULL; >> >> bond_reconfigure(bond, s); >> return bond; >> @@ -294,6 +297,7 @@ bond_unref(struct bond *bond) >> update_recirc_rules__(bond); >> >> hmap_destroy(&bond->pr_rule_ops); >> + free(bond->primary); >> free(bond->name); >> free(bond); >> } >> @@ -471,6 +475,12 @@ bond_reconfigure(struct bond *bond, const struct bond_settings *s) >> bond->bond_revalidate = false; >> } >> >> + if (!nullable_string_is_equal(bond->primary, s->primary)) { >> + free(bond->primary); >> + bond->primary = nullable_xstrdup(s->primary); >> + revalidate = true; >> + } >> + >> if (bond->balance != BM_AB) { >> if (!bond->recirc_id) { >> bond->recirc_id = recirc_alloc_id(bond->ofproto); >> @@ -575,6 +585,12 @@ bond_slave_register(struct bond *bond, void *slave_, >> slave->name = xstrdup(netdev_get_name(netdev)); >> bond->bond_revalidate = true; >> >> + if (bond->primary && !strcmp(bond->primary, slave->name)) { >> + slave->is_primary = true; >> + } else { >> + slave->is_primary = false; >> + } >> + >> slave->enabled = false; >> bond_enable_slave(slave, netdev_get_carrier(netdev)); >> } >> @@ -668,7 +684,7 @@ bond_slave_set_may_enable(struct bond *bond, void *slave_, bool may_enable) >> bool >> bond_run(struct bond *bond, enum lacp_status lacp_status) >> { >> - struct bond_slave *slave; >> + struct bond_slave *slave, *primary; >> bool revalidate; >> >> ovs_rwlock_wrlock(&rwlock); >> @@ -685,11 +701,19 @@ bond_run(struct bond *bond, enum lacp_status lacp_status) >> } >> >> /* Enable slaves based on link status and LACP feedback. */ >> + primary = NULL; >> HMAP_FOR_EACH (slave, hmap_node, &bond->slaves) { >> bond_link_status_update(slave); >> slave->change_seq = seq_read(connectivity_seq_get()); >> + >> + /* Discover if there is an active slave marked 'primary'. */ >> + if (bond->balance == BM_AB && slave->is_primary && slave->enabled) { >> + primary = slave; >> + } >> } >> - if (!bond->active_slave || !bond->active_slave->enabled) { >> + >> + if (!bond->active_slave || !bond->active_slave->enabled || >> + (primary && bond->active_slave != primary)) { >> bond_choose_active_slave(bond); >> } >> >> @@ -1437,16 +1461,25 @@ bond_print_details(struct ds *ds, const struct bond *bond) >> ds_put_format(ds, "lacp_fallback_ab: %s\n", >> bond->lacp_fallback_ab ? "true" : "false"); >> >> - ds_put_cstr(ds, "active slave mac: "); >> - ds_put_format(ds, ETH_ADDR_FMT, ETH_ADDR_ARGS(bond->active_slave_mac)); >> + bool found_primary = false; >> slave = bond_find_slave_by_mac(bond, bond->active_slave_mac); > ^^^^^^ > > >> - ds_put_format(ds,"(%s)\n", slave ? slave->name : "none"); >> - >> HMAP_FOR_EACH (slave, hmap_node, &bond->slaves) { > ^^^^^^ > So, the active slave is lost here. > > >> + if (slave->is_primary) { >> + found_primary = true; >> + } >> shash_add(&slave_shash, slave->name, slave); >> } >> - sorted_slaves = shash_sort(&slave_shash); >> >> + ds_put_format(ds, "active-backup primary: %s%s\n", >> + bond->primary ? bond->primary : "<none>", >> + (!found_primary && bond->primary) >> + ? " (no such slave)" : ""); > > Perhaps it should here: > slave = bond_find_slave_by_mac(bond, bond->active_slave_mac); Yoinks; yes. This happened in v8 when some code shuffled around in this area. Thanks -- fixed. >> + >> + ds_put_cstr(ds, "active slave mac: "); >> + ds_put_format(ds, ETH_ADDR_FMT, ETH_ADDR_ARGS(bond->active_slave_mac)); >> + ds_put_format(ds,"(%s)\n", slave ? slave->name : "none"); >> + >> + sorted_slaves = shash_sort(&slave_shash); >> for (i = 0; i < shash_count(&slave_shash); i++) { >> struct bond_entry *be; >> >> @@ -1906,6 +1939,13 @@ bond_choose_slave(const struct bond *bond) >> { >> struct bond_slave *slave, *best; >> >> + /* If there's a primary and it's active, return that. */ >> + HMAP_FOR_EACH (slave, hmap_node, &bond->slaves) { >> + if (slave->is_primary && slave->enabled) { >> + return slave; >> + } >> + } >> + >> /* Find the last active slave. */ >> slave = bond_find_slave_by_mac(bond, bond->active_slave_mac); >> if (slave && slave->enabled) { >> diff --git a/ofproto/bond.h b/ofproto/bond.h >> index 40c3258dc..1a42a37c9 100644 >> --- a/ofproto/bond.h >> +++ b/ofproto/bond.h >> @@ -45,6 +45,7 @@ struct bond_settings { >> >> /* Balancing configuration. */ >> enum bond_mode balance; >> + const char *primary; /* For AB balance, primary interface name. */ >> int rebalance_interval; /* Milliseconds between rebalances. >> Zero to disable rebalancing. */ >> >> diff --git a/tests/lacp.at b/tests/lacp.at >> index df1691731..5257f0cce 100644 >> --- a/tests/lacp.at >> +++ b/tests/lacp.at >> @@ -126,6 +126,7 @@ updelay: 0 ms >> downdelay: 0 ms >> lacp_status: negotiated >> lacp_fallback_ab: false >> +active-backup primary: <none> >> active slave mac: 00:00:00:00:00:00(none) >> >> slave p1: disabled >> @@ -292,6 +293,7 @@ updelay: 0 ms >> downdelay: 0 ms >> lacp_status: negotiated >> lacp_fallback_ab: false >> +active-backup primary: <none> >> >> slave p0: enabled >> may_enable: true >> @@ -308,6 +310,7 @@ updelay: 0 ms >> downdelay: 0 ms >> lacp_status: negotiated >> lacp_fallback_ab: false >> +active-backup primary: <none> >> >> slave p2: enabled >> may_enable: true >> @@ -431,6 +434,7 @@ updelay: 0 ms >> downdelay: 0 ms >> lacp_status: negotiated >> lacp_fallback_ab: false >> +active-backup primary: <none> >> <active slave mac del> >> >> slave p0: disabled >> @@ -449,6 +453,7 @@ updelay: 0 ms >> downdelay: 0 ms >> lacp_status: negotiated >> lacp_fallback_ab: false >> +active-backup primary: <none> >> <active slave mac del> >> >> slave p2: disabled >> @@ -565,6 +570,7 @@ updelay: 0 ms >> downdelay: 0 ms >> lacp_status: negotiated >> lacp_fallback_ab: false >> +active-backup primary: <none> >> <active slave mac del> >> >> slave p0: disabled >> @@ -583,6 +589,7 @@ updelay: 0 ms >> downdelay: 0 ms >> lacp_status: negotiated >> lacp_fallback_ab: false >> +active-backup primary: <none> >> <active slave mac del> >> >> slave p2: disabled >> @@ -704,6 +711,7 @@ updelay: 0 ms >> downdelay: 0 ms >> lacp_status: negotiated >> lacp_fallback_ab: false >> +active-backup primary: <none> >> <active slave mac del> >> >> slave p0: enabled >> @@ -722,6 +730,7 @@ updelay: 0 ms >> downdelay: 0 ms >> lacp_status: negotiated >> lacp_fallback_ab: false >> +active-backup primary: <none> >> <active slave mac del> >> >> slave p2: enabled >> diff --git a/tests/ofproto-dpif.at b/tests/ofproto-dpif.at >> index e3402e7b8..808200800 100644 >> --- a/tests/ofproto-dpif.at >> +++ b/tests/ofproto-dpif.at >> @@ -29,12 +29,16 @@ AT_CHECK([ovs-appctl revalidator/wait]) >> OVS_VSWITCHD_STOP >> AT_CLEANUP >> >> -AT_SETUP([ofproto-dpif - active-backup bonding]) >> -# Create br0 with interfaces p1, p2 and p7, creating bond0 with p1 and p2 >> -# and br1 with interfaces p3, p4 and p8. >> -# toggle p1,p2 of bond0 up and down to test bonding in active-backup mode. >> +AT_SETUP([ofproto-dpif - active-backup bonding (with primary)]) >> + >> +dnl Create br0 with interfaces p1, p2 and p7, creating bond0 with p1 and >> +dnl p2 (p1 as primary) and br1 with interfaces p3, p4 and p8. >> +dnl toggle p1,p2 of bond0 up and down to test bonding in active-backup mode. >> +dnl With p1 down and p2 up/active, bring p1 back up. Since p1 is the primary, >> +dnl it should become active. >> OVS_VSWITCHD_START( >> - [add-bond br0 bond0 p1 p2 bond_mode=active-backup --\ >> + [add-bond br0 bond0 p1 p2 bond_mode=active-backup \ >> + other_config:bond-primary=p1 -- \ >> set interface p1 type=dummy options:pstream=punix:$OVS_RUNDIR/p1.sock ofport_request=1 -- \ >> set interface p2 type=dummy options:pstream=punix:$OVS_RUNDIR/p2.sock ofport_request=2 -- \ >> add-port br0 p7 -- set interface p7 ofport_request=7 type=dummy -- \ >> @@ -45,9 +49,202 @@ OVS_VSWITCHD_START( >> add-port br1 p3 -- set interface p3 type=dummy options:stream=unix:$OVS_RUNDIR/p1.sock ofport_request=3 -- \ >> add-port br1 p4 -- set interface p4 type=dummy options:stream=unix:$OVS_RUNDIR/p2.sock ofport_request=4 -- \ >> add-port br1 p8 -- set interface p8 ofport_request=8 type=dummy --]) >> +AT_CHECK([ovs-appctl vlog/set dpif:dbg dpif_netdev:dbg]) >> WAIT_FOR_DUMMY_PORTS([p3], [p4]) >> +OVS_WAIT_UNTIL([test -n "`ovs-appctl bond/show | grep 'active-backup primary: p1'`"]) >> + >> + >> +AT_CHECK([ovs-ofctl add-flow br0 action=normal]) >> +AT_CHECK([ovs-ofctl add-flow br1 action=normal]) >> +ovs-appctl netdev-dummy/set-admin-state up > > Missing the interface name? This is from the original bonding test that I copied. When no interface name is specified, the command still runs successfully and reports "OK". "ovs-appctl list-commands" shows "netdev-dummy/set-admin-state [netdev] up|down", which implies that the device name is optional. It looks like not specifying the interface name brings up *all* the interfaces. From the context, I think that is what was desired...? >> +ovs-appctl time/warp 100 >> +ovs-appctl netdev-dummy/set-admin-state p2 down >> +ovs-appctl time/stop >> +ovs-appctl time/warp 100 >> +AT_CHECK([ovs-appctl netdev-dummy/receive p7 'in_port(7),eth(src=50:54:00:00:00:09,dst=50:54:00:00:00:0a),eth_type(0x0800),ipv4(src=10.0.0.2,dst=10.0.0.1,proto=1,tos=0,ttl=64,frag=no),icmp(type=8,code=0)']) >> +AT_CHECK([ovs-appctl netdev-dummy/receive p7 'in_port(7),eth(src=50:54:00:00:00:0b,dst=50:54:00:00:00:0c),eth_type(0x0800),ipv4(src=10.0.0.3,dst=10.0.0.4,proto=1,tos=0,ttl=64,frag=no),icmp(type=8,code=0)']) >> +ovs-appctl time/warp 100 >> +ovs-appctl netdev-dummy/set-admin-state p2 up >> +ovs-appctl netdev-dummy/set-admin-state p1 down >> +ovs-appctl time/warp 100 >> +AT_CHECK([ovs-appctl netdev-dummy/receive p7 'in_port(7),eth(src=50:54:00:00:00:09,dst=50:54:00:00:00:0d),eth_type(0x0800),ipv4(src=10.0.0.5,dst=10.0.0.1,proto=1,tos=0,ttl=64,frag=no),icmp(type=8,code=0)']) >> +AT_CHECK([ovs-appctl netdev-dummy/receive p7 'in_port(7),eth(src=50:54:00:00:00:09,dst=50:54:00:00:00:0e),eth_type(0x0800),ipv4(src=10.0.0.6,dst=10.0.0.1,proto=1,tos=0,ttl=64,frag=no),icmp(type=8,code=0)']) >> +ovs-appctl time/warp 2000 100 >> +AT_CHECK([ovs-appctl dpctl/dump-flows | grep 'in_port([[348]])' | strip_xout], [0], [dnl >> +recirc_id(0),in_port(3),packet_type(ns=0,id=0),eth(src=50:54:00:00:00:09,dst=50:54:00:00:00:0a),eth_type(0x0800),ipv4(frag=no), packets:0, bytes:0, used:never, actions: <del> >> +recirc_id(0),in_port(3),packet_type(ns=0,id=0),eth(src=50:54:00:00:00:0b,dst=50:54:00:00:00:0c),eth_type(0x0800),ipv4(frag=no), packets:0, bytes:0, used:never, actions: <del> >> +recirc_id(0),in_port(4),packet_type(ns=0,id=0),eth(src=50:54:00:00:00:09,dst=50:54:00:00:00:0d),eth_type(0x0800),ipv4(frag=no), packets:0, bytes:0, used:never, actions: <del> >> +recirc_id(0),in_port(4),packet_type(ns=0,id=0),eth(src=50:54:00:00:00:09,dst=50:54:00:00:00:0e),eth_type(0x0800),ipv4(frag=no), packets:0, bytes:0, used:never, actions: <del> >> +recirc_id(0),in_port(4),packet_type(ns=0,id=0),eth(src=50:54:00:00:00:09,dst=ff:ff:ff:ff:ff:ff),eth_type(0x8035), packets:0, bytes:0, used:never, actions: <del> >> +recirc_id(0),in_port(4),packet_type(ns=0,id=0),eth(src=50:54:00:00:00:0b,dst=ff:ff:ff:ff:ff:ff),eth_type(0x8035), packets:0, bytes:0, used:never, actions: <del> >> +]) >> + >> +ovs-appctl netdev-dummy/set-admin-state p1 up >> +ovs-appctl time/warp 100 >> +OVS_WAIT_UNTIL([ovs-appctl bond/show | STRIP_RECIRC_ID | STRIP_ACTIVE_SLAVE_MAC], [0], [dnl >> +---- bond0 ---- >> +bond_mode: active-backup >> +bond may use recirculation: no, <del> >> +bond-hash-basis: 0 >> +updelay: 0 ms >> +downdelay: 0 ms >> +lacp_status: off >> +lacp_fallback_ab: false >> +active-backup primary: p1 >> +<active slave mac del> >> + >> +slave p1: enabled >> + active slave >> + may_enable: true >> + >> +slave p2: enabled >> + may_enable: true >> + >> +]) >> + >> +OVS_VSWITCHD_STOP >> +AT_CLEANUP >> + >> +AT_SETUP([ofproto-dpif - active-backup bonding (primary validation)]) >> +dnl Make a switch with 3 ports in a bond, so that when we delete one of >> +dnl the ports from the bond, there are still 2 ports left and the bond >> +dnl remains functional. >> +OVS_VSWITCHD_START( >> + [add-bond br0 bond0 p1 p2 p3 bond_mode=active-backup \ >> + other_config:bond-primary=p1 -- \ >> + set interface p1 type=dummy options:pstream=punix:$OVS_RUNDIR/p1.sock ofport_request=1 -- \ >> + set interface p2 type=dummy options:pstream=punix:$OVS_RUNDIR/p2.sock ofport_request=2 -- \ >> + set interface p3 type=dummy options:pstream=punix:$OVS_RUNDIR/p3.sock ofport_request=3 -- \ >> + add-port br0 p7 -- set interface p7 ofport_request=7 type=dummy --]) >> AT_CHECK([ovs-appctl vlog/set dpif:dbg dpif_netdev:dbg]) >> >> +dnl Make sure the initial primary interface is set >> +OVS_WAIT_UNTIL([test -n "`ovs-appctl bond/show | grep 'active-backup primary: p1'`"]) >> + >> +dnl Down the primary interface and verify that we switched. Then >> +dnl bring the primary back and verify that we switched back to the >> +dnl primary. >> +ovs-appctl netdev-dummy/set-admin-state p1 down >> +ovs-appctl time/warp 100 >> +OVS_WAIT_UNTIL([test -n "`ovs-appctl bond/show | fgrep 'slave p1: disabled'`"]) >> +ovs-appctl netdev-dummy/set-admin-state p1 up >> +ovs-appctl time/warp 100 >> +OVS_WAIT_UNTIL([ovs-appctl bond/show | STRIP_RECIRC_ID | STRIP_ACTIVE_SLAVE_MAC], [0], [dnl >> +---- bond0 ---- >> +bond_mode: active-backup >> +bond may use recirculation: no, <del> >> +bond-hash-basis: 0 >> +updelay: 0 ms >> +downdelay: 0 ms >> +lacp_status: off >> +lacp_fallback_ab: false >> +active-backup primary: p1 >> +<active slave mac del> >> + >> +slave p1: enabled >> + active slave >> + may_enable: true >> + >> +slave p2: enabled >> + may_enable: true >> + >> +slave p3: enabled >> + may_enable: true >> + >> +]) >> + >> +dnl Now delete the primary and verify that the output shows that the >> +dnl primary is no longer enslaved >> +ovs-vsctl --id=@p1 get Interface p1 -- remove Port bond0 interfaces @p1 >> +ovs-appctl time/warp 100 >> +OVS_WAIT_UNTIL([test -n "`ovs-appctl bond/show | fgrep 'active-backup primary: p1 (no such slave)'`"]) >> + >> +dnl Now re-add the primary and verify that the output shows that the >> +dnl primary is available again. >> +dnl >> +dnl First, get the UUIDs of the interfaces that exist on bond0. >> +dnl Strip the trailing ] so that we can add a new UUID to the end. >> +uuids=`ovs-vsctl get Port bond0 interfaces | sed -e 's/]//'` >> +dnl Create a new port "p1" and add its UUID to the set of interfaces >> +dnl on bond0. >> +ovs-vsctl \ >> + --id=@p1 create Interface name=p1 type=dummy options:pstream=punix:$OVS_RUNDIR/p1.sock ofport_request=1 -- \ >> + set Port bond0 interfaces="$uuids, @p1]" >> +ovs-appctl time/warp 100 >> +OVS_WAIT_UNTIL([ovs-appctl bond/show | STRIP_RECIRC_ID | STRIP_ACTIVE_SLAVE_MAC], [0], [dnl >> +---- bond0 ---- >> +bond_mode: active-backup >> +bond may use recirculation: no, <del> >> +bond-hash-basis: 0 >> +updelay: 0 ms >> +downdelay: 0 ms >> +lacp_status: off >> +lacp_fallback_ab: false >> +active-backup primary: p1 >> +<active slave mac del> >> + >> +slave p1: enabled >> + active slave >> + may_enable: true >> + >> +slave p2: enabled >> + may_enable: true >> + >> +slave p3: enabled >> + may_enable: true >> + >> +]) >> + >> +dnl Remove the "bond-primary" config directive from the bond. >> +AT_CHECK([ovs-vsctl remove Port bond0 other_config bond-primary]) >> +ovs-appctl time/warp 100 >> +OVS_WAIT_UNTIL([ovs-appctl bond/show | STRIP_RECIRC_ID | STRIP_ACTIVE_SLAVE_MAC], [0], [dnl >> +---- bond0 ---- >> +bond_mode: active-backup >> +bond may use recirculation: no, <del> >> +bond-hash-basis: 0 >> +updelay: 0 ms >> +downdelay: 0 ms >> +lacp_status: off >> +lacp_fallback_ab: false >> +active-backup primary: <none> >> +<active slave mac del> >> + >> +slave p1: enabled >> + active slave >> + may_enable: true >> + >> +slave p2: enabled >> + may_enable: true >> + >> +slave p3: enabled >> + may_enable: true >> + >> +]) >> + >> +OVS_VSWITCHD_STOP >> +AT_CLEANUP >> + >> +AT_SETUP([ofproto-dpif - active-backup bonding (without primary)]) >> +dnl Create br0 with interfaces p1, p2 and p7, creating bond0 with p1 and p2 >> +dnl and br1 with interfaces p3, p4 and p8. >> +dnl toggle p1,p2 of bond0 up and down to test bonding in active-backup mode. >> +OVS_VSWITCHD_START( >> + [add-bond br0 bond0 p1 p2 bond_mode=active-backup --\ >> + set interface p1 type=dummy options:pstream=punix:$OVS_RUNDIR/p1.sock ofport_request=1 -- \ >> + set interface p2 type=dummy options:pstream=punix:$OVS_RUNDIR/p2.sock ofport_request=2 -- \ >> + add-port br0 p7 -- set interface p7 ofport_request=7 type=dummy -- \ >> + add-br br1 -- \ >> + set bridge br1 other-config:hwaddr=aa:66:aa:66:00:00 -- \ >> + set bridge br1 datapath-type=dummy other-config:datapath-id=1234 \ >> + fail-mode=secure -- \ >> + add-port br1 p3 -- set interface p3 type=dummy options:stream=unix:$OVS_RUNDIR/p1.sock ofport_request=3 -- \ >> + add-port br1 p4 -- set interface p4 type=dummy options:stream=unix:$OVS_RUNDIR/p2.sock ofport_request=4 -- \ >> + add-port br1 p8 -- set interface p8 ofport_request=8 type=dummy --]) >> +AT_CHECK([ovs-appctl vlog/set dpif:dbg dpif_netdev:dbg]) >> +WAIT_FOR_DUMMY_PORTS([p3], [p4]) >> +OVS_WAIT_UNTIL([test -n "`ovs-appctl bond/show | grep 'active-backup primary: <none>'`"]) >> + >> AT_CHECK([ovs-ofctl add-flow br0 action=normal]) >> AT_CHECK([ovs-ofctl add-flow br1 action=normal]) >> ovs-appctl netdev-dummy/set-admin-state up >> @@ -63,15 +260,14 @@ ovs-appctl netdev-dummy/set-admin-state p1 down >> ovs-appctl time/warp 100 >> AT_CHECK([ovs-appctl netdev-dummy/receive p7 'in_port(7),eth(src=50:54:00:00:00:09,dst=50:54:00:00:00:0d),eth_type(0x0800),ipv4(src=10.0.0.5,dst=10.0.0.1,proto=1,tos=0,ttl=64,frag=no),icmp(type=8,code=0)']) >> AT_CHECK([ovs-appctl netdev-dummy/receive p7 'in_port(7),eth(src=50:54:00:00:00:09,dst=50:54:00:00:00:0e),eth_type(0x0800),ipv4(src=10.0.0.6,dst=10.0.0.1,proto=1,tos=0,ttl=64,frag=no),icmp(type=8,code=0)']) >> -ovs-appctl time/warp 200 100 >> -sleep 1 >> -AT_CHECK([grep 'in_port([[348]])' ovs-vswitchd.log | filter_flow_install | strip_xout], [0], [dnl >> -recirc_id(0),in_port(3),packet_type(ns=0,id=0),eth(src=50:54:00:00:00:09,dst=50:54:00:00:00:0a),eth_type(0x0800),ipv4(frag=no), actions: <del> >> -recirc_id(0),in_port(3),packet_type(ns=0,id=0),eth(src=50:54:00:00:00:0b,dst=50:54:00:00:00:0c),eth_type(0x0800),ipv4(frag=no), actions: <del> >> -recirc_id(0),in_port(4),packet_type(ns=0,id=0),eth(src=50:54:00:00:00:09,dst=50:54:00:00:00:0d),eth_type(0x0800),ipv4(frag=no), actions: <del> >> -recirc_id(0),in_port(4),packet_type(ns=0,id=0),eth(src=50:54:00:00:00:09,dst=50:54:00:00:00:0e),eth_type(0x0800),ipv4(frag=no), actions: <del> >> -recirc_id(0),in_port(4),packet_type(ns=0,id=0),eth(src=50:54:00:00:00:09,dst=ff:ff:ff:ff:ff:ff),eth_type(0x8035), actions: <del> >> -recirc_id(0),in_port(4),packet_type(ns=0,id=0),eth(src=50:54:00:00:00:0b,dst=ff:ff:ff:ff:ff:ff),eth_type(0x8035), actions: <del> >> +ovs-appctl time/warp 2000 100 >> +AT_CHECK([ovs-appctl dpctl/dump-flows | grep 'in_port([[348]])' | strip_xout], [0], [dnl >> +recirc_id(0),in_port(3),packet_type(ns=0,id=0),eth(src=50:54:00:00:00:09,dst=50:54:00:00:00:0a),eth_type(0x0800),ipv4(frag=no), packets:0, bytes:0, used:never, actions: <del> >> +recirc_id(0),in_port(3),packet_type(ns=0,id=0),eth(src=50:54:00:00:00:0b,dst=50:54:00:00:00:0c),eth_type(0x0800),ipv4(frag=no), packets:0, bytes:0, used:never, actions: <del> >> +recirc_id(0),in_port(4),packet_type(ns=0,id=0),eth(src=50:54:00:00:00:09,dst=50:54:00:00:00:0d),eth_type(0x0800),ipv4(frag=no), packets:0, bytes:0, used:never, actions: <del> >> +recirc_id(0),in_port(4),packet_type(ns=0,id=0),eth(src=50:54:00:00:00:09,dst=50:54:00:00:00:0e),eth_type(0x0800),ipv4(frag=no), packets:0, bytes:0, used:never, actions: <del> >> +recirc_id(0),in_port(4),packet_type(ns=0,id=0),eth(src=50:54:00:00:00:09,dst=ff:ff:ff:ff:ff:ff),eth_type(0x8035), packets:0, bytes:0, used:never, actions: <del> >> +recirc_id(0),in_port(4),packet_type(ns=0,id=0),eth(src=50:54:00:00:00:0b,dst=ff:ff:ff:ff:ff:ff),eth_type(0x8035), packets:0, bytes:0, used:never, actions: <del> >> ]) >> OVS_VSWITCHD_STOP >> AT_CLEANUP >> diff --git a/vswitchd/bridge.c b/vswitchd/bridge.c >> index f312efd8e..7111fbac3 100644 >> --- a/vswitchd/bridge.c >> +++ b/vswitchd/bridge.c >> @@ -4563,6 +4563,11 @@ port_configure_bond(struct port *port, struct bond_settings *s) >> port->name); >> } >> >> + s->primary = NULL; >> + if (s->balance == BM_AB || s->lacp_fallback_ab_cfg) { >> + s->primary = smap_get(&port->cfg->other_config, "bond-primary"); >> + } >> + >> miimon_interval = smap_get_int(&port->cfg->other_config, >> "bond-miimon-interval", 0); >> if (miimon_interval <= 0) { >> diff --git a/vswitchd/vswitch.xml b/vswitchd/vswitch.xml >> index b6acb34ca..944708ed6 100644 >> --- a/vswitchd/vswitch.xml >> +++ b/vswitchd/vswitch.xml >> @@ -2003,6 +2003,14 @@ >> This knob does not affect other balancing modes. >> </column> >> >> + <column name="other_config" key="bond-primary" >> + type='{"type": "string"}'> >> + If a slave interface with this name exists in the bond and >> + is up, it will be made active. Relevant only when <ref >> + column="other_config" key="bond-mode"/> is > > I think it is bond_mode. Fixed! v10 coming shortly. > >> + <code>active-backup</code>. >> + </column> >> + >> <group title="Link Failure Detection"> >> <p> >> An important part of link bonding is detecting that links are down so >> -- >> 2.26.2 >> >> _______________________________________________ >> dev mailing list >> dev@openvswitch.org >> https://mail.openvswitch.org/mailman/listinfo/ovs-dev > > -- > fbl
Hi Jeff, On Tue, Jul 07, 2020 at 09:41:00PM +0000, Jeff Squyres (jsquyres) wrote: > On Jul 2, 2020, at 4:17 PM, Flavio Leitner <fbl@sysclose.org> wrote: > > On Thu, Jun 25, 2020 at 02:03:34PM -0700, Jeff Squyres via dev wrote: [...] > >> diff --git a/tests/ofproto-dpif.at b/tests/ofproto-dpif.at > >> index e3402e7b8..808200800 100644 > >> --- a/tests/ofproto-dpif.at > >> +++ b/tests/ofproto-dpif.at > >> @@ -29,12 +29,16 @@ AT_CHECK([ovs-appctl revalidator/wait]) > >> OVS_VSWITCHD_STOP > >> AT_CLEANUP > >> > >> -AT_SETUP([ofproto-dpif - active-backup bonding]) > >> -# Create br0 with interfaces p1, p2 and p7, creating bond0 with p1 and p2 > >> -# and br1 with interfaces p3, p4 and p8. > >> -# toggle p1,p2 of bond0 up and down to test bonding in active-backup mode. > >> +AT_SETUP([ofproto-dpif - active-backup bonding (with primary)]) > >> + > >> +dnl Create br0 with interfaces p1, p2 and p7, creating bond0 with p1 and > >> +dnl p2 (p1 as primary) and br1 with interfaces p3, p4 and p8. > >> +dnl toggle p1,p2 of bond0 up and down to test bonding in active-backup mode. > >> +dnl With p1 down and p2 up/active, bring p1 back up. Since p1 is the primary, > >> +dnl it should become active. > >> OVS_VSWITCHD_START( > >> - [add-bond br0 bond0 p1 p2 bond_mode=active-backup --\ > >> + [add-bond br0 bond0 p1 p2 bond_mode=active-backup \ > >> + other_config:bond-primary=p1 -- \ > >> set interface p1 type=dummy options:pstream=punix:$OVS_RUNDIR/p1.sock ofport_request=1 -- \ > >> set interface p2 type=dummy options:pstream=punix:$OVS_RUNDIR/p2.sock ofport_request=2 -- \ > >> add-port br0 p7 -- set interface p7 ofport_request=7 type=dummy -- \ > >> @@ -45,9 +49,202 @@ OVS_VSWITCHD_START( > >> add-port br1 p3 -- set interface p3 type=dummy options:stream=unix:$OVS_RUNDIR/p1.sock ofport_request=3 -- \ > >> add-port br1 p4 -- set interface p4 type=dummy options:stream=unix:$OVS_RUNDIR/p2.sock ofport_request=4 -- \ > >> add-port br1 p8 -- set interface p8 ofport_request=8 type=dummy --]) > >> +AT_CHECK([ovs-appctl vlog/set dpif:dbg dpif_netdev:dbg]) > >> WAIT_FOR_DUMMY_PORTS([p3], [p4]) > >> +OVS_WAIT_UNTIL([test -n "`ovs-appctl bond/show | grep 'active-backup primary: p1'`"]) > >> + > >> + > >> +AT_CHECK([ovs-ofctl add-flow br0 action=normal]) > >> +AT_CHECK([ovs-ofctl add-flow br1 action=normal]) > >> +ovs-appctl netdev-dummy/set-admin-state up > > > > Missing the interface name? > > This is from the original bonding test that I copied. > > When no interface name is specified, the command still runs successfully and reports "OK". > > "ovs-appctl list-commands" shows "netdev-dummy/set-admin-state [netdev] up|down", which implies that the device name is optional. > > It looks like not specifying the interface name brings up *all* the interfaces. From the context, I think that is what was desired...? Right, I forgot about that. Thanks,
diff --git a/ofproto/bond.c b/ofproto/bond.c index 9947e7531..58ee67beb 100644 --- a/ofproto/bond.c +++ b/ofproto/bond.c @@ -89,6 +89,7 @@ struct bond_slave { /* Link status. */ bool enabled; /* May be chosen for flows? */ bool may_enable; /* Client considers this slave bondable. */ + bool is_primary; /* This slave is preferred over others. */ long long delay_expires; /* Time after which 'enabled' may change. */ /* Rebalancing info. Used only by bond_rebalance(). */ @@ -124,6 +125,7 @@ struct bond { uint32_t basis; /* Basis for flow hash function. */ bool use_lb_output_action; /* Use lb_output action to avoid recirculation. Applicable only for Balance TCP mode. */ + char *primary; /* Name of the primary slave interface. */ /* SLB specific bonding info. */ struct bond_entry *hash; /* An array of BOND_BUCKETS elements. */ @@ -241,6 +243,7 @@ bond_create(const struct bond_settings *s, struct ofproto_dpif *ofproto) bond->active_slave_mac = eth_addr_zero; bond->active_slave_changed = false; + bond->primary = NULL; bond_reconfigure(bond, s); return bond; @@ -294,6 +297,7 @@ bond_unref(struct bond *bond) update_recirc_rules__(bond); hmap_destroy(&bond->pr_rule_ops); + free(bond->primary); free(bond->name); free(bond); } @@ -471,6 +475,12 @@ bond_reconfigure(struct bond *bond, const struct bond_settings *s) bond->bond_revalidate = false; } + if (!nullable_string_is_equal(bond->primary, s->primary)) { + free(bond->primary); + bond->primary = nullable_xstrdup(s->primary); + revalidate = true; + } + if (bond->balance != BM_AB) { if (!bond->recirc_id) { bond->recirc_id = recirc_alloc_id(bond->ofproto); @@ -575,6 +585,12 @@ bond_slave_register(struct bond *bond, void *slave_, slave->name = xstrdup(netdev_get_name(netdev)); bond->bond_revalidate = true; + if (bond->primary && !strcmp(bond->primary, slave->name)) { + slave->is_primary = true; + } else { + slave->is_primary = false; + } + slave->enabled = false; bond_enable_slave(slave, netdev_get_carrier(netdev)); } @@ -668,7 +684,7 @@ bond_slave_set_may_enable(struct bond *bond, void *slave_, bool may_enable) bool bond_run(struct bond *bond, enum lacp_status lacp_status) { - struct bond_slave *slave; + struct bond_slave *slave, *primary; bool revalidate; ovs_rwlock_wrlock(&rwlock); @@ -685,11 +701,19 @@ bond_run(struct bond *bond, enum lacp_status lacp_status) } /* Enable slaves based on link status and LACP feedback. */ + primary = NULL; HMAP_FOR_EACH (slave, hmap_node, &bond->slaves) { bond_link_status_update(slave); slave->change_seq = seq_read(connectivity_seq_get()); + + /* Discover if there is an active slave marked 'primary'. */ + if (bond->balance == BM_AB && slave->is_primary && slave->enabled) { + primary = slave; + } } - if (!bond->active_slave || !bond->active_slave->enabled) { + + if (!bond->active_slave || !bond->active_slave->enabled || + (primary && bond->active_slave != primary)) { bond_choose_active_slave(bond); } @@ -1437,16 +1461,25 @@ bond_print_details(struct ds *ds, const struct bond *bond) ds_put_format(ds, "lacp_fallback_ab: %s\n", bond->lacp_fallback_ab ? "true" : "false"); - ds_put_cstr(ds, "active slave mac: "); - ds_put_format(ds, ETH_ADDR_FMT, ETH_ADDR_ARGS(bond->active_slave_mac)); + bool found_primary = false; slave = bond_find_slave_by_mac(bond, bond->active_slave_mac); - ds_put_format(ds,"(%s)\n", slave ? slave->name : "none"); - HMAP_FOR_EACH (slave, hmap_node, &bond->slaves) { + if (slave->is_primary) { + found_primary = true; + } shash_add(&slave_shash, slave->name, slave); } - sorted_slaves = shash_sort(&slave_shash); + ds_put_format(ds, "active-backup primary: %s%s\n", + bond->primary ? bond->primary : "<none>", + (!found_primary && bond->primary) + ? " (no such slave)" : ""); + + ds_put_cstr(ds, "active slave mac: "); + ds_put_format(ds, ETH_ADDR_FMT, ETH_ADDR_ARGS(bond->active_slave_mac)); + ds_put_format(ds,"(%s)\n", slave ? slave->name : "none"); + + sorted_slaves = shash_sort(&slave_shash); for (i = 0; i < shash_count(&slave_shash); i++) { struct bond_entry *be; @@ -1906,6 +1939,13 @@ bond_choose_slave(const struct bond *bond) { struct bond_slave *slave, *best; + /* If there's a primary and it's active, return that. */ + HMAP_FOR_EACH (slave, hmap_node, &bond->slaves) { + if (slave->is_primary && slave->enabled) { + return slave; + } + } + /* Find the last active slave. */ slave = bond_find_slave_by_mac(bond, bond->active_slave_mac); if (slave && slave->enabled) { diff --git a/ofproto/bond.h b/ofproto/bond.h index 40c3258dc..1a42a37c9 100644 --- a/ofproto/bond.h +++ b/ofproto/bond.h @@ -45,6 +45,7 @@ struct bond_settings { /* Balancing configuration. */ enum bond_mode balance; + const char *primary; /* For AB balance, primary interface name. */ int rebalance_interval; /* Milliseconds between rebalances. Zero to disable rebalancing. */ diff --git a/tests/lacp.at b/tests/lacp.at index df1691731..5257f0cce 100644 --- a/tests/lacp.at +++ b/tests/lacp.at @@ -126,6 +126,7 @@ updelay: 0 ms downdelay: 0 ms lacp_status: negotiated lacp_fallback_ab: false +active-backup primary: <none> active slave mac: 00:00:00:00:00:00(none) slave p1: disabled @@ -292,6 +293,7 @@ updelay: 0 ms downdelay: 0 ms lacp_status: negotiated lacp_fallback_ab: false +active-backup primary: <none> slave p0: enabled may_enable: true @@ -308,6 +310,7 @@ updelay: 0 ms downdelay: 0 ms lacp_status: negotiated lacp_fallback_ab: false +active-backup primary: <none> slave p2: enabled may_enable: true @@ -431,6 +434,7 @@ updelay: 0 ms downdelay: 0 ms lacp_status: negotiated lacp_fallback_ab: false +active-backup primary: <none> <active slave mac del> slave p0: disabled @@ -449,6 +453,7 @@ updelay: 0 ms downdelay: 0 ms lacp_status: negotiated lacp_fallback_ab: false +active-backup primary: <none> <active slave mac del> slave p2: disabled @@ -565,6 +570,7 @@ updelay: 0 ms downdelay: 0 ms lacp_status: negotiated lacp_fallback_ab: false +active-backup primary: <none> <active slave mac del> slave p0: disabled @@ -583,6 +589,7 @@ updelay: 0 ms downdelay: 0 ms lacp_status: negotiated lacp_fallback_ab: false +active-backup primary: <none> <active slave mac del> slave p2: disabled @@ -704,6 +711,7 @@ updelay: 0 ms downdelay: 0 ms lacp_status: negotiated lacp_fallback_ab: false +active-backup primary: <none> <active slave mac del> slave p0: enabled @@ -722,6 +730,7 @@ updelay: 0 ms downdelay: 0 ms lacp_status: negotiated lacp_fallback_ab: false +active-backup primary: <none> <active slave mac del> slave p2: enabled diff --git a/tests/ofproto-dpif.at b/tests/ofproto-dpif.at index e3402e7b8..808200800 100644 --- a/tests/ofproto-dpif.at +++ b/tests/ofproto-dpif.at @@ -29,12 +29,16 @@ AT_CHECK([ovs-appctl revalidator/wait]) OVS_VSWITCHD_STOP AT_CLEANUP -AT_SETUP([ofproto-dpif - active-backup bonding]) -# Create br0 with interfaces p1, p2 and p7, creating bond0 with p1 and p2 -# and br1 with interfaces p3, p4 and p8. -# toggle p1,p2 of bond0 up and down to test bonding in active-backup mode. +AT_SETUP([ofproto-dpif - active-backup bonding (with primary)]) + +dnl Create br0 with interfaces p1, p2 and p7, creating bond0 with p1 and +dnl p2 (p1 as primary) and br1 with interfaces p3, p4 and p8. +dnl toggle p1,p2 of bond0 up and down to test bonding in active-backup mode. +dnl With p1 down and p2 up/active, bring p1 back up. Since p1 is the primary, +dnl it should become active. OVS_VSWITCHD_START( - [add-bond br0 bond0 p1 p2 bond_mode=active-backup --\ + [add-bond br0 bond0 p1 p2 bond_mode=active-backup \ + other_config:bond-primary=p1 -- \ set interface p1 type=dummy options:pstream=punix:$OVS_RUNDIR/p1.sock ofport_request=1 -- \ set interface p2 type=dummy options:pstream=punix:$OVS_RUNDIR/p2.sock ofport_request=2 -- \ add-port br0 p7 -- set interface p7 ofport_request=7 type=dummy -- \ @@ -45,9 +49,202 @@ OVS_VSWITCHD_START( add-port br1 p3 -- set interface p3 type=dummy options:stream=unix:$OVS_RUNDIR/p1.sock ofport_request=3 -- \ add-port br1 p4 -- set interface p4 type=dummy options:stream=unix:$OVS_RUNDIR/p2.sock ofport_request=4 -- \ add-port br1 p8 -- set interface p8 ofport_request=8 type=dummy --]) +AT_CHECK([ovs-appctl vlog/set dpif:dbg dpif_netdev:dbg]) WAIT_FOR_DUMMY_PORTS([p3], [p4]) +OVS_WAIT_UNTIL([test -n "`ovs-appctl bond/show | grep 'active-backup primary: p1'`"]) + + +AT_CHECK([ovs-ofctl add-flow br0 action=normal]) +AT_CHECK([ovs-ofctl add-flow br1 action=normal]) +ovs-appctl netdev-dummy/set-admin-state up +ovs-appctl time/warp 100 +ovs-appctl netdev-dummy/set-admin-state p2 down +ovs-appctl time/stop +ovs-appctl time/warp 100 +AT_CHECK([ovs-appctl netdev-dummy/receive p7 'in_port(7),eth(src=50:54:00:00:00:09,dst=50:54:00:00:00:0a),eth_type(0x0800),ipv4(src=10.0.0.2,dst=10.0.0.1,proto=1,tos=0,ttl=64,frag=no),icmp(type=8,code=0)']) +AT_CHECK([ovs-appctl netdev-dummy/receive p7 'in_port(7),eth(src=50:54:00:00:00:0b,dst=50:54:00:00:00:0c),eth_type(0x0800),ipv4(src=10.0.0.3,dst=10.0.0.4,proto=1,tos=0,ttl=64,frag=no),icmp(type=8,code=0)']) +ovs-appctl time/warp 100 +ovs-appctl netdev-dummy/set-admin-state p2 up +ovs-appctl netdev-dummy/set-admin-state p1 down +ovs-appctl time/warp 100 +AT_CHECK([ovs-appctl netdev-dummy/receive p7 'in_port(7),eth(src=50:54:00:00:00:09,dst=50:54:00:00:00:0d),eth_type(0x0800),ipv4(src=10.0.0.5,dst=10.0.0.1,proto=1,tos=0,ttl=64,frag=no),icmp(type=8,code=0)']) +AT_CHECK([ovs-appctl netdev-dummy/receive p7 'in_port(7),eth(src=50:54:00:00:00:09,dst=50:54:00:00:00:0e),eth_type(0x0800),ipv4(src=10.0.0.6,dst=10.0.0.1,proto=1,tos=0,ttl=64,frag=no),icmp(type=8,code=0)']) +ovs-appctl time/warp 2000 100 +AT_CHECK([ovs-appctl dpctl/dump-flows | grep 'in_port([[348]])' | strip_xout], [0], [dnl +recirc_id(0),in_port(3),packet_type(ns=0,id=0),eth(src=50:54:00:00:00:09,dst=50:54:00:00:00:0a),eth_type(0x0800),ipv4(frag=no), packets:0, bytes:0, used:never, actions: <del> +recirc_id(0),in_port(3),packet_type(ns=0,id=0),eth(src=50:54:00:00:00:0b,dst=50:54:00:00:00:0c),eth_type(0x0800),ipv4(frag=no), packets:0, bytes:0, used:never, actions: <del> +recirc_id(0),in_port(4),packet_type(ns=0,id=0),eth(src=50:54:00:00:00:09,dst=50:54:00:00:00:0d),eth_type(0x0800),ipv4(frag=no), packets:0, bytes:0, used:never, actions: <del> +recirc_id(0),in_port(4),packet_type(ns=0,id=0),eth(src=50:54:00:00:00:09,dst=50:54:00:00:00:0e),eth_type(0x0800),ipv4(frag=no), packets:0, bytes:0, used:never, actions: <del> +recirc_id(0),in_port(4),packet_type(ns=0,id=0),eth(src=50:54:00:00:00:09,dst=ff:ff:ff:ff:ff:ff),eth_type(0x8035), packets:0, bytes:0, used:never, actions: <del> +recirc_id(0),in_port(4),packet_type(ns=0,id=0),eth(src=50:54:00:00:00:0b,dst=ff:ff:ff:ff:ff:ff),eth_type(0x8035), packets:0, bytes:0, used:never, actions: <del> +]) + +ovs-appctl netdev-dummy/set-admin-state p1 up +ovs-appctl time/warp 100 +OVS_WAIT_UNTIL([ovs-appctl bond/show | STRIP_RECIRC_ID | STRIP_ACTIVE_SLAVE_MAC], [0], [dnl +---- bond0 ---- +bond_mode: active-backup +bond may use recirculation: no, <del> +bond-hash-basis: 0 +updelay: 0 ms +downdelay: 0 ms +lacp_status: off +lacp_fallback_ab: false +active-backup primary: p1 +<active slave mac del> + +slave p1: enabled + active slave + may_enable: true + +slave p2: enabled + may_enable: true + +]) + +OVS_VSWITCHD_STOP +AT_CLEANUP + +AT_SETUP([ofproto-dpif - active-backup bonding (primary validation)]) +dnl Make a switch with 3 ports in a bond, so that when we delete one of +dnl the ports from the bond, there are still 2 ports left and the bond +dnl remains functional. +OVS_VSWITCHD_START( + [add-bond br0 bond0 p1 p2 p3 bond_mode=active-backup \ + other_config:bond-primary=p1 -- \ + set interface p1 type=dummy options:pstream=punix:$OVS_RUNDIR/p1.sock ofport_request=1 -- \ + set interface p2 type=dummy options:pstream=punix:$OVS_RUNDIR/p2.sock ofport_request=2 -- \ + set interface p3 type=dummy options:pstream=punix:$OVS_RUNDIR/p3.sock ofport_request=3 -- \ + add-port br0 p7 -- set interface p7 ofport_request=7 type=dummy --]) AT_CHECK([ovs-appctl vlog/set dpif:dbg dpif_netdev:dbg]) +dnl Make sure the initial primary interface is set +OVS_WAIT_UNTIL([test -n "`ovs-appctl bond/show | grep 'active-backup primary: p1'`"]) + +dnl Down the primary interface and verify that we switched. Then +dnl bring the primary back and verify that we switched back to the +dnl primary. +ovs-appctl netdev-dummy/set-admin-state p1 down +ovs-appctl time/warp 100 +OVS_WAIT_UNTIL([test -n "`ovs-appctl bond/show | fgrep 'slave p1: disabled'`"]) +ovs-appctl netdev-dummy/set-admin-state p1 up +ovs-appctl time/warp 100 +OVS_WAIT_UNTIL([ovs-appctl bond/show | STRIP_RECIRC_ID | STRIP_ACTIVE_SLAVE_MAC], [0], [dnl +---- bond0 ---- +bond_mode: active-backup +bond may use recirculation: no, <del> +bond-hash-basis: 0 +updelay: 0 ms +downdelay: 0 ms +lacp_status: off +lacp_fallback_ab: false +active-backup primary: p1 +<active slave mac del> + +slave p1: enabled + active slave + may_enable: true + +slave p2: enabled + may_enable: true + +slave p3: enabled + may_enable: true + +]) + +dnl Now delete the primary and verify that the output shows that the +dnl primary is no longer enslaved +ovs-vsctl --id=@p1 get Interface p1 -- remove Port bond0 interfaces @p1 +ovs-appctl time/warp 100 +OVS_WAIT_UNTIL([test -n "`ovs-appctl bond/show | fgrep 'active-backup primary: p1 (no such slave)'`"]) + +dnl Now re-add the primary and verify that the output shows that the +dnl primary is available again. +dnl +dnl First, get the UUIDs of the interfaces that exist on bond0. +dnl Strip the trailing ] so that we can add a new UUID to the end. +uuids=`ovs-vsctl get Port bond0 interfaces | sed -e 's/]//'` +dnl Create a new port "p1" and add its UUID to the set of interfaces +dnl on bond0. +ovs-vsctl \ + --id=@p1 create Interface name=p1 type=dummy options:pstream=punix:$OVS_RUNDIR/p1.sock ofport_request=1 -- \ + set Port bond0 interfaces="$uuids, @p1]" +ovs-appctl time/warp 100 +OVS_WAIT_UNTIL([ovs-appctl bond/show | STRIP_RECIRC_ID | STRIP_ACTIVE_SLAVE_MAC], [0], [dnl +---- bond0 ---- +bond_mode: active-backup +bond may use recirculation: no, <del> +bond-hash-basis: 0 +updelay: 0 ms +downdelay: 0 ms +lacp_status: off +lacp_fallback_ab: false +active-backup primary: p1 +<active slave mac del> + +slave p1: enabled + active slave + may_enable: true + +slave p2: enabled + may_enable: true + +slave p3: enabled + may_enable: true + +]) + +dnl Remove the "bond-primary" config directive from the bond. +AT_CHECK([ovs-vsctl remove Port bond0 other_config bond-primary]) +ovs-appctl time/warp 100 +OVS_WAIT_UNTIL([ovs-appctl bond/show | STRIP_RECIRC_ID | STRIP_ACTIVE_SLAVE_MAC], [0], [dnl +---- bond0 ---- +bond_mode: active-backup +bond may use recirculation: no, <del> +bond-hash-basis: 0 +updelay: 0 ms +downdelay: 0 ms +lacp_status: off +lacp_fallback_ab: false +active-backup primary: <none> +<active slave mac del> + +slave p1: enabled + active slave + may_enable: true + +slave p2: enabled + may_enable: true + +slave p3: enabled + may_enable: true + +]) + +OVS_VSWITCHD_STOP +AT_CLEANUP + +AT_SETUP([ofproto-dpif - active-backup bonding (without primary)]) +dnl Create br0 with interfaces p1, p2 and p7, creating bond0 with p1 and p2 +dnl and br1 with interfaces p3, p4 and p8. +dnl toggle p1,p2 of bond0 up and down to test bonding in active-backup mode. +OVS_VSWITCHD_START( + [add-bond br0 bond0 p1 p2 bond_mode=active-backup --\ + set interface p1 type=dummy options:pstream=punix:$OVS_RUNDIR/p1.sock ofport_request=1 -- \ + set interface p2 type=dummy options:pstream=punix:$OVS_RUNDIR/p2.sock ofport_request=2 -- \ + add-port br0 p7 -- set interface p7 ofport_request=7 type=dummy -- \ + add-br br1 -- \ + set bridge br1 other-config:hwaddr=aa:66:aa:66:00:00 -- \ + set bridge br1 datapath-type=dummy other-config:datapath-id=1234 \ + fail-mode=secure -- \ + add-port br1 p3 -- set interface p3 type=dummy options:stream=unix:$OVS_RUNDIR/p1.sock ofport_request=3 -- \ + add-port br1 p4 -- set interface p4 type=dummy options:stream=unix:$OVS_RUNDIR/p2.sock ofport_request=4 -- \ + add-port br1 p8 -- set interface p8 ofport_request=8 type=dummy --]) +AT_CHECK([ovs-appctl vlog/set dpif:dbg dpif_netdev:dbg]) +WAIT_FOR_DUMMY_PORTS([p3], [p4]) +OVS_WAIT_UNTIL([test -n "`ovs-appctl bond/show | grep 'active-backup primary: <none>'`"]) + AT_CHECK([ovs-ofctl add-flow br0 action=normal]) AT_CHECK([ovs-ofctl add-flow br1 action=normal]) ovs-appctl netdev-dummy/set-admin-state up @@ -63,15 +260,14 @@ ovs-appctl netdev-dummy/set-admin-state p1 down ovs-appctl time/warp 100 AT_CHECK([ovs-appctl netdev-dummy/receive p7 'in_port(7),eth(src=50:54:00:00:00:09,dst=50:54:00:00:00:0d),eth_type(0x0800),ipv4(src=10.0.0.5,dst=10.0.0.1,proto=1,tos=0,ttl=64,frag=no),icmp(type=8,code=0)']) AT_CHECK([ovs-appctl netdev-dummy/receive p7 'in_port(7),eth(src=50:54:00:00:00:09,dst=50:54:00:00:00:0e),eth_type(0x0800),ipv4(src=10.0.0.6,dst=10.0.0.1,proto=1,tos=0,ttl=64,frag=no),icmp(type=8,code=0)']) -ovs-appctl time/warp 200 100 -sleep 1 -AT_CHECK([grep 'in_port([[348]])' ovs-vswitchd.log | filter_flow_install | strip_xout], [0], [dnl -recirc_id(0),in_port(3),packet_type(ns=0,id=0),eth(src=50:54:00:00:00:09,dst=50:54:00:00:00:0a),eth_type(0x0800),ipv4(frag=no), actions: <del> -recirc_id(0),in_port(3),packet_type(ns=0,id=0),eth(src=50:54:00:00:00:0b,dst=50:54:00:00:00:0c),eth_type(0x0800),ipv4(frag=no), actions: <del> -recirc_id(0),in_port(4),packet_type(ns=0,id=0),eth(src=50:54:00:00:00:09,dst=50:54:00:00:00:0d),eth_type(0x0800),ipv4(frag=no), actions: <del> -recirc_id(0),in_port(4),packet_type(ns=0,id=0),eth(src=50:54:00:00:00:09,dst=50:54:00:00:00:0e),eth_type(0x0800),ipv4(frag=no), actions: <del> -recirc_id(0),in_port(4),packet_type(ns=0,id=0),eth(src=50:54:00:00:00:09,dst=ff:ff:ff:ff:ff:ff),eth_type(0x8035), actions: <del> -recirc_id(0),in_port(4),packet_type(ns=0,id=0),eth(src=50:54:00:00:00:0b,dst=ff:ff:ff:ff:ff:ff),eth_type(0x8035), actions: <del> +ovs-appctl time/warp 2000 100 +AT_CHECK([ovs-appctl dpctl/dump-flows | grep 'in_port([[348]])' | strip_xout], [0], [dnl +recirc_id(0),in_port(3),packet_type(ns=0,id=0),eth(src=50:54:00:00:00:09,dst=50:54:00:00:00:0a),eth_type(0x0800),ipv4(frag=no), packets:0, bytes:0, used:never, actions: <del> +recirc_id(0),in_port(3),packet_type(ns=0,id=0),eth(src=50:54:00:00:00:0b,dst=50:54:00:00:00:0c),eth_type(0x0800),ipv4(frag=no), packets:0, bytes:0, used:never, actions: <del> +recirc_id(0),in_port(4),packet_type(ns=0,id=0),eth(src=50:54:00:00:00:09,dst=50:54:00:00:00:0d),eth_type(0x0800),ipv4(frag=no), packets:0, bytes:0, used:never, actions: <del> +recirc_id(0),in_port(4),packet_type(ns=0,id=0),eth(src=50:54:00:00:00:09,dst=50:54:00:00:00:0e),eth_type(0x0800),ipv4(frag=no), packets:0, bytes:0, used:never, actions: <del> +recirc_id(0),in_port(4),packet_type(ns=0,id=0),eth(src=50:54:00:00:00:09,dst=ff:ff:ff:ff:ff:ff),eth_type(0x8035), packets:0, bytes:0, used:never, actions: <del> +recirc_id(0),in_port(4),packet_type(ns=0,id=0),eth(src=50:54:00:00:00:0b,dst=ff:ff:ff:ff:ff:ff),eth_type(0x8035), packets:0, bytes:0, used:never, actions: <del> ]) OVS_VSWITCHD_STOP AT_CLEANUP diff --git a/vswitchd/bridge.c b/vswitchd/bridge.c index f312efd8e..7111fbac3 100644 --- a/vswitchd/bridge.c +++ b/vswitchd/bridge.c @@ -4563,6 +4563,11 @@ port_configure_bond(struct port *port, struct bond_settings *s) port->name); } + s->primary = NULL; + if (s->balance == BM_AB || s->lacp_fallback_ab_cfg) { + s->primary = smap_get(&port->cfg->other_config, "bond-primary"); + } + miimon_interval = smap_get_int(&port->cfg->other_config, "bond-miimon-interval", 0); if (miimon_interval <= 0) { diff --git a/vswitchd/vswitch.xml b/vswitchd/vswitch.xml index b6acb34ca..944708ed6 100644 --- a/vswitchd/vswitch.xml +++ b/vswitchd/vswitch.xml @@ -2003,6 +2003,14 @@ This knob does not affect other balancing modes. </column> + <column name="other_config" key="bond-primary" + type='{"type": "string"}'> + If a slave interface with this name exists in the bond and + is up, it will be made active. Relevant only when <ref + column="other_config" key="bond-mode"/> is + <code>active-backup</code>. + </column> + <group title="Link Failure Detection"> <p> An important part of link bonding is detecting that links are down so