Patchwork [net-next] net-bnx2x: dont reload on GRO change

login
register
mail settings
Submitter Eric Dumazet
Date May 18, 2013, 5:14 p.m.
Message ID <1368897293.3301.152.camel@edumazet-glaptop>
Download mbox | patch
Permalink /patch/244776/
State Accepted
Delegated to: David Miller
Headers show

Comments

Eric Dumazet - May 18, 2013, 5:14 p.m.
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(-)



--
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
Dmitry Kravkov - May 18, 2013, 9:36 p.m.
> -----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>
David Miller - May 20, 2013, 7:17 a.m.
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.
--
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/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)