From mboxrd@z Thu Jan 1 00:00:00 1970 From: Jay Vosburgh Subject: Re: [PATCH] bonding: If IP route look-up to send an ARP fails, mark in bonding structure as no ARP sent. Date: Wed, 20 Nov 2013 17:18:19 -0800 Message-ID: <12668.1384996699@death.nxdomain> References: <528D5980.3040309@oracle.com> Cc: netdev@vger.kernel.org To: rama nichanamatlu Return-path: Received: from e7.ny.us.ibm.com ([32.97.182.137]:34433 "EHLO e7.ny.us.ibm.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753481Ab3KUBSY (ORCPT ); Wed, 20 Nov 2013 20:18:24 -0500 Received: from /spool/local by e7.ny.us.ibm.com with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted for from ; Wed, 20 Nov 2013 20:18:23 -0500 Received: from b01cxnp22034.gho.pok.ibm.com (b01cxnp22034.gho.pok.ibm.com [9.57.198.24]) by d01dlp01.pok.ibm.com (Postfix) with ESMTP id 0099838C8045 for ; Wed, 20 Nov 2013 20:18:20 -0500 (EST) Received: from d01av05.pok.ibm.com (d01av05.pok.ibm.com [9.56.224.195]) by b01cxnp22034.gho.pok.ibm.com (8.13.8/8.13.8/NCO v10.0) with ESMTP id rAL1ILqE62849060 for ; Thu, 21 Nov 2013 01:18:21 GMT Received: from d01av05.pok.ibm.com (localhost [127.0.0.1]) by d01av05.pok.ibm.com (8.14.4/8.14.4/NCO v10.0 AVout) with ESMTP id rAL1ILlp016607 for ; Wed, 20 Nov 2013 20:18:21 -0500 In-reply-to: <528D5980.3040309@oracle.com> Sender: netdev-owner@vger.kernel.org List-ID: rama nichanamatlu wrote: >During the creation of VLAN's atop bonding the underlying interfaces are >made part of VLAN's, and at the same bonding driver gets aware that VLAN's >exists above it and hence would consult IP routing for every ARP to be >sent to determine the route which tells bonding driver the correct VLAN >tag to attach to the outgoing ARP packet. But, during the VLAN creation >when vlan driver puts the underlying interface into default vlan and then >actual vlan, in-between this if bonding driver consults the IP for a >route, IP fails to provide a correct route and upon which bonding driver >drops the ARP packet. ARP monitor when it >comes around next time, sees no ARP response and fails-over to the next >available slave. Consulting for a IP route, ip_route_output(),happens in >bond_arp_send_all(). > >To prevent this false fail-over, when bonding driver fails to send an ARP >out it marks in its private structure, bonding{}, not to expect an ARP >response, when ARP monitor comes around next time ARP sending will be >tried again. > >Extensively tested in a VM environment; sr-iov intf->bonding intf->vlan >intf. All virtual interfaces created at boot time. First, this patch appears to be for an older kernel, as the current mainline code is substantially different (e.g., master_ip is no longer used). Second, won't this methodology mask legitimate failures, such as when a single arp_ip_target specifies a destination that is not ever reachable? I.e., would specifying a permanently unreachable IP address as the arp_ip_target cause all slaves to always stay up (because no ARPs will ever be sent), even if no ARP replies are ever received? -J >Orabug: 17172660 >Signed-off-by: Venkat Venkatsubra >Signed-off-by: Rama Nichanamatlu >--- > drivers/net/bonding/bond_main.c | 13 ++++++++----- > drivers/net/bonding/bonding.h | 1 + > 2 files changed, 9 insertions(+), 5 deletions(-) > >diff --git a/drivers/net/bonding/bond_main.c >b/drivers/net/bonding/bond_main.c >index dde6b4a..d475161 100644 >--- a/drivers/net/bonding/bond_main.c >+++ b/drivers/net/bonding/bond_main.c >@@ -2661,7 +2661,7 @@ static int bond_has_this_ip(struct bonding *bond, >__be32 ip) > * switches in VLAN mode (especially if ports are configured as > * "native" to a VLAN) might not pass non-tagged frames. > */ >-static void bond_arp_send(struct net_device *slave_dev, int arp_op, >__be32 dest_ip, __be32 src_ip, unsigned short vlan_id) >+static void bond_arp_send(struct bonding *bond, struct net_device >*slave_dev, int arp_op, __be32 dest_ip, __be32 src_ip, unsigned short >vlan_id) > { > struct sk_buff *skb; > @@ -2683,6 +2683,7 @@ static void bond_arp_send(struct net_device >*slave_dev, int arp_op, __be32 dest_ > } > } > arp_xmit(skb); >+ bond->arp_sent=true; > } > @@ -2700,7 +2701,7 @@ static void bond_arp_send_all(struct bonding >*bond, struct slave *slave) > pr_debug("basa: target %x\n", targets[i]); > if (!bond->vlgrp) { > pr_debug("basa: empty vlan: arp_send\n"); >- bond_arp_send(slave->dev, ARPOP_REQUEST, targets[i], >+ bond_arp_send(bond, slave->dev, ARPOP_REQUEST, targets[i], > bond->master_ip, 0); > continue; > } >@@ -2726,7 +2727,7 @@ static void bond_arp_send_all(struct bonding *bond, >struct slave *slave) > if (rt->dst.dev == bond->dev) { > ip_rt_put(rt); > pr_debug("basa: rtdev == bond->dev: arp_send\n"); >- bond_arp_send(slave->dev, ARPOP_REQUEST, targets[i], >+ bond_arp_send(bond, slave->dev, ARPOP_REQUEST, targets[i], > bond->master_ip, 0); > continue; > } >@@ -2744,7 +2745,7 @@ static void bond_arp_send_all(struct bonding *bond, >struct slave *slave) > if (vlan_id) { > ip_rt_put(rt); >- bond_arp_send(slave->dev, ARPOP_REQUEST, targets[i], >+ bond_arp_send(bond, slave->dev, ARPOP_REQUEST, targets[i], > vlan->vlan_ip, vlan_id); > continue; > } >@@ -3206,7 +3207,7 @@ void bond_activebackup_arp_mon(struct work_struct >*work) > should_notify_peers = bond_should_notify_peers(bond); > - if (bond_ab_arp_inspect(bond, delta_in_ticks)) { >+ if (bond->arp_sent && bond_ab_arp_inspect(bond, delta_in_ticks)) { > read_unlock(&bond->lock); > rtnl_lock(); > read_lock(&bond->lock); >@@ -3218,6 +3219,7 @@ void bond_activebackup_arp_mon(struct work_struct >*work) > read_lock(&bond->lock); > } > + bond->arp_sent=false; > bond_ab_arp_probe(bond); > re_arm: >@@ -4425,6 +4427,7 @@ static void bond_setup(struct net_device *bond_dev) > bond_dev->hw_features &= ~(NETIF_F_ALL_CSUM & ~NETIF_F_NO_CSUM); > bond_dev->features |= bond_dev->hw_features; >+ bond->arp_sent=false; > } > static void bond_work_cancel_all(struct bonding *bond) >diff --git a/drivers/net/bonding/bonding.h b/drivers/net/bonding/bonding.h >index e9a3c56..3878bbd 100644 >--- a/drivers/net/bonding/bonding.h >+++ b/drivers/net/bonding/bonding.h >@@ -253,6 +253,7 @@ struct bonding { > /* debugging suport via debugfs */ > struct dentry *debug_dir; > #endif /* CONFIG_DEBUG_FS */ >+ bool arp_sent; > }; > #define bond_slave_get_rcu(dev) \ >-- >1.8.2.1 --- -Jay Vosburgh, IBM Linux Technology Center, fubar@us.ibm.com