Patchwork [5/7] bnx2x: do not set TPA flags and features in bnx2x_init_bp

login
register
mail settings
Submitter Michal Schmidt
Date Aug. 30, 2011, 2:30 p.m.
Message ID <1314714646-3642-6-git-send-email-mschmidt@redhat.com>
Download mbox | patch
Permalink /patch/112336/
State Rejected
Delegated to: David Miller
Headers show

Comments

Michal Schmidt - Aug. 30, 2011, 2:30 p.m.
The .ndo_{set,fix}_features callbacks are sufficient.

Signed-off-by: Michal Schmidt <mschmidt@redhat.com>
---
 drivers/net/ethernet/broadcom/bnx2x/bnx2x_main.c |    9 ---------
 1 files changed, 0 insertions(+), 9 deletions(-)
Vladislav Zolotarov - Aug. 30, 2011, 4:21 p.m.
On Tuesday 30 August 2011 17:30:44 Michal Schmidt wrote:
> The .ndo_{set,fix}_features callbacks are sufficient.
> 
> Signed-off-by: Michal Schmidt <mschmidt@redhat.com>
> ---
>  drivers/net/ethernet/broadcom/bnx2x/bnx2x_main.c |    9 ---------
>  1 files changed, 0 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/net/ethernet/broadcom/bnx2x/bnx2x_main.c
> b/drivers/net/ethernet/broadcom/bnx2x/bnx2x_main.c index 64314f7..617a072
> 100644
> --- a/drivers/net/ethernet/broadcom/bnx2x/bnx2x_main.c
> +++ b/drivers/net/ethernet/broadcom/bnx2x/bnx2x_main.c
> @@ -9752,15 +9752,6 @@ static int __devinit bnx2x_init_bp(struct bnx2x *bp)
>  					"must load devices in order!\n");
> 
>  	bp->multi_mode = multi_mode;
> -
> -	/* Set TPA flags */
> -	if (disable_tpa) {
> -		bp->flags &= ~TPA_ENABLE_FLAG;
> -		bp->dev->features &= ~NETIF_F_LRO;
> -	} else {
> -		bp->flags |= TPA_ENABLE_FLAG;
> -		bp->dev->features |= NETIF_F_LRO;
> -	}
>  	bp->disable_tpa = disable_tpa;
> 
>  	if (CHIP_IS_E1(bp))

NACK

This patch will cause the bnx2x to initialize HW with LRO disabled on the 
first ifup because our code considers the TPA_ENABLE_FLAG when desiding on 
whether LRO is enabled or not. ethtool would still report the LRO on though!

thanks,
vlad

--
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
Michal Schmidt - Aug. 30, 2011, 5:15 p.m.
On 08/30/2011 06:21 PM, Vlad Zolotarov wrote:
> NACK
>
> This patch will cause the bnx2x to initialize HW with LRO disabled on the
> first ifup because our code considers the TPA_ENABLE_FLAG when desiding on
> whether LRO is enabled or not. ethtool would still report the LRO on though!

I see. I thought register_netdevice() would always call 
bnx2x_set_features(), but this is not the case.

Patch 7/7 has a similar problem with RX_VLAN_STRIP_FLAG then.

Michal
--
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_main.c b/drivers/net/ethernet/broadcom/bnx2x/bnx2x_main.c
index 64314f7..617a072 100644
--- a/drivers/net/ethernet/broadcom/bnx2x/bnx2x_main.c
+++ b/drivers/net/ethernet/broadcom/bnx2x/bnx2x_main.c
@@ -9752,15 +9752,6 @@  static int __devinit bnx2x_init_bp(struct bnx2x *bp)
 					"must load devices in order!\n");
 
 	bp->multi_mode = multi_mode;
-
-	/* Set TPA flags */
-	if (disable_tpa) {
-		bp->flags &= ~TPA_ENABLE_FLAG;
-		bp->dev->features &= ~NETIF_F_LRO;
-	} else {
-		bp->flags |= TPA_ENABLE_FLAG;
-		bp->dev->features |= NETIF_F_LRO;
-	}
 	bp->disable_tpa = disable_tpa;
 
 	if (CHIP_IS_E1(bp))