[dpdk-dev] [PATCH v2 1/4] net/ixgbevf: unregister irq handler when other interrupts not allowed.

[dpdk-dev] [PATCH v2 1/4] net/ixgbevf: unregister irq handler when other interrupts not allowed.

[xiangxia.m.yue]

From: Tonghao Zhang <xiangxia.m.yue@gmail.com>

When bind the ixgbe VF (e.g 82599 card) to igb_uio and enable the
rx-interrupt, there will be more than one epoll_wait on intr_handle.fd.
One is in "eal-intr-thread" thread, and the others are in the thread
which call the "rte_epoll_wait". The problem is that sometimes
"eal-intr-thread" thread will process the rx interrupt, and then
rte_epoll_wait can't get the event anymore, and the packets may be lost.

The patch unregister the status interrupt handler in "eal-intr-thread"
thread and the ixgbe pf is in the same case.

Signed-off-by: Tonghao Zhang <xiangxia.m.yue@gmail.com>
Acked-by: Beilei Xing <beilei.xing@intel.com>
---
 drivers/net/ixgbe/ixgbe_ethdev.c | 15 +++++++++++++++
 1 file changed, 15 insertions(+)

diff --git a/drivers/net/ixgbe/ixgbe_ethdev.c b/drivers/net/ixgbe/ixgbe_ethdev.c
index ff19a56..0e056a2 100644
--- a/drivers/net/ixgbe/ixgbe_ethdev.c
+++ b/drivers/net/ixgbe/ixgbe_ethdev.c
@@ -5078,6 +5078,15 @@ static int ixgbevf_dev_xstats_get_names(__rte_unused struct rte_eth_dev *dev,
 	}
 	ixgbevf_configure_msix(dev);
 
+	if (!rte_intr_allow_others(intr_handle)) {
+		rte_intr_callback_unregister(intr_handle,
+					     ixgbevf_dev_interrupt_handler,
+					     dev);
+		if (dev->data->dev_conf.intr_conf.lsc != 0)
+			PMD_INIT_LOG(INFO, "lsc won't enable because of"
+				     " no intr multiplex");
+	}
+
 	/* When a VF port is bound to VFIO-PCI, only miscellaneous interrupt
 	 * is mapped to VFIO vector 0 in eth_ixgbevf_dev_init( ).
 	 * If previous VFIO interrupt mapping setting in eth_ixgbevf_dev_init( )
@@ -5120,6 +5129,12 @@ static int ixgbevf_dev_xstats_get_names(__rte_unused struct rte_eth_dev *dev,
 
 	ixgbe_dev_clear_queues(dev);
 
+	if (!rte_intr_allow_others(intr_handle))
+		/* resume to the default handler */
+		rte_intr_callback_register(intr_handle,
+					   ixgbevf_dev_interrupt_handler,
+					   (void *)dev);
+
 	/* Clean datapath event and queue/vec mapping */
 	rte_intr_efd_disable(intr_handle);
 	if (intr_handle->intr_vec != NULL) {
-- 
1.8.3.1

[dpdk-dev] [PATCH v2 2/4] net/ixgbevf: set the inter-interrupt interval for EITR.

[xiangxia.m.yue]

From: Tonghao Zhang <xiangxia.m.yue@gmail.com>

Set EITR interval as default. This patch can improve the
performance when we enable the rx-interrupt to process the
packets because we hope rx-interrupt reduce CPU. For example,
the 200us value of EITR makes the performance better with
the low CPU.

Signed-off-by: Tonghao Zhang <xiangxia.m.yue@gmail.com>
---
 config/common_base               | 2 ++
 drivers/net/ixgbe/ixgbe_ethdev.c | 4 ++++
 2 files changed, 6 insertions(+)

diff --git a/config/common_base b/config/common_base
index e74febe..9e6f048 100644
--- a/config/common_base
+++ b/config/common_base
@@ -196,6 +196,8 @@ CONFIG_RTE_LIBRTE_IXGBE_DEBUG_DRIVER=n
 CONFIG_RTE_LIBRTE_IXGBE_PF_DISABLE_STRIP_CRC=n
 CONFIG_RTE_IXGBE_INC_VECTOR=y
 CONFIG_RTE_LIBRTE_IXGBE_BYPASS=n
+# interval up to 1024 us, (0us as default value)
+CONFIG_RTE_LIBRTE_IXGBE_ITR_INTERVAL=0
 
 #
 # Compile burst-oriented I40E PMD driver
diff --git a/drivers/net/ixgbe/ixgbe_ethdev.c b/drivers/net/ixgbe/ixgbe_ethdev.c
index 0e056a2..49f486a 100644
--- a/drivers/net/ixgbe/ixgbe_ethdev.c
+++ b/drivers/net/ixgbe/ixgbe_ethdev.c
@@ -5809,6 +5809,10 @@ static void ixgbevf_set_vfta_all(struct rte_eth_dev *dev, bool on)
 		if (vector_idx < base + intr_handle->nb_efd - 1)
 			vector_idx++;
 	}
+
+	IXGBE_WRITE_REG(hw, IXGBE_VTEITR(IXGBE_MISC_VEC_ID),
+			IXGBE_EITR_INTERVAL_US(RTE_LIBRTE_IXGBE_ITR_INTERVAL)
+			| IXGBE_EITR_CNT_WDIS);
 }
 
 /**
-- 
1.8.3.1

[dpdk-dev] [PATCH v2 3/4] net/ixgbevf: save IXGBE_VTEIMS to intr->mask for performance.

[xiangxia.m.yue]

From: Tonghao Zhang <xiangxia.m.yue@gmail.com>

If dpdk APPs call the rte_eth_dev_rx_intr_enable or
rte_eth_dev_rx_intr_disable frequently, and ixgbe vf will read
the IXGBE_VTEIMS register everytime. The patch saves the IXGBE_VTEIMS
to mask to avoid read frequently.

Signed-off-by: Tonghao Zhang <xiangxia.m.yue@gmail.com>
Acked-by: Beilei Xing <beilei.xing@intel.com>
---
 drivers/net/ixgbe/ixgbe_ethdev.c | 55 +++++++++++++++++++++++++---------------
 1 file changed, 35 insertions(+), 20 deletions(-)

diff --git a/drivers/net/ixgbe/ixgbe_ethdev.c b/drivers/net/ixgbe/ixgbe_ethdev.c
index 49f486a..cab243d 100644
--- a/drivers/net/ixgbe/ixgbe_ethdev.c
+++ b/drivers/net/ixgbe/ixgbe_ethdev.c
@@ -270,8 +270,8 @@ static int ixgbevf_dev_link_update(struct rte_eth_dev *dev,
 static void ixgbevf_dev_stop(struct rte_eth_dev *dev);
 static void ixgbevf_dev_close(struct rte_eth_dev *dev);
 static int  ixgbevf_dev_reset(struct rte_eth_dev *dev);
-static void ixgbevf_intr_disable(struct ixgbe_hw *hw);
-static void ixgbevf_intr_enable(struct ixgbe_hw *hw);
+static void ixgbevf_intr_disable(struct rte_eth_dev *dev);
+static void ixgbevf_intr_enable(struct rte_eth_dev *dev);
 static int ixgbevf_dev_stats_get(struct rte_eth_dev *dev,
 		struct rte_eth_stats *stats);
 static void ixgbevf_dev_stats_reset(struct rte_eth_dev *dev);
@@ -1665,7 +1665,7 @@ static int ixgbe_l2_tn_filter_init(struct rte_eth_dev *eth_dev)
 	ixgbevf_dev_stats_reset(eth_dev);
 
 	/* Disable the interrupts for VF */
-	ixgbevf_intr_disable(hw);
+	ixgbevf_intr_disable(eth_dev);
 
 	hw->mac.num_rar_entries = 128; /* The MAX of the underlying PF */
 	diag = hw->mac.ops.reset_hw(hw);
@@ -1734,7 +1734,7 @@ static int ixgbe_l2_tn_filter_init(struct rte_eth_dev *eth_dev)
 	rte_intr_callback_register(intr_handle,
 				   ixgbevf_dev_interrupt_handler, eth_dev);
 	rte_intr_enable(intr_handle);
-	ixgbevf_intr_enable(hw);
+	ixgbevf_intr_enable(eth_dev);
 
 	PMD_INIT_LOG(DEBUG, "port %d vendorID=0x%x deviceID=0x%x mac.type=%s",
 		     eth_dev->data->port_id, pci_dev->id.vendor_id,
@@ -1767,7 +1767,7 @@ static int ixgbe_l2_tn_filter_init(struct rte_eth_dev *eth_dev)
 	eth_dev->tx_pkt_burst = NULL;
 
 	/* Disable the interrupts for VF */
-	ixgbevf_intr_disable(hw);
+	ixgbevf_intr_disable(eth_dev);
 
 	rte_free(eth_dev->data->mac_addrs);
 	eth_dev->data->mac_addrs = NULL;
@@ -4953,19 +4953,32 @@ static int ixgbevf_dev_xstats_get_names(__rte_unused struct rte_eth_dev *dev,
  * Virtual Function operations
  */
 static void
-ixgbevf_intr_disable(struct ixgbe_hw *hw)
+ixgbevf_intr_disable(struct rte_eth_dev *dev)
 {
+	struct ixgbe_interrupt *intr =
+		IXGBE_DEV_PRIVATE_TO_INTR(dev->data->dev_private);
+	struct ixgbe_hw *hw =
+		IXGBE_DEV_PRIVATE_TO_HW(dev->data->dev_private);
+
 	PMD_INIT_FUNC_TRACE();
 
 	/* Clear interrupt mask to stop from interrupts being generated */
 	IXGBE_WRITE_REG(hw, IXGBE_VTEIMC, IXGBE_VF_IRQ_CLEAR_MASK);
 
 	IXGBE_WRITE_FLUSH(hw);
+
+	/* Clear mask value. */
+	intr->mask = 0;
 }
 
 static void
-ixgbevf_intr_enable(struct ixgbe_hw *hw)
+ixgbevf_intr_enable(struct rte_eth_dev *dev)
 {
+	struct ixgbe_interrupt *intr =
+		IXGBE_DEV_PRIVATE_TO_INTR(dev->data->dev_private);
+	struct ixgbe_hw *hw =
+		IXGBE_DEV_PRIVATE_TO_HW(dev->data->dev_private);
+
 	PMD_INIT_FUNC_TRACE();
 
 	/* VF enable interrupt autoclean */
@@ -4974,6 +4987,9 @@ static int ixgbevf_dev_xstats_get_names(__rte_unused struct rte_eth_dev *dev,
 	IXGBE_WRITE_REG(hw, IXGBE_VTEIMS, IXGBE_VF_IRQ_ENABLE_MASK);
 
 	IXGBE_WRITE_FLUSH(hw);
+
+	/* Save IXGBE_VTEIMS value to mask. */
+	intr->mask = IXGBE_VF_IRQ_ENABLE_MASK;
 }
 
 static int
@@ -5099,7 +5115,7 @@ static int ixgbevf_dev_xstats_get_names(__rte_unused struct rte_eth_dev *dev,
 	rte_intr_enable(intr_handle);
 
 	/* Re-enable interrupt for VF */
-	ixgbevf_intr_enable(hw);
+	ixgbevf_intr_enable(dev);
 
 	return 0;
 }
@@ -5113,7 +5129,7 @@ static int ixgbevf_dev_xstats_get_names(__rte_unused struct rte_eth_dev *dev,
 
 	PMD_INIT_FUNC_TRACE();
 
-	ixgbevf_intr_disable(hw);
+	ixgbevf_intr_disable(dev);
 
 	hw->adapter_stopped = 1;
 	ixgbe_stop_adapter(hw);
@@ -5611,17 +5627,17 @@ static void ixgbevf_set_vfta_all(struct rte_eth_dev *dev, bool on)
 {
 	struct rte_pci_device *pci_dev = RTE_ETH_DEV_TO_PCI(dev);
 	struct rte_intr_handle *intr_handle = &pci_dev->intr_handle;
-	uint32_t mask;
+	struct ixgbe_interrupt *intr =
+		IXGBE_DEV_PRIVATE_TO_INTR(dev->data->dev_private);
 	struct ixgbe_hw *hw =
 		IXGBE_DEV_PRIVATE_TO_HW(dev->data->dev_private);
 	uint32_t vec = IXGBE_MISC_VEC_ID;
 
-	mask = IXGBE_READ_REG(hw, IXGBE_VTEIMS);
 	if (rte_intr_allow_others(intr_handle))
 		vec = IXGBE_RX_VEC_START;
-	mask |= (1 << vec);
+	intr->mask |= (1 << vec);
 	RTE_SET_USED(queue_id);
-	IXGBE_WRITE_REG(hw, IXGBE_VTEIMS, mask);
+	IXGBE_WRITE_REG(hw, IXGBE_VTEIMS, intr->mask);
 
 	rte_intr_enable(intr_handle);
 
@@ -5631,19 +5647,19 @@ static void ixgbevf_set_vfta_all(struct rte_eth_dev *dev, bool on)
 static int
 ixgbevf_dev_rx_queue_intr_disable(struct rte_eth_dev *dev, uint16_t queue_id)
 {
-	uint32_t mask;
+	struct ixgbe_interrupt *intr =
+		IXGBE_DEV_PRIVATE_TO_INTR(dev->data->dev_private);
 	struct ixgbe_hw *hw =
 		IXGBE_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;
 	uint32_t vec = IXGBE_MISC_VEC_ID;
 
-	mask = IXGBE_READ_REG(hw, IXGBE_VTEIMS);
 	if (rte_intr_allow_others(intr_handle))
 		vec = IXGBE_RX_VEC_START;
-	mask &= ~(1 << vec);
+	intr->mask &= ~(1 << vec);
 	RTE_SET_USED(queue_id);
-	IXGBE_WRITE_REG(hw, IXGBE_VTEIMS, mask);
+	IXGBE_WRITE_REG(hw, IXGBE_VTEIMS, intr->mask);
 
 	return 0;
 }
@@ -8200,7 +8216,7 @@ static void ixgbevf_mbx_process(struct rte_eth_dev *dev)
 	struct ixgbe_hw *hw = IXGBE_DEV_PRIVATE_TO_HW(dev->data->dev_private);
 	struct ixgbe_interrupt *intr =
 		IXGBE_DEV_PRIVATE_TO_INTR(dev->data->dev_private);
-	ixgbevf_intr_disable(hw);
+	ixgbevf_intr_disable(dev);
 
 	/* read-on-clear nic registers here */
 	eicr = IXGBE_READ_REG(hw, IXGBE_VTEICR);
@@ -8217,7 +8233,6 @@ static void ixgbevf_mbx_process(struct rte_eth_dev *dev)
 static int
 ixgbevf_dev_interrupt_action(struct rte_eth_dev *dev)
 {
-	struct ixgbe_hw *hw = IXGBE_DEV_PRIVATE_TO_HW(dev->data->dev_private);
 	struct ixgbe_interrupt *intr =
 		IXGBE_DEV_PRIVATE_TO_INTR(dev->data->dev_private);
 
@@ -8226,7 +8241,7 @@ static void ixgbevf_mbx_process(struct rte_eth_dev *dev)
 		intr->flags &= ~IXGBE_FLAG_MAILBOX;
 	}
 
-	ixgbevf_intr_enable(hw);
+	ixgbevf_intr_enable(dev);
 
 	return 0;
 }
-- 
1.8.3.1

[dpdk-dev] [PATCH v2 4/4] net/ixgbe: remove the unnecessary call rte_intr_enable.

[xiangxia.m.yue]

From: Tonghao Zhang <xiangxia.m.yue@gmail.com>

When binding the ixgbe pf or vf to vfio and call the
rte_eth_dev_rx_intr_enable frequently, the interrupt setting
(msi_set_mask_bit) will take more CPU as show below. rte_intr_enable
calls the ioctl to map the fd to interrupts frequently.

perf top:
5.45%  [kernel]   [k] msi_set_mask_bit

It is unnecessary to call the rte_intr_enable in
ixgbe_dev_rx_queue_intr_enable. Because the fds has been mapped to
interrupt and not unmapped in ixgbe_dev_rx_queue_intr_disable.

With the patch, msi_set_mask_bit is not listed in perl anymore.

Signed-off-by: Tonghao Zhang <xiangxia.m.yue@gmail.com>
---
 drivers/net/ixgbe/ixgbe_ethdev.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/drivers/net/ixgbe/ixgbe_ethdev.c b/drivers/net/ixgbe/ixgbe_ethdev.c
index cab243d..d081247 100644
--- a/drivers/net/ixgbe/ixgbe_ethdev.c
+++ b/drivers/net/ixgbe/ixgbe_ethdev.c
@@ -5639,8 +5639,6 @@ static void ixgbevf_set_vfta_all(struct rte_eth_dev *dev, bool on)
 	RTE_SET_USED(queue_id);
 	IXGBE_WRITE_REG(hw, IXGBE_VTEIMS, intr->mask);
 
-	rte_intr_enable(intr_handle);
-
 	return 0;
 }
 
@@ -5688,7 +5686,6 @@ static void ixgbevf_set_vfta_all(struct rte_eth_dev *dev, bool on)
 		mask &= (1 << (queue_id - 32));
 		IXGBE_WRITE_REG(hw, IXGBE_EIMS_EX(1), mask);
 	}
-	rte_intr_enable(intr_handle);
 
 	return 0;
 }
-- 
1.8.3.1

Re: [dpdk-dev] [PATCH v2 1/4] net/ixgbevf: unregister irq handler when other interrupts not allowed.

[Dai, Wei]

Hi, Tonghao & Beilei

The issue reported by Tonghao is caused by shortage of igb_uio.
If you want to use Rx queue interrupt in your DPDK application, 
I suggest use VFIO-PCI to bind NIC port instead of igb_uio.

Currently igb_uio only support single event fd. This fd is triggered by
both miscellaneous and RX queue interrupt in ixgbe VF.
But same event fd is added to epoll fd of eal-intr-thread 
and also epoll fd of normal thread for Rx task, this cause the issue.

If VFIO-PCI is used, different event fd for misc and Rx queue are created.
By the way, your patch also lead to missing of PF to VF reset interrupt.

So, I am sorry that I can't agree it now.


> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of
> xiangxia.m.yue@gmail.com
> Sent: Friday, January 12, 2018 12:40 AM
> To: Xing, Beilei <beilei.xing@intel.com>; dev@dpdk.org
> Cc: Tonghao Zhang <xiangxia.m.yue@gmail.com>
> Subject: [dpdk-dev] [PATCH v2 1/4] net/ixgbevf: unregister irq handler when
> other interrupts not allowed.
> 
> From: Tonghao Zhang <xiangxia.m.yue@gmail.com>
> 
> When bind the ixgbe VF (e.g 82599 card) to igb_uio and enable the
> rx-interrupt, there will be more than one epoll_wait on intr_handle.fd.
> One is in "eal-intr-thread" thread, and the others are in the thread which call
> the "rte_epoll_wait". The problem is that sometimes "eal-intr-thread" thread
> will process the rx interrupt, and then rte_epoll_wait can't get the event
> anymore, and the packets may be lost.
> 
> The patch unregister the status interrupt handler in "eal-intr-thread"
> thread and the ixgbe pf is in the same case.
> 
> Signed-off-by: Tonghao Zhang <xiangxia.m.yue@gmail.com>
> Acked-by: Beilei Xing <beilei.xing@intel.com>
> ---
>  drivers/net/ixgbe/ixgbe_ethdev.c | 15 +++++++++++++++
>  1 file changed, 15 insertions(+)
> 
> diff --git a/drivers/net/ixgbe/ixgbe_ethdev.c
> b/drivers/net/ixgbe/ixgbe_ethdev.c
> index ff19a56..0e056a2 100644
> --- a/drivers/net/ixgbe/ixgbe_ethdev.c
> +++ b/drivers/net/ixgbe/ixgbe_ethdev.c
> @@ -5078,6 +5078,15 @@ static int
> ixgbevf_dev_xstats_get_names(__rte_unused struct rte_eth_dev *dev,
>  	}
>  	ixgbevf_configure_msix(dev);
> 
> +	if (!rte_intr_allow_others(intr_handle)) {
> +		rte_intr_callback_unregister(intr_handle,
> +					     ixgbevf_dev_interrupt_handler,
> +					     dev);
> +		if (dev->data->dev_conf.intr_conf.lsc != 0)
> +			PMD_INIT_LOG(INFO, "lsc won't enable because of"
> +				     " no intr multiplex");
> +	}
> +
>  	/* When a VF port is bound to VFIO-PCI, only miscellaneous interrupt
>  	 * is mapped to VFIO vector 0 in eth_ixgbevf_dev_init( ).
>  	 * If previous VFIO interrupt mapping setting in eth_ixgbevf_dev_init( )
> @@ -5120,6 +5129,12 @@ static int
> ixgbevf_dev_xstats_get_names(__rte_unused struct rte_eth_dev *dev,
> 
>  	ixgbe_dev_clear_queues(dev);
> 
> +	if (!rte_intr_allow_others(intr_handle))
> +		/* resume to the default handler */
> +		rte_intr_callback_register(intr_handle,
> +					   ixgbevf_dev_interrupt_handler,
> +					   (void *)dev);
> +
>  	/* Clean datapath event and queue/vec mapping */
>  	rte_intr_efd_disable(intr_handle);
>  	if (intr_handle->intr_vec != NULL) {
> --
> 1.8.3.1

Re: [dpdk-dev] [PATCH v2 3/4] net/ixgbevf: save IXGBE_VTEIMS to intr->mask for performance.

[Dai, Wei]

> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of
> xiangxia.m.yue@gmail.com
> Sent: Friday, January 12, 2018 12:40 AM
> To: Xing, Beilei <beilei.xing@intel.com>; dev@dpdk.org
> Cc: Tonghao Zhang <xiangxia.m.yue@gmail.com>
> Subject: [dpdk-dev] [PATCH v2 3/4] net/ixgbevf: save IXGBE_VTEIMS to
> intr->mask for performance.
> 
> From: Tonghao Zhang <xiangxia.m.yue@gmail.com>
> 
> If dpdk APPs call the rte_eth_dev_rx_intr_enable or
> rte_eth_dev_rx_intr_disable frequently, and ixgbe vf will read the
> IXGBE_VTEIMS register everytime. The patch saves the IXGBE_VTEIMS to
> mask to avoid read frequently.
> 
> Signed-off-by: Tonghao Zhang <xiangxia.m.yue@gmail.com>
> Acked-by: Beilei Xing <beilei.xing@intel.com>
Acked-by: Wei Dai <wei.dai@intel.com>

> ---
>  drivers/net/ixgbe/ixgbe_ethdev.c | 55
> +++++++++++++++++++++++++---------------
>  1 file changed, 35 insertions(+), 20 deletions(-)
> 
> diff --git a/drivers/net/ixgbe/ixgbe_ethdev.c
> b/drivers/net/ixgbe/ixgbe_ethdev.c
> index 49f486a..cab243d 100644
> --- a/drivers/net/ixgbe/ixgbe_ethdev.c
> +++ b/drivers/net/ixgbe/ixgbe_ethdev.c
> @@ -270,8 +270,8 @@ static int ixgbevf_dev_link_update(struct
> rte_eth_dev *dev,  static void ixgbevf_dev_stop(struct rte_eth_dev *dev);
> static void ixgbevf_dev_close(struct rte_eth_dev *dev);  static int
> ixgbevf_dev_reset(struct rte_eth_dev *dev); -static void
> ixgbevf_intr_disable(struct ixgbe_hw *hw); -static void
> ixgbevf_intr_enable(struct ixgbe_hw *hw);
> +static void ixgbevf_intr_disable(struct rte_eth_dev *dev); static void
> +ixgbevf_intr_enable(struct rte_eth_dev *dev);
>  static int ixgbevf_dev_stats_get(struct rte_eth_dev *dev,
>  		struct rte_eth_stats *stats);
>  static void ixgbevf_dev_stats_reset(struct rte_eth_dev *dev); @@ -1665,7
> +1665,7 @@ static int ixgbe_l2_tn_filter_init(struct rte_eth_dev *eth_dev)
>  	ixgbevf_dev_stats_reset(eth_dev);
> 
>  	/* Disable the interrupts for VF */
> -	ixgbevf_intr_disable(hw);
> +	ixgbevf_intr_disable(eth_dev);
> 
>  	hw->mac.num_rar_entries = 128; /* The MAX of the underlying PF */
>  	diag = hw->mac.ops.reset_hw(hw);
> @@ -1734,7 +1734,7 @@ static int ixgbe_l2_tn_filter_init(struct
> rte_eth_dev *eth_dev)
>  	rte_intr_callback_register(intr_handle,
>  				   ixgbevf_dev_interrupt_handler, eth_dev);
>  	rte_intr_enable(intr_handle);
> -	ixgbevf_intr_enable(hw);
> +	ixgbevf_intr_enable(eth_dev);
> 
>  	PMD_INIT_LOG(DEBUG, "port %d vendorID=0x%x deviceID=0x%x
> mac.type=%s",
>  		     eth_dev->data->port_id, pci_dev->id.vendor_id, @@
> -1767,7 +1767,7 @@ static int ixgbe_l2_tn_filter_init(struct rte_eth_dev
> *eth_dev)
>  	eth_dev->tx_pkt_burst = NULL;
> 
>  	/* Disable the interrupts for VF */
> -	ixgbevf_intr_disable(hw);
> +	ixgbevf_intr_disable(eth_dev);
> 
>  	rte_free(eth_dev->data->mac_addrs);
>  	eth_dev->data->mac_addrs = NULL;
> @@ -4953,19 +4953,32 @@ static int
> ixgbevf_dev_xstats_get_names(__rte_unused struct rte_eth_dev *dev,
>   * Virtual Function operations
>   */
>  static void
> -ixgbevf_intr_disable(struct ixgbe_hw *hw)
> +ixgbevf_intr_disable(struct rte_eth_dev *dev)
>  {
> +	struct ixgbe_interrupt *intr =
> +		IXGBE_DEV_PRIVATE_TO_INTR(dev->data->dev_private);
> +	struct ixgbe_hw *hw =
> +		IXGBE_DEV_PRIVATE_TO_HW(dev->data->dev_private);
> +
>  	PMD_INIT_FUNC_TRACE();
> 
>  	/* Clear interrupt mask to stop from interrupts being generated */
>  	IXGBE_WRITE_REG(hw, IXGBE_VTEIMC, IXGBE_VF_IRQ_CLEAR_MASK);
> 
>  	IXGBE_WRITE_FLUSH(hw);
> +
> +	/* Clear mask value. */
> +	intr->mask = 0;
>  }
> 
>  static void
> -ixgbevf_intr_enable(struct ixgbe_hw *hw)
> +ixgbevf_intr_enable(struct rte_eth_dev *dev)
>  {
> +	struct ixgbe_interrupt *intr =
> +		IXGBE_DEV_PRIVATE_TO_INTR(dev->data->dev_private);
> +	struct ixgbe_hw *hw =
> +		IXGBE_DEV_PRIVATE_TO_HW(dev->data->dev_private);
> +
>  	PMD_INIT_FUNC_TRACE();
> 
>  	/* VF enable interrupt autoclean */
> @@ -4974,6 +4987,9 @@ static int
> ixgbevf_dev_xstats_get_names(__rte_unused struct rte_eth_dev *dev,
>  	IXGBE_WRITE_REG(hw, IXGBE_VTEIMS,
> IXGBE_VF_IRQ_ENABLE_MASK);
> 
>  	IXGBE_WRITE_FLUSH(hw);
> +
> +	/* Save IXGBE_VTEIMS value to mask. */
> +	intr->mask = IXGBE_VF_IRQ_ENABLE_MASK;
>  }
> 
>  static int
> @@ -5099,7 +5115,7 @@ static int
> ixgbevf_dev_xstats_get_names(__rte_unused struct rte_eth_dev *dev,
>  	rte_intr_enable(intr_handle);
> 
>  	/* Re-enable interrupt for VF */
> -	ixgbevf_intr_enable(hw);
> +	ixgbevf_intr_enable(dev);
> 
>  	return 0;
>  }
> @@ -5113,7 +5129,7 @@ static int
> ixgbevf_dev_xstats_get_names(__rte_unused struct rte_eth_dev *dev,
> 
>  	PMD_INIT_FUNC_TRACE();
> 
> -	ixgbevf_intr_disable(hw);
> +	ixgbevf_intr_disable(dev);
> 
>  	hw->adapter_stopped = 1;
>  	ixgbe_stop_adapter(hw);
> @@ -5611,17 +5627,17 @@ static void ixgbevf_set_vfta_all(struct
> rte_eth_dev *dev, bool on)  {
>  	struct rte_pci_device *pci_dev = RTE_ETH_DEV_TO_PCI(dev);
>  	struct rte_intr_handle *intr_handle = &pci_dev->intr_handle;
> -	uint32_t mask;
> +	struct ixgbe_interrupt *intr =
> +		IXGBE_DEV_PRIVATE_TO_INTR(dev->data->dev_private);
>  	struct ixgbe_hw *hw =
>  		IXGBE_DEV_PRIVATE_TO_HW(dev->data->dev_private);
>  	uint32_t vec = IXGBE_MISC_VEC_ID;
> 
> -	mask = IXGBE_READ_REG(hw, IXGBE_VTEIMS);
>  	if (rte_intr_allow_others(intr_handle))
>  		vec = IXGBE_RX_VEC_START;
> -	mask |= (1 << vec);
> +	intr->mask |= (1 << vec);
>  	RTE_SET_USED(queue_id);
> -	IXGBE_WRITE_REG(hw, IXGBE_VTEIMS, mask);
> +	IXGBE_WRITE_REG(hw, IXGBE_VTEIMS, intr->mask);
> 
>  	rte_intr_enable(intr_handle);
> 
> @@ -5631,19 +5647,19 @@ static void ixgbevf_set_vfta_all(struct
> rte_eth_dev *dev, bool on)  static int
> ixgbevf_dev_rx_queue_intr_disable(struct rte_eth_dev *dev, uint16_t
> queue_id)  {
> -	uint32_t mask;
> +	struct ixgbe_interrupt *intr =
> +		IXGBE_DEV_PRIVATE_TO_INTR(dev->data->dev_private);
>  	struct ixgbe_hw *hw =
>  		IXGBE_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;
>  	uint32_t vec = IXGBE_MISC_VEC_ID;
> 
> -	mask = IXGBE_READ_REG(hw, IXGBE_VTEIMS);
>  	if (rte_intr_allow_others(intr_handle))
>  		vec = IXGBE_RX_VEC_START;
> -	mask &= ~(1 << vec);
> +	intr->mask &= ~(1 << vec);
>  	RTE_SET_USED(queue_id);
> -	IXGBE_WRITE_REG(hw, IXGBE_VTEIMS, mask);
> +	IXGBE_WRITE_REG(hw, IXGBE_VTEIMS, intr->mask);
> 
>  	return 0;
>  }
> @@ -8200,7 +8216,7 @@ static void ixgbevf_mbx_process(struct
> rte_eth_dev *dev)
>  	struct ixgbe_hw *hw =
> IXGBE_DEV_PRIVATE_TO_HW(dev->data->dev_private);
>  	struct ixgbe_interrupt *intr =
>  		IXGBE_DEV_PRIVATE_TO_INTR(dev->data->dev_private);
> -	ixgbevf_intr_disable(hw);
> +	ixgbevf_intr_disable(dev);
> 
>  	/* read-on-clear nic registers here */
>  	eicr = IXGBE_READ_REG(hw, IXGBE_VTEICR); @@ -8217,7 +8233,6 @@
> static void ixgbevf_mbx_process(struct rte_eth_dev *dev)  static int
> ixgbevf_dev_interrupt_action(struct rte_eth_dev *dev)  {
> -	struct ixgbe_hw *hw =
> IXGBE_DEV_PRIVATE_TO_HW(dev->data->dev_private);
>  	struct ixgbe_interrupt *intr =
>  		IXGBE_DEV_PRIVATE_TO_INTR(dev->data->dev_private);
> 
> @@ -8226,7 +8241,7 @@ static void ixgbevf_mbx_process(struct
> rte_eth_dev *dev)
>  		intr->flags &= ~IXGBE_FLAG_MAILBOX;
>  	}
> 
> -	ixgbevf_intr_enable(hw);
> +	ixgbevf_intr_enable(dev);
> 
>  	return 0;
>  }
> --
> 1.8.3.1

Re: [dpdk-dev] [PATCH v2 1/4] net/ixgbevf: unregister irq handler when other interrupts not allowed.

[Tonghao Zhang]

On Fri, Jan 12, 2018 at 4:39 PM, Dai, Wei <wei.dai@intel.com> wrote:
> Hi, Tonghao & Beilei
>
> The issue reported by Tonghao is caused by shortage of igb_uio.
> If you want to use Rx queue interrupt in your DPDK application,
> I suggest use VFIO-PCI to bind NIC port instead of igb_uio.
The performance of igb_uio is better than vfio-pic. So igb_uio is
a better solution. And the PF binded with igb_uio can use the rx queue
interrupt,
because it unregister the irq handler in eal-intr-thread via
rte_intr_callback_unregister.
Then the PF will also not handler the irq (mailbox) from VF, right ?

To support the feature of rx queue interrupt , the PF and VF binded
with igb_uio may unregister the other  irq.

> Currently igb_uio only support single event fd. This fd is triggered by
> both miscellaneous and RX queue interrupt in ixgbe VF.
> But same event fd is added to epoll fd of eal-intr-thread
> and also epoll fd of normal thread for Rx task, this cause the issue.
Yes, it's right

> If VFIO-PCI is used, different event fd for misc and Rx queue are created.
> By the way, your patch also lead to missing of PF to VF reset interrupt.


> So, I am sorry that I can't agree it now.
>
>
>> -----Original Message-----
>> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of
>> xiangxia.m.yue@gmail.com
>> Sent: Friday, January 12, 2018 12:40 AM
>> To: Xing, Beilei <beilei.xing@intel.com>; dev@dpdk.org
>> Cc: Tonghao Zhang <xiangxia.m.yue@gmail.com>
>> Subject: [dpdk-dev] [PATCH v2 1/4] net/ixgbevf: unregister irq handler when
>> other interrupts not allowed.
>>
>> From: Tonghao Zhang <xiangxia.m.yue@gmail.com>
>>
>> When bind the ixgbe VF (e.g 82599 card) to igb_uio and enable the
>> rx-interrupt, there will be more than one epoll_wait on intr_handle.fd.
>> One is in "eal-intr-thread" thread, and the others are in the thread which call
>> the "rte_epoll_wait". The problem is that sometimes "eal-intr-thread" thread
>> will process the rx interrupt, and then rte_epoll_wait can't get the event
>> anymore, and the packets may be lost.
>>
>> The patch unregister the status interrupt handler in "eal-intr-thread"
>> thread and the ixgbe pf is in the same case.
>>
>> Signed-off-by: Tonghao Zhang <xiangxia.m.yue@gmail.com>
>> Acked-by: Beilei Xing <beilei.xing@intel.com>
>> ---
>>  drivers/net/ixgbe/ixgbe_ethdev.c | 15 +++++++++++++++
>>  1 file changed, 15 insertions(+)
>>
>> diff --git a/drivers/net/ixgbe/ixgbe_ethdev.c
>> b/drivers/net/ixgbe/ixgbe_ethdev.c
>> index ff19a56..0e056a2 100644
>> --- a/drivers/net/ixgbe/ixgbe_ethdev.c
>> +++ b/drivers/net/ixgbe/ixgbe_ethdev.c
>> @@ -5078,6 +5078,15 @@ static int
>> ixgbevf_dev_xstats_get_names(__rte_unused struct rte_eth_dev *dev,
>>       }
>>       ixgbevf_configure_msix(dev);
>>
>> +     if (!rte_intr_allow_others(intr_handle)) {
>> +             rte_intr_callback_unregister(intr_handle,
>> +                                          ixgbevf_dev_interrupt_handler,
>> +                                          dev);
>> +             if (dev->data->dev_conf.intr_conf.lsc != 0)
>> +                     PMD_INIT_LOG(INFO, "lsc won't enable because of"
>> +                                  " no intr multiplex");
>> +     }
>> +
>>       /* When a VF port is bound to VFIO-PCI, only miscellaneous interrupt
>>        * is mapped to VFIO vector 0 in eth_ixgbevf_dev_init( ).
>>        * If previous VFIO interrupt mapping setting in eth_ixgbevf_dev_init( )
>> @@ -5120,6 +5129,12 @@ static int
>> ixgbevf_dev_xstats_get_names(__rte_unused struct rte_eth_dev *dev,
>>
>>       ixgbe_dev_clear_queues(dev);
>>
>> +     if (!rte_intr_allow_others(intr_handle))
>> +             /* resume to the default handler */
>> +             rte_intr_callback_register(intr_handle,
>> +                                        ixgbevf_dev_interrupt_handler,
>> +                                        (void *)dev);
>> +
>>       /* Clean datapath event and queue/vec mapping */
>>       rte_intr_efd_disable(intr_handle);
>>       if (intr_handle->intr_vec != NULL) {
>> --
>> 1.8.3.1
>

[dpdk-dev] [PATCH v2 3/4] net/ixgbevf: save IXGBE_VTEIMS to intr->mask for performance.

[xiangxia.m.yue]

From: Tonghao Zhang <zhangtonghao@didichuxing.com>

If dpdk APPs call the rte_eth_dev_rx_intr_enable or
rte_eth_dev_rx_intr_disable frequently, and ixgbe vf will read the
IXGBE_VTEIMS register everytime. We can optimize the driver function.
The patch save the IXGBE_VTEIMS to mask to avoid read frequently.

Signed-off-by: Tonghao Zhang <zhangtonghao@didichuxing.com>
---
 drivers/net/ixgbe/ixgbe_ethdev.c | 55 +++++++++++++++++++++++++---------------
 1 file changed, 35 insertions(+), 20 deletions(-)

diff --git a/drivers/net/ixgbe/ixgbe_ethdev.c b/drivers/net/ixgbe/ixgbe_ethdev.c
index 5d652d8..4cfa6ce 100644
--- a/drivers/net/ixgbe/ixgbe_ethdev.c
+++ b/drivers/net/ixgbe/ixgbe_ethdev.c
@@ -268,8 +268,8 @@ static int ixgbevf_dev_link_update(struct rte_eth_dev *dev,
 static void ixgbevf_dev_stop(struct rte_eth_dev *dev);
 static void ixgbevf_dev_close(struct rte_eth_dev *dev);
 static int  ixgbevf_dev_reset(struct rte_eth_dev *dev);
-static void ixgbevf_intr_disable(struct ixgbe_hw *hw);
-static void ixgbevf_intr_enable(struct ixgbe_hw *hw);
+static void ixgbevf_intr_disable(struct rte_eth_dev *dev);
+static void ixgbevf_intr_enable(struct rte_eth_dev *dev);
 static int ixgbevf_dev_stats_get(struct rte_eth_dev *dev,
 		struct rte_eth_stats *stats);
 static void ixgbevf_dev_stats_reset(struct rte_eth_dev *dev);
@@ -1659,7 +1659,7 @@ static int ixgbe_l2_tn_filter_init(struct rte_eth_dev *eth_dev)
 	ixgbevf_dev_stats_reset(eth_dev);
 
 	/* Disable the interrupts for VF */
-	ixgbevf_intr_disable(hw);
+	ixgbevf_intr_disable(eth_dev);
 
 	hw->mac.num_rar_entries = 128; /* The MAX of the underlying PF */
 	diag = hw->mac.ops.reset_hw(hw);
@@ -1728,7 +1728,7 @@ static int ixgbe_l2_tn_filter_init(struct rte_eth_dev *eth_dev)
 	rte_intr_callback_register(intr_handle,
 				   ixgbevf_dev_interrupt_handler, eth_dev);
 	rte_intr_enable(intr_handle);
-	ixgbevf_intr_enable(hw);
+	ixgbevf_intr_enable(eth_dev);
 
 	PMD_INIT_LOG(DEBUG, "port %d vendorID=0x%x deviceID=0x%x mac.type=%s",
 		     eth_dev->data->port_id, pci_dev->id.vendor_id,
@@ -1761,7 +1761,7 @@ static int ixgbe_l2_tn_filter_init(struct rte_eth_dev *eth_dev)
 	eth_dev->tx_pkt_burst = NULL;
 
 	/* Disable the interrupts for VF */
-	ixgbevf_intr_disable(hw);
+	ixgbevf_intr_disable(eth_dev);
 
 	rte_free(eth_dev->data->mac_addrs);
 	eth_dev->data->mac_addrs = NULL;
@@ -4945,19 +4945,32 @@ static int ixgbevf_dev_xstats_get_names(__rte_unused struct rte_eth_dev *dev,
  * Virtual Function operations
  */
 static void
-ixgbevf_intr_disable(struct ixgbe_hw *hw)
+ixgbevf_intr_disable(struct rte_eth_dev *dev)
 {
+	struct ixgbe_interrupt *intr =
+		IXGBE_DEV_PRIVATE_TO_INTR(dev->data->dev_private);
+	struct ixgbe_hw *hw =
+		IXGBE_DEV_PRIVATE_TO_HW(dev->data->dev_private);
+
 	PMD_INIT_FUNC_TRACE();
 
 	/* Clear interrupt mask to stop from interrupts being generated */
 	IXGBE_WRITE_REG(hw, IXGBE_VTEIMC, IXGBE_VF_IRQ_CLEAR_MASK);
 
 	IXGBE_WRITE_FLUSH(hw);
+
+	/* Clear mask value. */
+	intr->mask = 0;
 }
 
 static void
-ixgbevf_intr_enable(struct ixgbe_hw *hw)
+ixgbevf_intr_enable(struct rte_eth_dev *dev)
 {
+	struct ixgbe_interrupt *intr =
+		IXGBE_DEV_PRIVATE_TO_INTR(dev->data->dev_private);
+	struct ixgbe_hw *hw =
+		IXGBE_DEV_PRIVATE_TO_HW(dev->data->dev_private);
+
 	PMD_INIT_FUNC_TRACE();
 
 	/* VF enable interrupt autoclean */
@@ -4966,6 +4979,9 @@ static int ixgbevf_dev_xstats_get_names(__rte_unused struct rte_eth_dev *dev,
 	IXGBE_WRITE_REG(hw, IXGBE_VTEIMS, IXGBE_VF_IRQ_ENABLE_MASK);
 
 	IXGBE_WRITE_FLUSH(hw);
+
+	/* Save IXGBE_VTEIMS value to mask. */
+	intr->mask = IXGBE_VF_IRQ_ENABLE_MASK;
 }
 
 static int
@@ -5091,7 +5107,7 @@ static int ixgbevf_dev_xstats_get_names(__rte_unused struct rte_eth_dev *dev,
 	rte_intr_enable(intr_handle);
 
 	/* Re-enable interrupt for VF */
-	ixgbevf_intr_enable(hw);
+	ixgbevf_intr_enable(dev);
 
 	return 0;
 }
@@ -5105,7 +5121,7 @@ static int ixgbevf_dev_xstats_get_names(__rte_unused struct rte_eth_dev *dev,
 
 	PMD_INIT_FUNC_TRACE();
 
-	ixgbevf_intr_disable(hw);
+	ixgbevf_intr_disable(dev);
 
 	hw->adapter_stopped = 1;
 	ixgbe_stop_adapter(hw);
@@ -5603,17 +5619,17 @@ static void ixgbevf_set_vfta_all(struct rte_eth_dev *dev, bool on)
 {
 	struct rte_pci_device *pci_dev = RTE_ETH_DEV_TO_PCI(dev);
 	struct rte_intr_handle *intr_handle = &pci_dev->intr_handle;
-	uint32_t mask;
+	struct ixgbe_interrupt *intr =
+		IXGBE_DEV_PRIVATE_TO_INTR(dev->data->dev_private);
 	struct ixgbe_hw *hw =
 		IXGBE_DEV_PRIVATE_TO_HW(dev->data->dev_private);
 	uint32_t vec = IXGBE_MISC_VEC_ID;
 
-	mask = IXGBE_READ_REG(hw, IXGBE_VTEIMS);
 	if (rte_intr_allow_others(intr_handle))
 		vec = IXGBE_RX_VEC_START;
-	mask |= (1 << vec);
+	intr->mask |= (1 << vec);
 	RTE_SET_USED(queue_id);
-	IXGBE_WRITE_REG(hw, IXGBE_VTEIMS, mask);
+	IXGBE_WRITE_REG(hw, IXGBE_VTEIMS, intr->mask);
 
 	rte_intr_enable(intr_handle);
 
@@ -5623,19 +5639,19 @@ static void ixgbevf_set_vfta_all(struct rte_eth_dev *dev, bool on)
 static int
 ixgbevf_dev_rx_queue_intr_disable(struct rte_eth_dev *dev, uint16_t queue_id)
 {
-	uint32_t mask;
+	struct ixgbe_interrupt *intr =
+		IXGBE_DEV_PRIVATE_TO_INTR(dev->data->dev_private);
 	struct ixgbe_hw *hw =
 		IXGBE_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;
 	uint32_t vec = IXGBE_MISC_VEC_ID;
 
-	mask = IXGBE_READ_REG(hw, IXGBE_VTEIMS);
 	if (rte_intr_allow_others(intr_handle))
 		vec = IXGBE_RX_VEC_START;
-	mask &= ~(1 << vec);
+	intr->mask &= ~(1 << vec);
 	RTE_SET_USED(queue_id);
-	IXGBE_WRITE_REG(hw, IXGBE_VTEIMS, mask);
+	IXGBE_WRITE_REG(hw, IXGBE_VTEIMS, intr->mask);
 
 	return 0;
 }
@@ -8191,7 +8207,7 @@ static void ixgbevf_mbx_process(struct rte_eth_dev *dev)
 	struct ixgbe_hw *hw = IXGBE_DEV_PRIVATE_TO_HW(dev->data->dev_private);
 	struct ixgbe_interrupt *intr =
 		IXGBE_DEV_PRIVATE_TO_INTR(dev->data->dev_private);
-	ixgbevf_intr_disable(hw);
+	ixgbevf_intr_disable(dev);
 
 	/* read-on-clear nic registers here */
 	eicr = IXGBE_READ_REG(hw, IXGBE_VTEICR);
@@ -8208,7 +8224,6 @@ static void ixgbevf_mbx_process(struct rte_eth_dev *dev)
 static int
 ixgbevf_dev_interrupt_action(struct rte_eth_dev *dev)
 {
-	struct ixgbe_hw *hw = IXGBE_DEV_PRIVATE_TO_HW(dev->data->dev_private);
 	struct ixgbe_interrupt *intr =
 		IXGBE_DEV_PRIVATE_TO_INTR(dev->data->dev_private);
 
@@ -8217,7 +8232,7 @@ static void ixgbevf_mbx_process(struct rte_eth_dev *dev)
 		intr->flags &= ~IXGBE_FLAG_MAILBOX;
 	}
 
-	ixgbevf_intr_enable(hw);
+	ixgbevf_intr_enable(dev);
 
 	return 0;
 }
-- 
1.8.3.1