From 85062f856e40c06c8a9c16a70895cec71b967cc4 Mon Sep 17 00:00:00 2001 From: Alexander Duyck Date: Wed, 24 Jan 2018 13:39:44 -0800 Subject: [PATCH 1/7] fm10k: Fix configuration for macvlan offload The fm10k driver didn't work correctly when macvlan offload was enabled. Specifically what would occur is that we would see no unicast packets being received. This was traced down to us not correctly configuring the default VLAN ID for the port and defaulting to 0. To correct this we either use the default ID provided by the switch or simply use 1. With that we are able to pass and receive traffic without any issues. In addition we were not repopulating the filter table following a reset. To correct that I have added a bit of code to fm10k_restore_rx_state that will repopulate the Rx filter configuration for the macvlan interfaces. Signed-off-by: Alexander Duyck Tested-by: Krishneil Singh Signed-off-by: Jeff Kirsher --- .../net/ethernet/intel/fm10k/fm10k_netdev.c | 25 ++++++++++++++++--- 1 file changed, 22 insertions(+), 3 deletions(-) diff --git a/drivers/net/ethernet/intel/fm10k/fm10k_netdev.c b/drivers/net/ethernet/intel/fm10k/fm10k_netdev.c index adc62fb38c49..6d9088956407 100644 --- a/drivers/net/ethernet/intel/fm10k/fm10k_netdev.c +++ b/drivers/net/ethernet/intel/fm10k/fm10k_netdev.c @@ -1182,9 +1182,10 @@ static void fm10k_set_rx_mode(struct net_device *dev) void fm10k_restore_rx_state(struct fm10k_intfc *interface) { + struct fm10k_l2_accel *l2_accel = interface->l2_accel; struct net_device *netdev = interface->netdev; struct fm10k_hw *hw = &interface->hw; - int xcast_mode; + int xcast_mode, i; u16 vid, glort; /* record glort for this interface */ @@ -1234,6 +1235,24 @@ void fm10k_restore_rx_state(struct fm10k_intfc *interface) __dev_uc_sync(netdev, fm10k_uc_sync, fm10k_uc_unsync); __dev_mc_sync(netdev, fm10k_mc_sync, fm10k_mc_unsync); + /* synchronize macvlan addresses */ + if (l2_accel) { + for (i = 0; i < l2_accel->size; i++) { + struct net_device *sdev = l2_accel->macvlan[i]; + + if (!sdev) + continue; + + glort = l2_accel->dglort + 1 + i; + + hw->mac.ops.update_xcast_mode(hw, glort, + FM10K_XCAST_MODE_MULTI); + fm10k_queue_mac_request(interface, glort, + sdev->dev_addr, + hw->mac.default_vid, true); + } + } + fm10k_mbx_unlock(interface); /* record updated xcast mode state */ @@ -1490,7 +1509,7 @@ static void *fm10k_dfwd_add_station(struct net_device *dev, hw->mac.ops.update_xcast_mode(hw, glort, FM10K_XCAST_MODE_MULTI); fm10k_queue_mac_request(interface, glort, sdev->dev_addr, - 0, true); + hw->mac.default_vid, true); } fm10k_mbx_unlock(interface); @@ -1530,7 +1549,7 @@ static void fm10k_dfwd_del_station(struct net_device *dev, void *priv) hw->mac.ops.update_xcast_mode(hw, glort, FM10K_XCAST_MODE_NONE); fm10k_queue_mac_request(interface, glort, sdev->dev_addr, - 0, false); + hw->mac.default_vid, false); } fm10k_mbx_unlock(interface); From c8eeacb3b02c217150a20a1bea9aef3a6929b205 Mon Sep 17 00:00:00 2001 From: Jacob Keller Date: Wed, 24 Jan 2018 14:17:15 -0800 Subject: [PATCH 2/7] fm10k: cleanup unnecessary parenthesis in fm10k_iov.c This fixes a few warnings found by checkpatch.pl --strict Signed-off-by: Jacob Keller Tested-by: Krishneil Singh Signed-off-by: Jeff Kirsher --- drivers/net/ethernet/intel/fm10k/fm10k_iov.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/net/ethernet/intel/fm10k/fm10k_iov.c b/drivers/net/ethernet/intel/fm10k/fm10k_iov.c index ea3ab24265ee..760cfa52d02c 100644 --- a/drivers/net/ethernet/intel/fm10k/fm10k_iov.c +++ b/drivers/net/ethernet/intel/fm10k/fm10k_iov.c @@ -353,7 +353,7 @@ int fm10k_iov_resume(struct pci_dev *pdev) struct fm10k_vf_info *vf_info = &iov_data->vf_info[i]; /* allocate all but the last GLORT to the VFs */ - if (i == ((~hw->mac.dglort_map) >> FM10K_DGLORTMAP_MASK_SHIFT)) + if (i == (~hw->mac.dglort_map >> FM10K_DGLORTMAP_MASK_SHIFT)) break; /* assign GLORT to VF, and restrict it to multicast */ @@ -511,7 +511,7 @@ int fm10k_iov_configure(struct pci_dev *pdev, int num_vfs) return err; /* allocate VFs if not already allocated */ - if (num_vfs && (num_vfs != current_vfs)) { + if (num_vfs && num_vfs != current_vfs) { /* Disable completer abort error reporting as * the VFs can trigger this any time they read a queue * that they don't own. From cf315ea596ec26d7aa542a9ce354990875a920c0 Mon Sep 17 00:00:00 2001 From: Ngai-Mint Kwan Date: Wed, 24 Jan 2018 14:18:22 -0800 Subject: [PATCH 3/7] fm10k: fix "failed to kill vid" message for VF When a VF is under PF VLAN assignment: ip link set vf <#> vlan This will remove all previous entries in the VLAN table including those generated by VLAN interfaces created on the VF. The issue arises when the VF is under PF VLAN assignment and one or more of these VLAN interfaces of the VF are deleted. When deleting these VLAN interfaces, the following message will be generated in "dmesg": failed to kill vid 0081/ for device This is due to the fact that "ndo_vlan_rx_kill_vid" exits with an error. The handler for this ndo is "fm10k_update_vid". Any calls to this function while under PF VLAN management will exit prematurely and, thus, it will generate the failure message. Additionally, since "fm10k_update_vid" exits prematurely, none of the VLAN update is performed. So, even though the actual VLAN interfaces of the VF will be deleted, the active_vlans bitmask is not cleared. When the VF is no longer under PF VLAN assignment, the driver mistakenly restores the previous entries of the VLAN table based on an unsynchronized list of active VLANs. The solution to this issue involves checking the VLAN update action type before exiting "fm10k_update_vid". If the VLAN update action type is to "add", this action will not be permitted while the VF is under PF VLAN assignment and the VLAN update is abandoned like before. However, if the VLAN update action type is to "kill", then we need to also clear the active_vlans bitmask. However, we don't need to actually queue any messages to the PF, because the MAC and VLAN tables have already been cleared, and the PF would silently ignore these requests anyways. Signed-off-by: Ngai-Mint Kwan Signed-off-by: Jacob Keller Tested-by: Krishneil Singh Signed-off-by: Jeff Kirsher --- drivers/net/ethernet/intel/fm10k/fm10k_netdev.c | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/drivers/net/ethernet/intel/fm10k/fm10k_netdev.c b/drivers/net/ethernet/intel/fm10k/fm10k_netdev.c index 6d9088956407..e85e0b077da3 100644 --- a/drivers/net/ethernet/intel/fm10k/fm10k_netdev.c +++ b/drivers/net/ethernet/intel/fm10k/fm10k_netdev.c @@ -934,8 +934,12 @@ static int fm10k_update_vid(struct net_device *netdev, u16 vid, bool set) if (vid >= VLAN_N_VID) return -EINVAL; - /* Verify we have permission to add VLANs */ - if (hw->mac.vlan_override) + /* Verify that we have permission to add VLANs. If this is a request + * to remove a VLAN, we still want to allow the user to remove the + * VLAN device. In that case, we need to clear the bit in the + * active_vlans bitmask. + */ + if (set && hw->mac.vlan_override) return -EACCES; /* update active_vlans bitmask */ @@ -954,6 +958,12 @@ static int fm10k_update_vid(struct net_device *netdev, u16 vid, bool set) rx_ring->vid &= ~FM10K_VLAN_CLEAR; } + /* If our VLAN has been overridden, there is no reason to send VLAN + * removal requests as they will be silently ignored. + */ + if (hw->mac.vlan_override) + return 0; + /* Do not remove default VLAN ID related entries from VLAN and MAC * tables */ From 8c2c5039073849d5b6f9cedaea604ebab1a50dc8 Mon Sep 17 00:00:00 2001 From: Jacob Keller Date: Wed, 24 Jan 2018 14:19:30 -0800 Subject: [PATCH 4/7] fm10k: stop adding VLAN 0 to the VLAN table Currently, when the driver loads, it sends a request to add VLAN 0 to the VLAN table. For the PF, this is honored, and VLAN 0 is indeed set. For the VF, this request is silently converted into a request for the default VLAN as defined by either the switch vid or the PF vid. This results in the odd behavior that the VLAN table doesn't appear consistent between the PF and the VF. Furthermore, setting a MAC filter with VLAN 0 is generally considered an invalid configuration by the switch, and since commit 856dfd69e84f ("fm10k: Fix multicast mode synch issues", 2016-03-03) we've had code which prevents us from ever sending such a request. Since there's not really a good reason to keep VLAN 0 in the VLAN table, stop requesting it in fm10k_restore_rx_state(). This might seem to indicate that we would no longer properly configure the MAC and VLAN tables for the default vid. However, due to the way that fm10k_find_next_vlan() behaves, it will always return the default_vid as enabled. Signed-off-by: Jacob Keller Tested-by: Krishneil Singh Signed-off-by: Jeff Kirsher --- drivers/net/ethernet/intel/fm10k/fm10k_netdev.c | 3 --- 1 file changed, 3 deletions(-) diff --git a/drivers/net/ethernet/intel/fm10k/fm10k_netdev.c b/drivers/net/ethernet/intel/fm10k/fm10k_netdev.c index e85e0b077da3..4cf68a235318 100644 --- a/drivers/net/ethernet/intel/fm10k/fm10k_netdev.c +++ b/drivers/net/ethernet/intel/fm10k/fm10k_netdev.c @@ -1222,9 +1222,6 @@ void fm10k_restore_rx_state(struct fm10k_intfc *interface) fm10k_queue_vlan_request(interface, FM10K_VLAN_ALL, 0, xcast_mode == FM10K_XCAST_MODE_PROMISC); - /* Add filter for VLAN 0 */ - fm10k_queue_vlan_request(interface, 0, 0, true); - /* update table with current entries */ for (vid = hw->mac.default_vid ? fm10k_find_next_vlan(interface, 0) : 1; vid < VLAN_N_VID; From 74d2950c8055a631e196349dd1345ff6deb11c73 Mon Sep 17 00:00:00 2001 From: Jacob Keller Date: Wed, 24 Jan 2018 14:20:29 -0800 Subject: [PATCH 5/7] fm10k: don't assume VLAN 1 is enabled Since commit 856dfd69e84f ("fm10k: Fix multicast mode synch issues", 2016-03-03) we've incorrectly assumed that VLAN 1 is enabled when the default VID is not set. This occurs because we check the default_vid and if it's zero, start several loops over the active_vlans bitmask at 1, instead of checking to ensure that that bit is active. This happened because of commit d9ff3ee8efe9 ("fm10k: Add support for VLAN 0 w/o default VLAN", 2014-08-07) which mistakenly assumed that we should send requests for MAC and VLAN filters with VLAN 0 when the default_vid isn't set. However, the switch generally considers this an invalid configuration, so the only time we'd have a default_vid of 0 is when the switch is down. Instead, lets just not request any filters for the default_vid if it's not yet been assigned. Signed-off-by: Jacob Keller Tested-by: Krishneil Singh Signed-off-by: Jeff Kirsher --- drivers/net/ethernet/intel/fm10k/fm10k_netdev.c | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/drivers/net/ethernet/intel/fm10k/fm10k_netdev.c b/drivers/net/ethernet/intel/fm10k/fm10k_netdev.c index 4cf68a235318..4c9d8e52415b 100644 --- a/drivers/net/ethernet/intel/fm10k/fm10k_netdev.c +++ b/drivers/net/ethernet/intel/fm10k/fm10k_netdev.c @@ -1050,14 +1050,13 @@ static int __fm10k_uc_sync(struct net_device *dev, const unsigned char *addr, bool sync) { struct fm10k_intfc *interface = netdev_priv(dev); - struct fm10k_hw *hw = &interface->hw; u16 vid, glort = interface->glort; s32 err; if (!is_valid_ether_addr(addr)) return -EADDRNOTAVAIL; - for (vid = hw->mac.default_vid ? fm10k_find_next_vlan(interface, 0) : 1; + for (vid = fm10k_find_next_vlan(interface, 0); vid < VLAN_N_VID; vid = fm10k_find_next_vlan(interface, vid)) { err = fm10k_queue_mac_request(interface, glort, @@ -1116,14 +1115,13 @@ static int __fm10k_mc_sync(struct net_device *dev, const unsigned char *addr, bool sync) { struct fm10k_intfc *interface = netdev_priv(dev); - struct fm10k_hw *hw = &interface->hw; u16 vid, glort = interface->glort; s32 err; if (!is_multicast_ether_addr(addr)) return -EADDRNOTAVAIL; - for (vid = hw->mac.default_vid ? fm10k_find_next_vlan(interface, 0) : 1; + for (vid = fm10k_find_next_vlan(interface, 0); vid < VLAN_N_VID; vid = fm10k_find_next_vlan(interface, vid)) { err = fm10k_queue_mac_request(interface, glort, @@ -1223,7 +1221,7 @@ void fm10k_restore_rx_state(struct fm10k_intfc *interface) xcast_mode == FM10K_XCAST_MODE_PROMISC); /* update table with current entries */ - for (vid = hw->mac.default_vid ? fm10k_find_next_vlan(interface, 0) : 1; + for (vid = fm10k_find_next_vlan(interface, 0); vid < VLAN_N_VID; vid = fm10k_find_next_vlan(interface, vid)) { fm10k_queue_vlan_request(interface, vid, 0, true); From 6ee98686d1fa6e6e3621802a4cd3ff31bb26dda8 Mon Sep 17 00:00:00 2001 From: Ngai-Mint Kwan Date: Wed, 24 Jan 2018 14:22:18 -0800 Subject: [PATCH 6/7] fm10k: correct typo in fm10k_pf.c Signed-off-by: Ngai-Mint Kwan Tested-by: Krishneil Singh Signed-off-by: Jeff Kirsher --- drivers/net/ethernet/intel/fm10k/fm10k_pf.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/net/ethernet/intel/fm10k/fm10k_pf.c b/drivers/net/ethernet/intel/fm10k/fm10k_pf.c index 425d814aed4d..d6406fc31ffb 100644 --- a/drivers/net/ethernet/intel/fm10k/fm10k_pf.c +++ b/drivers/net/ethernet/intel/fm10k/fm10k_pf.c @@ -866,7 +866,7 @@ static s32 fm10k_iov_assign_default_mac_vlan_pf(struct fm10k_hw *hw, /* Determine correct default VLAN ID. The FM10K_VLAN_OVERRIDE bit is * used here to indicate to the VF that it will not have privilege to * write VLAN_TABLE. All policy is enforced on the PF but this allows - * the VF to correctly report errors to userspace rqeuests. + * the VF to correctly report errors to userspace requests. */ if (vf_info->pf_vid) vf_vid = vf_info->pf_vid | FM10K_VLAN_OVERRIDE; From e0752a68d8de2a6fcb2909d86cab6b23896896de Mon Sep 17 00:00:00 2001 From: Ngai-Mint Kwan Date: Wed, 24 Jan 2018 14:23:27 -0800 Subject: [PATCH 7/7] fm10k: clarify action when updating the VLAN table Clarify the comment for when entering promiscuous mode that we update the VLAN table. Add a comment distinguishing the case where we're exiting promiscuous mode and need to clear the entire VLAN table. Signed-off-by: Ngai-Mint Kwan Signed-off-by: Jacob Keller Tested-by: Krishneil Singh Signed-off-by: Jeff Kirsher --- drivers/net/ethernet/intel/fm10k/fm10k_netdev.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/drivers/net/ethernet/intel/fm10k/fm10k_netdev.c b/drivers/net/ethernet/intel/fm10k/fm10k_netdev.c index 4c9d8e52415b..a38ae5c54da3 100644 --- a/drivers/net/ethernet/intel/fm10k/fm10k_netdev.c +++ b/drivers/net/ethernet/intel/fm10k/fm10k_netdev.c @@ -1165,10 +1165,12 @@ static void fm10k_set_rx_mode(struct net_device *dev) /* update xcast mode first, but only if it changed */ if (interface->xcast_mode != xcast_mode) { - /* update VLAN table */ + /* update VLAN table when entering promiscuous mode */ if (xcast_mode == FM10K_XCAST_MODE_PROMISC) fm10k_queue_vlan_request(interface, FM10K_VLAN_ALL, 0, true); + + /* clear VLAN table when exiting promiscuous mode */ if (interface->xcast_mode == FM10K_XCAST_MODE_PROMISC) fm10k_clear_unused_vlans(interface);