[dpdk-dev] [PATCH 0/3] RFC: implement VF reset for i40e, e1000 and ixgbe

[dpdk-dev] [PATCH 0/3] RFC: implement VF reset for i40e, e1000 and ixgbe

[luca.boccassi]

From: Luca Boccassi <bluca@debian.org>

These patches were originally sent by Wenzhuo Lu:

http://dpdk.org/dev/patchwork/patch/14009/
http://dpdk.org/dev/patchwork/patch/14010/
http://dpdk.org/dev/patchwork/patch/14011/

The current rte_eth_dev_reset API does not correctly reset the VF
when the PF flaps on the host. With these patches, at least the ixgbe
driver with a X540 card (Linux kernel 4.9 on the host) appears to
work correctly.
The test is as simple ip link down/up on the host, and then check that
traffic still flows through the VF in the guest.

I am not an expert in these PMDs hence the RFC mark - I would like to
ask for feedback and help from the PMD maintainers and developers.

Thanks!

Luca Boccassi (3):
  net/i40e: implement VF reset
  net/e1000: implement VF reset
  net/ixgbe: implement VF reset

 drivers/net/e1000/igb_ethdev.c    | 59 ++++++++++++++++++++++++++++++++++++++
 drivers/net/i40e/i40e_ethdev.h    |  3 ++
 drivers/net/i40e/i40e_ethdev_vf.c | 56 +++++++++++++++++++++++++++++++++---
 drivers/net/i40e/i40e_rxtx.c      | 11 +++++++
 drivers/net/i40e/i40e_rxtx.h      |  4 +++
 drivers/net/ixgbe/ixgbe_ethdev.c  | 60 ++++++++++++++++++++++++++++++++++-----
 drivers/net/ixgbe/ixgbe_ethdev.h  |  2 +-
 drivers/net/ixgbe/ixgbe_rxtx.c    | 12 ++++++--
 8 files changed, 192 insertions(+), 15 deletions(-)

-- 
2.11.0

[dpdk-dev] [PATCH 1/3] net/i40e: implement VF reset

[luca.boccassi]

From: Luca Boccassi <bluca@debian.org>

This reset function will detach then re-attach the device, reconfigure
it, and re-setup the Rx/Tx queues.

Signed-off-by: Luca Boccassi <bluca@debian.org>
---
 drivers/net/i40e/i40e_ethdev.h    |  3 +++
 drivers/net/i40e/i40e_ethdev_vf.c | 56 ++++++++++++++++++++++++++++++++++++---
 drivers/net/i40e/i40e_rxtx.c      | 11 ++++++++
 drivers/net/i40e/i40e_rxtx.h      |  4 +++
 4 files changed, 70 insertions(+), 4 deletions(-)

diff --git a/drivers/net/i40e/i40e_ethdev.h b/drivers/net/i40e/i40e_ethdev.h
index 2f1905e13..45714959a 100644
--- a/drivers/net/i40e/i40e_ethdev.h
+++ b/drivers/net/i40e/i40e_ethdev.h
@@ -1047,6 +1047,9 @@ struct i40e_adapter {
 	struct rte_timecounter rx_tstamp_tc;
 	struct rte_timecounter tx_tstamp_tc;
 
+	/* For VF reset */
+	uint8_t reset_number;
+
 	/* ptype mapping table */
 	uint32_t ptype_tbl[I40E_MAX_PKT_TYPE] __rte_cache_min_aligned;
 	/* flow type to pctype mapping table */
diff --git a/drivers/net/i40e/i40e_ethdev_vf.c b/drivers/net/i40e/i40e_ethdev_vf.c
index 9f1487509..333ddb4ca 100644
--- a/drivers/net/i40e/i40e_ethdev_vf.c
+++ b/drivers/net/i40e/i40e_ethdev_vf.c
@@ -2294,21 +2294,69 @@ i40evf_dev_close(struct rte_eth_dev *dev)
 	i40evf_disable_irq0(hw);
 }
 
-/*
- * Reset VF device only to re-initialize resources in PMD layer
- */
+static void
+i40e_vf_queue_reset(struct rte_eth_dev *dev)
+{
+	uint16_t i;
+
+	for (i = 0; i < dev->data->nb_rx_queues; i++) {
+		 struct i40e_rx_queue *rxq = dev->data->rx_queues[i];
+
+		 if (rxq->q_set) {
+			  i40e_dev_rx_queue_setup(dev,
+						     rxq->queue_id,
+						     rxq->nb_rx_desc,
+						     rxq->socket_id,
+						     &rxq->rxconf,
+						     rxq->mp);
+		 }
+	}
+	for (i = 0; i < dev->data->nb_tx_queues; i++) {
+		 struct i40e_tx_queue *txq = dev->data->tx_queues[i];
+
+		 if (txq->q_set) {
+			  i40e_dev_tx_queue_setup(dev,
+						     txq->queue_id,
+						     txq->nb_tx_desc,
+						     txq->socket_id,
+						     &txq->txconf);
+		 }
+	}
+}
+
 static int
 i40evf_dev_reset(struct rte_eth_dev *dev)
 {
 	int ret;
+	struct i40e_adapter *adapter =
+		 I40E_DEV_PRIVATE_TO_ADAPTER(dev->data->dev_private);
 
+	if (!dev->data->dev_started)
+		 return -EAGAIN;
+
+	adapter->reset_number = 1;
+	i40evf_dev_close(dev);
+	PMD_DRV_LOG(DEBUG, "i40evf dev close complete");
 	ret = i40evf_dev_uninit(dev);
 	if (ret)
 		return ret;
+	PMD_DRV_LOG(DEBUG, "i40evf dev detached");
+	memset(dev->data->dev_private, 0,
+		(uint64_t)&adapter->reset_number - (uint64_t)adapter);
 
+	i40evf_dev_configure(dev);
 	ret = i40evf_dev_init(dev);
+	if (ret)
+		return ret;
+	PMD_DRV_LOG(DEBUG, "i40evf dev attached");
 
-	return ret;
+	i40e_vf_queue_reset(dev);
+	PMD_DRV_LOG(DEBUG, "i40evf queue reset");
+	i40evf_dev_start(dev);
+	PMD_DRV_LOG(DEBUG, "i40evf dev restart");
+	adapter->reset_number = 0;
+
+	return 0;
 }
 
 static int
diff --git a/drivers/net/i40e/i40e_rxtx.c b/drivers/net/i40e/i40e_rxtx.c
index f21c1c5d2..3d5125260 100644
--- a/drivers/net/i40e/i40e_rxtx.c
+++ b/drivers/net/i40e/i40e_rxtx.c
@@ -1745,6 +1745,7 @@ i40e_dev_rx_queue_setup(struct rte_eth_dev *dev,
 	uint32_t ring_size;
 	uint16_t len, i;
 	uint16_t reg_idx, base, bsf, tc_mapping;
+	struct rte_eth_rxconf conf = *rx_conf;
 	int q_offset, use_def_burst_func = 1;
 
 	if (hw->mac.type == I40E_MAC_VF || hw->mac.type == I40E_MAC_X722_VF) {
@@ -1789,6 +1790,8 @@ i40e_dev_rx_queue_setup(struct rte_eth_dev *dev,
 		return -ENOMEM;
 	}
 	rxq->mp = mp;
+	rxq->socket_id = socket_id;
+	rxq->rxconf = conf;
 	rxq->nb_rx_desc = nb_desc;
 	rxq->rx_free_thresh = rx_conf->rx_free_thresh;
 	rxq->queue_id = queue_idx;
@@ -2024,6 +2027,8 @@ i40e_dev_tx_queue_setup(struct rte_eth_dev *dev,
 	uint16_t reg_idx, i, base, bsf, tc_mapping;
 	int q_offset;
 
+	struct rte_eth_txconf conf = *tx_conf;
+
 	if (hw->mac.type == I40E_MAC_VF || hw->mac.type == I40E_MAC_X722_VF) {
 		vf = I40EVF_DEV_PRIVATE_TO_VF(dev->data->dev_private);
 		vsi = &vf->vsi;
@@ -2149,6 +2154,8 @@ i40e_dev_tx_queue_setup(struct rte_eth_dev *dev,
 	}
 
 	txq->nb_tx_desc = nb_desc;
+	txq->socket_id = socket_id;
+	txq->txconf = conf;
 	txq->tx_rs_thresh = tx_rs_thresh;
 	txq->tx_free_thresh = tx_free_thresh;
 	txq->pthresh = tx_conf->tx_thresh.pthresh;
@@ -2628,8 +2635,12 @@ void
 i40e_dev_free_queues(struct rte_eth_dev *dev)
 {
 	uint16_t i;
+	struct i40e_adapter *adapter =
+		I40E_DEV_PRIVATE_TO_ADAPTER(dev->data->dev_private);
 
 	PMD_INIT_FUNC_TRACE();
+	if (adapter->reset_number)
+		return;
 
 	for (i = 0; i < dev->data->nb_rx_queues; i++) {
 		if (!dev->data->rx_queues[i])
diff --git a/drivers/net/i40e/i40e_rxtx.h b/drivers/net/i40e/i40e_rxtx.h
index 06c6a6592..43e227a52 100644
--- a/drivers/net/i40e/i40e_rxtx.h
+++ b/drivers/net/i40e/i40e_rxtx.h
@@ -136,6 +136,8 @@ struct i40e_rx_queue {
 	bool rx_deferred_start; /**< don't start this queue in dev start */
 	uint16_t rx_using_sse; /**<flag indicate the usage of vPMD for rx */
 	uint8_t dcb_tc;         /**< Traffic class of rx queue */
+	uint8_t socket_id;
+	struct rte_eth_rxconf rxconf;
 };
 
 struct i40e_tx_entry {
@@ -177,6 +179,8 @@ struct i40e_tx_queue {
 	bool q_set; /**< indicate if tx queue has been configured */
 	bool tx_deferred_start; /**< don't start this queue in dev start */
 	uint8_t dcb_tc;         /**< Traffic class of tx queue */
+	uint8_t socket_id;
+	struct rte_eth_txconf txconf;
 };
 
 /** Offload features */
-- 
2.11.0

[dpdk-dev] [PATCH 2/3] net/e1000: implement VF reset

[luca.boccassi]

From: Luca Boccassi <bluca@debian.org>

This reset function will stop and restart the device, and then reset
the stats and check that the registers have been updated.

Signed-off-by: Luca Boccassi <bluca@debian.org>
---
 drivers/net/e1000/igb_ethdev.c | 59 ++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 59 insertions(+)

diff --git a/drivers/net/e1000/igb_ethdev.c b/drivers/net/e1000/igb_ethdev.c
index 003bdf0f6..1211d5371 100644
--- a/drivers/net/e1000/igb_ethdev.c
+++ b/drivers/net/e1000/igb_ethdev.c
@@ -280,6 +280,7 @@ static void eth_igb_configure_msix_intr(struct rte_eth_dev *dev);
 static void eth_igbvf_interrupt_handler(void *param);
 static void igbvf_mbx_process(struct rte_eth_dev *dev);
 static int igb_filter_restore(struct rte_eth_dev *dev);
+static int igbvf_dev_reset(struct rte_eth_dev *dev);
 
 /*
  * Define VF Stats MACRO for Non "cleared on read" register
@@ -468,6 +469,7 @@ static const struct eth_dev_ops igbvf_eth_dev_ops = {
 	.txq_info_get         = igb_txq_info_get,
 	.mac_addr_set         = igbvf_default_mac_addr_set,
 	.get_reg              = igbvf_get_regs,
+	.dev_reset            = igbvf_dev_reset,
 };
 
 /* store statistics names and its offset in stats structure */
@@ -2975,6 +2977,63 @@ void igbvf_mbx_process(struct rte_eth_dev *dev)
 }
 
 static int
+igbvf_dev_reset(struct rte_eth_dev *dev)
+{
+	struct e1000_hw *hw =
+		 E1000_DEV_PRIVATE_TO_HW(dev->data->dev_private);
+	int diag = 0;
+	uint32_t eiam;
+	/* Reference igbvf_intr_enable */
+	uint32_t eiam_mbx = 1 << E1000_VTIVAR_MISC_MAILBOX;
+
+	/* Nothing needs to be done if the device is not started. */
+	if (!dev->data->dev_started)
+		 return -EAGAIN;
+
+	PMD_DRV_LOG(DEBUG, "Link up/down event detected.");
+
+	/* Performance VF reset. */
+	do {
+		 dev->data->dev_started = 0;
+		 igbvf_dev_stop(dev);
+		 if (dev->data->dev_conf.intr_conf.lsc == 0)
+			  diag = eth_igb_link_update(dev, 0);
+		 if (diag) {
+			  PMD_INIT_LOG(INFO, "Igb VF reset: "
+					 "Failed to update link.");
+		 }
+		 rte_delay_ms(1000);
+
+		 diag = igbvf_dev_start(dev);
+		 dev->data->dev_started = 1;
+		 if (diag) {
+			  PMD_INIT_LOG(ERR, "Igb VF reset: "
+					 "Failed to start device.");
+			  return -EAGAIN;
+		 }
+		 eth_igbvf_stats_reset(dev);
+		 if (dev->data->dev_conf.intr_conf.lsc == 0)
+			  diag = eth_igb_link_update(dev, 0);
+		 if (diag) {
+			  PMD_INIT_LOG(INFO, "Igb VF reset: "
+					 "Failed to update link.");
+		 }
+
+		 /**
+		  * When the PF link is down, there is a chance
+		  * that the VF cannot operate its registers.
+		  * Check if the registers are written
+		  * successfully. If not, repeat stop/start until
+		  * the PF link is up, in other words, until the
+		  * registers can be written.
+		  */
+		 eiam = E1000_READ_REG(hw, E1000_EIAM);
+	} while (!(eiam & eiam_mbx));
+
+	return 0;
+}
+
+static int
 eth_igbvf_interrupt_action(struct rte_eth_dev *dev, struct rte_intr_handle *intr_handle)
 {
 	struct e1000_interrupt *intr =
-- 
2.11.0

[dpdk-dev] [PATCH 3/3] net/ixgbe: implement VF reset

[luca.boccassi]

From: Luca Boccassi <bluca@debian.org>

This reset function will stop and restart the device, and then reset
the stats and check that the registers have been updated.

Signed-off-by: Luca Boccassi <bluca@debian.org>
---
 drivers/net/ixgbe/ixgbe_ethdev.c | 60 +++++++++++++++++++++++++++++++++++-----
 drivers/net/ixgbe/ixgbe_ethdev.h |  2 +-
 drivers/net/ixgbe/ixgbe_rxtx.c   | 12 ++++++--
 3 files changed, 63 insertions(+), 11 deletions(-)

diff --git a/drivers/net/ixgbe/ixgbe_ethdev.c b/drivers/net/ixgbe/ixgbe_ethdev.c
index 14b9c5303..13307466b 100644
--- a/drivers/net/ixgbe/ixgbe_ethdev.c
+++ b/drivers/net/ixgbe/ixgbe_ethdev.c
@@ -5026,7 +5026,9 @@ ixgbevf_dev_start(struct rte_eth_dev *dev)
 		ETH_VLAN_EXTEND_MASK;
 	ixgbevf_vlan_offload_set(dev, mask);
 
-	ixgbevf_dev_rxtx_start(dev);
+	err = ixgbevf_dev_rxtx_start(dev);
+	if (err)
+		return err;
 
 	/* check and configure queue intr-vector mapping */
 	if (dev->data->dev_conf.intr_conf.rxq != 0) {
@@ -5127,15 +5129,59 @@ ixgbevf_dev_close(struct rte_eth_dev *dev)
 static int
 ixgbevf_dev_reset(struct rte_eth_dev *dev)
 {
-	int ret;
+	struct ixgbe_hw *hw = IXGBE_DEV_PRIVATE_TO_HW(dev->data->dev_private);
+	int diag = 0;
+	uint32_t vteiam;
 
-	ret = eth_ixgbevf_dev_uninit(dev);
-	if (ret)
-		return ret;
+	/* Nothing needs to be done if the device is not started. */
+	if (!dev->data->dev_started)
+		return -EAGAIN;
 
-	ret = eth_ixgbevf_dev_init(dev);
+	PMD_DRV_LOG(DEBUG, "Link up/down event detected.");
 
-	return ret;
+	/* 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.");
+			dev->data->dev_started = 1;
+			return -EAGAIN;
+		}
+		dev->data->dev_started = 1;
+		ixgbevf_dev_stats_reset(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.");
+			diag = 0;
+		}
+
+		/**
+		 * When the PF link is down, there is a chance
+		 * that the VF cannot operate its registers.
+		 * Check if the registers are written
+		 * successfully. If not, repeat stop/start until
+		 * the PF link is up, or in other words, until the
+		 * registers can be written.
+		 */
+		vteiam = IXGBE_READ_REG(hw, IXGBE_VTEIAM);
+	/* Reference ixgbevf_intr_enable when checking */
+	} while (diag || vteiam != IXGBE_VF_IRQ_ENABLE_MASK);
+
+	return 0;
 }
 
 static void ixgbevf_set_vfta_all(struct rte_eth_dev *dev, bool on)
diff --git a/drivers/net/ixgbe/ixgbe_ethdev.h b/drivers/net/ixgbe/ixgbe_ethdev.h
index e28c8567e..87b929518 100644
--- a/drivers/net/ixgbe/ixgbe_ethdev.h
+++ b/drivers/net/ixgbe/ixgbe_ethdev.h
@@ -595,7 +595,7 @@ int ixgbevf_dev_rx_init(struct rte_eth_dev *dev);
 
 void ixgbevf_dev_tx_init(struct rte_eth_dev *dev);
 
-void ixgbevf_dev_rxtx_start(struct rte_eth_dev *dev);
+int ixgbevf_dev_rxtx_start(struct rte_eth_dev *dev);
 
 uint16_t ixgbe_recv_pkts(void *rx_queue, struct rte_mbuf **rx_pkts,
 		uint16_t nb_pkts);
diff --git a/drivers/net/ixgbe/ixgbe_rxtx.c b/drivers/net/ixgbe/ixgbe_rxtx.c
index 0038dfbb4..5a0be2cfe 100644
--- a/drivers/net/ixgbe/ixgbe_rxtx.c
+++ b/drivers/net/ixgbe/ixgbe_rxtx.c
@@ -5419,7 +5419,7 @@ ixgbevf_dev_tx_init(struct rte_eth_dev *dev)
 /*
  * [VF] Start Transmit and Receive Units.
  */
-void __attribute__((cold))
+int __attribute__((cold))
 ixgbevf_dev_rxtx_start(struct rte_eth_dev *dev)
 {
 	struct ixgbe_hw     *hw;
@@ -5455,8 +5455,10 @@ ixgbevf_dev_rxtx_start(struct rte_eth_dev *dev)
 			rte_delay_ms(1);
 			txdctl = IXGBE_READ_REG(hw, IXGBE_VFTXDCTL(i));
 		} while (--poll_ms && !(txdctl & IXGBE_TXDCTL_ENABLE));
-		if (!poll_ms)
+		if (!poll_ms) {
 			PMD_INIT_LOG(ERR, "Could not enable Tx Queue %d", i);
+			return -1;
+		}
 	}
 	for (i = 0; i < dev->data->nb_rx_queues; i++) {
 
@@ -5472,12 +5474,16 @@ ixgbevf_dev_rxtx_start(struct rte_eth_dev *dev)
 			rte_delay_ms(1);
 			rxdctl = IXGBE_READ_REG(hw, IXGBE_VFRXDCTL(i));
 		} while (--poll_ms && !(rxdctl & IXGBE_RXDCTL_ENABLE));
-		if (!poll_ms)
+		if (!poll_ms) {
 			PMD_INIT_LOG(ERR, "Could not enable Rx Queue %d", i);
+			return -1;
+		}
 		rte_wmb();
 		IXGBE_WRITE_REG(hw, IXGBE_VFRDT(i), rxq->nb_rx_desc - 1);
 
 	}
+
+	return 0;
 }
 
 /* Stubs needed for linkage when CONFIG_RTE_IXGBE_INC_VECTOR is set to 'n' */
-- 
2.11.0

Re: [dpdk-dev] [PATCH 0/3] RFC: implement VF reset for i40e, e1000 and ixgbe

[Ferruh Yigit]

On 10/24/2017 6:16 AM, luca.boccassi@gmail.com wrote:
> From: Luca Boccassi <bluca@debian.org>
> 
> These patches were originally sent by Wenzhuo Lu:
> 
> http://dpdk.org/dev/patchwork/patch/14009/
> http://dpdk.org/dev/patchwork/patch/14010/
> http://dpdk.org/dev/patchwork/patch/14011/
> 
> The current rte_eth_dev_reset API does not correctly reset the VF
> when the PF flaps on the host. With these patches, at least the ixgbe
> driver with a X540 card (Linux kernel 4.9 on the host) appears to
> work correctly.
> The test is as simple ip link down/up on the host, and then check that
> traffic still flows through the VF in the guest.
> 
> I am not an expert in these PMDs hence the RFC mark - I would like to
> ask for feedback and help from the PMD maintainers and developers.
> 
> Thanks!
> 
> Luca Boccassi (3):
>   net/i40e: implement VF reset
>   net/e1000: implement VF reset
>   net/ixgbe: implement VF reset

Hi Luca,

Is this set targets this release?
Would you mind deferring this to next release, taking account that we are so
close to release?

Thanks,
ferruh

> 
>  drivers/net/e1000/igb_ethdev.c    | 59 ++++++++++++++++++++++++++++++++++++++
>  drivers/net/i40e/i40e_ethdev.h    |  3 ++
>  drivers/net/i40e/i40e_ethdev_vf.c | 56 +++++++++++++++++++++++++++++++++---
>  drivers/net/i40e/i40e_rxtx.c      | 11 +++++++
>  drivers/net/i40e/i40e_rxtx.h      |  4 +++
>  drivers/net/ixgbe/ixgbe_ethdev.c  | 60 ++++++++++++++++++++++++++++++++++-----
>  drivers/net/ixgbe/ixgbe_ethdev.h  |  2 +-
>  drivers/net/ixgbe/ixgbe_rxtx.c    | 12 ++++++--
>  8 files changed, 192 insertions(+), 15 deletions(-)
> 

Re: [dpdk-dev] [PATCH 0/3] RFC: implement VF reset for i40e, e1000 and ixgbe

[Luca Boccassi]

On Tue, 2017-10-24 at 18:17 -0700, Ferruh Yigit wrote:
> On 10/24/2017 6:16 AM, luca.boccassi@gmail.com wrote:
> > From: Luca Boccassi <bluca@debian.org>
> > 
> > These patches were originally sent by Wenzhuo Lu:
> > 
> > http://dpdk.org/dev/patchwork/patch/14009/
> > http://dpdk.org/dev/patchwork/patch/14010/
> > http://dpdk.org/dev/patchwork/patch/14011/
> > 
> > The current rte_eth_dev_reset API does not correctly reset the VF
> > when the PF flaps on the host. With these patches, at least the
> > ixgbe
> > driver with a X540 card (Linux kernel 4.9 on the host) appears to
> > work correctly.
> > The test is as simple ip link down/up on the host, and then check
> > that
> > traffic still flows through the VF in the guest.
> > 
> > I am not an expert in these PMDs hence the RFC mark - I would like
> > to
> > ask for feedback and help from the PMD maintainers and developers.
> > 
> > Thanks!
> > 
> > Luca Boccassi (3):
> >   net/i40e: implement VF reset
> >   net/e1000: implement VF reset
> >   net/ixgbe: implement VF reset
> 
> Hi Luca,
> 
> Is this set targets this release?
> Would you mind deferring this to next release, taking account that we
> are so
> close to release?
> 
> Thanks,
> ferruh

Hi Ferruh,

Yes of course, actually not even the next release as it really needs
feedback from the PMD developers/maintainers.
I've marked it as RFC on patchwork as well, is that all that's needed?

-- 
Kind regards,
Luca Boccassi

Re: [dpdk-dev] [PATCH 0/3] RFC: implement VF reset for i40e, e1000 and ixgbe

[Dai, Wei]

Current rte_eth_dev_reset( ) from my patches are just to reset the NIC port, NOT recovery its traffic.
All my patches for NIC reset have already been accepted.

It is the duty of user application to recovery any settings and traffic by going through rte_eth_dev_configure(...), 
rte_eth_rx_queue_setup(...)/rte_eth_tx_queue_setup(...), rte_eth_dev_start(...) 
and then rte_eth_rx_burst(...)/rte_eth_tx_burst(...).
In the recovery process,  user application can use same or different settings from those before reset.

Indeed, my patch use more generic way to reset NIC.
It has only been implemented on ixgbe VF and i40e VF currently.

You can test ixgbe VF reset with testpmd as follows:
1.  run testpmd with 2 ixgbe VF ports belonging to same PF in rxonly mode
2.  testpmd > set verbose 1 //to observe VF working 
3.  testpmd > show port info all //show port number and MAC addr 
4.  testpmd > start 
5.  let all ports forwarding work for a while 
6.  testpmd > show port stats all 
7.  ifconfig name-of-PF down 
8.  A message is shown in testpmd to indicate PF reset 
9.  ifconfig name-of-PF up 
10. testpmd > stop // stop forwarding to avoid crash during reset 
11. testpmd > port stop all 
12. testpmd > port reset all 
13. testpmd > port start all //recofnig all ports {color} 
14. testpmd > show port info all    //get mapping of port id and MAC addr for forwarding 
15. testpmd > start // restore forwarding 
16. let all ports forwarding work for a while 
17. testpmd > show port stats all //confirm all port can work again 
18. repeat above step 7 - 17

The codes to recover traffic using rte_eth_dev_reset(...) should like as follows:
1. rte_eth_dev_configure(...)
2. rte_eth_rx_queue_setup(...) + rte_eth_tx_queue_setup(...)
3. rte_eth_dev_callback_register(port_id, RTE_ETH_EVENT_INTR_RESET, eth_event_callback, cb_arg)
4. rte_eth_dev_start(...)
5. start transmitting and receiving
6. When detecting Reset event due to 'ifconfig PF down', eth_event_callback will be called. 
eth_event_callback( ) should trigger following step 7-14
7. stop transmitting and receiving
8. wait and confirm both transmitting and receiving are stopped.
9. rte_eth_dev_stop(...)
10. try rte_eth_dev_reset(...) multiple times until it return 0, and then following step 11-14 to restart the port
11. rte_eth_dev_configure(...)
12. rte_eth_rx_queue_setup(...) + rte_eth_tx_queue_setup(...)
13. rte_eth_dev_start(...)
14. restart transmitting and receiving
All configurations including step 1/2/3/4/6/8/9/10/11/12/13 had better be run in same thread like testpmd main thread.
In above step 10, if the PF is down, rte_eth_dev_reset(...) will fail, so it should try multiple times until the PF is UP.
 
Regards
-Wei

> -----Original Message-----
> From: luca.boccassi@gmail.com [mailto:luca.boccassi@gmail.com]
> Sent: Tuesday, October 24, 2017 9:16 PM
> To: dev@dpdk.org
> Cc: Lu, Wenzhuo <wenzhuo.lu@intel.com>; Dai, Wei <wei.dai@intel.com>;
> Horton, Remy <remy.horton@intel.com>
> Subject: [PATCH 0/3] RFC: implement VF reset for i40e, e1000 and ixgbe
> 
> From: Luca Boccassi <bluca@debian.org>
> 
> These patches were originally sent by Wenzhuo Lu:
> 
> http://dpdk.org/dev/patchwork/patch/14009/
> http://dpdk.org/dev/patchwork/patch/14010/
> http://dpdk.org/dev/patchwork/patch/14011/
> 
> The current rte_eth_dev_reset API does not correctly reset the VF when the
> PF flaps on the host. With these patches, at least the ixgbe driver with a X540
> card (Linux kernel 4.9 on the host) appears to work correctly.
> The test is as simple ip link down/up on the host, and then check that traffic
> still flows through the VF in the guest.
> 
> I am not an expert in these PMDs hence the RFC mark - I would like to ask for
> feedback and help from the PMD maintainers and developers.
> 
> Thanks!
> 
> Luca Boccassi (3):
>   net/i40e: implement VF reset
>   net/e1000: implement VF reset
>   net/ixgbe: implement VF reset
> 
>  drivers/net/e1000/igb_ethdev.c    | 59
> ++++++++++++++++++++++++++++++++++++++
>  drivers/net/i40e/i40e_ethdev.h    |  3 ++
>  drivers/net/i40e/i40e_ethdev_vf.c | 56
> +++++++++++++++++++++++++++++++++---
>  drivers/net/i40e/i40e_rxtx.c      | 11 +++++++
>  drivers/net/i40e/i40e_rxtx.h      |  4 +++
>  drivers/net/ixgbe/ixgbe_ethdev.c  | 60
> ++++++++++++++++++++++++++++++++++-----
>  drivers/net/ixgbe/ixgbe_ethdev.h  |  2 +-
>  drivers/net/ixgbe/ixgbe_rxtx.c    | 12 ++++++--
>  8 files changed, 192 insertions(+), 15 deletions(-)
> 
> --
> 2.11.0

Re: [dpdk-dev] [PATCH 0/3] RFC: implement VF reset for i40e, e1000 and ixgbe

[Luca Boccassi]

On Thu, 2017-10-26 at 08:08 +0000, Dai, Wei wrote:
> Current rte_eth_dev_reset( ) from my patches are just to reset the
> NIC port, NOT recovery its traffic.
> All my patches for NIC reset have already been accepted.
> 
> It is the duty of user application to recovery any settings and
> traffic by going through rte_eth_dev_configure(...), 
> rte_eth_rx_queue_setup(...)/rte_eth_tx_queue_setup(...),
> rte_eth_dev_start(...) 
> and then rte_eth_rx_burst(...)/rte_eth_tx_burst(...).
> In the recovery process,  user application can use same or different
> settings from those before reset.
> 
> Indeed, my patch use more generic way to reset NIC.
> It has only been implemented on ixgbe VF and i40e VF currently.
> 
> You can test ixgbe VF reset with testpmd as follows:
> 1.  run testpmd with 2 ixgbe VF ports belonging to same PF in rxonly
> mode
> 2.  testpmd > set verbose 1 //to observe VF working 
> 3.  testpmd > show port info all //show port number and MAC addr 
> 4.  testpmd > start 
> 5.  let all ports forwarding work for a while 
> 6.  testpmd > show port stats all 
> 7.  ifconfig name-of-PF down 
> 8.  A message is shown in testpmd to indicate PF reset 
> 9.  ifconfig name-of-PF up 
> 10. testpmd > stop // stop forwarding to avoid crash during reset 
> 11. testpmd > port stop all 
> 12. testpmd > port reset all 
> 13. testpmd > port start all //recofnig all ports {color} 
> 14. testpmd > show port info all    //get mapping of port id and MAC
> addr for forwarding 
> 15. testpmd > start // restore forwarding 
> 16. let all ports forwarding work for a while 
> 17. testpmd > show port stats all //confirm all port can work again 
> 18. repeat above step 7 - 17
> 
> The codes to recover traffic using rte_eth_dev_reset(...) should like
> as follows:
> 1. rte_eth_dev_configure(...)
> 2. rte_eth_rx_queue_setup(...) + rte_eth_tx_queue_setup(...)
> 3. rte_eth_dev_callback_register(port_id, RTE_ETH_EVENT_INTR_RESET,
> eth_event_callback, cb_arg)
> 4. rte_eth_dev_start(...)
> 5. start transmitting and receiving
> 6. When detecting Reset event due to 'ifconfig PF down',
> eth_event_callback will be called. 
> eth_event_callback( ) should trigger following step 7-14
> 7. stop transmitting and receiving
> 8. wait and confirm both transmitting and receiving are stopped.
> 9. rte_eth_dev_stop(...)
> 10. try rte_eth_dev_reset(...) multiple times until it return 0, and
> then following step 11-14 to restart the port
> 11. rte_eth_dev_configure(...)
> 12. rte_eth_rx_queue_setup(...) + rte_eth_tx_queue_setup(...)
> 13. rte_eth_dev_start(...)
> 14. restart transmitting and receiving
> All configurations including step 1/2/3/4/6/8/9/10/11/12/13 had
> better be run in same thread like testpmd main thread.
> In above step 10, if the PF is down, rte_eth_dev_reset(...) will
> fail, so it should try multiple times until the PF is UP.
>  
> Regards
> -Wei

Hi Wei,

Thanks for having a look!

So currently the documentation does not explicitly say that rte_eth-
dev_configure/stop/start have to be called, but only to stop the
queues, so it would be good to add it if that's the case.

But on top of that, wouldn't it be better if the PMD handled that
transparently? So that the application, as the documentation currently
says, would only have to stop the queues, call reset and then restart
them? Is there any particular reason why this can't be done like the
RFC patches implement?

> > -----Original Message-----
> > From: luca.boccassi@gmail.com [mailto:luca.boccassi@gmail.com]
> > Sent: Tuesday, October 24, 2017 9:16 PM
> > To: dev@dpdk.org
> > Cc: Lu, Wenzhuo <wenzhuo.lu@intel.com>; Dai, Wei <wei.dai@intel.com
> > >;
> > Horton, Remy <remy.horton@intel.com>
> > Subject: [PATCH 0/3] RFC: implement VF reset for i40e, e1000 and
> > ixgbe
> > 
> > From: Luca Boccassi <bluca@debian.org>
> > 
> > These patches were originally sent by Wenzhuo Lu:
> > 
> > http://dpdk.org/dev/patchwork/patch/14009/
> > http://dpdk.org/dev/patchwork/patch/14010/
> > http://dpdk.org/dev/patchwork/patch/14011/
> > 
> > The current rte_eth_dev_reset API does not correctly reset the VF
> > when the
> > PF flaps on the host. With these patches, at least the ixgbe driver
> > with a X540
> > card (Linux kernel 4.9 on the host) appears to work correctly.
> > The test is as simple ip link down/up on the host, and then check
> > that traffic
> > still flows through the VF in the guest.
> > 
> > I am not an expert in these PMDs hence the RFC mark - I would like
> > to ask for
> > feedback and help from the PMD maintainers and developers.
> > 
> > Thanks!
> > 
> > Luca Boccassi (3):
> >   net/i40e: implement VF reset
> >   net/e1000: implement VF reset
> >   net/ixgbe: implement VF reset
> > 
> >  drivers/net/e1000/igb_ethdev.c    | 59
> > ++++++++++++++++++++++++++++++++++++++
> >  drivers/net/i40e/i40e_ethdev.h    |  3 ++
> >  drivers/net/i40e/i40e_ethdev_vf.c | 56
> > +++++++++++++++++++++++++++++++++---
> >  drivers/net/i40e/i40e_rxtx.c      | 11 +++++++
> >  drivers/net/i40e/i40e_rxtx.h      |  4 +++
> >  drivers/net/ixgbe/ixgbe_ethdev.c  | 60
> > ++++++++++++++++++++++++++++++++++-----
> >  drivers/net/ixgbe/ixgbe_ethdev.h  |  2 +-
> >  drivers/net/ixgbe/ixgbe_rxtx.c    | 12 ++++++--
> >  8 files changed, 192 insertions(+), 15 deletions(-)
> > 
> > --
> > 2.11.0
> 
> 

-- 
Kind regards,
Luca Boccassi