From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mails.dpdk.org (mails.dpdk.org [217.70.189.124]) by inbox.dpdk.org (Postfix) with ESMTP id 31D6B42506; Wed, 6 Sep 2023 07:24:30 +0200 (CEST) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 164354029D; Wed, 6 Sep 2023 07:24:30 +0200 (CEST) Received: from mgamail.intel.com (mgamail.intel.com [192.55.52.93]) by mails.dpdk.org (Postfix) with ESMTP id 98E014026A for ; Wed, 6 Sep 2023 07:24:28 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=intel.com; i=@intel.com; q=dns/txt; s=Intel; t=1693977868; x=1725513868; h=from:to:cc:subject:date:message-id:references: in-reply-to:content-transfer-encoding:mime-version; bh=kgy5PMGbYMDy7Hx66yQF7YMv4r2javooizOa+S10eHE=; b=V0CXWn3uIabtcgn4bDrDbl91eDFm9Lsnx6pH74IAcrWp+Ozu4jcUrPaU 8zy5rrIbllLrCMs4ZSxgavPJ6Pqzszta3xTowSYrHese+MTUgUfjKGcid LSKMzEMYLrrTDhEzlgWuYTvkMK20/yVEWcVdit3fs0Sxnorails2ixp3L LR7/Az2hHAZi2b37UTiARsXMPc+Yx+tLpPrWIESDteyqHMXKT+9zKii1Q +TIZ21kIxUrM0EmVNiQ7z2YLI+H38rKO06w3asvx/oOblNzGJeOie1LeT xjYCgvLFlfbEQKk2F9auCQdfh+FNjiAYPs1aeHCCji4KO1fUI7KmmbwSc A==; X-IronPort-AV: E=McAfee;i="6600,9927,10824"; a="374366747" X-IronPort-AV: E=Sophos;i="6.02,231,1688454000"; d="scan'208";a="374366747" Received: from fmsmga002.fm.intel.com ([10.253.24.26]) by fmsmga102.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 05 Sep 2023 22:24:25 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=McAfee;i="6600,9927,10824"; a="856278564" X-IronPort-AV: E=Sophos;i="6.02,231,1688454000"; d="scan'208";a="856278564" Received: from orsmsx601.amr.corp.intel.com ([10.22.229.14]) by fmsmga002.fm.intel.com with ESMTP/TLS/AES256-GCM-SHA384; 05 Sep 2023 22:24:25 -0700 Received: from orsmsx611.amr.corp.intel.com (10.22.229.24) by ORSMSX601.amr.corp.intel.com (10.22.229.14) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.2507.27; Tue, 5 Sep 2023 22:24:24 -0700 Received: from ORSEDG602.ED.cps.intel.com (10.7.248.7) by orsmsx611.amr.corp.intel.com (10.22.229.24) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.2507.27 via Frontend Transport; Tue, 5 Sep 2023 22:24:24 -0700 Received: from NAM11-BN8-obe.outbound.protection.outlook.com (104.47.58.169) by edgegateway.intel.com (134.134.137.103) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.1.2507.27; Tue, 5 Sep 2023 22:24:24 -0700 ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=ThG3p+iqjKonyDw2dIOY2n4/sFzLAWfKjgd4ng8Dq0MwQQ1fjnrHmYv6ycmstNnfQEVpldjyDGPaOO2NevP1KOjlt2SkkX3kl41xMtRtH/L1E9fVfsafFfqvPOJjrPsli8TCdkYpANJ5xhIxm6vWliiOyFz9YTFLujfbAKyKzZxBzGbnr8+2KsauWvTMuOKeEAJz27xtBwxarxhZiNlanJbL6TMmvFfG/jpLc0l6gCG5CviYgEJ+bhYeor7XtkP3SngrF5Si/s9uOc5ccZ4yLPs+8etLYClzL6IdhXV1C3GgiLK+3gSHWfaIqf1rOx7Ztf3ie+qbpTRD79qd/JePog== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector9901; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-AntiSpam-MessageData-ChunkCount:X-MS-Exchange-AntiSpam-MessageData-0:X-MS-Exchange-AntiSpam-MessageData-1; bh=zjnsZycFpNPh8oaAenio9wBnowXLQHWkYvxNp7K0FvQ=; b=iavxErkKncbFidI5X1EhPu7Ce5+W3lryKBYYAdsFzJqYDisEJ/HyadJoE22RUrSL0K5eF/Kl2pbyNDJUSaCHw2RKdl5VRrgbOHPxmXh3GDE7gQfc7rZrMo/ZzJRljZg9hpskjUk2hupG5ChvDfgvrJvkT+PN07/OjyaBxmp4poJQb8mFmZT9cpxd2rMiL1gjLKau5NBZqEA354VQMTQKHV92OG9Xy43m9jpEdtiicH0xri8LEwn3YE6Q6bX5hMAX/whYBBKLuCIXpXDic7k8RSLWXcbxNCfeGlpXGmqAL0MAp/gipcdOBQBjTELxONzSPIZ4iVHOf0J+AtqW6T4Zfg== ARC-Authentication-Results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=intel.com; dmarc=pass action=none header.from=intel.com; dkim=pass header.d=intel.com; arc=none Received: from DM4PR11MB5994.namprd11.prod.outlook.com (2603:10b6:8:5d::20) by BL1PR11MB5286.namprd11.prod.outlook.com (2603:10b6:208:312::16) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.6745.34; Wed, 6 Sep 2023 05:24:19 +0000 Received: from DM4PR11MB5994.namprd11.prod.outlook.com ([fe80::8a49:15a2:ab69:91c3]) by DM4PR11MB5994.namprd11.prod.outlook.com ([fe80::8a49:15a2:ab69:91c3%4]) with mapi id 15.20.6745.030; Wed, 6 Sep 2023 05:24:19 +0000 From: "Zhang, Qi Z" To: "Singhal, Saurabh" , Thomas Monjalon , "Wu, Jingjing" , "Xing, Beilei" CC: "dev@dpdk.org" , "Singhal, Saurabh" Subject: RE: [PATCH] net/iavf: unregister intr handler before FD close Thread-Topic: [PATCH] net/iavf: unregister intr handler before FD close Thread-Index: AQHZ3zpEcaXKqjhjFkCmZUp4xjn6lbANQ2pA Date: Wed, 6 Sep 2023 05:24:19 +0000 Message-ID: References: <20230904130123.40084-1-saurabhs@arista.com> In-Reply-To: <20230904130123.40084-1-saurabhs@arista.com> Accept-Language: en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: authentication-results: dkim=none (message not signed) header.d=none;dmarc=none action=none header.from=intel.com; x-ms-publictraffictype: Email x-ms-traffictypediagnostic: DM4PR11MB5994:EE_|BL1PR11MB5286:EE_ x-ms-office365-filtering-correlation-id: 7335f78a-e5d1-4516-098d-08dbae998585 x-ld-processed: 46c98d88-e344-4ed4-8496-4ed7712e255d,ExtAddr x-ms-exchange-senderadcheck: 1 x-ms-exchange-antispam-relay: 0 x-microsoft-antispam: BCL:0; x-microsoft-antispam-message-info: DZI5a4yNZPBSx0NOh/HGv3g39wmGYQC4xRIzHY5DZOJlGoyFZ36tb2/Q3vqKmTKrfuFD6jcs17FuvpQXQZwht3g+BypczA1dsnIpIgAxzrcKAeZiyzC/H7B6dZy1bmzKlllpSvhsIuZkoR/9d1oj947egR+tXLY1RWIGSgHNwjhBt5tVU+WDF65JWXvFZvLGHElOer/knP2e1OL/6myT/HmNURq1KYfHzGvQtYsvFBoEM1/KP6KEpd9pAji+wKEh14cnsS/IpRC519mlMYlXVtX0dpKQSw0r/yqC2LuqzWr9nJsTYXw1lp2mOLR91qWK9y2mCY35mSVYl5PnOE/UjsuoSnGtK3i8pFZmMAvynAIv+nCl5HP+MuTPRStikYQNuhkbSVlttHFFG2nMrYhpvypIRQwo/HezE9a+MRR7sAh1HS+JHIbufmSVQ6qudAgJZ67dBHdmJo+b/+1Khosw4BVf7osVUQmrDVSV0Jo1hyHzoxXcqeG9lkKwEXs0UBjAPLDHeoSYa3HHVYRgzcsC1k68ZuXknBQrDFbZn2EXzSt7dUuawPi7lHth8s7o/9JGfb20izBJiaBuwBoxiuK/17qX0P5xMH0K8mND7DVLsruWvSnW4QbzYKcZ/0tb6v03 x-forefront-antispam-report: CIP:255.255.255.255; CTRY:; LANG:en; SCL:1; SRV:; IPV:NLI; SFV:NSPM; H:DM4PR11MB5994.namprd11.prod.outlook.com; PTR:; CAT:NONE; SFS:(13230031)(376002)(136003)(346002)(366004)(396003)(39860400002)(451199024)(186009)(1800799009)(71200400001)(53546011)(9686003)(6506007)(7696005)(122000001)(82960400001)(33656002)(55016003)(38070700005)(86362001)(38100700002)(2906002)(26005)(83380400001)(478600001)(52536014)(110136005)(316002)(66946007)(8676002)(8936002)(5660300002)(76116006)(4326008)(6636002)(41300700001)(66446008)(64756008)(66556008)(54906003)(66476007); DIR:OUT; SFP:1102; x-ms-exchange-antispam-messagedata-chunkcount: 1 x-ms-exchange-antispam-messagedata-0: =?us-ascii?Q?UQNX2aq4N2oaq4orJv8GqZzPhalM6E3e/VJeHepo46PZY5oVh/73si90mS9i?= =?us-ascii?Q?MXnazzj/caxhK+JuE4DaqDSS3UrJEPFniae/hDu/0nyJrn0r7yboQcA3EL5j?= =?us-ascii?Q?87Rq8g+bLXxrJ5UjCYmRxOTLaAPo+0L2llasW4n49f2BDOqfgGTzSdWaB75w?= =?us-ascii?Q?gpG3j6la7dk3syzypxdfWgVL5i+eaAySm9to6GXHX+J33wQUNFj3pVqm0z2x?= =?us-ascii?Q?m2NGUkDiAt8KCPJtewqFbostk7t1eiSAyqEgcKDE9zRW2Wtk/OxUFb1CWGFj?= =?us-ascii?Q?kH8cecCoxWt050ynqRgDYhC4Rn8MtDCTO42wMCKQ2fD92T3V9jnH+jy5J+ex?= =?us-ascii?Q?6QkNNutlqflrDBCvKVSB6lEc+RshlrfipjgDlA0CBAuqW2CrlYQc4GG7D84o?= =?us-ascii?Q?lPiv1sWgTusASFk0IZ5lWUU6/3VSyi3sH9yp46gVNNEbIFBa7wpfYgnI5KDX?= =?us-ascii?Q?uMbvCPh5LXlddNfoxfWFYtjDIO2StRJTyPPTlFMS++ezNjvYY0Uyl1TKbuZ1?= =?us-ascii?Q?u8Muvori6Pnxo2MDTICyx/pDKa1B8d1SZ3gRhSBYWS4QdoCdYUHOoyYTpu5P?= =?us-ascii?Q?KL8an+r8INwUD1YXrVVoL6oVHjINHE3QaMKXaLsqo+/azfGYtn2n6AzNAH7W?= =?us-ascii?Q?px3PU516CrRT2QMi0tJf6WqekiQXPoWV40Y/cjd+iOprWEfDx5e4hM8JvCvT?= =?us-ascii?Q?tXGcXlrn3PO8G0xQpxYbObgJKbCcrWxTti6C2q11WI0y37e90Kueh+fDlZAq?= =?us-ascii?Q?qVQq90gy5U3JZSN4awCUexowqzwbKBkb+wd/JmQ0b+tXTP5CWSljTDrbJv6y?= =?us-ascii?Q?uXIq5EKFxRv6VgFD+055aqVcmmNRPyMJacVyLQYPAvbTC4Q06f8fZRysYUV0?= =?us-ascii?Q?ko4gPzTmN43QcJDYf6u04ftkDWRoxJ6jw8HR8K8uTFDS4+SgqhuCHhnJAA2W?= =?us-ascii?Q?Ssq02sqpx12ZLao2xrc1mVdLIjzbOVpdNBgtkTWMfo1sFceqBjOqtnbL8vl+?= =?us-ascii?Q?LUIoRIsXIqHAyMO/7Xa+Z3FCnMLx+KvqkYsM6OddL6+boaLw/c9YG4X+U44R?= =?us-ascii?Q?44ahrcfR8quYKm7iXfJZsm1hc9uudJZy6P0WXC3evdeSpbg9wwuM50yB6JQu?= =?us-ascii?Q?DcTAV/WAA05gyKeS0TV0Cow7qr98dnS6/guVwmcgiEcYAvNtidTxy3aq2DYu?= =?us-ascii?Q?/fqeNVaXOpezGeRjvnJzaWI7DiZiq4H0wmHZQ41gze7hsukseFITEZOT23BZ?= =?us-ascii?Q?9U4qEqH260UQyhn4c7Jzp1rqL9PHftVFVpnKMPy4+oGv2jk/puHc1Dwwr3Ma?= =?us-ascii?Q?AO5/AbqDwUfTQNHRoOfw2ggHO9mmKwoOvW0HulE8zalo/Wl+vZUC1a2ne3ce?= =?us-ascii?Q?pHUup0k7l9+TpwczbENA3ISOQFHDnIbrsFVGiljvAHNd56o+EFuVrec7YiW3?= =?us-ascii?Q?E8DF+gvvsG0Wgs5Cny47FBgaJF71Ly/zfFtUNct4l43q40yZJv/ZKm9Wn69p?= =?us-ascii?Q?I2ayIGUkciFZvINC4LXUKH3PggfV4V88tvb0Y9RzIKNpYDL+ic4aznZ5GOCY?= =?us-ascii?Q?eKTi2f2DgbDF0v8Y+nyiEDJH0xG2amH9GLWF9p/T?= Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 X-MS-Exchange-CrossTenant-AuthAs: Internal X-MS-Exchange-CrossTenant-AuthSource: DM4PR11MB5994.namprd11.prod.outlook.com X-MS-Exchange-CrossTenant-Network-Message-Id: 7335f78a-e5d1-4516-098d-08dbae998585 X-MS-Exchange-CrossTenant-originalarrivaltime: 06 Sep 2023 05:24:19.6305 (UTC) X-MS-Exchange-CrossTenant-fromentityheader: Hosted X-MS-Exchange-CrossTenant-id: 46c98d88-e344-4ed4-8496-4ed7712e255d X-MS-Exchange-CrossTenant-mailboxtype: HOSTED X-MS-Exchange-CrossTenant-userprincipalname: 1SV9tjBeryc/+Cvb0fGAf0nhFqP9UH4/0VzWoxv2RKdKriax732wm4dFGIeOfQHUH8O/j0eHZNqt/BUAc+acFg== X-MS-Exchange-Transport-CrossTenantHeadersStamped: BL1PR11MB5286 X-OriginatorOrg: intel.com 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 > -----Original Message----- > From: Saurabh Singhal > Sent: Monday, September 4, 2023 9:01 PM > To: Thomas Monjalon ; Wu, Jingjing > ; Xing, Beilei > Cc: dev@dpdk.org; Singhal, Saurabh > Subject: [PATCH] net/iavf: unregister intr handler before FD close >=20 > Unregister VFIO interrupt handler before the interrupt fd gets closed in = case > iavf_dev_init() returns an error. >=20 > dpdk creates a standalone thread named eal-intr-thread for processing > interrupts for the PCI devices. The interrupt handler callbacks are regis= tered > by the VF driver(iavf, in this case). >=20 > When we do a PCI probe of the network interfaces, we register an interrup= t > 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-int= r-thread > keeps iterating over for handling the interrupts. In our internal testing= , we see > eal-intr-thread crash in these two ways: >=20 > Error adding fd 660 epoll_ctl, Operation not permitted >=20 > or >=20 > Error adding fd 660 epoll_ctl, Bad file descriptor >=20 > 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 c= losed. >=20 > When the first type of crash happens, we see that the fd 660 is > anon_inode:[vfio-device] which does not support poll. >=20 > 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. >=20 > 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. >=20 > 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. >=20 > The fix is to unregister the interrupt handle in the > iavf_dev_init() error path. Thanks for all these explanations! >=20 > Signed-off-by: Saurabh Singhal > --- > .mailmap | 1 + > drivers/net/iavf/iavf_ethdev.c | 15 +++++++++++++++ > 2 files changed, 16 insertions(+) >=20 > 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_ethde= v.c > index f2fc5a5621..df87a553db 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 inline 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, @@ -2490,9 > +2492,22 @@ iavf_uninit_vf(struct rte_eth_dev *dev) { > struct iavf_hw *hw =3D IAVF_DEV_PRIVATE_TO_HW(dev->data- > >dev_private); > struct iavf_info *vf =3D IAVF_DEV_PRIVATE_TO_VF(dev->data- > >dev_private); > + struct rte_pci_device *pci_dev =3D RTE_ETH_DEV_TO_PCI(dev); > + struct rte_intr_handle *intr_handle =3D pci_dev->intr_handle; >=20 > iavf_shutdown_adminq(hw); >=20 > + if (vf->vf_res->vf_cap_flags & VIRTCHNL_VF_OFFLOAD_WB_ON_ITR) { > + /* disable uio intr before callback unregiser */ > + rte_intr_disable(intr_handle); > + > + /* unregister callback func from eal lib */ > + rte_intr_callback_unregister(intr_handle, > + iavf_dev_interrupt_handler, dev); > + } else { > + rte_eal_alarm_cancel(iavf_dev_alarm_handler, dev); > + } > + I assume the error handling should follow the reversed order. So, can we move above code right after the goto label "flow_init_err", like= below? .... iavf_enable_irq0(hw); ret =3D iavf_flow_init(adapter); if (ret) { PMD_INIT_LOG(ERR, "Failed to initialize flow"); goto flow_init_err; } ... flow_init_err: iavf_disable_irq0(hw) if (vf->vf_res->vf_cap_flags & VIRTCHNL_VF_OFFLOAD_WB_ON_ITR) { ... } Btw, I saw missing error handling in iavf_ipsec_crypto_supported branch whi= ch should be fixed, if you are hesitating to apply above fix because this i= nconsistent, please ignore this. Thanks Qi > rte_free(vf->vf_res); > vf->vsi_res =3D NULL; > vf->vf_res =3D NULL; > -- > 2.41.0