netdev - Re: [PATCH] bonding: check if clients MAC addr has changed

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-prev] [day] [month] [year] [list]

Message-ID: <13473.1277823800@death.nxdomain.ibm.com>
Date:	Tue, 29 Jun 2010 08:03:20 -0700
From:	Jay Vosburgh <fubar@...ibm.com>
To:	Flavio Leitner <fleitner@...hat.com>
cc:	bonding-devel@...ts.sourceforge.net, netdev@...r.kernel.org,
	Andy Gospodarek <gospo@...hat.com>
Subject: Re: [PATCH] bonding: check if clients MAC addr has changed

Flavio Leitner <fleitner@...hat.com> wrote:

>When two systems using bonding devices in adaptive load
>balancing (ALB) communicates with each other, an endless
>ping-pong of ARP replies starts between these two systems.
>
>What happens? In the ALB mode, bonding driver keeps track
>of each client connected in a hash table, so it can do the
>receive load balancing (RLB). This hash table is updated
>when an ARP reply is received, then it scans for the client
>entry, updates its MAC address and flag it to be announced
>later. Therefore, two seconds later, the alb monitor runs
>and send for each updated client entry two ARP replies
>updating this specific client. The same process happens on
>the receiving system, causing the endless ping-pong of arp
>replies.
>
>See more information including the relevant functions below:
>
>   System 1                          System 2
>    bond0                             bond0
>
>   ping <system2>
>    ARP request  --------->
>                           <--------- ARP reply
>
>+->rlb_arp_recv  <---------------------+   <--- loop begins
>|  rlb_update_entry_from_arp           |
>|  client_info->ntt = 1;               |
>|  bond_info->rx_ntt = 1;              |
>|                                      |
>|         <communication succeed>      |
>|                                      |
>|  bond_alb_monitor                    |
>|  rlb_update_rx_clients               |
>|  rlb_update_client                   |
>|  arp_create(ARPOP_REPLY)             |
>|   send ARP reply -------------->     V
>|   send ARP reply -------------->
>|                               rlb_arp_recv
>|                               rlb_update_entry_from_arp
>|                               client_info->ntt = 1;
>|                               bond_info->rx_ntt = 1;
>|                           < snipped, same as in system 1>
>+-------           <-------------- send ARP reply
>                   <-------------- send ARP reply
>
>Besides the unneeded networking traffic, this loop breaks
>a cluster because a backup system can't take over the IP
>address. There is always one system sending an ARP reply
>poisoning the network.
>
>This patch fixes the problem adding a check for the MAC
>address before updating it. Thus, if the MAC address didn't
>change, there is no need to update neither to announce it later.
>
>Signed-off-by: Flavio Leitner <fleitner@...hat.com>
>---
> drivers/net/bonding/bond_alb.c |    3 ++-
> 1 files changed, 2 insertions(+), 1 deletions(-)
>
>diff --git a/drivers/net/bonding/bond_alb.c b/drivers/net/bonding/bond_alb.c
>index 40fdc41..67154bb 100644
>--- a/drivers/net/bonding/bond_alb.c
>+++ b/drivers/net/bonding/bond_alb.c
>@@ -340,7 +340,8 @@ static void rlb_update_entry_from_arp(struct bonding *bond, struct arp_pkt *arp)
>
> 	if ((client_info->assigned) &&
> 	    (client_info->ip_src == arp->ip_dst) &&
>-	    (client_info->ip_dst == arp->ip_src)) {
>+	    (client_info->ip_dst == arp->ip_src) &&
>+	    (memcmp(client_info->mac_dst, arp->mac_src, ETH_ALEN))) {

	This should use compare_ether_addr instead of memcmp.

	Other than that, this looks good, so add me to the updated patch:

Signed-off-by: Jay Vosburgh <fubar@...ibm.com>

	-J

> 		/* update the clients MAC address */
> 		memcpy(client_info->mac_dst, arp->mac_src, ETH_ALEN);
> 		client_info->ntt = 1;
>-- 
>1.7.0.1

---
	-Jay Vosburgh, IBM Linux Technology Center, fubar@...ibm.com
--
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