netdev - bonding: time limits too tight in bond_ab_arp

lists.openwall.net		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 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [thread-next>] [day] [month] [year] [list]

Message-ID: <20120822174534.GA20260@midget.suse.cz>
Date:	Wed, 22 Aug 2012 19:45:34 +0200
From:	Jiri Bohac <jbohac@...e.cz>
To:	Jay Vosburgh <fubar@...ibm.com>,
	Andy Gospodarek <andy@...yhouse.net>, netdev@...r.kernel.org
Cc:	Petr Tesarik <ptesarik@...e.cz>
Subject: bonding: time limits too tight in bond_ab_arp_inspect

Hi,

a customer reported that a bonding slave did not come back up
after setting their link down and then up again. ARP monitoring +
arp_validate were used.

Petr has tracked the problem down to the time comaprisons in
bond_ab_arp_inspect().

                if (slave->link != BOND_LINK_UP) {
                        if (time_in_range(jiffies,
                                slave_last_rx(bond, slave) - delta_in_ticks,
                                slave_last_rx(bond, slave) + delta_in_ticks)) {

                                slave->new_link = BOND_LINK_UP;
                                commit++;
                        }

                        continue;
                }

This code is run from bond_activebackup_arp_mon() about
delta_in_ticks jiffies after the previous ARP probe has been
sent. If the delayed work gets executed exactly in delta_in_ticks
jiffies, there is a chance the slave will be brought up.  If the
delayed work runs one jiffy later, the slave will stay down.

With arp_validate this is more noticeable, since traffic other than the
bonding-generated ARP probes does not update the slave_last_rx timestamp.

A simple patch will fix this case.

--- a/drivers/net/bonding/bond_main.c
+++ b/drivers/net/bonding/bond_main.c
@@ -3001,7 +3001,7 @@ static int bond_ab_arp_inspect(struct bo
 		if (slave->link != BOND_LINK_UP) {
 			if (time_in_range(jiffies,
 				slave_last_rx(bond, slave) - delta_in_ticks,
-				slave_last_rx(bond, slave) + delta_in_ticks)) {
+				slave_last_rx(bond, slave) + 2 * delta_in_ticks)) {

 				slave->new_link = BOND_LINK_UP;
 				commit++;

The remaining time comparisons inside bond_ab_arp_inspect() have larger
tolerances (3*delta_in_ticks or 2*delta_in_ticks), but it still seems strange
that the precision of delayed work scheduling should steal a full
arp_interval from the time limits.

What is the intention of e.g. the "3*delta since last receive" limit? 
Was this really meant to be "as little as 2*delta + 1 jiffy"?

Should they perhaps all be increased by, say, delta_in_ticks/2, to make this
less dependent on the current scheduling latencies?

Thoughts?

-- 
Jiri Bohac <jbohac@...e.cz>
SUSE Labs, SUSE CZ

--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html