netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
To: davem@davemloft.net
Cc: Alexander Duyck <aduyck@mirantis.com>,
	netdev@vger.kernel.org, nhorman@redhat.com, sassmann@redhat.com,
	jogreene@redhat.com, Jeff Kirsher <jeffrey.t.kirsher@intel.com>
Subject: [net-next v2 8/8] ixgbe: Fix bugs in ixgbe_clear_vf_vlans()
Date: Tue, 29 Dec 2015 19:23:29 -0800	[thread overview]
Message-ID: <1451445809-70497-9-git-send-email-jeffrey.t.kirsher@intel.com> (raw)
In-Reply-To: <1451445809-70497-1-git-send-email-jeffrey.t.kirsher@intel.com>

From: Alexander Duyck <aduyck@mirantis.com>

When I had rewritten the code for ixgbe_clear_vf_vlans() it looks like I
had transitioned back and forth between using word as an offset and using
word as a register offset.  As a result I honestly don't see how the code
was working before other than the fact that resetting the VLANs on the VF
like didn't do much to clear them.

Another issue found is that the mask was using a divide instead of a
modulus.  As a result the mask bit was incorrectly being set to either bit
0 or 1 based on the value of the VF being tested.  As a result the wrong
VFs were having their VLANs cleared if they were enabled.

I have updated the code so that word represents the offset in the array.
This way we can use the modulus and xor operations and they will make sense
instead of being performed on a 4 byte aligned value.

I replaced the statement "(word % 2) ^ 1" with "~word % 2" in order to
reduce the line length as the line exceeded 80 characters with the register
name inserted.  The two should be equivalent so the change should be safe.

Reported-by: Emil Tantilov <emil.s.tantilov@intel.com>
Signed-off-by: Alexander Duyck <aduyck@mirantis.com>
Tested-by: Phil Schmitt <phillip.j.schmitt@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
 drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.c
index eeff3d0..8025a3f 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.c
@@ -593,11 +593,11 @@ static void ixgbe_clear_vf_vlans(struct ixgbe_adapter *adapter, u32 vf)
 
 	/* post increment loop, covers VLVF_ENTRIES - 1 to 0 */
 	for (i = IXGBE_VLVF_ENTRIES; i--;) {
-		u32 word = IXGBE_VLVFB(i * 2 + vf / 32);
 		u32 bits[2], vlvfb, vid, vfta, vlvf;
-		u32 mask = 1 << (vf / 32);
+		u32 word = i * 2 + vf / 32;
+		u32 mask = 1 << (vf % 32);
 
-		vlvfb = IXGBE_READ_REG(hw, word);
+		vlvfb = IXGBE_READ_REG(hw, IXGBE_VLVFB(word));
 
 		/* if our bit isn't set we can skip it */
 		if (!(vlvfb & mask))
@@ -608,7 +608,7 @@ static void ixgbe_clear_vf_vlans(struct ixgbe_adapter *adapter, u32 vf)
 
 		/* create 64b mask to chedk to see if we should clear VLVF */
 		bits[word % 2] = vlvfb;
-		bits[(word % 2) ^ 1] = IXGBE_READ_REG(hw, word ^ 1);
+		bits[~word % 2] = IXGBE_READ_REG(hw, IXGBE_VLVFB(word ^ 1));
 
 		/* if promisc is enabled, PF will be present, leave VFTA */
 		if (adapter->flags2 & IXGBE_FLAG2_VLAN_PROMISC) {
-- 
2.5.0

  parent reply	other threads:[~2015-12-30  3:23 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-12-30  3:23 [net-next v2 0/8][pull request] 10GbE Intel Wired LAN Driver Updates 2015-12-29 Jeff Kirsher
2015-12-30  3:23 ` [net-next v2 1/8] ixgbevf: Fix handling of NAPI budget when multiple queues are enabled per vector Jeff Kirsher
2015-12-30  3:23 ` [net-next v2 2/8] ixgbevf: minor cleanups for ixgbevf_set_itr() Jeff Kirsher
2015-12-30  3:23 ` [net-next v2 3/8] ixgbe: add support for QSFP PHY types in ixgbe_get_settings() Jeff Kirsher
2015-12-30  3:23 ` [net-next v2 4/8] ixgbe: report correct media type for KR, KX and KX4 interfaces Jeff Kirsher
2015-12-30  3:23 ` [net-next v2 5/8] ixgbe: Clean up redundancy in hw_enc_features Jeff Kirsher
2015-12-30  3:23 ` [net-next v2 6/8] ixgbe: fix RSS limit for X550 Jeff Kirsher
2015-12-30  3:23 ` [net-next v2 7/8] ixgbe: Correct X550EM_x revision check Jeff Kirsher
2015-12-30  3:23 ` Jeff Kirsher [this message]
2015-12-30 21:35 ` [net-next v2 0/8][pull request] 10GbE Intel Wired LAN Driver Updates 2015-12-29 David Miller

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1451445809-70497-9-git-send-email-jeffrey.t.kirsher@intel.com \
    --to=jeffrey.t.kirsher@intel.com \
    --cc=aduyck@mirantis.com \
    --cc=davem@davemloft.net \
    --cc=jogreene@redhat.com \
    --cc=netdev@vger.kernel.org \
    --cc=nhorman@redhat.com \
    --cc=sassmann@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).