From e792779e6b639c182df91b46ac1e5803460b0b15 Mon Sep 17 00:00:00 2001 From: Nitesh B Venkatesh Date: Fri, 4 Jun 2021 09:53:31 -0700 Subject: [PATCH 1/4] iavf: Prevent changing static ITR values if adaptive moderation is on Resolve being able to change static values on VF when adaptive interrupt moderation is enabled. This problem is fixed by checking the interrupt settings is not a combination of change of static value while adaptive interrupt moderation is turned on. Without this fix, the user would be able to change static values on VF with adaptive moderation enabled. Fixes: 65e87c0398f5 ("i40evf: support queue-specific settings for interrupt moderation") Signed-off-by: Nitesh B Venkatesh Tested-by: George Kuruvinakunnel Signed-off-by: Tony Nguyen --- .../net/ethernet/intel/iavf/iavf_ethtool.c | 30 ++++++++++++++++--- 1 file changed, 26 insertions(+), 4 deletions(-) diff --git a/drivers/net/ethernet/intel/iavf/iavf_ethtool.c b/drivers/net/ethernet/intel/iavf/iavf_ethtool.c index 144a776793597..71b23922089fb 100644 --- a/drivers/net/ethernet/intel/iavf/iavf_ethtool.c +++ b/drivers/net/ethernet/intel/iavf/iavf_ethtool.c @@ -723,12 +723,31 @@ static int iavf_get_per_queue_coalesce(struct net_device *netdev, u32 queue, * * Change the ITR settings for a specific queue. **/ -static void iavf_set_itr_per_queue(struct iavf_adapter *adapter, - struct ethtool_coalesce *ec, int queue) +static int iavf_set_itr_per_queue(struct iavf_adapter *adapter, + struct ethtool_coalesce *ec, int queue) { struct iavf_ring *rx_ring = &adapter->rx_rings[queue]; struct iavf_ring *tx_ring = &adapter->tx_rings[queue]; struct iavf_q_vector *q_vector; + u16 itr_setting; + + itr_setting = rx_ring->itr_setting & ~IAVF_ITR_DYNAMIC; + + if (ec->rx_coalesce_usecs != itr_setting && + ec->use_adaptive_rx_coalesce) { + netif_info(adapter, drv, adapter->netdev, + "Rx interrupt throttling cannot be changed if adaptive-rx is enabled\n"); + return -EINVAL; + } + + itr_setting = tx_ring->itr_setting & ~IAVF_ITR_DYNAMIC; + + if (ec->tx_coalesce_usecs != itr_setting && + ec->use_adaptive_tx_coalesce) { + netif_info(adapter, drv, adapter->netdev, + "Tx interrupt throttling cannot be changed if adaptive-tx is enabled\n"); + return -EINVAL; + } rx_ring->itr_setting = ITR_REG_ALIGN(ec->rx_coalesce_usecs); tx_ring->itr_setting = ITR_REG_ALIGN(ec->tx_coalesce_usecs); @@ -751,6 +770,7 @@ static void iavf_set_itr_per_queue(struct iavf_adapter *adapter, * the Tx and Rx ITR values based on the values we have entered * into the q_vector, no need to write the values now. */ + return 0; } /** @@ -792,9 +812,11 @@ static int __iavf_set_coalesce(struct net_device *netdev, */ if (queue < 0) { for (i = 0; i < adapter->num_active_queues; i++) - iavf_set_itr_per_queue(adapter, ec, i); + if (iavf_set_itr_per_queue(adapter, ec, i)) + return -EINVAL; } else if (queue < adapter->num_active_queues) { - iavf_set_itr_per_queue(adapter, ec, queue); + if (iavf_set_itr_per_queue(adapter, ec, queue)) + return -EINVAL; } else { netif_info(adapter, drv, netdev, "Invalid queue value, queue range is 0 - %d\n", adapter->num_active_queues - 1); From 0cc318d2e8408bc0ffb4662a0c3e5e57005ac6ff Mon Sep 17 00:00:00 2001 From: Jedrzej Jagielski Date: Tue, 7 Sep 2021 09:25:40 +0000 Subject: [PATCH 2/4] iavf: Fix deadlock occurrence during resetting VF interface System hangs if close the interface is called from the kernel during the interface is in resetting state. During resetting operation the link is closing but kernel didn't know it and it tried to close this interface again what sometimes led to deadlock. Inform kernel about current state of interface and turn off the flag IFF_UP when interface is closing until reset is finished. Previously it was most likely to hang the system when kernel (network manager) tried to close the interface in the same time when interface was in resetting state because of deadlock. Fixes: 3c8e0b989aa1 ("i40vf: don't stop me now") Signed-off-by: Jaroslaw Gawin Signed-off-by: Jedrzej Jagielski Tested-by: Konrad Jankowski Signed-off-by: Tony Nguyen --- drivers/net/ethernet/intel/iavf/iavf_main.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/drivers/net/ethernet/intel/iavf/iavf_main.c b/drivers/net/ethernet/intel/iavf/iavf_main.c index 336e6bf95e48f..84680777ac12b 100644 --- a/drivers/net/ethernet/intel/iavf/iavf_main.c +++ b/drivers/net/ethernet/intel/iavf/iavf_main.c @@ -2254,6 +2254,7 @@ static void iavf_reset_task(struct work_struct *work) (adapter->state == __IAVF_RESETTING)); if (running) { + netdev->flags &= ~IFF_UP; netif_carrier_off(netdev); netif_tx_stop_all_queues(netdev); adapter->link_up = false; @@ -2365,7 +2366,7 @@ static void iavf_reset_task(struct work_struct *work) * to __IAVF_RUNNING */ iavf_up_complete(adapter); - + netdev->flags |= IFF_UP; iavf_irq_enable(adapter, true); } else { iavf_change_state(adapter, __IAVF_DOWN); @@ -2378,8 +2379,10 @@ static void iavf_reset_task(struct work_struct *work) reset_err: mutex_unlock(&adapter->client_lock); mutex_unlock(&adapter->crit_lock); - if (running) + if (running) { iavf_change_state(adapter, __IAVF_RUNNING); + netdev->flags |= IFF_UP; + } dev_err(&adapter->pdev->dev, "failed to allocate resources during reinit\n"); iavf_close(netdev); } From 3b5bdd18eb76e7570d9bacbcab6828a9b26ae121 Mon Sep 17 00:00:00 2001 From: Jedrzej Jagielski Date: Wed, 15 Sep 2021 09:01:00 +0000 Subject: [PATCH 3/4] iavf: Fix refreshing iavf adapter stats on ethtool request Currently iavf adapter statistics are refreshed only in a watchdog task, triggered approximately every two seconds, which causes some ethtool requests to return outdated values. Add explicit statistics refresh when requested by ethtool -S. Fixes: b476b0030e61 ("iavf: Move commands processing to the separate function") Signed-off-by: Jan Sokolowski Signed-off-by: Jedrzej Jagielski Tested-by: Konrad Jankowski Signed-off-by: Tony Nguyen --- drivers/net/ethernet/intel/iavf/iavf.h | 2 ++ drivers/net/ethernet/intel/iavf/iavf_ethtool.c | 3 +++ drivers/net/ethernet/intel/iavf/iavf_main.c | 18 ++++++++++++++++++ .../net/ethernet/intel/iavf/iavf_virtchnl.c | 2 ++ 4 files changed, 25 insertions(+) diff --git a/drivers/net/ethernet/intel/iavf/iavf.h b/drivers/net/ethernet/intel/iavf/iavf.h index 75635bd57cf6d..bb9cc227d1e1c 100644 --- a/drivers/net/ethernet/intel/iavf/iavf.h +++ b/drivers/net/ethernet/intel/iavf/iavf.h @@ -305,6 +305,7 @@ struct iavf_adapter { #define IAVF_FLAG_AQ_DEL_FDIR_FILTER BIT(26) #define IAVF_FLAG_AQ_ADD_ADV_RSS_CFG BIT(27) #define IAVF_FLAG_AQ_DEL_ADV_RSS_CFG BIT(28) +#define IAVF_FLAG_AQ_REQUEST_STATS BIT(29) /* OS defined structs */ struct net_device *netdev; @@ -444,6 +445,7 @@ int iavf_up(struct iavf_adapter *adapter); void iavf_down(struct iavf_adapter *adapter); int iavf_process_config(struct iavf_adapter *adapter); void iavf_schedule_reset(struct iavf_adapter *adapter); +void iavf_schedule_request_stats(struct iavf_adapter *adapter); void iavf_reset(struct iavf_adapter *adapter); void iavf_set_ethtool_ops(struct net_device *netdev); void iavf_update_stats(struct iavf_adapter *adapter); diff --git a/drivers/net/ethernet/intel/iavf/iavf_ethtool.c b/drivers/net/ethernet/intel/iavf/iavf_ethtool.c index 71b23922089fb..0cecaff38d042 100644 --- a/drivers/net/ethernet/intel/iavf/iavf_ethtool.c +++ b/drivers/net/ethernet/intel/iavf/iavf_ethtool.c @@ -354,6 +354,9 @@ static void iavf_get_ethtool_stats(struct net_device *netdev, struct iavf_adapter *adapter = netdev_priv(netdev); unsigned int i; + /* Explicitly request stats refresh */ + iavf_schedule_request_stats(adapter); + iavf_add_ethtool_stats(&data, adapter, iavf_gstrings_stats); rcu_read_lock(); diff --git a/drivers/net/ethernet/intel/iavf/iavf_main.c b/drivers/net/ethernet/intel/iavf/iavf_main.c index 84680777ac12b..8e96ae746c3df 100644 --- a/drivers/net/ethernet/intel/iavf/iavf_main.c +++ b/drivers/net/ethernet/intel/iavf/iavf_main.c @@ -174,6 +174,19 @@ void iavf_schedule_reset(struct iavf_adapter *adapter) } } +/** + * iavf_schedule_request_stats - Set the flags and schedule statistics request + * @adapter: board private structure + * + * Sets IAVF_FLAG_AQ_REQUEST_STATS flag so iavf_watchdog_task() will explicitly + * request and refresh ethtool stats + **/ +void iavf_schedule_request_stats(struct iavf_adapter *adapter) +{ + adapter->aq_required |= IAVF_FLAG_AQ_REQUEST_STATS; + mod_delayed_work(iavf_wq, &adapter->watchdog_task, 0); +} + /** * iavf_tx_timeout - Respond to a Tx Hang * @netdev: network interface device structure @@ -1709,6 +1722,11 @@ static int iavf_process_aq_command(struct iavf_adapter *adapter) iavf_del_adv_rss_cfg(adapter); return 0; } + if (adapter->aq_required & IAVF_FLAG_AQ_REQUEST_STATS) { + iavf_request_stats(adapter); + return 0; + } + return -EAGAIN; } diff --git a/drivers/net/ethernet/intel/iavf/iavf_virtchnl.c b/drivers/net/ethernet/intel/iavf/iavf_virtchnl.c index 8c3f0f77cb574..8421cbe6a1973 100644 --- a/drivers/net/ethernet/intel/iavf/iavf_virtchnl.c +++ b/drivers/net/ethernet/intel/iavf/iavf_virtchnl.c @@ -784,6 +784,8 @@ void iavf_request_stats(struct iavf_adapter *adapter) /* no error message, this isn't crucial */ return; } + + adapter->aq_required &= ~IAVF_FLAG_AQ_REQUEST_STATS; adapter->current_op = VIRTCHNL_OP_GET_STATS; vqs.vsi_id = adapter->vsi_res->vsi_id; /* queue maps are ignored for this message - only the vsi is used */ From 5951a2b9812d8227d33f20d1899fae60e4f72c04 Mon Sep 17 00:00:00 2001 From: Brett Creeley Date: Fri, 5 Nov 2021 09:20:25 -0700 Subject: [PATCH 4/4] iavf: Fix VLAN feature flags after VFR When a VF goes through a reset, it's possible for the VF's feature set to change. For example it may lose the VIRTCHNL_VF_OFFLOAD_VLAN capability after VF reset. Unfortunately, the driver doesn't correctly deal with this situation and errors are seen from downing/upping the interface and/or moving the interface in/out of a network namespace. When setting the interface down/up we see the following errors after the VIRTCHNL_VF_OFFLOAD_VLAN capability was taken away from the VF: ice 0000:51:00.1: VF 1 failed opcode 12, retval: -64 iavf 0000:51:09.1: Failed to add VLAN filter, error IAVF_NOT_SUPPORTED ice 0000:51:00.1: VF 1 failed opcode 13, retval: -64 iavf 0000:51:09.1: Failed to delete VLAN filter, error IAVF_NOT_SUPPORTED These add/delete errors are happening because the VLAN filters are tracked internally to the driver and regardless of the VLAN_ALLOWED() setting the driver tries to delete/re-add them over virtchnl. Fix the delete failure by making sure to delete any VLAN filter tracking in the driver when a removal request is made, while preventing the virtchnl request. This makes it so the driver's VLAN list is up to date and the errors are Fix the add failure by making sure the check for VLAN_ALLOWED() during reset is done after the VF receives its capability list from the PF via VIRTCHNL_OP_GET_VF_RESOURCES. If VLAN functionality is not allowed, then prevent requesting re-adding the filters over virtchnl. When moving the interface into a network namespace we see the following errors after the VIRTCHNL_VF_OFFLOAD_VLAN capability was taken away from the VF: iavf 0000:51:09.1 enp81s0f1v1: NIC Link is Up Speed is 25 Gbps Full Duplex iavf 0000:51:09.1 temp_27: renamed from enp81s0f1v1 iavf 0000:51:09.1 mgmt: renamed from temp_27 iavf 0000:51:09.1 dev27: set_features() failed (-22); wanted 0x020190001fd54833, left 0x020190001fd54bb3 These errors are happening because we aren't correctly updating the netdev capabilities and dealing with ndo_fix_features() and ndo_set_features() correctly. Fix this by only reporting errors in the driver's ndo_set_features() callback when VIRTCHNL_VF_OFFLOAD_VLAN is not allowed and any attempt to enable the VLAN features is made. Also, make sure to disable VLAN insertion, filtering, and stripping since the VIRTCHNL_VF_OFFLOAD_VLAN flag applies to all of them and not just VLAN stripping. Also, after we process the capabilities in the VF reset path, make sure to call netdev_update_features() in case the capabilities have changed in order to update the netdev's feature set to match the VF's actual capabilities. Lastly, make sure to always report success on VLAN filter delete when VIRTCHNL_VF_OFFLOAD_VLAN is not supported. The changed flow in iavf_del_vlans() allows the stack to delete previosly existing VLAN filters even if VLAN filtering is not allowed. This makes it so the VLAN filter list is up to date. Fixes: 8774370d268f ("i40e/i40evf: support for VF VLAN tag stripping control") Signed-off-by: Brett Creeley Tested-by: Konrad Jankowski Signed-off-by: Tony Nguyen --- drivers/net/ethernet/intel/iavf/iavf.h | 1 + drivers/net/ethernet/intel/iavf/iavf_main.c | 33 ++++++-------- .../net/ethernet/intel/iavf/iavf_virtchnl.c | 45 +++++++++++++++++-- 3 files changed, 56 insertions(+), 23 deletions(-) diff --git a/drivers/net/ethernet/intel/iavf/iavf.h b/drivers/net/ethernet/intel/iavf/iavf.h index bb9cc227d1e1c..3789269ce741d 100644 --- a/drivers/net/ethernet/intel/iavf/iavf.h +++ b/drivers/net/ethernet/intel/iavf/iavf.h @@ -503,4 +503,5 @@ void iavf_add_adv_rss_cfg(struct iavf_adapter *adapter); void iavf_del_adv_rss_cfg(struct iavf_adapter *adapter); struct iavf_mac_filter *iavf_add_filter(struct iavf_adapter *adapter, const u8 *macaddr); +int iavf_lock_timeout(struct mutex *lock, unsigned int msecs); #endif /* _IAVF_H_ */ diff --git a/drivers/net/ethernet/intel/iavf/iavf_main.c b/drivers/net/ethernet/intel/iavf/iavf_main.c index 8e96ae746c3df..14934a7a13efd 100644 --- a/drivers/net/ethernet/intel/iavf/iavf_main.c +++ b/drivers/net/ethernet/intel/iavf/iavf_main.c @@ -147,7 +147,7 @@ enum iavf_status iavf_free_virt_mem_d(struct iavf_hw *hw, * * Returns 0 on success, negative on failure **/ -static int iavf_lock_timeout(struct mutex *lock, unsigned int msecs) +int iavf_lock_timeout(struct mutex *lock, unsigned int msecs) { unsigned int wait, delay = 10; @@ -717,13 +717,11 @@ static void iavf_del_vlan(struct iavf_adapter *adapter, u16 vlan) **/ static void iavf_restore_filters(struct iavf_adapter *adapter) { - /* re-add all VLAN filters */ - if (VLAN_ALLOWED(adapter)) { - u16 vid; + u16 vid; - for_each_set_bit(vid, adapter->vsi.active_vlans, VLAN_N_VID) - iavf_add_vlan(adapter, vid); - } + /* re-add all VLAN filters */ + for_each_set_bit(vid, adapter->vsi.active_vlans, VLAN_N_VID) + iavf_add_vlan(adapter, vid); } /** @@ -758,9 +756,6 @@ static int iavf_vlan_rx_kill_vid(struct net_device *netdev, { struct iavf_adapter *adapter = netdev_priv(netdev); - if (!VLAN_ALLOWED(adapter)) - return -EIO; - iavf_del_vlan(adapter, vid); clear_bit(vid, adapter->vsi.active_vlans); @@ -2191,7 +2186,6 @@ static void iavf_reset_task(struct work_struct *work) struct net_device *netdev = adapter->netdev; struct iavf_hw *hw = &adapter->hw; struct iavf_mac_filter *f, *ftmp; - struct iavf_vlan_filter *vlf; struct iavf_cloud_filter *cf; u32 reg_val; int i = 0, err; @@ -2332,11 +2326,6 @@ static void iavf_reset_task(struct work_struct *work) list_for_each_entry(f, &adapter->mac_filter_list, list) { f->add = true; } - /* re-add all VLAN filters */ - list_for_each_entry(vlf, &adapter->vlan_filter_list, list) { - vlf->add = true; - } - spin_unlock_bh(&adapter->mac_vlan_list_lock); /* check if TCs are running and re-add all cloud filters */ @@ -2350,7 +2339,6 @@ static void iavf_reset_task(struct work_struct *work) spin_unlock_bh(&adapter->cloud_filter_list_lock); adapter->aq_required |= IAVF_FLAG_AQ_ADD_MAC_FILTER; - adapter->aq_required |= IAVF_FLAG_AQ_ADD_VLAN_FILTER; adapter->aq_required |= IAVF_FLAG_AQ_ADD_CLOUD_FILTER; iavf_misc_irq_enable(adapter); @@ -3462,11 +3450,16 @@ static int iavf_set_features(struct net_device *netdev, { struct iavf_adapter *adapter = netdev_priv(netdev); - /* Don't allow changing VLAN_RX flag when adapter is not capable - * of VLAN offload + /* Don't allow enabling VLAN features when adapter is not capable + * of VLAN offload/filtering */ if (!VLAN_ALLOWED(adapter)) { - if ((netdev->features ^ features) & NETIF_F_HW_VLAN_CTAG_RX) + netdev->hw_features &= ~(NETIF_F_HW_VLAN_CTAG_RX | + NETIF_F_HW_VLAN_CTAG_TX | + NETIF_F_HW_VLAN_CTAG_FILTER); + if (features & (NETIF_F_HW_VLAN_CTAG_RX | + NETIF_F_HW_VLAN_CTAG_TX | + NETIF_F_HW_VLAN_CTAG_FILTER)) return -EINVAL; } else if ((netdev->features ^ features) & NETIF_F_HW_VLAN_CTAG_RX) { if (features & NETIF_F_HW_VLAN_CTAG_RX) diff --git a/drivers/net/ethernet/intel/iavf/iavf_virtchnl.c b/drivers/net/ethernet/intel/iavf/iavf_virtchnl.c index 8421cbe6a1973..d60bf7c212006 100644 --- a/drivers/net/ethernet/intel/iavf/iavf_virtchnl.c +++ b/drivers/net/ethernet/intel/iavf/iavf_virtchnl.c @@ -607,7 +607,7 @@ void iavf_add_vlans(struct iavf_adapter *adapter) if (f->add) count++; } - if (!count) { + if (!count || !VLAN_ALLOWED(adapter)) { adapter->aq_required &= ~IAVF_FLAG_AQ_ADD_VLAN_FILTER; spin_unlock_bh(&adapter->mac_vlan_list_lock); return; @@ -673,9 +673,19 @@ void iavf_del_vlans(struct iavf_adapter *adapter) spin_lock_bh(&adapter->mac_vlan_list_lock); - list_for_each_entry(f, &adapter->vlan_filter_list, list) { - if (f->remove) + list_for_each_entry_safe(f, ftmp, &adapter->vlan_filter_list, list) { + /* since VLAN capabilities are not allowed, we dont want to send + * a VLAN delete request because it will most likely fail and + * create unnecessary errors/noise, so just free the VLAN + * filters marked for removal to enable bailing out before + * sending a virtchnl message + */ + if (f->remove && !VLAN_ALLOWED(adapter)) { + list_del(&f->list); + kfree(f); + } else if (f->remove) { count++; + } } if (!count) { adapter->aq_required &= ~IAVF_FLAG_AQ_DEL_VLAN_FILTER; @@ -1724,8 +1734,37 @@ void iavf_virtchnl_completion(struct iavf_adapter *adapter, } spin_lock_bh(&adapter->mac_vlan_list_lock); iavf_add_filter(adapter, adapter->hw.mac.addr); + + if (VLAN_ALLOWED(adapter)) { + if (!list_empty(&adapter->vlan_filter_list)) { + struct iavf_vlan_filter *vlf; + + /* re-add all VLAN filters over virtchnl */ + list_for_each_entry(vlf, + &adapter->vlan_filter_list, + list) + vlf->add = true; + + adapter->aq_required |= + IAVF_FLAG_AQ_ADD_VLAN_FILTER; + } + } + spin_unlock_bh(&adapter->mac_vlan_list_lock); iavf_process_config(adapter); + + /* unlock crit_lock before acquiring rtnl_lock as other + * processes holding rtnl_lock could be waiting for the same + * crit_lock + */ + mutex_unlock(&adapter->crit_lock); + rtnl_lock(); + netdev_update_features(adapter->netdev); + rtnl_unlock(); + if (iavf_lock_timeout(&adapter->crit_lock, 10000)) + dev_warn(&adapter->pdev->dev, "failed to acquire crit_lock in %s\n", + __FUNCTION__); + } break; case VIRTCHNL_OP_ENABLE_QUEUES: