Upon reflection, I think you're right. I've just dropped this patch.
Ethan
On Tue, Feb 5, 2013 at 8:40 AM, Ben Pfaff <b...@nicira.com> wrote:
> 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 <et...@nicira.com>
>
> 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
>> dev@openvswitch.org
>> http://openvswitch.org/mailman/listinfo/dev
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
