From patchwork Thu Apr 18 13:53:07 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: edwin.brossette@6wind.com X-Patchwork-Id: 139494 X-Patchwork-Delegate: bruce.richardson@intel.com Return-Path: X-Original-To: patchwork@inbox.dpdk.org Delivered-To: patchwork@inbox.dpdk.org Received: from mails.dpdk.org (mails.dpdk.org [217.70.189.124]) by inbox.dpdk.org (Postfix) with ESMTP id 5358643EA2; Thu, 18 Apr 2024 15:53:18 +0200 (CEST) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id B88B840A7D; Thu, 18 Apr 2024 15:53:17 +0200 (CEST) Received: from mail-ej1-f97.google.com (mail-ej1-f97.google.com [209.85.218.97]) by mails.dpdk.org (Postfix) with ESMTP id A157D40647 for ; Thu, 18 Apr 2024 15:53:16 +0200 (CEST) Received: by mail-ej1-f97.google.com with SMTP id a640c23a62f3a-a555faf94fcso97665366b.0 for ; Thu, 18 Apr 2024 06:53:16 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=6wind.com; s=google; t=1713448396; x=1714053196; darn=dpdk.org; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:from:to:cc:subject:date :message-id:reply-to; bh=99EkEXoADGp35ztea/DScD8wHxV+lZrBFpUscb/WQbE=; b=Q648RXfPn23v6RQ4JPz43A6up4zTVzcIaGdsOc4EBLt6bS/LqD+Vn2wojJQ22qT6Tp eZ+hye0VtxXMbFjgMAns4ESbXEgf5fvg9Y854I3kCYHbF5gqz8eVdbcVYWc/rIMG93+D 84Pfd3T+8XB4aqw1IP1bHvYsD0QV5UA3clGIJgsBFZItaOh4UDlXmLyCj/RHnrBCspzB OgA91oY0RDC5k0FbGgiXGBkFIksdZ8OxMHhxyLeT54TXdUVrsoTVA5+yicVeV435bIsz ahARayPrblCjVWnqnC9RrtNGxMPVTjIMHEvml6ERYe/EjWVohWg5IrKj4nlAShHFKZ0d m3sQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1713448396; x=1714053196; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=99EkEXoADGp35ztea/DScD8wHxV+lZrBFpUscb/WQbE=; b=GrH04Qy1KyUex9Mb4+NvAGb3CnCBoVD8Mpj1i/GNc5iXFIAsb2z2Itysht7bkCALWh UP6Mai7G1vZIymbRIRArs/W0OPx4PYSOT5mGS/599q+iE5Ww5VSnvS3z1juSyRh+3ZUe jMeUY/SERaw4leaqDrUOQLLsMnfdTI5cfnDOQasZ5ASuE+8Zx90jqYoJOq3Pdw1TVLHG 4TpuKt9TSHl6jWL+Oa8x67jqZqX6rX8emHK9zoEF7bbG9bhQkRpL8OM1bSErFUfVzEbw Xhl/wieqt6M6AXn0Kenvbn83Z+vITXmtdX/ZewR8j902muW9KPg3vSgc5XhObQ7wP7Li C+1A== X-Gm-Message-State: AOJu0YyaHwN+ixQQMUacDOPOK3JSh9onZIqtC+B+eCg+Wt90tY74JobR Ruczj8pCNFh0zO/nhy5jvM882cf7DVlu4/NR9UGVXBCeEYn6r/SWZdz7InoMfprvw4BbCU+ZV60 1Q68pH/eGryejzSbTmVPjVq5TysMTyFwXyXfLL6b8 X-Google-Smtp-Source: AGHT+IHzP2AAeh1qtn8SbiQ9OqQSEq91rmqLbeoOcb7k9rkJBelv06QQf88ALB0g8zf0Ytg0lVsAv7RmosVg X-Received: by 2002:a17:906:4686:b0:a52:33b1:80a with SMTP id a6-20020a170906468600b00a5233b1080amr1787736ejr.10.1713448396193; Thu, 18 Apr 2024 06:53:16 -0700 (PDT) Received: from smtpservice.6wind.com ([185.13.181.2]) by smtp-relay.gmail.com with ESMTP id cs21-20020a170906dc9500b00a51c764c93esm31713ejc.83.2024.04.18.06.53.15; Thu, 18 Apr 2024 06:53:16 -0700 (PDT) X-Relaying-Domain: 6wind.com Received: from localhost (dio.dev.6wind.com [10.17.1.86]) by smtpservice.6wind.com (Postfix) with ESMTP id BDE366011D; Thu, 18 Apr 2024 15:53:15 +0200 (CEST) From: edwin.brossette@6wind.com To: dev@dpdk.org Cc: olivier.matz@6wind.com, Edwin Brossette , stable@dpdk.org Subject: [PATCH] net/ixgbe: don't create a delayed interrupt handler if one already exists Date: Thu, 18 Apr 2024 15:53:07 +0200 Message-Id: <20240418135307.3270094-1-edwin.brossette@6wind.com> X-Mailer: git-send-email 2.35.0.4.g44a5d4affccf In-Reply-To: References: MIME-Version: 1.0 X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org From: Edwin Brossette 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 --- 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; + } } }