From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga01.intel.com (mga01.intel.com [192.55.52.88]) by dpdk.org (Postfix) with ESMTP id 792119617 for ; Wed, 8 Jun 2016 09:24:39 +0200 (CEST) Received: from fmsmga002.fm.intel.com ([10.253.24.26]) by fmsmga101.fm.intel.com with ESMTP; 08 Jun 2016 00:24:38 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.26,438,1459839600"; d="scan'208";a="997588405" Received: from fmsmsx103.amr.corp.intel.com ([10.18.124.201]) by fmsmga002.fm.intel.com with ESMTP; 08 Jun 2016 00:24:26 -0700 Received: from fmsmsx114.amr.corp.intel.com (10.18.116.8) by FMSMSX103.amr.corp.intel.com (10.18.124.201) with Microsoft SMTP Server (TLS) id 14.3.248.2; Wed, 8 Jun 2016 00:24:25 -0700 Received: from shsmsx103.ccr.corp.intel.com (10.239.4.69) by FMSMSX114.amr.corp.intel.com (10.18.116.8) with Microsoft SMTP Server (TLS) id 14.3.248.2; Wed, 8 Jun 2016 00:24:25 -0700 Received: from shsmsx102.ccr.corp.intel.com ([169.254.2.147]) by SHSMSX103.ccr.corp.intel.com ([169.254.4.181]) with mapi id 14.03.0248.002; Wed, 8 Jun 2016 15:24:23 +0800 From: "Lu, Wenzhuo" To: "Ananyev, Konstantin" , "Tao, Zhe" , "dev@dpdk.org" CC: "Richardson, Bruce" , "Chen, Jing D" , "Liang, Cunming" , "Wu, Jingjing" , "Zhang, Helin" Thread-Topic: [PATCH v4 4/8] ixgbe: implement device reset on VF Thread-Index: AQHRwIlSvCHYW1W/D0WZNVy55cLE2J/dQGEAgAGIBWA= Date: Wed, 8 Jun 2016 07:24:22 +0000 Message-ID: <6A0DE07E22DDAD4C9103DF62FEBC090903483A3D@shsmsx102.ccr.corp.intel.com> References: <1465278858-5131-1-git-send-email-zhe.tao@intel.com> <1465282390-6025-1-git-send-email-zhe.tao@intel.com> <1465282390-6025-5-git-send-email-zhe.tao@intel.com> <2601191342CEEE43887BDE71AB97725836B6C46D@irsmsx105.ger.corp.intel.com> In-Reply-To: <2601191342CEEE43887BDE71AB97725836B6C46D@irsmsx105.ger.corp.intel.com> Accept-Language: en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-originating-ip: [10.239.127.40] Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 Subject: Re: [dpdk-dev] [PATCH v4 4/8] ixgbe: implement device reset on VF X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: patches and discussions about DPDK List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 08 Jun 2016 07:24:40 -0000 Hi Konstantin, > -----Original Message----- > From: Ananyev, Konstantin > Sent: Tuesday, June 7, 2016 6:03 PM > To: Tao, Zhe; dev@dpdk.org > Cc: Lu, Wenzhuo; Richardson, Bruce; Chen, Jing D; Liang, Cunming; Wu, Jin= gjing; > Zhang, Helin > Subject: RE: [PATCH v4 4/8] ixgbe: implement device reset on VF >=20 >=20 >=20 > > -----Original Message----- > > From: Tao, Zhe > > Sent: Tuesday, June 07, 2016 7:53 AM > > To: dev@dpdk.org > > Cc: Lu, Wenzhuo; Tao, Zhe; Ananyev, Konstantin; Richardson, Bruce; > > Chen, Jing D; Liang, Cunming; Wu, Jingjing; Zhang, Helin > > Subject: [PATCH v4 4/8] ixgbe: implement device reset on VF > > > > Implement the device reset function. > > 1, Add the fake RX/TX functions. > > 2, The reset function tries to stop RX/TX by replacing > > the RX/TX functions with the fake ones and getting the > > locks to make sure the regular RX/TX finished. > > 3, After the RX/TX stopped, reset the VF port, and then > > release the locks and restore the RX/TX functions. > > > > Signed-off-by: Wenzhuo Lu > > > > static int > > +ixgbevf_dev_reset(struct rte_eth_dev *dev) { > > + struct ixgbe_hw *hw =3D IXGBE_DEV_PRIVATE_TO_HW(dev->data- > >dev_private); > > + struct ixgbe_adapter *adapter =3D > > + (struct ixgbe_adapter *)dev->data->dev_private; > > + int diag =3D 0; > > + uint32_t vteiam; > > + uint16_t i; > > + struct ixgbe_rx_queue *rxq; > > + struct ixgbe_tx_queue *txq; > > + > > + /* Nothing needs to be done if the device is not started. */ > > + if (!dev->data->dev_started) > > + return 0; > > + > > + PMD_DRV_LOG(DEBUG, "Link up/down event detected."); > > + > > + /** > > + * Stop RX/TX by fake functions and locks. > > + * Fake functions are used to make RX/TX lock easier. > > + */ > > + adapter->rx_backup =3D dev->rx_pkt_burst; > > + adapter->tx_backup =3D dev->tx_pkt_burst; > > + dev->rx_pkt_burst =3D ixgbevf_recv_pkts_fake; > > + dev->tx_pkt_burst =3D ixgbevf_xmit_pkts_fake; >=20 > If you have locking over each queue underneath, why do you still need fak= e > functions? The fake functions are used to help saving the time of waiting for the lock= s. As you see, we want to lock every queue. If we don't use fake functions we = have to wait for every queue. But if the real functions are replaced by fake functions, ideally when we'r= e waiting for the release of the first queue's lock, the other queues will run into the fake functions. So we need not wait for = them and get the locks directly. >=20 > > + > > + if (dev->data->rx_queues) > > + for (i =3D 0; i < dev->data->nb_rx_queues; i++) { > > + rxq =3D dev->data->rx_queues[i]; > > + rte_spinlock_lock(&rxq->rx_lock); > > + } > > + > > + if (dev->data->tx_queues) > > + for (i =3D 0; i < dev->data->nb_tx_queues; i++) { > > + txq =3D dev->data->tx_queues[i]; > > + rte_spinlock_lock(&txq->tx_lock); > > + } >=20 > Probably worth to create a separate function for the lines above: > lock_all_queues(), unlock_all_queues. > But as I sadi in previous mail - I think that code better be in rte_ethde= v. We're discussing it in the previous thread :) > > > > @@ -5235,11 +5243,21 @@ ixgbevf_dev_rxtx_start(struct rte_eth_dev *dev) > > rxdctl =3D IXGBE_READ_REG(hw, IXGBE_VFRXDCTL(i)); > > } while (--poll_ms && !(rxdctl & IXGBE_RXDCTL_ENABLE)); > > if (!poll_ms) > > +#ifndef RTE_NEXT_ABI > > + PMD_INIT_LOG(ERR, "Could not enable Rx Queue %d", > i); #else > > + { > > PMD_INIT_LOG(ERR, "Could not enable Rx Queue %d", > i); > > + if (dev->data->dev_conf.rxmode.lock_mode) > > + return -1; > > + } > > +#endif >=20 >=20 > Why the code has to be different here? As you see this rxtx_start may have chance to fail. I want to expose this f= ailure, so the reset function can try again. > Thanks > Konstantin