Netdev List
 help / color / mirror / Atom feed
From: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
To: davem@davemloft.net
Cc: Jacob Keller <jacob.e.keller@intel.com>,
	netdev@vger.kernel.org, nhorman@redhat.com, sassmann@redhat.com,
	jogreene@redhat.com, guru.anbalagane@oracle.com,
	Jeff Kirsher <jeffrey.t.kirsher@intel.com>
Subject: [net-next v2 15/19] i40e: refactor i40e_update_filter_state to avoid passing aq_err
Date: Wed,  7 Dec 2016 14:19:14 -0800	[thread overview]
Message-ID: <20161207221918.57932-16-jeffrey.t.kirsher@intel.com> (raw)
In-Reply-To: <20161207221918.57932-1-jeffrey.t.kirsher@intel.com>

From: Jacob Keller <jacob.e.keller@intel.com>

The current caller of i40e_update_filter_state incorrectly passes
aq_ret, an i40e_status variable, instead of the expected aq_err. This
happens to work because i40e_status is actually just a typedef integer,
and 0 is still the successful return. However i40e_update_filter_state
has special handling for ENOSPC which is currently being ignored.

Also notice that firmware does not update the per-filter response for
many types of errors, such as EINVAL. Thus, modify the filter setup so
that the firmware response memory is pre-set with I40E_AQC_MM_ERR_NO_RES.

This enables us to refactor i40e_update_filter_state, removing the need
to pass aq_err and avoiding a need for having 3 different flows for
checking the filter state.

The resulting code for i40e_update_filter_state is much simpler, only
a single loop and we always check each filter response value every time.
Since we pre-set the response value to match our expected error this
correctly works for all success and error flows.

Change-ID: Ie292c9511f34ee18c6ef40f955ad13e28b7aea7d
Signed-off-by: Jacob Keller <jacob.e.keller@intel.com>
Tested-by: Andrew Bowers <andrewx.bowers@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
 drivers/net/ethernet/intel/i40e/i40e_main.c | 58 +++++++++++------------------
 1 file changed, 21 insertions(+), 37 deletions(-)

diff --git a/drivers/net/ethernet/intel/i40e/i40e_main.c b/drivers/net/ethernet/intel/i40e/i40e_main.c
index 2ccf376..8e65972 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_main.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_main.c
@@ -1757,7 +1757,6 @@ static void i40e_undo_filter_entries(struct i40e_vsi *vsi,
  * @count: Number of filters added
  * @add_list: return data from fw
  * @head: pointer to first filter in current batch
- * @aq_err: status from fw
  *
  * MAC filter entries from list were slated to be added to device. Returns
  * number of successful filters. Note that 0 does NOT mean success!
@@ -1765,47 +1764,30 @@ static void i40e_undo_filter_entries(struct i40e_vsi *vsi,
 static int
 i40e_update_filter_state(int count,
 			 struct i40e_aqc_add_macvlan_element_data *add_list,
-			 struct i40e_mac_filter *add_head, int aq_err)
+			 struct i40e_mac_filter *add_head)
 {
 	int retval = 0;
 	int i;
 
-
-	if (!aq_err) {
-		retval = count;
-		/* Everything's good, mark all filters active. */
-		for (i = 0; i < count ; i++) {
-			add_head->state = I40E_FILTER_ACTIVE;
-			add_head = hlist_entry(add_head->hlist.next,
-					       typeof(struct i40e_mac_filter),
-					       hlist);
-		}
-	} else if (aq_err == I40E_AQ_RC_ENOSPC) {
-		/* Device ran out of filter space. Check the return value
-		 * for each filter to see which ones are active.
+	for (i = 0; i < count; i++) {
+		/* Always check status of each filter. We don't need to check
+		 * the firmware return status because we pre-set the filter
+		 * status to I40E_AQC_MM_ERR_NO_RES when sending the filter
+		 * request to the adminq. Thus, if it no longer matches then
+		 * we know the filter is active.
 		 */
-		for (i = 0; i < count ; i++) {
-			if (add_list[i].match_method ==
-			    I40E_AQC_MM_ERR_NO_RES) {
-				add_head->state = I40E_FILTER_FAILED;
-			} else {
-				add_head->state = I40E_FILTER_ACTIVE;
-				retval++;
-			}
-			add_head = hlist_entry(add_head->hlist.next,
-					       typeof(struct i40e_mac_filter),
-					       hlist);
-		}
-	} else {
-		/* Some other horrible thing happened, fail all filters */
-		retval = 0;
-		for (i = 0; i < count ; i++) {
+		if (add_list[i].match_method == I40E_AQC_MM_ERR_NO_RES) {
 			add_head->state = I40E_FILTER_FAILED;
-			add_head = hlist_entry(add_head->hlist.next,
-					       typeof(struct i40e_mac_filter),
-					       hlist);
+		} else {
+			add_head->state = I40E_FILTER_ACTIVE;
+			retval++;
 		}
+
+		add_head = hlist_entry(add_head->hlist.next,
+				       typeof(struct i40e_mac_filter),
+				       hlist);
 	}
+
 	return retval;
 }
 
@@ -1864,12 +1846,11 @@ void i40e_aqc_add_filters(struct i40e_vsi *vsi, const char *vsi_name,
 			  int num_add, bool *promisc_changed)
 {
 	struct i40e_hw *hw = &vsi->back->hw;
-	i40e_status aq_ret;
 	int aq_err, fcnt;
 
-	aq_ret = i40e_aq_add_macvlan(hw, vsi->seid, list, num_add, NULL);
+	i40e_aq_add_macvlan(hw, vsi->seid, list, num_add, NULL);
 	aq_err = hw->aq.asq_last_status;
-	fcnt = i40e_update_filter_state(num_add, list, add_head, aq_ret);
+	fcnt = i40e_update_filter_state(num_add, list, add_head);
 
 	if (fcnt != num_add) {
 		*promisc_changed = true;
@@ -2168,6 +2149,9 @@ int i40e_sync_vsi_filters(struct i40e_vsi *vsi)
 					cpu_to_le16((u16)(f->vlan));
 			}
 			add_list[num_add].queue_number = 0;
+			/* set invalid match method for later detection */
+			add_list[num_add].match_method =
+				cpu_to_le16((u16)I40E_AQC_MM_ERR_NO_RES);
 			cmd_flags |= I40E_AQC_MACVLAN_ADD_PERFECT_MATCH;
 			add_list[num_add].flags = cpu_to_le16(cmd_flags);
 			num_add++;
-- 
2.9.3

  parent reply	other threads:[~2016-12-07 22:20 UTC|newest]

Thread overview: 25+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-12-07 22:18 [net-next v2 00/19][pull request] 40GbE Intel Wired LAN Driver Updates 2016-12-07 Jeff Kirsher
2016-12-07 22:19 ` [net-next v2 01/19] i40e: Driver prints log message on link speed change Jeff Kirsher
2016-12-07 22:19 ` [net-next v2 02/19] i40e: simplify txd use count calculation Jeff Kirsher
2016-12-08  0:16   ` Eric Dumazet
2016-12-08  0:35     ` Duyck, Alexander H
2016-12-08  1:03       ` Eric Dumazet
2016-12-08  1:09         ` Duyck, Alexander H
2016-12-07 22:19 ` [net-next v2 03/19] i40e: restore workaround for removing default MAC filter Jeff Kirsher
2016-12-07 22:19 ` [net-next v2 04/19] i40e/i40evf: napi_poll must return the work done Jeff Kirsher
2016-12-07 22:19 ` [net-next v2 05/19] i40e: remove code to handle dev_addr specially Jeff Kirsher
2016-12-07 22:19 ` [net-next v2 06/19] i40e: Blink LED on 1G BaseT boards Jeff Kirsher
2016-12-07 22:19 ` [net-next v2 07/19] Changed version from 1.6.21 to 1.6.25 Jeff Kirsher
2016-12-07 22:19 ` [net-next v2 08/19] i40e: use unsigned printf format specifier for active_filters count Jeff Kirsher
2016-12-07 22:19 ` [net-next v2 09/19] i40e: Add support for 25G devices Jeff Kirsher
2016-12-07 22:19 ` [net-next v2 10/19] i40e: Add FEC for 25g Jeff Kirsher
2016-12-07 22:19 ` [net-next v2 11/19] i40e: Add functions which apply correct PHY access method for read and write operation Jeff Kirsher
2016-12-07 22:19 ` [net-next v2 12/19] i40e: lock service task correctly Jeff Kirsher
2016-12-07 22:19 ` [net-next v2 13/19] i40e: defeature support for PTP L4 frame detection on XL710 Jeff Kirsher
2016-12-07 22:19 ` [net-next v2 14/19] i40e: recalculate vsi->active_filters from hash contents Jeff Kirsher
2016-12-07 22:19 ` Jeff Kirsher [this message]
2016-12-07 22:19 ` [net-next v2 16/19] i40e: delete filter after adding its replacement when converting Jeff Kirsher
2016-12-07 22:19 ` [net-next v2 17/19] i40e: factor out addition/deletion of VLAN per each MAC address Jeff Kirsher
2016-12-07 22:19 ` [net-next v2 18/19] i40e: use (add|rm)_vlan_all_mac helper functions when changing PVID Jeff Kirsher
2016-12-07 22:19 ` [net-next v2 19/19] i40e: move all updates for VLAN mode into i40e_sync_vsi_filters Jeff Kirsher
2016-12-08  0:15 ` [net-next v2 00/19][pull request] 40GbE Intel Wired LAN Driver Updates 2016-12-07 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=20161207221918.57932-16-jeffrey.t.kirsher@intel.com \
    --to=jeffrey.t.kirsher@intel.com \
    --cc=davem@davemloft.net \
    --cc=guru.anbalagane@oracle.com \
    --cc=jacob.e.keller@intel.com \
    --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