iavf: Fix promiscuous mode configuration flow messages

Currently when configuring promiscuous mode on the AVF we detect a
change in the netdev->flags. We use IFF_PROMISC and IFF_ALLMULTI to
determine whether or not we need to request/release promiscuous mode
and/or multicast promiscuous mode. The problem is that the AQ calls for
setting/clearing promiscuous/multicast mode are treated separately. This
leads to a case where we can trigger two promiscuous mode AQ calls in
a row with the incorrect state. To fix this make a few changes.

Use IAVF_FLAG_AQ_CONFIGURE_PROMISC_MODE instead of the previous
IAVF_FLAG_AQ_[REQUEST|RELEASE]_[PROMISC|ALLMULTI] flags.

In iavf_set_rx_mode() detect if there is a change in the
netdev->flags in comparison with adapter->flags and set the
IAVF_FLAG_AQ_CONFIGURE_PROMISC_MODE aq_required bit. Then in
iavf_process_aq_command() only check for IAVF_FLAG_CONFIGURE_PROMISC_MODE
and call iavf_set_promiscuous() if it's set.

In iavf_set_promiscuous() check again to see which (if any) promiscuous
mode bits have changed when comparing the netdev->flags with the
adapter->flags. Use this to set the flags which get sent to the PF
driver.

Add a spinlock that is used for updating current_netdev_promisc_flags
and only allows one promiscuous mode AQ at a time.

[1] Fixes the fact that we will only have one AQ call in the aq_required
queue at any one time.

[2] Streamlines the change in promiscuous mode to only set one AQ
required bit.

[3] This allows us to keep track of the current state of the flags and
also makes it so we can take the most recent netdev->flags promiscuous
mode state.

[4] This fixes the problem where a change in the netdev->flags can cause
IAVF_FLAG_AQ_CONFIGURE_PROMISC_MODE to be set in iavf_set_rx_mode(),
but cleared in iavf_set_promiscuous() before the change is ever made via
AQ call.

Fixes: 47d3483988f6 ("i40evf: Add driver support for promiscuous mode")
Signed-off-by: Brett Creeley <brett.creeley@intel.com>
Signed-off-by: Ahmed Zaki <ahmed.zaki@intel.com>
Tested-by: Rafal Romanowski <rafal.romanowski@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 85fba85..738e256 100644 (file)
--- a/drivers/net/ethernet/intel/iavf/iavf.h
+++ b/drivers/net/ethernet/intel/iavf/iavf.h
@@ -298,8 +298,6 @@ struct iavf_adapter {
 #define IAVF_FLAG_CLIENT_NEEDS_OPEN            BIT(10)
 #define IAVF_FLAG_CLIENT_NEEDS_CLOSE           BIT(11)
 #define IAVF_FLAG_CLIENT_NEEDS_L2_PARAMS       BIT(12)
-#define IAVF_FLAG_PROMISC_ON                   BIT(13)
-#define IAVF_FLAG_ALLMULTI_ON                  BIT(14)
 #define IAVF_FLAG_LEGACY_RX                    BIT(15)
 #define IAVF_FLAG_REINIT_ITR_NEEDED            BIT(16)
 #define IAVF_FLAG_QUEUES_DISABLED              BIT(17)
@@ -325,10 +323,7 @@ struct iavf_adapter {
 #define IAVF_FLAG_AQ_SET_HENA                  BIT_ULL(12)
 #define IAVF_FLAG_AQ_SET_RSS_KEY               BIT_ULL(13)
 #define IAVF_FLAG_AQ_SET_RSS_LUT               BIT_ULL(14)
-#define IAVF_FLAG_AQ_REQUEST_PROMISC           BIT_ULL(15)
-#define IAVF_FLAG_AQ_RELEASE_PROMISC           BIT_ULL(16)
-#define IAVF_FLAG_AQ_REQUEST_ALLMULTI          BIT_ULL(17)
-#define IAVF_FLAG_AQ_RELEASE_ALLMULTI          BIT_ULL(18)
+#define IAVF_FLAG_AQ_CONFIGURE_PROMISC_MODE    BIT_ULL(15)
 #define IAVF_FLAG_AQ_ENABLE_VLAN_STRIPPING     BIT_ULL(19)
 #define IAVF_FLAG_AQ_DISABLE_VLAN_STRIPPING    BIT_ULL(20)
 #define IAVF_FLAG_AQ_ENABLE_CHANNELS           BIT_ULL(21)
@@ -365,6 +360,12 @@ struct iavf_adapter {
        (IAVF_EXTENDED_CAP_SEND_VLAN_V2 |               \
         IAVF_EXTENDED_CAP_RECV_VLAN_V2)
 
+       /* Lock to prevent possible clobbering of
+        * current_netdev_promisc_flags
+        */
+       spinlock_t current_netdev_promisc_flags_lock;
+       netdev_features_t current_netdev_promisc_flags;
+
        /* OS defined structs */
        struct net_device *netdev;
        struct pci_dev *pdev;
@@ -551,7 +552,8 @@ void iavf_add_ether_addrs(struct iavf_adapter *adapter);
 void iavf_del_ether_addrs(struct iavf_adapter *adapter);
 void iavf_add_vlans(struct iavf_adapter *adapter);
 void iavf_del_vlans(struct iavf_adapter *adapter);
-void iavf_set_promiscuous(struct iavf_adapter *adapter, int flags);
+void iavf_set_promiscuous(struct iavf_adapter *adapter);
+bool iavf_promiscuous_mode_changed(struct iavf_adapter *adapter);
 void iavf_request_stats(struct iavf_adapter *adapter);
 int iavf_request_reset(struct iavf_adapter *adapter);
 void iavf_get_hena(struct iavf_adapter *adapter);

diff --git a/drivers/net/ethernet/intel/iavf/iavf_main.c b/drivers/net/ethernet/intel/iavf/iavf_main.c
index 7b300c8..36b94ee 100644 (file)
--- a/drivers/net/ethernet/intel/iavf/iavf_main.c
+++ b/drivers/net/ethernet/intel/iavf/iavf_main.c
@@ -1188,6 +1188,16 @@ static int iavf_addr_unsync(struct net_device *netdev, const u8 *addr)
        return 0;
 }
 
+/**
+ * iavf_promiscuous_mode_changed - check if promiscuous mode bits changed
+ * @adapter: device specific adapter
+ */
+bool iavf_promiscuous_mode_changed(struct iavf_adapter *adapter)
+{
+       return (adapter->current_netdev_promisc_flags ^ adapter->netdev->flags) &
+               (IFF_PROMISC | IFF_ALLMULTI);
+}
+
 /**
  * iavf_set_rx_mode - NDO callback to set the netdev filters
  * @netdev: network interface device structure
@@ -1201,19 +1211,10 @@ static void iavf_set_rx_mode(struct net_device *netdev)
        __dev_mc_sync(netdev, iavf_addr_sync, iavf_addr_unsync);
        spin_unlock_bh(&adapter->mac_vlan_list_lock);
 
-       if (netdev->flags & IFF_PROMISC &&
-           !(adapter->flags & IAVF_FLAG_PROMISC_ON))
-               adapter->aq_required |= IAVF_FLAG_AQ_REQUEST_PROMISC;
-       else if (!(netdev->flags & IFF_PROMISC) &&
-                adapter->flags & IAVF_FLAG_PROMISC_ON)
-               adapter->aq_required |= IAVF_FLAG_AQ_RELEASE_PROMISC;
-
-       if (netdev->flags & IFF_ALLMULTI &&
-           !(adapter->flags & IAVF_FLAG_ALLMULTI_ON))
-               adapter->aq_required |= IAVF_FLAG_AQ_REQUEST_ALLMULTI;
-       else if (!(netdev->flags & IFF_ALLMULTI) &&
-                adapter->flags & IAVF_FLAG_ALLMULTI_ON)
-               adapter->aq_required |= IAVF_FLAG_AQ_RELEASE_ALLMULTI;
+       spin_lock_bh(&adapter->current_netdev_promisc_flags_lock);
+       if (iavf_promiscuous_mode_changed(adapter))
+               adapter->aq_required |= IAVF_FLAG_AQ_CONFIGURE_PROMISC_MODE;
+       spin_unlock_bh(&adapter->current_netdev_promisc_flags_lock);
 }
 
 /**
@@ -2163,19 +2164,8 @@ static int iavf_process_aq_command(struct iavf_adapter *adapter)
                return 0;
        }
 
-       if (adapter->aq_required & IAVF_FLAG_AQ_REQUEST_PROMISC) {
-               iavf_set_promiscuous(adapter, FLAG_VF_UNICAST_PROMISC |
-                                      FLAG_VF_MULTICAST_PROMISC);
-               return 0;
-       }
-
-       if (adapter->aq_required & IAVF_FLAG_AQ_REQUEST_ALLMULTI) {
-               iavf_set_promiscuous(adapter, FLAG_VF_MULTICAST_PROMISC);
-               return 0;
-       }
-       if ((adapter->aq_required & IAVF_FLAG_AQ_RELEASE_PROMISC) ||
-           (adapter->aq_required & IAVF_FLAG_AQ_RELEASE_ALLMULTI)) {
-               iavf_set_promiscuous(adapter, 0);
+       if (adapter->aq_required & IAVF_FLAG_AQ_CONFIGURE_PROMISC_MODE) {
+               iavf_set_promiscuous(adapter);
                return 0;
        }
 
@@ -4971,6 +4961,7 @@ static int iavf_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
        spin_lock_init(&adapter->cloud_filter_list_lock);
        spin_lock_init(&adapter->fdir_fltr_lock);
        spin_lock_init(&adapter->adv_rss_lock);
+       spin_lock_init(&adapter->current_netdev_promisc_flags_lock);
 
        INIT_LIST_HEAD(&adapter->mac_filter_list);
        INIT_LIST_HEAD(&adapter->vlan_filter_list);

diff --git a/drivers/net/ethernet/intel/iavf/iavf_virtchnl.c b/drivers/net/ethernet/intel/iavf/iavf_virtchnl.c
index f9727e9..0b97b42 100644 (file)
--- a/drivers/net/ethernet/intel/iavf/iavf_virtchnl.c
+++ b/drivers/net/ethernet/intel/iavf/iavf_virtchnl.c
@@ -936,14 +936,14 @@ void iavf_del_vlans(struct iavf_adapter *adapter)
 /**
  * iavf_set_promiscuous
  * @adapter: adapter structure
- * @flags: bitmask to control unicast/multicast promiscuous.
  *
  * Request that the PF enable promiscuous mode for our VSI.
  **/
-void iavf_set_promiscuous(struct iavf_adapter *adapter, int flags)
+void iavf_set_promiscuous(struct iavf_adapter *adapter)
 {
+       struct net_device *netdev = adapter->netdev;
        struct virtchnl_promisc_info vpi;
-       int promisc_all;
+       unsigned int flags;
 
        if (adapter->current_op != VIRTCHNL_OP_UNKNOWN) {
                /* bail because we already have a command pending */
@@ -952,36 +952,57 @@ void iavf_set_promiscuous(struct iavf_adapter *adapter, int flags)
                return;
        }
 
-       promisc_all = FLAG_VF_UNICAST_PROMISC |
-                     FLAG_VF_MULTICAST_PROMISC;
-       if ((flags & promisc_all) == promisc_all) {
-               adapter->flags |= IAVF_FLAG_PROMISC_ON;
-               adapter->aq_required &= ~IAVF_FLAG_AQ_REQUEST_PROMISC;
-               dev_info(&adapter->pdev->dev, "Entering promiscuous mode\n");
-       }
+       /* prevent changes to promiscuous flags */
+       spin_lock_bh(&adapter->current_netdev_promisc_flags_lock);
 
-       if (flags & FLAG_VF_MULTICAST_PROMISC) {
-               adapter->flags |= IAVF_FLAG_ALLMULTI_ON;
-               adapter->aq_required &= ~IAVF_FLAG_AQ_REQUEST_ALLMULTI;
-               dev_info(&adapter->pdev->dev, "%s is entering multicast promiscuous mode\n",
-                        adapter->netdev->name);
+       /* sanity check to prevent duplicate AQ calls */
+       if (!iavf_promiscuous_mode_changed(adapter)) {
+               adapter->aq_required &= ~IAVF_FLAG_AQ_CONFIGURE_PROMISC_MODE;
+               dev_dbg(&adapter->pdev->dev, "No change in promiscuous mode\n");
+               /* allow changes to promiscuous flags */
+               spin_unlock_bh(&adapter->current_netdev_promisc_flags_lock);
+               return;
        }
 
-       if (!flags) {
-               if (adapter->flags & IAVF_FLAG_PROMISC_ON) {
-                       adapter->flags &= ~IAVF_FLAG_PROMISC_ON;
-                       adapter->aq_required &= ~IAVF_FLAG_AQ_RELEASE_PROMISC;
-                       dev_info(&adapter->pdev->dev, "Leaving promiscuous mode\n");
-               }
+       /* there are 2 bits, but only 3 states */
+       if (!(netdev->flags & IFF_PROMISC) &&
+           netdev->flags & IFF_ALLMULTI) {
+               /* State 1  - only multicast promiscuous mode enabled
+                * - !IFF_PROMISC && IFF_ALLMULTI
+                */
+               flags = FLAG_VF_MULTICAST_PROMISC;
+               adapter->current_netdev_promisc_flags |= IFF_ALLMULTI;
+               adapter->current_netdev_promisc_flags &= ~IFF_PROMISC;
+               dev_info(&adapter->pdev->dev, "Entering multicast promiscuous mode\n");
+       } else if (!(netdev->flags & IFF_PROMISC) &&
+                  !(netdev->flags & IFF_ALLMULTI)) {
+               /* State 2 - unicast/multicast promiscuous mode disabled
+                * - !IFF_PROMISC && !IFF_ALLMULTI
+                */
+               flags = 0;
+               adapter->current_netdev_promisc_flags &=
+                       ~(IFF_PROMISC | IFF_ALLMULTI);
+               dev_info(&adapter->pdev->dev, "Leaving promiscuous mode\n");
+       } else {
+               /* State 3 - unicast/multicast promiscuous mode enabled
+                * - IFF_PROMISC && IFF_ALLMULTI
+                * - IFF_PROMISC && !IFF_ALLMULTI
+                */
+               flags = FLAG_VF_UNICAST_PROMISC | FLAG_VF_MULTICAST_PROMISC;
+               adapter->current_netdev_promisc_flags |= IFF_PROMISC;
+               if (netdev->flags & IFF_ALLMULTI)
+                       adapter->current_netdev_promisc_flags |= IFF_ALLMULTI;
+               else
+                       adapter->current_netdev_promisc_flags &= ~IFF_ALLMULTI;
 
-               if (adapter->flags & IAVF_FLAG_ALLMULTI_ON) {
-                       adapter->flags &= ~IAVF_FLAG_ALLMULTI_ON;
-                       adapter->aq_required &= ~IAVF_FLAG_AQ_RELEASE_ALLMULTI;
-                       dev_info(&adapter->pdev->dev, "%s is leaving multicast promiscuous mode\n",
-                                adapter->netdev->name);
-               }
+               dev_info(&adapter->pdev->dev, "Entering promiscuous mode\n");
        }
 
+       adapter->aq_required &= ~IAVF_FLAG_AQ_CONFIGURE_PROMISC_MODE;
+
+       /* allow changes to promiscuous flags */
+       spin_unlock_bh(&adapter->current_netdev_promisc_flags_lock);
+
        adapter->current_op = VIRTCHNL_OP_CONFIG_PROMISCUOUS_MODE;
        vpi.vsi_id = adapter->vsi_res->vsi_id;
        vpi.flags = flags;