From patchwork Wed Aug 31 16:55:07 2011 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Michal Schmidt X-Patchwork-Id: 112632 X-Patchwork-Delegate: davem@davemloft.net Return-Path: X-Original-To: patchwork-incoming@ozlabs.org Delivered-To: patchwork-incoming@ozlabs.org Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by ozlabs.org (Postfix) with ESMTP id 201F5B6F77 for ; Thu, 1 Sep 2011 03:45:52 +1000 (EST) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1756922Ab1HaRpq (ORCPT ); Wed, 31 Aug 2011 13:45:46 -0400 Received: from mx1.redhat.com ([209.132.183.28]:7593 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1756877Ab1HaRpp convert rfc822-to-8bit (ORCPT ); Wed, 31 Aug 2011 13:45:45 -0400 Received: from int-mx10.intmail.prod.int.phx2.redhat.com (int-mx10.intmail.prod.int.phx2.redhat.com [10.5.11.23]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id p7VHhY3i019397 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK); Wed, 31 Aug 2011 13:45:44 -0400 Received: from alice (vpn1-7-228.ams2.redhat.com [10.36.7.228]) by int-mx10.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id p7VGt8E4004247; Wed, 31 Aug 2011 12:55:09 -0400 Date: Wed, 31 Aug 2011 18:55:07 +0200 From: Michal Schmidt To: "netdev@vger.kernel.org" Cc: "Vlad Zolotarov" , "Dmitry Kravkov" , "Eilon Greenstein" , "mirqus@gmail.com" Subject: Re: [PATCH 1/3] bnx2x: remove TPA_ENABLE_FLAG Message-ID: <20110831185507.50a1940c@alice> In-Reply-To: <201108311816.30468.vladz@broadcom.com> References: <1314802836-9862-1-git-send-email-mschmidt@redhat.com> <1314802836-9862-2-git-send-email-mschmidt@redhat.com> <201108311816.30468.vladz@broadcom.com> Organization: Red Hat Czech Mime-Version: 1.0 X-Scanned-By: MIMEDefang 2.68 on 10.5.11.23 Sender: netdev-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: netdev@vger.kernel.org On Wed, 31 Aug 2011 18:16:30 +0300 Vlad Zolotarov wrote: > This is bogus - what if bnx2x_reload_if_running(dev) > (bnx2x_nic_load()) failes? The original dev->features would be > lost... Pls., see my latest response on your previouse patch series > thread. OK, so the conclusion in the other thread was to restore the features before returning. Here's an updated patch. Subject: [PATCH] bnx2x: remove TPA_ENABLE_FLAG TPA_ENABLE_FLAG is unnecessary, because it mirrors NETIF_F_LRO. "Cheating" in bnx2x_set_features() was suggested by Michał Mirosław. Signed-off-by: Michal Schmidt --- drivers/net/ethernet/broadcom/bnx2x/bnx2x.h | 2 +- drivers/net/ethernet/broadcom/bnx2x/bnx2x_cmn.c | 31 ++++++++++++--------- drivers/net/ethernet/broadcom/bnx2x/bnx2x_main.c | 7 +--- 3 files changed, 21 insertions(+), 19 deletions(-) diff --git a/drivers/net/ethernet/broadcom/bnx2x/bnx2x.h b/drivers/net/ethernet/broadcom/bnx2x/bnx2x.h index 735e491..5d5f323 100644 --- a/drivers/net/ethernet/broadcom/bnx2x/bnx2x.h +++ b/drivers/net/ethernet/broadcom/bnx2x/bnx2x.h @@ -1174,7 +1174,7 @@ struct bnx2x { #define USING_MSIX_FLAG (1 << 5) #define USING_MSI_FLAG (1 << 6) #define DISABLE_MSI_FLAG (1 << 7) -#define TPA_ENABLE_FLAG (1 << 8) + #define NO_MCP_FLAG (1 << 9) #define BP_NOMCP(bp) (bp->flags & NO_MCP_FLAG) diff --git a/drivers/net/ethernet/broadcom/bnx2x/bnx2x_cmn.c b/drivers/net/ethernet/broadcom/bnx2x/bnx2x_cmn.c index a08b101..399e71c 100644 --- a/drivers/net/ethernet/broadcom/bnx2x/bnx2x_cmn.c +++ b/drivers/net/ethernet/broadcom/bnx2x/bnx2x_cmn.c @@ -62,7 +62,7 @@ static inline void bnx2x_bz_fp(struct bnx2x *bp, int index) * set the tpa flag for each queue. The tpa flag determines the queue * minimal size so it must be set prior to queue memory allocation */ - fp->disable_tpa = ((bp->flags & TPA_ENABLE_FLAG) == 0); + fp->disable_tpa = !(bp->dev->features & NETIF_F_LRO); #ifdef BCM_CNIC /* We don't want TPA on an FCoE L2 ring */ @@ -3410,13 +3410,10 @@ u32 bnx2x_fix_features(struct net_device *dev, u32 features) int bnx2x_set_features(struct net_device *dev, u32 features) { struct bnx2x *bp = netdev_priv(dev); - u32 flags = bp->flags; bool bnx2x_reload = false; - if (features & NETIF_F_LRO) - flags |= TPA_ENABLE_FLAG; - else - flags &= ~TPA_ENABLE_FLAG; + if ((features ^ dev->features) & NETIF_F_LRO) + bnx2x_reload = true; if (features & NETIF_F_LOOPBACK) { if (bp->link_params.loopback_mode != LOOPBACK_BMAC) { @@ -3430,14 +3427,22 @@ int bnx2x_set_features(struct net_device *dev, u32 features) } } - if (flags ^ bp->flags) { - bp->flags = flags; - bnx2x_reload = true; - } - if (bnx2x_reload) { - if (bp->recovery_state == BNX2X_RECOVERY_DONE) - return bnx2x_reload_if_running(dev); + if (bp->recovery_state == BNX2X_RECOVERY_DONE) { + /* + * Cheat! Normally dev->features will be set after we + * return, but that's too late. We need to know how to + * configure the NIC when reloading it, so copy the + * features beforehand. Restore them after reload to + * avoid any possible confusion of the caller. + */ + int ret; + u32 orig_features = dev->features; + dev->features = features; + ret = bnx2x_reload_if_running(dev); + dev->features = orig_features; + return ret; + } /* else: bnx2x_nic_load() will be called at end of recovery */ } diff --git a/drivers/net/ethernet/broadcom/bnx2x/bnx2x_main.c b/drivers/net/ethernet/broadcom/bnx2x/bnx2x_main.c index e7b584b..fd32c04 100644 --- a/drivers/net/ethernet/broadcom/bnx2x/bnx2x_main.c +++ b/drivers/net/ethernet/broadcom/bnx2x/bnx2x_main.c @@ -9757,13 +9757,10 @@ static int __devinit bnx2x_init_bp(struct bnx2x *bp) bp->multi_mode = multi_mode; /* Set TPA flags */ - if (disable_tpa) { - bp->flags &= ~TPA_ENABLE_FLAG; + if (disable_tpa) bp->dev->features &= ~NETIF_F_LRO; - } else { - bp->flags |= TPA_ENABLE_FLAG; + else bp->dev->features |= NETIF_F_LRO; - } bp->disable_tpa = disable_tpa; if (CHIP_IS_E1(bp))