lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening PHC | |
Open Source and information security mailing list archives
| ||
|
Date: Thu, 15 Dec 2022 23:50:49 +0100 From: Michal Schmidt <mschmidt@...hat.com> To: intel-wired-lan@...ts.osuosl.org Cc: Ivan Vecera <ivecera@...hat.com>, netdev@...r.kernel.org, Mateusz Palczewski <mateusz.palczewski@...el.com>, Patryk Piotrowski <patryk.piotrowski@...el.com> Subject: [PATCH net 2/2] iavf: avoid taking rtnl_lock in adminq_task adminq_task processes virtchnl completions. iavf_set_mac() needs virtchnl communication to progress while it holds rtnl_lock. So adminq_task must not take rtnl_lock. Do the handling of netdev features updates in a new work, features_task. The new work cannot run on the same ordered workqueue as adminq_task. The system-wide system_unbound_wq will do. iavf_set_queue_vlan_tag_loc(), which iterates through queues, must run under crit_lock to prevent a concurrent iavf_free_queues() possibly called from watchdog_task or reset_task. IAVF_FLAG_SETUP_NETDEV_FEATURES becomes unnecessary. features_task can be queued directly from iavf_virtchnl_completion(). Fixes: 35a2443d0910 ("iavf: Add waiting for response from PF in set mac") Signed-off-by: Michal Schmidt <mschmidt@...hat.com> --- drivers/net/ethernet/intel/iavf/iavf.h | 2 +- drivers/net/ethernet/intel/iavf/iavf_main.c | 49 ++++++++++++------- .../net/ethernet/intel/iavf/iavf_virtchnl.c | 6 ++- 3 files changed, 37 insertions(+), 20 deletions(-) diff --git a/drivers/net/ethernet/intel/iavf/iavf.h b/drivers/net/ethernet/intel/iavf/iavf.h index 2a9f1eeeb701..7dfd6dac74e4 100644 --- a/drivers/net/ethernet/intel/iavf/iavf.h +++ b/drivers/net/ethernet/intel/iavf/iavf.h @@ -252,6 +252,7 @@ struct iavf_adapter { struct workqueue_struct *wq; struct work_struct reset_task; struct work_struct adminq_task; + struct work_struct features_task; struct delayed_work client_task; wait_queue_head_t down_waitqueue; wait_queue_head_t vc_waitqueue; @@ -297,7 +298,6 @@ struct iavf_adapter { #define IAVF_FLAG_LEGACY_RX BIT(15) #define IAVF_FLAG_REINIT_ITR_NEEDED BIT(16) #define IAVF_FLAG_QUEUES_DISABLED BIT(17) -#define IAVF_FLAG_SETUP_NETDEV_FEATURES BIT(18) #define IAVF_FLAG_REINIT_MSIX_NEEDED BIT(20) /* duplicates for common code */ #define IAVF_FLAG_DCB_ENABLED 0 diff --git a/drivers/net/ethernet/intel/iavf/iavf_main.c b/drivers/net/ethernet/intel/iavf/iavf_main.c index e7380f1b4acc..e53f5262c047 100644 --- a/drivers/net/ethernet/intel/iavf/iavf_main.c +++ b/drivers/net/ethernet/intel/iavf/iavf_main.c @@ -3187,6 +3187,35 @@ static void iavf_reset_task(struct work_struct *work) rtnl_unlock(); } +/** + * iavf_features_task - update netdev features after caps negotiation + * @work: pointer to work_struct + * + * After negotiating VLAN caps with the PF, we need to update our netdev + * features, but this requires rtnl_lock. We cannot take it directly in + * adminq_task - we might deadlock with iavf_set_mac, which waits on + * virtchnl communication while holding rtnl_lock. + * So the features are updated here, using a different workqueue. + **/ +static void iavf_features_task(struct work_struct *work) +{ + struct iavf_adapter *adapter = container_of(work, + struct iavf_adapter, + features_task); + struct net_device *netdev = adapter->netdev; + + rtnl_lock(); + netdev_update_features(netdev); + rtnl_unlock(); + /* Request VLAN offload settings */ + if (VLAN_V2_ALLOWED(adapter)) + iavf_set_vlan_offload_features(adapter, 0, netdev->features); + + mutex_lock(&adapter->crit_lock); + iavf_set_queue_vlan_tag_loc(adapter); + mutex_unlock(&adapter->crit_lock); +} + /** * iavf_adminq_task - worker thread to clean the admin queue * @work: pointer to work_struct containing our data @@ -3233,24 +3262,6 @@ static void iavf_adminq_task(struct work_struct *work) } while (pending); mutex_unlock(&adapter->crit_lock); - if ((adapter->flags & IAVF_FLAG_SETUP_NETDEV_FEATURES)) { - if (adapter->netdev_registered || - !test_bit(__IAVF_IN_REMOVE_TASK, &adapter->crit_section)) { - struct net_device *netdev = adapter->netdev; - - rtnl_lock(); - netdev_update_features(netdev); - rtnl_unlock(); - /* Request VLAN offload settings */ - if (VLAN_V2_ALLOWED(adapter)) - iavf_set_vlan_offload_features - (adapter, 0, netdev->features); - - iavf_set_queue_vlan_tag_loc(adapter); - } - - adapter->flags &= ~IAVF_FLAG_SETUP_NETDEV_FEATURES; - } if ((adapter->flags & (IAVF_FLAG_RESET_PENDING | IAVF_FLAG_RESET_NEEDED)) || adapter->state == __IAVF_RESETTING) @@ -4948,6 +4959,7 @@ static int iavf_probe(struct pci_dev *pdev, const struct pci_device_id *ent) INIT_WORK(&adapter->reset_task, iavf_reset_task); INIT_WORK(&adapter->adminq_task, iavf_adminq_task); + INIT_WORK(&adapter->features_task, iavf_features_task); INIT_DELAYED_WORK(&adapter->watchdog_task, iavf_watchdog_task); INIT_DELAYED_WORK(&adapter->client_task, iavf_client_task); queue_delayed_work(adapter->wq, &adapter->watchdog_task, @@ -5115,6 +5127,7 @@ static void iavf_remove(struct pci_dev *pdev) cancel_delayed_work_sync(&adapter->watchdog_task); cancel_work_sync(&adapter->adminq_task); cancel_delayed_work_sync(&adapter->client_task); + cancel_work_sync(&adapter->features_task); adapter->aq_required = 0; adapter->flags &= ~IAVF_FLAG_REINIT_ITR_NEEDED; diff --git a/drivers/net/ethernet/intel/iavf/iavf_virtchnl.c b/drivers/net/ethernet/intel/iavf/iavf_virtchnl.c index 0752fd67c96e..a644ab3804de 100644 --- a/drivers/net/ethernet/intel/iavf/iavf_virtchnl.c +++ b/drivers/net/ethernet/intel/iavf/iavf_virtchnl.c @@ -2225,7 +2225,6 @@ void iavf_virtchnl_completion(struct iavf_adapter *adapter, sizeof(adapter->vlan_v2_caps))); iavf_process_config(adapter); - adapter->flags |= IAVF_FLAG_SETUP_NETDEV_FEATURES; was_mac_changed = !ether_addr_equal(netdev->dev_addr, adapter->hw.mac.addr); @@ -2266,6 +2265,11 @@ void iavf_virtchnl_completion(struct iavf_adapter *adapter, adapter->aq_required |= IAVF_FLAG_AQ_ADD_MAC_FILTER | aq_required; + + if (adapter->netdev_registered || + !test_bit(__IAVF_IN_REMOVE_TASK, &adapter->crit_section)) + queue_work(system_unbound_wq, + &adapter->features_task); } break; case VIRTCHNL_OP_ENABLE_QUEUES: -- 2.37.2
Powered by blists - more mailing lists