iavf: Fix locking for VIRTCHNL_OP_GET_OFFLOAD_VLAN_V2_CAPS

iavf_virtchnl_completion is called under crit_lock but when
the code for VIRTCHNL_OP_GET_OFFLOAD_VLAN_V2_CAPS is called,
this lock is released in order to obtain rtnl_lock to avoid
ABBA deadlock with unregister_netdev.

Along with the new way iavf_remove behaves, there exist
many risks related to the lock release and attmepts to regrab
it. The driver faces crashes related to races between
unregister_netdev and netdev_update_features. Yet another
risk is that the driver could already obtain the crit_lock
in order to destroy it and iavf_virtchnl_completion could
crash or block forever.

Make iavf_virtchnl_completion never relock crit_lock in it's
call paths.

Extract rtnl_lock locking logic to the driver for
unregister_netdev in order to set the netdev_registered flag
inside the lock.

Introduce a new flag that will inform adminq_task to perform
the code from VIRTCHNL_OP_GET_OFFLOAD_VLAN_V2_CAPS right after
it finishes processing messages. Guard this code with remove
flags so it's never called when the driver is in remove state.

Fixes: 5951a2b9812d ("iavf: Fix VLAN feature flags after VFR")
Signed-off-by: Slawomir Laba <slawomirx.laba@intel.com>
Signed-off-by: Phani Burra <phani.r.burra@intel.com>
Signed-off-by: Jacob Keller <jacob.e.keller@intel.com>
Signed-off-by: Mateusz Palczewski <mateusz.palczewski@intel.com>
Tested-by: Konrad Jankowski <konrad0.jankowski@intel.com>
Signed-off-by: Tony Nguyen <anthony.l.nguyen@intel.com>

diff --git a/drivers/net/ethernet/intel/iavf/iavf.h b/drivers/net/ethernet/intel/iavf/iavf.h
index f259fd517b2cd26101c56ec5346661ccdfc1ba84..89423947ee65fafa197aef33f508e2c9c6dfe3a8 100644 (file)
--- a/drivers/net/ethernet/intel/iavf/iavf.h
+++ b/drivers/net/ethernet/intel/iavf/iavf.h
@@ -287,6 +287,7 @@ 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)
 /* duplicates for common code */
 #define IAVF_FLAG_DCB_ENABLED                  0
        /* flags for admin queue service task */

diff --git a/drivers/net/ethernet/intel/iavf/iavf_main.c b/drivers/net/ethernet/intel/iavf/iavf_main.c
index be51da978e7cde138f4f2807cb2760e89f7030ff..67349d24dc9065b3ecfa05f059a3f4c66a74deef 100644 (file)
--- a/drivers/net/ethernet/intel/iavf/iavf_main.c
+++ b/drivers/net/ethernet/intel/iavf/iavf_main.c
@@ -2879,6 +2879,24 @@ 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)
@@ -4606,8 +4624,10 @@ static void iavf_remove(struct pci_dev *pdev)
        cancel_delayed_work_sync(&adapter->watchdog_task);
 
        if (adapter->netdev_registered) {
-               unregister_netdev(netdev);
+               rtnl_lock();
+               unregister_netdevice(netdev);
                adapter->netdev_registered = false;
+               rtnl_unlock();
        }
        if (CLIENT_ALLOWED(adapter)) {
                err = iavf_lan_del_device(adapter);

diff --git a/drivers/net/ethernet/intel/iavf/iavf_virtchnl.c b/drivers/net/ethernet/intel/iavf/iavf_virtchnl.c
index 5ee1d118fd30f2b73232481de0d2651e84cb9367..88844d68e1504fa91f9e9461e1146ab2a1f26ebc 100644 (file)
--- a/drivers/net/ethernet/intel/iavf/iavf_virtchnl.c
+++ b/drivers/net/ethernet/intel/iavf/iavf_virtchnl.c
@@ -2146,29 +2146,7 @@ void iavf_virtchnl_completion(struct iavf_adapter *adapter,
                                     sizeof(adapter->vlan_v2_caps)));
 
                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);
-               /* VLAN capabilities can change during VFR, so make sure to
-                * update the netdev features with the new capabilities
-                */
-               rtnl_lock();
-               netdev_update_features(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__);
-
-               /* 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;
                }
                break;
        case VIRTCHNL_OP_ENABLE_QUEUES: