patches for DPDK stable branches
 help / color / mirror / Atom feed
* [PATCH] net/ixgbe: don't create a delayed interrupt handler if one already exists
       [not found] <CANDF9xC+4yBVxX8sgXzM0EdYcQFhzonG8FnnTQ1trSDxAkvAaQ@mail.gmail.com>
@ 2024-04-18 13:53 ` edwin.brossette
  0 siblings, 0 replies; only message in thread
From: edwin.brossette @ 2024-04-18 13:53 UTC (permalink / raw)
  To: dev; +Cc: olivier.matz, Edwin Brossette, stable

From: Edwin Brossette <edwin.brossette@6wind.com>

Since link state may need some time to stabilize after a link state
change, we cannot update the link state right after one occurs. So link
state change interrupts (lsc) are handled after a delay. To do this, an
alarm to call a delayed handler is programmed. This delayed handler is
tasked with updating the link after a variable delay of one to four
seconds which should be enough time for the link state to become stable
again.

However, a problem can occur with some models of network cards. For
example, ixgbe_mac_X550EM_x may trigger this interrupt twice because
another interrupt signal is received on the General Purpose Interrupt
pin SPD0, which has the same interrupt handler. In such a case, the
delayed interrupt handler would be programmed to be executed twice.

Since we save the original interrupt mask value to restore it after the
delayed handler is done with its work, we end up overwritting its value
after the second alarm is programmed. Even worse: when restoring it the
first time, the saved original mask variable is reset to 0, so we end up
completely disabling all interrupts when trying to restore this mask
after the second time the delayed handler is executed.

Add a check on the interrupt mask value when programming the alarm for
the delayed handler. If the bit for lsc interrupts is unset, it means an
alarm was already programmed for the delayed handler. In this case, skip
the alarm creation.

Fixes: 9b667210700e ("net/ixgbe: fix blocked interrupts")
Cc: stable@dpdk.org

Signed-off-by: Edwin Brossette <edwin.brossette@6wind.com>
---
 drivers/net/ixgbe/ixgbe_ethdev.c | 22 ++++++++++++++--------
 1 file changed, 14 insertions(+), 8 deletions(-)

diff --git a/drivers/net/ixgbe/ixgbe_ethdev.c b/drivers/net/ixgbe/ixgbe_ethdev.c
index c61c52b2966b..52cafcbc965f 100644
--- a/drivers/net/ixgbe/ixgbe_ethdev.c
+++ b/drivers/net/ixgbe/ixgbe_ethdev.c
@@ -4667,14 +4667,20 @@ ixgbe_dev_interrupt_action(struct rte_eth_dev *dev)
 			timeout = IXGBE_LINK_DOWN_CHECK_TIMEOUT;
 
 		ixgbe_dev_link_status_print(dev);
-		if (rte_eal_alarm_set(timeout * 1000,
-				      ixgbe_dev_interrupt_delayed_handler, (void *)dev) < 0)
-			PMD_DRV_LOG(ERR, "Error setting alarm");
-		else {
-			/* remember original mask */
-			intr->mask_original = intr->mask;
-			/* only disable lsc interrupt */
-			intr->mask &= ~IXGBE_EIMS_LSC;
+
+		/* Don't program delayed handler if LSC interrupt is disabled.
+		 * It means one is already programmed.
+		 */
+		if (intr->mask & IXGBE_EIMS_LSC){
+			if (rte_eal_alarm_set(timeout * 1000,
+					      ixgbe_dev_interrupt_delayed_handler, (void *)dev) < 0)
+				PMD_DRV_LOG(ERR, "Error setting alarm");
+			else {
+				/* remember original mask */
+				intr->mask_original = intr->mask;
+				/* only disable lsc interrupt */
+				intr->mask &= ~IXGBE_EIMS_LSC;
+			}
 		}
 	}
 
-- 
2.35.0.4.g44a5d4affccf


^ permalink raw reply	[flat|nested] only message in thread

only message in thread, other threads:[~2024-04-18 13:53 UTC | newest]

Thread overview: (only message) (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
     [not found] <CANDF9xC+4yBVxX8sgXzM0EdYcQFhzonG8FnnTQ1trSDxAkvAaQ@mail.gmail.com>
2024-04-18 13:53 ` [PATCH] net/ixgbe: don't create a delayed interrupt handler if one already exists edwin.brossette

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).