[dpdk-dev] [PATCH] net/iavf: remove interrupt handler

[dpdk-dev] [PATCH] net/iavf: remove interrupt handler

[Robin Zhang]

For VF hosted by Intel 700 series NICs, internal rx interrupt and adminq
interrupt share the same source, that cause a lot cpu cycles be wasted on
interrupt handler on rx path.

The patch disable pci interrupt and remove the interrupt handler, replace
it with a low frequency(50ms) interrupt polling daemon which is
implemtented by registering an alarm callback periodly.

The virtual channel capability bit VIRTCHNL_VF_OFFLOAD_WB_ON_ITR can be
used to negotiate if iavf PMD needs to enable background alarm or not, so
ideally this change will not impact the case hosted by Intel 800 series
NICS.

Suggested-by: Jingjing Wu <jingjing.wu@intel.com>
Signed-off-by: Qi Zhang <qi.z.zhang@intel.com>
Signed-off-by: Robin Zhang <robinx.zhang@intel.com>
---
 drivers/net/iavf/iavf.h        |  3 +++
 drivers/net/iavf/iavf_ethdev.c | 37 ++++++++++++++--------------------
 drivers/net/iavf/iavf_vchnl.c  | 11 ++++------
 3 files changed, 22 insertions(+), 29 deletions(-)

diff --git a/drivers/net/iavf/iavf.h b/drivers/net/iavf/iavf.h
index b3bd078111..771f3b79d7 100644
--- a/drivers/net/iavf/iavf.h
+++ b/drivers/net/iavf/iavf.h
@@ -69,6 +69,8 @@
 #define IAVF_QUEUE_ITR_INTERVAL_DEFAULT 32 /* 32 us */
 #define IAVF_QUEUE_ITR_INTERVAL_MAX     8160 /* 8160 us */
 
+#define IAVF_ALARM_INTERVAL 50000 /* us */
+
 /* The overhead from MTU to max frame size.
  * Considering QinQ packet, the VLAN tag needs to be counted twice.
  */
@@ -372,6 +374,7 @@ int iavf_config_irq_map_lv(struct iavf_adapter *adapter, uint16_t num,
 void iavf_add_del_all_mac_addr(struct iavf_adapter *adapter, bool add);
 int iavf_dev_link_update(struct rte_eth_dev *dev,
 			__rte_unused int wait_to_complete);
+void iavf_dev_alarm_handler(void *param);
 int iavf_query_stats(struct iavf_adapter *adapter,
 		    struct virtchnl_eth_stats **pstats);
 int iavf_config_promisc(struct iavf_adapter *adapter, bool enable_unicast,
diff --git a/drivers/net/iavf/iavf_ethdev.c b/drivers/net/iavf/iavf_ethdev.c
index 41382c6d66..bbe5b3ddb1 100644
--- a/drivers/net/iavf/iavf_ethdev.c
+++ b/drivers/net/iavf/iavf_ethdev.c
@@ -16,6 +16,7 @@
 #include <rte_interrupts.h>
 #include <rte_debug.h>
 #include <rte_pci.h>
+#include <rte_alarm.h>
 #include <rte_atomic.h>
 #include <rte_eal.h>
 #include <rte_ether.h>
@@ -692,9 +693,9 @@ static int iavf_config_rx_queues_irqs(struct rte_eth_dev *dev,
 			 */
 			vf->msix_base = IAVF_MISC_VEC_ID;
 
-			/* set ITR to max */
+			/* set ITR to default */
 			interval = iavf_calc_itr_interval(
-					IAVF_QUEUE_ITR_INTERVAL_MAX);
+					IAVF_QUEUE_ITR_INTERVAL_DEFAULT);
 			IAVF_WRITE_REG(hw, IAVF_VFINT_DYN_CTL01,
 				       IAVF_VFINT_DYN_CTL01_INTENA_MASK |
 				       (IAVF_ITR_INDEX_DEFAULT <<
@@ -853,9 +854,8 @@ iavf_dev_start(struct rte_eth_dev *dev)
 		PMD_DRV_LOG(ERR, "configure irq failed");
 		goto err_queue;
 	}
-	/* re-enable intr again, because efd assign may change */
+	/* only enable interrupt in rx interrupt mode */
 	if (dev->data->dev_conf.intr_conf.rxq != 0) {
-		rte_intr_disable(intr_handle);
 		rte_intr_enable(intr_handle);
 	}
 
@@ -889,6 +889,9 @@ iavf_dev_stop(struct rte_eth_dev *dev)
 
 	PMD_INIT_FUNC_TRACE();
 
+	if (dev->data->dev_conf.intr_conf.rxq != 0)
+		rte_intr_disable(intr_handle);
+
 	if (adapter->stopped == 1)
 		return 0;
 
@@ -1669,8 +1672,6 @@ iavf_dev_rx_queue_intr_enable(struct rte_eth_dev *dev, uint16_t queue_id)
 
 	IAVF_WRITE_FLUSH(hw);
 
-	rte_intr_ack(&pci_dev->intr_handle);
-
 	return 0;
 }
 
@@ -2201,8 +2202,8 @@ iavf_disable_irq0(struct iavf_hw *hw)
 	IAVF_WRITE_FLUSH(hw);
 }
 
-static void
-iavf_dev_interrupt_handler(void *param)
+void
+iavf_dev_alarm_handler(void *param)
 {
 	struct rte_eth_dev *dev = (struct rte_eth_dev *)param;
 	struct iavf_hw *hw = IAVF_DEV_PRIVATE_TO_HW(dev->data->dev_private);
@@ -2212,6 +2213,9 @@ iavf_dev_interrupt_handler(void *param)
 	iavf_handle_virtchnl_msg(dev);
 
 	iavf_enable_irq0(hw);
+
+	rte_eal_alarm_set(IAVF_ALARM_INTERVAL,
+			  iavf_dev_alarm_handler, dev);
 }
 
 static int
@@ -2314,13 +2318,8 @@ iavf_dev_init(struct rte_eth_dev *eth_dev)
 	rte_ether_addr_copy((struct rte_ether_addr *)hw->mac.addr,
 			&eth_dev->data->mac_addrs[0]);
 
-	/* register callback func to eal lib */
-	rte_intr_callback_register(&pci_dev->intr_handle,
-				   iavf_dev_interrupt_handler,
-				   (void *)eth_dev);
-
-	/* enable uio intr after callback register */
-	rte_intr_enable(&pci_dev->intr_handle);
+	rte_eal_alarm_set(IAVF_ALARM_INTERVAL,
+			  iavf_dev_alarm_handler, eth_dev);
 
 	/* configure and enable device interrupt */
 	iavf_enable_irq0(hw);
@@ -2341,7 +2340,6 @@ iavf_dev_close(struct rte_eth_dev *dev)
 {
 	struct iavf_hw *hw = IAVF_DEV_PRIVATE_TO_HW(dev->data->dev_private);
 	struct rte_pci_device *pci_dev = RTE_ETH_DEV_TO_PCI(dev);
-	struct rte_intr_handle *intr_handle = &pci_dev->intr_handle;
 	struct iavf_adapter *adapter =
 		IAVF_DEV_PRIVATE_TO_ADAPTER(dev->data->dev_private);
 	struct iavf_info *vf = IAVF_DEV_PRIVATE_TO_VF(dev->data->dev_private);
@@ -2364,12 +2362,7 @@ iavf_dev_close(struct rte_eth_dev *dev)
 		iavf_config_promisc(adapter, false, false);
 
 	iavf_shutdown_adminq(hw);
-	/* disable uio intr before callback unregister */
-	rte_intr_disable(intr_handle);
-
-	/* unregister callback func from eal lib */
-	rte_intr_callback_unregister(intr_handle,
-				     iavf_dev_interrupt_handler, dev);
+	rte_eal_alarm_cancel(iavf_dev_alarm_handler, dev);
 	iavf_disable_irq0(hw);
 
 	if (vf->vf_res->vf_cap_flags & VIRTCHNL_VF_OFFLOAD_QOS)
diff --git a/drivers/net/iavf/iavf_vchnl.c b/drivers/net/iavf/iavf_vchnl.c
index 06dc663947..af19dd6412 100644
--- a/drivers/net/iavf/iavf_vchnl.c
+++ b/drivers/net/iavf/iavf_vchnl.c
@@ -13,6 +13,7 @@
 #include <rte_common.h>
 
 #include <rte_debug.h>
+#include <rte_alarm.h>
 #include <rte_atomic.h>
 #include <rte_eal.h>
 #include <rte_ether.h>
@@ -1663,7 +1664,6 @@ iavf_request_queues(struct iavf_adapter *adapter, uint16_t num)
 {
 	struct rte_eth_dev *dev = adapter->eth_dev;
 	struct iavf_info *vf =  IAVF_DEV_PRIVATE_TO_VF(adapter);
-	struct rte_pci_device *pci_dev = RTE_ETH_DEV_TO_PCI(dev);
 	struct virtchnl_vf_res_request vfres;
 	struct iavf_cmd_info args;
 	uint16_t num_queue_pairs;
@@ -1687,13 +1687,10 @@ iavf_request_queues(struct iavf_adapter *adapter, uint16_t num)
 	args.out_buffer = vf->aq_resp;
 	args.out_size = IAVF_AQ_BUF_SZ;
 
-	/*
-	 * disable interrupt to avoid the admin queue message to be read
-	 * before iavf_read_msg_from_pf.
-	 */
-	rte_intr_disable(&pci_dev->intr_handle);
+	rte_eal_alarm_cancel(iavf_dev_alarm_handler, dev);
 	err = iavf_execute_vf_cmd(adapter, &args);
-	rte_intr_enable(&pci_dev->intr_handle);
+	rte_eal_alarm_set(IAVF_ALARM_INTERVAL,
+			  iavf_dev_alarm_handler, dev);
 	if (err) {
 		PMD_DRV_LOG(ERR, "fail to execute command OP_REQUEST_QUEUES");
 		return err;
-- 
2.25.1

Re: [dpdk-dev] [PATCH] net/iavf: remove interrupt handler

[Zhang, Qi Z]

> -----Original Message-----
> From: Zhang, RobinX <robinx.zhang@intel.com>
> Sent: Friday, July 23, 2021 3:47 PM
> To: dev@dpdk.org
> Cc: Wu, Jingjing <jingjing.wu@intel.com>; Xing, Beilei <beilei.xing@intel.com>;
> Zhang, Qi Z <qi.z.zhang@intel.com>; Guo, Junfeng <junfeng.guo@intel.com>;
> Yang, SteveX <stevex.yang@intel.com>; Zhang, RobinX
> <robinx.zhang@intel.com>
> Subject: [PATCH] net/iavf: remove interrupt handler

As you are not going to remove interrupt handler for all the cases, the title is misleading
Better replace it with "enable interrupt polling"

> 
> For VF hosted by Intel 700 series NICs, internal rx interrupt and adminq
> interrupt share the same source, that cause a lot cpu cycles be wasted on
> interrupt handler on rx path.
> 
> The patch disable pci interrupt and remove the interrupt handler, replace it
> with a low frequency(50ms) interrupt polling daemon which is implemtented by
> registering an alarm callback periodly.
> 
> The virtual channel capability bit VIRTCHNL_VF_OFFLOAD_WB_ON_ITR can be
> used to negotiate if iavf PMD needs to enable background alarm or not, so
> ideally this change will not impact the case hosted by Intel 800 series NICS.
> 
> Suggested-by: Jingjing Wu <jingjing.wu@intel.com>
> Signed-off-by: Qi Zhang <qi.z.zhang@intel.com>

No need to add me as the author for this patch but, you can add a reference to the original i40e commit to explain you implement the same logic.

> Signed-off-by: Robin Zhang <robinx.zhang@intel.com>
> ---
>  drivers/net/iavf/iavf.h        |  3 +++
>  drivers/net/iavf/iavf_ethdev.c | 37 ++++++++++++++--------------------
>  drivers/net/iavf/iavf_vchnl.c  | 11 ++++------
>  3 files changed, 22 insertions(+), 29 deletions(-)
> 
> diff --git a/drivers/net/iavf/iavf.h b/drivers/net/iavf/iavf.h index
> b3bd078111..771f3b79d7 100644
> --- a/drivers/net/iavf/iavf.h
> +++ b/drivers/net/iavf/iavf.h
> @@ -69,6 +69,8 @@
>  #define IAVF_QUEUE_ITR_INTERVAL_DEFAULT 32 /* 32 us */
>  #define IAVF_QUEUE_ITR_INTERVAL_MAX     8160 /* 8160 us */
> 
> +#define IAVF_ALARM_INTERVAL 50000 /* us */
> +
>  /* The overhead from MTU to max frame size.
>   * Considering QinQ packet, the VLAN tag needs to be counted twice.
>   */
> @@ -372,6 +374,7 @@ int iavf_config_irq_map_lv(struct iavf_adapter
> *adapter, uint16_t num,  void iavf_add_del_all_mac_addr(struct iavf_adapter
> *adapter, bool add);  int iavf_dev_link_update(struct rte_eth_dev *dev,
>  			__rte_unused int wait_to_complete);
> +void iavf_dev_alarm_handler(void *param);
>  int iavf_query_stats(struct iavf_adapter *adapter,
>  		    struct virtchnl_eth_stats **pstats);  int
> iavf_config_promisc(struct iavf_adapter *adapter, bool enable_unicast, diff
> --git a/drivers/net/iavf/iavf_ethdev.c b/drivers/net/iavf/iavf_ethdev.c index
> 41382c6d66..bbe5b3ddb1 100644
> --- a/drivers/net/iavf/iavf_ethdev.c
> +++ b/drivers/net/iavf/iavf_ethdev.c
> @@ -16,6 +16,7 @@
>  #include <rte_interrupts.h>
>  #include <rte_debug.h>
>  #include <rte_pci.h>
> +#include <rte_alarm.h>
>  #include <rte_atomic.h>
>  #include <rte_eal.h>
>  #include <rte_ether.h>
> @@ -692,9 +693,9 @@ static int iavf_config_rx_queues_irqs(struct
> rte_eth_dev *dev,
>  			 */
>  			vf->msix_base = IAVF_MISC_VEC_ID;
> 
> -			/* set ITR to max */
> +			/* set ITR to default */
>  			interval = iavf_calc_itr_interval(
> -					IAVF_QUEUE_ITR_INTERVAL_MAX);
> +					IAVF_QUEUE_ITR_INTERVAL_DEFAULT);
>  			IAVF_WRITE_REG(hw, IAVF_VFINT_DYN_CTL01,
>  				       IAVF_VFINT_DYN_CTL01_INTENA_MASK |
>  				       (IAVF_ITR_INDEX_DEFAULT <<
> @@ -853,9 +854,8 @@ iavf_dev_start(struct rte_eth_dev *dev)
>  		PMD_DRV_LOG(ERR, "configure irq failed");
>  		goto err_queue;
>  	}
> -	/* re-enable intr again, because efd assign may change */
> +	/* only enable interrupt in rx interrupt mode */
>  	if (dev->data->dev_conf.intr_conf.rxq != 0) {
> -		rte_intr_disable(intr_handle);
>  		rte_intr_enable(intr_handle);
>  	}
> 
> @@ -889,6 +889,9 @@ iavf_dev_stop(struct rte_eth_dev *dev)
> 
>  	PMD_INIT_FUNC_TRACE();
> 
> +	if (dev->data->dev_conf.intr_conf.rxq != 0)
> +		rte_intr_disable(intr_handle);
> +
>  	if (adapter->stopped == 1)
>  		return 0;
> 
> @@ -1669,8 +1672,6 @@ iavf_dev_rx_queue_intr_enable(struct rte_eth_dev
> *dev, uint16_t queue_id)
> 
>  	IAVF_WRITE_FLUSH(hw);
> 
> -	rte_intr_ack(&pci_dev->intr_handle);
> -
>  	return 0;
>  }
> 
> @@ -2201,8 +2202,8 @@ iavf_disable_irq0(struct iavf_hw *hw)
>  	IAVF_WRITE_FLUSH(hw);
>  }
> 
> -static void
> -iavf_dev_interrupt_handler(void *param)
> +void
> +iavf_dev_alarm_handler(void *param)
>  {
>  	struct rte_eth_dev *dev = (struct rte_eth_dev *)param;
>  	struct iavf_hw *hw =
> IAVF_DEV_PRIVATE_TO_HW(dev->data->dev_private);
> @@ -2212,6 +2213,9 @@ iavf_dev_interrupt_handler(void *param)
>  	iavf_handle_virtchnl_msg(dev);
> 
>  	iavf_enable_irq0(hw);
> +
> +	rte_eal_alarm_set(IAVF_ALARM_INTERVAL,
> +			  iavf_dev_alarm_handler, dev);
>  }
> 
>  static int
> @@ -2314,13 +2318,8 @@ iavf_dev_init(struct rte_eth_dev *eth_dev)
>  	rte_ether_addr_copy((struct rte_ether_addr *)hw->mac.addr,
>  			&eth_dev->data->mac_addrs[0]);
> 
> -	/* register callback func to eal lib */
> -	rte_intr_callback_register(&pci_dev->intr_handle,
> -				   iavf_dev_interrupt_handler,
> -				   (void *)eth_dev);
> -
> -	/* enable uio intr after callback register */
> -	rte_intr_enable(&pci_dev->intr_handle);
> +	rte_eal_alarm_set(IAVF_ALARM_INTERVAL,
> +			  iavf_dev_alarm_handler, eth_dev);
> 
>  	/* configure and enable device interrupt */
>  	iavf_enable_irq0(hw);
> @@ -2341,7 +2340,6 @@ iavf_dev_close(struct rte_eth_dev *dev)  {
>  	struct iavf_hw *hw =
> IAVF_DEV_PRIVATE_TO_HW(dev->data->dev_private);
>  	struct rte_pci_device *pci_dev = RTE_ETH_DEV_TO_PCI(dev);
> -	struct rte_intr_handle *intr_handle = &pci_dev->intr_handle;
>  	struct iavf_adapter *adapter =
>  		IAVF_DEV_PRIVATE_TO_ADAPTER(dev->data->dev_private);
>  	struct iavf_info *vf =
> IAVF_DEV_PRIVATE_TO_VF(dev->data->dev_private);
> @@ -2364,12 +2362,7 @@ iavf_dev_close(struct rte_eth_dev *dev)
>  		iavf_config_promisc(adapter, false, false);
> 
>  	iavf_shutdown_adminq(hw);
> -	/* disable uio intr before callback unregister */
> -	rte_intr_disable(intr_handle);
> -
> -	/* unregister callback func from eal lib */
> -	rte_intr_callback_unregister(intr_handle,
> -				     iavf_dev_interrupt_handler, dev);
> +	rte_eal_alarm_cancel(iavf_dev_alarm_handler, dev);
>  	iavf_disable_irq0(hw);
> 
>  	if (vf->vf_res->vf_cap_flags & VIRTCHNL_VF_OFFLOAD_QOS) diff --git
> a/drivers/net/iavf/iavf_vchnl.c b/drivers/net/iavf/iavf_vchnl.c index
> 06dc663947..af19dd6412 100644
> --- a/drivers/net/iavf/iavf_vchnl.c
> +++ b/drivers/net/iavf/iavf_vchnl.c
> @@ -13,6 +13,7 @@
>  #include <rte_common.h>
> 
>  #include <rte_debug.h>
> +#include <rte_alarm.h>
>  #include <rte_atomic.h>
>  #include <rte_eal.h>
>  #include <rte_ether.h>
> @@ -1663,7 +1664,6 @@ iavf_request_queues(struct iavf_adapter *adapter,
> uint16_t num)  {
>  	struct rte_eth_dev *dev = adapter->eth_dev;
>  	struct iavf_info *vf =  IAVF_DEV_PRIVATE_TO_VF(adapter);
> -	struct rte_pci_device *pci_dev = RTE_ETH_DEV_TO_PCI(dev);
>  	struct virtchnl_vf_res_request vfres;
>  	struct iavf_cmd_info args;
>  	uint16_t num_queue_pairs;
> @@ -1687,13 +1687,10 @@ iavf_request_queues(struct iavf_adapter
> *adapter, uint16_t num)
>  	args.out_buffer = vf->aq_resp;
>  	args.out_size = IAVF_AQ_BUF_SZ;
> 
> -	/*
> -	 * disable interrupt to avoid the admin queue message to be read
> -	 * before iavf_read_msg_from_pf.
> -	 */
> -	rte_intr_disable(&pci_dev->intr_handle);
> +	rte_eal_alarm_cancel(iavf_dev_alarm_handler, dev);
>  	err = iavf_execute_vf_cmd(adapter, &args);
> -	rte_intr_enable(&pci_dev->intr_handle);
> +	rte_eal_alarm_set(IAVF_ALARM_INTERVAL,
> +			  iavf_dev_alarm_handler, dev);
>  	if (err) {
>  		PMD_DRV_LOG(ERR, "fail to execute command
> OP_REQUEST_QUEUES");
>  		return err;
> --
> 2.25.1

Re: [dpdk-dev] [PATCH] net/iavf: remove interrupt handler

[Wu, Jingjing]

> > -----Original Message-----
> > From: Zhang, RobinX <robinx.zhang@intel.com>
> > Sent: Friday, July 23, 2021 3:47 PM
> > To: dev@dpdk.org
> > Cc: Wu, Jingjing <jingjing.wu@intel.com>; Xing, Beilei
> > <beilei.xing@intel.com>; Zhang, Qi Z <qi.z.zhang@intel.com>; Guo,
> > Junfeng <junfeng.guo@intel.com>; Yang, SteveX
> <stevex.yang@intel.com>;
> > Zhang, RobinX <robinx.zhang@intel.com>
> > Subject: [PATCH] net/iavf: remove interrupt handler
> 
> As you are not going to remove interrupt handler for all the cases, the title is
> misleading Better replace it with "enable interrupt polling"
> 
> >
> > For VF hosted by Intel 700 series NICs, internal rx interrupt and
> > adminq interrupt share the same source, that cause a lot cpu cycles be
> > wasted on interrupt handler on rx path.
> >
> > The patch disable pci interrupt and remove the interrupt handler,
> > replace it with a low frequency(50ms) interrupt polling daemon which
> > is implemtented by registering an alarm callback periodly.
> >
> > The virtual channel capability bit VIRTCHNL_VF_OFFLOAD_WB_ON_ITR can
> > be used to negotiate if iavf PMD needs to enable background alarm or
> > not, so ideally this change will not impact the case hosted by Intel 800 series
> NICS.
> >
> > Suggested-by: Jingjing Wu <jingjing.wu@intel.com>
> > Signed-off-by: Qi Zhang <qi.z.zhang@intel.com>
> 
As it is a kind of problem solving, can it be sent to dpdk-stable too?

> No need to add me as the author for this patch but, you can add a reference
> to the original i40e commit to explain you implement the same logic.
> 
> > Signed-off-by: Robin Zhang <robinx.zhang@intel.com>
> > ---
> >  drivers/net/iavf/iavf.h        |  3 +++
> >  drivers/net/iavf/iavf_ethdev.c | 37
> > ++++++++++++++--------------------
> >  drivers/net/iavf/iavf_vchnl.c  | 11 ++++------
> >  3 files changed, 22 insertions(+), 29 deletions(-)
> >
> > diff --git a/drivers/net/iavf/iavf.h b/drivers/net/iavf/iavf.h index
> > b3bd078111..771f3b79d7 100644
> > --- a/drivers/net/iavf/iavf.h
> > +++ b/drivers/net/iavf/iavf.h
> > @@ -69,6 +69,8 @@
> >  #define IAVF_QUEUE_ITR_INTERVAL_DEFAULT 32 /* 32 us */
> >  #define IAVF_QUEUE_ITR_INTERVAL_MAX     8160 /* 8160 us */
> >
> > +#define IAVF_ALARM_INTERVAL 50000 /* us */
> > +
> >  /* The overhead from MTU to max frame size.
> >   * Considering QinQ packet, the VLAN tag needs to be counted twice.
> >   */
> > @@ -372,6 +374,7 @@ int iavf_config_irq_map_lv(struct iavf_adapter
> > *adapter, uint16_t num,  void iavf_add_del_all_mac_addr(struct
> > iavf_adapter *adapter, bool add);  int iavf_dev_link_update(struct
> > rte_eth_dev *dev,  __rte_unused int wait_to_complete);
> > +void iavf_dev_alarm_handler(void *param);
> >  int iavf_query_stats(struct iavf_adapter *adapter,
> >      struct virtchnl_eth_stats **pstats);  int
> > iavf_config_promisc(struct iavf_adapter *adapter, bool enable_unicast,
> > diff --git a/drivers/net/iavf/iavf_ethdev.c
> > b/drivers/net/iavf/iavf_ethdev.c index
> > 41382c6d66..bbe5b3ddb1 100644
> > --- a/drivers/net/iavf/iavf_ethdev.c
> > +++ b/drivers/net/iavf/iavf_ethdev.c
> > @@ -16,6 +16,7 @@
> >  #include <rte_interrupts.h>
> >  #include <rte_debug.h>
> >  #include <rte_pci.h>
> > +#include <rte_alarm.h>
> >  #include <rte_atomic.h>
> >  #include <rte_eal.h>
> >  #include <rte_ether.h>
> > @@ -692,9 +693,9 @@ static int iavf_config_rx_queues_irqs(struct
> > rte_eth_dev *dev,
> >   */
> >  vf->msix_base = IAVF_MISC_VEC_ID;
> >
> > -/* set ITR to max */
> > +/* set ITR to default */
> >  interval = iavf_calc_itr_interval(
> > -IAVF_QUEUE_ITR_INTERVAL_MAX);
> > +IAVF_QUEUE_ITR_INTERVAL_DEFAULT);
> >  IAVF_WRITE_REG(hw, IAVF_VFINT_DYN_CTL01,
> >         IAVF_VFINT_DYN_CTL01_INTENA_MASK |
> >         (IAVF_ITR_INDEX_DEFAULT <<
> > @@ -853,9 +854,8 @@ iavf_dev_start(struct rte_eth_dev *dev)
> > PMD_DRV_LOG(ERR, "configure irq failed");  goto err_queue;  }
> > -/* re-enable intr again, because efd assign may change */
> > +/* only enable interrupt in rx interrupt mode */
> >  if (dev->data->dev_conf.intr_conf.rxq != 0) {
> > -rte_intr_disable(intr_handle);  rte_intr_enable(intr_handle);  }
> >
> > @@ -889,6 +889,9 @@ iavf_dev_stop(struct rte_eth_dev *dev)
> >
> >  PMD_INIT_FUNC_TRACE();
> >
> > +if (dev->data->dev_conf.intr_conf.rxq != 0)
> > +rte_intr_disable(intr_handle);
> > +
> >  if (adapter->stopped == 1)
> >  return 0;
> >
> > @@ -1669,8 +1672,6 @@ iavf_dev_rx_queue_intr_enable(struct
> rte_eth_dev
> > *dev, uint16_t queue_id)
> >
> >  IAVF_WRITE_FLUSH(hw);
> >
> > -rte_intr_ack(&pci_dev->intr_handle);
> > -
> >  return 0;
> >  }
> >
> > @@ -2201,8 +2202,8 @@ iavf_disable_irq0(struct iavf_hw *hw)
> > IAVF_WRITE_FLUSH(hw);  }
> >
> > -static void
> > -iavf_dev_interrupt_handler(void *param)
> > +void
> > +iavf_dev_alarm_handler(void *param)
> >  {
> >  struct rte_eth_dev *dev = (struct rte_eth_dev *)param;  struct
> > iavf_hw *hw = IAVF_DEV_PRIVATE_TO_HW(dev->data->dev_private);
> > @@ -2212,6 +2213,9 @@ iavf_dev_interrupt_handler(void *param)
> > iavf_handle_virtchnl_msg(dev);
> >
> >  iavf_enable_irq0(hw);
> > +
> > +rte_eal_alarm_set(IAVF_ALARM_INTERVAL,
> > +  iavf_dev_alarm_handler, dev);
> >  }
> >
> >  static int
> > @@ -2314,13 +2318,8 @@ iavf_dev_init(struct rte_eth_dev *eth_dev)
> > rte_ether_addr_copy((struct rte_ether_addr *)hw->mac.addr,
> > &eth_dev->data->mac_addrs[0]);
> >
> > -/* register callback func to eal lib */
> > -rte_intr_callback_register(&pci_dev->intr_handle,
> > -   iavf_dev_interrupt_handler,
> > -   (void *)eth_dev);
> > -
> > -/* enable uio intr after callback register */
> > -rte_intr_enable(&pci_dev->intr_handle);
> > +rte_eal_alarm_set(IAVF_ALARM_INTERVAL,
> > +  iavf_dev_alarm_handler, eth_dev);
> >
> >  /* configure and enable device interrupt */  iavf_enable_irq0(hw); @@
> > -2341,7 +2340,6 @@ iavf_dev_close(struct rte_eth_dev *dev)  {  struct
> > iavf_hw *hw = IAVF_DEV_PRIVATE_TO_HW(dev->data->dev_private);
> >  struct rte_pci_device *pci_dev = RTE_ETH_DEV_TO_PCI(dev); -struct
> > rte_intr_handle *intr_handle = &pci_dev->intr_handle;  struct
> > iavf_adapter *adapter =
> > IAVF_DEV_PRIVATE_TO_ADAPTER(dev->data->dev_private);
> >  struct iavf_info *vf =
> > IAVF_DEV_PRIVATE_TO_VF(dev->data->dev_private);
> > @@ -2364,12 +2362,7 @@ iavf_dev_close(struct rte_eth_dev *dev)
> > iavf_config_promisc(adapter, false, false);
> >
> >  iavf_shutdown_adminq(hw);
> > -/* disable uio intr before callback unregister */
> > -rte_intr_disable(intr_handle);
> > -
> > -/* unregister callback func from eal lib */
> > -rte_intr_callback_unregister(intr_handle,
> > -     iavf_dev_interrupt_handler, dev);
> > +rte_eal_alarm_cancel(iavf_dev_alarm_handler, dev);
> >  iavf_disable_irq0(hw);
> >
> >  if (vf->vf_res->vf_cap_flags & VIRTCHNL_VF_OFFLOAD_QOS) diff --git
> > a/drivers/net/iavf/iavf_vchnl.c b/drivers/net/iavf/iavf_vchnl.c index
> > 06dc663947..af19dd6412 100644
> > --- a/drivers/net/iavf/iavf_vchnl.c
> > +++ b/drivers/net/iavf/iavf_vchnl.c
> > @@ -13,6 +13,7 @@
> >  #include <rte_common.h>
> >
> >  #include <rte_debug.h>
> > +#include <rte_alarm.h>
> >  #include <rte_atomic.h>
> >  #include <rte_eal.h>
> >  #include <rte_ether.h>
> > @@ -1663,7 +1664,6 @@ iavf_request_queues(struct iavf_adapter
> > *adapter, uint16_t num)  {  struct rte_eth_dev *dev =
> > adapter->eth_dev;  struct iavf_info *vf =
> > IAVF_DEV_PRIVATE_TO_VF(adapter); -struct rte_pci_device *pci_dev =
> > RTE_ETH_DEV_TO_PCI(dev);  struct virtchnl_vf_res_request vfres;
> > struct iavf_cmd_info args;  uint16_t num_queue_pairs; @@ -1687,13
> > +1687,10 @@ iavf_request_queues(struct iavf_adapter *adapter, uint16_t
> > num)  args.out_buffer = vf->aq_resp;  args.out_size = IAVF_AQ_BUF_SZ;
> >
> > -/*
> > - * disable interrupt to avoid the admin queue message to be read
> > - * before iavf_read_msg_from_pf.
> > - */
> > -rte_intr_disable(&pci_dev->intr_handle);
> > +rte_eal_alarm_cancel(iavf_dev_alarm_handler, dev);
> >  err = iavf_execute_vf_cmd(adapter, &args);
> > -rte_intr_enable(&pci_dev->intr_handle);
> > +rte_eal_alarm_set(IAVF_ALARM_INTERVAL,
> > +  iavf_dev_alarm_handler, dev);
> >  if (err) {
> >  PMD_DRV_LOG(ERR, "fail to execute command OP_REQUEST_QUEUES");
> > return err;
> > --
> > 2.25.1
> 

[dpdk-dev] [PATCH v2] net/iavf: enable interrupt polling

[Robin Zhang]

For VF hosted by Intel 700 series NICs, internal rx interrupt and adminq
interrupt share the same source, that cause a lot cpu cycles be wasted on
interrupt handler on rx path.

The patch disable pci interrupt and remove the interrupt handler, replace
it with a low frequency(50ms) interrupt polling daemon which is
implemtented by registering an alarm callback periodly.

The virtual channel capability bit VIRTCHNL_VF_OFFLOAD_WB_ON_ITR can be
used to negotiate if iavf PMD needs to enable background alarm or not, so
ideally this change will not impact the case hosted by Intel 800 series
NICS.

This patch implements the same logic with an early i40e commit:
commit 864a800d706d ("net/i40e: remove VF interrupt handler")

Signed-off-by: Robin Zhang <robinx.zhang@intel.com>

v2:
- only enable interrupt polling for VF of i40e devices.

---
 drivers/net/iavf/iavf.h        |  3 ++
 drivers/net/iavf/iavf_ethdev.c | 71 +++++++++++++++++++++++++++-------
 drivers/net/iavf/iavf_vchnl.c  | 22 +++++++----
 3 files changed, 74 insertions(+), 22 deletions(-)

diff --git a/drivers/net/iavf/iavf.h b/drivers/net/iavf/iavf.h
index b3bd078111..771f3b79d7 100644
--- a/drivers/net/iavf/iavf.h
+++ b/drivers/net/iavf/iavf.h
@@ -69,6 +69,8 @@
 #define IAVF_QUEUE_ITR_INTERVAL_DEFAULT 32 /* 32 us */
 #define IAVF_QUEUE_ITR_INTERVAL_MAX     8160 /* 8160 us */
 
+#define IAVF_ALARM_INTERVAL 50000 /* us */
+
 /* The overhead from MTU to max frame size.
  * Considering QinQ packet, the VLAN tag needs to be counted twice.
  */
@@ -372,6 +374,7 @@ int iavf_config_irq_map_lv(struct iavf_adapter *adapter, uint16_t num,
 void iavf_add_del_all_mac_addr(struct iavf_adapter *adapter, bool add);
 int iavf_dev_link_update(struct rte_eth_dev *dev,
 			__rte_unused int wait_to_complete);
+void iavf_dev_alarm_handler(void *param);
 int iavf_query_stats(struct iavf_adapter *adapter,
 		    struct virtchnl_eth_stats **pstats);
 int iavf_config_promisc(struct iavf_adapter *adapter, bool enable_unicast,
diff --git a/drivers/net/iavf/iavf_ethdev.c b/drivers/net/iavf/iavf_ethdev.c
index 574cfe055e..29d2aaa10e 100644
--- a/drivers/net/iavf/iavf_ethdev.c
+++ b/drivers/net/iavf/iavf_ethdev.c
@@ -16,6 +16,7 @@
 #include <rte_interrupts.h>
 #include <rte_debug.h>
 #include <rte_pci.h>
+#include <rte_alarm.h>
 #include <rte_atomic.h>
 #include <rte_eal.h>
 #include <rte_ether.h>
@@ -704,9 +705,9 @@ static int iavf_config_rx_queues_irqs(struct rte_eth_dev *dev,
 			 */
 			vf->msix_base = IAVF_MISC_VEC_ID;
 
-			/* set ITR to max */
+			/* set ITR to default */
 			interval = iavf_calc_itr_interval(
-					IAVF_QUEUE_ITR_INTERVAL_MAX);
+					IAVF_QUEUE_ITR_INTERVAL_DEFAULT);
 			IAVF_WRITE_REG(hw, IAVF_VFINT_DYN_CTL01,
 				       IAVF_VFINT_DYN_CTL01_INTENA_MASK |
 				       (IAVF_ITR_INDEX_DEFAULT <<
@@ -867,7 +868,8 @@ iavf_dev_start(struct rte_eth_dev *dev)
 	}
 	/* re-enable intr again, because efd assign may change */
 	if (dev->data->dev_conf.intr_conf.rxq != 0) {
-		rte_intr_disable(intr_handle);
+		if (vf->vf_res->vf_cap_flags & VIRTCHNL_VF_OFFLOAD_WB_ON_ITR)
+			rte_intr_disable(intr_handle);
 		rte_intr_enable(intr_handle);
 	}
 
@@ -901,6 +903,10 @@ iavf_dev_stop(struct rte_eth_dev *dev)
 
 	PMD_INIT_FUNC_TRACE();
 
+	if (!(vf->vf_res->vf_cap_flags & VIRTCHNL_VF_OFFLOAD_WB_ON_ITR) &&
+	    dev->data->dev_conf.intr_conf.rxq != 0)
+		rte_intr_disable(intr_handle);
+
 	if (adapter->stopped == 1)
 		return 0;
 
@@ -1659,6 +1665,7 @@ iavf_dev_rx_queue_intr_enable(struct rte_eth_dev *dev, uint16_t queue_id)
 		IAVF_DEV_PRIVATE_TO_ADAPTER(dev->data->dev_private);
 	struct rte_pci_device *pci_dev = RTE_ETH_DEV_TO_PCI(dev);
 	struct iavf_hw *hw = IAVF_DEV_PRIVATE_TO_HW(adapter);
+	struct iavf_info *vf = IAVF_DEV_PRIVATE_TO_VF(adapter);
 	uint16_t msix_intr;
 
 	msix_intr = pci_dev->intr_handle.intr_vec[queue_id];
@@ -1679,7 +1686,8 @@ iavf_dev_rx_queue_intr_enable(struct rte_eth_dev *dev, uint16_t queue_id)
 
 	IAVF_WRITE_FLUSH(hw);
 
-	rte_intr_ack(&pci_dev->intr_handle);
+	if (vf->vf_res->vf_cap_flags & VIRTCHNL_VF_OFFLOAD_WB_ON_ITR)
+		rte_intr_ack(&pci_dev->intr_handle);
 
 	return 0;
 }
@@ -2224,6 +2232,29 @@ iavf_dev_interrupt_handler(void *param)
 	iavf_enable_irq0(hw);
 }
 
+void
+iavf_dev_alarm_handler(void *param)
+{
+	struct rte_eth_dev *dev = (struct rte_eth_dev *)param;
+	struct iavf_hw *hw = IAVF_DEV_PRIVATE_TO_HW(dev->data->dev_private);
+	uint32_t icr0;
+
+	iavf_disable_irq0(hw);
+
+	/* read out interrupt causes */
+	icr0 = IAVF_READ_REG(hw, IAVF_VFINT_ICR01);
+
+	if (icr0 & IAVF_VFINT_ICR01_ADMINQ_MASK) {
+		PMD_DRV_LOG(DEBUG, "ICR01_ADMINQ is reported");
+		iavf_handle_virtchnl_msg(dev);
+	}
+
+	iavf_enable_irq0(hw);
+
+	rte_eal_alarm_set(IAVF_ALARM_INTERVAL,
+			  iavf_dev_alarm_handler, dev);
+}
+
 static int
 iavf_dev_flow_ops_get(struct rte_eth_dev *dev,
 		      const struct rte_flow_ops **ops)
@@ -2260,6 +2291,7 @@ iavf_dev_init(struct rte_eth_dev *eth_dev)
 	struct iavf_adapter *adapter =
 		IAVF_DEV_PRIVATE_TO_ADAPTER(eth_dev->data->dev_private);
 	struct iavf_hw *hw = IAVF_DEV_PRIVATE_TO_HW(adapter);
+	struct iavf_info *vf = IAVF_DEV_PRIVATE_TO_VF(adapter);
 	struct rte_pci_device *pci_dev = RTE_ETH_DEV_TO_PCI(eth_dev);
 	int ret = 0;
 
@@ -2324,13 +2356,18 @@ iavf_dev_init(struct rte_eth_dev *eth_dev)
 	rte_ether_addr_copy((struct rte_ether_addr *)hw->mac.addr,
 			&eth_dev->data->mac_addrs[0]);
 
-	/* register callback func to eal lib */
-	rte_intr_callback_register(&pci_dev->intr_handle,
-				   iavf_dev_interrupt_handler,
-				   (void *)eth_dev);
+	if (vf->vf_res->vf_cap_flags & VIRTCHNL_VF_OFFLOAD_WB_ON_ITR) {
+		/* register callback func to eal lib */
+		rte_intr_callback_register(&pci_dev->intr_handle,
+					   iavf_dev_interrupt_handler,
+					   (void *)eth_dev);
 
-	/* enable uio intr after callback register */
-	rte_intr_enable(&pci_dev->intr_handle);
+		/* enable uio intr after callback register */
+		rte_intr_enable(&pci_dev->intr_handle);
+	} else {
+		rte_eal_alarm_set(IAVF_ALARM_INTERVAL,
+				  iavf_dev_alarm_handler, eth_dev);
+	}
 
 	/* configure and enable device interrupt */
 	iavf_enable_irq0(hw);
@@ -2374,12 +2411,16 @@ iavf_dev_close(struct rte_eth_dev *dev)
 		iavf_config_promisc(adapter, false, false);
 
 	iavf_shutdown_adminq(hw);
-	/* disable uio intr before callback unregister */
-	rte_intr_disable(intr_handle);
+	if (vf->vf_res->vf_cap_flags & VIRTCHNL_VF_OFFLOAD_WB_ON_ITR) {
+		/* disable uio intr before callback unregister */
+		rte_intr_disable(intr_handle);
 
-	/* unregister callback func from eal lib */
-	rte_intr_callback_unregister(intr_handle,
-				     iavf_dev_interrupt_handler, dev);
+		/* unregister callback func from eal lib */
+		rte_intr_callback_unregister(intr_handle,
+					     iavf_dev_interrupt_handler, dev);
+	} else {
+		rte_eal_alarm_cancel(iavf_dev_alarm_handler, dev);
+	}
 	iavf_disable_irq0(hw);
 
 	if (vf->vf_res->vf_cap_flags & VIRTCHNL_VF_OFFLOAD_QOS)
diff --git a/drivers/net/iavf/iavf_vchnl.c b/drivers/net/iavf/iavf_vchnl.c
index 06dc663947..71ecf7f202 100644
--- a/drivers/net/iavf/iavf_vchnl.c
+++ b/drivers/net/iavf/iavf_vchnl.c
@@ -13,6 +13,7 @@
 #include <rte_common.h>
 
 #include <rte_debug.h>
+#include <rte_alarm.h>
 #include <rte_atomic.h>
 #include <rte_eal.h>
 #include <rte_ether.h>
@@ -1687,13 +1688,20 @@ iavf_request_queues(struct iavf_adapter *adapter, uint16_t num)
 	args.out_buffer = vf->aq_resp;
 	args.out_size = IAVF_AQ_BUF_SZ;
 
-	/*
-	 * disable interrupt to avoid the admin queue message to be read
-	 * before iavf_read_msg_from_pf.
-	 */
-	rte_intr_disable(&pci_dev->intr_handle);
-	err = iavf_execute_vf_cmd(adapter, &args);
-	rte_intr_enable(&pci_dev->intr_handle);
+	if (vf->vf_res->vf_cap_flags & VIRTCHNL_VF_OFFLOAD_WB_ON_ITR) {
+		/* disable interrupt to avoid the admin queue message to be read
+		 * before iavf_read_msg_from_pf.
+		 */
+		rte_intr_disable(&pci_dev->intr_handle);
+		err = iavf_execute_vf_cmd(adapter, &args);
+		rte_intr_enable(&pci_dev->intr_handle);
+	} else {
+		rte_eal_alarm_cancel(iavf_dev_alarm_handler, dev);
+		err = iavf_execute_vf_cmd(adapter, &args);
+		rte_eal_alarm_set(IAVF_ALARM_INTERVAL,
+				  iavf_dev_alarm_handler, dev);
+	}
+
 	if (err) {
 		PMD_DRV_LOG(ERR, "fail to execute command OP_REQUEST_QUEUES");
 		return err;
-- 
2.25.1

Re: [dpdk-dev] [PATCH v2] net/iavf: enable interrupt polling

[Kadam, Pallavi]

On 8/25/2021 1:34 AM, Robin Zhang wrote:
> For VF hosted by Intel 700 series NICs, internal rx interrupt and adminq
> interrupt share the same source, that cause a lot cpu cycles be wasted on
> interrupt handler on rx path.
>
> The patch disable pci interrupt and remove the interrupt handler, replace
> it with a low frequency(50ms) interrupt polling daemon which is
> implemtented by registering an alarm callback periodly.
>
> The virtual channel capability bit VIRTCHNL_VF_OFFLOAD_WB_ON_ITR can be
> used to negotiate if iavf PMD needs to enable background alarm or not, so
> ideally this change will not impact the case hosted by Intel 800 series
> NICS.
>
> This patch implements the same logic with an early i40e commit:
> commit 864a800d706d ("net/i40e: remove VF interrupt handler")
>
> Signed-off-by: Robin Zhang <robinx.zhang@intel.com>
>
> v2:
> - only enable interrupt polling for VF of i40e devices.
>
> ---
Acked-by: Pallavi Kadam <pallavi.kadam@intel.com>

Re: [dpdk-dev] [PATCH v2] net/iavf: enable interrupt polling

[Zhang, Qi Z]

> -----Original Message-----
> From: Zhang, RobinX <robinx.zhang@intel.com>
> Sent: Wednesday, August 25, 2021 4:35 PM
> To: dev@dpdk.org
> Cc: Wu, Jingjing <jingjing.wu@intel.com>; Xing, Beilei <beilei.xing@intel.com>;
> Zhang, Qi Z <qi.z.zhang@intel.com>; Guo, Junfeng <junfeng.guo@intel.com>;
> Yang, SteveX <stevex.yang@intel.com>; Zhang, RobinX
> <robinx.zhang@intel.com>
> Subject: [PATCH v2] net/iavf: enable interrupt polling
> 
> For VF hosted by Intel 700 series NICs, internal rx interrupt and adminq
> interrupt share the same source, that cause a lot cpu cycles be wasted on
> interrupt handler on rx path.
> 
> The patch disable pci interrupt and remove the interrupt handler, replace it
> with a low frequency(50ms) interrupt polling daemon which is implemtented
> by registering an alarm callback periodly.
> 
> The virtual channel capability bit VIRTCHNL_VF_OFFLOAD_WB_ON_ITR can be
> used to negotiate if iavf PMD needs to enable background alarm or not, so
> ideally this change will not impact the case hosted by Intel 800 series NICS.
> 
> This patch implements the same logic with an early i40e commit:
> commit 864a800d706d ("net/i40e: remove VF interrupt handler")
> 
> Signed-off-by: Robin Zhang <robinx.zhang@intel.com>

Acked-by: Qi Zhang <qi.z.zhang@intel.com>

Applied to dpdk-next-net-intel.

Thanks
Qi