netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Jarod Wilson <jarod@redhat.com>
To: linux-kernel@vger.kernel.org
Cc: Uwe Koziolek <uwe.koziolek@redknee.com>,
	Jay Vosburgh <jay.vosburgh@canonical.com>,
	Andy Gospodarek <gospo@cumulusnetworks.com>,
	Veaceslav Falico <vfalico@gmail.com>,
	netdev@vger.kernel.org, Jarod Wilson <jarod@redhat.com>
Subject: [PATCH v4] net/bonding: send arp in interval if no active slave
Date: Tue,  6 Oct 2015 15:53:17 -0400	[thread overview]
Message-ID: <1444161197-38442-1-git-send-email-jarod@redhat.com> (raw)
In-Reply-To: <56094137.9030206@redhat.com>

From: Uwe Koziolek <uwe.koziolek@redknee.com>

With some very finicky switch hardware, active backup bonding can get into
a situation where we play ping-pong between interfaces, trying to get one
to come up as the active slave. There seems to be an issue with the
switch's arp replies either taking too long, or simply getting lost, so we
wind up unable to get any interface up and active. Sometimes, the issue
sorts itself out after a while, sometimes it doesn't.

Testing with num_grat_arp has proven fruitless, but sending an additional
arp on curr_arp_slave if we're still in the arp_interval timeslice in
bond_ab_arp_probe(), has shown to produce 100% reliability in testing with
this hardware combination.

[jarod: manufacturing of changelog, addition of modparam gating]
CC: Jay Vosburgh <jay.vosburgh@canonical.com>
CC: Andy Gospodarek <gospo@cumulusnetworks.com>
CC: Veaceslav Falico <vfalico@gmail.com>
CC: netdev@vger.kernel.org
Signed-off-by: Uwe Koziolek <uwe.koziolek@redknee.com>
Signed-off-by: Jarod Wilson <jarod@redhat.com>
---
v2: add code comment as to why change is needed
v3: fix wrapping of comments
v4: [jarod] add module parameter gating of code addition

 drivers/net/bonding/bond_main.c | 24 ++++++++++++++++++++++++
 include/net/bonding.h           |  1 +
 2 files changed, 25 insertions(+)

diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
index 90f2615..72ab512 100644
--- a/drivers/net/bonding/bond_main.c
+++ b/drivers/net/bonding/bond_main.c
@@ -95,6 +95,7 @@ static int miimon;
 static int updelay;
 static int downdelay;
 static int use_carrier	= 1;
+static int arp_slow_switch;
 static char *mode;
 static char *primary;
 static char *primary_reselect;
@@ -133,6 +134,10 @@ MODULE_PARM_DESC(downdelay, "Delay before considering link down, "
 module_param(use_carrier, int, 0);
 MODULE_PARM_DESC(use_carrier, "Use netif_carrier_ok (vs MII ioctls) in miimon; "
 			      "0 for off, 1 for on (default)");
+module_param(arp_slow_switch, int, 0);
+MODULE_PARM_DESC(arp_slow_switch, "Do extra arp checks for switches with arp "
+				  "caches that are slow to update; "
+				  "0 for off (default), 1 for on");
 module_param(mode, charp, 0);
 MODULE_PARM_DESC(mode, "Mode of operation; 0 for balance-rr, "
 		       "1 for active-backup, 2 for balance-xor, "
@@ -2793,6 +2798,18 @@ static bool bond_ab_arp_probe(struct bonding *bond)
 			return should_notify_rtnl;
 	}
 
+	/* Sometimes the forwarding tables of the switches are not update
+	 * fast enough, so the first arp response after a slave change is
+	 * received on the wrong slave.
+	 *
+	 * The arp requests will be retried 2 times on the same slave.
+	 */
+	if (arp_slow_switch &&
+	    bond_time_in_interval(bond, curr_arp_slave->last_link_up, 2)) {
+		bond_arp_send_all(bond, curr_arp_slave);
+		return should_notify_rtnl;
+	}
+
 	bond_set_slave_inactive_flags(curr_arp_slave, BOND_SLAVE_NOTIFY_LATER);
 
 	bond_for_each_slave_rcu(bond, slave, iter) {
@@ -4280,6 +4297,12 @@ static int bond_check_params(struct bond_params *params)
 		use_carrier = 1;
 	}
 
+	if ((arp_slow_switch != 0) && (arp_slow_switch != 1)) {
+		pr_warn("Warning: arp_slow_switch module parameter (%d), not of valid value (0/1), so it was set to 1\n",
+			arp_slow_switch);
+		arp_slow_switch = 1;
+	}
+
 	if (num_peer_notif < 0 || num_peer_notif > 255) {
 		pr_warn("Warning: num_grat_arp/num_unsol_na (%d) not in range 0-255 so it was reset to 1\n",
 			num_peer_notif);
@@ -4516,6 +4539,7 @@ static int bond_check_params(struct bond_params *params)
 	params->updelay = updelay;
 	params->downdelay = downdelay;
 	params->use_carrier = use_carrier;
+	params->arp_slow_switch = arp_slow_switch;
 	params->lacp_fast = lacp_fast;
 	params->primary[0] = 0;
 	params->primary_reselect = primary_reselect_value;
diff --git a/include/net/bonding.h b/include/net/bonding.h
index c1740a2..208d31c 100644
--- a/include/net/bonding.h
+++ b/include/net/bonding.h
@@ -120,6 +120,7 @@ struct bond_params {
 	int arp_validate;
 	int arp_all_targets;
 	int use_carrier;
+	int arp_slow_switch;
 	int fail_over_mac;
 	int updelay;
 	int downdelay;
-- 
1.8.3.1

  reply	other threads:[~2015-10-06 19:53 UTC|newest]

Thread overview: 22+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-08-17 16:23 [PATCH] net/bonding: send arp in interval if no active slave Jarod Wilson
2015-08-17 16:55 ` Veaceslav Falico
2015-08-17 17:12   ` Jarod Wilson
2015-08-17 18:56     ` Uwe Koziolek
2015-08-17 19:14       ` Jay Vosburgh
2015-08-17 20:51         ` Uwe Koziolek
2015-08-31 22:21           ` Jarod Wilson
2015-09-01 23:15             ` Uwe Koziolek
2015-09-01 15:41           ` Andy Gospodarek
2015-09-01 23:10             ` Uwe Koziolek
2015-09-03 15:05               ` Jay Vosburgh
2015-09-04 11:04                 ` Uwe Koziolek
2015-09-28 13:31                   ` Jarod Wilson
2015-10-06 19:53                     ` Jarod Wilson [this message]
2015-10-06 19:58                       ` [PATCH v4] " Jarod Wilson
2015-10-07 12:03                       ` Nikolay Aleksandrov
2015-10-07 13:29                         ` Jarod Wilson
2015-10-09 14:36                           ` Jarod Wilson
2015-10-09 15:25                             ` Nikolay Aleksandrov
2015-10-09 15:31                             ` Jay Vosburgh
2015-10-12 15:33                               ` Jarod Wilson
2015-10-30 18:59                           ` Uwe Koziolek

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1444161197-38442-1-git-send-email-jarod@redhat.com \
    --to=jarod@redhat.com \
    --cc=gospo@cumulusnetworks.com \
    --cc=jay.vosburgh@canonical.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=netdev@vger.kernel.org \
    --cc=uwe.koziolek@redknee.com \
    --cc=vfalico@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).