Subject: Re: [PATCH v3 net-next 02/12] bonding: permit using arp_validate with non-ab modes

From: Veaceslav Falico <[email-masked]>
Date: Mon, 17 Feb 2014 15:01:44 +0100

On Mon, Feb 17, 2014 at 02:48:29PM +0100, Nikolay Aleksandrov wrote:
>On 02/17/2014 02:39 PM, Veaceslav Falico wrote:
>> Currently it's diabled because it's sometimes hard, in typical configs, to
>> make it work - because of the nature how the loadbalance modes work - as
>> it's hard to deliver valid arp replies to correct slaves by the switch.
>>
>> However we still can use arp_validation in loadbalance in several other
>> configs, per example with arp_validate == 2 for backup with one broadcast
>> domain, without the switch(es) doing any balancing - this way we'd be (a
>> bit more) sure that the slave is up.
>>
>> So, enable it to let users decide which one works/suits them best.
>>
>> CC: Jay Vosburgh <[email-masked]>
>> CC: Andy Gospodarek <[email-masked]>
>> Signed-off-by: Veaceslav Falico <[email-masked]>
>> ---
>>  Documentation/networking/bonding.txt | 6 +++---
>>  drivers/net/bonding/bond_main.c      | 4 ----
>>  2 files changed, 3 insertions(+), 7 deletions(-)
>>
>> diff --git a/Documentation/networking/bonding.txt b/Documentation/networking/bonding.txt
>> index 5cdb229..96b4ad8 100644
>> --- a/Documentation/networking/bonding.txt
>> +++ b/Documentation/networking/bonding.txt
>> @@ -270,9 +270,9 @@ arp_ip_target
>>  arp_validate
>>
>>  	Specifies whether or not ARP probes and replies should be
>> -	validated in the active-backup mode.  This causes the ARP
>> -	monitor to examine the incoming ARP requests and replies, and
>> -	only consider a slave to be up if it is receiving the
>> +	validated in any mode that supports arp monitoring.  This causes
>> +	the ARP monitor to examine the incoming ARP requests and replies,
>> +	and only consider a slave to be up if it is receiving the
>>  	appropriate ARP traffic.
>>
>>  	Possible values are:
>> diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
>> index 3c50bec..91c0248 100644
>> --- a/drivers/net/bonding/bond_main.c
>> +++ b/drivers/net/bonding/bond_main.c
>> @@ -4183,10 +4183,6 @@ static int bond_check_params(struct bond_params *params)
>>  	}
>>
>>  	if (arp_validate) {
>> -		if (bond_mode != BOND_MODE_ACTIVEBACKUP) {
>> -			pr_err("arp_validate only supported in active-backup mode\n");
>> -			return -EINVAL;
>> -		}
>>  		if (!arp_interval) {
>>  			pr_err("arp_validate requires arp_interval\n");
>>  			return -EINVAL;
>>
>Hi Slava,

Hi Nik,

>After the option conversion patches to the new API I think you should modify the
>.unsuppmodes of BOND_OPT_ARP_VALIDATE in bond_options.c in order to allow it to
>be set in non-ab modes. Take a look at how BOND_OPT_PRIMARY is done for example.

Indeed, missed this part while rebasing. Thanks for the tip, I guess it's
enough to just remove the

  157                 .unsuppmodes = BOND_MODE_ALL_EX(BIT(BOND_MODE_ACTIVEBACKUP)),

line, right? Will remove and send v4.

Thanks a lot!

>Cheers,
> Nik