[dpdk-dev] [PATCH v2] net/i40e: fix VF/PF port reset

[Zhirun Yan]

Port reset will call i40evf_uninit_vf() to release resource. It wants
to call i40evf_dev_close() to do some clean work. Before this patch,
port reset will never call i40evf_dev_close() to shutdown adminq. So
the i40evf_dev_init() will failed.

The status of stop/start should be flaged by dev->data->dev_started.
Close should be flaged by hw->adapter_stopped. This patch modify
these flags both in VF and PF.

Signed-off-by: Zhirun Yan <***@intel.com>
Signed-off-by: Haiyue Wang <***@intel.com>
---
drivers/net/i40e/i40e_ethdev.c | 13 ++++++-------
drivers/net/i40e/i40e_ethdev_vf.c | 17 ++++++++---------
2 files changed, 14 insertions(+), 16 deletions(-)

diff --git a/drivers/net/i40e/i40e_ethdev.c b/drivers/net/i40e/i40e_ethdev.c
index 7030eb1fa..8eaf6eba6 100644
--- a/drivers/net/i40e/i40e_ethdev.c
+++ b/drivers/net/i40e/i40e_ethdev.c
@@ -2391,15 +2391,11 @@ static void
i40e_dev_stop(struct rte_eth_dev *dev)
{
struct i40e_pf *pf = I40E_DEV_PRIVATE_TO_PF(dev->data->dev_private);
- struct i40e_hw *hw = I40E_DEV_PRIVATE_TO_HW(dev->data->dev_private);
struct i40e_vsi *main_vsi = pf->main_vsi;
struct rte_pci_device *pci_dev = RTE_ETH_DEV_TO_PCI(dev);
struct rte_intr_handle *intr_handle = &pci_dev->intr_handle;
int i;

- if (hw->adapter_stopped == 1)
- return;
-
if (dev->data->dev_conf.intr_conf.rxq == 0) {
rte_eal_alarm_cancel(i40e_dev_alarm_handler, dev);
rte_intr_enable(intr_handle);
@@ -2442,8 +2438,6 @@ i40e_dev_stop(struct rte_eth_dev *dev)

/* reset hierarchy commit */
pf->tm_conf.committed = false;
-
- hw->adapter_stopped = 1;
}

static void
@@ -2460,7 +2454,10 @@ i40e_dev_close(struct rte_eth_dev *dev)

PMD_INIT_FUNC_TRACE();

- i40e_dev_stop(dev);
+ if (dev->data->dev_started != 0) {
+ dev->data->dev_started = 0;
+ i40e_dev_stop(dev);
+ }

/* Remove all mirror rules */
while ((p_mirror = TAILQ_FIRST(&pf->mirror_list))) {
@@ -2523,6 +2520,8 @@ i40e_dev_close(struct rte_eth_dev *dev)
I40E_WRITE_REG(hw, I40E_PFGEN_CTRL,
(reg | I40E_PFGEN_CTRL_PFSWR_MASK));
I40E_WRITE_FLUSH(hw);
+
+ hw->adapter_stopped = 1;
}

/*
diff --git a/drivers/net/i40e/i40e_ethdev_vf.c b/drivers/net/i40e/i40e_ethdev_vf.c
index ae55b9b18..cbe9bec3c 100644
--- a/drivers/net/i40e/i40e_ethdev_vf.c
+++ b/drivers/net/i40e/i40e_ethdev_vf.c
@@ -1924,15 +1924,12 @@ static int
i40evf_dev_start(struct rte_eth_dev *dev)
{
struct i40e_vf *vf = I40EVF_DEV_PRIVATE_TO_VF(dev->data->dev_private);
- struct i40e_hw *hw = I40E_DEV_PRIVATE_TO_HW(dev->data->dev_private);
struct rte_pci_device *pci_dev = RTE_ETH_DEV_TO_PCI(dev);
struct rte_intr_handle *intr_handle = &pci_dev->intr_handle;
uint32_t intr_vector = 0;

PMD_INIT_FUNC_TRACE();

- hw->adapter_stopped = 0;
-
vf->max_pkt_len = dev->data->dev_conf.rxmode.max_rx_pkt_len;
vf->num_queue_pairs = RTE_MAX(dev->data->nb_rx_queues,
dev->data->nb_tx_queues);
@@ -2004,7 +2001,6 @@ i40evf_dev_stop(struct rte_eth_dev *dev)
{
struct rte_pci_device *pci_dev = RTE_ETH_DEV_TO_PCI(dev);
struct rte_intr_handle *intr_handle = &pci_dev->intr_handle;
- struct i40e_hw *hw = I40E_DEV_PRIVATE_TO_HW(dev->data->dev_private);
struct i40e_vf *vf = I40EVF_DEV_PRIVATE_TO_VF(dev->data->dev_private);

PMD_INIT_FUNC_TRACE();
@@ -2012,8 +2008,6 @@ i40evf_dev_stop(struct rte_eth_dev *dev)
if (dev->data->dev_conf.intr_conf.rxq != 0)
rte_intr_disable(intr_handle);

- if (hw->adapter_stopped == 1)
- return;
i40evf_stop_queues(dev);
i40evf_disable_queues_intr(dev);
i40e_dev_clear_queues(dev);
@@ -2029,7 +2023,6 @@ i40evf_dev_stop(struct rte_eth_dev *dev)
/* remove all multicast addresses */
i40evf_add_del_mc_addr_list(dev, vf->mc_addrs, vf->mc_addrs_num,
FALSE);
- hw->adapter_stopped = 1;

}

@@ -2246,7 +2239,11 @@ i40evf_dev_close(struct rte_eth_dev *dev)
{
struct i40e_hw *hw = I40E_DEV_PRIVATE_TO_HW(dev->data->dev_private);

- i40evf_dev_stop(dev);
+ if (dev->data->dev_started != 0) {
+ dev->data->dev_started = 0;
+ i40evf_dev_stop(dev);
+ }
+
i40e_dev_free_queues(dev);
/*
* disable promiscuous mode before reset vf
@@ -2255,11 +2252,13 @@ i40evf_dev_close(struct rte_eth_dev *dev)
*/
i40evf_dev_promiscuous_disable(dev);
i40evf_dev_allmulticast_disable(dev);
-
i40evf_reset_vf(hw);
+
i40e_shutdown_adminq(hw);
i40evf_disable_irq0(hw);
rte_eal_alarm_cancel(i40evf_dev_alarm_handler, dev);
+
+ hw->adapter_stopped = 1;
}

/*

[Zhang, Qi Z]

-----Original Message-----
From: Yan, Zhirun
Sent: Wednesday, December 5, 2018 8:46 PM
Subject: [PATCH v2] net/i40e: fix VF/PF port reset
Port reset will call i40evf_uninit_vf() to release resource. It wants to call
i40evf_dev_close() to do some clean work. Before this patch, port reset will
never call i40evf_dev_close() to shutdown adminq. So the i40evf_dev_init() will
failed.
The status of stop/start should be flaged by dev->data->dev_started.
Close should be flaged by hw->adapter_stopped. This patch modify these flags
both in VF and PF.

I agree with this idea, we need a flag to indicate if a device is closed or not.

---

It's better to add change log here.

drivers/net/i40e/i40e_ethdev.c | 13 ++++++-------
drivers/net/i40e/i40e_ethdev_vf.c | 17 ++++++++---------
2 files changed, 14 insertions(+), 16 deletions(-)
diff --git a/drivers/net/i40e/i40e_ethdev.c b/drivers/net/i40e/i40e_ethdev.c
index 7030eb1fa..8eaf6eba6 100644
--- a/drivers/net/i40e/i40e_ethdev.c
+++ b/drivers/net/i40e/i40e_ethdev.c
@@ -2391,15 +2391,11 @@ static void
i40e_dev_stop(struct rte_eth_dev *dev)
{
struct i40e_pf *pf = I40E_DEV_PRIVATE_TO_PF(dev->data->dev_private);
- struct i40e_hw *hw =
I40E_DEV_PRIVATE_TO_HW(dev->data->dev_private);
struct i40e_vsi *main_vsi = pf->main_vsi;
struct rte_pci_device *pci_dev = RTE_ETH_DEV_TO_PCI(dev);
struct rte_intr_handle *intr_handle = &pci_dev->intr_handle;
int i;
- if (hw->adapter_stopped == 1)
- return;
-
if (dev->data->dev_conf.intr_conf.rxq == 0) {
rte_eal_alarm_cancel(i40e_dev_alarm_handler, dev);
rte_intr_enable(intr_handle);
@@ -2442,8 +2438,6 @@ i40e_dev_stop(struct rte_eth_dev *dev)
/* reset hierarchy commit */
pf->tm_conf.committed = false;
-
- hw->adapter_stopped = 1;
}
static void
@@ -2460,7 +2454,10 @@ i40e_dev_close(struct rte_eth_dev *dev)
PMD_INIT_FUNC_TRACE();
- i40e_dev_stop(dev);
+ if (dev->data->dev_started != 0) {
+ dev->data->dev_started = 0;
+ i40e_dev_stop(dev);
+ }

please check rte_eth_dev_close:

dev->data->dev_started will be set to 0 before call i40e_dev_close,
so above check will always fail and i40e_dev_stop will not be called which is not expected.

I think we can keep adapter_stopped for original usage and add a new flag "adapter_closed" to fix the gap for dev_reset.

/* Remove all mirror rules */
@@ i40e_dev_close(struct rte_eth_dev *dev)
I40E_WRITE_REG(hw, I40E_PFGEN_CTRL,
(reg | I40E_PFGEN_CTRL_PFSWR_MASK));
I40E_WRITE_FLUSH(hw);
+
+ hw->adapter_stopped = 1;
}
/*
diff --git a/drivers/net/i40e/i40e_ethdev_vf.c
b/drivers/net/i40e/i40e_ethdev_vf.c
index ae55b9b18..cbe9bec3c 100644
--- a/drivers/net/i40e/i40e_ethdev_vf.c
+++ b/drivers/net/i40e/i40e_ethdev_vf.c
@@ -1924,15 +1924,12 @@ static int
i40evf_dev_start(struct rte_eth_dev *dev) {
struct i40e_vf *vf =
I40EVF_DEV_PRIVATE_TO_VF(dev->data->dev_private);
- struct i40e_hw *hw =
I40E_DEV_PRIVATE_TO_HW(dev->data->dev_private);
struct rte_pci_device *pci_dev = RTE_ETH_DEV_TO_PCI(dev);
struct rte_intr_handle *intr_handle = &pci_dev->intr_handle;
uint32_t intr_vector = 0;
PMD_INIT_FUNC_TRACE();
- hw->adapter_stopped = 0;
-
vf->max_pkt_len = dev->data->dev_conf.rxmode.max_rx_pkt_len;
vf->num_queue_pairs = RTE_MAX(dev->data->nb_rx_queues,
dev->data->nb_tx_queues);
@@ -2004,7 +2001,6 @@ i40evf_dev_stop(struct rte_eth_dev *dev) {
struct rte_pci_device *pci_dev = RTE_ETH_DEV_TO_PCI(dev);
struct rte_intr_handle *intr_handle = &pci_dev->intr_handle;
- struct i40e_hw *hw =
I40E_DEV_PRIVATE_TO_HW(dev->data->dev_private);
struct i40e_vf *vf =
I40EVF_DEV_PRIVATE_TO_VF(dev->data->dev_private);
PMD_INIT_FUNC_TRACE();
@@ -2012,8 +2008,6 @@ i40evf_dev_stop(struct rte_eth_dev *dev)
if (dev->data->dev_conf.intr_conf.rxq != 0)
rte_intr_disable(intr_handle);
- if (hw->adapter_stopped == 1)
- return;
i40evf_stop_queues(dev);
i40evf_disable_queues_intr(dev);
i40e_dev_clear_queues(dev);
@@ -2029,7 +2023,6 @@ i40evf_dev_stop(struct rte_eth_dev *dev)
/* remove all multicast addresses */
i40evf_add_del_mc_addr_list(dev, vf->mc_addrs, vf->mc_addrs_num,
FALSE);
- hw->adapter_stopped = 1;
}
@@ -2246,7 +2239,11 @@ i40evf_dev_close(struct rte_eth_dev *dev) {
struct i40e_hw *hw =
I40E_DEV_PRIVATE_TO_HW(dev->data->dev_private);
- i40evf_dev_stop(dev);
+ if (dev->data->dev_started != 0) {
+ dev->data->dev_started = 0;
+ i40evf_dev_stop(dev);
+ }

Same comment as above for VF.

+
i40e_dev_free_queues(dev);
/*
i40evf_dev_close(struct rte_eth_dev *dev)
*/
i40evf_dev_promiscuous_disable(dev);
i40evf_dev_allmulticast_disable(dev);
-
i40evf_reset_vf(hw);
+

This change is redundant, should be removed

i40e_shutdown_adminq(hw);
i40evf_disable_irq0(hw);
rte_eal_alarm_cancel(i40evf_dev_alarm_handler, dev);
+
+ hw->adapter_stopped = 1;
}
/*
--
2.17.1

[Yang, Qiming]

Hi, Zhirun

-----Original Message-----
Sent: Wednesday, December 5, 2018 8:46 PM
Subject: [dpdk-dev] [PATCH v2] net/i40e: fix VF/PF port reset
Port reset will call i40evf_uninit_vf() to release resource. It wants to call
i40evf_dev_close() to do some clean work. Before this patch, port reset will
never call i40evf_dev_close() to shutdown adminq. So the i40evf_dev_init() will
failed.
The status of stop/start should be flaged by dev->data->dev_started.
Close should be flaged by hw->adapter_stopped. This patch modify these flags
both in VF and PF.

For the fix patch need to add Fix line like:
Fixes: commit number ("commit log"), just refer to the old patch.

---
drivers/net/i40e/i40e_ethdev.c | 13 ++++++-------
drivers/net/i40e/i40e_ethdev_vf.c | 17 ++++++++---------
2 files changed, 14 insertions(+), 16 deletions(-)
diff --git a/drivers/net/i40e/i40e_ethdev.c b/drivers/net/i40e/i40e_ethdev.c
index 7030eb1fa..8eaf6eba6 100644
--- a/drivers/net/i40e/i40e_ethdev.c
+++ b/drivers/net/i40e/i40e_ethdev.c
@@ -2391,15 +2391,11 @@ static void
i40e_dev_stop(struct rte_eth_dev *dev)
{
struct i40e_pf *pf = I40E_DEV_PRIVATE_TO_PF(dev->data-

Post by Zhirun Yan
dev_private);

- struct i40e_hw *hw = I40E_DEV_PRIVATE_TO_HW(dev->data-

Post by Zhirun Yan
dev_private);

struct i40e_vsi *main_vsi = pf->main_vsi;
struct rte_pci_device *pci_dev = RTE_ETH_DEV_TO_PCI(dev);
struct rte_intr_handle *intr_handle = &pci_dev->intr_handle;
int i;
- if (hw->adapter_stopped == 1)
- return;
-

adapter_stopped is used to avoid double call dev_stop, why you remove the check here?
Do you add another mechanism to do the same thing?

if (dev->data->dev_conf.intr_conf.rxq == 0) {
rte_eal_alarm_cancel(i40e_dev_alarm_handler, dev);
rte_intr_enable(intr_handle);
@@ -2442,8 +2438,6 @@ i40e_dev_stop(struct rte_eth_dev *dev)
/* reset hierarchy commit */
pf->tm_conf.committed = false;
-
- hw->adapter_stopped = 1;
}
static void
@@ -2460,7 +2454,10 @@ i40e_dev_close(struct rte_eth_dev *dev)
PMD_INIT_FUNC_TRACE();
- i40e_dev_stop(dev);
+ if (dev->data->dev_started != 0) {
+ dev->data->dev_started = 0;
+ i40e_dev_stop(dev);
+ }
/* Remove all mirror rules */
I40E_WRITE_REG(hw, I40E_PFGEN_CTRL,
(reg | I40E_PFGEN_CTRL_PFSWR_MASK));
I40E_WRITE_FLUSH(hw);
+
+ hw->adapter_stopped = 1;

If you remove all the other hw->adapter_stopped, it's useless to add here.

}
/*
diff --git a/drivers/net/i40e/i40e_ethdev_vf.c
b/drivers/net/i40e/i40e_ethdev_vf.c
index ae55b9b18..cbe9bec3c 100644
--- a/drivers/net/i40e/i40e_ethdev_vf.c
+++ b/drivers/net/i40e/i40e_ethdev_vf.c
@@ -1924,15 +1924,12 @@ static int
i40evf_dev_start(struct rte_eth_dev *dev) {
struct i40e_vf *vf = I40EVF_DEV_PRIVATE_TO_VF(dev->data-

Post by Zhirun Yan
dev_private);

- struct i40e_hw *hw = I40E_DEV_PRIVATE_TO_HW(dev->data-

Post by Zhirun Yan
dev_private);

struct rte_pci_device *pci_dev = RTE_ETH_DEV_TO_PCI(dev);
struct rte_intr_handle *intr_handle = &pci_dev->intr_handle;
uint32_t intr_vector = 0;
PMD_INIT_FUNC_TRACE();
- hw->adapter_stopped = 0;
-

Question the same, will induce any problem?

vf->max_pkt_len = dev->data->dev_conf.rxmode.max_rx_pkt_len;
vf->num_queue_pairs = RTE_MAX(dev->data->nb_rx_queues,
dev->data->nb_tx_queues);
@@ -2004,7 +2001,6 @@ i40evf_dev_stop(struct rte_eth_dev *dev) {
struct rte_pci_device *pci_dev = RTE_ETH_DEV_TO_PCI(dev);
struct rte_intr_handle *intr_handle = &pci_dev->intr_handle;
- struct i40e_hw *hw = I40E_DEV_PRIVATE_TO_HW(dev->data-

Post by Zhirun Yan
dev_private);

struct i40e_vf *vf = I40EVF_DEV_PRIVATE_TO_VF(dev->data-

Post by Zhirun Yan
dev_private);

PMD_INIT_FUNC_TRACE();
@@ -2012,8 +2008,6 @@ i40evf_dev_stop(struct rte_eth_dev *dev)
if (dev->data->dev_conf.intr_conf.rxq != 0)
rte_intr_disable(intr_handle);
- if (hw->adapter_stopped == 1)
- return;
i40evf_stop_queues(dev);
i40evf_disable_queues_intr(dev);
i40e_dev_clear_queues(dev);
@@ -2029,7 +2023,6 @@ i40evf_dev_stop(struct rte_eth_dev *dev)
/* remove all multicast addresses */
i40evf_add_del_mc_addr_list(dev, vf->mc_addrs, vf->mc_addrs_num,
FALSE);
- hw->adapter_stopped = 1;
}
@@ -2246,7 +2239,11 @@ i40evf_dev_close(struct rte_eth_dev *dev) {
struct i40e_hw *hw = I40E_DEV_PRIVATE_TO_HW(dev->data-

Post by Zhirun Yan
dev_private);

- i40evf_dev_stop(dev);
+ if (dev->data->dev_started != 0) {
+ dev->data->dev_started = 0;
+ i40evf_dev_stop(dev);
+ }
+
i40e_dev_free_queues(dev);
/*
@@ i40evf_dev_close(struct rte_eth_dev *dev)
*/
i40evf_dev_promiscuous_disable(dev);
i40evf_dev_allmulticast_disable(dev);
-
i40evf_reset_vf(hw);
+
i40e_shutdown_adminq(hw);
i40evf_disable_irq0(hw);
rte_eal_alarm_cancel(i40evf_dev_alarm_handler, dev);
+
+ hw->adapter_stopped = 1;
}
/*
--
2.17.1

[Yan, Zhirun]

Hi Qiming,

-----Original Message-----
From: Yang, Qiming
Sent: Thursday, December 6, 2018 10:57 AM
Subject: RE: [dpdk-dev] [PATCH v2] net/i40e: fix VF/PF port reset
Hi, Zhirun

-----Original Message-----
Sent: Wednesday, December 5, 2018 8:46 PM
Subject: [dpdk-dev] [PATCH v2] net/i40e: fix VF/PF port reset
Port reset will call i40evf_uninit_vf() to release resource. It wants to call
i40evf_dev_close() to do some clean work. Before this patch, port
reset will never call i40evf_dev_close() to shutdown adminq. So the
i40evf_dev_init() will failed.
The status of stop/start should be flaged by dev->data->dev_started.
Close should be flaged by hw->adapter_stopped. This patch modify these
flags both in VF and PF.

Fixes: commit number ("commit log"), just refer to the old patch.

Thanks, I will added it in the next version.

---
drivers/net/i40e/i40e_ethdev.c | 13 ++++++-------
drivers/net/i40e/i40e_ethdev_vf.c | 17 ++++++++---------
2 files changed, 14 insertions(+), 16 deletions(-)
diff --git a/drivers/net/i40e/i40e_ethdev.c
b/drivers/net/i40e/i40e_ethdev.c index 7030eb1fa..8eaf6eba6 100644
--- a/drivers/net/i40e/i40e_ethdev.c
+++ b/drivers/net/i40e/i40e_ethdev.c
@@ -2391,15 +2391,11 @@ static void
i40e_dev_stop(struct rte_eth_dev *dev) {
struct i40e_pf *pf = I40E_DEV_PRIVATE_TO_PF(dev->data-

Post by Zhirun Yan
dev_private);

- struct i40e_hw *hw = I40E_DEV_PRIVATE_TO_HW(dev->data-

Post by Zhirun Yan
dev_private);

struct i40e_vsi *main_vsi = pf->main_vsi;
struct rte_pci_device *pci_dev = RTE_ETH_DEV_TO_PCI(dev);
struct rte_intr_handle *intr_handle = &pci_dev->intr_handle;
int i;
- if (hw->adapter_stopped == 1)
- return;
-

adapter_stopped is used to avoid double call dev_stop, why you remove the check here?

hw->adapter_stopped try to control start/stop and close, but close will do more
than stop.

Do you add another mechanism to do the same thing?

I guess dev->data->dev_started is the right flag to avoid the issue.

if (dev->data->dev_conf.intr_conf.rxq == 0) {
rte_eal_alarm_cancel(i40e_dev_alarm_handler, dev);
rte_intr_enable(intr_handle);
@@ -2442,8 +2438,6 @@ i40e_dev_stop(struct rte_eth_dev *dev)
/* reset hierarchy commit */
pf->tm_conf.committed = false;
-
- hw->adapter_stopped = 1;
}
static void
@@ -2460,7 +2454,10 @@ i40e_dev_close(struct rte_eth_dev *dev)
PMD_INIT_FUNC_TRACE();
- i40e_dev_stop(dev);
+ if (dev->data->dev_started != 0) {
+ dev->data->dev_started = 0;
+ i40e_dev_stop(dev);
+ }
/* Remove all mirror rules */
I40E_WRITE_REG(hw, I40E_PFGEN_CTRL,
(reg | I40E_PFGEN_CTRL_PFSWR_MASK));
I40E_WRITE_FLUSH(hw);
+
+ hw->adapter_stopped = 1;

If you remove all the other hw->adapter_stopped, it's useless to add here.

I will use another flag like adapter_closed to mark the close status.

}
/*
diff --git a/drivers/net/i40e/i40e_ethdev_vf.c
b/drivers/net/i40e/i40e_ethdev_vf.c
index ae55b9b18..cbe9bec3c 100644
--- a/drivers/net/i40e/i40e_ethdev_vf.c
+++ b/drivers/net/i40e/i40e_ethdev_vf.c
@@ -1924,15 +1924,12 @@ static int
i40evf_dev_start(struct rte_eth_dev *dev) {
struct i40e_vf *vf = I40EVF_DEV_PRIVATE_TO_VF(dev->data-

Post by Zhirun Yan
dev_private);

- struct i40e_hw *hw = I40E_DEV_PRIVATE_TO_HW(dev->data-

Post by Zhirun Yan
dev_private);

struct rte_pci_device *pci_dev = RTE_ETH_DEV_TO_PCI(dev);
struct rte_intr_handle *intr_handle = &pci_dev->intr_handle;
uint32_t intr_vector = 0;
PMD_INIT_FUNC_TRACE();
- hw->adapter_stopped = 0;
-

Question the same, will induce any problem?

I will check it.

vf->max_pkt_len = dev->data->dev_conf.rxmode.max_rx_pkt_len;
vf->num_queue_pairs = RTE_MAX(dev->data->nb_rx_queues,
dev->data->nb_tx_queues);
@@ -2004,7 +2001,6 @@ i40evf_dev_stop(struct rte_eth_dev *dev) {
struct rte_pci_device *pci_dev = RTE_ETH_DEV_TO_PCI(dev);
struct rte_intr_handle *intr_handle = &pci_dev->intr_handle;
- struct i40e_hw *hw = I40E_DEV_PRIVATE_TO_HW(dev->data-

Post by Zhirun Yan
dev_private);

struct i40e_vf *vf = I40EVF_DEV_PRIVATE_TO_VF(dev->data-

Post by Zhirun Yan
dev_private);

PMD_INIT_FUNC_TRACE();
@@ -2012,8 +2008,6 @@ i40evf_dev_stop(struct rte_eth_dev *dev)
if (dev->data->dev_conf.intr_conf.rxq != 0)
rte_intr_disable(intr_handle);
- if (hw->adapter_stopped == 1)
- return;
i40evf_stop_queues(dev);
i40evf_disable_queues_intr(dev);
i40e_dev_clear_queues(dev);
@@ -2029,7 +2023,6 @@ i40evf_dev_stop(struct rte_eth_dev *dev)
/* remove all multicast addresses */
i40evf_add_del_mc_addr_list(dev, vf->mc_addrs, vf->mc_addrs_num,
FALSE);
- hw->adapter_stopped = 1;
}
@@ -2246,7 +2239,11 @@ i40evf_dev_close(struct rte_eth_dev *dev) {
struct i40e_hw *hw = I40E_DEV_PRIVATE_TO_HW(dev->data-

Post by Zhirun Yan
dev_private);

- i40evf_dev_stop(dev);
+ if (dev->data->dev_started != 0) {
+ dev->data->dev_started = 0;
+ i40evf_dev_stop(dev);
+ }
+
i40e_dev_free_queues(dev);
/*

@@

i40evf_dev_close(struct rte_eth_dev *dev)
*/
i40evf_dev_promiscuous_disable(dev);
i40evf_dev_allmulticast_disable(dev);
-
i40evf_reset_vf(hw);
+
i40e_shutdown_adminq(hw);
i40evf_disable_irq0(hw);
rte_eal_alarm_cancel(i40evf_dev_alarm_handler, dev);
+
+ hw->adapter_stopped = 1;
}
/*
--
2.17.1

[Zhirun Yan]

Port reset will call i40evf_uninit_vf() to release resource. It wants
to call i40evf_dev_close() to release resources. Befort this patch,
hw->adapter_stopped was used to mark the status about start/stop and
close. So it will never call i40evf_dev_close() after stoped the port.

This patch added hw->adapter_closed flag in i40evf_dev_close() and
i40e_dev_close() to control the status of close.

Fixes: 4861cde4611601ccc9 ("i40e: new poll mode driver")

Signed-off-by: Zhirun Yan <***@intel.com>
Signed-off-by: Haiyue Wang <***@intel.com>
---
drivers/net/i40e/base/i40e_type.h | 1 +
drivers/net/i40e/i40e_ethdev.c | 5 ++++-
drivers/net/i40e/i40e_ethdev_vf.c | 4 +++-
3 files changed, 8 insertions(+), 2 deletions(-)

diff --git a/drivers/net/i40e/base/i40e_type.h b/drivers/net/i40e/base/i40e_type.h
index 77562f24e..7ba62cc12 100644
--- a/drivers/net/i40e/base/i40e_type.h
+++ b/drivers/net/i40e/base/i40e_type.h
@@ -670,6 +670,7 @@ struct i40e_hw {
u8 revision_id;
u8 port;
bool adapter_stopped;
+ bool adapter_closed;

/* capabilities for entire device and PCI func */
struct i40e_hw_capabilities dev_caps;
diff --git a/drivers/net/i40e/i40e_ethdev.c b/drivers/net/i40e/i40e_ethdev.c
index 7030eb1fa..3d9603afd 100644
--- a/drivers/net/i40e/i40e_ethdev.c
+++ b/drivers/net/i40e/i40e_ethdev.c
@@ -1316,6 +1316,7 @@ eth_i40e_dev_init(struct rte_eth_dev *dev, void *init_params __rte_unused)
hw->bus.device = pci_dev->addr.devid;
hw->bus.func = pci_dev->addr.function;
hw->adapter_stopped = 0;
+ hw->adapter_closed = 0;

/*
* Switch Tag value should not be identical to either the First Tag
@@ -1704,7 +1705,7 @@ eth_i40e_dev_uninit(struct rte_eth_dev *dev)
if (ret)
PMD_INIT_LOG(WARNING, "failed to free switch domain: %d", ret);

- if (hw->adapter_stopped == 0)
+ if (hw->adapter_closed == 0)
i40e_dev_close(dev);

dev->dev_ops = NULL;
@@ -2523,6 +2524,8 @@ i40e_dev_close(struct rte_eth_dev *dev)
I40E_WRITE_REG(hw, I40E_PFGEN_CTRL,
(reg | I40E_PFGEN_CTRL_PFSWR_MASK));
I40E_WRITE_FLUSH(hw);
+
+ hw->adapter_closed = 1;
}

/*
diff --git a/drivers/net/i40e/i40e_ethdev_vf.c b/drivers/net/i40e/i40e_ethdev_vf.c
index ae55b9b18..8374b98a9 100644
--- a/drivers/net/i40e/i40e_ethdev_vf.c
+++ b/drivers/net/i40e/i40e_ethdev_vf.c
@@ -1256,7 +1256,7 @@ i40evf_uninit_vf(struct rte_eth_dev *dev)

PMD_INIT_FUNC_TRACE();

- if (hw->adapter_stopped == 0)
+ if (hw->adapter_closed == 0)
i40evf_dev_close(dev);
rte_free(vf->vf_res);
vf->vf_res = NULL;
@@ -1438,6 +1438,7 @@ i40evf_dev_init(struct rte_eth_dev *eth_dev)
hw->bus.func = pci_dev->addr.function;
hw->hw_addr = (void *)pci_dev->mem_resource[0].addr;
hw->adapter_stopped = 0;
+ hw->adapter_closed = 0;

if(i40evf_init_vf(eth_dev) != 0) {
PMD_INIT_LOG(ERR, "Init vf failed");
@@ -2260,6 +2261,7 @@ i40evf_dev_close(struct rte_eth_dev *dev)
i40e_shutdown_adminq(hw);
i40evf_disable_irq0(hw);
rte_eal_alarm_cancel(i40evf_dev_alarm_handler, dev);
+ hw->adapter_closed = 1;
}

/*

[Zhirun Yan]

Port reset will call i40evf_uninit_vf() to release resource. It wants
to call i40evf_dev_close() to release resources. Befort this patch,
hw->adapter_stopped was used to mark the status about start/stop and
close. So it will never call i40evf_dev_close() after stopped the port.

This patch added hw->adapter_closed flag in i40evf_dev_close() and
i40e_dev_close() to control the status of close.

Fixes: 4861cde4611601ccc9 ("i40e: new poll mode driver")

Signed-off-by: Zhirun Yan <***@intel.com>
Signed-off-by: Haiyue Wang <***@intel.com>
---

V4:
- Modify coding style issues.

drivers/net/i40e/base/i40e_type.h | 1 +
drivers/net/i40e/i40e_ethdev.c | 5 ++++-
drivers/net/i40e/i40e_ethdev_vf.c | 4 +++-
3 files changed, 8 insertions(+), 2 deletions(-)

diff --git a/drivers/net/i40e/base/i40e_type.h b/drivers/net/i40e/base/i40e_type.h
index 77562f24e..7ba62cc12 100644
--- a/drivers/net/i40e/base/i40e_type.h
+++ b/drivers/net/i40e/base/i40e_type.h
@@ -670,6 +670,7 @@ struct i40e_hw {
u8 revision_id;
u8 port;
bool adapter_stopped;
+ bool adapter_closed;

/* capabilities for entire device and PCI func */
struct i40e_hw_capabilities dev_caps;
diff --git a/drivers/net/i40e/i40e_ethdev.c b/drivers/net/i40e/i40e_ethdev.c
index 7030eb1fa..f5bd48577 100644
--- a/drivers/net/i40e/i40e_ethdev.c
+++ b/drivers/net/i40e/i40e_ethdev.c
@@ -1316,6 +1316,7 @@ eth_i40e_dev_init(struct rte_eth_dev *dev, void *init_params __rte_unused)
hw->bus.device = pci_dev->addr.devid;
hw->bus.func = pci_dev->addr.function;
hw->adapter_stopped = 0;
+ hw->adapter_closed = 0;

/*
* Switch Tag value should not be identical to either the First Tag
@@ -1704,7 +1705,7 @@ eth_i40e_dev_uninit(struct rte_eth_dev *dev)
if (ret)
PMD_INIT_LOG(WARNING, "failed to free switch domain: %d", ret);

- if (hw->adapter_stopped == 0)
+ if (hw->adapter_closed == 0)
i40e_dev_close(dev);

dev->dev_ops = NULL;
@@ -2523,6 +2524,8 @@ i40e_dev_close(struct rte_eth_dev *dev)
I40E_WRITE_REG(hw, I40E_PFGEN_CTRL,
(reg | I40E_PFGEN_CTRL_PFSWR_MASK));
I40E_WRITE_FLUSH(hw);
+
+ hw->adapter_closed = 1;
}

/*
diff --git a/drivers/net/i40e/i40e_ethdev_vf.c b/drivers/net/i40e/i40e_ethdev_vf.c
index ae55b9b18..05dc6596b 100644
--- a/drivers/net/i40e/i40e_ethdev_vf.c
+++ b/drivers/net/i40e/i40e_ethdev_vf.c
@@ -1256,7 +1256,7 @@ i40evf_uninit_vf(struct rte_eth_dev *dev)

PMD_INIT_FUNC_TRACE();

- if (hw->adapter_stopped == 0)
+ if (hw->adapter_closed == 0)
i40evf_dev_close(dev);
rte_free(vf->vf_res);
vf->vf_res = NULL;
@@ -1438,6 +1438,7 @@ i40evf_dev_init(struct rte_eth_dev *eth_dev)
hw->bus.func = pci_dev->addr.function;
hw->hw_addr = (void *)pci_dev->mem_resource[0].addr;
hw->adapter_stopped = 0;
+ hw->adapter_closed = 0;

if(i40evf_init_vf(eth_dev) != 0) {
PMD_INIT_LOG(ERR, "Init vf failed");
@@ -2260,6 +2261,7 @@ i40evf_dev_close(struct rte_eth_dev *dev)
i40e_shutdown_adminq(hw);
i40evf_disable_irq0(hw);
rte_eal_alarm_cancel(i40evf_dev_alarm_handler, dev);
+ hw->adapter_closed = 1;
}

/*

[Zhang, Qi Z]

-----Original Message-----
From: Yan, Zhirun
Sent: Monday, December 10, 2018 10:09 PM
Subject: [PATCH v4] net/i40e: fix VF/PF port close
Port reset will call i40evf_uninit_vf() to release resource. It wants to call
i40evf_dev_close() to release resources. Befort this patch,

s/Befort/before

hw->adapter_stopped was used to mark the status about start/stop and
close. So it will never call i40evf_dev_close() after stopped the port.

s/stopped/stopping

This patch added hw->adapter_closed flag in i40evf_dev_close() and
i40e_dev_close() to control the status of close.
Fixes: 4861cde4611601ccc9 ("i40e: new poll mode driver")

Acked-by: Qi Zhang <***@intel.com>

Applied to dpdk-next-net-intel with above typo fix

Thanks
Qi