Commit 221465de authored by Brett Creeley's avatar Brett Creeley Committed by Tony Nguyen
Browse files

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: 47d34839 ("i40evf: Add driver support for promiscuous mode")
Signed-off-by: default avatarBrett Creeley <brett.creeley@intel.com>
Signed-off-by: default avatarAhmed Zaki <ahmed.zaki@intel.com>
Tested-by: default avatarRafal Romanowski <rafal.romanowski@intel.com>
Signed-off-by: default avatarTony Nguyen <anthony.l.nguyen@intel.com>
parent 5ca636d9

drivers/net/ethernet/intel/iavf/iavf.h

+9 −7
Original line number Diff line number Diff line
@@ -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);

drivers/net/ethernet/intel/iavf/iavf_main.c

+17 −26
Original line number Diff line number Diff line
@@ -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);

drivers/net/ethernet/intel/iavf/iavf_virtchnl.c

+48 −27
Original line number Diff line number Diff line
@@ -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;

	/* 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;