From patchwork Thu Sep 7 03:15:29 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Saurabh Singhal X-Patchwork-Id: 131227 X-Patchwork-Delegate: qi.z.zhang@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 AE5B442534; Thu, 7 Sep 2023 10:25:43 +0200 (CEST) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id CF28C402EC; Thu, 7 Sep 2023 10:25:35 +0200 (CEST) Received: from mail-pj1-f41.google.com (mail-pj1-f41.google.com [209.85.216.41]) by mails.dpdk.org (Postfix) with ESMTP id 4EB144029D for ; Thu, 7 Sep 2023 05:15:38 +0200 (CEST) Received: by mail-pj1-f41.google.com with SMTP id 98e67ed59e1d1-269304c135aso372156a91.3 for ; Wed, 06 Sep 2023 20:15:38 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=arista.com; s=google; t=1694056537; x=1694661337; 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=7Xob+TBXK70dN4oo5VBxzAZn9ZWTDhHYK2xCKf71fzo=; b=NF1kWD4TJa6qbFFVrPjvYP1JKY6ifuN33BH53rHPR3HBd4aEEKYF2IMOxyavDGKcQR 3hGm1dRIzTlLnRGPYoh8FxnYQkKvuKCLt8ikKrkJz8e4KdZrhuV2EWSpNV+WMXIIBuii 84bMxLjToWwA46wBxYBmXXnfXIERvjAPgV9R8j3UiSyaeqA+xGCmjMhlUcS19FXKQz9x dIlQeBEC816quYU0CTUj6qSoB/fjgwxLbMONsuYX6hqT7R+sxOSSystSrUm5iotYQ7Hm lh69KoErfaPQOt3M2CK0wThD/UlQRGdk81y4ZSjYaQ1QVXbJa3i9eAFp77M8+vOJpXFE ruyg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20221208; t=1694056537; x=1694661337; 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=7Xob+TBXK70dN4oo5VBxzAZn9ZWTDhHYK2xCKf71fzo=; b=Tx0p962QXP46qDpZdEZ9FT3NW9j9VPXCFWFVmxnSjQDJrCGoERJGpLsSHDvyWStdFw 2MtMBZJyOx8f5+hNixngVvi/W/VK+/BlUe93QjptmnrD9JcA1NdqPW7o7mBEbyX2h0J3 M7YN847xmpx7wy1ThYiPf89JqX//W3Ql9fVe58l8LCfrYB6cCoFulXDqBcdAL3KLnBVs bK8elqxqwbEaLjBKiKXyA8ZNYwYVK5piBzh3HJJfFpzL/o+BczUO4r0R7ArXsf0SvVIh O5umCAzU44Waj+duyEfbW6vmFWs4brxEHYlr5YS5023lnOZQiGdHKCTei2gGKwcgmEjL sjTg== X-Gm-Message-State: AOJu0YzkYr1nji7OHEZDi73f7MAV+Zqv9CC5ynlQoXx+BlUMz37u+BjN exc1YnWbIllyS+qTOcQ+7bovbg== X-Google-Smtp-Source: AGHT+IHsyvTajjj9dSqVVVCqnH5Q5th7INU351JxFpOSjWlMGsily2SQtQsmb77ZsEdZ3XMXr6B18w== X-Received: by 2002:a17:90a:a790:b0:262:e6d2:2d6 with SMTP id f16-20020a17090aa79000b00262e6d202d6mr17670379pjq.47.1694056537276; Wed, 06 Sep 2023 20:15:37 -0700 (PDT) Received: from saurabhs-sfelagsubintfimpl-0.sjc.aristanetworks.com ([74.123.28.18]) by smtp.gmail.com with ESMTPSA id 9-20020a17090a19c900b00263f5ac814esm518922pjj.38.2023.09.06.20.15.36 (version=TLS1_2 cipher=ECDHE-ECDSA-AES128-GCM-SHA256 bits=128/128); Wed, 06 Sep 2023 20:15:36 -0700 (PDT) From: Saurabh Singhal To: Thomas Monjalon , Jingjing Wu , Beilei Xing Cc: dev@dpdk.org, Saurabh Singhal Subject: [PATCH v4] net/iavf: unregister intr handler before FD close Date: Wed, 6 Sep 2023 20:15:29 -0700 Message-ID: <20230907031529.84232-1-saurabhs@arista.com> X-Mailer: git-send-email 2.41.0 In-Reply-To: References: MIME-Version: 1.0 X-Mailman-Approved-At: Thu, 07 Sep 2023 10:25:32 +0200 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 Unregister VFIO interrupt handler before the interrupt fd gets closed in case iavf_dev_init() returns an error. dpdk creates a standalone thread named eal-intr-thread for processing interrupts for the PCI devices. The interrupt handler callbacks are registered by the VF driver(iavf, in this case). When we do a PCI probe of the network interfaces, we register an interrupt handler, open a vfio-device fd using ioctl, and an eventfd in dpdk. These interrupt sources are registered in a global linked list that the eal-intr-thread keeps iterating over for handling the interrupts. In our internal testing, we see eal-intr-thread crash in these two ways: Error adding fd 660 epoll_ctl, Operation not permitted or Error adding fd 660 epoll_ctl, Bad file descriptor epoll_ctl() returns EPERM if the target fd does not support poll. It returns EBADF when the epoll fd itself is closed or the target fd is closed. When the first type of crash happens, we see that the fd 660 is anon_inode:[vfio-device] which does not support poll. When the second type of crash happens, we could see from the fd map of the crashing process that the fd 660 was already closed. This means the said fd has been closed and in certain cases may have been reassigned to a different device by the operating system but the eal-intr-thread does not know about it. We observed that these crashes were always accompanied by an error in iavf_dev_init() after rte_intr_callback_register() and iavf_enable_irq0() have already happened. In the error path, the intr_handle_fd was being closed but the interrupt handler wasn't being unregistered. The fix is to unregister the interrupt handle in the iavf_dev_init() error path. Ensure proper cleanup if iavf_security_init() or iavf_security_ctx_create() fail. Earlier, we were leaking memory by simply returning from iavf_dev_init(). Signed-off-by: Saurabh Singhal Acked-by: Qi Zhang --- .mailmap | 1 + drivers/net/iavf/iavf_ethdev.c | 22 ++++++++++++++++++++-- 2 files changed, 21 insertions(+), 2 deletions(-) diff --git a/.mailmap b/.mailmap index 864d33ee46..4dac53011b 100644 --- a/.mailmap +++ b/.mailmap @@ -1227,6 +1227,7 @@ Satananda Burla Satha Rao Satheesh Paul Sathesh Edara +Saurabh Singhal Savinay Dharmappa Scott Branden Scott Daniels diff --git a/drivers/net/iavf/iavf_ethdev.c b/drivers/net/iavf/iavf_ethdev.c index f2fc5a5621..47c1399a52 100644 --- a/drivers/net/iavf/iavf_ethdev.c +++ b/drivers/net/iavf/iavf_ethdev.c @@ -133,6 +133,8 @@ static int iavf_dev_rx_queue_intr_enable(struct rte_eth_dev *dev, uint16_t queue_id); static int iavf_dev_rx_queue_intr_disable(struct rte_eth_dev *dev, uint16_t queue_id); +static void iavf_dev_interrupt_handler(void *param); +static void iavf_disable_irq0(struct iavf_hw *hw); static int iavf_dev_flow_ops_get(struct rte_eth_dev *dev, const struct rte_flow_ops **ops); static int iavf_set_mc_addr_list(struct rte_eth_dev *dev, @@ -2709,13 +2711,13 @@ iavf_dev_init(struct rte_eth_dev *eth_dev) ret = iavf_security_ctx_create(adapter); if (ret) { PMD_INIT_LOG(ERR, "failed to create ipsec crypto security instance"); - return ret; + goto flow_init_err; } ret = iavf_security_init(adapter); if (ret) { PMD_INIT_LOG(ERR, "failed to initialized ipsec crypto resources"); - return ret; + goto security_init_err; } } @@ -2728,7 +2730,23 @@ iavf_dev_init(struct rte_eth_dev *eth_dev) return 0; +security_init_err: + iavf_security_ctx_destroy(adapter); + flow_init_err: + iavf_disable_irq0(hw); + + if (vf->vf_res->vf_cap_flags & VIRTCHNL_VF_OFFLOAD_WB_ON_ITR) { + /* disable uio intr before callback unregiser */ + rte_intr_disable(pci_dev->intr_handle); + + /* unregister callback func from eal lib */ + rte_intr_callback_unregister(pci_dev->intr_handle, + iavf_dev_interrupt_handler, eth_dev); + } else { + rte_eal_alarm_cancel(iavf_dev_alarm_handler, eth_dev); + } + rte_free(eth_dev->data->mac_addrs); eth_dev->data->mac_addrs = NULL;