[dpdk-dev] net/ixgbe: fix interrupt block issue

Message ID 1484528629-18298-1-git-send-email-qi.z.zhang@intel.com (mailing list archive)
State Changes Requested, archived
Delegated to: Ferruh Yigit
Headers

Checks

Context Check Description
ci/checkpatch warning coding style issues
ci/Intel compilation fail apply patch file failure

Commit Message

Qi Zhang Jan. 16, 2017, 1:03 a.m. UTC
  When handle link status change interrupt, interrupt
will be blocked until delayed handler finish, the
duration is at least 1 second, this may cause following
VF to PF mailbox traffic be blocked and sometimes PF
can't ack to VF in time before VF think it's time out.
This patch remove this limitation, interrupt will be
enabled before interrupt handler finish, and a flag is
used to prevent re-entering delayed handler.

Signed-off-by: Qi Zhang <qi.z.zhang@intel.com>
---
 drivers/net/ixgbe/ixgbe_ethdev.c | 19 +++++++++----------
 drivers/net/ixgbe/ixgbe_ethdev.h |  2 ++
 2 files changed, 11 insertions(+), 10 deletions(-)
  

Comments

Ferruh Yigit Jan. 16, 2017, 1:42 p.m. UTC | #1
Hi Qi,

On 1/16/2017 1:03 AM, Qi Zhang wrote:
> When handle link status change interrupt, interrupt
> will be blocked until delayed handler finish, the
> duration is at least 1 second, this may cause following
> VF to PF mailbox traffic be blocked and sometimes PF
> can't ack to VF in time before VF think it's time out.
> This patch remove this limitation, interrupt will be
> enabled before interrupt handler finish, and a flag is
> used to prevent re-entering delayed handler.
> 
> Signed-off-by: Qi Zhang <qi.z.zhang@intel.com>

Can you please rebase patch to latest next-net?

Some part of this patch conflicts with the work that removes direct
access from eth_dev to pci_dev.

Thanks,
ferruh
  

Patch

diff --git a/drivers/net/ixgbe/ixgbe_ethdev.c b/drivers/net/ixgbe/ixgbe_ethdev.c
index eb9b340..7efff3c 100644
--- a/drivers/net/ixgbe/ixgbe_ethdev.c
+++ b/drivers/net/ixgbe/ixgbe_ethdev.c
@@ -3459,7 +3459,6 @@  ixgbe_dev_interrupt_action(struct rte_eth_dev *dev)
 		IXGBE_DEV_PRIVATE_TO_INTR(dev->data->dev_private);
 	int64_t timeout;
 	struct rte_eth_link link;
-	int intr_enable_delay = false;
 	struct ixgbe_hw *hw =
 		IXGBE_DEV_PRIVATE_TO_HW(dev->data->dev_private);
 
@@ -3475,7 +3474,7 @@  ixgbe_dev_interrupt_action(struct rte_eth_dev *dev)
 		intr->flags &= ~IXGBE_FLAG_PHY_INTERRUPT;
 	}
 
-	if (intr->flags & IXGBE_FLAG_NEED_LINK_UPDATE) {
+	if (intr->flags & IXGBE_FLAG_NEED_LINK_UPDATE && !intr->delay) {
 		/* get the link status before link update, for predicting later */
 		memset(&link, 0, sizeof(link));
 		rte_ixgbe_dev_atomic_read_link_status(dev, &link);
@@ -3493,19 +3492,15 @@  ixgbe_dev_interrupt_action(struct rte_eth_dev *dev)
 
 		ixgbe_dev_link_status_print(dev);
 
-		intr_enable_delay = true;
-	}
-
-	if (intr_enable_delay) {
+		intr->delay = true;
 		if (rte_eal_alarm_set(timeout * 1000,
 				      ixgbe_dev_interrupt_delayed_handler, (void *)dev) < 0)
 			PMD_DRV_LOG(ERR, "Error setting alarm");
-	} else {
-		PMD_DRV_LOG(DEBUG, "enable intr immediately");
-		ixgbe_enable_intr(dev);
-		rte_intr_enable(&(dev->pci_dev->intr_handle));
 	}
 
+	PMD_DRV_LOG(DEBUG, "enable intr immediately");
+	ixgbe_enable_intr(dev);
+	rte_intr_enable(&(dev->pci_dev->intr_handle));
 
 	return 0;
 }
@@ -3534,6 +3529,8 @@  ixgbe_dev_interrupt_delayed_handler(void *param)
 		IXGBE_DEV_PRIVATE_TO_HW(dev->data->dev_private);
 	uint32_t eicr;
 
+	ixgbe_disable_intr(hw);
+
 	eicr = IXGBE_READ_REG(hw, IXGBE_EICR);
 	if (eicr & IXGBE_EICR_MAILBOX)
 		ixgbe_pf_mbx_process(dev);
@@ -3550,6 +3547,8 @@  ixgbe_dev_interrupt_delayed_handler(void *param)
 		_rte_eth_dev_callback_process(dev, RTE_ETH_EVENT_INTR_LSC, NULL);
 	}
 
+	intr->delay = false;
+
 	PMD_DRV_LOG(DEBUG, "enable intr in delayed handler S[%08x]", eicr);
 	ixgbe_enable_intr(dev);
 	rte_intr_enable(&(dev->pci_dev->intr_handle));
diff --git a/drivers/net/ixgbe/ixgbe_ethdev.h b/drivers/net/ixgbe/ixgbe_ethdev.h
index 4ff6338..922fcff 100644
--- a/drivers/net/ixgbe/ixgbe_ethdev.h
+++ b/drivers/net/ixgbe/ixgbe_ethdev.h
@@ -165,6 +165,8 @@  struct ixgbe_hw_fdir_info {
 struct ixgbe_interrupt {
 	uint32_t flags;
 	uint32_t mask;
+	/* to prevent re-enter delayed handler */
+	uint8_t delay;
 };
 
 struct ixgbe_stat_mapping_registers {