lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  PHC 
Open Source and information security mailing list archives
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Date:	Mon, 24 Jun 2013 11:26:52 +0200
From:	Veaceslav Falico <>
To:	Jay Vosburgh <>
Subject: Re: [PATCH v3 net-next 6/6] bonding: add an option to fail when any
 of arp_ip_target is inaccessible

On Fri, Jun 21, 2013 at 09:21:43PM +0200, Veaceslav Falico wrote:
>On Fri, Jun 21, 2013 at 11:21:20AM -0700, Jay Vosburgh wrote:
>>Veaceslav Falico <> wrote:
>>>--- a/Documentation/networking/bonding.txt
>>>+++ b/Documentation/networking/bonding.txt
>>>@@ -321,6 +321,23 @@ arp_validate
>>>	This option was added in bonding version 3.1.0.
>>>+	Specifies whether, in active-backup mode with arp validation,
>>>+	any of the arp_ip_targets should be up to keep the slave up
>>>+	(default) or it should go down if at least one of
>>>+	arp_ip_targets doesn't reply to arp requests.
>>	I would write the above as:
>>	Specifies the quantity of arp_ip_targets that must be reachable
>>	in order for the ARP monitor to consider a slave as being up.
>>	This option affects only active-backup mode for slaves with
>>	arp_validation enabled.
>Yeah, a lot more clear. However, doesn't "the quantity" suggest some
>number, but not binary (as we have it)?
>Hrm, it's an interesting idea btw, to make it accept the exact quantity of
>reachable arp_ip_targets to be up, instead of any/all, though I don't think
>it'll find use in real world situation. It's easy to implement btw, we
>just need to get the n-th least fresh arp_ip_target in slave_last_rx(). And
>we can have -1 for all.
>If you think that it's viable - I can add it to the next version.

Anyway, I think that it's better to do with another patch, not to
complicate again this patchset.

>>>+		if (ind == 0 && !targets[1] && bond->params.arp_validate)
>>>+			pr_warn("%s: removing last arp target with arp_validate on\n",
>>>+				bond->dev->name);
>>	There's no warning in the current code for removing the last
>>arp_ip_target; I don't think adding one is necessary, particularly if it
>>is just for the validate case.
>My logic was - if we're currently configured to be up by the arp_validate -
>and we removing the last arp_ip_target - then we're either in the process
>of shutting down the interface or something is really wrong. And, btw,
>there's such an pr_info() for bonding_store_arp_interval():
> 588                 if (!bond->params.arp_targets[0])
> 589                         pr_info("%s: ARP monitoring has been set up, but no ARP targets have been specified.\n",
> 590                                 bond->dev->name);
>So, maybe, only change to bond->params.arp_interval instead of arp_validate?

I've changed it to arp_interval in the meanwhile for v4, it's a harmless
warning but can help a lot in the case of misconfiguration :).

Will send the v4 now.

>Thank you very much for the review!
>>	-J
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to
More majordomo info at

Powered by blists - more mailing lists