diff mbox

[net-next,2/3] ixgbe: Use __dev_uc_sync and __dev_uc_unsync for unicast addresses

Message ID 20151022232636.24612.12045.stgit@ahduyck-vm-fedora22
State Awaiting Upstream, archived
Delegated to: David Miller
Headers show

Commit Message

Alexander Duyck Oct. 22, 2015, 11:26 p.m. UTC
This change replaces the ixgbe_write_uc_addr_list call in ixgbe_set_rx_mode
with a call to __dev_uc_sync instead.  This works much better with the MAC
addr list code that was already in place and solves an issue in which you
couldn't remove an fdb address without having to reset the port.

Signed-off-by: Alexander Duyck <aduyck@mirantis.com>
---
 drivers/net/ethernet/intel/ixgbe/ixgbe_main.c |   28 ++++++++++++++++++++++---
 1 file changed, 25 insertions(+), 3 deletions(-)


--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Comments

Miller, Darin J Nov. 4, 2015, 6:39 p.m. UTC | #1
-----Original Message-----
From: Intel-wired-lan [mailto:intel-wired-lan-bounces@lists.osuosl.org] On Behalf Of Alexander Duyck
Sent: Thursday, October 22, 2015 4:27 PM
To: netdev@vger.kernel.org; intel-wired-lan@lists.osuosl.org; Kirsher, Jeffrey T
Subject: [Intel-wired-lan] [net-next PATCH 2/3] ixgbe: Use __dev_uc_sync and __dev_uc_unsync for unicast addresses

This change replaces the ixgbe_write_uc_addr_list call in ixgbe_set_rx_mode with a call to __dev_uc_sync instead.  This works much better with the MAC addr list code that was already in place and solves an issue in which you couldn't remove an fdb address without having to reset the port.

Signed-off-by: Alexander Duyck <aduyck@mirantis.com>
---
 drivers/net/ethernet/intel/ixgbe/ixgbe_main.c |   28 ++++++++++++++++++++++---
 1 file changed, 25 insertions(+), 3 deletions(-)

Tested-by: Darin Miller <darin.j.miller@intel.com>

--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Stephen Hemminger Nov. 12, 2015, 1:35 a.m. UTC | #2
On Thu, 22 Oct 2015 16:26:36 -0700
Alexander Duyck <aduyck@mirantis.com> wrote:

> +static int ixgbe_uc_unsync(struct net_device *netdev, const unsigned char *addr)
> +{
> +	struct ixgbe_adapter *adapter = netdev_priv(netdev);
> +
> +	ixgbe_del_mac_filter(adapter, addr, VMDQ_P(0));
> +
> +	return 0;

Why add an internal function that always returns 0?
Rather than making it void.
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Alexander H Duyck Nov. 12, 2015, 5:21 a.m. UTC | #3
On 11/11/2015 05:35 PM, Stephen Hemminger wrote:
> On Thu, 22 Oct 2015 16:26:36 -0700
> Alexander Duyck <aduyck@mirantis.com> wrote:
>
>> +static int ixgbe_uc_unsync(struct net_device *netdev, const unsigned char *addr)
>> +{
>> +	struct ixgbe_adapter *adapter = netdev_priv(netdev);
>> +
>> +	ixgbe_del_mac_filter(adapter, addr, VMDQ_P(0));
>> +
>> +	return 0;
> Why add an internal function that always returns 0?
> Rather than making it void.

Because the function pointer is passed to the __dev_uc_sync call and it 
requires a return value on the unsync function.  Basically if we 
returned an error it would delay flushing the address from the device 
until we could complete the call successfully, or __dev_uc_unsysnc was 
called without a function pointer.

- Alex
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
index 650c9526bbde..ba5c0d5b95b9 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
@@ -4215,6 +4215,25 @@  static int ixgbe_write_uc_addr_list(struct net_device *netdev, int vfn)
 	return count;
 }
 
+static int ixgbe_uc_sync(struct net_device *netdev, const unsigned char *addr)
+{
+	struct ixgbe_adapter *adapter = netdev_priv(netdev);
+	int ret;
+
+	ret = ixgbe_add_mac_filter(adapter, addr, VMDQ_P(0));
+
+	return min_t(int, ret, 0);
+}
+
+static int ixgbe_uc_unsync(struct net_device *netdev, const unsigned char *addr)
+{
+	struct ixgbe_adapter *adapter = netdev_priv(netdev);
+
+	ixgbe_del_mac_filter(adapter, addr, VMDQ_P(0));
+
+	return 0;
+}
+
 /**
  * ixgbe_set_rx_mode - Unicast, Multicast and Promiscuous mode set
  * @netdev: network interface device structure
@@ -4270,8 +4289,7 @@  void ixgbe_set_rx_mode(struct net_device *netdev)
 	 * sufficient space to store all the addresses then enable
 	 * unicast promiscuous mode
 	 */
-	count = ixgbe_write_uc_addr_list(netdev, VMDQ_P(0));
-	if (count < 0) {
+	if (__dev_uc_sync(netdev, ixgbe_uc_sync, ixgbe_uc_unsync)) {
 		fctrl |= IXGBE_FCTRL_UPE;
 		vmolr |= IXGBE_VMOLR_ROPE;
 	}
@@ -5109,8 +5127,12 @@  void ixgbe_reset(struct ixgbe_adapter *adapter)
 	}
 
 	clear_bit(__IXGBE_IN_SFP_INIT, &adapter->state);
-	/* do not flush user set addresses */
+
+	/* flush entries out of MAC table */
 	ixgbe_flush_sw_mac_table(adapter);
+	__dev_uc_unsync(netdev, NULL);
+
+	/* do not flush user set addresses */
 	ixgbe_mac_set_default_filter(adapter);
 
 	/* update SAN MAC vmdq pool selection */