From: "Lu, Wenzhuo" <wenzhuo.lu@intel.com>
To: "Ananyev, Konstantin" <konstantin.ananyev@intel.com>,
"Tao, Zhe" <zhe.tao@intel.com>, "dev@dpdk.org" <dev@dpdk.org>
Cc: "Richardson, Bruce" <bruce.richardson@intel.com>,
"Chen, Jing D" <jing.d.chen@intel.com>,
"Liang, Cunming" <cunming.liang@intel.com>,
"Wu, Jingjing" <jingjing.wu@intel.com>,
"Zhang, Helin" <helin.zhang@intel.com>
Subject: Re: [dpdk-dev] [PATCH v4 4/8] ixgbe: implement device reset on VF
Date: Wed, 8 Jun 2016 07:24:22 +0000 [thread overview]
Message-ID: <6A0DE07E22DDAD4C9103DF62FEBC090903483A3D@shsmsx102.ccr.corp.intel.com> (raw)
In-Reply-To: <2601191342CEEE43887BDE71AB97725836B6C46D@irsmsx105.ger.corp.intel.com>
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, Jingjing;
> Zhang, Helin
> Subject: RE: [PATCH v4 4/8] ixgbe: implement device reset on VF
>
>
>
> > -----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 <wenzhuo.lu@intel.com>
> >
> > static int
> > +ixgbevf_dev_reset(struct rte_eth_dev *dev) {
> > + struct ixgbe_hw *hw = IXGBE_DEV_PRIVATE_TO_HW(dev->data-
> >dev_private);
> > + struct ixgbe_adapter *adapter =
> > + (struct ixgbe_adapter *)dev->data->dev_private;
> > + int diag = 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 = dev->rx_pkt_burst;
> > + adapter->tx_backup = dev->tx_pkt_burst;
> > + dev->rx_pkt_burst = ixgbevf_recv_pkts_fake;
> > + dev->tx_pkt_burst = ixgbevf_xmit_pkts_fake;
>
> If you have locking over each queue underneath, why do you still need fake
> functions?
The fake functions are used to help saving the time of waiting for the locks.
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're 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.
>
> > +
> > + if (dev->data->rx_queues)
> > + for (i = 0; i < dev->data->nb_rx_queues; i++) {
> > + rxq = dev->data->rx_queues[i];
> > + rte_spinlock_lock(&rxq->rx_lock);
> > + }
> > +
> > + if (dev->data->tx_queues)
> > + for (i = 0; i < dev->data->nb_tx_queues; i++) {
> > + txq = dev->data->tx_queues[i];
> > + rte_spinlock_lock(&txq->tx_lock);
> > + }
>
> 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_ethdev.
We're discussing it in the previous thread :)
> >
> > @@ -5235,11 +5243,21 @@ ixgbevf_dev_rxtx_start(struct rte_eth_dev *dev)
> > rxdctl = 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
>
>
> Why the code has to be different here?
As you see this rxtx_start may have chance to fail. I want to expose this failure, so the reset function can try again.
> Thanks
> Konstantin
next prev parent reply other threads:[~2016-06-08 7:24 UTC|newest]
Thread overview: 24+ messages / expand[flat|nested] mbox.gz Atom feed top
[not found] <1465278858-5131-1-git-send-email-zhe.tao@intel.com>
2016-06-07 6:53 ` [dpdk-dev] [PATCH v4 0/8] support reset of VF link Zhe Tao
2016-06-07 6:53 ` [dpdk-dev] [PATCH v4 1/8] lib/librte_ether: support device reset Zhe Tao
2016-06-07 6:53 ` [dpdk-dev] [PATCH v4 2/8] lib/librte_ether: defind RX/TX lock mode Zhe Tao
2016-06-07 9:58 ` Ananyev, Konstantin
2016-06-08 7:24 ` Lu, Wenzhuo
2016-06-08 9:19 ` Ananyev, Konstantin
2016-06-12 2:00 ` Lu, Wenzhuo
2016-06-12 23:16 ` Ananyev, Konstantin
2016-06-13 1:06 ` Lu, Wenzhuo
2016-06-13 10:47 ` Ananyev, Konstantin
2016-06-14 0:42 ` Lu, Wenzhuo
2016-06-14 8:42 ` Ananyev, Konstantin
2016-06-07 6:53 ` [dpdk-dev] [PATCH v4 3/8] ixgbe: RX/TX with lock on VF Zhe Tao
2016-06-07 6:53 ` [dpdk-dev] [PATCH v4 4/8] ixgbe: implement device reset " Zhe Tao
2016-06-07 10:03 ` Ananyev, Konstantin
2016-06-08 7:24 ` Lu, Wenzhuo [this message]
2016-06-08 8:42 ` Ananyev, Konstantin
2016-06-08 9:22 ` Ananyev, Konstantin
2016-06-12 1:03 ` Lu, Wenzhuo
2016-06-12 0:58 ` Lu, Wenzhuo
2016-06-07 6:53 ` [dpdk-dev] [PATCH v4 5/8] igb: RX/TX with lock " Zhe Tao
2016-06-07 6:53 ` [dpdk-dev] [PATCH v4 6/8] igb: implement device reset " Zhe Tao
2016-06-07 6:53 ` [dpdk-dev] [PATCH v4 7/8] i40e: RX/TX with lock " Zhe Tao
2016-06-07 6:53 ` [dpdk-dev] [PATCH v4 8/8] i40e: implement device reset " Zhe Tao
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=6A0DE07E22DDAD4C9103DF62FEBC090903483A3D@shsmsx102.ccr.corp.intel.com \
--to=wenzhuo.lu@intel.com \
--cc=bruce.richardson@intel.com \
--cc=cunming.liang@intel.com \
--cc=dev@dpdk.org \
--cc=helin.zhang@intel.com \
--cc=jing.d.chen@intel.com \
--cc=jingjing.wu@intel.com \
--cc=konstantin.ananyev@intel.com \
--cc=zhe.tao@intel.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).