Subject: Re: [PATCH] net/bonding: send arp in interval if no active slave

From: Jarod Wilson <[email-masked]>
Date: Mon, 28 Sep 2015 09:31:35 -0400

Uwe Koziolek wrote:
> Am 03.09.2015 um 17:05 schrieb Jay Vosburgh:
>> Uwe Koziolek <[email-masked]> wrote:
>>
>>> On Tue, Sep 01, 2015 at 05:41 PM +0200, Andy Gospodarek wrote:
>>>> On Mon, Aug 17, 2015 at 10:51:27PM +0200, Uwe Koziolek wrote:
>>>>> On Mon, Aug 17, 2015 at 09:14PM +0200, Jay Vosburgh wrote:
>>>>>> Uwe Koziolek <[email-masked]> wrote:
...
>> I probably should have asked this in the beginning, but at what
>> range of arp_interval values does the problem manifest? If it's a race
>> condition with the switch update, I'd expect that only very small
>> arp_interval values would be affected.
>>
>> Also, your proposed comment wraps past 80 columns.
>>
>> -J
>>
> Only 500 msecs arp interval is used, no other values are checked.
> Wraps in patch are now removed.
>
> diff -up ./drivers/net/bonding/bond_main.c.orig
> ./drivers/net/bonding/bond_main.c
> --- ./drivers/net/bonding/bond_main.c.orig 2015-08-30 20:34:09.000000000
> +0200
> +++ ./drivers/net/bonding/bond_main.c 2015-09-04 11:59:05.755897182 +0200
> @@ -2795,6 +2795,17 @@ static bool bond_ab_arp_probe(struct bon
> return should_notify_rtnl;
> }
>
> + /* sometimes the forwarding tables of the switches are not updated
> + * fast enough. the first arp response after a slave change is received
> + * on the wrong slave.
> + * the arp requests will be retried 2 times on the same slave
> + */
> +
> + if (bond_time_in_interval(bond, curr_arp_slave->last_link_up, 2)) {
> + bond_arp_send_all(bond, curr_arp_slave);
> + return should_notify_rtnl;
> + }
> +
> bond_set_slave_inactive_flags(curr_arp_slave, BOND_SLAVE_NOTIFY_LATER);
>
> bond_for_each_slave_rcu(bond, slave, iter) {

Jay, any further issues with this patch? I know Veaceslav was concerned 
about it breaking the logic for link state management if there's no 
current active slave for 2 * arp_interval, while Andy seemed okay with 
it, provided there was a comment explaining. Just looking at what might 
have to be done next here to keep heading towards a resolution.

Thanks much,

-- 
Jarod Wilson
[email-masked]