Re: bug in bonding driver

[Jay Vosburgh <fubar@us.ibm.com>]

Chris Friesen <cfriesen@nortel.com> wrote:
>One of our guys pointed out what appears to be a bug in
>bond_ab_arp_inspect().  There's a chunk of code that looks like this:
>
>	/*
>	 * Give slaves 2*delta after being enslaved or made
>	 * active.  This avoids bouncing, as the last receive
>	 * times need a full ARP monitor cycle to be updated.
>	 */
>	if (!time_after_eq(jiffies, slave->jiffies +
>			   2 * delta_in_ticks))
>		continue;
>
>The catch here is that slave->jiffies may not ever get updated after
>being set initially, and on long-running systems jiffies will overflow.
> That could cause this check to be true for a substantial amount of time
>rather than for just a short period.

	The definition for time_after in include/linux/jiffies.h claims
to handle timer wrapping, but even so, there presumably has to be a
cutoff at which "after" becomes "before" again.  

	Some quick fooling around suggests that if, for example,
slave->jiffies is near the top of the range (ULONG_MAX - a few hundred),
when jiffies gets up to around ULONG_MAX / 2 time_after_eq will flip
from "after" to "before."

	I don't think this is a particularly farfetched example, since
jiffies is intentionally started near the top of the range, so
slave->jiffies is likely to be high in the range after bonding is
configured at boot.

>One way to fix it would be a boolean which tracks whether or not we've
>gone past the time, and if we have then we don't bother actually
>checking the time anymore.

	It might be clearer to make the slave->jiffies some kind of
countdown instead, perhaps reusing the slave->delay used for
updelay/downdelay and eliminating slave->jiffies entirely.

	-J

---
	-Jay Vosburgh, IBM Linux Technology Center, fubar@us.ibm.com