| 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