[dpdk-dev] [PATCH v4 4/8] ixgbe: implement device reset on VF
Hi Konstantin,
> -Original Message-
> From: Ananyev, Konstantin
> Sent: Wednesday, June 8, 2016 4:42 PM
> To: Lu, Wenzhuo; Tao, Zhe; dev at 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 at 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 at 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 at 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 = 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(>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(>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
[dpdk-dev] [PATCH v4 4/8] ixgbe: implement device reset on VF
> -Original Message-
> From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of Ananyev, Konstantin
> Sent: Wednesday, June 08, 2016 9:42 AM
> To: Lu, Wenzhuo; Tao, Zhe; dev at dpdk.org
> Cc: Richardson, Bruce; Chen, Jing D; Liang, Cunming; Wu, Jingjing; Zhang,
> Helin
> Subject: Re: [dpdk-dev] [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 at 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 at 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 at 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 = 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
>
> >
> > >
> &g
[dpdk-dev] [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 at 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 at 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 at 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 = 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
>
> >
> > > +
> > > + if (dev->data->rx_queues)
> > > + for (i = 0; i < dev->data->nb_rx_queues; i++) {
> > > + rxq = dev->data->rx_queues[i];
> > > + rte_spinlock_lock(>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(>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
[dpdk-dev] [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 at 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 at 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 = 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(>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(>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
[dpdk-dev] [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
---
doc/guides/rel_notes/release_16_07.rst | 9 +++
drivers/net/ixgbe/ixgbe_ethdev.c | 108 -
drivers/net/ixgbe/ixgbe_ethdev.h | 12 +++-
drivers/net/ixgbe/ixgbe_rxtx.c | 42 -
4 files changed, 168 insertions(+), 3 deletions(-)
diff --git a/doc/guides/rel_notes/release_16_07.rst
b/doc/guides/rel_notes/release_16_07.rst
index a761e3c..d36c4b1 100644
--- a/doc/guides/rel_notes/release_16_07.rst
+++ b/doc/guides/rel_notes/release_16_07.rst
@@ -53,6 +53,15 @@ New Features
VF. To handle this link up/down event, add the mailbox interruption
support to receive the message.
+* **Added device reset support for ixgbe VF.**
+
+ Added the device reset API. APP can call this API to reset the VF port
+ when it's not working.
+ Based on the mailbox interruption support, when VF reseives the control
+ message from PF, it means the PF link state changes, VF uses the reset
+ callback in the message handler to notice the APP. APP need call the device
+ reset API to reset the VF port.
+
Resolved Issues
---
diff --git a/drivers/net/ixgbe/ixgbe_ethdev.c b/drivers/net/ixgbe/ixgbe_ethdev.c
index fd2682f..1e3520b 100644
--- a/drivers/net/ixgbe/ixgbe_ethdev.c
+++ b/drivers/net/ixgbe/ixgbe_ethdev.c
@@ -381,6 +381,8 @@ static int ixgbe_dev_udp_tunnel_port_add(struct rte_eth_dev
*dev,
static int ixgbe_dev_udp_tunnel_port_del(struct rte_eth_dev *dev,
struct rte_eth_udp_tunnel *udp_tunnel);
+static int ixgbevf_dev_reset(struct rte_eth_dev *dev);
+
/*
* Define VF Stats MACRO for Non "cleared on read" register
*/
@@ -586,6 +588,7 @@ static const struct eth_dev_ops ixgbevf_eth_dev_ops = {
.reta_query = ixgbe_dev_rss_reta_query,
.rss_hash_update = ixgbe_dev_rss_hash_update,
.rss_hash_conf_get= ixgbe_dev_rss_hash_conf_get,
+ .dev_reset= ixgbevf_dev_reset,
};
/* store statistics names and its offset in stats structure */
@@ -4060,7 +4063,8 @@ ixgbevf_dev_start(struct rte_eth_dev *dev)
ETH_VLAN_EXTEND_MASK;
ixgbevf_vlan_offload_set(dev, mask);
- ixgbevf_dev_rxtx_start(dev);
+ if (ixgbevf_dev_rxtx_start(dev))
+ return -1;
/* check and configure queue intr-vector mapping */
if (dev->data->dev_conf.intr_conf.rxq != 0) {
@@ -7193,6 +7197,108 @@ static void ixgbevf_mbx_process(struct rte_eth_dev *dev)
}
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 (dev->data->rx_queues)
+ for (i = 0; i < dev->data->nb_rx_queues; i++) {
+ rxq = dev->data->rx_queues[i];
+ rte_spinlock_lock(>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(>tx_lock);
+ }
+
+ /* Performance VF reset. */
+ do {
+ dev->data->dev_started = 0;
+ ixgbevf_dev_stop(dev);
+ if (dev->data->dev_conf.intr_conf.lsc == 0)
+ diag = ixgbe_dev_link_update(dev, 0);
+ if (diag) {
+ PMD_INIT_LOG(INFO, "Ixgbe VF reset: "
+"Failed to update link.");
+ }
+ rte_delay_ms(1000);
+
+ diag = ixgbevf_dev_start(dev);
+ /*If fail to start the device, need to stop/start it again. */
+ if (diag) {
+ PMD_INIT_LOG(ERR, "Ixgbe VF reset: "
+"Failed to start device.");
+
[dpdk-dev] [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 at 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
> ---
> doc/guides/rel_notes/release_16_07.rst | 9 +++
> drivers/net/ixgbe/ixgbe_ethdev.c | 108
> -
> drivers/net/ixgbe/ixgbe_ethdev.h | 12 +++-
> drivers/net/ixgbe/ixgbe_rxtx.c | 42 -
> 4 files changed, 168 insertions(+), 3 deletions(-)
>
> diff --git a/doc/guides/rel_notes/release_16_07.rst
> b/doc/guides/rel_notes/release_16_07.rst
> index a761e3c..d36c4b1 100644
> --- a/doc/guides/rel_notes/release_16_07.rst
> +++ b/doc/guides/rel_notes/release_16_07.rst
> @@ -53,6 +53,15 @@ New Features
>VF. To handle this link up/down event, add the mailbox interruption
>support to receive the message.
>
> +* **Added device reset support for ixgbe VF.**
> +
> + Added the device reset API. APP can call this API to reset the VF port
> + when it's not working.
> + Based on the mailbox interruption support, when VF reseives the control
> + message from PF, it means the PF link state changes, VF uses the reset
> + callback in the message handler to notice the APP. APP need call the device
> + reset API to reset the VF port.
> +
>
> Resolved Issues
> ---
> diff --git a/drivers/net/ixgbe/ixgbe_ethdev.c
> b/drivers/net/ixgbe/ixgbe_ethdev.c
> index fd2682f..1e3520b 100644
> --- a/drivers/net/ixgbe/ixgbe_ethdev.c
> +++ b/drivers/net/ixgbe/ixgbe_ethdev.c
> @@ -381,6 +381,8 @@ static int ixgbe_dev_udp_tunnel_port_add(struct
> rte_eth_dev *dev,
> static int ixgbe_dev_udp_tunnel_port_del(struct rte_eth_dev *dev,
>struct rte_eth_udp_tunnel *udp_tunnel);
>
> +static int ixgbevf_dev_reset(struct rte_eth_dev *dev);
> +
> /*
> * Define VF Stats MACRO for Non "cleared on read" register
> */
> @@ -586,6 +588,7 @@ static const struct eth_dev_ops ixgbevf_eth_dev_ops = {
> .reta_query = ixgbe_dev_rss_reta_query,
> .rss_hash_update = ixgbe_dev_rss_hash_update,
> .rss_hash_conf_get= ixgbe_dev_rss_hash_conf_get,
> + .dev_reset= ixgbevf_dev_reset,
> };
>
> /* store statistics names and its offset in stats structure */
> @@ -4060,7 +4063,8 @@ ixgbevf_dev_start(struct rte_eth_dev *dev)
> ETH_VLAN_EXTEND_MASK;
> ixgbevf_vlan_offload_set(dev, mask);
>
> - ixgbevf_dev_rxtx_start(dev);
> + if (ixgbevf_dev_rxtx_start(dev))
> + return -1;
>
> /* check and configure queue intr-vector mapping */
> if (dev->data->dev_conf.intr_conf.rxq != 0) {
> @@ -7193,6 +7197,108 @@ static void ixgbevf_mbx_process(struct rte_eth_dev
> *dev)
> }
>
> 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?
> +
> + if (dev->data->rx_queues)
> + for (i = 0; i < dev->data->nb_rx_queues; i++) {
> + rxq = dev->data->rx_queues[i];
> + rte_spinlock_lock(>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(>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.
> +
> + /* Performance VF reset. */
> + do {
> +
