Re: [PATCH v2 net-next 6/6] bonding: add an option to fail when any of arp_ip_target is inaccessible

[Veaceslav Falico <vfalico@redhat.com>]

On Fri, Jun 21, 2013 at 12:23:18PM +0200, Michal Kubecek wrote:
>On Thu, Jun 20, 2013 at 06:35:05PM +0200, Veaceslav Falico wrote:
>> @@ -1712,6 +1721,8 @@ int bond_enslave(struct net_device *bond_dev, struct net_device *slave_dev)
>>
>>  	new_slave->last_arp_rx = jiffies -
>>  		(msecs_to_jiffies(bond->params.arp_interval) + 1);
>> +	for (i = 0; i < BOND_MAX_ARP_TARGETS; i++)
>> +		new_slave->target_last_arp_rx[i] = jiffies;
>>
>>  	if (bond->params.miimon && !bond->params.use_carrier) {
>>  		link_reporting = bond_check_dev_link(bond, slave_dev, 1);
>
>For cards with slow initial negotiation, this can cause a down -> up ->
>down -> up flap on enslaving. This is why initial walue of last_arp_rx
>was modified in commit f31c7937. Is there a reason not to initialize
>target_last_arp_rx[i] to the same value?

Yep, I've seen this commit, however I didn't really understand it.

My logic is:

1) on enslaving, we suppose that the new slave is up and give it a chance
to prove it.
	1.1) if there is no active slave, lets try the new one, anyway
	     we're down.
	1.2) if there is one - nothing changes

2) if, as you've said, it's still initializing - then it basically will just
be marked as down until it finishes the initialization, and after that will
go up. So, it goes up -> down (while initializing) -> up (when arps are
received).

So, by using jiffies, we can start using the slave immediately, without
waiting to receive the confirmation - if we don't have an active one,
obviously. If we have one - nothing changes.

Did I miss something?

Thank you!

>
>                                                        Michal Kubecek
>