On Mon, Feb 04, 2013 at 07:13:34PM -0800, Ethan Jackson wrote:
> Traditionally, (unless balancing was turned off), the bonding code
> chose the active slave for all flows which hadn't previously been
> allocated to another. This worked fine in theory because traffic
> would eventually be balanced away if there was congestion.
>
> Instead of the historical approach, this patch uses a hash to
> choose the initial slave, giving what behaves like a random
> allocation. This performs better on switches using a long
> rebalance interval, is less confusing to users, and removes a
> special case needed for dealing with non-balanced bonds.
>
> Signed-off-by: Ethan Jackson <[email protected]>
This doesn't change anything in the common case, only in the case where
the first randomly chosen slave (via hmap_random_node()) is disabled.
Is that corner case worth worrying about? It can only make a difference
in a bond with three or more slaves. If it is worth dealing with, why
pass a hash to choose_stb_slave() instead of just a random number? (And
why not just use choose_stb_slave() by itself instead of as a second
pass?)
> --- a/lib/bond.c
> +++ b/lib/bond.c
> @@ -1442,15 +1442,12 @@ choose_output_slave(const struct bond *bond, const
> struct flow *flow,
> }
> /* Fall Through. */
> case BM_SLB:
> - if (!bond_is_balanced(bond)) {
> - return choose_stb_slave(bond, bond_hash(bond, flow, vlan));
> - }
> e = lookup_bond_entry(bond, flow, vlan);
> if (!e->slave || !e->slave->enabled) {
> e->slave = CONTAINER_OF(hmap_random_node(&bond->slaves),
> struct bond_slave, hmap_node);
> if (!e->slave->enabled) {
> - e->slave = bond->active_slave;
> + e->slave = choose_stb_slave(bond, bond_hash(bond, flow,
> vlan));
> }
> e->tag = tag_create_random();
> }
> --
> 1.7.9.5
>
> _______________________________________________
> dev mailing list
> [email protected]
> http://openvswitch.org/mailman/listinfo/dev
_______________________________________________
dev mailing list
[email protected]
http://openvswitch.org/mailman/listinfo/dev
