Netdev List
 help / color / mirror / Atom feed
* [PATCH net-next] net-bnx2x: dont reload on GRO change
@ 2013-05-18 17:14 Eric Dumazet
  2013-05-18 21:36 ` Dmitry Kravkov
  0 siblings, 1 reply; 3+ messages in thread
From: Eric Dumazet @ 2013-05-18 17:14 UTC (permalink / raw)
  To: David Miller; +Cc: netdev, Dmitry Kravkov, Eilon Greenstein

From: Eric Dumazet <edumazet@google.com>

bnx2x_set_features() forces a driver reload if GRO setting is changed.

A reload makes the ethernet port unresponsive for about 5 seconds.

This is not needed in the common case LRO is enabled, as LRO
(TPA_ENABLE_FLAG) has precedence over GRO (GRO_ENABLE_FLAG)
    
Tested:
 Verified that "ethtool -K eth0 gro {on|off}" doesn't blackout
 the NIC anymore
    
Google-Bug-Id: 8440442
Signed-off-by: Eric Dumazet <edumazet@google.com>
Cc: Dmitry Kravkov <dmitry@broadcom.com>
---
 drivers/net/ethernet/broadcom/bnx2x/bnx2x_cmn.c |   13 ++++++++++---
 1 file changed, 10 insertions(+), 3 deletions(-)

diff --git a/drivers/net/ethernet/broadcom/bnx2x/bnx2x_cmn.c b/drivers/net/ethernet/broadcom/bnx2x/bnx2x_cmn.c
index b8fbe26..6cc5101 100644
--- a/drivers/net/ethernet/broadcom/bnx2x/bnx2x_cmn.c
+++ b/drivers/net/ethernet/broadcom/bnx2x/bnx2x_cmn.c
@@ -4599,6 +4599,7 @@ int bnx2x_set_features(struct net_device *dev, netdev_features_t features)
 {
 	struct bnx2x *bp = netdev_priv(dev);
 	u32 flags = bp->flags;
+	u32 changes;
 	bool bnx2x_reload = false;
 
 	if (features & NETIF_F_LRO)
@@ -4623,10 +4624,16 @@ int bnx2x_set_features(struct net_device *dev, netdev_features_t features)
 		}
 	}
 
-	if (flags ^ bp->flags) {
-		bp->flags = flags;
+	changes = flags ^ bp->flags;
+
+	/* if GRO is changed while LRO is enabled, dont force a reload */
+	if ((changes & GRO_ENABLE_FLAG) && (flags & TPA_ENABLE_FLAG))
+		changes &= ~GRO_ENABLE_FLAG;
+
+	if (changes)
 		bnx2x_reload = true;
-	}
+
+	bp->flags = flags;
 
 	if (bnx2x_reload) {
 		if (bp->recovery_state == BNX2X_RECOVERY_DONE)

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

* RE: [PATCH net-next] net-bnx2x: dont reload on GRO change
  2013-05-18 17:14 [PATCH net-next] net-bnx2x: dont reload on GRO change Eric Dumazet
@ 2013-05-18 21:36 ` Dmitry Kravkov
  2013-05-20  7:17   ` David Miller
  0 siblings, 1 reply; 3+ messages in thread
From: Dmitry Kravkov @ 2013-05-18 21:36 UTC (permalink / raw)
  To: Eric Dumazet, David Miller; +Cc: netdev, Eilon Greenstein

> -----Original Message-----
> From: Eric Dumazet [mailto:eric.dumazet@gmail.com]
> Sent: Saturday, May 18, 2013 8:15 PM
> To: David Miller
> Cc: netdev; Dmitry Kravkov; Eilon Greenstein
> Subject: [PATCH net-next] net-bnx2x: dont reload on GRO change
> 
> From: Eric Dumazet <edumazet@google.com>
> 
> bnx2x_set_features() forces a driver reload if GRO setting is changed.
> 
> A reload makes the ethernet port unresponsive for about 5 seconds.
> 
> This is not needed in the common case LRO is enabled, as LRO
> (TPA_ENABLE_FLAG) has precedence over GRO (GRO_ENABLE_FLAG)
> 
> Tested:
>  Verified that "ethtool -K eth0 gro {on|off}" doesn't blackout
>  the NIC anymore
> 
> Google-Bug-Id: 8440442
> Signed-off-by: Eric Dumazet <edumazet@google.com>
> Cc: Dmitry Kravkov <dmitry@broadcom.com>
> ---
>  drivers/net/ethernet/broadcom/bnx2x/bnx2x_cmn.c |   13 ++++++++++---
>  1 file changed, 10 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/net/ethernet/broadcom/bnx2x/bnx2x_cmn.c b/drivers/net/ethernet/broadcom/bnx2x/bnx2x_cmn.c
> index b8fbe26..6cc5101 100644
> --- a/drivers/net/ethernet/broadcom/bnx2x/bnx2x_cmn.c
> +++ b/drivers/net/ethernet/broadcom/bnx2x/bnx2x_cmn.c
> @@ -4599,6 +4599,7 @@ int bnx2x_set_features(struct net_device *dev, netdev_features_t features)
>  {
>  	struct bnx2x *bp = netdev_priv(dev);
>  	u32 flags = bp->flags;
> +	u32 changes;
>  	bool bnx2x_reload = false;
> 
>  	if (features & NETIF_F_LRO)
> @@ -4623,10 +4624,16 @@ int bnx2x_set_features(struct net_device *dev, netdev_features_t features)
>  		}
>  	}
> 
> -	if (flags ^ bp->flags) {
> -		bp->flags = flags;
> +	changes = flags ^ bp->flags;
> +
> +	/* if GRO is changed while LRO is enabled, dont force a reload */
> +	if ((changes & GRO_ENABLE_FLAG) && (flags & TPA_ENABLE_FLAG))
> +		changes &= ~GRO_ENABLE_FLAG;
> +
> +	if (changes)
>  		bnx2x_reload = true;
> -	}
> +
> +	bp->flags = flags;
> 
>  	if (bnx2x_reload) {
>  		if (bp->recovery_state == BNX2X_RECOVERY_DONE)
> 
>
Looks good! Thanks, Eric

Acked-by: Dmitry Kravkov <dmitry@broadcom.com>


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

* Re: [PATCH net-next] net-bnx2x: dont reload on GRO change
  2013-05-18 21:36 ` Dmitry Kravkov
@ 2013-05-20  7:17   ` David Miller
  0 siblings, 0 replies; 3+ messages in thread
From: David Miller @ 2013-05-20  7:17 UTC (permalink / raw)
  To: dmitry; +Cc: eric.dumazet, netdev, eilong

From: "Dmitry Kravkov" <dmitry@broadcom.com>
Date: Sat, 18 May 2013 21:36:07 +0000

>> -----Original Message-----
>> From: Eric Dumazet [mailto:eric.dumazet@gmail.com]
>> Sent: Saturday, May 18, 2013 8:15 PM
>> To: David Miller
>> Cc: netdev; Dmitry Kravkov; Eilon Greenstein
>> Subject: [PATCH net-next] net-bnx2x: dont reload on GRO change
>> 
>> From: Eric Dumazet <edumazet@google.com>
>> 
>> bnx2x_set_features() forces a driver reload if GRO setting is changed.
>> 
>> A reload makes the ethernet port unresponsive for about 5 seconds.
>> 
>> This is not needed in the common case LRO is enabled, as LRO
>> (TPA_ENABLE_FLAG) has precedence over GRO (GRO_ENABLE_FLAG)
>> 
>> Tested:
>>  Verified that "ethtool -K eth0 gro {on|off}" doesn't blackout
>>  the NIC anymore
>> 
>> Google-Bug-Id: 8440442
>> Signed-off-by: Eric Dumazet <edumazet@google.com>
...
> Looks good! Thanks, Eric
> 
> Acked-by: Dmitry Kravkov <dmitry@broadcom.com>
> 

Applied, thanks.

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

end of thread, other threads:[~2013-05-20  7:17 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-05-18 17:14 [PATCH net-next] net-bnx2x: dont reload on GRO change Eric Dumazet
2013-05-18 21:36 ` Dmitry Kravkov
2013-05-20  7:17   ` David Miller

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox