Commit 8ae6af83 authored by Jay Vosburgh's avatar Jay Vosburgh Committed by Luis Henriques

bonding: Fix ARP monitor validation

commit 21a75f09 upstream.

The current logic in bond_arp_rcv will accept an incoming ARP for
validation if (a) the receiving slave is either "active" (which includes
the currently active slave, or the current ARP slave) or, (b) there is a
currently active slave, and it has received an ARP since it became active.
For case (b), the receiving slave isn't the currently active slave, and is
receiving the original broadcast ARP request, not an ARP reply from the
target.

	This logic can fail if there is no currently active slave.  In
this situation, the ARP probe logic cycles through all slaves, assigning
each in turn as the "current_arp_slave" for one arp_interval, then setting
that one as "active," and sending an ARP probe from that slave.  The
current logic expects the ARP reply to arrive on the sending
current_arp_slave, however, due to switch FDB updating delays, the reply
may be directed to another slave.

	This can arise if the bonding slaves and switch are working, but
the ARP target is not responding.  When the ARP target recovers, a
condition may result wherein the ARP target host replies faster than the
switch can update its forwarding table, causing each ARP reply to be sent
to the previous current_arp_slave.  This will never pass the logic in
bond_arp_rcv, as neither of the above conditions (a) or (b) are met.

	Some experimentation on a LAN shows ARP reply round trips in the
200 usec range, but my available switches never update their FDB in less
than 4000 usec.

	This patch changes the logic in bond_arp_rcv to additionally
accept an ARP reply for validation on any slave if there is a current ARP
slave and it sent an ARP probe during the previous arp_interval.

Fixes: aeea64ac ("bonding: don't trust arp requests unless active slave really works")
Cc: Veaceslav Falico <vfalico@gmail.com>
Cc: Andy Gospodarek <gospo@cumulusnetworks.com>
Signed-off-by: default avatarJay Vosburgh <jay.vosburgh@canonical.com>
Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
[ luis: backported to 3.16: adjusted context ]
Signed-off-by: default avatarLuis Henriques <luis.henriques@canonical.com>
parent e0267997
...@@ -210,6 +210,8 @@ static int lacp_fast; ...@@ -210,6 +210,8 @@ static int lacp_fast;
static int bond_init(struct net_device *bond_dev); static int bond_init(struct net_device *bond_dev);
static void bond_uninit(struct net_device *bond_dev); static void bond_uninit(struct net_device *bond_dev);
static bool bond_time_in_interval(struct bonding *bond, unsigned long last_act,
int mod);
/*---------------------------- General routines -----------------------------*/ /*---------------------------- General routines -----------------------------*/
...@@ -2352,7 +2354,7 @@ int bond_arp_rcv(const struct sk_buff *skb, struct bonding *bond, ...@@ -2352,7 +2354,7 @@ int bond_arp_rcv(const struct sk_buff *skb, struct bonding *bond,
struct slave *slave) struct slave *slave)
{ {
struct arphdr *arp = (struct arphdr *)skb->data; struct arphdr *arp = (struct arphdr *)skb->data;
struct slave *curr_active_slave; struct slave *curr_active_slave, *curr_arp_slave;
unsigned char *arp_ptr; unsigned char *arp_ptr;
__be32 sip, tip; __be32 sip, tip;
int alen, is_arp = skb->protocol == __cpu_to_be16(ETH_P_ARP); int alen, is_arp = skb->protocol == __cpu_to_be16(ETH_P_ARP);
...@@ -2399,27 +2401,41 @@ int bond_arp_rcv(const struct sk_buff *skb, struct bonding *bond, ...@@ -2399,27 +2401,41 @@ int bond_arp_rcv(const struct sk_buff *skb, struct bonding *bond,
&sip, &tip); &sip, &tip);
curr_active_slave = rcu_dereference(bond->curr_active_slave); curr_active_slave = rcu_dereference(bond->curr_active_slave);
curr_arp_slave = rcu_dereference(bond->current_arp_slave);
/* /* We 'trust' the received ARP enough to validate it if:
* Backup slaves won't see the ARP reply, but do come through *
* here for each ARP probe (so we swap the sip/tip to validate * (a) the slave receiving the ARP is active (which includes the
* the probe). In a "redundant switch, common router" type of * current ARP slave, if any), or
* configuration, the ARP probe will (hopefully) travel from *
* the active, through one switch, the router, then the other * (b) the receiving slave isn't active, but there is a currently
* switch before reaching the backup. * active slave and it received valid arp reply(s) after it became
* the currently active slave, or
* *
* We 'trust' the arp requests if there is an active slave and * (c) there is an ARP slave that sent an ARP during the prior ARP
* it received valid arp reply(s) after it became active. This * interval, and we receive an ARP reply on any slave. We accept
* is done to avoid endless looping when we can't reach the * these because switch FDB update delays may deliver the ARP
* reply to a slave other than the sender of the ARP request.
*
* Note: for (b), backup slaves are receiving the broadcast ARP
* request, not a reply. This request passes from the sending
* slave through the L2 switch(es) to the receiving slave. Since
* this is checking the request, sip/tip are swapped for
* validation.
*
* This is done to avoid endless looping when we can't reach the
* arp_ip_target and fool ourselves with our own arp requests. * arp_ip_target and fool ourselves with our own arp requests.
*/ */
if (bond_is_active_slave(slave)) if (bond_is_active_slave(slave))
bond_validate_arp(bond, slave, sip, tip); bond_validate_arp(bond, slave, sip, tip);
else if (curr_active_slave && else if (curr_active_slave &&
time_after(slave_last_rx(bond, curr_active_slave), time_after(slave_last_rx(bond, curr_active_slave),
curr_active_slave->last_link_up)) curr_active_slave->last_link_up))
bond_validate_arp(bond, slave, tip, sip); bond_validate_arp(bond, slave, tip, sip);
else if (curr_arp_slave && (arp->ar_op == htons(ARPOP_REPLY)) &&
bond_time_in_interval(bond,
dev_trans_start(curr_arp_slave->dev), 1))
bond_validate_arp(bond, slave, sip, tip);
out_unlock: out_unlock:
if (arp != (struct arphdr *)skb->data) if (arp != (struct arphdr *)skb->data)
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment