From mboxrd@z Thu Jan 1 00:00:00 1970 From: Nikolay Aleksandrov Subject: Re: [PATCH v3 net-next 02/12] bonding: permit using arp_validate with non-ab modes Date: Mon, 17 Feb 2014 14:59:14 +0100 Message-ID: <530215B2.3090704@redhat.com> References: <1392644361-8510-1-git-send-email-vfalico@redhat.com> <1392644361-8510-3-git-send-email-vfalico@redhat.com> <5302132D.7070401@redhat.com> <20140217140144.GC13038@redhat.com> Mime-Version: 1.0 Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit Cc: netdev@vger.kernel.org, Jay Vosburgh , Andy Gospodarek To: Veaceslav Falico Return-path: Received: from mx1.redhat.com ([209.132.183.28]:44138 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750851AbaBQODq (ORCPT ); Mon, 17 Feb 2014 09:03:46 -0500 In-Reply-To: <20140217140144.GC13038@redhat.com> Sender: netdev-owner@vger.kernel.org List-ID: On 02/17/2014 03:01 PM, Veaceslav Falico wrote: > 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 >>> CC: Andy Gospodarek >>> Signed-off-by: Veaceslav Falico >>> --- >>> 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! > Yep, that should do it :-) >> >> Cheers, >> Nik