From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from dpdk.org (dpdk.org [92.243.14.124]) by inbox.dpdk.org (Postfix) with ESMTP id 1B995A00C3; Thu, 14 May 2020 10:40:12 +0200 (CEST) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id A92C11D6BE; Thu, 14 May 2020 10:40:11 +0200 (CEST) Received: from mga11.intel.com (mga11.intel.com [192.55.52.93]) by dpdk.org (Postfix) with ESMTP id 3B4571D6A7; Thu, 14 May 2020 10:40:10 +0200 (CEST) IronPort-SDR: LHF+ijfxvSc8vxwX/43JdbeCwpd413kd5lA7MWNHy0NpNjrSOIFcX4ljO4gT4iHi3i9gNj1jWe lgyPgotfHf2g== X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from orsmga003.jf.intel.com ([10.7.209.27]) by fmsmga102.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 14 May 2020 01:40:09 -0700 IronPort-SDR: sa6j6ez7pKoYk8Tr7iMkv23u/vIgW5F+1/MBNCR1bFFfpWAaFNIX7sw0100dZVf18dMCgcuMto C1ffJe78HNmQ== X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.73,390,1583222400"; d="scan'208";a="262768316" Received: from orsmsx108.amr.corp.intel.com ([10.22.240.6]) by orsmga003.jf.intel.com with ESMTP; 14 May 2020 01:40:09 -0700 Received: from orsmsx608.amr.corp.intel.com (10.22.229.21) by ORSMSX108.amr.corp.intel.com (10.22.240.6) with Microsoft SMTP Server (TLS) id 14.3.439.0; Thu, 14 May 2020 01:40:08 -0700 Received: from orsmsx608.amr.corp.intel.com (10.22.229.21) by ORSMSX608.amr.corp.intel.com (10.22.229.21) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.1713.5; Thu, 14 May 2020 01:40:08 -0700 Received: from ORSEDG002.ED.cps.intel.com (10.7.248.5) by orsmsx608.amr.corp.intel.com (10.22.229.21) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA256) id 15.1.1713.5 via Frontend Transport; Thu, 14 May 2020 01:40:08 -0700 Received: from NAM12-BN8-obe.outbound.protection.outlook.com (104.47.55.168) by edgegateway.intel.com (134.134.137.101) with Microsoft SMTP Server (TLS) id 14.3.439.0; Thu, 14 May 2020 01:40:08 -0700 ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=RunhuAndxR5fYSZgi1gFfaueOgrMIfjy33TvMeodtv8611vogr7CBal3hJJ91Fobf2mtyELtIdu/XjAhf2TfRvBvWAq1X5NPLXd6CDJwky7S/mR+4u8e3JEZTeaBtIMGlAumJuuSwN+WQs0JiHjO29HQRaL1Vu+tewlkTUt4WtJnjlxpEtuPt5/3Dmw6rKwTDCHXeLTrbus1mOdM6ydbYxbl3f6vFQ3d5S8FsKVsS8lfvWCsq/0UDAIT+geJlS5Iax9r0nbn19o6otHtzl1F0YDl79/7MzkqW2itDZkW0x/xCtNcZ36U3eCvImrnPihI1zUTx/Wb1A+25At4aRoxcA== 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-SenderADCheck; bh=TltHaB+3351wBiXdynOQPuCNXfx9vnFdRY/CaaTSJac=; b=ViO9atZocQVZXfMIb8XBFb2P4DZcMK3fHRCcDkZF0PpJpKaaaTNrP3DaRqD2NesTF2YEpQv4Ly+DwjSplp7Bs4S2p+PTPHjr28KOHP8Hf0Vtkzh/GQFGB2hsqJFndIWKPaGvISnykJB1nk0MFQO2MnSqD9fm4i5T3WcjVnXQ/Jg9KZ/efBOi+I5ix7eKRtREQ/OjUJ4SlJYKenc/Plhe5l1c7ipvlqXoAb8sKOLKL42OaPSCOXetKOjrD/zaUtOJI2EutU0ZVSWplfbTer1f1hcKyRQJ27NYzpyZ/L8WDUOQunRIx8zBK0+gzIsZBfvPEqlDAWnDAlRIJr1uKeotFQ== 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 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=intel.onmicrosoft.com; s=selector2-intel-onmicrosoft-com; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=TltHaB+3351wBiXdynOQPuCNXfx9vnFdRY/CaaTSJac=; b=cbzihn5Ye3QgzxW63tYAXnrlOPi3+HgxVCnTbBjApfFWZ7WphTFXAJT1UCEL/PpqCDU5PjeZIzn7ud30Dz0JTdnJjhrYsJhjoQPW1iWsg+B7zk8r5KV33GhRe2e2FJ6g2Izif7nGZjLEreKDoX0Cjxe4JuzjqR6ak1VfnDMgYFc= Received: from MN2PR11MB3582.namprd11.prod.outlook.com (2603:10b6:208:ec::28) by MN2PR11MB4534.namprd11.prod.outlook.com (2603:10b6:208:265::16) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.3000.20; Thu, 14 May 2020 08:40:05 +0000 Received: from MN2PR11MB3582.namprd11.prod.outlook.com ([fe80::e1c5:b5cf:5a94:379f]) by MN2PR11MB3582.namprd11.prod.outlook.com ([fe80::e1c5:b5cf:5a94:379f%3]) with mapi id 15.20.2979.033; Thu, 14 May 2020 08:40:05 +0000 From: "Yang, Qiming" To: "Ye, Xiaolong" CC: "dev@dpdk.org" , "Xing, Beilei" , "stable@dpdk.org" Thread-Topic: [dpdk-stable] [PATCH] net/i40e: fix queue related exception handling Thread-Index: AQHWKP7rWKliPoIAnECGXqJ1Nt56u6inNOMAgAALfxA= Date: Thu, 14 May 2020 08:40:04 +0000 Message-ID: References: <20200513075630.98139-1-qiming.yang@intel.com> <20200514074404.GC102809@intel.com> In-Reply-To: <20200514074404.GC102809@intel.com> Accept-Language: en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: authentication-results: intel.com; dkim=none (message not signed) header.d=none;intel.com; dmarc=none action=none header.from=intel.com; x-originating-ip: [192.102.204.45] x-ms-publictraffictype: Email x-ms-office365-filtering-correlation-id: b80ca5f3-c24e-4a0f-78b5-08d7f7e2669b x-ms-traffictypediagnostic: MN2PR11MB4534: x-ld-processed: 46c98d88-e344-4ed4-8496-4ed7712e255d,ExtAddr x-ms-exchange-transport-forked: True x-microsoft-antispam-prvs: x-ms-oob-tlc-oobclassifiers: OLM:7219; x-forefront-prvs: 040359335D x-ms-exchange-senderadcheck: 1 x-microsoft-antispam: BCL:0; x-microsoft-antispam-message-info: ZO9hVLLEhgjZ02l8uRMabJ43EFhVKYDCsY0t9kDUjp09N3X/1VUrTP/2PY6AjS0RKKjwsobB4xwUZcjSbYksydIpeMu4Hl/7T564TIvXLRklUk6YI/zOace6Tq3aYg+01B5To8KhkSjuas7QKZ3DslnpdYG2a+HuvANuL+aXyRDEahSYbV/QI7UiVY4DNq9MNHzKHxYiT2CfP7lkRay3dOG7VtkNPQT0Jd7dqpbSVOy/Kl+ELZ9XN76aPdAvDKuivUgRFfAaKxH5hPEQG6jMAZ9z2XMMjAT2cUao86GxZG7Rfg6Udmpwkk2mymfHvaeHrXCOsC6HW9+duqHpqCioDLtg2xRYjm87Npdto8C3eRNzbj/JppFl4YYsX9iEC0cR7aUtJScoRbNoermkYTJab/gW5UJ6HbqPfm3cJY02ftx+kEwt+zexBTo0b9qlHFv+ x-forefront-antispam-report: CIP:255.255.255.255; CTRY:; LANG:en; SCL:1; SRV:; IPV:NLI; SFV:NSPM; H:MN2PR11MB3582.namprd11.prod.outlook.com; PTR:; CAT:NONE; SFTY:; SFS:(396003)(136003)(376002)(366004)(346002)(39860400002)(8936002)(6862004)(9686003)(478600001)(450100002)(6636002)(64756008)(66556008)(71200400001)(4326008)(76116006)(52536014)(66476007)(66946007)(66446008)(54906003)(316002)(6506007)(55016002)(8676002)(86362001)(53546011)(5660300002)(7696005)(33656002)(26005)(186003)(2906002); DIR:OUT; SFP:1102; x-ms-exchange-antispam-messagedata: EMAMwMJMrUiDLT4hagVxABEUuJRSqELwOVacNQG15Mek8QamFdBbJJM452hQPjl3yaCTQiLRtAvG2pRMcJLaDeZwtMcGqd2/1cAOnfzrsoTwmKxOL0A/Kn8wx/+KUSjilyY3QFHURs42nkQsYaq3/csUe1RzRb23mBz6l0X6mru76tT/yiOQZwuHv2dWOFO8GTQwSBvU6FtnT6W6M0VpDMajJk6xPmXZiw/d7r33CgKrTt8JwSb+siFpdSZe6txGLkRtDmoUMCaebl7g0RH+HJ+9Od6yrTkRdDYZH3CjbBeZOqjlHxDoea7iJD94hw+Tq4OW8Xu8qy/wxXQCwCOh4LPneii1RRxwIH6vyLRnn3ezPdrQIJI4Ddu/wTI6cOBRfpy/CzfereDJsEjSiGPpCEmF5mZlY20PhY21CaLkvhYWVFjhkbDjfVtAiiFr6plPriRct8stA2BNk4ByCn5EV2ZTr0ssgedEn9o21s8SdC9ETEifMioiFMuXYXWd8rmj Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 X-MS-Exchange-CrossTenant-Network-Message-Id: b80ca5f3-c24e-4a0f-78b5-08d7f7e2669b X-MS-Exchange-CrossTenant-originalarrivaltime: 14 May 2020 08:40:05.0640 (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: pySfFv4GF1UsIizl5DzWOOOyM3N0BCymldV01me/qJOOtFbDyRPRS7WwD06z9xSbhfHb4mVbCC92Eh99XzEI6g== X-MS-Exchange-Transport-CrossTenantHeadersStamped: MN2PR11MB4534 X-OriginatorOrg: intel.com Subject: Re: [dpdk-dev] [dpdk-stable] [PATCH] net/i40e: fix queue related exception handling X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" Hi, Xiaolong > -----Original Message----- > From: Ye, Xiaolong > Sent: Thursday, May 14, 2020 15:44 > To: Yang, Qiming > Cc: dev@dpdk.org; Xing, Beilei ; stable@dpdk.org > Subject: Re: [dpdk-stable] [PATCH] net/i40e: fix queue related exception > handling >=20 > On 05/13, Qiming Yang wrote: > >There should have different behavior in queue start fail and stop fail c= ase. > >When queue start fail, all the next actions should be terminated and > >then started queues should be cleared. But for queue stop stage, one > >queue stop fail should not end other queues stop. This patch fixed that > >issue in PF and VF. > > > >Fixes: b6583ee40265 ("i40e: full VMDQ pools support") > >Fixes: 3f6a696f1054 ("i40evf: queue start and stop") > > > >Signed-off-by: Qiming Yang > >--- > > drivers/net/i40e/i40e_ethdev.c | 116 ++++++++-----------------------= ------- > > drivers/net/i40e/i40e_ethdev_vf.c | 2 - > > drivers/net/i40e/i40e_rxtx.c | 28 +++++++++ > > 3 files changed, 53 insertions(+), 93 deletions(-) > > Snip ... > > return I40E_SUCCESS; > > > >-err_up: > >- i40e_dev_switch_queues(pf, FALSE); > >- i40e_dev_clear_queues(dev); > >+tx_err: > >+ for (i =3D 0; i < nb_txq; i++) > >+ i40e_dev_tx_queue_stop(dev, i); > >+rx_err: > >+ for (i =3D 0; i < nb_rxq; i++) > >+ i40e_dev_rx_queue_stop(dev, i); >=20 > I think we still need to clear queues in the error handling. I delete clear function doesn't means clear queue action be deleted. In i40e_dev_clear_queues, it calls these four functions i40e_tx_queue_release_mbufs(dev->data->tx_queues[i]); i40e_reset_tx_queue(dev->data->tx_queues[i]); i40e_rx_queue_release_mbufs(dev->data->rx_queues[i]); i40e_reset_rx_queue(dev->data->rx_queues[i]); it covered by queue_stop function. >=20 > > > > return ret; > > } > >@@ -2442,7 +2452,11 @@ i40e_dev_stop(struct rte_eth_dev *dev) > > } > > > > /* Disable all queues */ > >- i40e_dev_switch_queues(pf, FALSE); > >+ for (i =3D 0; i < dev->data->nb_tx_queues; i++) > >+ i40e_dev_tx_queue_stop(dev, i); > >+ > >+ for (i =3D 0; i < dev->data->nb_rx_queues; i++) > >+ i40e_dev_rx_queue_stop(dev, i); > > > > /* un-map queues with interrupt registers */ > > i40e_vsi_disable_queues_intr(main_vsi); >=20 > [snip] >=20 > >diff --git a/drivers/net/i40e/i40e_rxtx.c > >b/drivers/net/i40e/i40e_rxtx.c index f6d23c9..d0bada9 100644 > >--- a/drivers/net/i40e/i40e_rxtx.c > >+++ b/drivers/net/i40e/i40e_rxtx.c > >@@ -1570,6 +1570,15 @@ i40e_dev_rx_queue_start(struct rte_eth_dev > *dev, uint16_t rx_queue_id) > > PMD_INIT_FUNC_TRACE(); > > > > rxq =3D dev->data->rx_queues[rx_queue_id]; > >+ if (!rxq || !rxq->q_set) { > >+ PMD_DRV_LOG(ERR, "RX queue %u not available or setup", > >+ rx_queue_id); > >+ return -EINVAL; > >+ } > >+ > >+ if (rxq->rx_deferred_start) > >+ PMD_DRV_LOG(ERR, "RX queue %u is deferrd start", > >+ rx_queue_id); >=20 > Do we need to take any action if rx_deferred_start is set? > Just print an ERR log doesn't make sense to me. If defer set, this queue start will be skipped. But this should not block o= ther queues' start. So I add a log to let user know but no return error. Maybe return WARNING will be more softer, what do you think about? >=20 > > > > err =3D i40e_alloc_rx_queue_mbufs(rxq); > > if (err) { > >@@ -1602,6 +1611,11 @@ i40e_dev_rx_queue_stop(struct rte_eth_dev > *dev, uint16_t rx_queue_id) > > struct i40e_hw *hw =3D I40E_DEV_PRIVATE_TO_HW(dev->data- > >dev_private); > > > > rxq =3D dev->data->rx_queues[rx_queue_id]; > >+ if (!rxq || !rxq->q_set) { > >+ PMD_DRV_LOG(ERR, "RX queue %u not available or setup", > >+ rx_queue_id); > >+ return -EINVAL; > >+ } > > > > /* > > * rx_queue_id is queue id application refers to, while @@ -1630,6 > >+1644,15 @@ i40e_dev_tx_queue_start(struct rte_eth_dev *dev, uint16_t > tx_queue_id) > > PMD_INIT_FUNC_TRACE(); > > > > txq =3D dev->data->tx_queues[tx_queue_id]; > >+ if (!txq || !txq->q_set) { > >+ PMD_DRV_LOG(ERR, "TX queue %u is not available or setup", > >+ tx_queue_id); > >+ return -EINVAL; > >+ } > >+ > >+ if (txq->tx_deferred_start) > >+ PMD_DRV_LOG(ERR, "TX queue %u is deferrd start", > >+ tx_queue_id); >=20 > Ditto. >=20 > Thanks, > Xiaolong >=20 > > > > /* > > * tx_queue_id is queue id application refers to, while @@ -1654,6 > >+1677,11 @@ i40e_dev_tx_queue_stop(struct rte_eth_dev *dev, uint16_t > tx_queue_id) > > struct i40e_hw *hw =3D I40E_DEV_PRIVATE_TO_HW(dev->data- > >dev_private); > > > > txq =3D dev->data->tx_queues[tx_queue_id]; > >+ if (!txq || !txq->q_set) { > >+ PMD_DRV_LOG(ERR, "TX queue %u is not available or setup", > >+ tx_queue_id); > >+ return -EINVAL; > >+ } > > > > /* > > * tx_queue_id is queue id application refers to, while > >-- > >2.9.5 > >