netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [net-next PATCH v3] bonding: start slaves with link down for ARP monitor
@ 2012-04-17 12:02 Michal Kubecek
  2012-04-18 17:45 ` Jay Vosburgh
  2012-04-18 19:19 ` Flavio Leitner
  0 siblings, 2 replies; 4+ messages in thread
From: Michal Kubecek @ 2012-04-17 12:02 UTC (permalink / raw)
  To: netdev; +Cc: Jay Vosburgh, Andy Gospodarek, Flavio Leitner

Initialize slave device link state as down if ARP monitor is
active and net_carrier_ok() returns zero. Also shift initial
value of its last_arp_tx so that it doesn't immediately cause
fake detection of "up" state.

When ARP monitoring is used, initializing the slave device with
up link state can cause ARP monitor to detect link failure
before the device is really up (with igb driver, this can take
more than two seconds).

Signed-off-by: Michal Kubecek <mkubecek@suse.cz>
---
 drivers/net/bonding/bond_main.c |   33 +++++++++++++++++++++------------
 1 files changed, 21 insertions(+), 12 deletions(-)

diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
index bb92899..44e6a64 100644
--- a/drivers/net/bonding/bond_main.c
+++ b/drivers/net/bonding/bond_main.c
@@ -1726,7 +1726,8 @@ int bond_enslave(struct net_device *bond_dev, struct net_device *slave_dev)
 
 	read_lock(&bond->lock);
 
-	new_slave->last_arp_rx = jiffies;
+	new_slave->last_arp_rx = jiffies -
+		(msecs_to_jiffies(bond->params.arp_interval) + 1);
 
 	if (bond->params.miimon && !bond->params.use_carrier) {
 		link_reporting = bond_check_dev_link(bond, slave_dev, 1);
@@ -1751,22 +1752,30 @@ int bond_enslave(struct net_device *bond_dev, struct net_device *slave_dev)
 	}
 
 	/* check for initial state */
-	if (!bond->params.miimon ||
-	    (bond_check_dev_link(bond, slave_dev, 0) == BMSR_LSTATUS)) {
-		if (bond->params.updelay) {
-			pr_debug("Initial state of slave_dev is BOND_LINK_BACK\n");
-			new_slave->link  = BOND_LINK_BACK;
-			new_slave->delay = bond->params.updelay;
+	if (bond->params.miimon) {
+		if (bond_check_dev_link(bond, slave_dev, 0) == BMSR_LSTATUS) {
+			if (bond->params.updelay) {
+				new_slave->link = BOND_LINK_BACK;
+				new_slave->delay = bond->params.updelay;
+			} else {
+				new_slave->link = BOND_LINK_UP;
+			}
 		} else {
-			pr_debug("Initial state of slave_dev is BOND_LINK_UP\n");
-			new_slave->link  = BOND_LINK_UP;
+			new_slave->link = BOND_LINK_DOWN;
 		}
-		new_slave->jiffies = jiffies;
+	} else if (bond->params.arp_interval) {
+		new_slave->link = (netif_carrier_ok(slave_dev) ?
+			BOND_LINK_UP : BOND_LINK_DOWN);
 	} else {
-		pr_debug("Initial state of slave_dev is BOND_LINK_DOWN\n");
-		new_slave->link  = BOND_LINK_DOWN;
+		new_slave->link = BOND_LINK_UP;
 	}
 
+	if (new_slave->link != BOND_LINK_DOWN)
+		new_slave->jiffies = jiffies;
+	pr_debug("Initial state of slave_dev is BOND_LINK_%s\n",
+		new_slave->link == BOND_LINK_DOWN ? "DOWN" :
+			(new_slave->link == BOND_LINK_UP ? "UP" : "BACK"));
+
 	bond_update_speed_duplex(new_slave);
 
 	if (USES_PRIMARY(bond->params.mode) && bond->params.primary[0]) {
-- 
1.7.7

^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [net-next PATCH v3] bonding: start slaves with link down for ARP monitor
  2012-04-17 12:02 [net-next PATCH v3] bonding: start slaves with link down for ARP monitor Michal Kubecek
@ 2012-04-18 17:45 ` Jay Vosburgh
  2012-04-18 19:19 ` Flavio Leitner
  1 sibling, 0 replies; 4+ messages in thread
From: Jay Vosburgh @ 2012-04-18 17:45 UTC (permalink / raw)
  To: Michal Kubecek; +Cc: netdev, Andy Gospodarek, Flavio Leitner

Michal Kubecek <mkubecek@suse.cz> wrote:

>Initialize slave device link state as down if ARP monitor is
>active and net_carrier_ok() returns zero. Also shift initial
>value of its last_arp_tx so that it doesn't immediately cause
>fake detection of "up" state.
>
>When ARP monitoring is used, initializing the slave device with
>up link state can cause ARP monitor to detect link failure
>before the device is really up (with igb driver, this can take
>more than two seconds).
>
>Signed-off-by: Michal Kubecek <mkubecek@suse.cz>

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

>---
> drivers/net/bonding/bond_main.c |   33 +++++++++++++++++++++------------
> 1 files changed, 21 insertions(+), 12 deletions(-)
>
>diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
>index bb92899..44e6a64 100644
>--- a/drivers/net/bonding/bond_main.c
>+++ b/drivers/net/bonding/bond_main.c
>@@ -1726,7 +1726,8 @@ int bond_enslave(struct net_device *bond_dev, struct net_device *slave_dev)
>
> 	read_lock(&bond->lock);
>
>-	new_slave->last_arp_rx = jiffies;
>+	new_slave->last_arp_rx = jiffies -
>+		(msecs_to_jiffies(bond->params.arp_interval) + 1);
>
> 	if (bond->params.miimon && !bond->params.use_carrier) {
> 		link_reporting = bond_check_dev_link(bond, slave_dev, 1);
>@@ -1751,22 +1752,30 @@ int bond_enslave(struct net_device *bond_dev, struct net_device *slave_dev)
> 	}
>
> 	/* check for initial state */
>-	if (!bond->params.miimon ||
>-	    (bond_check_dev_link(bond, slave_dev, 0) == BMSR_LSTATUS)) {
>-		if (bond->params.updelay) {
>-			pr_debug("Initial state of slave_dev is BOND_LINK_BACK\n");
>-			new_slave->link  = BOND_LINK_BACK;
>-			new_slave->delay = bond->params.updelay;
>+	if (bond->params.miimon) {
>+		if (bond_check_dev_link(bond, slave_dev, 0) == BMSR_LSTATUS) {
>+			if (bond->params.updelay) {
>+				new_slave->link = BOND_LINK_BACK;
>+				new_slave->delay = bond->params.updelay;
>+			} else {
>+				new_slave->link = BOND_LINK_UP;
>+			}
> 		} else {
>-			pr_debug("Initial state of slave_dev is BOND_LINK_UP\n");
>-			new_slave->link  = BOND_LINK_UP;
>+			new_slave->link = BOND_LINK_DOWN;
> 		}
>-		new_slave->jiffies = jiffies;
>+	} else if (bond->params.arp_interval) {
>+		new_slave->link = (netif_carrier_ok(slave_dev) ?
>+			BOND_LINK_UP : BOND_LINK_DOWN);
> 	} else {
>-		pr_debug("Initial state of slave_dev is BOND_LINK_DOWN\n");
>-		new_slave->link  = BOND_LINK_DOWN;
>+		new_slave->link = BOND_LINK_UP;
> 	}
>
>+	if (new_slave->link != BOND_LINK_DOWN)
>+		new_slave->jiffies = jiffies;
>+	pr_debug("Initial state of slave_dev is BOND_LINK_%s\n",
>+		new_slave->link == BOND_LINK_DOWN ? "DOWN" :
>+			(new_slave->link == BOND_LINK_UP ? "UP" : "BACK"));
>+
> 	bond_update_speed_duplex(new_slave);
>
> 	if (USES_PRIMARY(bond->params.mode) && bond->params.primary[0]) {
>-- 
>1.7.7
>

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [net-next PATCH v3] bonding: start slaves with link down for ARP monitor
  2012-04-17 12:02 [net-next PATCH v3] bonding: start slaves with link down for ARP monitor Michal Kubecek
  2012-04-18 17:45 ` Jay Vosburgh
@ 2012-04-18 19:19 ` Flavio Leitner
  2012-04-19 19:25   ` David Miller
  1 sibling, 1 reply; 4+ messages in thread
From: Flavio Leitner @ 2012-04-18 19:19 UTC (permalink / raw)
  To: Michal Kubecek; +Cc: netdev, Jay Vosburgh, Andy Gospodarek

On Tue, 17 Apr 2012 14:02:06 +0200
Michal Kubecek <mkubecek@suse.cz> wrote:

> Initialize slave device link state as down if ARP monitor is
> active and net_carrier_ok() returns zero. Also shift initial
> value of its last_arp_tx so that it doesn't immediately cause
> fake detection of "up" state.
> 
> When ARP monitoring is used, initializing the slave device with
> up link state can cause ARP monitor to detect link failure
> before the device is really up (with igb driver, this can take
> more than two seconds).
> 
> Signed-off-by: Michal Kubecek <mkubecek@suse.cz>

Signed-off-by: Flavio Leitner <fbl@redhat.com>

fbl

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [net-next PATCH v3] bonding: start slaves with link down for ARP monitor
  2012-04-18 19:19 ` Flavio Leitner
@ 2012-04-19 19:25   ` David Miller
  0 siblings, 0 replies; 4+ messages in thread
From: David Miller @ 2012-04-19 19:25 UTC (permalink / raw)
  To: fbl; +Cc: mkubecek, netdev, fubar, andy

From: Flavio Leitner <fbl@redhat.com>
Date: Wed, 18 Apr 2012 16:19:12 -0300

> On Tue, 17 Apr 2012 14:02:06 +0200
> Michal Kubecek <mkubecek@suse.cz> wrote:
> 
>> Initialize slave device link state as down if ARP monitor is
>> active and net_carrier_ok() returns zero. Also shift initial
>> value of its last_arp_tx so that it doesn't immediately cause
>> fake detection of "up" state.
>> 
>> When ARP monitoring is used, initializing the slave device with
>> up link state can cause ARP monitor to detect link failure
>> before the device is really up (with igb driver, this can take
>> more than two seconds).
>> 
>> Signed-off-by: Michal Kubecek <mkubecek@suse.cz>
> 
> Signed-off-by: Flavio Leitner <fbl@redhat.com>

Applied, thanks everyone.

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2012-04-19 19:26 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-04-17 12:02 [net-next PATCH v3] bonding: start slaves with link down for ARP monitor Michal Kubecek
2012-04-18 17:45 ` Jay Vosburgh
2012-04-18 19:19 ` Flavio Leitner
2012-04-19 19:25   ` David Miller

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).