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: Sun, 12 Jun 2016 00:58:05 +0000 [thread overview]
Message-ID: <6A0DE07E22DDAD4C9103DF62FEBC090903484910@shsmsx102.ccr.corp.intel.com> (raw)
In-Reply-To: <2601191342CEEE43887BDE71AB97725836B6CC99@irsmsx105.ger.corp.intel.com>
Hi Konstantin,
> -----Original Message-----
> From: Ananyev, Konstantin
> Sent: Wednesday, June 8, 2016 4:42 PM
> To: Lu, Wenzhuo; Tao, Zhe; dev@dpdk.org
> Cc: 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: Lu, Wenzhuo
> > Sent: Wednesday, June 08, 2016 8:24 AM
> > To: Ananyev, Konstantin; Tao, Zhe; dev@dpdk.org
> > Cc: Richardson, Bruce; Chen, Jing D; Liang, Cunming; Wu, Jingjing;
> > Zhang, Helin
> > Subject: RE: [PATCH v4 4/8] ixgbe: implement device reset on VF
> >
> > 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.
>
> Well, data-path invokes only try_lock(), so it shouldn't be affected significantly,
> right?
> Control path still have to spin on lock and grab it before it can proceed, if it'll
> spin a bit longer I wouldn't see a big deal here.
> What I am trying to say - if we'll go that way - introduce sync control/datapath
> API anyway, we don't need any additional tricks here with rx/tx function
> replacement, correct?
> So let's keep it clean and simple, after all it is a control path and not need to be
> lightning fast.
> Konstantin
Agree, it's not necessary to add the fake functions. I'll remove them to make it simple.
>
> >
> > >
> > > > +
> > > > + 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-12 0:58 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
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 [this message]
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=6A0DE07E22DDAD4C9103DF62FEBC090903484910@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).