From patchwork Fri Dec 29 13:50:34 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: "Michael, Alice" X-Patchwork-Id: 854008 X-Patchwork-Delegate: jeffrey.t.kirsher@intel.com Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Authentication-Results: ozlabs.org; spf=pass (mailfrom) smtp.mailfrom=osuosl.org (client-ip=140.211.166.133; helo=hemlock.osuosl.org; envelope-from=intel-wired-lan-bounces@osuosl.org; receiver=) Received: from hemlock.osuosl.org (smtp2.osuosl.org [140.211.166.133]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 3z7gRz0qWnz9s7G for ; Sat, 30 Dec 2017 08:57:35 +1100 (AEDT) Received: from localhost (localhost [127.0.0.1]) by hemlock.osuosl.org (Postfix) with ESMTP id A3729876C5; Fri, 29 Dec 2017 21:57:33 +0000 (UTC) X-Virus-Scanned: amavisd-new at osuosl.org Received: from hemlock.osuosl.org ([127.0.0.1]) by localhost (.osuosl.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id 21aecx041Txw; Fri, 29 Dec 2017 21:57:32 +0000 (UTC) Received: from ash.osuosl.org (ash.osuosl.org [140.211.166.34]) by hemlock.osuosl.org (Postfix) with ESMTP id A542A8769B; Fri, 29 Dec 2017 21:57:32 +0000 (UTC) X-Original-To: intel-wired-lan@lists.osuosl.org Delivered-To: intel-wired-lan@lists.osuosl.org Received: from silver.osuosl.org (smtp3.osuosl.org [140.211.166.136]) by ash.osuosl.org (Postfix) with ESMTP id EA2441C0180 for ; Fri, 29 Dec 2017 21:57:31 +0000 (UTC) Received: from localhost (localhost [127.0.0.1]) by silver.osuosl.org (Postfix) with ESMTP id E6A66292AA for ; Fri, 29 Dec 2017 21:57:31 +0000 (UTC) X-Virus-Scanned: amavisd-new at osuosl.org Received: from silver.osuosl.org ([127.0.0.1]) by localhost (.osuosl.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id X7yZeh1BZuiX for ; Fri, 29 Dec 2017 21:57:31 +0000 (UTC) X-Greylist: domain auto-whitelisted by SQLgrey-1.7.6 Received: from mga09.intel.com (mga09.intel.com [134.134.136.24]) by silver.osuosl.org (Postfix) with ESMTPS id 48377253F9 for ; Fri, 29 Dec 2017 21:57:31 +0000 (UTC) X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from fmsmga005.fm.intel.com ([10.253.24.32]) by orsmga102.jf.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 29 Dec 2017 13:57:29 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.45,478,1508828400"; d="scan'208";a="190738098" Received: from alicemic-2.jf.intel.com ([10.166.16.121]) by fmsmga005.fm.intel.com with ESMTP; 29 Dec 2017 13:57:30 -0800 From: Alice Michael To: alice.michael@intel.com, intel-wired-lan@lists.osuosl.org Date: Fri, 29 Dec 2017 08:50:34 -0500 Message-Id: <20171229135034.14504-1-alice.michael@intel.com> X-Mailer: git-send-email 2.9.5 Subject: [Intel-wired-lan] [next PATCH S85-V1 08/14] i40e: use changed_flags to check I40E_FLAG_DISABLE_FW_LLDP X-BeenThere: intel-wired-lan@osuosl.org X-Mailman-Version: 2.1.24 Precedence: list List-Id: Intel Wired Ethernet Linux Kernel Driver Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , MIME-Version: 1.0 Errors-To: intel-wired-lan-bounces@osuosl.org Sender: "Intel-wired-lan" From: Alan Brady Currently in i40e_set_priv_flags we use new_flags to check for the I40E_FLAG_DISABLE_FW_LLDP flag. This is an issue for a few a reasons. DISABLE_FW_LLDP is persistent across reboots/driver reloads. This means we need some way to detect if FW LLDP is enabled on init. We do this by trying to init_dcb and if it fails with EPERM we know LLDP is disabled in FW. This could be a problem on older FW versions or NPAR enabled PFs because there are situations where the FW could disable LLDP, but they do _not_ support using this flag to change it. If we do end up in this situation, the flag will be set, then when the user tries to change any priv flags, the driver thinks the user is trying to disable FW LLDP on a FW that doesn't support it and essentially forbids any priv flag changes. The fix is simple, instead of checking if this flag is set, we should be checking if the user is trying to _change_ the flag on unsupported FW versions. This patch also adds a comment explaining that the cmpxchg is the point of no return. Once we put the new flags into pf->flags we can't back out. Signed-off-by: Alan Brady Tested-by: Andrew Bowers --- drivers/net/ethernet/intel/i40e/i40e_ethtool.c | 22 +++++++++++++++------- 1 file changed, 15 insertions(+), 7 deletions(-) diff --git a/drivers/net/ethernet/intel/i40e/i40e_ethtool.c b/drivers/net/ethernet/intel/i40e/i40e_ethtool.c index 7267a93..5b18e20 100644 --- a/drivers/net/ethernet/intel/i40e/i40e_ethtool.c +++ b/drivers/net/ethernet/intel/i40e/i40e_ethtool.c @@ -4406,6 +4406,8 @@ static int i40e_set_priv_flags(struct net_device *dev, u32 flags) } flags_complete: + changed_flags = orig_flags ^ new_flags; + /* Before we finalize any flag changes, we need to perform some * checks to ensure that the changes are supported and safe. */ @@ -4415,13 +4417,17 @@ static int i40e_set_priv_flags(struct net_device *dev, u32 flags) !(pf->hw_features & I40E_HW_ATR_EVICT_CAPABLE)) return -EOPNOTSUPP; - /* Disable FW LLDP not supported if NPAR active or if FW - * API version < 1.7 + /* If the driver detected FW LLDP was disabled on init, this flag could + * be set, however we do not support _changing_ the flag if NPAR is + * enabled or FW API version < 1.7. There are situations where older + * FW versions/NPAR enabled PFs could disable LLDP, however we _must_ + * not allow the user to enable/disable LLDP with this flag on + * unsupported FW verions. */ - if (new_flags & I40E_FLAG_DISABLE_FW_LLDP) { + if (changed_flags & I40E_FLAG_DISABLE_FW_LLDP) { if (pf->hw.func_caps.npar_enable) { dev_warn(&pf->pdev->dev, - "Unable to stop FW LLDP if NPAR active\n"); + "Unable to change FW LLDP if NPAR active\n"); return -EOPNOTSUPP; } @@ -4429,7 +4435,7 @@ static int i40e_set_priv_flags(struct net_device *dev, u32 flags) (pf->hw.aq.api_maj_ver == 1 && pf->hw.aq.api_min_ver < 7)) { dev_warn(&pf->pdev->dev, - "FW ver does not support stopping FW LLDP\n"); + "FW ver does not support changing FW LLDP\n"); return -EOPNOTSUPP; } } @@ -4439,6 +4445,10 @@ static int i40e_set_priv_flags(struct net_device *dev, u32 flags) * something else has modified the flags variable since we copied it * originally. We'll just punt with an error and log something in the * message buffer. + * + * This is the point of no return for this function. We need to have + * checked any discrepencies or misconfigurations and returned + * EOPNOTSUPP before updating pf->flags here. */ if (cmpxchg64(&pf->flags, orig_flags, new_flags) != orig_flags) { dev_warn(&pf->pdev->dev, @@ -4446,8 +4456,6 @@ static int i40e_set_priv_flags(struct net_device *dev, u32 flags) return -EAGAIN; } - changed_flags = orig_flags ^ new_flags; - /* Process any additional changes needed as a result of flag changes. * The changed_flags value reflects the list of bits that were * changed in the code above.