[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date: Wed, 20 Nov 2013 17:18:19 -0800
From: Jay Vosburgh <fubar@...ibm.com>
To: rama nichanamatlu <rama.nichanamatlu@...cle.com>
cc: netdev@...r.kernel.org
Subject: Re: [PATCH] bonding: If IP route look-up to send an ARP fails, mark in bonding structure as no ARP sent.
rama nichanamatlu <rama.nichanamatlu@...cle.com> 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 <venkat.x.venkatsubra@...cle.com>
>Signed-off-by: Rama Nichanamatlu <rama.nichanamatlu@...cle.com>
>---
> 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@...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
Powered by blists - more mailing lists