OSDN Git Service

i40e: avoid looping to check whether we're in VLAN mode
authorJacob Keller <jacob.e.keller@intel.com>
Wed, 5 Oct 2016 16:30:40 +0000 (09:30 -0700)
committerJeff Kirsher <jeffrey.t.kirsher@intel.com>
Mon, 31 Oct 2016 21:26:40 +0000 (14:26 -0700)
We determine that a VSI is in vlan_mode whenever it has any filters
with a VLAN other than -1 (I40E_VLAN_ALL). The previous method of doing
so was to perform a loop whenever we needed the check. However, we can
notice that only place where filters are added (i40e_add_filter) can
change the condition from false to true, and the only place we can
return to false is in i40e_vsi_sync_filters_subtask. Thus, we can remove
the loop and use a boolean directly.

Doing this avoids looping over filters repeatedly especially while we're
already inside a loop over all the filters. This should reduce the
latency of filter operations throughout the driver.

Change-ID: Iafde08df588da2a2ea666997d05e11fad8edc338
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.h
drivers/net/ethernet/intel/i40e/i40e_main.c

index 6545550..60dbb5b 100644 (file)
@@ -517,6 +517,7 @@ struct i40e_vsi {
        spinlock_t mac_filter_hash_lock;
        /* Fixed size hash table with 2^8 buckets for MAC filters */
        DECLARE_HASHTABLE(mac_filter_hash, 8);
+       bool has_vlan_filter;
 
        /* VSI stats */
        struct rtnl_link_stats64 net_stats;
index 6cf9089..b4529b5 100644 (file)
@@ -1198,19 +1198,31 @@ struct i40e_mac_filter *i40e_find_mac(struct i40e_vsi *vsi, const u8 *macaddr)
  **/
 bool i40e_is_vsi_in_vlan(struct i40e_vsi *vsi)
 {
-       struct i40e_mac_filter *f;
-       struct hlist_node *h;
-       int bkt;
+       /* If we have a PVID, always operate in VLAN mode */
+       if (vsi->info.pvid)
+               return true;
 
-       /* Only -1 for all the filters denotes not in vlan mode
-        * so we have to go through all the list in order to make sure
+       /* We need to operate in VLAN mode whenever we have any filters with
+        * a VLAN other than I40E_VLAN_ALL. We could check the table each
+        * time, incurring search cost repeatedly. However, we can notice two
+        * things:
+        *
+        * 1) the only place where we can gain a VLAN filter is in
+        *    i40e_add_filter.
+        *
+        * 2) the only place where filters are actually removed is in
+        *    i40e_vsi_sync_filters_subtask.
+        *
+        * Thus, we can simply use a boolean value, has_vlan_filters which we
+        * will set to true when we add a VLAN filter in i40e_add_filter. Then
+        * we have to perform the full search after deleting filters in
+        * i40e_vsi_sync_filters_subtask, but we already have to search
+        * filters here and can perform the check at the same time. This
+        * results in avoiding embedding a loop for VLAN mode inside another
+        * loop over all the filters, and should maintain correctness as noted
+        * above.
         */
-       hash_for_each_safe(vsi->mac_filter_hash, bkt, h, f, hlist) {
-               if (f->vlan >= 0 || vsi->info.pvid)
-                       return true;
-       }
-
-       return false;
+       return vsi->has_vlan_filter;
 }
 
 /**
@@ -1246,6 +1258,12 @@ struct i40e_mac_filter *i40e_add_filter(struct i40e_vsi *vsi,
                if (!f)
                        return NULL;
 
+               /* Update the boolean indicating if we need to function in
+                * VLAN mode.
+                */
+               if (vlan >= 0)
+                       vsi->has_vlan_filter = true;
+
                ether_addr_copy(f->macaddr, macaddr);
                f->vlan = vlan;
                /* If we're in overflow promisc mode, set the state directly
@@ -1979,6 +1997,14 @@ int i40e_sync_vsi_filters(struct i40e_vsi *vsi)
                del_list = NULL;
        }
 
+       /* After finishing notifying firmware of the deleted filters, update
+        * the cached value of vsi->has_vlan_filter. Note that we are safe to
+        * use just !!vlan_filters here because if we only have VLAN=0 (that
+        * is, non_vlan_filters) these will all be converted to VLAN=-1 in the
+        * logic above already so this value would still be correct.
+        */
+       vsi->has_vlan_filter = !!vlan_filters;
+
        if (!hlist_empty(&tmp_add_list)) {
                /* Do all the adds now. */
                filter_list_len = hw->aq.asq_buf_size /