i40e/i40evf updates

From: Kiran Patil <[email protected]>

This patch sets VSI brodcast promiscuous mode during VSI add sequence and prevents adding MAC filter if specified MAC address is broadcast.

Likewise tried the test case mentioned in HSD where source MAC is broadcast and those packets are getting forwarded to one of the rx-queue.
HSD-number: 7661069
HSD-database: LADSW

From: Kiran Patil <[email protected]>

This patch sets VSI brodcast promiscuous mode during VSI add sequence and prevents adding MAC filter if specified MAC address is broadcast.

Likewise tried the test case mentioned in HSD where source MAC is broadcast and those packets are getting forwarded to one of the rx-queue.
HSD-number: 7661069
HSD-database: LADSW

From: Kiran Patil <[email protected]>

This patch sets VSI brodcast promiscuous mode during VSI add sequence and prevents adding MAC filter if specified MAC address is broadcast.

Likewise tried the test case mentioned in HSD where source MAC is broadcast and those packets are getting forwarded to one of the rx-queue.
HSD-number: 7661069
HSD-database: LADSW

From: Mitch Williams <[email protected]>

Properly track filter adds and deletes so the driver doesn't lose filters during resets and up/down cycles. Add a tracking mechanism so that the driver knows when to enter and leave promiscuous mode.

Implement a simple state machine so the driver can track the status of each filter throughout its lifecycle. Properly manage the overflow promiscuous state for the each VSI, and provide a way for the driver to detect when to exit overflow promiscuous mode.

Remove all possible default MAC filters that the firmware may have set up so that the driver can manage these correctly, particularly when VLANs come into play.

Finally, add the state of each filter to debugfs output so we can see what's going on inside the driver's pointy little head

Signed-off-by Mitch Williams <[email protected]>

Change-ID I97c5e366fac2254fa01eaff4f65c0af61dcf2e1f

---
Testing Hints : Add many filters. Remove many filters. Reset. Change

MAC address. Reset some more. Do VLANish stuff Check debugfs to make sure filter count and state are as expected.

  drivers/net/ethernet/intel/i40e/i40e.h         |  14 +-
  drivers/net/ethernet/intel/i40e/i40e_debugfs.c |  16 +-
  drivers/net/ethernet/intel/i40e/i40e_main.c    | 513 +++++++++++--------------
  3 files changed, 257 insertions(+), 286 deletions(-)

drivers/net/ethernet/intel/i40e/i40e.h +8 -0

index e83fc8a..2a88291 100644
@@ -448,6 +448,14 @@ struct i40e_pf {
  	u16 phy_led_val;
  };
 
+enum i40e_filter_state {
+	I40E_FILTER_INVALID = 0,	/* Invalid state */
+	I40E_FILTER_NEW,		/* New, not sent to FW yet */
+	I40E_FILTER_ACTIVE,		/* Added to switch by FW */
+	I40E_FILTER_FAILED,		/* Rejected by FW */
+	I40E_FILTER_REMOVE,		/* To be removed */
+/* There is no 'removed' state; the filter struct is freed */
+};
  struct i40e_mac_filter {
  	struct list_head list;
  	u8 macaddr[ETH_ALEN];

drivers/net/ethernet/intel/i40e/i40e.h +4 -0

+	enum i40e_filter_state state;
  };
 
  struct i40e_veb {

@@ -523,6 +530,9 @@ struct i40e_vsi {
  	struct i40e_ring **rx_rings;
  	struct i40e_ring **tx_rings;
 
+	u32  active_filters;
+	u32  promisc_threshold;
+
  	u16 work_limit;
  	u16 int_rate_limit;  /* value in usecs */
 

drivers/net/ethernet/intel/i40e/i40e_debugfs.c +14 -2

index e6af8c8..05cf9a7 100644
@@ -116,6 +116,14 @@ static ssize_t i40e_dbg_command_read(struct file *filp, char __user *buffer,
  	return len;
  }
 
+static char *i40e_filter_state_string[] = {
+	"INVALID",
+	"NEW",
+	"ACTIVE",
+	"FAILED",
+	"REMOVE",
+};
+
  /**
   * i40e_dbg_dump_vsi_seid - handles dump vsi seid write into command datum
   * @pf: the i40e_pf created in command write

@@ -160,10 +168,14 @@ static void i40e_dbg_dump_vsi_seid(struct i40e_pf *pf, int seid)
  			 pf->hw.mac.port_addr);
  	list_for_each_entry(f, &vsi->mac_filter_list, list) {
  		dev_info(&pf->pdev->dev,
-			 "    mac_filter_list: %pM vid=%d, is_netdev=%d is_vf=%d counter=%d\n",
+			 "    mac_filter_list: %pM vid=%d, is_netdev=%d is_vf=%d counter=%d, state %s\n",
  			 f->macaddr, f->vlan, f->is_netdev, f->is_vf,
-			 f->counter);
+			 f->counter, i40e_filter_state_string[f->state]);
  	}
+	dev_info(&pf->pdev->dev, "    active_filters %d, promisc_threshold %d, overflow promisc %s\n",
+		 vsi->active_filters, vsi->promisc_threshold,
+		 (test_bit(__I40E_FILTER_OVERFLOW_PROMISC, &vsi->state) ?
+		  "ON" : "OFF"));
  	nstat = i40e_get_vsi_stats_struct(vsi);
  	dev_info(&pf->pdev->dev,
  		 "    net_stats: rx_packets = %lu, rx_bytes = %lu, rx_errors = %lu, rx_dropped = %lu\n",

drivers/net/ethernet/intel/i40e/i40e_main.c +46 -89

index 0898c9d..fe4aa9c 100644
@@ -1279,8 +1279,9 @@ int i40e_del_mac_all_vlan(struct i40e_vsi *vsi, u8 *macaddr,
  		    (is_vf == f->is_vf) &&
  		    (is_netdev == f->is_netdev)) {
  			f->counter--;
-			f->changed = true;
  			changed = 1;
+			if (f->counter == 0)
+				f->state = I40E_FILTER_REMOVE;
  		}
  	}
  	if (changed) {

@@ -1296,29 +1297,32 @@ int i40e_del_mac_all_vlan(struct i40e_vsi *vsi, u8 *macaddr,
   * @vsi: the PF Main VSI - inappropriate for any other VSI
   * @macaddr: the MAC address
   *
- * Some older firmware configurations set up a default promiscuous VLAN
- * filter that needs to be removed.
+ * Remove whatever filter the firmware set up so the driver can manage
+ * its own filtering intelligently.
   **/
-static int i40e_rm_default_mac_filter(struct i40e_vsi *vsi, u8 *macaddr)
+static void i40e_rm_default_mac_filter(struct i40e_vsi *vsi, u8 *macaddr)
  {
  	struct i40e_aqc_remove_macvlan_element_data element;
  	struct i40e_pf *pf = vsi->back;
-	i40e_status ret;
 
  	/* Only appropriate for the PF main VSI */
  	if (vsi->type != I40E_VSI_MAIN)
-		return -EINVAL;
+		return;
 
  	memset(&element, 0, sizeof(element));
  	ether_addr_copy(element.mac_addr, macaddr);
  	element.vlan_tag = 0;
-	element.flags = I40E_AQC_MACVLAN_DEL_PERFECT_MATCH |
-			I40E_AQC_MACVLAN_DEL_IGNORE_VLAN;
-	ret = i40e_aq_remove_macvlan(&pf->hw, vsi->seid, &element, 1, NULL);
-	if (ret)
-		return -ENOENT;
+	/* Ignore error returns, some firmware does it this way... */
+	element.flags = I40E_AQC_MACVLAN_DEL_PERFECT_MATCH;
+	i40e_aq_remove_macvlan(&pf->hw, vsi->seid, &element, 1, NULL);
 
-	return 0;
+	memset(&element, 0, sizeof(element));
+	ether_addr_copy(element.mac_addr, macaddr);
+	element.vlan_tag = 0;
+	/* ...and some firmware does it this way. */
+	element.flags = I40E_AQC_MACVLAN_DEL_PERFECT_MATCH |
+			I40E_AQC_MACVLAN_ADD_IGNORE_VLAN;
+	i40e_aq_remove_macvlan(&pf->hw, vsi->seid, &element, 1, NULL);
  }
 
  /**

@@ -1339,6 +1343,7 @@ struct i40e_mac_filter *i40e_add_filter(struct i40e_vsi *vsi,
  					bool is_vf, bool is_netdev)
  {
  	struct i40e_mac_filter *f;
+	int changed = false;
 
  	if (!vsi || !macaddr)
  		return NULL;

@@ -1358,8 +1363,15 @@ struct i40e_mac_filter *i40e_add_filter(struct i40e_vsi *vsi,
 
  		ether_addr_copy(f->macaddr, macaddr);
  		f->vlan = vlan;
-		f->changed = true;
-
+		/* If we're in overflow promisc mode, set the state directly
+		 * to failed, so we don't bother to try sending the filter
+		 * to the hardware.
+		 */
+		if (test_bit(__I40E_FILTER_OVERFLOW_PROMISC, &vsi->state))
+			f->state = I40E_FILTER_FAILED;
+		else
+			f->state = I40E_FILTER_NEW;
+		changed = true;
  		INIT_LIST_HEAD(&f->list);
  		list_add_tail(&f->list, &vsi->mac_filter_list);
  	}

@@ -1379,10 +1391,7 @@ struct i40e_mac_filter *i40e_add_filter(struct i40e_vsi *vsi,
  		f->counter++;
  	}
 
-	/* changed tells sync_filters_subtask to
-	 * push the filter down to the firmware
-	 */
-	if (f->changed) {
+	if (changed) {
  		vsi->flags |= I40E_VSI_FLAG_FILTER_CHANGED;
  		vsi->back->flags |= I40E_FLAG_FILTER_SYNC;
  	}

@@ -1401,6 +1410,9 @@ add_filter_out:
   *
   * NOTE: This function is expected to be called with mac_filter_list_lock
   * being held.
+ * ANOTHER NOTE: This function MUST be called from within the context of
+ * the "safe" variants of any list iterators, e.g. list_for_each_entry_safe()
+ * instead of list_for_each_entry().
   **/
  void i40e_del_filter(struct i40e_vsi *vsi,
  		     u8 *macaddr, s16 vlan,

@@ -1440,9 +1452,18 @@ void i40e_del_filter(struct i40e_vsi *vsi,
  	 * remove the filter from the firmware's list
  	 */
  	if (f->counter == 0) {
-		f->changed = true;
-		vsi->flags |= I40E_VSI_FLAG_FILTER_CHANGED;
-		vsi->back->flags |= I40E_FLAG_FILTER_SYNC;
+		if ((f->state == I40E_FILTER_FAILED) ||
+		    (f->state == I40E_FILTER_NEW)) {
+			/* this one never got added by the FW. Just remove it,
+			 * no need to sync anything.
+			 */
+			list_del(&f->list);
+			kfree(f);
+		} else {
+			f->state = I40E_FILTER_REMOVE;
+			vsi->flags |= I40E_VSI_FLAG_FILTER_CHANGED;
+			vsi->back->flags |= I40E_FLAG_FILTER_SYNC;
+		}
  	}
  }
 

@@ -1464,7 +1485,6 @@ static int i40e_set_mac(struct net_device *netdev, void *p)
  	struct i40e_pf *pf = vsi->back;
  	struct i40e_hw *hw = &pf->hw;
  	struct sockaddr *addr = p;
-	struct i40e_mac_filter *f;
 
  	if (!is_valid_ether_addr(addr->sa_data))
  		return -EADDRNOTAVAIL;

@@ -1485,50 +1505,10 @@ static int i40e_set_mac(struct net_device *netdev, void *p)
  	else
  		netdev_info(netdev, "set new mac address %pM\n", addr->sa_data);
 
-	if (vsi->type == I40E_VSI_MAIN) {
-		i40e_status ret;
-
-		ret = i40e_aq_mac_address_write(&vsi->back->hw,
-						I40E_AQC_WRITE_TYPE_LAA_WOL,
-						addr->sa_data, NULL);
-		if (ret) {
-			netdev_info(netdev,
-				    "Addr change for Main VSI failed: %d\n",
-				    ret);
-			return -EADDRNOTAVAIL;
-		}
-	}
-
-	if (ether_addr_equal(netdev->dev_addr, hw->mac.addr)) {
-		struct i40e_aqc_remove_macvlan_element_data element;
-
-		memset(&element, 0, sizeof(element));
-		ether_addr_copy(element.mac_addr, netdev->dev_addr);
-		element.flags = I40E_AQC_MACVLAN_DEL_PERFECT_MATCH;
-		i40e_aq_remove_macvlan(&pf->hw, vsi->seid, &element, 1, NULL);
-	} else {
-		spin_lock_bh(&vsi->mac_filter_list_lock);
-		i40e_del_filter(vsi, netdev->dev_addr, I40E_VLAN_ANY,
-				false, false);
-		spin_unlock_bh(&vsi->mac_filter_list_lock);
-	}
-
-	if (ether_addr_equal(addr->sa_data, hw->mac.addr)) {
-		struct i40e_aqc_add_macvlan_element_data element;
-
-		memset(&element, 0, sizeof(element));
-		ether_addr_copy(element.mac_addr, hw->mac.addr);
-		element.flags = cpu_to_le16(I40E_AQC_MACVLAN_ADD_PERFECT_MATCH);
-		i40e_aq_add_macvlan(&pf->hw, vsi->seid, &element, 1, NULL);
-	} else {
-		spin_lock_bh(&vsi->mac_filter_list_lock);
-		f = i40e_add_filter(vsi, addr->sa_data, I40E_VLAN_ANY,
-				    false, false);
-		if (f)
-			f->is_laa = true;
-		spin_unlock_bh(&vsi->mac_filter_list_lock);
-	}
-
+	spin_lock_bh(&vsi->mac_filter_list_lock);
+	i40e_del_mac_all_vlan(vsi, netdev->dev_addr, false, true);
+	i40e_put_mac_in_vlan(vsi, addr->sa_data, false, true);
+	spin_unlock_bh(&vsi->mac_filter_list_lock);
  	ether_addr_copy(netdev->dev_addr, addr->sa_data);
 
  	/* schedule our worker thread which will take care of

@@ -1761,28 +1741,6 @@ bottom_of_search_loop:
  }
 
  /**
- * i40e_mac_filter_entry_clone - Clones a MAC filter entry
- * @src: source MAC filter entry to be clones
- *
- * Returns the pointer to newly cloned MAC filter entry or NULL
- * in case of error
- **/
-static struct i40e_mac_filter *i40e_mac_filter_entry_clone(
-					struct i40e_mac_filter *src)
-{
-	struct i40e_mac_filter *f;
-
-	f = kzalloc(sizeof(*f), GFP_ATOMIC);
-	if (!f)
-		return NULL;
-	*f = *src;
-
-	INIT_LIST_HEAD(&f->list);
-
-	return f;
-}
-
-/**
   * i40e_undo_del_filter_entries - Undo the changes made to MAC filter entries
   * @vsi: pointer to vsi struct
   * @from: Pointer to list which contains MAC filter entries - changes to

@@ -1796,41 +1754,61 @@ static void i40e_undo_del_filter_entries(struct i40e_vsi *vsi,
  	struct i40e_mac_filter *f, *ftmp;
 
  	list_for_each_entry_safe(f, ftmp, from, list) {
-		f->changed = true;

-----Original Message-----
From: Intel-wired-lan [mailto:intel-wired-lan-bounces at lists.osuosl.org] On Behalf Of Shannon Nelson
Sent: Wednesday, June 15, 2016 7:29 PM
To: Pujari, Bimmy <[email protected]>; intel-wired-lan at lists.osuosl.org
Subject: Re: [Intel-wired-lan] [next PATCH S38 6/7] i40e: refactor rx filter handling

Quoted 5 lines · Bimmy Pujari

From: Mitch Williams <[email protected]>

Properly track filter adds and deletes so the driver doesn't lose filters during resets and up/down cycles. Add a tracking mechanism so that the driver knows when to enter and leave promiscuous mode.

Thanks, this was sorely needed.

Quoted 3 lines

Implement a simple state machine so the driver can track the status of

each

Quoted 6 lines

filter throughout its lifecycle. Properly manage the overflow promiscuous state for the each VSI, and provide a way for the driver to detect when to exit overflow promiscuous mode.

Remove all possible default MAC filters that the firmware may have set up so that the driver can manage these correctly, particularly when VLANs

come

Quoted 3 lines

into play.

Finally, add the state of each filter to debugfs output so we can see

what's

Quoted 2 lines

going on inside the driver's pointy little head

Quoted 4 lines

Signed-off-by Mitch Williams <[email protected]>

Change-ID I97c5e366fac2254fa01eaff4f65c0af61dcf2e1f

---
Testing Hints : Add many filters. Remove many filters. Reset. Change

Some concept for "many" would be useful here. How many will it take to trip the thing into promiscuous mode given 1 port or 4 ports or some number of VFs running?

Quoted 7 lines

MAC address. Reset some more. Do VLANish stuff Check debugfs to make sure filter count and state are as expected.

  drivers/net/ethernet/intel/i40e/i40e.h         |  14 +-
  drivers/net/ethernet/intel/i40e/i40e_debugfs.c |  16 +-
  drivers/net/ethernet/intel/i40e/i40e_main.c    | 513 +++++++++++--------

------

Review

Quoted 3 lines

  3 files changed, 257 insertions(+), 286 deletions(-)

(quoted diff) +0 -0

b/drivers/net/ethernet/intel/i40e/i40e.h

Quoted 19 lines

index e83fc8a..2a88291 100644

drivers/net/ethernet/intel/i40e/i40e.h +8 -0

@@ -448,6 +448,14 @@ struct i40e_pf {
  	u16 phy_led_val;
  };
 
+enum i40e_filter_state {
+	I40E_FILTER_INVALID = 0,	/* Invalid state */
+	I40E_FILTER_NEW,		/* New, not sent to FW yet */
+	I40E_FILTER_ACTIVE,		/* Added to switch by FW */
+	I40E_FILTER_FAILED,		/* Rejected by FW */
+	I40E_FILTER_REMOVE,		/* To be removed */
+/* There is no 'removed' state; the filter struct is freed */
+};
  struct i40e_mac_filter {
  	struct list_head list;
  	u8 macaddr[ETH_ALEN];

@@ -456,8 +464,7 @@ struct i40e_mac_filter {

drivers/net/ethernet/intel/i40e/i40e.h struct i40e_mac_filter { · line 460

Quoted 5 lines

drivers/net/ethernet/intel/i40e/i40e.h +0 -2

  	u8 counter;		/* number of instances of this filter */
  	bool is_vf;		/* filter belongs to a VF */
  	bool is_netdev;		/* filter belongs to a netdev */
-	bool changed;		/* filter needs to be sync'd to the HW */
-	bool is_laa;		/* filter is a Locally Administered Address */

So you're dumping LAA support? Or is this extra work no longer needed? I would have expected some discussion of this in the commit notes.

Review

Quoted 16 lines

drivers/net/ethernet/intel/i40e/i40e.h +4 -0

+	enum i40e_filter_state state;
  };
 
  struct i40e_veb {

@@ -523,6 +530,9 @@ struct i40e_vsi {
  	struct i40e_ring **rx_rings;
  	struct i40e_ring **tx_rings;
 
+	u32  active_filters;
+	u32  promisc_threshold;
+
  	u16 work_limit;
  	u16 int_rate_limit;  /* value in usecs */
 

(quoted diff) +0 -0

b/drivers/net/ethernet/intel/i40e/i40e_debugfs.c

drivers/net/ethernet/intel/i40e/i40e_debugfs.c static ssize_t i40e_dbg_command_read(struct file

Quoted 4 lines

index e6af8c8..05cf9a7 100644

drivers/net/ethernet/intel/i40e/i40e_debugfs.c +0 -0

@@ -116,6 +116,14 @@ static ssize_t i40e_dbg_command_read(struct file

*filp, char __user *buffer,

drivers/net/ethernet/intel/i40e/i40e_debugfs.c static ssize_t i40e_dbg_command_read(struct file *filp, char __user *buffer,

Quoted 13 lines

drivers/net/ethernet/intel/i40e/i40e_debugfs.c +8 -0

  	return len;
  }
 
+static char *i40e_filter_state_string[] = {
+	"INVALID",
+	"NEW",
+	"ACTIVE",
+	"FAILED",
+	"REMOVE",
+};
+
  /**
   * i40e_dbg_dump_vsi_seid - handles dump vsi seid write into command

datum

drivers/net/ethernet/intel/i40e/i40e_debugfs.c static void i40e_dbg_dump_vsi_seid(struct i40e_pf

Quoted 2 lines

drivers/net/ethernet/intel/i40e/i40e_debugfs.c +0 -0

   * @pf: the i40e_pf created in command write

@@ -160,10 +168,14 @@ static void i40e_dbg_dump_vsi_seid(struct i40e_pf

*pf, int seid)

drivers/net/ethernet/intel/i40e/i40e_debugfs.c static void i40e_dbg_dump_vsi_seid(struct i40e_pf *pf, int seid)

Quoted 4 lines

drivers/net/ethernet/intel/i40e/i40e_debugfs.c +0 -1

  			 pf->hw.mac.port_addr);
  	list_for_each_entry(f, &vsi->mac_filter_list, list) {
  		dev_info(&pf->pdev->dev,
-			 "    mac_filter_list: %pM vid=%d, is_netdev=%d is_vf=%d

counter=%d\n",

drivers/net/ethernet/intel/i40e/i40e_debugfs.c static void i40e_dbg_dump_vsi_seid(struct i40e_pf · line 171

Quoted 1 line

drivers/net/ethernet/intel/i40e/i40e_debugfs.c +1 -0

+			 "    mac_filter_list: %pM vid=%d, is_netdev=%d is_vf=%d

counter=%d, state %s\n",

drivers/net/ethernet/intel/i40e/i40e_debugfs.c static void i40e_dbg_dump_vsi_seid(struct i40e_pf *pf, int seid)

Quoted 5 lines

drivers/net/ethernet/intel/i40e/i40e_debugfs.c +2 -1

  			 f->macaddr, f->vlan, f->is_netdev, f->is_vf,
-			 f->counter);
+			 f->counter, i40e_filter_state_string[f->state]);
  	}
+	dev_info(&pf->pdev->dev, "    active_filters %d, promisc_threshold %d,

overflow promisc %s\n",

drivers/net/ethernet/intel/i40e/i40e_debugfs.c static void i40e_dbg_dump_vsi_seid(struct i40e_pf *pf, int seid)

Quoted 6 lines

drivers/net/ethernet/intel/i40e/i40e_debugfs.c +3 -0

+		 vsi->active_filters, vsi->promisc_threshold,
+		 (test_bit(__I40E_FILTER_OVERFLOW_PROMISC, &vsi->state) ?
+		  "ON" : "OFF"));
  	nstat = i40e_get_vsi_stats_struct(vsi);
  	dev_info(&pf->pdev->dev,
  		 "    net_stats: rx_packets = %lu, rx_bytes = %lu, rx_errors =

%lu, rx_dropped = %lu\n",

Review

Quoted 1 line

(quoted diff) +0 -0

b/drivers/net/ethernet/intel/i40e/i40e_main.c

drivers/net/ethernet/intel/i40e/i40e_main.c int i40e_del_mac_all_vlan(struct i40e_vsi *vsi, u8

Quoted 4 lines

index 0898c9d..fe4aa9c 100644

drivers/net/ethernet/intel/i40e/i40e_main.c +0 -0

@@ -1279,8 +1279,9 @@ int i40e_del_mac_all_vlan(struct i40e_vsi *vsi, u8

*macaddr,

drivers/net/ethernet/intel/i40e/i40e_main.c int i40e_del_mac_all_vlan(struct i40e_vsi *vsi, u8

Quoted 11 lines

drivers/net/ethernet/intel/i40e/i40e_main.c +2 -1

  		    (is_vf == f->is_vf) &&
  		    (is_netdev == f->is_netdev)) {
  			f->counter--;
-			f->changed = true;
  			changed = 1;
+			if (f->counter == 0)
+				f->state = I40E_FILTER_REMOVE;
  		}
  	}
  	if (changed) {

@@ -1296,29 +1297,32 @@ int i40e_del_mac_all_vlan(struct i40e_vsi *vsi, u8

*macaddr,

drivers/net/ethernet/intel/i40e/i40e_main.c struct i40e_mac_filter *i40e_add_filter(struct

Quoted 44 lines

drivers/net/ethernet/intel/i40e/i40e_main.c +14 -11

   * @vsi: the PF Main VSI - inappropriate for any other VSI
   * @macaddr: the MAC address
   *
- * Some older firmware configurations set up a default promiscuous VLAN
- * filter that needs to be removed.
+ * Remove whatever filter the firmware set up so the driver can manage
+ * its own filtering intelligently.
   **/
-static int i40e_rm_default_mac_filter(struct i40e_vsi *vsi, u8 *macaddr)
+static void i40e_rm_default_mac_filter(struct i40e_vsi *vsi, u8 *macaddr)
  {
  	struct i40e_aqc_remove_macvlan_element_data element;
  	struct i40e_pf *pf = vsi->back;
-	i40e_status ret;
 
  	/* Only appropriate for the PF main VSI */
  	if (vsi->type != I40E_VSI_MAIN)
-		return -EINVAL;
+		return;
 
  	memset(&element, 0, sizeof(element));
  	ether_addr_copy(element.mac_addr, macaddr);
  	element.vlan_tag = 0;
-	element.flags = I40E_AQC_MACVLAN_DEL_PERFECT_MATCH |
-			I40E_AQC_MACVLAN_DEL_IGNORE_VLAN;
-	ret = i40e_aq_remove_macvlan(&pf->hw, vsi->seid, &element, 1, NULL);
-	if (ret)
-		return -ENOENT;
+	/* Ignore error returns, some firmware does it this way... */
+	element.flags = I40E_AQC_MACVLAN_DEL_PERFECT_MATCH;
+	i40e_aq_remove_macvlan(&pf->hw, vsi->seid, &element, 1, NULL);
 
-	return 0;
+	memset(&element, 0, sizeof(element));
+	ether_addr_copy(element.mac_addr, macaddr);
+	element.vlan_tag = 0;
+	/* ...and some firmware does it this way. */
+	element.flags = I40E_AQC_MACVLAN_DEL_PERFECT_MATCH |
+			I40E_AQC_MACVLAN_ADD_IGNORE_VLAN;
+	i40e_aq_remove_macvlan(&pf->hw, vsi->seid, &element, 1, NULL);
  }
 
  /**

@@ -1339,6 +1343,7 @@ struct i40e_mac_filter *i40e_add_filter(struct

i40e_vsi *vsi,

drivers/net/ethernet/intel/i40e/i40e_main.c struct i40e_mac_filter *i40e_add_filter(struct

Quoted 8 lines

drivers/net/ethernet/intel/i40e/i40e_main.c +1 -0

  					bool is_vf, bool is_netdev)
  {
  	struct i40e_mac_filter *f;
+	int changed = false;
 
  	if (!vsi || !macaddr)
  		return NULL;

@@ -1358,8 +1363,15 @@ struct i40e_mac_filter *i40e_add_filter(struct

i40e_vsi *vsi,

drivers/net/ethernet/intel/i40e/i40e_main.c struct i40e_mac_filter *i40e_add_filter(struct

Quoted 18 lines

drivers/net/ethernet/intel/i40e/i40e_main.c +9 -2

  		ether_addr_copy(f->macaddr, macaddr);
  		f->vlan = vlan;
-		f->changed = true;
-
+		/* If we're in overflow promisc mode, set the state directly
+		 * to failed, so we don't bother to try sending the filter
+		 * to the hardware.
+		 */
+		if (test_bit(__I40E_FILTER_OVERFLOW_PROMISC, &vsi->state))
+			f->state = I40E_FILTER_FAILED;
+		else
+			f->state = I40E_FILTER_NEW;
+		changed = true;
  		INIT_LIST_HEAD(&f->list);
  		list_add_tail(&f->list, &vsi->mac_filter_list);
  	}

@@ -1379,10 +1391,7 @@ struct i40e_mac_filter *i40e_add_filter(struct

i40e_vsi *vsi,

Quoted 12 lines

drivers/net/ethernet/intel/i40e/i40e_main.c +1 -4

  		f->counter++;
  	}
 
-	/* changed tells sync_filters_subtask to
-	 * push the filter down to the firmware
-	 */
-	if (f->changed) {
+	if (changed) {
  		vsi->flags |= I40E_VSI_FLAG_FILTER_CHANGED;
  		vsi->back->flags |= I40E_FLAG_FILTER_SYNC;
  	}

drivers/net/ethernet/intel/i40e/i40e_main.c add_filter_out: · line 1411

Quoted 2 lines

drivers/net/ethernet/intel/i40e/i40e_main.c +0 -0

@@ -1401,6 +1410,9 @@ add_filter_out:
   *
   * NOTE: This function is expected to be called with

mac_filter_list_lock

drivers/net/ethernet/intel/i40e/i40e_main.c add_filter_out:

Quoted 3 lines

drivers/net/ethernet/intel/i40e/i40e_main.c +2 -0

   * being held.
+ * ANOTHER NOTE: This function MUST be called from within the context of
+ * the "safe" variants of any list iterators, e.g.

list_for_each_entry_safe()

drivers/net/ethernet/intel/i40e/i40e_main.c static int i40e_set_mac(struct net_device *netdev,

Quoted 27 lines

drivers/net/ethernet/intel/i40e/i40e_main.c +13 -3

+ * instead of list_for_each_entry().
   **/
  void i40e_del_filter(struct i40e_vsi *vsi,
  		     u8 *macaddr, s16 vlan,

@@ -1440,9 +1452,18 @@ void i40e_del_filter(struct i40e_vsi *vsi,
  	 * remove the filter from the firmware's list
  	 */
  	if (f->counter == 0) {
-		f->changed = true;
-		vsi->flags |= I40E_VSI_FLAG_FILTER_CHANGED;
-		vsi->back->flags |= I40E_FLAG_FILTER_SYNC;
+		if ((f->state == I40E_FILTER_FAILED) ||
+		    (f->state == I40E_FILTER_NEW)) {
+			/* this one never got added by the FW. Just remove it,
+			 * no need to sync anything.
+			 */
+			list_del(&f->list);
+			kfree(f);
+		} else {
+			f->state = I40E_FILTER_REMOVE;
+			vsi->flags |= I40E_VSI_FLAG_FILTER_CHANGED;
+			vsi->back->flags |= I40E_FLAG_FILTER_SYNC;
+		}
  	}
  }
 

@@ -1464,7 +1485,6 @@ static int i40e_set_mac(struct net_device *netdev,

void *p)

drivers/net/ethernet/intel/i40e/i40e_main.c static int i40e_set_mac(struct net_device *netdev,

Quoted 8 lines

drivers/net/ethernet/intel/i40e/i40e_main.c +0 -1

  	struct i40e_pf *pf = vsi->back;
  	struct i40e_hw *hw = &pf->hw;
  	struct sockaddr *addr = p;
-	struct i40e_mac_filter *f;
 
  	if (!is_valid_ether_addr(addr->sa_data))
  		return -EADDRNOTAVAIL;

@@ -1485,50 +1505,10 @@ static int i40e_set_mac(struct net_device *netdev,

void *p)

Quoted 71 lines

drivers/net/ethernet/intel/i40e/i40e_main.c +4 -57

  	else
  		netdev_info(netdev, "set new mac address %pM\n", addr->sa_data);
 
-	if (vsi->type == I40E_VSI_MAIN) {
-		i40e_status ret;
-
-		ret = i40e_aq_mac_address_write(&vsi->back->hw,
-						I40E_AQC_WRITE_TYPE_LAA_WOL,
-						addr->sa_data, NULL);
-		if (ret) {
-			netdev_info(netdev,
-				    "Addr change for Main VSI failed: %d\n",
-				    ret);
-			return -EADDRNOTAVAIL;
-		}
-	}
-
-	if (ether_addr_equal(netdev->dev_addr, hw->mac.addr)) {
-		struct i40e_aqc_remove_macvlan_element_data element;
-
-		memset(&element, 0, sizeof(element));
-		ether_addr_copy(element.mac_addr, netdev->dev_addr);
-		element.flags = I40E_AQC_MACVLAN_DEL_PERFECT_MATCH;
-		i40e_aq_remove_macvlan(&pf->hw, vsi->seid, &element, 1, NULL);
-	} else {
-		spin_lock_bh(&vsi->mac_filter_list_lock);
-		i40e_del_filter(vsi, netdev->dev_addr, I40E_VLAN_ANY,
-				false, false);
-		spin_unlock_bh(&vsi->mac_filter_list_lock);
-	}
-
-	if (ether_addr_equal(addr->sa_data, hw->mac.addr)) {
-		struct i40e_aqc_add_macvlan_element_data element;
-
-		memset(&element, 0, sizeof(element));
-		ether_addr_copy(element.mac_addr, hw->mac.addr);
-		element.flags = cpu_to_le16(I40E_AQC_MACVLAN_ADD_PERFECT_MATCH);
-		i40e_aq_add_macvlan(&pf->hw, vsi->seid, &element, 1, NULL);
-	} else {
-		spin_lock_bh(&vsi->mac_filter_list_lock);
-		f = i40e_add_filter(vsi, addr->sa_data, I40E_VLAN_ANY,
-				    false, false);
-		if (f)
-			f->is_laa = true;
-		spin_unlock_bh(&vsi->mac_filter_list_lock);
-	}
-
+	spin_lock_bh(&vsi->mac_filter_list_lock);
+	i40e_del_mac_all_vlan(vsi, netdev->dev_addr, false, true);
+	i40e_put_mac_in_vlan(vsi, addr->sa_data, false, true);
+	spin_unlock_bh(&vsi->mac_filter_list_lock);
  	ether_addr_copy(netdev->dev_addr, addr->sa_data);
 
  	/* schedule our worker thread which will take care of

@@ -1761,28 +1741,6 @@ bottom_of_search_loop:
  }
 
  /**
- * i40e_mac_filter_entry_clone - Clones a MAC filter entry
- * @src: source MAC filter entry to be clones
- *
- * Returns the pointer to newly cloned MAC filter entry or NULL
- * in case of error
- **/
-static struct i40e_mac_filter *i40e_mac_filter_entry_clone(
-					struct i40e_mac_filter *src)
-{
-	struct i40e_mac_filter *f;
-
-	f = kzalloc(sizeof(*f), GFP_ATOMIC);
-	if (!f)

drivers/net/ethernet/intel/i40e/i40e_main.c bottom_of_search_loop: · line 1744

Quoted 10 lines

drivers/net/ethernet/intel/i40e/i40e_main.c +0 -9

-		return NULL;
-	*f = *src;
-
-	INIT_LIST_HEAD(&f->list);
-
-	return f;
-}
-
-/**
   * i40e_undo_del_filter_entries - Undo the changes made to MAC filter

entries

drivers/net/ethernet/intel/i40e/i40e_main.c static void i40e_undo_del_filter_entries(struct

Quoted 3 lines

drivers/net/ethernet/intel/i40e/i40e_main.c +0 -0

   * @vsi: pointer to vsi struct
   * @from: Pointer to list which contains MAC filter entries - changes to

@@ -1796,41 +1754,61 @@ static void i40e_undo_del_filter_entries(struct

i40e_vsi *vsi,

Quoted 9 lines

drivers/net/ethernet/intel/i40e/i40e_main.c +0 -1

  	struct i40e_mac_filter *f, *ftmp;
 
  	list_for_each_entry_safe(f, ftmp, from, list) {
-		f->changed = true;
  		/* Move the element back into MAC filter list*/
  		list_move_tail(&f->list, &vsi->mac_filter_list);
  	}
  }
 

drivers/net/ethernet/intel/i40e/i40e_main.c static void i40e_undo_del_filter_entries(struct · line 1806

Quoted 2 lines

drivers/net/ethernet/intel/i40e/i40e_main.c +0 -1

  /**
- * i40e_undo_add_filter_entries - Undo the changes made to MAC filter

entries

drivers/net/ethernet/intel/i40e/i40e_main.c static void i40e_undo_del_filter_entries(struct i40e_vsi *vsi, · line 1764

Quoted 3 lines

drivers/net/ethernet/intel/i40e/i40e_main.c +2 -1

- * @vsi: pointer to vsi struct
+ * i40e_update_filter_state - Update filter state based on return data
+ * from firmware

The description should be a single line.

sln

_______________________________________________
Intel-wired-lan mailing list
Intel-wired-lan at lists.osuosl.org
http://lists.osuosl.org/mailman/listinfo/intel-wired-lan

Quoted 6 lines

Quoted 5 lines

From: Mitch Williams <[email protected]>

Properly track filter adds and deletes so the driver doesn't lose filters during resets and up/down cycles. Add a tracking mechanism so that the driver knows when to enter and leave promiscuous mode.

Thanks, this was sorely needed.

Yes, it was needed, but you should NOT CARE.

   struct i40e_mac_filter {
   	struct list_head list;
   	u8 macaddr[ETH_ALEN];

drivers/net/ethernet/intel/i40e/i40e.h +0 -2

@@ -456,8 +464,7 @@ struct i40e_mac_filter {
   	u8 counter;		/* number of instances of this filter */
   	bool is_vf;		/* filter belongs to a VF */
   	bool is_netdev;		/* filter belongs to a netdev */
-	bool changed;		/* filter needs to be sync'd to the HW */
-	bool is_laa;		/* filter is a Locally Administered Address */

Quoted 2 lines

So you're dumping LAA support? Or is this extra work no longer needed? I would have expected some discussion of this in the commit notes.

I'll update the commit message. It's currently looking like we don't need this code.