[ovs-dev] [PATCH 1/3] bond: Pseudo-randomly choose initial slave.

Ethan Jackson ethan at nicira.com
Tue Feb 5 20:46:21 UTC 2013


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 <blp at 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 <ethan at 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 at openvswitch.org
>> http://openvswitch.org/mailman/listinfo/dev



More information about the dev mailing list