Patchwork 3c59x: shorten timer period for slave devices

login
register
mail settings
Submitter Eric Dumazet
Date Feb. 14, 2012, 8:27 p.m.
Message ID <1329251229.2555.10.camel@edumazet-laptop>
Download mbox | patch
Permalink /patch/141184/
State Accepted
Delegated to: David Miller
Headers show

Comments

Eric Dumazet - Feb. 14, 2012, 8:27 p.m.
Jean Delvare reported bonding on top of 3c59x adapters was not detecting
network cable removal fast enough.

3c59x indeed uses a 60 seconds timer to check link status if carrier is
on, and 5 seconds if carrier is off.

This patch reduces timer period to 5 seconds if device is a bonding
slave.

Reported-by: Jean Delvare <jdelvare@suse.de>
Acked-by: Jean Delvare <jdelvare@suse.de>
Acked-by: Steffen Klassert <steffen.klassert@secunet.com>
Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
---
 drivers/net/ethernet/3com/3c59x.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)



--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Dan Williams - Feb. 14, 2012, 8:51 p.m.
On Tue, 2012-02-14 at 21:27 +0100, Eric Dumazet wrote:
> Jean Delvare reported bonding on top of 3c59x adapters was not detecting
> network cable removal fast enough.
> 
> 3c59x indeed uses a 60 seconds timer to check link status if carrier is
> on, and 5 seconds if carrier is off.
> 
> This patch reduces timer period to 5 seconds if device is a bonding
> slave.

Maybe for posterity give some rationale as to why we feel we can reduce
it to 5 seconds for slaves instead of reducing the timer period in
general?  If you weren't party to this discussion that won't be apparent
from the commit log.

Dan

> Reported-by: Jean Delvare <jdelvare@suse.de>
> Acked-by: Jean Delvare <jdelvare@suse.de>
> Acked-by: Steffen Klassert <steffen.klassert@secunet.com>
> Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
> ---
>  drivers/net/ethernet/3com/3c59x.c |    2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/net/ethernet/3com/3c59x.c b/drivers/net/ethernet/3com/3c59x.c
> index 1282f04..e463d10 100644
> --- a/drivers/net/ethernet/3com/3c59x.c
> +++ b/drivers/net/ethernet/3com/3c59x.c
> @@ -1841,7 +1841,7 @@ vortex_timer(unsigned long data)
>  		ok = 1;
>  	}
>  
> -	if (!netif_carrier_ok(dev))
> +	if (dev->flags & IFF_SLAVE || !netif_carrier_ok(dev))
>  		next_tick = 5*HZ;
>  
>  	if (vp->medialock)
> 
> 
> --
> To unsubscribe from this list: send the line "unsubscribe netdev" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html


--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Eric Dumazet - Feb. 14, 2012, 9:02 p.m.
Le mardi 14 février 2012 à 14:51 -0600, Dan Williams a écrit :
> On Tue, 2012-02-14 at 21:27 +0100, Eric Dumazet wrote:
> > Jean Delvare reported bonding on top of 3c59x adapters was not detecting
> > network cable removal fast enough.
> > 
> > 3c59x indeed uses a 60 seconds timer to check link status if carrier is
> > on, and 5 seconds if carrier is off.
> > 
> > This patch reduces timer period to 5 seconds if device is a bonding
> > slave.
> 
> Maybe for posterity give some rationale as to why we feel we can reduce
> it to 5 seconds for slaves instead of reducing the timer period in
> general?  If you weren't party to this discussion that won't be apparent
> from the commit log.

Apparently, Andrew Morton considered firing a timer every 5 seconds was
too expensive. I am not sure we want to state this (probably old fact)
in the changelog. Anyway are these slow NICS still used in 2011 ? ;)



--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
David Miller - Feb. 14, 2012, 9:28 p.m.
From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Tue, 14 Feb 2012 22:02:56 +0100

> Le mardi 14 février 2012 à 14:51 -0600, Dan Williams a écrit :
>> On Tue, 2012-02-14 at 21:27 +0100, Eric Dumazet wrote:
>> > Jean Delvare reported bonding on top of 3c59x adapters was not detecting
>> > network cable removal fast enough.
>> > 
>> > 3c59x indeed uses a 60 seconds timer to check link status if carrier is
>> > on, and 5 seconds if carrier is off.
>> > 
>> > This patch reduces timer period to 5 seconds if device is a bonding
>> > slave.
>> 
>> Maybe for posterity give some rationale as to why we feel we can reduce
>> it to 5 seconds for slaves instead of reducing the timer period in
>> general?  If you weren't party to this discussion that won't be apparent
>> from the commit log.
> 
> Apparently, Andrew Morton considered firing a timer every 5 seconds was
> too expensive. I am not sure we want to state this (probably old fact)
> in the changelog. Anyway are these slow NICS still used in 2011 ? ;)

At least Jean Delvare is :-)

I'm going to apply this as-is, if you configure bonding on one of these
chips you expect it to work.  And working is more important than optimal.
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Patch

diff --git a/drivers/net/ethernet/3com/3c59x.c b/drivers/net/ethernet/3com/3c59x.c
index 1282f04..e463d10 100644
--- a/drivers/net/ethernet/3com/3c59x.c
+++ b/drivers/net/ethernet/3com/3c59x.c
@@ -1841,7 +1841,7 @@  vortex_timer(unsigned long data)
 		ok = 1;
 	}
 
-	if (!netif_carrier_ok(dev))
+	if (dev->flags & IFF_SLAVE || !netif_carrier_ok(dev))
 		next_tick = 5*HZ;
 
 	if (vp->medialock)