[dpdk-dev] [PATCH v4 0/3] net/i40e: vf port reset

[dpdk-dev] [PATCH v4 0/3] net/i40e: vf port reset

[Wei Zhao]

The patches mainly finish following functions:
1) get pf reset vf comamand falg from interrupt server function.
2) reset vf port from testpmd app using a command.
3) sore and restore vf related parameters.

v2:
-change the order of patch in the set.
-add more details in patch comment to clarify that
 the rte and tespmd patch can also used in pf port reset.
-add  rte_free for memory free after restore.
-change varible name of reset_number to reset_falg.
-fix patch check warning.

v3:
-fix error of author mail address

v4:
-fix typo error
-add rte_wmb() after change the reset_ports.

zhao wei (3):
  app/testpmd: add port reset command into testpmd
  lib/librte_ether: add support for port reset
  net/i40e: implement device reset on port

 app/test-pmd/cmdline.c                 |  17 ++-
 app/test-pmd/testpmd.c                 |  65 ++++++++++++
 app/test-pmd/testpmd.h                 |   1 +
 drivers/net/i40e/i40e_ethdev.c         |   2 +-
 drivers/net/i40e/i40e_ethdev.h         |  16 +++
 drivers/net/i40e/i40e_ethdev_vf.c      | 185 +++++++++++++++++++++++++++++++++
 drivers/net/i40e/i40e_rxtx.c           |  10 ++
 drivers/net/i40e/i40e_rxtx.h           |   4 +
 lib/librte_ether/rte_ethdev.c          |  17 +++
 lib/librte_ether/rte_ethdev.h          |  25 +++++
 lib/librte_ether/rte_ether_version.map |   6 ++
 11 files changed, 343 insertions(+), 5 deletions(-)

-- 
2.9.3

[dpdk-dev] [PATCH v4 1/3] lib/librte_ether: add support for port reset

[Wei Zhao]

Add support for port reset in rte layer.This reset
feature can not only used in vf port reset in later code
develop, but alsopf port.But in this patch set, we only
limit the discussion scope to vf reset.
This patch Add an API to restart the device.
It's for VF device in this scenario, kernel PF + DPDK VF.
When the PF port down->up, APP should call this API to
restart VF port. Most likely, APP should call it in its
management thread and guarantee the thread safe. It means
APP should stop the rx/tx and the device, then restart
the device, then recover the device and rx/tx.
Also, it's APP's responsibilty to release the occupied
memory.

Signed-off-by: Wenzhuo Lu <wenzhuo.lu@intel.com>
Signed-off-by: Wei Zhao <wei.zhao1@intel.com>
---
 lib/librte_ether/rte_ethdev.c          | 17 +++++++++++++++++
 lib/librte_ether/rte_ethdev.h          | 25 +++++++++++++++++++++++++
 lib/librte_ether/rte_ether_version.map |  6 ++++++
 3 files changed, 48 insertions(+)

diff --git a/lib/librte_ether/rte_ethdev.c b/lib/librte_ether/rte_ethdev.c
index eb0a94a..2e06dca 100644
--- a/lib/librte_ether/rte_ethdev.c
+++ b/lib/librte_ether/rte_ethdev.c
@@ -3273,3 +3273,20 @@ rte_eth_dev_l2_tunnel_offload_set(uint8_t port_id,
 				-ENOTSUP);
 	return (*dev->dev_ops->l2_tunnel_offload_set)(dev, l2_tunnel, mask, en);
 }
+
+int
+rte_eth_dev_reset(uint8_t port_id)
+{
+	struct rte_eth_dev *dev;
+	int diag;
+
+	RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -ENODEV);
+
+	dev = &rte_eth_devices[port_id];
+
+	RTE_FUNC_PTR_OR_ERR_RET(*dev->dev_ops->dev_reset, -ENOTSUP);
+
+	diag = (*dev->dev_ops->dev_reset)(dev);
+
+	return diag;
+}
diff --git a/lib/librte_ether/rte_ethdev.h b/lib/librte_ether/rte_ethdev.h
index 4be217c..34412c0 100644
--- a/lib/librte_ether/rte_ethdev.h
+++ b/lib/librte_ether/rte_ethdev.h
@@ -1367,6 +1367,9 @@ typedef int (*eth_l2_tunnel_offload_set_t)
 	 uint8_t en);
 /**< @internal enable/disable the l2 tunnel offload functions */
 
+typedef int  (*eth_dev_reset_t)(struct rte_eth_dev *dev);
+/**< @internal Function used to reset a configured Ethernet device. */
+
 #ifdef RTE_NIC_BYPASS
 
 enum {
@@ -1509,6 +1512,9 @@ struct eth_dev_ops {
 	eth_l2_tunnel_offload_set_t   l2_tunnel_offload_set;
 	/** Enable/disable l2 tunnel offload functions. */
 
+	/** Reset device. */
+	eth_dev_reset_t dev_reset;
+
 	eth_set_queue_rate_limit_t set_queue_rate_limit; /**< Set queue rate limit. */
 
 	rss_hash_update_t          rss_hash_update; /** Configure RSS hash protocols. */
@@ -4413,6 +4419,25 @@ int
 rte_eth_dev_get_name_by_port(uint8_t port_id, char *name);
 
 /**
+ * Reset an ethernet device when it's not working. One scenario is, after PF
+ * port is down and up, the related VF port should be reset.
+ * The API will stop the port, clear the rx/tx queues, re-setup the rx/tx
+ * queues, restart the port.
+ * Before calling this API, APP should stop the rx/tx. When tx is being stopped,
+ * APP can drop the packets and release the buffer instead of sending them.
+ *
+ * @param port_id
+ *   The port identifier of the Ethernet device.
+ *
+ * @return
+ *   - (0) if successful.
+ *   - (-ENODEV) if port identifier is invalid.
+ *   - (-ENOTSUP) if hardware doesn't support this function.
+ */
+int
+rte_eth_dev_reset(uint8_t port_id);
+
+/**
  * @internal
  * Wrapper for use by pci drivers as a .probe function to attach to a ethdev
  * interface.
diff --git a/lib/librte_ether/rte_ether_version.map b/lib/librte_ether/rte_ether_version.map
index c6c9d0d..529b27f 100644
--- a/lib/librte_ether/rte_ether_version.map
+++ b/lib/librte_ether/rte_ether_version.map
@@ -154,3 +154,9 @@ DPDK_17.02 {
 	rte_flow_validate;
 
 } DPDK_16.11;
+
+DPDK_17.05 {
+	global:
+
+	rte_eth_dev_reset;
+} DPDK_17.02;
\ No newline at end of file
-- 
2.9.3

Re: [dpdk-dev] [PATCH v4 1/3] lib/librte_ether: add support for port reset

[Thomas Monjalon]

Hi,

Please help reviewers, use --in-reply-to to keep patches threaded.

2017-03-30 17:34, Wei Zhao:
> Add support for port reset in rte layer.This reset
> feature can not only used in vf port reset in later code
> develop, but alsopf port.But in this patch set, we only
> limit the discussion scope to vf reset.
> This patch Add an API to restart the device.
> It's for VF device in this scenario, kernel PF + DPDK VF.
> When the PF port down->up, APP should call this API to
> restart VF port. Most likely, APP should call it in its
> management thread and guarantee the thread safe. It means
> APP should stop the rx/tx and the device, then restart
> the device, then recover the device and rx/tx.
> Also, it's APP's responsibilty to release the occupied
> memory.

Which memory should be released?

[...]
  /**
> + * Reset an ethernet device when it's not working. One scenario is, after PF
> + * port is down and up, the related VF port should be reset.
> + * The API will stop the port, clear the rx/tx queues, re-setup the rx/tx
> + * queues, restart the port.

s/The API/This function/

Please explain exactly the responsibility of this function,
and how it is different from calling stop/configure/start.

> + * Before calling this API, APP should stop the rx/tx. When tx is being stopped,
> + * APP can drop the packets and release the buffer instead of sending them.
> + *
> + * @param port_id
> + *   The port identifier of the Ethernet device.
> + *
> + * @return
> + *   - (0) if successful.
> + *   - (-ENODEV) if port identifier is invalid.
> + *   - (-ENOTSUP) if hardware doesn't support this function.
> + */
> +int
> +rte_eth_dev_reset(uint8_t port_id);

Please John, could you help with the API documentation here?

Re: [dpdk-dev] [PATCH v4 1/3] lib/librte_ether: add support for port reset

[Zhao1, Wei]

Hi, Thomas

> -----Original Message-----
> From: Thomas Monjalon [mailto:thomas.monjalon@6wind.com]
> Sent: Friday, March 31, 2017 3:55 AM
> To: Zhao1, Wei <wei.zhao1@intel.com>; Mcnamara, John
> <john.mcnamara@intel.com>
> Cc: dev@dpdk.org; Lu, Wenzhuo <wenzhuo.lu@intel.com>
> Subject: Re: [dpdk-dev] [PATCH v4 1/3] lib/librte_ether: add support for port
> reset
> 
> Hi,
> 
> Please help reviewers, use --in-reply-to to keep patches threaded.
> 

Ok， I will use that use --in-reply-to in later when commit patch.

> 2017-03-30 17:34, Wei Zhao:
> > Add support for port reset in rte layer.This reset feature can not
> > only used in vf port reset in later code develop, but alsopf port.But
> > in this patch set, we only limit the discussion scope to vf reset.
> > This patch Add an API to restart the device.
> > It's for VF device in this scenario, kernel PF + DPDK VF.
> > When the PF port down->up, APP should call this API to restart VF
> > port. Most likely, APP should call it in its management thread and
> > guarantee the thread safe. It means APP should stop the rx/tx and the
> > device, then restart the device, then recover the device and rx/tx.
> > Also, it's APP's responsibilty to release the occupied memory.
> 
> Which memory should be released?

That is a redundancy now, because the older version need to allocation some memory for this feature, 
But it do not need now. So I will delete it in next version. 

> 
> [...]
>   /**
> > + * Reset an ethernet device when it's not working. One scenario is,
> > + after PF
> > + * port is down and up, the related VF port should be reset.
> > + * The API will stop the port, clear the rx/tx queues, re-setup the
> > + rx/tx
> > + * queues, restart the port.
> 
> s/The API/This function/
> 
> Please explain exactly the responsibility of this function, and how it is
> different from calling stop/configure/start.

In this reset feature, reset function can do the calling stop/configure/start process, but also 
It can also do some restore work for the port, for example, it can restore the added parameters 
 of vlan,  mac_addrs, promisc_unicast_enabled falg and promisc_multicast_enabled flag.
Maybe , I should add this explanation in the patch comments or function comments?


> 
> > + * Before calling this API, APP should stop the rx/tx. When tx is
> > +being stopped,
> > + * APP can drop the packets and release the buffer instead of sending
> them.
> > + *
> > + * @param port_id
> > + *   The port identifier of the Ethernet device.
> > + *
> > + * @return
> > + *   - (0) if successful.
> > + *   - (-ENODEV) if port identifier is invalid.
> > + *   - (-ENOTSUP) if hardware doesn't support this function.
> > + */
> > +int
> > +rte_eth_dev_reset(uint8_t port_id);
> 
> Please John, could you help with the API documentation here?

Thank you for John, please contact me if any  puzzle.

Re: [dpdk-dev] [PATCH v4 1/3] lib/librte_ether: add support for port reset

[Thomas Monjalon]

2017-04-06 02:57, Zhao1, Wei:
> >   /**
> > > + * Reset an ethernet device when it's not working. One scenario is,
> > > + after PF
> > > + * port is down and up, the related VF port should be reset.
> > > + * The API will stop the port, clear the rx/tx queues, re-setup the
> > > + rx/tx
> > > + * queues, restart the port.
> > 
> > s/The API/This function/
> > 
> > Please explain exactly the responsibility of this function, and how it is
> > different from calling stop/configure/start.
> 
> In this reset feature, reset function can do the calling stop/configure/start process, but also 
> It can also do some restore work for the port, for example, it can restore the added parameters 
>  of vlan,  mac_addrs, promisc_unicast_enabled falg and promisc_multicast_enabled flag.
> Maybe , I should add this explanation in the patch comments or function comments?

Yes it must be explain in the doxygen part of the function.

Re: [dpdk-dev] [PATCH v4 1/3] lib/librte_ether: add support for port reset

[Zhao1, Wei]

Hi, Thomas

> -----Original Message-----
> From: Thomas Monjalon [mailto:thomas.monjalon@6wind.com]
> Sent: Thursday, April 6, 2017 3:11 PM
> To: Zhao1, Wei <wei.zhao1@intel.com>
> Cc: Mcnamara, John <john.mcnamara@intel.com>; dev@dpdk.org; Lu,
> Wenzhuo <wenzhuo.lu@intel.com>
> Subject: Re: [dpdk-dev] [PATCH v4 1/3] lib/librte_ether: add support for port
> reset
> 
> 2017-04-06 02:57, Zhao1, Wei:
> > >   /**
> > > > + * Reset an ethernet device when it's not working. One scenario
> > > > + is, after PF
> > > > + * port is down and up, the related VF port should be reset.
> > > > + * The API will stop the port, clear the rx/tx queues, re-setup
> > > > + the rx/tx
> > > > + * queues, restart the port.
> > >
> > > s/The API/This function/
> > >
> > > Please explain exactly the responsibility of this function, and how
> > > it is different from calling stop/configure/start.
> >
> > In this reset feature, reset function can do the calling
> > stop/configure/start process, but also It can also do some restore
> > work for the port, for example, it can restore the added parameters  of
> vlan,  mac_addrs, promisc_unicast_enabled falg and
> promisc_multicast_enabled flag.
> > Maybe , I should add this explanation in the patch comments or function
> comments?
> 
> Yes it must be explain in the doxygen part of the function.

Yes, I have add that explanation in v5 which has been commit to dpdk.org. 

Re: [dpdk-dev] [PATCH v4 1/3] lib/librte_ether: add support for port reset

[Ananyev, Konstantin]

> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Zhao1, Wei
> Sent: Thursday, April 6, 2017 9:53 AM
> To: Thomas Monjalon <thomas.monjalon@6wind.com>
> Cc: Mcnamara, John <john.mcnamara@intel.com>; dev@dpdk.org; Lu, Wenzhuo <wenzhuo.lu@intel.com>
> Subject: Re: [dpdk-dev] [PATCH v4 1/3] lib/librte_ether: add support for port reset
> 
> Hi, Thomas
> 
> > -----Original Message-----
> > From: Thomas Monjalon [mailto:thomas.monjalon@6wind.com]
> > Sent: Thursday, April 6, 2017 3:11 PM
> > To: Zhao1, Wei <wei.zhao1@intel.com>
> > Cc: Mcnamara, John <john.mcnamara@intel.com>; dev@dpdk.org; Lu,
> > Wenzhuo <wenzhuo.lu@intel.com>
> > Subject: Re: [dpdk-dev] [PATCH v4 1/3] lib/librte_ether: add support for port
> > reset
> >
> > 2017-04-06 02:57, Zhao1, Wei:
> > > >   /**
> > > > > + * Reset an ethernet device when it's not working. One scenario
> > > > > + is, after PF
> > > > > + * port is down and up, the related VF port should be reset.
> > > > > + * The API will stop the port, clear the rx/tx queues, re-setup
> > > > > + the rx/tx
> > > > > + * queues, restart the port.
> > > >
> > > > s/The API/This function/
> > > >
> > > > Please explain exactly the responsibility of this function, and how
> > > > it is different from calling stop/configure/start.
> > >
> > > In this reset feature, reset function can do the calling
> > > stop/configure/start process, but also It can also do some restore
> > > work for the port, for example, it can restore the added parameters  of
> > vlan,  mac_addrs, promisc_unicast_enabled falg and
> > promisc_multicast_enabled flag.

Ok, but why start/stop can't do these things?
Konstantin

> > > Maybe , I should add this explanation in the patch comments or function
> > comments?
> >
> > Yes it must be explain in the doxygen part of the function.
> 
> Yes, I have add that explanation in v5 which has been commit to dpdk.org.

Re: [dpdk-dev] [PATCH v4 1/3] lib/librte_ether: add support for port reset

[Thomas Monjalon]

2017-04-06 09:02, Ananyev, Konstantin:
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Zhao1, Wei
> > From: Thomas Monjalon [mailto:thomas.monjalon@6wind.com]
> > > 2017-04-06 02:57, Zhao1, Wei:
> > > > >   /**
> > > > > > + * Reset an ethernet device when it's not working. One scenario
> > > > > > + is, after PF
> > > > > > + * port is down and up, the related VF port should be reset.
> > > > > > + * The API will stop the port, clear the rx/tx queues, re-setup
> > > > > > + the rx/tx
> > > > > > + * queues, restart the port.
> > > > >
> > > > > s/The API/This function/
> > > > >
> > > > > Please explain exactly the responsibility of this function, and how
> > > > > it is different from calling stop/configure/start.
> > > >
> > > > In this reset feature, reset function can do the calling
> > > > stop/configure/start process, but also It can also do some restore
> > > > work for the port, for example, it can restore the added parameters  of
> > > vlan,  mac_addrs, promisc_unicast_enabled falg and
> > > promisc_multicast_enabled flag.
> 
> Ok, but why start/stop can't do these things?

Please could you try to answer this question?

We cannot accept v7 if there are some doubts remaining.

Re: [dpdk-dev] [PATCH v4 1/3] lib/librte_ether: add support for port reset

[Zhao1, Wei]

Hi,  Konstantin

> -----Original Message-----
> From: Ananyev, Konstantin
> Sent: Thursday, April 6, 2017 5:03 PM
> To: Zhao1, Wei <wei.zhao1@intel.com>; Thomas Monjalon
> <thomas.monjalon@6wind.com>
> Cc: Mcnamara, John <john.mcnamara@intel.com>; dev@dpdk.org; Lu,
> Wenzhuo <wenzhuo.lu@intel.com>
> Subject: RE: [dpdk-dev] [PATCH v4 1/3] lib/librte_ether: add support for port
> reset
> 
> 
> 
> > -----Original Message-----
> > From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Zhao1, Wei
> > Sent: Thursday, April 6, 2017 9:53 AM
> > To: Thomas Monjalon <thomas.monjalon@6wind.com>
> > Cc: Mcnamara, John <john.mcnamara@intel.com>; dev@dpdk.org; Lu,
> > Wenzhuo <wenzhuo.lu@intel.com>
> > Subject: Re: [dpdk-dev] [PATCH v4 1/3] lib/librte_ether: add support
> > for port reset
> >
> > Hi, Thomas
> >
> > > -----Original Message-----
> > > From: Thomas Monjalon [mailto:thomas.monjalon@6wind.com]
> > > Sent: Thursday, April 6, 2017 3:11 PM
> > > To: Zhao1, Wei <wei.zhao1@intel.com>
> > > Cc: Mcnamara, John <john.mcnamara@intel.com>; dev@dpdk.org; Lu,
> > > Wenzhuo <wenzhuo.lu@intel.com>
> > > Subject: Re: [dpdk-dev] [PATCH v4 1/3] lib/librte_ether: add support
> > > for port reset
> > >
> > > 2017-04-06 02:57, Zhao1, Wei:
> > > > >   /**
> > > > > > + * Reset an ethernet device when it's not working. One
> > > > > > + scenario is, after PF
> > > > > > + * port is down and up, the related VF port should be reset.
> > > > > > + * The API will stop the port, clear the rx/tx queues,
> > > > > > + re-setup the rx/tx
> > > > > > + * queues, restart the port.
> > > > >
> > > > > s/The API/This function/
> > > > >
> > > > > Please explain exactly the responsibility of this function, and
> > > > > how it is different from calling stop/configure/start.
> > > >
> > > > In this reset feature, reset function can do the calling
> > > > stop/configure/start process, but also It can also do some restore
> > > > work for the port, for example, it can restore the added
> > > > parameters  of
> > > vlan,  mac_addrs, promisc_unicast_enabled falg and
> > > promisc_multicast_enabled flag.
> 
> Ok, but why start/stop can't do these things?
> Konstantin

This is because in i40e PMD code, start and stop process do not have the process of store and restore
the added key parameters. Not  only i40e but also other PMD code. So, in the function pointed to by dev_reset,
we add specific function do store and restore of some of the important  parameters  listed above.


> 
> > > > Maybe , I should add this explanation in the patch comments or
> > > > function
> > > comments?
> > >
> > > Yes it must be explain in the doxygen part of the function.
> >
> > Yes, I have add that explanation in v5 which has been commit to dpdk.org.

Re: [dpdk-dev] [PATCH v4 1/3] lib/librte_ether: add support for port reset

[Thomas Monjalon]

2017-04-13 08:55, Zhao1, Wei:
> From: Ananyev, Konstantin
> > From: Zhao1, Wei
> > > From: Thomas Monjalon [mailto:thomas.monjalon@6wind.com]
> > > > 2017-04-06 02:57, Zhao1, Wei:
> > > > > >   /**
> > > > > > > + * Reset an ethernet device when it's not working. One
> > > > > > > + scenario is, after PF
> > > > > > > + * port is down and up, the related VF port should be reset.
> > > > > > > + * The API will stop the port, clear the rx/tx queues,
> > > > > > > + re-setup the rx/tx
> > > > > > > + * queues, restart the port.
> > > > > >
> > > > > > s/The API/This function/
> > > > > >
> > > > > > Please explain exactly the responsibility of this function, and
> > > > > > how it is different from calling stop/configure/start.
> > > > >
> > > > > In this reset feature, reset function can do the calling
> > > > > stop/configure/start process, but also It can also do some restore
> > > > > work for the port, for example, it can restore the added
> > > > > parameters  of
> > > > vlan,  mac_addrs, promisc_unicast_enabled falg and
> > > > promisc_multicast_enabled flag.
> > 
> > Ok, but why start/stop can't do these things?
> > Konstantin
> 
> This is because in i40e PMD code, start and stop process do not have the process of store and restore
> the added key parameters. Not  only i40e but also other PMD code. So, in the function pointed to by dev_reset,
> we add specific function do store and restore of some of the important  parameters  listed above.

Why store and restore cannot be implemented in start/stop functions?

Re: [dpdk-dev] [PATCH v4 1/3] lib/librte_ether: add support for port reset

[Zhao1, Wei]

Hi, Thomas Monjalon

> -----Original Message-----
> From: Thomas Monjalon [mailto:thomas.monjalon@6wind.com]
> Sent: Thursday, April 13, 2017 6:07 PM
> To: Zhao1, Wei <wei.zhao1@intel.com>
> Cc: Ananyev, Konstantin <konstantin.ananyev@intel.com>; Mcnamara, John
> <john.mcnamara@intel.com>; dev@dpdk.org; Lu, Wenzhuo
> <wenzhuo.lu@intel.com>
> Subject: Re: [dpdk-dev] [PATCH v4 1/3] lib/librte_ether: add support for port
> reset
> 
> 2017-04-13 08:55, Zhao1, Wei:
> > From: Ananyev, Konstantin
> > > From: Zhao1, Wei
> > > > From: Thomas Monjalon [mailto:thomas.monjalon@6wind.com]
> > > > > 2017-04-06 02:57, Zhao1, Wei:
> > > > > > >   /**
> > > > > > > > + * Reset an ethernet device when it's not working. One
> > > > > > > > + scenario is, after PF
> > > > > > > > + * port is down and up, the related VF port should be reset.
> > > > > > > > + * The API will stop the port, clear the rx/tx queues,
> > > > > > > > + re-setup the rx/tx
> > > > > > > > + * queues, restart the port.
> > > > > > >
> > > > > > > s/The API/This function/
> > > > > > >
> > > > > > > Please explain exactly the responsibility of this function,
> > > > > > > and how it is different from calling stop/configure/start.
> > > > > >
> > > > > > In this reset feature, reset function can do the calling
> > > > > > stop/configure/start process, but also It can also do some
> > > > > > restore work for the port, for example, it can restore the
> > > > > > added parameters  of
> > > > > vlan,  mac_addrs, promisc_unicast_enabled falg and
> > > > > promisc_multicast_enabled flag.
> > >
> > > Ok, but why start/stop can't do these things?
> > > Konstantin
> >
> > This is because in i40e PMD code, start and stop process do not have
> > the process of store and restore the added key parameters. Not  only
> > i40e but also other PMD code. So, in the function pointed to by dev_reset,
> we add specific function do store and restore of some of the important
> parameters  listed above.
> 
> Why store and restore cannot be implemented in start/stop functions?

Because reset and  start/stop are used for two purposes,  for example:
Some user maybe just start/stop the port  and he do not care what key parameters
has been configuration last time, and even worse when he want to clear all the configuration last time ,
if we add specific function do store and restore in  that two function, it is useless for them,
and may cause a result that user do not expect.

Re: [dpdk-dev] [PATCH v4 1/3] lib/librte_ether: add support for port reset

[Thomas Monjalon]

2017-04-14 01:29, Zhao1, Wei:
> From: Thomas Monjalon [mailto:thomas.monjalon@6wind.com]
> > 2017-04-13 08:55, Zhao1, Wei:
> > > From: Ananyev, Konstantin
> > > > From: Zhao1, Wei
> > > > > From: Thomas Monjalon [mailto:thomas.monjalon@6wind.com]
> > > > > > 2017-04-06 02:57, Zhao1, Wei:
> > > > > > > >   /**
> > > > > > > > > + * Reset an ethernet device when it's not working. One
> > > > > > > > > + scenario is, after PF
> > > > > > > > > + * port is down and up, the related VF port should be reset.
> > > > > > > > > + * The API will stop the port, clear the rx/tx queues,
> > > > > > > > > + re-setup the rx/tx
> > > > > > > > > + * queues, restart the port.
> > > > > > > >
> > > > > > > > s/The API/This function/
> > > > > > > >
> > > > > > > > Please explain exactly the responsibility of this function,
> > > > > > > > and how it is different from calling stop/configure/start.
> > > > > > >
> > > > > > > In this reset feature, reset function can do the calling
> > > > > > > stop/configure/start process, but also It can also do some
> > > > > > > restore work for the port, for example, it can restore the
> > > > > > > added parameters  of
> > > > > > vlan,  mac_addrs, promisc_unicast_enabled falg and
> > > > > > promisc_multicast_enabled flag.
> > > >
> > > > Ok, but why start/stop can't do these things?
> > > > Konstantin
> > >
> > > This is because in i40e PMD code, start and stop process do not have
> > > the process of store and restore the added key parameters. Not  only
> > > i40e but also other PMD code. So, in the function pointed to by dev_reset,
> > we add specific function do store and restore of some of the important
> > parameters  listed above.
> > 
> > Why store and restore cannot be implemented in start/stop functions?
> 
> Because reset and  start/stop are used for two purposes,  for example:
> Some user maybe just start/stop the port  and he do not care what key parameters
> has been configuration last time, and even worse when he want to clear all the configuration last time ,
> if we add specific function do store and restore in  that two function, it is useless for them,
> and may cause a result that user do not expect.

Is it said somewhere in the doc that the configuration is lost when
stopping a device?
Can we say which configuration parameter is kept and which one is lost?

rte_eth_dev_config_restore() is called in rte_eth_dev_start().

Re: [dpdk-dev] [PATCH v4 1/3] lib/librte_ether: add support for port reset

[Zhao1, Wei]

Hi, Thomas

> -----Original Message-----
> From: Thomas Monjalon [mailto:thomas.monjalon@6wind.com]
> Sent: Friday, April 14, 2017 2:31 PM
> To: Zhao1, Wei <wei.zhao1@intel.com>
> Cc: Ananyev, Konstantin <konstantin.ananyev@intel.com>; Mcnamara, John
> <john.mcnamara@intel.com>; dev@dpdk.org; Lu, Wenzhuo
> <wenzhuo.lu@intel.com>
> Subject: Re: [dpdk-dev] [PATCH v4 1/3] lib/librte_ether: add support for port
> reset
> 
> 2017-04-14 01:29, Zhao1, Wei:
> > From: Thomas Monjalon [mailto:thomas.monjalon@6wind.com]
> > > 2017-04-13 08:55, Zhao1, Wei:
> > > > From: Ananyev, Konstantin
> > > > > From: Zhao1, Wei
> > > > > > From: Thomas Monjalon [mailto:thomas.monjalon@6wind.com]
> > > > > > > 2017-04-06 02:57, Zhao1, Wei:
> > > > > > > > >   /**
> > > > > > > > > > + * Reset an ethernet device when it's not working.
> > > > > > > > > > + One scenario is, after PF
> > > > > > > > > > + * port is down and up, the related VF port should be reset.
> > > > > > > > > > + * The API will stop the port, clear the rx/tx
> > > > > > > > > > + queues, re-setup the rx/tx
> > > > > > > > > > + * queues, restart the port.
> > > > > > > > >
> > > > > > > > > s/The API/This function/
> > > > > > > > >
> > > > > > > > > Please explain exactly the responsibility of this
> > > > > > > > > function, and how it is different from calling
> stop/configure/start.
> > > > > > > >
> > > > > > > > In this reset feature, reset function can do the calling
> > > > > > > > stop/configure/start process, but also It can also do some
> > > > > > > > restore work for the port, for example, it can restore the
> > > > > > > > added parameters  of
> > > > > > > vlan,  mac_addrs, promisc_unicast_enabled falg and
> > > > > > > promisc_multicast_enabled flag.
> > > > >
> > > > > Ok, but why start/stop can't do these things?
> > > > > Konstantin
> > > >
> > > > This is because in i40e PMD code, start and stop process do not
> > > > have the process of store and restore the added key parameters.
> > > > Not  only i40e but also other PMD code. So, in the function
> > > > pointed to by dev_reset,
> > > we add specific function do store and restore of some of the
> > > important parameters  listed above.
> > >
> > > Why store and restore cannot be implemented in start/stop functions?
> >
> > Because reset and  start/stop are used for two purposes,  for example:
> > Some user maybe just start/stop the port  and he do not care what key
> > parameters has been configuration last time, and even worse when he
> > want to clear all the configuration last time , if we add specific
> > function do store and restore in  that two function, it is useless for them,
> and may cause a result that user do not expect.
> 
> Is it said somewhere in the doc that the configuration is lost when stopping a
> device?
> Can we say which configuration parameter is kept and which one is lost?
> 
> rte_eth_dev_config_restore() is called in rte_eth_dev_start().

Port reset process not only involve the rte_eth_dev_start() and rte_eth_dev_stop().
It also involve eth_dev_uninit() and eth_dev_init() process，
in which PMD device uninit and init. In this case, for example, data->mac_addrs buffer is freed so it need to add store and restore function.
BUT, if you only call stop/configure/start process, that is not strictly what named "device reset".

Re: [dpdk-dev] [PATCH v4 1/3] lib/librte_ether: add support for port reset

[Zhao1, Wei]

Hi, Thomas

    All questions about this patch set has been answered, is it clear or not?And is there anything that I should do for it?
Or it is OK for merge into 17.02-RC2?

Thank you.

> -----Original Message-----
> From: Zhao1, Wei
> Sent: Friday, April 14, 2017 4:03 PM
> To: Thomas Monjalon <thomas.monjalon@6wind.com>
> Cc: Ananyev, Konstantin <konstantin.ananyev@intel.com>; Mcnamara, John
> <john.mcnamara@intel.com>; dev@dpdk.org; Lu, Wenzhuo
> <wenzhuo.lu@intel.com>
> Subject: RE: [dpdk-dev] [PATCH v4 1/3] lib/librte_ether: add support for port
> reset
> 
> Hi, Thomas
> 
> > -----Original Message-----
> > From: Thomas Monjalon [mailto:thomas.monjalon@6wind.com]
> > Sent: Friday, April 14, 2017 2:31 PM
> > To: Zhao1, Wei <wei.zhao1@intel.com>
> > Cc: Ananyev, Konstantin <konstantin.ananyev@intel.com>; Mcnamara,
> John
> > <john.mcnamara@intel.com>; dev@dpdk.org; Lu, Wenzhuo
> > <wenzhuo.lu@intel.com>
> > Subject: Re: [dpdk-dev] [PATCH v4 1/3] lib/librte_ether: add support
> > for port reset
> >
> > 2017-04-14 01:29, Zhao1, Wei:
> > > From: Thomas Monjalon [mailto:thomas.monjalon@6wind.com]
> > > > 2017-04-13 08:55, Zhao1, Wei:
> > > > > From: Ananyev, Konstantin
> > > > > > From: Zhao1, Wei
> > > > > > > From: Thomas Monjalon [mailto:thomas.monjalon@6wind.com]
> > > > > > > > 2017-04-06 02:57, Zhao1, Wei:
> > > > > > > > > >   /**
> > > > > > > > > > > + * Reset an ethernet device when it's not working.
> > > > > > > > > > > + One scenario is, after PF
> > > > > > > > > > > + * port is down and up, the related VF port should be
> reset.
> > > > > > > > > > > + * The API will stop the port, clear the rx/tx
> > > > > > > > > > > + queues, re-setup the rx/tx
> > > > > > > > > > > + * queues, restart the port.
> > > > > > > > > >
> > > > > > > > > > s/The API/This function/
> > > > > > > > > >
> > > > > > > > > > Please explain exactly the responsibility of this
> > > > > > > > > > function, and how it is different from calling
> > stop/configure/start.
> > > > > > > > >
> > > > > > > > > In this reset feature, reset function can do the calling
> > > > > > > > > stop/configure/start process, but also It can also do
> > > > > > > > > some restore work for the port, for example, it can
> > > > > > > > > restore the added parameters  of
> > > > > > > > vlan,  mac_addrs, promisc_unicast_enabled falg and
> > > > > > > > promisc_multicast_enabled flag.
> > > > > >
> > > > > > Ok, but why start/stop can't do these things?
> > > > > > Konstantin
> > > > >
> > > > > This is because in i40e PMD code, start and stop process do not
> > > > > have the process of store and restore the added key parameters.
> > > > > Not  only i40e but also other PMD code. So, in the function
> > > > > pointed to by dev_reset,
> > > > we add specific function do store and restore of some of the
> > > > important parameters  listed above.
> > > >
> > > > Why store and restore cannot be implemented in start/stop functions?
> > >
> > > Because reset and  start/stop are used for two purposes,  for example:
> > > Some user maybe just start/stop the port  and he do not care what
> > > key parameters has been configuration last time, and even worse when
> > > he want to clear all the configuration last time , if we add
> > > specific function do store and restore in  that two function, it is
> > > useless for them,
> > and may cause a result that user do not expect.
> >
> > Is it said somewhere in the doc that the configuration is lost when
> > stopping a device?
> > Can we say which configuration parameter is kept and which one is lost?
> >
> > rte_eth_dev_config_restore() is called in rte_eth_dev_start().
> 
> Port reset process not only involve the rte_eth_dev_start() and
> rte_eth_dev_stop().
> It also involve eth_dev_uninit() and eth_dev_init() process，
> in which PMD device uninit and init. In this case, for example, data-
> >mac_addrs buffer is freed so it need to add store and restore function.
> BUT, if you only call stop/configure/start process, that is not strictly what
> named "device reset".

Re: [dpdk-dev] [PATCH v4 1/3] lib/librte_ether: add support for port reset

[Zhao1, Wei]

Add Thomas new mail address for this mail.

> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Zhao1, Wei
> Sent: Monday, April 17, 2017 10:09 AM
> To: 'Thomas Monjalon' <thomas.monjalon@6wind.com>
> Cc: Ananyev, Konstantin <konstantin.ananyev@intel.com>; Mcnamara, John
> <john.mcnamara@intel.com>; 'dev@dpdk.org' <dev@dpdk.org>; Lu,
> Wenzhuo <wenzhuo.lu@intel.com>
> Subject: Re: [dpdk-dev] [PATCH v4 1/3] lib/librte_ether: add support for port
> reset
> 
> Hi, Thomas
> 
>     All questions about this patch set has been answered, is it clear or not?And
> is there anything that I should do for it?
> Or it is OK for merge into 17.02-RC2?
> 
> Thank you.
> 
> > -----Original Message-----
> > From: Zhao1, Wei
> > Sent: Friday, April 14, 2017 4:03 PM
> > To: Thomas Monjalon <thomas.monjalon@6wind.com>
> > Cc: Ananyev, Konstantin <konstantin.ananyev@intel.com>; Mcnamara,
> John
> > <john.mcnamara@intel.com>; dev@dpdk.org; Lu, Wenzhuo
> > <wenzhuo.lu@intel.com>
> > Subject: RE: [dpdk-dev] [PATCH v4 1/3] lib/librte_ether: add support
> > for port reset
> >
> > Hi, Thomas
> >
> > > -----Original Message-----
> > > From: Thomas Monjalon [mailto:thomas.monjalon@6wind.com]
> > > Sent: Friday, April 14, 2017 2:31 PM
> > > To: Zhao1, Wei <wei.zhao1@intel.com>
> > > Cc: Ananyev, Konstantin <konstantin.ananyev@intel.com>; Mcnamara,
> > John
> > > <john.mcnamara@intel.com>; dev@dpdk.org; Lu, Wenzhuo
> > > <wenzhuo.lu@intel.com>
> > > Subject: Re: [dpdk-dev] [PATCH v4 1/3] lib/librte_ether: add support
> > > for port reset
> > >
> > > 2017-04-14 01:29, Zhao1, Wei:
> > > > From: Thomas Monjalon [mailto:thomas.monjalon@6wind.com]
> > > > > 2017-04-13 08:55, Zhao1, Wei:
> > > > > > From: Ananyev, Konstantin
> > > > > > > From: Zhao1, Wei
> > > > > > > > From: Thomas Monjalon [mailto:thomas.monjalon@6wind.com]
> > > > > > > > > 2017-04-06 02:57, Zhao1, Wei:
> > > > > > > > > > >   /**
> > > > > > > > > > > > + * Reset an ethernet device when it's not working.
> > > > > > > > > > > > + One scenario is, after PF
> > > > > > > > > > > > + * port is down and up, the related VF port
> > > > > > > > > > > > + should be
> > reset.
> > > > > > > > > > > > + * The API will stop the port, clear the rx/tx
> > > > > > > > > > > > + queues, re-setup the rx/tx
> > > > > > > > > > > > + * queues, restart the port.
> > > > > > > > > > >
> > > > > > > > > > > s/The API/This function/
> > > > > > > > > > >
> > > > > > > > > > > Please explain exactly the responsibility of this
> > > > > > > > > > > function, and how it is different from calling
> > > stop/configure/start.
> > > > > > > > > >
> > > > > > > > > > In this reset feature, reset function can do the
> > > > > > > > > > calling stop/configure/start process, but also It can
> > > > > > > > > > also do some restore work for the port, for example,
> > > > > > > > > > it can restore the added parameters  of
> > > > > > > > > vlan,  mac_addrs, promisc_unicast_enabled falg and
> > > > > > > > > promisc_multicast_enabled flag.
> > > > > > >
> > > > > > > Ok, but why start/stop can't do these things?
> > > > > > > Konstantin
> > > > > >
> > > > > > This is because in i40e PMD code, start and stop process do
> > > > > > not have the process of store and restore the added key
> parameters.
> > > > > > Not  only i40e but also other PMD code. So, in the function
> > > > > > pointed to by dev_reset,
> > > > > we add specific function do store and restore of some of the
> > > > > important parameters  listed above.
> > > > >
> > > > > Why store and restore cannot be implemented in start/stop functions?
> > > >
> > > > Because reset and  start/stop are used for two purposes,  for example:
> > > > Some user maybe just start/stop the port  and he do not care what
> > > > key parameters has been configuration last time, and even worse
> > > > when he want to clear all the configuration last time , if we add
> > > > specific function do store and restore in  that two function, it
> > > > is useless for them,
> > > and may cause a result that user do not expect.
> > >
> > > Is it said somewhere in the doc that the configuration is lost when
> > > stopping a device?
> > > Can we say which configuration parameter is kept and which one is lost?
> > >
> > > rte_eth_dev_config_restore() is called in rte_eth_dev_start().
> >
> > Port reset process not only involve the rte_eth_dev_start() and
> > rte_eth_dev_stop().
> > It also involve eth_dev_uninit() and eth_dev_init() process，
> > in which PMD device uninit and init. In this case, for example, data-
> > >mac_addrs buffer is freed so it need to add store and restore function.
> > BUT, if you only call stop/configure/start process, that is not
> > strictly what named "device reset".

Re: [dpdk-dev] [PATCH v4 1/3] lib/librte_ether: add support for port reset

[Yuanhan Liu]

On Thu, Apr 06, 2017 at 02:57:29AM +0000, Zhao1, Wei wrote:
> > > + * Reset an ethernet device when it's not working. One scenario is,
> > > + after PF
> > > + * port is down and up, the related VF port should be reset.
> > > + * The API will stop the port, clear the rx/tx queues, re-setup the
> > > + rx/tx
> > > + * queues, restart the port.
> > 
> > s/The API/This function/
> > 
> > Please explain exactly the responsibility of this function, and how it is
> > different from calling stop/configure/start.
> 
> In this reset feature, reset function can do the calling stop/configure/start process, but also 
> It can also do some restore work for the port, for example, it can restore the added parameters 
>  of vlan,  mac_addrs, promisc_unicast_enabled falg and promisc_multicast_enabled flag.
> Maybe , I should add this explanation in the patch comments or function comments?

I'm curious why we have to do save & restore for a reset operation.
Why some configures have to be saved and restored? Doesn't "reset"
literally means reset everything?

Even though, how do you tell what kind of configures need be restored
and what should not? Again, even though, will all PMDs supports
restoring the same set of configurations?

While looking at your reset implementation for i40e, it looks more
complex than necessary: just thinking we have to call "xxx_queue_setup"
for all PMDs.

I'm thinking a simple hardware reset might be enough?

    /* literally reset the hardware: reset everything */
    rte_eth_reset(port) 
    {
    	eth_dev->ops->reset();
    }

Assume the application already has a function (say, port_init()) to
initiate a specific port, it then just needs do something like following
to handle the case you described in the commit log:

    rte_eth_reset(port);
    port_init(port);

Makes sense? Sorry it's completely wrong; I've limited knowledge on NIC
pmd drivers after all :/

	--yliu

Re: [dpdk-dev] [PATCH v4 1/3] lib/librte_ether: add support for port reset

[Zhao1, Wei]

Hi, Yuanhan

> -----Original Message-----
> From: Yuanhan Liu [mailto:yuanhan.liu@linux.intel.com]
> Sent: Thursday, April 20, 2017 2:08 PM
> To: Zhao1, Wei <wei.zhao1@intel.com>
> Cc: Ananyev, Konstantin <konstantin.ananyev@intel.com>; Mcnamara, John
> <john.mcnamara@intel.com>; dev@dpdk.org; Lu, Wenzhuo
> <wenzhuo.lu@intel.com>; Thomas Monjalon <thomas@monjalon.net>; Liu,
> Yu Y <yu.y.liu@intel.com>
> Subject: Re: [dpdk-dev] [PATCH v4 1/3] lib/librte_ether: add support for port
> reset
> 
> On Thu, Apr 06, 2017 at 02:57:29AM +0000, Zhao1, Wei wrote:
> > > > + * Reset an ethernet device when it's not working. One scenario
> > > > + is, after PF
> > > > + * port is down and up, the related VF port should be reset.
> > > > + * The API will stop the port, clear the rx/tx queues, re-setup
> > > > + the rx/tx
> > > > + * queues, restart the port.
> > >
> > > s/The API/This function/
> > >
> > > Please explain exactly the responsibility of this function, and how
> > > it is different from calling stop/configure/start.
> >
> > In this reset feature, reset function can do the calling
> > stop/configure/start process, but also It can also do some restore
> > work for the port, for example, it can restore the added parameters  of
> vlan,  mac_addrs, promisc_unicast_enabled falg and
> promisc_multicast_enabled flag.
> > Maybe , I should add this explanation in the patch comments or function
> comments?
> 
> I'm curious why we have to do save & restore for a reset operation.
> Why some configures have to be saved and restored? Doesn't "reset"
> literally means reset everything?
> 

Users maybe do not want to do a second configuration operation to waste time after reset which lost all previous configuration.
But he still want these configuration valid after reset.
So, save & restore can help them to save this process time and effort.

> Even though, how do you tell what kind of configures need be restored and
> what should not? Again, even though, will all PMDs supports restoring the
> same set of configurations?
> 

Yes, this is hard to say what may be need and what may be not for user.
Now, the kinds of supported is list in patch set comment. And only i40e NIC support this feature.

> While looking at your reset implementation for i40e, it looks more complex
> than necessary: just thinking we have to call "xxx_queue_setup"
> for all PMDs.
> 
> I'm thinking a simple hardware reset might be enough?
> 
>     /* literally reset the hardware: reset everything */
>     rte_eth_reset(port)
>     {
>     	eth_dev->ops->reset();
>     }
> 

You mean just do a reset  and do not restore any configuration?
That may not meet the need for this feature from customer?

> Assume the application already has a function (say, port_init()) to initiate a
> specific port, it then just needs do something like following to handle the
> case you described in the commit log:
> 
>     rte_eth_reset(port);
>     port_init(port);
> 

You mean "rte_eth_reset" is the function of "rte_eth_dev_reset"?
I do not find any function named rte_eth_reset.

> Makes sense? Sorry it's completely wrong; I've limited knowledge on NIC
> pmd drivers after all :/
> 
> 	--yliu

Re: [dpdk-dev] [PATCH v4 1/3] lib/librte_ether: add support for port reset

[Yuanhan Liu]

On Thu, Apr 20, 2017 at 09:17:24AM +0000, Zhao1, Wei wrote:
> > > > Please explain exactly the responsibility of this function, and how
> > > > it is different from calling stop/configure/start.
> > >
> > > In this reset feature, reset function can do the calling
> > > stop/configure/start process, but also It can also do some restore
> > > work for the port, for example, it can restore the added parameters  of
> > vlan,  mac_addrs, promisc_unicast_enabled falg and
> > promisc_multicast_enabled flag.
> > > Maybe , I should add this explanation in the patch comments or function
> > comments?
> > 
> > I'm curious why we have to do save & restore for a reset operation.
> > Why some configures have to be saved and restored? Doesn't "reset"
> > literally means reset everything?
> > 
> 
> Users maybe do not want to do a second configuration operation to waste time after reset which lost all previous configuration.
> But he still want these configuration valid after reset.
> So, save & restore can help them to save this process time and effort.
>
> > Even though, how do you tell what kind of configures need be restored and
> > what should not? Again, even though, will all PMDs supports restoring the
> > same set of configurations?
> > 
> 
> Yes, this is hard to say what may be need and what may be not for user.
> Now, the kinds of supported is list in patch set comment. And only i40e NIC support this feature.

Why it's the configurations listed in patch 2? Because they are requested
by customers?

Is that all could be saved? If not, are you going to save & restore all
possible configurations?

Assuming the configurations saved & restored may differ from different
PMD drivers, how could you keep the consistency then? And judging that the
application has no idea about the difference (yet it knows nothing about
what kind of configurations will be saved), how could the application know
that some configurations are not saved & restored by the driver that it
has to do re-configuration by itself?

> > While looking at your reset implementation for i40e, it looks more complex
> > than necessary: just thinking we have to call "xxx_queue_setup"
> > for all PMDs.
> > 
> > I'm thinking a simple hardware reset might be enough?
> > 
> >     /* literally reset the hardware: reset everything */
> >     rte_eth_reset(port)
> >     {
> >     	eth_dev->ops->reset();
> >     }
> > 
> 
> You mean just do a reset  and do not restore any configuration?
> That may not meet the need for this feature from customer?

Right, I'm just aware of the configuration might be done by PF (but not
only by the application), that the VF port may be not aware of those
configurations. So the save & restore is needed. I don't quite like
how it is done in this patch set though. I also don't think the API is
well named: as said, reset should literally reset everything.

We may need think how to do it properly.

Thomas, Konstantin, what do you guys think of it?

	--yliu

Re: [dpdk-dev] [PATCH v4 1/3] lib/librte_ether: add support for port reset

[Thomas Monjalon]

21/04/2017 04:27, Yuanhan Liu:
> On Thu, Apr 20, 2017 at 09:17:24AM +0000, Zhao1, Wei wrote:
> > > > > Please explain exactly the responsibility of this function, and how
> > > > > it is different from calling stop/configure/start.
> > > > 
> > > > In this reset feature, reset function can do the calling
> > > > stop/configure/start process, but also It can also do some restore
> > > > work for the port, for example, it can restore the added parameters 
> > > > of
> > > 
> > > vlan,  mac_addrs, promisc_unicast_enabled falg and
> > > promisc_multicast_enabled flag.
> > > 
> > > > Maybe , I should add this explanation in the patch comments or
> > > > function
> > > 
> > > comments?
> > > 
> > > I'm curious why we have to do save & restore for a reset operation.
> > > Why some configures have to be saved and restored? Doesn't "reset"
> > > literally means reset everything?
> > 
> > Users maybe do not want to do a second configuration operation to waste
> > time after reset which lost all previous configuration. But he still want
> > these configuration valid after reset.
> > So, save & restore can help them to save this process time and effort.
> > 
> > > Even though, how do you tell what kind of configures need be restored
> > > and
> > > what should not? Again, even though, will all PMDs supports restoring
> > > the
> > > same set of configurations?
> > 
> > Yes, this is hard to say what may be need and what may be not for user.
> > Now, the kinds of supported is list in patch set comment. And only i40e
> > NIC support this feature.
> Why it's the configurations listed in patch 2? Because they are requested
> by customers?
> 
> Is that all could be saved? If not, are you going to save & restore all
> possible configurations?
> 
> Assuming the configurations saved & restored may differ from different
> PMD drivers, how could you keep the consistency then? And judging that the
> application has no idea about the difference (yet it knows nothing about
> what kind of configurations will be saved), how could the application know
> that some configurations are not saved & restored by the driver that it
> has to do re-configuration by itself?
> 
> > > While looking at your reset implementation for i40e, it looks more
> > > complex
> > > than necessary: just thinking we have to call "xxx_queue_setup"
> > > for all PMDs.
> > > 
> > > I'm thinking a simple hardware reset might be enough?
> > > 
> > >     /* literally reset the hardware: reset everything */
> > >     rte_eth_reset(port)
> > >     {
> > >     
> > >     	eth_dev->ops->reset();
> > >     
> > >     }
> > 
> > You mean just do a reset  and do not restore any configuration?
> > That may not meet the need for this feature from customer?
> 
> Right, I'm just aware of the configuration might be done by PF (but not
> only by the application), that the VF port may be not aware of those
> configurations. So the save & restore is needed. I don't quite like
> how it is done in this patch set though. I also don't think the API is
> well named: as said, reset should literally reset everything.
> 
> We may need think how to do it properly.
> 
> Thomas, Konstantin, what do you guys think of it?

I have the same concerns.
I think we should better document the current status of start/stop
and which configuration parameters are lost or saved.

Re: [dpdk-dev] [PATCH v4 1/3] lib/librte_ether: add support for port reset

[Zhao1, Wei]

Hi, thomas

> -----Original Message-----
> From: Thomas Monjalon [mailto:thomas@monjalon.net]
> Sent: Friday, April 21, 2017 4:28 PM
> To: Yuanhan Liu <yuanhan.liu@linux.intel.com>; Zhao1, Wei
> <wei.zhao1@intel.com>
> Cc: Ananyev, Konstantin <konstantin.ananyev@intel.com>; Mcnamara, John
> <john.mcnamara@intel.com>; dev@dpdk.org; Lu, Wenzhuo
> <wenzhuo.lu@intel.com>; Liu, Yu Y <yu.y.liu@intel.com>
> Subject: Re: [dpdk-dev] [PATCH v4 1/3] lib/librte_ether: add support for port
> reset
> 
> 21/04/2017 04:27, Yuanhan Liu:
> > On Thu, Apr 20, 2017 at 09:17:24AM +0000, Zhao1, Wei wrote:
> > > > > > Please explain exactly the responsibility of this function,
> > > > > > and how it is different from calling stop/configure/start.
> > > > >
> > > > > In this reset feature, reset function can do the calling
> > > > > stop/configure/start process, but also It can also do some
> > > > > restore work for the port, for example, it can restore the added
> > > > > parameters of
> > > >
> > > > vlan,  mac_addrs, promisc_unicast_enabled falg and
> > > > promisc_multicast_enabled flag.
> > > >
> > > > > Maybe , I should add this explanation in the patch comments or
> > > > > function
> > > >
> > > > comments?
> > > >
> > > > I'm curious why we have to do save & restore for a reset operation.
> > > > Why some configures have to be saved and restored? Doesn't "reset"
> > > > literally means reset everything?
> > >
> > > Users maybe do not want to do a second configuration operation to
> > > waste time after reset which lost all previous configuration. But he
> > > still want these configuration valid after reset.
> > > So, save & restore can help them to save this process time and effort.
> > >
> > > > Even though, how do you tell what kind of configures need be
> > > > restored and what should not? Again, even though, will all PMDs
> > > > supports restoring the same set of configurations?
> > >
> > > Yes, this is hard to say what may be need and what may be not for user.
> > > Now, the kinds of supported is list in patch set comment. And only
> > > i40e NIC support this feature.
> > Why it's the configurations listed in patch 2? Because they are
> > requested by customers?
> >
> > Is that all could be saved? If not, are you going to save & restore
> > all possible configurations?
> >
> > Assuming the configurations saved & restored may differ from different
> > PMD drivers, how could you keep the consistency then? And judging that
> > the application has no idea about the difference (yet it knows nothing
> > about what kind of configurations will be saved), how could the
> > application know that some configurations are not saved & restored by
> > the driver that it has to do re-configuration by itself?
> >
> > > > While looking at your reset implementation for i40e, it looks more
> > > > complex than necessary: just thinking we have to call
> > > > "xxx_queue_setup"
> > > > for all PMDs.
> > > >
> > > > I'm thinking a simple hardware reset might be enough?
> > > >
> > > >     /* literally reset the hardware: reset everything */
> > > >     rte_eth_reset(port)
> > > >     {
> > > >
> > > >     	eth_dev->ops->reset();
> > > >
> > > >     }
> > >
> > > You mean just do a reset  and do not restore any configuration?
> > > That may not meet the need for this feature from customer?
> >
> > Right, I'm just aware of the configuration might be done by PF (but
> > not only by the application), that the VF port may be not aware of
> > those configurations. So the save & restore is needed. I don't quite
> > like how it is done in this patch set though. I also don't think the
> > API is well named: as said, reset should literally reset everything.
> >
> > We may need think how to do it properly.
> >
> > Thomas, Konstantin, what do you guys think of it?
> 
> I have the same concerns.
> I think we should better document the current status of start/stop and which
> configuration parameters are lost or saved.


Maybe I should add some words in doc\guides\nics\i40e.rst to Record which configurations are  saved  and restored by the PMD driver in reset function.
Which not list in that are recognized as not saved  and restored  by default.
OTHER  NIC for this feature can add similar record in their xxx.rst.

Re: [dpdk-dev] [PATCH v4 1/3] lib/librte_ether: add support for port reset

[Thomas Monjalon]

21/04/2017 10:59, Zhao1, Wei:
> Hi, thomas
> 
> From: Thomas Monjalon [mailto:thomas@monjalon.net]
> > 21/04/2017 04:27, Yuanhan Liu:
> > > On Thu, Apr 20, 2017 at 09:17:24AM +0000, Zhao1, Wei wrote:
> > > > > > > Please explain exactly the responsibility of this function,
> > > > > > > and how it is different from calling stop/configure/start.
> > > > > > 
> > > > > > In this reset feature, reset function can do the calling
> > > > > > stop/configure/start process, but also It can also do some
> > > > > > restore work for the port, for example, it can restore the added
> > > > > > parameters of
> > > > > 
> > > > > vlan,  mac_addrs, promisc_unicast_enabled falg and
> > > > > promisc_multicast_enabled flag.
> > > > > 
> > > > > > Maybe , I should add this explanation in the patch comments or
> > > > > > function
> > > > > 
> > > > > comments?
> > > > > 
> > > > > I'm curious why we have to do save & restore for a reset operation.
> > > > > Why some configures have to be saved and restored? Doesn't "reset"
> > > > > literally means reset everything?
> > > > 
> > > > Users maybe do not want to do a second configuration operation to
> > > > waste time after reset which lost all previous configuration. But he
> > > > still want these configuration valid after reset.
> > > > So, save & restore can help them to save this process time and effort.
> > > > 
> > > > > Even though, how do you tell what kind of configures need be
> > > > > restored and what should not? Again, even though, will all PMDs
> > > > > supports restoring the same set of configurations?
> > > > 
> > > > Yes, this is hard to say what may be need and what may be not for
> > > > user.
> > > > Now, the kinds of supported is list in patch set comment. And only
> > > > i40e NIC support this feature.
> > > 
> > > Why it's the configurations listed in patch 2? Because they are
> > > requested by customers?
> > > 
> > > Is that all could be saved? If not, are you going to save & restore
> > > all possible configurations?
> > > 
> > > Assuming the configurations saved & restored may differ from different
> > > PMD drivers, how could you keep the consistency then? And judging that
> > > the application has no idea about the difference (yet it knows nothing
> > > about what kind of configurations will be saved), how could the
> > > application know that some configurations are not saved & restored by
> > > the driver that it has to do re-configuration by itself?
> > > 
> > > > > While looking at your reset implementation for i40e, it looks more
> > > > > complex than necessary: just thinking we have to call
> > > > > "xxx_queue_setup"
> > > > > for all PMDs.
> > > > > 
> > > > > I'm thinking a simple hardware reset might be enough?
> > > > > 
> > > > >     /* literally reset the hardware: reset everything */
> > > > >     rte_eth_reset(port)
> > > > >     {
> > > > >     
> > > > >     	eth_dev->ops->reset();
> > > > >     
> > > > >     }
> > > > 
> > > > You mean just do a reset  and do not restore any configuration?
> > > > That may not meet the need for this feature from customer?
> > > 
> > > Right, I'm just aware of the configuration might be done by PF (but
> > > not only by the application), that the VF port may be not aware of
> > > those configurations. So the save & restore is needed. I don't quite
> > > like how it is done in this patch set though. I also don't think the
> > > API is well named: as said, reset should literally reset everything.
> > > 
> > > We may need think how to do it properly.
> > > 
> > > Thomas, Konstantin, what do you guys think of it?
> > 
> > I have the same concerns.
> > I think we should better document the current status of start/stop and
> > which configuration parameters are lost or saved.
> 
> Maybe I should add some words in doc\guides\nics\i40e.rst to Record which
> configurations are  saved  and restored by the PMD driver in reset
> function. Which not list in that are recognized as not saved  and restored 
> by default. OTHER  NIC for this feature can add similar record in their
> xxx.rst.

No, when defining a generic API in ethdev, we must define the same
behaviour for every drivers.
Please check how to make the behaviour consistent and documented
in ethdev. We may need to document your new function and start/stop also.

Re: [dpdk-dev] [PATCH v4 1/3] lib/librte_ether: add support for port reset

[Yuanhan Liu]

On Fri, Apr 21, 2017 at 11:28:37AM +0200, Thomas Monjalon wrote:
> > Maybe I should add some words in doc\guides\nics\i40e.rst to Record which
> > configurations are  saved  and restored by the PMD driver in reset
> > function. Which not list in that are recognized as not saved  and restored 
> > by default. OTHER  NIC for this feature can add similar record in their
> > xxx.rst.
> 
> No, when defining a generic API in ethdev, we must define the same
> behaviour for every drivers.

Agreed. That was my point.

	--yliu

> Please check how to make the behaviour consistent and documented
> in ethdev. We may need to document your new function and start/stop also.

Re: [dpdk-dev] [PATCH v4 1/3] lib/librte_ether: add support for port reset

[Zhao1, Wei]

Hi, yuanhan & thomas

> -----Original Message-----
> From: Yuanhan Liu [mailto:yuanhan.liu@linux.intel.com]
> Sent: Monday, April 24, 2017 10:01 AM
> To: Thomas Monjalon <thomas@monjalon.net>
> Cc: Zhao1, Wei <wei.zhao1@intel.com>; dev@dpdk.org; Ananyev,
> Konstantin <konstantin.ananyev@intel.com>; Mcnamara, John
> <john.mcnamara@intel.com>; Lu, Wenzhuo <wenzhuo.lu@intel.com>; Liu,
> Yu Y <yu.y.liu@intel.com>
> Subject: Re: [dpdk-dev] [PATCH v4 1/3] lib/librte_ether: add support for port
> reset
> 
> On Fri, Apr 21, 2017 at 11:28:37AM +0200, Thomas Monjalon wrote:
> > > Maybe I should add some words in doc\guides\nics\i40e.rst to Record
> > > which configurations are  saved  and restored by the PMD driver in
> > > reset function. Which not list in that are recognized as not saved
> > > and restored by default. OTHER  NIC for this feature can add similar
> > > record in their xxx.rst.
> >
> > No, when defining a generic API in ethdev, we must define the same
> > behaviour for every drivers.
> 
> Agreed. That was my point.
> 
> 	--yliu
> 
> > Please check how to make the behaviour consistent and documented in
> > ethdev. We may need to document your new function and start/stop also.

Maybe I should record these reset and restore info in some doc in rte layer.
And  we have only implement this feature in i40e NIC  by now.

Re: [dpdk-dev] [PATCH v4 1/3] lib/librte_ether: add support for port reset

[Zhao1, Wei]

Hi, yuanhan & thomas

> -----Original Message-----
> From: Yuanhan Liu [mailto:yuanhan.liu@linux.intel.com]
> Sent: Monday, April 24, 2017 10:01 AM
> To: Thomas Monjalon <thomas@monjalon.net>
> Cc: Zhao1, Wei <wei.zhao1@intel.com>; dev@dpdk.org; Ananyev,
> Konstantin <konstantin.ananyev@intel.com>; Mcnamara, John
> <john.mcnamara@intel.com>; Lu, Wenzhuo <wenzhuo.lu@intel.com>; Liu,
> Yu Y <yu.y.liu@intel.com>
> Subject: Re: [dpdk-dev] [PATCH v4 1/3] lib/librte_ether: add support for port
> reset
> 
> On Fri, Apr 21, 2017 at 11:28:37AM +0200, Thomas Monjalon wrote:
> > > Maybe I should add some words in doc\guides\nics\i40e.rst to Record
> > > which configurations are  saved  and restored by the PMD driver in
> > > reset function. Which not list in that are recognized as not saved
> > > and restored by default. OTHER  NIC for this feature can add similar
> > > record in their xxx.rst.
> >
> > No, when defining a generic API in ethdev, we must define the same
> > behaviour for every drivers.
> 
> Agreed. That was my point.
> 
> 	--yliu
> 
> > Please check how to make the behaviour consistent and documented in
> > ethdev. We may need to document your new function and start/stop also.

Do you have any suggestion on which document  in rte layer to record store and restore info by me?

Re: [dpdk-dev] [PATCH v4 1/3] lib/librte_ether: add support for port reset

[Thomas Monjalon]

24/04/2017 05:39, Zhao1, Wei:
> Hi, yuanhan & thomas
> From: Yuanhan Liu [mailto:yuanhan.liu@linux.intel.com]
> > On Fri, Apr 21, 2017 at 11:28:37AM +0200, Thomas Monjalon wrote:
> > > > Maybe I should add some words in doc\guides\nics\i40e.rst to Record
> > > > which configurations are  saved  and restored by the PMD driver in
> > > > reset function. Which not list in that are recognized as not saved
> > > > and restored by default. OTHER  NIC for this feature can add similar
> > > > record in their xxx.rst.
> > > 
> > > No, when defining a generic API in ethdev, we must define the same
> > > behaviour for every drivers.
> > 
> > Agreed. That was my point.
> > 
> > 	--yliu
> > 	
> > > Please check how to make the behaviour consistent and documented in
> > > ethdev. We may need to document your new function and start/stop also.
> 
> Do you have any suggestion on which document  in rte layer to record store
> and restore info by me?

It should be documented in the doxygen comment of the functions.
Either we explain which configuration is restored on start and reset,
or we state everything (or nothing) is restored except the configurations
commented in the related configuration functions.

Re: [dpdk-dev] [PATCH v4 1/3] lib/librte_ether: add support for port reset

[Zhao1, Wei]

Ok.

> -----Original Message-----
> From: Thomas Monjalon [mailto:thomas@monjalon.net]
> Sent: Monday, April 24, 2017 4:05 PM
> To: Zhao1, Wei <wei.zhao1@intel.com>
> Cc: dev@dpdk.org; Yuanhan Liu <yuanhan.liu@linux.intel.com>; Ananyev,
> Konstantin <konstantin.ananyev@intel.com>; Mcnamara, John
> <john.mcnamara@intel.com>; Lu, Wenzhuo <wenzhuo.lu@intel.com>; Liu,
> Yu Y <yu.y.liu@intel.com>
> Subject: Re: [dpdk-dev] [PATCH v4 1/3] lib/librte_ether: add support for port
> reset
> 
> 24/04/2017 05:39, Zhao1, Wei:
> > Hi, yuanhan & thomas
> > From: Yuanhan Liu [mailto:yuanhan.liu@linux.intel.com]
> > > On Fri, Apr 21, 2017 at 11:28:37AM +0200, Thomas Monjalon wrote:
> > > > > Maybe I should add some words in doc\guides\nics\i40e.rst to
> > > > > Record which configurations are  saved  and restored by the PMD
> > > > > driver in reset function. Which not list in that are recognized
> > > > > as not saved and restored by default. OTHER  NIC for this
> > > > > feature can add similar record in their xxx.rst.
> > > >
> > > > No, when defining a generic API in ethdev, we must define the same
> > > > behaviour for every drivers.
> > >
> > > Agreed. That was my point.
> > >
> > > 	--yliu
> > >
> > > > Please check how to make the behaviour consistent and documented
> > > > in ethdev. We may need to document your new function and start/stop
> also.
> >
> > Do you have any suggestion on which document  in rte layer to record
> > store and restore info by me?
> 
> It should be documented in the doxygen comment of the functions.
> Either we explain which configuration is restored on start and reset, or we
> state everything (or nothing) is restored except the configurations
> commented in the related configuration functions.

Re: [dpdk-dev] [PATCH v4 1/3] lib/librte_ether: add support for port reset

[Zhao1, Wei]

> -----Original Message-----
> From: Yuanhan Liu [mailto:yuanhan.liu@linux.intel.com]
> Sent: Friday, April 21, 2017 10:28 AM
> To: Zhao1, Wei <wei.zhao1@intel.com>
> Cc: Ananyev, Konstantin <konstantin.ananyev@intel.com>; Mcnamara, John
> <john.mcnamara@intel.com>; dev@dpdk.org; Lu, Wenzhuo
> <wenzhuo.lu@intel.com>; Thomas Monjalon <thomas@monjalon.net>; Liu,
> Yu Y <yu.y.liu@intel.com>
> Subject: Re: [dpdk-dev] [PATCH v4 1/3] lib/librte_ether: add support for port
> reset
> 
> On Thu, Apr 20, 2017 at 09:17:24AM +0000, Zhao1, Wei wrote:
> > > > > Please explain exactly the responsibility of this function, and
> > > > > how it is different from calling stop/configure/start.
> > > >
> > > > In this reset feature, reset function can do the calling
> > > > stop/configure/start process, but also It can also do some restore
> > > > work for the port, for example, it can restore the added
> > > > parameters  of
> > > vlan,  mac_addrs, promisc_unicast_enabled falg and
> > > promisc_multicast_enabled flag.
> > > > Maybe , I should add this explanation in the patch comments or
> > > > function
> > > comments?
> > >
> > > I'm curious why we have to do save & restore for a reset operation.
> > > Why some configures have to be saved and restored? Doesn't "reset"
> > > literally means reset everything?
> > >
> >
> > Users maybe do not want to do a second configuration operation to waste
> time after reset which lost all previous configuration.
> > But he still want these configuration valid after reset.
> > So, save & restore can help them to save this process time and effort.
> >
> > > Even though, how do you tell what kind of configures need be
> > > restored and what should not? Again, even though, will all PMDs
> > > supports restoring the same set of configurations?
> > >
> >
> > Yes, this is hard to say what may be need and what may be not for user.
> > Now, the kinds of supported is list in patch set comment. And only i40e NIC
> support this feature.
> 
> Why it's the configurations listed in patch 2? Because they are requested by
> customers?
> 
> Is that all could be saved? If not, are you going to save & restore all possible
> configurations?
> 
> Assuming the configurations saved & restored may differ from different
> PMD drivers, how could you keep the consistency then? And judging that the
> application has no idea about the difference (yet it knows nothing about
> what kind of configurations will be saved), how could the application know
> that some configurations are not saved & restored by the driver that it has to
> do re-configuration by itself?
> 

Good idea, so maybe I should add some words in doc\guides\nics\i40e.rst to
Record which configurations are  saved  and restored by the PMD driver in reset function.
Which not list in that are recognized as not saved  and restored  by default.
Is that ok ?

> > > While looking at your reset implementation for i40e, it looks more
> > > complex than necessary: just thinking we have to call
> "xxx_queue_setup"
> > > for all PMDs.
> > >
> > > I'm thinking a simple hardware reset might be enough?
> > >
> > >     /* literally reset the hardware: reset everything */
> > >     rte_eth_reset(port)
> > >     {
> > >     	eth_dev->ops->reset();
> > >     }
> > >
> >
> > You mean just do a reset  and do not restore any configuration?
> > That may not meet the need for this feature from customer?
> 
> Right, I'm just aware of the configuration might be done by PF (but not only
> by the application), that the VF port may be not aware of those
> configurations. So the save & restore is needed. I don't quite like how it is
> done in this patch set though. I also don't think the API is well named: as said,
> reset should literally reset everything.
> 
> We may need think how to do it properly.
> 
> Thomas, Konstantin, what do you guys think of it?

So, your opinion is if it is named "reset", we had better do not do any restore work?
If we keep this restore feature, we had better change function name?
May be use the name "reset_and_restore" as function and feature name ?
 
> 
> 	--yliu

[dpdk-dev] [PATCH v4 2/3] net/i40e: implement device reset on port

[Wei Zhao]

This patch implement the device reset function on vf port.
This restart function will detach device then
attach device, reconfigure dev, re-setup
the Rx/Tx queues.

Signed-off-by: Wei Zhao <wei.zhao1@intel.com>
Signed-off-by: Wenzhuo Lu <wenzhuo.lu@intel.com>
---
 drivers/net/i40e/i40e_ethdev.c    |   2 +-
 drivers/net/i40e/i40e_ethdev.h    |  16 ++++
 drivers/net/i40e/i40e_ethdev_vf.c | 185 ++++++++++++++++++++++++++++++++++++++
 drivers/net/i40e/i40e_rxtx.c      |  10 +++
 drivers/net/i40e/i40e_rxtx.h      |   4 +
 5 files changed, 216 insertions(+), 1 deletion(-)

diff --git a/drivers/net/i40e/i40e_ethdev.c b/drivers/net/i40e/i40e_ethdev.c
index 3702214..f2fdb2f 100644
--- a/drivers/net/i40e/i40e_ethdev.c
+++ b/drivers/net/i40e/i40e_ethdev.c
@@ -6020,7 +6020,7 @@ i40e_find_vlan_filter(struct i40e_vsi *vsi,
 		return 0;
 }
 
-static void
+void
 i40e_store_vlan_filter(struct i40e_vsi *vsi,
 		       uint16_t vlan_id, bool on)
 {
diff --git a/drivers/net/i40e/i40e_ethdev.h b/drivers/net/i40e/i40e_ethdev.h
index aebb097..515a288 100644
--- a/drivers/net/i40e/i40e_ethdev.h
+++ b/drivers/net/i40e/i40e_ethdev.h
@@ -652,6 +652,16 @@ struct i40e_vf_tx_queues {
 	uint32_t tx_ring_len;
 };
 
+struct i40e_vf_reset_store {
+	struct ether_addr *mac_addrs;  /* Device Ethernet Link address. */
+	bool promisc_unicast_enabled;
+	bool promisc_multicast_enabled;
+	uint16_t vlan_num;    /* Total VLAN number */
+	uint32_t vfta[I40E_VFTA_SIZE];        /* VLAN bitmap */
+	uint16_t mac_num;        /* Total mac number */
+};
+
+
 /*
  * Structure to store private data specific for VF instance.
  */
@@ -709,6 +719,10 @@ struct i40e_adapter {
 	struct rte_timecounter systime_tc;
 	struct rte_timecounter rx_tstamp_tc;
 	struct rte_timecounter tx_tstamp_tc;
+
+	/* For port reset */
+	volatile uint8_t reset_flag;
+	void *reset_store_data;
 };
 
 extern const struct rte_flow_ops i40e_flow_ops;
@@ -749,6 +763,8 @@ int i40e_dev_link_update(struct rte_eth_dev *dev,
 			 __rte_unused int wait_to_complete);
 void i40e_vsi_queues_bind_intr(struct i40e_vsi *vsi);
 void i40e_vsi_queues_unbind_intr(struct i40e_vsi *vsi);
+void i40e_store_vlan_filter(struct i40e_vsi *vsi,
+		       uint16_t vlan_id, bool on);
 int i40e_vsi_vlan_pvid_set(struct i40e_vsi *vsi,
 			   struct i40e_vsi_vlan_pvid_info *info);
 int i40e_vsi_config_vlan_stripping(struct i40e_vsi *vsi, bool on);
diff --git a/drivers/net/i40e/i40e_ethdev_vf.c b/drivers/net/i40e/i40e_ethdev_vf.c
index 55fd344..9ab035c 100644
--- a/drivers/net/i40e/i40e_ethdev_vf.c
+++ b/drivers/net/i40e/i40e_ethdev_vf.c
@@ -161,6 +161,9 @@ i40evf_dev_rx_queue_intr_disable(struct rte_eth_dev *dev, uint16_t queue_id);
 static void i40evf_handle_pf_event(__rte_unused struct rte_eth_dev *dev,
 				   uint8_t *msg,
 				   uint16_t msglen);
+static int i40evf_dev_uninit(struct rte_eth_dev *eth_dev);
+static int i40evf_dev_init(struct rte_eth_dev *eth_dev);
+static int i40evf_reset_dev(struct rte_eth_dev *dev);
 
 /* Default hash key buffer for RSS */
 static uint32_t rss_key_default[I40E_VFQF_HKEY_MAX_INDEX + 1];
@@ -230,6 +233,7 @@ static const struct eth_dev_ops i40evf_eth_dev_ops = {
 	.rss_hash_conf_get    = i40evf_dev_rss_hash_conf_get,
 	.mtu_set              = i40evf_dev_mtu_set,
 	.mac_addr_set         = i40evf_set_default_mac_addr,
+	.dev_reset            = i40evf_reset_dev,
 };
 
 /*
@@ -889,6 +893,7 @@ i40evf_add_mac_addr(struct rte_eth_dev *dev,
 		PMD_DRV_LOG(ERR, "fail to execute command "
 			    "OP_ADD_ETHER_ADDRESS");
 
+	vf->vsi.mac_num++;
 	return;
 }
 
@@ -926,6 +931,7 @@ i40evf_del_mac_addr_by_addr(struct rte_eth_dev *dev,
 	if (err)
 		PMD_DRV_LOG(ERR, "fail to execute command "
 			    "OP_DEL_ETHER_ADDRESS");
+	vf->vsi.mac_num--;
 	return;
 }
 
@@ -1047,6 +1053,7 @@ static int
 i40evf_add_vlan(struct rte_eth_dev *dev, uint16_t vlanid)
 {
 	struct i40e_vf *vf = I40EVF_DEV_PRIVATE_TO_VF(dev->data->dev_private);
+	struct i40e_vsi *vsi = &vf->vsi;
 	struct i40e_virtchnl_vlan_filter_list *vlan_list;
 	uint8_t cmd_buffer[sizeof(struct i40e_virtchnl_vlan_filter_list) +
 							sizeof(uint16_t)];
@@ -1066,6 +1073,8 @@ i40evf_add_vlan(struct rte_eth_dev *dev, uint16_t vlanid)
 	err = i40evf_execute_vf_cmd(dev, &args);
 	if (err)
 		PMD_DRV_LOG(ERR, "fail to execute command OP_ADD_VLAN");
+	i40e_store_vlan_filter(vsi, vlanid, 1);
+	vsi->vlan_num++;
 
 	return err;
 }
@@ -1074,6 +1083,7 @@ static int
 i40evf_del_vlan(struct rte_eth_dev *dev, uint16_t vlanid)
 {
 	struct i40e_vf *vf = I40EVF_DEV_PRIVATE_TO_VF(dev->data->dev_private);
+	struct i40e_vsi *vsi = &vf->vsi;
 	struct i40e_virtchnl_vlan_filter_list *vlan_list;
 	uint8_t cmd_buffer[sizeof(struct i40e_virtchnl_vlan_filter_list) +
 							sizeof(uint16_t)];
@@ -1093,6 +1103,8 @@ i40evf_del_vlan(struct rte_eth_dev *dev, uint16_t vlanid)
 	err = i40evf_execute_vf_cmd(dev, &args);
 	if (err)
 		PMD_DRV_LOG(ERR, "fail to execute command OP_DEL_VLAN");
+	i40e_store_vlan_filter(vsi, vlanid, 0);
+	vsi->vlan_num--;
 
 	return err;
 }
@@ -2716,3 +2728,176 @@ i40evf_set_default_mac_addr(struct rte_eth_dev *dev,
 
 	i40evf_add_mac_addr(dev, mac_addr, 0, 0);
 }
+
+static void
+i40evf_restore_vlan_filter(struct rte_eth_dev *dev,
+			uint32_t *vfta)
+{
+	uint32_t vid_idx, vid_bit;
+	uint16_t vlan_id;
+
+	for  (vid_idx = 0; vid_idx < I40E_VFTA_SIZE; vid_idx++) {
+		for  (vid_bit = 0; vid_bit < I40E_UINT32_BIT_SIZE; vid_bit++) {
+			if (vfta[vid_idx] & (1 << vid_bit)) {
+				vlan_id = (vid_idx << 5) + vid_bit;
+				i40evf_add_vlan(dev, vlan_id);
+			}
+		}
+	}
+}
+
+static void
+i40evf_restore_macaddr(struct rte_eth_dev *dev,
+		struct ether_addr *mac_addrs)
+{
+	struct ether_addr *addr;
+	uint16_t i;
+
+	/* replay MAC address configuration including default MAC */
+	addr = &mac_addrs[0];
+
+	i40evf_set_default_mac_addr(dev, addr);
+	memcpy(dev->data->mac_addrs, mac_addrs,
+			ETHER_ADDR_LEN * I40E_NUM_MACADDR_MAX);
+
+	for (i = 1; i < I40E_NUM_MACADDR_MAX; i++) {
+		addr = &mac_addrs[i];
+
+		/* skip zero address */
+		if (is_zero_ether_addr(addr))
+			continue;
+
+		i40evf_add_mac_addr(dev, addr, 0, 0);
+	}
+}
+
+
+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_store_before_reset(struct rte_eth_dev *dev)
+{
+	struct i40e_adapter *adapter =
+		I40E_DEV_PRIVATE_TO_ADAPTER(dev->data->dev_private);
+	struct i40e_vf_reset_store *store_data;
+	struct i40e_vf *vf = I40EVF_DEV_PRIVATE_TO_VF(dev->data->dev_private);
+
+	adapter->reset_store_data = rte_zmalloc("i40evf_store_reset",
+					sizeof(struct i40e_vf_reset_store), 0);
+	if (adapter->reset_store_data == NULL) {
+		PMD_INIT_LOG(ERR, "Failed to allocate %ld bytes needed to"
+				" to store data when reset vf",
+				sizeof(struct i40e_vf_reset_store));
+		return -ENOMEM;
+	}
+	store_data =
+		(struct i40e_vf_reset_store *)adapter->reset_store_data;
+	store_data->mac_addrs = rte_zmalloc("i40evf_mac_store_reset",
+			ETHER_ADDR_LEN * I40E_NUM_MACADDR_MAX, 0);
+	if (store_data->mac_addrs == NULL) {
+		PMD_INIT_LOG(ERR, "Failed to allocate %d bytes needed to"
+				" to store MAC addresses when reset vf",
+				ETHER_ADDR_LEN * I40E_NUM_MACADDR_MAX);
+	}
+
+	memcpy(store_data->mac_addrs, dev->data->mac_addrs,
+			ETHER_ADDR_LEN * I40E_NUM_MACADDR_MAX);
+
+	store_data->promisc_unicast_enabled = vf->promisc_unicast_enabled;
+	store_data->promisc_multicast_enabled = vf->promisc_multicast_enabled;
+
+	store_data->vlan_num = vf->vsi.vlan_num;
+	memcpy(store_data->vfta, vf->vsi.vfta,
+			sizeof(uint32_t) * I40E_VFTA_SIZE);
+
+	store_data->mac_num = vf->vsi.mac_num;
+
+	return 0;
+}
+
+static void
+i40evf_restore_after_reset(struct rte_eth_dev *dev)
+{
+	struct i40e_adapter *adapter =
+		I40E_DEV_PRIVATE_TO_ADAPTER(dev->data->dev_private);
+	struct i40e_vf_reset_store *store_data =
+		(struct i40e_vf_reset_store *)adapter->reset_store_data;
+
+	if (store_data->promisc_unicast_enabled)
+		i40evf_dev_promiscuous_enable(dev);
+	if (store_data->promisc_multicast_enabled)
+		i40evf_dev_allmulticast_enable(dev);
+
+	if (store_data->vlan_num)
+		i40evf_restore_vlan_filter(dev, store_data->vfta);
+
+	if (store_data->mac_num)
+		i40evf_restore_macaddr(dev, store_data->mac_addrs);
+
+	rte_free(store_data->mac_addrs);
+	rte_free(adapter->reset_store_data);
+}
+
+static int
+i40evf_reset_dev(struct rte_eth_dev *dev)
+{
+	struct i40e_adapter *adapter =
+		I40E_DEV_PRIVATE_TO_ADAPTER(dev->data->dev_private);
+
+	adapter->reset_flag = 1;
+	i40evf_store_before_reset(dev);
+
+	i40evf_dev_close(dev);
+	PMD_DRV_LOG(DEBUG, "i40evf dev close complete");
+
+	i40evf_dev_uninit(dev);
+	PMD_DRV_LOG(DEBUG, "i40evf dev detached");
+
+	memset(dev->data->dev_private, 0,
+	       (uint64_t)&adapter->reset_flag - (uint64_t)adapter);
+
+	i40evf_dev_init(dev);
+	PMD_DRV_LOG(DEBUG, "i40evf dev attached");
+
+	i40evf_dev_configure(dev);
+
+	i40e_vf_queue_reset(dev);
+	PMD_DRV_LOG(DEBUG, "i40evf queue reset");
+
+	i40evf_restore_after_reset(dev);
+
+	i40evf_dev_start(dev);
+	PMD_DRV_LOG(DEBUG, "i40evf dev restart");
+
+	adapter->reset_flag = 0;
+
+	return 0;
+}
diff --git a/drivers/net/i40e/i40e_rxtx.c b/drivers/net/i40e/i40e_rxtx.c
index 02367b7..d891a54 100644
--- a/drivers/net/i40e/i40e_rxtx.c
+++ b/drivers/net/i40e/i40e_rxtx.c
@@ -1709,6 +1709,7 @@ i40e_dev_rx_queue_setup(struct rte_eth_dev *dev,
 	uint16_t len, i;
 	uint16_t base, bsf, tc_mapping;
 	int use_def_burst_func = 1;
+	struct rte_eth_rxconf conf = *rx_conf;
 
 	if (hw->mac.type == I40E_MAC_VF || hw->mac.type == I40E_MAC_X722_VF) {
 		struct i40e_vf *vf =
@@ -1748,6 +1749,8 @@ i40e_dev_rx_queue_setup(struct rte_eth_dev *dev,
 	}
 	rxq->mp = mp;
 	rxq->nb_rx_desc = nb_desc;
+	rxq->socket_id = socket_id;
+	rxq->rxconf = conf;
 	rxq->rx_free_thresh = rx_conf->rx_free_thresh;
 	rxq->queue_id = queue_idx;
 	if (hw->mac.type == I40E_MAC_VF || hw->mac.type == I40E_MAC_X722_VF)
@@ -1932,6 +1935,7 @@ i40e_dev_tx_queue_setup(struct rte_eth_dev *dev,
 	uint32_t ring_size;
 	uint16_t tx_rs_thresh, tx_free_thresh;
 	uint16_t i, base, bsf, tc_mapping;
+	struct rte_eth_txconf conf = *tx_conf;
 
 	if (hw->mac.type == I40E_MAC_VF || hw->mac.type == I40E_MAC_X722_VF) {
 		struct i40e_vf *vf =
@@ -2054,6 +2058,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;
@@ -2520,8 +2526,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_flag)
+		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 a87bdb0..0e3cc19 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.9.3

[dpdk-dev] [PATCH v4 3/3] app/testpmd: add port reset command into testpmd

[Wei Zhao]

Add port reset command into testpmd project,
it is the interface for user to reset port.
And also it is not limit to be used only in vf reset,
but also for pf port reset.But this patch set only
realted to vf reset feature.

Signed-off-by: Wei Zhao <wei.zhao1@intel.com>
Signed-off-by: Wenzhuo Lu <wenzhuo.lu@intel.com>
---
 app/test-pmd/cmdline.c | 17 +++++++++----
 app/test-pmd/testpmd.c | 65 ++++++++++++++++++++++++++++++++++++++++++++++++++
 app/test-pmd/testpmd.h |  1 +
 3 files changed, 79 insertions(+), 4 deletions(-)

diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c
index 47f935d..6cbdcc8 100644
--- a/app/test-pmd/cmdline.c
+++ b/app/test-pmd/cmdline.c
@@ -599,6 +599,9 @@ static void cmd_help_long_parsed(void *parsed_result,
 			"port close (port_id|all)\n"
 			"    Close all ports or port_id.\n\n"
 
+			"port reset (port_id|all)\n"
+			"    Reset all ports or port_id.\n\n"
+
 			"port attach (ident)\n"
 			"    Attach physical or virtual dev by pci address or virtual device name\n\n"
 
@@ -909,6 +912,8 @@ static void cmd_operate_port_parsed(void *parsed_result,
 		stop_port(RTE_PORT_ALL);
 	else if (!strcmp(res->name, "close"))
 		close_port(RTE_PORT_ALL);
+	else if (!strcmp(res->name, "reset"))
+		reset_port(RTE_PORT_ALL);
 	else
 		printf("Unknown parameter\n");
 }
@@ -918,14 +923,15 @@ cmdline_parse_token_string_t cmd_operate_port_all_cmd =
 								"port");
 cmdline_parse_token_string_t cmd_operate_port_all_port =
 	TOKEN_STRING_INITIALIZER(struct cmd_operate_port_result, name,
-						"start#stop#close");
+						"start#stop#close#reset");
 cmdline_parse_token_string_t cmd_operate_port_all_all =
 	TOKEN_STRING_INITIALIZER(struct cmd_operate_port_result, value, "all");
 
 cmdline_parse_inst_t cmd_operate_port = {
 	.f = cmd_operate_port_parsed,
 	.data = NULL,
-	.help_str = "port start|stop|close all: Start/Stop/Close all ports",
+	.help_str = "port start|stop|close|reset all: Start/Stop/Close/"
+			"Reset all ports",
 	.tokens = {
 		(void *)&cmd_operate_port_all_cmd,
 		(void *)&cmd_operate_port_all_port,
@@ -953,6 +959,8 @@ static void cmd_operate_specific_port_parsed(void *parsed_result,
 		stop_port(res->value);
 	else if (!strcmp(res->name, "close"))
 		close_port(res->value);
+	else if (!strcmp(res->name, "reset"))
+		reset_port(res->value);
 	else
 		printf("Unknown parameter\n");
 }
@@ -962,7 +970,7 @@ cmdline_parse_token_string_t cmd_operate_specific_port_cmd =
 							keyword, "port");
 cmdline_parse_token_string_t cmd_operate_specific_port_port =
 	TOKEN_STRING_INITIALIZER(struct cmd_operate_specific_port_result,
-						name, "start#stop#close");
+						name, "start#stop#close#reset");
 cmdline_parse_token_num_t cmd_operate_specific_port_id =
 	TOKEN_NUM_INITIALIZER(struct cmd_operate_specific_port_result,
 							value, UINT8);
@@ -970,7 +978,8 @@ cmdline_parse_token_num_t cmd_operate_specific_port_id =
 cmdline_parse_inst_t cmd_operate_specific_port = {
 	.f = cmd_operate_specific_port_parsed,
 	.data = NULL,
-	.help_str = "port start|stop|close <port_id>: Start/Stop/Close port_id",
+	.help_str = "port start|stop|close|reset <port_id>: Start/Stop/Close/"
+			"Reset port_id",
 	.tokens = {
 		(void *)&cmd_operate_specific_port_cmd,
 		(void *)&cmd_operate_specific_port_port,
diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c
index 484c19b..a4f5330 100644
--- a/app/test-pmd/testpmd.c
+++ b/app/test-pmd/testpmd.c
@@ -137,6 +137,7 @@ portid_t  nb_fwd_ports;  /**< Number of forwarding ports. */
 
 unsigned int fwd_lcores_cpuids[RTE_MAX_LCORE]; /**< CPU ids configuration. */
 portid_t fwd_ports_ids[RTE_MAX_ETHPORTS];      /**< Port ids configuration. */
+volatile char reset_ports[RTE_MAX_ETHPORTS] = {0};  /**< Port reset flag. */
 
 struct fwd_stream **fwd_streams; /**< For each RX queue of each port. */
 streamid_t nb_fwd_streams;       /**< Is equal to (nb_ports * nb_rxq). */
@@ -1305,6 +1306,18 @@ port_is_closed(portid_t port_id)
 	return 1;
 }
 
+static void
+reset_event_callback(uint8_t port_id, enum rte_eth_event_type type, void *param)
+{
+	RTE_SET_USED(param);
+
+	printf("Event type: %s on port %d\n",
+		type == RTE_ETH_EVENT_INTR_RESET ? "RESET interrupt" :
+		"unknown event", port_id);
+	reset_ports[port_id] = 1;
+	rte_wmb();
+}
+
 int
 start_port(portid_t pid)
 {
@@ -1350,6 +1363,10 @@ start_port(portid_t pid)
 				return -1;
 			}
 		}
+
+		/* register reset interrupt callback */
+		rte_eth_dev_callback_register(pi, RTE_ETH_EVENT_INTR_RESET,
+			reset_event_callback, NULL);
 		if (port->need_reconfig_queues > 0) {
 			port->need_reconfig_queues = 0;
 			/* setup tx queues */
@@ -1559,6 +1576,54 @@ close_port(portid_t pid)
 }
 
 void
+reset_port(portid_t pid)
+{
+	portid_t pi;
+	struct rte_port *port;
+
+	if (port_id_is_invalid(pid, ENABLED_WARN))
+		return;
+
+	printf("Reset ports...\n");
+
+	FOREACH_PORT(pi, ports) {
+		if (pid != pi && pid != (portid_t)RTE_PORT_ALL)
+			continue;
+
+		if (port_is_forwarding(pi) != 0 && test_done == 0) {
+			printf("Please remove port %d from forwarding "
+					"configuration.\n", pi);
+			continue;
+		}
+
+		if (port_is_bonding_slave(pi)) {
+			printf("Please remove port %d from "
+					"bonded device.\n", pi);
+			continue;
+		}
+
+		if (!reset_ports[pi]) {
+			printf("vf must get reset port %d info from "
+					"pf before reset.\n", pi);
+			continue;
+		}
+
+		port = &ports[pi];
+		if (rte_atomic16_cmpset(&(port->port_status),
+			RTE_PORT_STOPPED, RTE_PORT_HANDLING) == 1) {
+			printf("Port %d is not stopped\n", pi);
+			continue;
+		}
+
+		rte_eth_dev_reset(pi);
+		reset_ports[pi] = 0;
+		rte_wmb();
+	}
+
+	printf("Done\n");
+}
+
+void
 attach_port(char *identifier)
 {
 	portid_t pi = 0;
diff --git a/app/test-pmd/testpmd.h b/app/test-pmd/testpmd.h
index 8cf2860..0c7e44c 100644
--- a/app/test-pmd/testpmd.h
+++ b/app/test-pmd/testpmd.h
@@ -586,6 +586,7 @@ int init_port_dcb_config(portid_t pid, enum dcb_mode_enable dcb_mode,
 int start_port(portid_t pid);
 void stop_port(portid_t pid);
 void close_port(portid_t pid);
+void reset_port(portid_t pid);
 void attach_port(char *identifier);
 void detach_port(uint8_t port_id);
 int all_ports_stopped(void);
-- 
2.9.3

Re: [dpdk-dev] [PATCH v4 0/3] net/i40e: vf port reset

[Wu, Jingjing]

> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Wei Zhao
> Sent: Thursday, March 30, 2017 5:34 PM
> To: dev@dpdk.org
> Subject: [dpdk-dev] [PATCH v4 0/3] net/i40e: vf port reset
> 
> The patches mainly finish following functions:
> 1) get pf reset vf comamand falg from interrupt server function.
> 2) reset vf port from testpmd app using a command.
> 3) sore and restore vf related parameters.
> 
> v2:
> -change the order of patch in the set.
> -add more details in patch comment to clarify that  the rte and tespmd patch can
> also used in pf port reset.
> -add  rte_free for memory free after restore.
> -change varible name of reset_number to reset_falg.
> -fix patch check warning.
> 
> v3:
> -fix error of author mail address
> 
> v4:
> -fix typo error
> -add rte_wmb() after change the reset_ports.
> 
> zhao wei (3):
>   app/testpmd: add port reset command into testpmd
>   lib/librte_ether: add support for port reset
>   net/i40e: implement device reset on port


Acked-by Jingjing Wu <jingjing.wu@intel.com>

It looks like you sent 2 V4? Could you suspend one of them in Patchwork?

Re: [dpdk-dev] [PATCH v4 0/3] net/i40e: vf port reset

[Zhao1, Wei]

Hi， jingjing 

> -----Original Message-----
> From: Wu, Jingjing
> Sent: Thursday, March 30, 2017 8:33 PM
> To: Zhao1, Wei <wei.zhao1@intel.com>; dev@dpdk.org
> Subject: RE: [dpdk-dev] [PATCH v4 0/3] net/i40e: vf port reset
> 
> 
> 
> > -----Original Message-----
> > From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Wei Zhao
> > Sent: Thursday, March 30, 2017 5:34 PM
> > To: dev@dpdk.org
> > Subject: [dpdk-dev] [PATCH v4 0/3] net/i40e: vf port reset
> >
> > The patches mainly finish following functions:
> > 1) get pf reset vf comamand falg from interrupt server function.
> > 2) reset vf port from testpmd app using a command.
> > 3) sore and restore vf related parameters.
> >
> > v2:
> > -change the order of patch in the set.
> > -add more details in patch comment to clarify that  the rte and tespmd
> > patch can also used in pf port reset.
> > -add  rte_free for memory free after restore.
> > -change varible name of reset_number to reset_falg.
> > -fix patch check warning.
> >
> > v3:
> > -fix error of author mail address
> >
> > v4:
> > -fix typo error
> > -add rte_wmb() after change the reset_ports.
> >
> > zhao wei (3):
> >   app/testpmd: add port reset command into testpmd
> >   lib/librte_ether: add support for port reset
> >   net/i40e: implement device reset on port
> 
> 
> Acked-by Jingjing Wu <jingjing.wu@intel.com>
> 
> It looks like you sent 2 V4? Could you suspend one of them in Patchwork?

I have suspend the repetition one.

[dpdk-dev] [PATCH v5 0/3] net/i40e: vf port reset

[Wei Zhao]

The patches mainly finish following functions:
1) get pf reset vf comamand falg from interrupt server function.
2) reset vf port from testpmd app using a command.
3) sore and restore vf related parameters.

v2:
-change the order of patch in the set.
-add more details in patch comment to clarify that
 the rte and tespmd patch can also used in pf port reset.
-add  rte_free for memory free after restore.
-change varible name of reset_number to reset_falg.
-fix patch check warning.

v3:
-fix error of author mail address

v4:
-fix typo error
-add rte_wmb() after change the reset_ports.

v5:
-add more comments for rte reset function. 

zhao wei (3):
  app/testpmd: add port reset command into testpmd
  lib/librte_ether: add support for port reset
  net/i40e: implement device reset on port

 app/test-pmd/cmdline.c                 |  17 ++-
 app/test-pmd/testpmd.c                 |  65 ++++++++++++
 app/test-pmd/testpmd.h                 |   1 +
 drivers/net/i40e/i40e_ethdev.c         |   2 +-
 drivers/net/i40e/i40e_ethdev.h         |  16 +++
 drivers/net/i40e/i40e_ethdev_vf.c      | 185 +++++++++++++++++++++++++++++++++
 drivers/net/i40e/i40e_rxtx.c           |  10 ++
 drivers/net/i40e/i40e_rxtx.h           |   4 +
 lib/librte_ether/rte_ethdev.c          |  17 +++
 lib/librte_ether/rte_ethdev.h          |  28 +++++
 lib/librte_ether/rte_ether_version.map |   6 ++
 11 files changed, 346 insertions(+), 5 deletions(-)

-- 
2.9.3

Acked-by: Jingjing Wu <jingjing.wu@intel.com>

[dpdk-dev] [PATCH v5 1/3] lib/librte_ether: add support for port reset

[Wei Zhao]

Add support for port reset in rte layer.This reset
feature can not only used in vf port reset in later code
develop, but alsopf port.But in this patch set, we only
limit the discussion scope to vf reset.
This patch Add an API to restart the device.
It's for VF device in this scenario, kernel PF + DPDK VF.
When the PF port down->up, APP should call this API to
restart VF port. Most likely, APP should call it in its
management thread and guarantee the thread safe. It means
APP should stop the rx/tx and the device, then restart
the device, then recover the device and rx/tx.This API can
also do some restore work for the port.

Signed-off-by: Wenzhuo Lu <wenzhuo.lu@intel.com>
Signed-off-by: Wei Zhao <wei.zhao1@intel.com>
---
 lib/librte_ether/rte_ethdev.c          | 17 +++++++++++++++++
 lib/librte_ether/rte_ethdev.h          | 28 ++++++++++++++++++++++++++++
 lib/librte_ether/rte_ether_version.map |  6 ++++++
 3 files changed, 51 insertions(+)

diff --git a/lib/librte_ether/rte_ethdev.c b/lib/librte_ether/rte_ethdev.c
index eb0a94a..2e06dca 100644
--- a/lib/librte_ether/rte_ethdev.c
+++ b/lib/librte_ether/rte_ethdev.c
@@ -3273,3 +3273,20 @@ rte_eth_dev_l2_tunnel_offload_set(uint8_t port_id,
 				-ENOTSUP);
 	return (*dev->dev_ops->l2_tunnel_offload_set)(dev, l2_tunnel, mask, en);
 }
+
+int
+rte_eth_dev_reset(uint8_t port_id)
+{
+	struct rte_eth_dev *dev;
+	int diag;
+
+	RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -ENODEV);
+
+	dev = &rte_eth_devices[port_id];
+
+	RTE_FUNC_PTR_OR_ERR_RET(*dev->dev_ops->dev_reset, -ENOTSUP);
+
+	diag = (*dev->dev_ops->dev_reset)(dev);
+
+	return diag;
+}
diff --git a/lib/librte_ether/rte_ethdev.h b/lib/librte_ether/rte_ethdev.h
index 4be217c..48c0d0b 100644
--- a/lib/librte_ether/rte_ethdev.h
+++ b/lib/librte_ether/rte_ethdev.h
@@ -1367,6 +1367,9 @@ typedef int (*eth_l2_tunnel_offload_set_t)
 	 uint8_t en);
 /**< @internal enable/disable the l2 tunnel offload functions */
 
+typedef int  (*eth_dev_reset_t)(struct rte_eth_dev *dev);
+/**< @internal Function used to reset a configured Ethernet device. */
+
 #ifdef RTE_NIC_BYPASS
 
 enum {
@@ -1509,6 +1512,9 @@ struct eth_dev_ops {
 	eth_l2_tunnel_offload_set_t   l2_tunnel_offload_set;
 	/** Enable/disable l2 tunnel offload functions. */
 
+	/** Reset device. */
+	eth_dev_reset_t dev_reset;
+
 	eth_set_queue_rate_limit_t set_queue_rate_limit; /**< Set queue rate limit. */
 
 	rss_hash_update_t          rss_hash_update; /** Configure RSS hash protocols. */
@@ -4413,6 +4419,28 @@ int
 rte_eth_dev_get_name_by_port(uint8_t port_id, char *name);
 
 /**
+ * Reset an ethernet device when it's not working. One scenario is, after PF
+ * port is down and up, the related VF port should be reset.
+ * The API will stop the port, clear the rx/tx queues, re-setup the rx/tx
+ * queues, restart the port.
+ * Before calling this API, APP should stop the rx/tx. When tx is being stopped,
+ * APP can drop the packets and release the buffer instead of sending them.
+ * This function can also do some restore work for the port, for example, it can 
+ * restore the added parameters of vlan,  mac_addrs, promisc_unicast_enabled flag
+ * and promisc_multicast_enabled flag.
+ *
+ * @param port_id
+ *   The port identifier of the Ethernet device.
+ *
+ * @return
+ *   - (0) if successful.
+ *   - (-ENODEV) if port identifier is invalid.
+ *   - (-ENOTSUP) if hardware doesn't support this function.
+ */
+int
+rte_eth_dev_reset(uint8_t port_id);
+
+/**
  * @internal
  * Wrapper for use by pci drivers as a .probe function to attach to a ethdev
  * interface.
diff --git a/lib/librte_ether/rte_ether_version.map b/lib/librte_ether/rte_ether_version.map
index c6c9d0d..529b27f 100644
--- a/lib/librte_ether/rte_ether_version.map
+++ b/lib/librte_ether/rte_ether_version.map
@@ -154,3 +154,9 @@ DPDK_17.02 {
 	rte_flow_validate;
 
 } DPDK_16.11;
+
+DPDK_17.05 {
+	global:
+
+	rte_eth_dev_reset;
+} DPDK_17.02;
\ No newline at end of file
-- 
2.9.3

[dpdk-dev] [PATCH v5 2/3] net/i40e: implement device reset on port

[Wei Zhao]

This patch implement the device reset function on vf port.
This restart function will detach device then
attach device, reconfigure dev, re-setup
the Rx/Tx queues.

Signed-off-by: Wei Zhao <wei.zhao1@intel.com>
Signed-off-by: Wenzhuo Lu <wenzhuo.lu@intel.com>
---
 drivers/net/i40e/i40e_ethdev.c    |   2 +-
 drivers/net/i40e/i40e_ethdev.h    |  16 ++++
 drivers/net/i40e/i40e_ethdev_vf.c | 185 ++++++++++++++++++++++++++++++++++++++
 drivers/net/i40e/i40e_rxtx.c      |  10 +++
 drivers/net/i40e/i40e_rxtx.h      |   4 +
 5 files changed, 216 insertions(+), 1 deletion(-)

diff --git a/drivers/net/i40e/i40e_ethdev.c b/drivers/net/i40e/i40e_ethdev.c
index 3702214..f2fdb2f 100644
--- a/drivers/net/i40e/i40e_ethdev.c
+++ b/drivers/net/i40e/i40e_ethdev.c
@@ -6020,7 +6020,7 @@ i40e_find_vlan_filter(struct i40e_vsi *vsi,
 		return 0;
 }
 
-static void
+void
 i40e_store_vlan_filter(struct i40e_vsi *vsi,
 		       uint16_t vlan_id, bool on)
 {
diff --git a/drivers/net/i40e/i40e_ethdev.h b/drivers/net/i40e/i40e_ethdev.h
index aebb097..515a288 100644
--- a/drivers/net/i40e/i40e_ethdev.h
+++ b/drivers/net/i40e/i40e_ethdev.h
@@ -652,6 +652,16 @@ struct i40e_vf_tx_queues {
 	uint32_t tx_ring_len;
 };
 
+struct i40e_vf_reset_store {
+	struct ether_addr *mac_addrs;  /* Device Ethernet Link address. */
+	bool promisc_unicast_enabled;
+	bool promisc_multicast_enabled;
+	uint16_t vlan_num;    /* Total VLAN number */
+	uint32_t vfta[I40E_VFTA_SIZE];        /* VLAN bitmap */
+	uint16_t mac_num;        /* Total mac number */
+};
+
+
 /*
  * Structure to store private data specific for VF instance.
  */
@@ -709,6 +719,10 @@ struct i40e_adapter {
 	struct rte_timecounter systime_tc;
 	struct rte_timecounter rx_tstamp_tc;
 	struct rte_timecounter tx_tstamp_tc;
+
+	/* For port reset */
+	volatile uint8_t reset_flag;
+	void *reset_store_data;
 };
 
 extern const struct rte_flow_ops i40e_flow_ops;
@@ -749,6 +763,8 @@ int i40e_dev_link_update(struct rte_eth_dev *dev,
 			 __rte_unused int wait_to_complete);
 void i40e_vsi_queues_bind_intr(struct i40e_vsi *vsi);
 void i40e_vsi_queues_unbind_intr(struct i40e_vsi *vsi);
+void i40e_store_vlan_filter(struct i40e_vsi *vsi,
+		       uint16_t vlan_id, bool on);
 int i40e_vsi_vlan_pvid_set(struct i40e_vsi *vsi,
 			   struct i40e_vsi_vlan_pvid_info *info);
 int i40e_vsi_config_vlan_stripping(struct i40e_vsi *vsi, bool on);
diff --git a/drivers/net/i40e/i40e_ethdev_vf.c b/drivers/net/i40e/i40e_ethdev_vf.c
index 55fd344..9ab035c 100644
--- a/drivers/net/i40e/i40e_ethdev_vf.c
+++ b/drivers/net/i40e/i40e_ethdev_vf.c
@@ -161,6 +161,9 @@ i40evf_dev_rx_queue_intr_disable(struct rte_eth_dev *dev, uint16_t queue_id);
 static void i40evf_handle_pf_event(__rte_unused struct rte_eth_dev *dev,
 				   uint8_t *msg,
 				   uint16_t msglen);
+static int i40evf_dev_uninit(struct rte_eth_dev *eth_dev);
+static int i40evf_dev_init(struct rte_eth_dev *eth_dev);
+static int i40evf_reset_dev(struct rte_eth_dev *dev);
 
 /* Default hash key buffer for RSS */
 static uint32_t rss_key_default[I40E_VFQF_HKEY_MAX_INDEX + 1];
@@ -230,6 +233,7 @@ static const struct eth_dev_ops i40evf_eth_dev_ops = {
 	.rss_hash_conf_get    = i40evf_dev_rss_hash_conf_get,
 	.mtu_set              = i40evf_dev_mtu_set,
 	.mac_addr_set         = i40evf_set_default_mac_addr,
+	.dev_reset            = i40evf_reset_dev,
 };
 
 /*
@@ -889,6 +893,7 @@ i40evf_add_mac_addr(struct rte_eth_dev *dev,
 		PMD_DRV_LOG(ERR, "fail to execute command "
 			    "OP_ADD_ETHER_ADDRESS");
 
+	vf->vsi.mac_num++;
 	return;
 }
 
@@ -926,6 +931,7 @@ i40evf_del_mac_addr_by_addr(struct rte_eth_dev *dev,
 	if (err)
 		PMD_DRV_LOG(ERR, "fail to execute command "
 			    "OP_DEL_ETHER_ADDRESS");
+	vf->vsi.mac_num--;
 	return;
 }
 
@@ -1047,6 +1053,7 @@ static int
 i40evf_add_vlan(struct rte_eth_dev *dev, uint16_t vlanid)
 {
 	struct i40e_vf *vf = I40EVF_DEV_PRIVATE_TO_VF(dev->data->dev_private);
+	struct i40e_vsi *vsi = &vf->vsi;
 	struct i40e_virtchnl_vlan_filter_list *vlan_list;
 	uint8_t cmd_buffer[sizeof(struct i40e_virtchnl_vlan_filter_list) +
 							sizeof(uint16_t)];
@@ -1066,6 +1073,8 @@ i40evf_add_vlan(struct rte_eth_dev *dev, uint16_t vlanid)
 	err = i40evf_execute_vf_cmd(dev, &args);
 	if (err)
 		PMD_DRV_LOG(ERR, "fail to execute command OP_ADD_VLAN");
+	i40e_store_vlan_filter(vsi, vlanid, 1);
+	vsi->vlan_num++;
 
 	return err;
 }
@@ -1074,6 +1083,7 @@ static int
 i40evf_del_vlan(struct rte_eth_dev *dev, uint16_t vlanid)
 {
 	struct i40e_vf *vf = I40EVF_DEV_PRIVATE_TO_VF(dev->data->dev_private);
+	struct i40e_vsi *vsi = &vf->vsi;
 	struct i40e_virtchnl_vlan_filter_list *vlan_list;
 	uint8_t cmd_buffer[sizeof(struct i40e_virtchnl_vlan_filter_list) +
 							sizeof(uint16_t)];
@@ -1093,6 +1103,8 @@ i40evf_del_vlan(struct rte_eth_dev *dev, uint16_t vlanid)
 	err = i40evf_execute_vf_cmd(dev, &args);
 	if (err)
 		PMD_DRV_LOG(ERR, "fail to execute command OP_DEL_VLAN");
+	i40e_store_vlan_filter(vsi, vlanid, 0);
+	vsi->vlan_num--;
 
 	return err;
 }
@@ -2716,3 +2728,176 @@ i40evf_set_default_mac_addr(struct rte_eth_dev *dev,
 
 	i40evf_add_mac_addr(dev, mac_addr, 0, 0);
 }
+
+static void
+i40evf_restore_vlan_filter(struct rte_eth_dev *dev,
+			uint32_t *vfta)
+{
+	uint32_t vid_idx, vid_bit;
+	uint16_t vlan_id;
+
+	for  (vid_idx = 0; vid_idx < I40E_VFTA_SIZE; vid_idx++) {
+		for  (vid_bit = 0; vid_bit < I40E_UINT32_BIT_SIZE; vid_bit++) {
+			if (vfta[vid_idx] & (1 << vid_bit)) {
+				vlan_id = (vid_idx << 5) + vid_bit;
+				i40evf_add_vlan(dev, vlan_id);
+			}
+		}
+	}
+}
+
+static void
+i40evf_restore_macaddr(struct rte_eth_dev *dev,
+		struct ether_addr *mac_addrs)
+{
+	struct ether_addr *addr;
+	uint16_t i;
+
+	/* replay MAC address configuration including default MAC */
+	addr = &mac_addrs[0];
+
+	i40evf_set_default_mac_addr(dev, addr);
+	memcpy(dev->data->mac_addrs, mac_addrs,
+			ETHER_ADDR_LEN * I40E_NUM_MACADDR_MAX);
+
+	for (i = 1; i < I40E_NUM_MACADDR_MAX; i++) {
+		addr = &mac_addrs[i];
+
+		/* skip zero address */
+		if (is_zero_ether_addr(addr))
+			continue;
+
+		i40evf_add_mac_addr(dev, addr, 0, 0);
+	}
+}
+
+
+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_store_before_reset(struct rte_eth_dev *dev)
+{
+	struct i40e_adapter *adapter =
+		I40E_DEV_PRIVATE_TO_ADAPTER(dev->data->dev_private);
+	struct i40e_vf_reset_store *store_data;
+	struct i40e_vf *vf = I40EVF_DEV_PRIVATE_TO_VF(dev->data->dev_private);
+
+	adapter->reset_store_data = rte_zmalloc("i40evf_store_reset",
+					sizeof(struct i40e_vf_reset_store), 0);
+	if (adapter->reset_store_data == NULL) {
+		PMD_INIT_LOG(ERR, "Failed to allocate %ld bytes needed to"
+				" to store data when reset vf",
+				sizeof(struct i40e_vf_reset_store));
+		return -ENOMEM;
+	}
+	store_data =
+		(struct i40e_vf_reset_store *)adapter->reset_store_data;
+	store_data->mac_addrs = rte_zmalloc("i40evf_mac_store_reset",
+			ETHER_ADDR_LEN * I40E_NUM_MACADDR_MAX, 0);
+	if (store_data->mac_addrs == NULL) {
+		PMD_INIT_LOG(ERR, "Failed to allocate %d bytes needed to"
+				" to store MAC addresses when reset vf",
+				ETHER_ADDR_LEN * I40E_NUM_MACADDR_MAX);
+	}
+
+	memcpy(store_data->mac_addrs, dev->data->mac_addrs,
+			ETHER_ADDR_LEN * I40E_NUM_MACADDR_MAX);
+
+	store_data->promisc_unicast_enabled = vf->promisc_unicast_enabled;
+	store_data->promisc_multicast_enabled = vf->promisc_multicast_enabled;
+
+	store_data->vlan_num = vf->vsi.vlan_num;
+	memcpy(store_data->vfta, vf->vsi.vfta,
+			sizeof(uint32_t) * I40E_VFTA_SIZE);
+
+	store_data->mac_num = vf->vsi.mac_num;
+
+	return 0;
+}
+
+static void
+i40evf_restore_after_reset(struct rte_eth_dev *dev)
+{
+	struct i40e_adapter *adapter =
+		I40E_DEV_PRIVATE_TO_ADAPTER(dev->data->dev_private);
+	struct i40e_vf_reset_store *store_data =
+		(struct i40e_vf_reset_store *)adapter->reset_store_data;
+
+	if (store_data->promisc_unicast_enabled)
+		i40evf_dev_promiscuous_enable(dev);
+	if (store_data->promisc_multicast_enabled)
+		i40evf_dev_allmulticast_enable(dev);
+
+	if (store_data->vlan_num)
+		i40evf_restore_vlan_filter(dev, store_data->vfta);
+
+	if (store_data->mac_num)
+		i40evf_restore_macaddr(dev, store_data->mac_addrs);
+
+	rte_free(store_data->mac_addrs);
+	rte_free(adapter->reset_store_data);
+}
+
+static int
+i40evf_reset_dev(struct rte_eth_dev *dev)
+{
+	struct i40e_adapter *adapter =
+		I40E_DEV_PRIVATE_TO_ADAPTER(dev->data->dev_private);
+
+	adapter->reset_flag = 1;
+	i40evf_store_before_reset(dev);
+
+	i40evf_dev_close(dev);
+	PMD_DRV_LOG(DEBUG, "i40evf dev close complete");
+
+	i40evf_dev_uninit(dev);
+	PMD_DRV_LOG(DEBUG, "i40evf dev detached");
+
+	memset(dev->data->dev_private, 0,
+	       (uint64_t)&adapter->reset_flag - (uint64_t)adapter);
+
+	i40evf_dev_init(dev);
+	PMD_DRV_LOG(DEBUG, "i40evf dev attached");
+
+	i40evf_dev_configure(dev);
+
+	i40e_vf_queue_reset(dev);
+	PMD_DRV_LOG(DEBUG, "i40evf queue reset");
+
+	i40evf_restore_after_reset(dev);
+
+	i40evf_dev_start(dev);
+	PMD_DRV_LOG(DEBUG, "i40evf dev restart");
+
+	adapter->reset_flag = 0;
+
+	return 0;
+}
diff --git a/drivers/net/i40e/i40e_rxtx.c b/drivers/net/i40e/i40e_rxtx.c
index 02367b7..d891a54 100644
--- a/drivers/net/i40e/i40e_rxtx.c
+++ b/drivers/net/i40e/i40e_rxtx.c
@@ -1709,6 +1709,7 @@ i40e_dev_rx_queue_setup(struct rte_eth_dev *dev,
 	uint16_t len, i;
 	uint16_t base, bsf, tc_mapping;
 	int use_def_burst_func = 1;
+	struct rte_eth_rxconf conf = *rx_conf;
 
 	if (hw->mac.type == I40E_MAC_VF || hw->mac.type == I40E_MAC_X722_VF) {
 		struct i40e_vf *vf =
@@ -1748,6 +1749,8 @@ i40e_dev_rx_queue_setup(struct rte_eth_dev *dev,
 	}
 	rxq->mp = mp;
 	rxq->nb_rx_desc = nb_desc;
+	rxq->socket_id = socket_id;
+	rxq->rxconf = conf;
 	rxq->rx_free_thresh = rx_conf->rx_free_thresh;
 	rxq->queue_id = queue_idx;
 	if (hw->mac.type == I40E_MAC_VF || hw->mac.type == I40E_MAC_X722_VF)
@@ -1932,6 +1935,7 @@ i40e_dev_tx_queue_setup(struct rte_eth_dev *dev,
 	uint32_t ring_size;
 	uint16_t tx_rs_thresh, tx_free_thresh;
 	uint16_t i, base, bsf, tc_mapping;
+	struct rte_eth_txconf conf = *tx_conf;
 
 	if (hw->mac.type == I40E_MAC_VF || hw->mac.type == I40E_MAC_X722_VF) {
 		struct i40e_vf *vf =
@@ -2054,6 +2058,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;
@@ -2520,8 +2526,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_flag)
+		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 a87bdb0..0e3cc19 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.9.3

[dpdk-dev] [PATCH v5 3/3] app/testpmd: add port reset command into testpmd

[Wei Zhao]

Add port reset command into testpmd project,
it is the interface for user to reset port.
And also it is not limit to be used only in vf reset,
but also for pf port reset.But this patch set only
realted to vf reset feature.

Signed-off-by: Wei Zhao <wei.zhao1@intel.com>
Signed-off-by: Wenzhuo Lu <wenzhuo.lu@intel.com>
---
 app/test-pmd/cmdline.c | 17 +++++++++----
 app/test-pmd/testpmd.c | 65 ++++++++++++++++++++++++++++++++++++++++++++++++++
 app/test-pmd/testpmd.h |  1 +
 3 files changed, 79 insertions(+), 4 deletions(-)

diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c
index 47f935d..6cbdcc8 100644
--- a/app/test-pmd/cmdline.c
+++ b/app/test-pmd/cmdline.c
@@ -599,6 +599,9 @@ static void cmd_help_long_parsed(void *parsed_result,
 			"port close (port_id|all)\n"
 			"    Close all ports or port_id.\n\n"
 
+			"port reset (port_id|all)\n"
+			"    Reset all ports or port_id.\n\n"
+
 			"port attach (ident)\n"
 			"    Attach physical or virtual dev by pci address or virtual device name\n\n"
 
@@ -909,6 +912,8 @@ static void cmd_operate_port_parsed(void *parsed_result,
 		stop_port(RTE_PORT_ALL);
 	else if (!strcmp(res->name, "close"))
 		close_port(RTE_PORT_ALL);
+	else if (!strcmp(res->name, "reset"))
+		reset_port(RTE_PORT_ALL);
 	else
 		printf("Unknown parameter\n");
 }
@@ -918,14 +923,15 @@ cmdline_parse_token_string_t cmd_operate_port_all_cmd =
 								"port");
 cmdline_parse_token_string_t cmd_operate_port_all_port =
 	TOKEN_STRING_INITIALIZER(struct cmd_operate_port_result, name,
-						"start#stop#close");
+						"start#stop#close#reset");
 cmdline_parse_token_string_t cmd_operate_port_all_all =
 	TOKEN_STRING_INITIALIZER(struct cmd_operate_port_result, value, "all");
 
 cmdline_parse_inst_t cmd_operate_port = {
 	.f = cmd_operate_port_parsed,
 	.data = NULL,
-	.help_str = "port start|stop|close all: Start/Stop/Close all ports",
+	.help_str = "port start|stop|close|reset all: Start/Stop/Close/"
+			"Reset all ports",
 	.tokens = {
 		(void *)&cmd_operate_port_all_cmd,
 		(void *)&cmd_operate_port_all_port,
@@ -953,6 +959,8 @@ static void cmd_operate_specific_port_parsed(void *parsed_result,
 		stop_port(res->value);
 	else if (!strcmp(res->name, "close"))
 		close_port(res->value);
+	else if (!strcmp(res->name, "reset"))
+		reset_port(res->value);
 	else
 		printf("Unknown parameter\n");
 }
@@ -962,7 +970,7 @@ cmdline_parse_token_string_t cmd_operate_specific_port_cmd =
 							keyword, "port");
 cmdline_parse_token_string_t cmd_operate_specific_port_port =
 	TOKEN_STRING_INITIALIZER(struct cmd_operate_specific_port_result,
-						name, "start#stop#close");
+						name, "start#stop#close#reset");
 cmdline_parse_token_num_t cmd_operate_specific_port_id =
 	TOKEN_NUM_INITIALIZER(struct cmd_operate_specific_port_result,
 							value, UINT8);
@@ -970,7 +978,8 @@ cmdline_parse_token_num_t cmd_operate_specific_port_id =
 cmdline_parse_inst_t cmd_operate_specific_port = {
 	.f = cmd_operate_specific_port_parsed,
 	.data = NULL,
-	.help_str = "port start|stop|close <port_id>: Start/Stop/Close port_id",
+	.help_str = "port start|stop|close|reset <port_id>: Start/Stop/Close/"
+			"Reset port_id",
 	.tokens = {
 		(void *)&cmd_operate_specific_port_cmd,
 		(void *)&cmd_operate_specific_port_port,
diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c
index 484c19b..a4f5330 100644
--- a/app/test-pmd/testpmd.c
+++ b/app/test-pmd/testpmd.c
@@ -137,6 +137,7 @@ portid_t  nb_fwd_ports;  /**< Number of forwarding ports. */
 
 unsigned int fwd_lcores_cpuids[RTE_MAX_LCORE]; /**< CPU ids configuration. */
 portid_t fwd_ports_ids[RTE_MAX_ETHPORTS];      /**< Port ids configuration. */
+volatile char reset_ports[RTE_MAX_ETHPORTS] = {0};  /**< Port reset flag. */
 
 struct fwd_stream **fwd_streams; /**< For each RX queue of each port. */
 streamid_t nb_fwd_streams;       /**< Is equal to (nb_ports * nb_rxq). */
@@ -1305,6 +1306,18 @@ port_is_closed(portid_t port_id)
 	return 1;
 }
 
+static void
+reset_event_callback(uint8_t port_id, enum rte_eth_event_type type, void *param)
+{
+	RTE_SET_USED(param);
+
+	printf("Event type: %s on port %d\n",
+		type == RTE_ETH_EVENT_INTR_RESET ? "RESET interrupt" :
+		"unknown event", port_id);
+	reset_ports[port_id] = 1;
+	rte_wmb();
+}
+
 int
 start_port(portid_t pid)
 {
@@ -1350,6 +1363,10 @@ start_port(portid_t pid)
 				return -1;
 			}
 		}
+
+		/* register reset interrupt callback */
+		rte_eth_dev_callback_register(pi, RTE_ETH_EVENT_INTR_RESET,
+			reset_event_callback, NULL);
 		if (port->need_reconfig_queues > 0) {
 			port->need_reconfig_queues = 0;
 			/* setup tx queues */
@@ -1559,6 +1576,54 @@ close_port(portid_t pid)
 }
 
 void
+reset_port(portid_t pid)
+{
+	portid_t pi;
+	struct rte_port *port;
+
+	if (port_id_is_invalid(pid, ENABLED_WARN))
+		return;
+
+	printf("Reset ports...\n");
+
+	FOREACH_PORT(pi, ports) {
+		if (pid != pi && pid != (portid_t)RTE_PORT_ALL)
+			continue;
+
+		if (port_is_forwarding(pi) != 0 && test_done == 0) {
+			printf("Please remove port %d from forwarding "
+					"configuration.\n", pi);
+			continue;
+		}
+
+		if (port_is_bonding_slave(pi)) {
+			printf("Please remove port %d from "
+					"bonded device.\n", pi);
+			continue;
+		}
+
+		if (!reset_ports[pi]) {
+			printf("vf must get reset port %d info from "
+					"pf before reset.\n", pi);
+			continue;
+		}
+
+		port = &ports[pi];
+		if (rte_atomic16_cmpset(&(port->port_status),
+			RTE_PORT_STOPPED, RTE_PORT_HANDLING) == 1) {
+			printf("Port %d is not stopped\n", pi);
+			continue;
+		}
+
+		rte_eth_dev_reset(pi);
+		reset_ports[pi] = 0;
+		rte_wmb();
+	}
+
+	printf("Done\n");
+}
+
+void
 attach_port(char *identifier)
 {
 	portid_t pi = 0;
diff --git a/app/test-pmd/testpmd.h b/app/test-pmd/testpmd.h
index 8cf2860..0c7e44c 100644
--- a/app/test-pmd/testpmd.h
+++ b/app/test-pmd/testpmd.h
@@ -586,6 +586,7 @@ int init_port_dcb_config(portid_t pid, enum dcb_mode_enable dcb_mode,
 int start_port(portid_t pid);
 void stop_port(portid_t pid);
 void close_port(portid_t pid);
+void reset_port(portid_t pid);
 void attach_port(char *identifier);
 void detach_port(uint8_t port_id);
 int all_ports_stopped(void);
-- 
2.9.3

[dpdk-dev] [PATCH v6 0/3] net/i40e: vf port reset

[Wei Zhao]

The patches mainly finish following functions:
1) get pf reset vf comamand falg from interrupt server function.
2) reset vf port from testpmd app using a command.
3) sore and restore vf related parameters.

v2:
-change the order of patch in the set.
-add more details in patch comment to clarify that
 the rte and tespmd patch can also used in pf port reset.
-add  rte_free for memory free after restore.
-change varible name of reset_number to reset_falg.
-fix patch check warning.

v3:
-fix error of author mail address

v4:
-fix typo error
-add rte_wmb() after change the reset_ports.

v5:
-add more comments for rte reset function. 

v6:
-fix checkpatch warning.

zhao wei (3):
  app/testpmd: add port reset command into testpmd
  lib/librte_ether: add support for port reset
  net/i40e: implement device reset on port

 app/test-pmd/cmdline.c                 |  17 ++-
 app/test-pmd/testpmd.c                 |  65 ++++++++++++
 app/test-pmd/testpmd.h                 |   1 +
 drivers/net/i40e/i40e_ethdev.c         |   2 +-
 drivers/net/i40e/i40e_ethdev.h         |  16 +++
 drivers/net/i40e/i40e_ethdev_vf.c      | 185 +++++++++++++++++++++++++++++++++
 drivers/net/i40e/i40e_rxtx.c           |  10 ++
 drivers/net/i40e/i40e_rxtx.h           |   4 +
 lib/librte_ether/rte_ethdev.c          |  17 +++
 lib/librte_ether/rte_ethdev.h          |  28 +++++
 lib/librte_ether/rte_ether_version.map |   6 ++
 11 files changed, 346 insertions(+), 5 deletions(-)

-- 
2.9.3

Acked-by: Jingjing Wu <jingjing.wu@intel.com>

[dpdk-dev] [PATCH v6 1/3] lib/librte_ether: add support for port reset

[Wei Zhao]

Add support for port reset in rte layer.This reset
feature can not only used in vf port reset in later code
develop, but alsopf port.But in this patch set, we only
limit the discussion scope to vf reset.
This patch Add an API to restart the device.
It's for VF device in this scenario, kernel PF + DPDK VF.
When the PF port down->up, APP should call this API to
restart VF port. Most likely, APP should call it in its
management thread and guarantee the thread safe. It means
APP should stop the rx/tx and the device, then restart
the device, then recover the device and rx/tx.This API can
also do some restore work for the port.

Signed-off-by: Wenzhuo Lu <wenzhuo.lu@intel.com>
Signed-off-by: Wei Zhao <wei.zhao1@intel.com>
---
 lib/librte_ether/rte_ethdev.c          | 17 +++++++++++++++++
 lib/librte_ether/rte_ethdev.h          | 28 ++++++++++++++++++++++++++++
 lib/librte_ether/rte_ether_version.map |  6 ++++++
 3 files changed, 51 insertions(+)

diff --git a/lib/librte_ether/rte_ethdev.c b/lib/librte_ether/rte_ethdev.c
index eb0a94a..2e06dca 100644
--- a/lib/librte_ether/rte_ethdev.c
+++ b/lib/librte_ether/rte_ethdev.c
@@ -3273,3 +3273,20 @@ rte_eth_dev_l2_tunnel_offload_set(uint8_t port_id,
 				-ENOTSUP);
 	return (*dev->dev_ops->l2_tunnel_offload_set)(dev, l2_tunnel, mask, en);
 }
+
+int
+rte_eth_dev_reset(uint8_t port_id)
+{
+	struct rte_eth_dev *dev;
+	int diag;
+
+	RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -ENODEV);
+
+	dev = &rte_eth_devices[port_id];
+
+	RTE_FUNC_PTR_OR_ERR_RET(*dev->dev_ops->dev_reset, -ENOTSUP);
+
+	diag = (*dev->dev_ops->dev_reset)(dev);
+
+	return diag;
+}
diff --git a/lib/librte_ether/rte_ethdev.h b/lib/librte_ether/rte_ethdev.h
index 4be217c..8287c50 100644
--- a/lib/librte_ether/rte_ethdev.h
+++ b/lib/librte_ether/rte_ethdev.h
@@ -1367,6 +1367,9 @@ typedef int (*eth_l2_tunnel_offload_set_t)
 	 uint8_t en);
 /**< @internal enable/disable the l2 tunnel offload functions */
 
+typedef int  (*eth_dev_reset_t)(struct rte_eth_dev *dev);
+/**< @internal Function used to reset a configured Ethernet device. */
+
 #ifdef RTE_NIC_BYPASS
 
 enum {
@@ -1509,6 +1512,9 @@ struct eth_dev_ops {
 	eth_l2_tunnel_offload_set_t   l2_tunnel_offload_set;
 	/** Enable/disable l2 tunnel offload functions. */
 
+	/** Reset device. */
+	eth_dev_reset_t dev_reset;
+
 	eth_set_queue_rate_limit_t set_queue_rate_limit; /**< Set queue rate limit. */
 
 	rss_hash_update_t          rss_hash_update; /** Configure RSS hash protocols. */
@@ -4413,6 +4419,28 @@ int
 rte_eth_dev_get_name_by_port(uint8_t port_id, char *name);
 
 /**
+ * Reset an ethernet device when it's not working. One scenario is, after PF
+ * port is down and up, the related VF port should be reset.
+ * The API will stop the port, clear the rx/tx queues, re-setup the rx/tx
+ * queues, restart the port.
+ * Before calling this API, APP should stop the rx/tx. When tx is being stopped,
+ * APP can drop the packets and release the buffer instead of sending them.
+ * This function can also do some restore work for the port, for example, it can
+ * restore the added parameters of vlan,  mac_addrs, promisc_unicast_enabled
+ * flag and promisc_multicast_enabled flag.
+ *
+ * @param port_id
+ *   The port identifier of the Ethernet device.
+ *
+ * @return
+ *   - (0) if successful.
+ *   - (-ENODEV) if port identifier is invalid.
+ *   - (-ENOTSUP) if hardware doesn't support this function.
+ */
+int
+rte_eth_dev_reset(uint8_t port_id);
+
+/**
  * @internal
  * Wrapper for use by pci drivers as a .probe function to attach to a ethdev
  * interface.
diff --git a/lib/librte_ether/rte_ether_version.map b/lib/librte_ether/rte_ether_version.map
index c6c9d0d..529b27f 100644
--- a/lib/librte_ether/rte_ether_version.map
+++ b/lib/librte_ether/rte_ether_version.map
@@ -154,3 +154,9 @@ DPDK_17.02 {
 	rte_flow_validate;
 
 } DPDK_16.11;
+
+DPDK_17.05 {
+	global:
+
+	rte_eth_dev_reset;
+} DPDK_17.02;
\ No newline at end of file
-- 
2.9.3

Re: [dpdk-dev] [PATCH v6 1/3] lib/librte_ether: add support for port reset

[Yang, Qiming]

Hi, Wei

> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Wei Zhao
> Sent: Thursday, April 6, 2017 2:51 PM
> To: dev@dpdk.org
> Cc: Lu, Wenzhuo <wenzhuo.lu@intel.com>; Zhao1, Wei
> <wei.zhao1@intel.com>
> Subject: [dpdk-dev] [PATCH v6 1/3] lib/librte_ether: add support for port
> reset
> 
> Add support for port reset in rte layer.This reset feature can not only used in
> vf port reset in later code develop, but alsopf port.But in this patch set, we
> only limit the discussion scope to vf reset.
> This patch Add an API to restart the device.

' alsopf' should add space. 'Add' should be lowercase.

> It's for VF device in this scenario, kernel PF + DPDK VF.
> When the PF port down->up, APP should call this API to restart VF port. Most
> likely, APP should call it in its management thread and guarantee the thread
> safe. It means APP should stop the rx/tx and the device, then restart the
> device, then recover the device and rx/tx.This API can also do some restore
> work for the port.

Please check the grammar problems in this paragraph.
> 
> Signed-off-by: Wenzhuo Lu <wenzhuo.lu@intel.com>
> Signed-off-by: Wei Zhao <wei.zhao1@intel.com>
> ---
>  lib/librte_ether/rte_ethdev.c          | 17 +++++++++++++++++
>  lib/librte_ether/rte_ethdev.h          | 28 ++++++++++++++++++++++++++++
>  lib/librte_ether/rte_ether_version.map |  6 ++++++
>  3 files changed, 51 insertions(+)
> 
> diff --git a/lib/librte_ether/rte_ethdev.c b/lib/librte_ether/rte_ethdev.c
> index eb0a94a..2e06dca 100644
> --- a/lib/librte_ether/rte_ethdev.c
> +++ b/lib/librte_ether/rte_ethdev.c
> @@ -3273,3 +3273,20 @@ rte_eth_dev_l2_tunnel_offload_set(uint8_t
> port_id,
>  				-ENOTSUP);
>  	return (*dev->dev_ops->l2_tunnel_offload_set)(dev, l2_tunnel,
> mask, en);  }
> +
> +int
> +rte_eth_dev_reset(uint8_t port_id)
> +{
> +	struct rte_eth_dev *dev;
> +	int diag;
> +
> +	RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -ENODEV);
> +
> +	dev = &rte_eth_devices[port_id];
> +
> +	RTE_FUNC_PTR_OR_ERR_RET(*dev->dev_ops->dev_reset, -
> ENOTSUP);
> +
> +	diag = (*dev->dev_ops->dev_reset)(dev);
> +
> +	return diag;
> +}
> diff --git a/lib/librte_ether/rte_ethdev.h b/lib/librte_ether/rte_ethdev.h
> index 4be217c..8287c50 100644
> --- a/lib/librte_ether/rte_ethdev.h
> +++ b/lib/librte_ether/rte_ethdev.h
> @@ -1367,6 +1367,9 @@ typedef int (*eth_l2_tunnel_offload_set_t)
>  	 uint8_t en);
>  /**< @internal enable/disable the l2 tunnel offload functions */
> 
> +typedef int  (*eth_dev_reset_t)(struct rte_eth_dev *dev); /**<
> +@internal Function used to reset a configured Ethernet device. */
> +
>  #ifdef RTE_NIC_BYPASS
> 
>  enum {
> @@ -1509,6 +1512,9 @@ struct eth_dev_ops {
>  	eth_l2_tunnel_offload_set_t   l2_tunnel_offload_set;
>  	/** Enable/disable l2 tunnel offload functions. */
> 
> +	/** Reset device. */
> +	eth_dev_reset_t dev_reset;
> +
>  	eth_set_queue_rate_limit_t set_queue_rate_limit; /**< Set queue
> rate limit. */
> 
>  	rss_hash_update_t          rss_hash_update; /** Configure RSS hash
> protocols. */
> @@ -4413,6 +4419,28 @@ int
>  rte_eth_dev_get_name_by_port(uint8_t port_id, char *name);
> 
>  /**
> + * Reset an ethernet device when it's not working. One scenario is,
> +after PF
> + * port is down and up, the related VF port should be reset.

Is there have other scenario? I don't know what is 'down and up' means? Can you put it another way? 

> + * The API will stop the port, clear the rx/tx queues, re-setup the
> +rx/tx
> + * queues, restart the port.
> + * Before calling this API, APP should stop the rx/tx. When tx is being
> +stopped,
> + * APP can drop the packets and release the buffer instead of sending them.
> + * This function can also do some restore work for the port, for
> +example, it can
> + * restore the added parameters of vlan,  mac_addrs,
> +promisc_unicast_enabled
> + * flag and promisc_multicast_enabled flag.
> + *
> + * @param port_id
> + *   The port identifier of the Ethernet device.
> + *
> + * @return
> + *   - (0) if successful.
> + *   - (-ENODEV) if port identifier is invalid.
> + *   - (-ENOTSUP) if hardware doesn't support this function.
> + */
> +int
> +rte_eth_dev_reset(uint8_t port_id);
> +
> +/**
>   * @internal
>   * Wrapper for use by pci drivers as a .probe function to attach to a ethdev
>   * interface.
> diff --git a/lib/librte_ether/rte_ether_version.map
> b/lib/librte_ether/rte_ether_version.map
> index c6c9d0d..529b27f 100644
> --- a/lib/librte_ether/rte_ether_version.map
> +++ b/lib/librte_ether/rte_ether_version.map
> @@ -154,3 +154,9 @@ DPDK_17.02 {
>  	rte_flow_validate;
> 
>  } DPDK_16.11;
> +
> +DPDK_17.05 {
> +	global:
> +
> +	rte_eth_dev_reset;
> +} DPDK_17.02;
> \ No newline at end of file
> --
> 2.9.3

Re: [dpdk-dev] [PATCH v6 1/3] lib/librte_ether: add support for port reset

[Zhao1, Wei]

Hi, Qiming

> -----Original Message-----
> From: Yang, Qiming
> Sent: Friday, April 7, 2017 2:59 PM
> To: Zhao1, Wei <wei.zhao1@intel.com>; dev@dpdk.org
> Cc: Lu, Wenzhuo <wenzhuo.lu@intel.com>; Zhao1, Wei
> <wei.zhao1@intel.com>
> Subject: RE: [dpdk-dev] [PATCH v6 1/3] lib/librte_ether: add support for port
> reset
> 
> Hi, Wei
> 
> > -----Original Message-----
> > From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Wei Zhao
> > Sent: Thursday, April 6, 2017 2:51 PM
> > To: dev@dpdk.org
> > Cc: Lu, Wenzhuo <wenzhuo.lu@intel.com>; Zhao1, Wei
> > <wei.zhao1@intel.com>
> > Subject: [dpdk-dev] [PATCH v6 1/3] lib/librte_ether: add support for
> > port reset
> >
> > Add support for port reset in rte layer.This reset feature can not
> > only used in vf port reset in later code develop, but alsopf port.But
> > in this patch set, we only limit the discussion scope to vf reset.
> > This patch Add an API to restart the device.
> 
> ' alsopf' should add space. 'Add' should be lowercase.


Ok, I will fix it in next version.
 
> 
> > It's for VF device in this scenario, kernel PF + DPDK VF.
> > When the PF port down->up, APP should call this API to restart VF
> > port. Most likely, APP should call it in its management thread and
> > guarantee the thread safe. It means APP should stop the rx/tx and the
> > device, then restart the device, then recover the device and
> > rx/tx.This API can also do some restore work for the port.
> 
> Please check the grammar problems in this paragraph.

Ok, I will do a double check.

> >
> > Signed-off-by: Wenzhuo Lu <wenzhuo.lu@intel.com>
> > Signed-off-by: Wei Zhao <wei.zhao1@intel.com>
> > ---
> >  lib/librte_ether/rte_ethdev.c          | 17 +++++++++++++++++
> >  lib/librte_ether/rte_ethdev.h          | 28
> ++++++++++++++++++++++++++++
> >  lib/librte_ether/rte_ether_version.map |  6 ++++++
> >  3 files changed, 51 insertions(+)
> >
> > diff --git a/lib/librte_ether/rte_ethdev.c
> > b/lib/librte_ether/rte_ethdev.c index eb0a94a..2e06dca 100644
> > --- a/lib/librte_ether/rte_ethdev.c
> > +++ b/lib/librte_ether/rte_ethdev.c
> > @@ -3273,3 +3273,20 @@ rte_eth_dev_l2_tunnel_offload_set(uint8_t
> > port_id,
> >  				-ENOTSUP);
> >  	return (*dev->dev_ops->l2_tunnel_offload_set)(dev, l2_tunnel,
> mask,
> > en);  }
> > +
> > +int
> > +rte_eth_dev_reset(uint8_t port_id)
> > +{
> > +	struct rte_eth_dev *dev;
> > +	int diag;
> > +
> > +	RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -ENODEV);
> > +
> > +	dev = &rte_eth_devices[port_id];
> > +
> > +	RTE_FUNC_PTR_OR_ERR_RET(*dev->dev_ops->dev_reset, -
> > ENOTSUP);
> > +
> > +	diag = (*dev->dev_ops->dev_reset)(dev);
> > +
> > +	return diag;
> > +}
> > diff --git a/lib/librte_ether/rte_ethdev.h
> > b/lib/librte_ether/rte_ethdev.h index 4be217c..8287c50 100644
> > --- a/lib/librte_ether/rte_ethdev.h
> > +++ b/lib/librte_ether/rte_ethdev.h
> > @@ -1367,6 +1367,9 @@ typedef int (*eth_l2_tunnel_offload_set_t)
> >  	 uint8_t en);
> >  /**< @internal enable/disable the l2 tunnel offload functions */
> >
> > +typedef int  (*eth_dev_reset_t)(struct rte_eth_dev *dev); /**<
> > +@internal Function used to reset a configured Ethernet device. */
> > +
> >  #ifdef RTE_NIC_BYPASS
> >
> >  enum {
> > @@ -1509,6 +1512,9 @@ struct eth_dev_ops {
> >  	eth_l2_tunnel_offload_set_t   l2_tunnel_offload_set;
> >  	/** Enable/disable l2 tunnel offload functions. */
> >
> > +	/** Reset device. */
> > +	eth_dev_reset_t dev_reset;
> > +
> >  	eth_set_queue_rate_limit_t set_queue_rate_limit; /**< Set queue
> rate
> > limit. */
> >
> >  	rss_hash_update_t          rss_hash_update; /** Configure RSS hash
> > protocols. */
> > @@ -4413,6 +4419,28 @@ int
> >  rte_eth_dev_get_name_by_port(uint8_t port_id, char *name);
> >
> >  /**
> > + * Reset an ethernet device when it's not working. One scenario is,
> > +after PF
> > + * port is down and up, the related VF port should be reset.
> 
> Is there have other scenario? I don't know what is 'down and up' means? Can
> you put it another way?

"down and up" ,you can see testpmd has the state for all DPDK bound port 
The scenario for pf to reset vf is too much to list all, for example, 
" ifconfig port promisc " can also cause the reset.

> 
> > + * The API will stop the port, clear the rx/tx queues, re-setup the
> > +rx/tx
> > + * queues, restart the port.
> > + * Before calling this API, APP should stop the rx/tx. When tx is
> > +being stopped,
> > + * APP can drop the packets and release the buffer instead of sending
> them.
> > + * This function can also do some restore work for the port, for
> > +example, it can
> > + * restore the added parameters of vlan,  mac_addrs,
> > +promisc_unicast_enabled
> > + * flag and promisc_multicast_enabled flag.
> > + *
> > + * @param port_id
> > + *   The port identifier of the Ethernet device.
> > + *
> > + * @return
> > + *   - (0) if successful.
> > + *   - (-ENODEV) if port identifier is invalid.
> > + *   - (-ENOTSUP) if hardware doesn't support this function.
> > + */
> > +int
> > +rte_eth_dev_reset(uint8_t port_id);
> > +
> > +/**
> >   * @internal
> >   * Wrapper for use by pci drivers as a .probe function to attach to a ethdev
> >   * interface.
> > diff --git a/lib/librte_ether/rte_ether_version.map
> > b/lib/librte_ether/rte_ether_version.map
> > index c6c9d0d..529b27f 100644
> > --- a/lib/librte_ether/rte_ether_version.map
> > +++ b/lib/librte_ether/rte_ether_version.map
> > @@ -154,3 +154,9 @@ DPDK_17.02 {
> >  	rte_flow_validate;
> >
> >  } DPDK_16.11;
> > +
> > +DPDK_17.05 {
> > +	global:
> > +
> > +	rte_eth_dev_reset;
> > +} DPDK_17.02;
> > \ No newline at end of file
> > --
> > 2.9.3

[dpdk-dev] [PATCH v6 2/3] net/i40e: implement device reset on port

[Wei Zhao]

This patch implement the device reset function on vf port.
This restart function will detach device then
attach device, reconfigure dev, re-setup
the Rx/Tx queues.

Signed-off-by: Wei Zhao <wei.zhao1@intel.com>
Signed-off-by: Wenzhuo Lu <wenzhuo.lu@intel.com>
---
 drivers/net/i40e/i40e_ethdev.c    |   2 +-
 drivers/net/i40e/i40e_ethdev.h    |  16 ++++
 drivers/net/i40e/i40e_ethdev_vf.c | 185 ++++++++++++++++++++++++++++++++++++++
 drivers/net/i40e/i40e_rxtx.c      |  10 +++
 drivers/net/i40e/i40e_rxtx.h      |   4 +
 5 files changed, 216 insertions(+), 1 deletion(-)

diff --git a/drivers/net/i40e/i40e_ethdev.c b/drivers/net/i40e/i40e_ethdev.c
index 3702214..f2fdb2f 100644
--- a/drivers/net/i40e/i40e_ethdev.c
+++ b/drivers/net/i40e/i40e_ethdev.c
@@ -6020,7 +6020,7 @@ i40e_find_vlan_filter(struct i40e_vsi *vsi,
 		return 0;
 }
 
-static void
+void
 i40e_store_vlan_filter(struct i40e_vsi *vsi,
 		       uint16_t vlan_id, bool on)
 {
diff --git a/drivers/net/i40e/i40e_ethdev.h b/drivers/net/i40e/i40e_ethdev.h
index aebb097..515a288 100644
--- a/drivers/net/i40e/i40e_ethdev.h
+++ b/drivers/net/i40e/i40e_ethdev.h
@@ -652,6 +652,16 @@ struct i40e_vf_tx_queues {
 	uint32_t tx_ring_len;
 };
 
+struct i40e_vf_reset_store {
+	struct ether_addr *mac_addrs;  /* Device Ethernet Link address. */
+	bool promisc_unicast_enabled;
+	bool promisc_multicast_enabled;
+	uint16_t vlan_num;    /* Total VLAN number */
+	uint32_t vfta[I40E_VFTA_SIZE];        /* VLAN bitmap */
+	uint16_t mac_num;        /* Total mac number */
+};
+
+
 /*
  * Structure to store private data specific for VF instance.
  */
@@ -709,6 +719,10 @@ struct i40e_adapter {
 	struct rte_timecounter systime_tc;
 	struct rte_timecounter rx_tstamp_tc;
 	struct rte_timecounter tx_tstamp_tc;
+
+	/* For port reset */
+	volatile uint8_t reset_flag;
+	void *reset_store_data;
 };
 
 extern const struct rte_flow_ops i40e_flow_ops;
@@ -749,6 +763,8 @@ int i40e_dev_link_update(struct rte_eth_dev *dev,
 			 __rte_unused int wait_to_complete);
 void i40e_vsi_queues_bind_intr(struct i40e_vsi *vsi);
 void i40e_vsi_queues_unbind_intr(struct i40e_vsi *vsi);
+void i40e_store_vlan_filter(struct i40e_vsi *vsi,
+		       uint16_t vlan_id, bool on);
 int i40e_vsi_vlan_pvid_set(struct i40e_vsi *vsi,
 			   struct i40e_vsi_vlan_pvid_info *info);
 int i40e_vsi_config_vlan_stripping(struct i40e_vsi *vsi, bool on);
diff --git a/drivers/net/i40e/i40e_ethdev_vf.c b/drivers/net/i40e/i40e_ethdev_vf.c
index 55fd344..9ab035c 100644
--- a/drivers/net/i40e/i40e_ethdev_vf.c
+++ b/drivers/net/i40e/i40e_ethdev_vf.c
@@ -161,6 +161,9 @@ i40evf_dev_rx_queue_intr_disable(struct rte_eth_dev *dev, uint16_t queue_id);
 static void i40evf_handle_pf_event(__rte_unused struct rte_eth_dev *dev,
 				   uint8_t *msg,
 				   uint16_t msglen);
+static int i40evf_dev_uninit(struct rte_eth_dev *eth_dev);
+static int i40evf_dev_init(struct rte_eth_dev *eth_dev);
+static int i40evf_reset_dev(struct rte_eth_dev *dev);
 
 /* Default hash key buffer for RSS */
 static uint32_t rss_key_default[I40E_VFQF_HKEY_MAX_INDEX + 1];
@@ -230,6 +233,7 @@ static const struct eth_dev_ops i40evf_eth_dev_ops = {
 	.rss_hash_conf_get    = i40evf_dev_rss_hash_conf_get,
 	.mtu_set              = i40evf_dev_mtu_set,
 	.mac_addr_set         = i40evf_set_default_mac_addr,
+	.dev_reset            = i40evf_reset_dev,
 };
 
 /*
@@ -889,6 +893,7 @@ i40evf_add_mac_addr(struct rte_eth_dev *dev,
 		PMD_DRV_LOG(ERR, "fail to execute command "
 			    "OP_ADD_ETHER_ADDRESS");
 
+	vf->vsi.mac_num++;
 	return;
 }
 
@@ -926,6 +931,7 @@ i40evf_del_mac_addr_by_addr(struct rte_eth_dev *dev,
 	if (err)
 		PMD_DRV_LOG(ERR, "fail to execute command "
 			    "OP_DEL_ETHER_ADDRESS");
+	vf->vsi.mac_num--;
 	return;
 }
 
@@ -1047,6 +1053,7 @@ static int
 i40evf_add_vlan(struct rte_eth_dev *dev, uint16_t vlanid)
 {
 	struct i40e_vf *vf = I40EVF_DEV_PRIVATE_TO_VF(dev->data->dev_private);
+	struct i40e_vsi *vsi = &vf->vsi;
 	struct i40e_virtchnl_vlan_filter_list *vlan_list;
 	uint8_t cmd_buffer[sizeof(struct i40e_virtchnl_vlan_filter_list) +
 							sizeof(uint16_t)];
@@ -1066,6 +1073,8 @@ i40evf_add_vlan(struct rte_eth_dev *dev, uint16_t vlanid)
 	err = i40evf_execute_vf_cmd(dev, &args);
 	if (err)
 		PMD_DRV_LOG(ERR, "fail to execute command OP_ADD_VLAN");
+	i40e_store_vlan_filter(vsi, vlanid, 1);
+	vsi->vlan_num++;
 
 	return err;
 }
@@ -1074,6 +1083,7 @@ static int
 i40evf_del_vlan(struct rte_eth_dev *dev, uint16_t vlanid)
 {
 	struct i40e_vf *vf = I40EVF_DEV_PRIVATE_TO_VF(dev->data->dev_private);
+	struct i40e_vsi *vsi = &vf->vsi;
 	struct i40e_virtchnl_vlan_filter_list *vlan_list;
 	uint8_t cmd_buffer[sizeof(struct i40e_virtchnl_vlan_filter_list) +
 							sizeof(uint16_t)];
@@ -1093,6 +1103,8 @@ i40evf_del_vlan(struct rte_eth_dev *dev, uint16_t vlanid)
 	err = i40evf_execute_vf_cmd(dev, &args);
 	if (err)
 		PMD_DRV_LOG(ERR, "fail to execute command OP_DEL_VLAN");
+	i40e_store_vlan_filter(vsi, vlanid, 0);
+	vsi->vlan_num--;
 
 	return err;
 }
@@ -2716,3 +2728,176 @@ i40evf_set_default_mac_addr(struct rte_eth_dev *dev,
 
 	i40evf_add_mac_addr(dev, mac_addr, 0, 0);
 }
+
+static void
+i40evf_restore_vlan_filter(struct rte_eth_dev *dev,
+			uint32_t *vfta)
+{
+	uint32_t vid_idx, vid_bit;
+	uint16_t vlan_id;
+
+	for  (vid_idx = 0; vid_idx < I40E_VFTA_SIZE; vid_idx++) {
+		for  (vid_bit = 0; vid_bit < I40E_UINT32_BIT_SIZE; vid_bit++) {
+			if (vfta[vid_idx] & (1 << vid_bit)) {
+				vlan_id = (vid_idx << 5) + vid_bit;
+				i40evf_add_vlan(dev, vlan_id);
+			}
+		}
+	}
+}
+
+static void
+i40evf_restore_macaddr(struct rte_eth_dev *dev,
+		struct ether_addr *mac_addrs)
+{
+	struct ether_addr *addr;
+	uint16_t i;
+
+	/* replay MAC address configuration including default MAC */
+	addr = &mac_addrs[0];
+
+	i40evf_set_default_mac_addr(dev, addr);
+	memcpy(dev->data->mac_addrs, mac_addrs,
+			ETHER_ADDR_LEN * I40E_NUM_MACADDR_MAX);
+
+	for (i = 1; i < I40E_NUM_MACADDR_MAX; i++) {
+		addr = &mac_addrs[i];
+
+		/* skip zero address */
+		if (is_zero_ether_addr(addr))
+			continue;
+
+		i40evf_add_mac_addr(dev, addr, 0, 0);
+	}
+}
+
+
+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_store_before_reset(struct rte_eth_dev *dev)
+{
+	struct i40e_adapter *adapter =
+		I40E_DEV_PRIVATE_TO_ADAPTER(dev->data->dev_private);
+	struct i40e_vf_reset_store *store_data;
+	struct i40e_vf *vf = I40EVF_DEV_PRIVATE_TO_VF(dev->data->dev_private);
+
+	adapter->reset_store_data = rte_zmalloc("i40evf_store_reset",
+					sizeof(struct i40e_vf_reset_store), 0);
+	if (adapter->reset_store_data == NULL) {
+		PMD_INIT_LOG(ERR, "Failed to allocate %ld bytes needed to"
+				" to store data when reset vf",
+				sizeof(struct i40e_vf_reset_store));
+		return -ENOMEM;
+	}
+	store_data =
+		(struct i40e_vf_reset_store *)adapter->reset_store_data;
+	store_data->mac_addrs = rte_zmalloc("i40evf_mac_store_reset",
+			ETHER_ADDR_LEN * I40E_NUM_MACADDR_MAX, 0);
+	if (store_data->mac_addrs == NULL) {
+		PMD_INIT_LOG(ERR, "Failed to allocate %d bytes needed to"
+				" to store MAC addresses when reset vf",
+				ETHER_ADDR_LEN * I40E_NUM_MACADDR_MAX);
+	}
+
+	memcpy(store_data->mac_addrs, dev->data->mac_addrs,
+			ETHER_ADDR_LEN * I40E_NUM_MACADDR_MAX);
+
+	store_data->promisc_unicast_enabled = vf->promisc_unicast_enabled;
+	store_data->promisc_multicast_enabled = vf->promisc_multicast_enabled;
+
+	store_data->vlan_num = vf->vsi.vlan_num;
+	memcpy(store_data->vfta, vf->vsi.vfta,
+			sizeof(uint32_t) * I40E_VFTA_SIZE);
+
+	store_data->mac_num = vf->vsi.mac_num;
+
+	return 0;
+}
+
+static void
+i40evf_restore_after_reset(struct rte_eth_dev *dev)
+{
+	struct i40e_adapter *adapter =
+		I40E_DEV_PRIVATE_TO_ADAPTER(dev->data->dev_private);
+	struct i40e_vf_reset_store *store_data =
+		(struct i40e_vf_reset_store *)adapter->reset_store_data;
+
+	if (store_data->promisc_unicast_enabled)
+		i40evf_dev_promiscuous_enable(dev);
+	if (store_data->promisc_multicast_enabled)
+		i40evf_dev_allmulticast_enable(dev);
+
+	if (store_data->vlan_num)
+		i40evf_restore_vlan_filter(dev, store_data->vfta);
+
+	if (store_data->mac_num)
+		i40evf_restore_macaddr(dev, store_data->mac_addrs);
+
+	rte_free(store_data->mac_addrs);
+	rte_free(adapter->reset_store_data);
+}
+
+static int
+i40evf_reset_dev(struct rte_eth_dev *dev)
+{
+	struct i40e_adapter *adapter =
+		I40E_DEV_PRIVATE_TO_ADAPTER(dev->data->dev_private);
+
+	adapter->reset_flag = 1;
+	i40evf_store_before_reset(dev);
+
+	i40evf_dev_close(dev);
+	PMD_DRV_LOG(DEBUG, "i40evf dev close complete");
+
+	i40evf_dev_uninit(dev);
+	PMD_DRV_LOG(DEBUG, "i40evf dev detached");
+
+	memset(dev->data->dev_private, 0,
+	       (uint64_t)&adapter->reset_flag - (uint64_t)adapter);
+
+	i40evf_dev_init(dev);
+	PMD_DRV_LOG(DEBUG, "i40evf dev attached");
+
+	i40evf_dev_configure(dev);
+
+	i40e_vf_queue_reset(dev);
+	PMD_DRV_LOG(DEBUG, "i40evf queue reset");
+
+	i40evf_restore_after_reset(dev);
+
+	i40evf_dev_start(dev);
+	PMD_DRV_LOG(DEBUG, "i40evf dev restart");
+
+	adapter->reset_flag = 0;
+
+	return 0;
+}
diff --git a/drivers/net/i40e/i40e_rxtx.c b/drivers/net/i40e/i40e_rxtx.c
index 02367b7..d891a54 100644
--- a/drivers/net/i40e/i40e_rxtx.c
+++ b/drivers/net/i40e/i40e_rxtx.c
@@ -1709,6 +1709,7 @@ i40e_dev_rx_queue_setup(struct rte_eth_dev *dev,
 	uint16_t len, i;
 	uint16_t base, bsf, tc_mapping;
 	int use_def_burst_func = 1;
+	struct rte_eth_rxconf conf = *rx_conf;
 
 	if (hw->mac.type == I40E_MAC_VF || hw->mac.type == I40E_MAC_X722_VF) {
 		struct i40e_vf *vf =
@@ -1748,6 +1749,8 @@ i40e_dev_rx_queue_setup(struct rte_eth_dev *dev,
 	}
 	rxq->mp = mp;
 	rxq->nb_rx_desc = nb_desc;
+	rxq->socket_id = socket_id;
+	rxq->rxconf = conf;
 	rxq->rx_free_thresh = rx_conf->rx_free_thresh;
 	rxq->queue_id = queue_idx;
 	if (hw->mac.type == I40E_MAC_VF || hw->mac.type == I40E_MAC_X722_VF)
@@ -1932,6 +1935,7 @@ i40e_dev_tx_queue_setup(struct rte_eth_dev *dev,
 	uint32_t ring_size;
 	uint16_t tx_rs_thresh, tx_free_thresh;
 	uint16_t i, base, bsf, tc_mapping;
+	struct rte_eth_txconf conf = *tx_conf;
 
 	if (hw->mac.type == I40E_MAC_VF || hw->mac.type == I40E_MAC_X722_VF) {
 		struct i40e_vf *vf =
@@ -2054,6 +2058,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;
@@ -2520,8 +2526,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_flag)
+		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 a87bdb0..0e3cc19 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.9.3

[dpdk-dev] [PATCH v6 3/3] app/testpmd: add port reset command into testpmd

[Wei Zhao]

Add port reset command into testpmd project,
it is the interface for user to reset port.
And also it is not limit to be used only in vf reset,
but also for pf port reset.But this patch set only
realted to vf reset feature.

Signed-off-by: Wei Zhao <wei.zhao1@intel.com>
Signed-off-by: Wenzhuo Lu <wenzhuo.lu@intel.com>
---
 app/test-pmd/cmdline.c | 17 +++++++++----
 app/test-pmd/testpmd.c | 65 ++++++++++++++++++++++++++++++++++++++++++++++++++
 app/test-pmd/testpmd.h |  1 +
 3 files changed, 79 insertions(+), 4 deletions(-)

diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c
index 47f935d..6cbdcc8 100644
--- a/app/test-pmd/cmdline.c
+++ b/app/test-pmd/cmdline.c
@@ -599,6 +599,9 @@ static void cmd_help_long_parsed(void *parsed_result,
 			"port close (port_id|all)\n"
 			"    Close all ports or port_id.\n\n"
 
+			"port reset (port_id|all)\n"
+			"    Reset all ports or port_id.\n\n"
+
 			"port attach (ident)\n"
 			"    Attach physical or virtual dev by pci address or virtual device name\n\n"
 
@@ -909,6 +912,8 @@ static void cmd_operate_port_parsed(void *parsed_result,
 		stop_port(RTE_PORT_ALL);
 	else if (!strcmp(res->name, "close"))
 		close_port(RTE_PORT_ALL);
+	else if (!strcmp(res->name, "reset"))
+		reset_port(RTE_PORT_ALL);
 	else
 		printf("Unknown parameter\n");
 }
@@ -918,14 +923,15 @@ cmdline_parse_token_string_t cmd_operate_port_all_cmd =
 								"port");
 cmdline_parse_token_string_t cmd_operate_port_all_port =
 	TOKEN_STRING_INITIALIZER(struct cmd_operate_port_result, name,
-						"start#stop#close");
+						"start#stop#close#reset");
 cmdline_parse_token_string_t cmd_operate_port_all_all =
 	TOKEN_STRING_INITIALIZER(struct cmd_operate_port_result, value, "all");
 
 cmdline_parse_inst_t cmd_operate_port = {
 	.f = cmd_operate_port_parsed,
 	.data = NULL,
-	.help_str = "port start|stop|close all: Start/Stop/Close all ports",
+	.help_str = "port start|stop|close|reset all: Start/Stop/Close/"
+			"Reset all ports",
 	.tokens = {
 		(void *)&cmd_operate_port_all_cmd,
 		(void *)&cmd_operate_port_all_port,
@@ -953,6 +959,8 @@ static void cmd_operate_specific_port_parsed(void *parsed_result,
 		stop_port(res->value);
 	else if (!strcmp(res->name, "close"))
 		close_port(res->value);
+	else if (!strcmp(res->name, "reset"))
+		reset_port(res->value);
 	else
 		printf("Unknown parameter\n");
 }
@@ -962,7 +970,7 @@ cmdline_parse_token_string_t cmd_operate_specific_port_cmd =
 							keyword, "port");
 cmdline_parse_token_string_t cmd_operate_specific_port_port =
 	TOKEN_STRING_INITIALIZER(struct cmd_operate_specific_port_result,
-						name, "start#stop#close");
+						name, "start#stop#close#reset");
 cmdline_parse_token_num_t cmd_operate_specific_port_id =
 	TOKEN_NUM_INITIALIZER(struct cmd_operate_specific_port_result,
 							value, UINT8);
@@ -970,7 +978,8 @@ cmdline_parse_token_num_t cmd_operate_specific_port_id =
 cmdline_parse_inst_t cmd_operate_specific_port = {
 	.f = cmd_operate_specific_port_parsed,
 	.data = NULL,
-	.help_str = "port start|stop|close <port_id>: Start/Stop/Close port_id",
+	.help_str = "port start|stop|close|reset <port_id>: Start/Stop/Close/"
+			"Reset port_id",
 	.tokens = {
 		(void *)&cmd_operate_specific_port_cmd,
 		(void *)&cmd_operate_specific_port_port,
diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c
index 484c19b..a4f5330 100644
--- a/app/test-pmd/testpmd.c
+++ b/app/test-pmd/testpmd.c
@@ -137,6 +137,7 @@ portid_t  nb_fwd_ports;  /**< Number of forwarding ports. */
 
 unsigned int fwd_lcores_cpuids[RTE_MAX_LCORE]; /**< CPU ids configuration. */
 portid_t fwd_ports_ids[RTE_MAX_ETHPORTS];      /**< Port ids configuration. */
+volatile char reset_ports[RTE_MAX_ETHPORTS] = {0};  /**< Port reset flag. */
 
 struct fwd_stream **fwd_streams; /**< For each RX queue of each port. */
 streamid_t nb_fwd_streams;       /**< Is equal to (nb_ports * nb_rxq). */
@@ -1305,6 +1306,18 @@ port_is_closed(portid_t port_id)
 	return 1;
 }
 
+static void
+reset_event_callback(uint8_t port_id, enum rte_eth_event_type type, void *param)
+{
+	RTE_SET_USED(param);
+
+	printf("Event type: %s on port %d\n",
+		type == RTE_ETH_EVENT_INTR_RESET ? "RESET interrupt" :
+		"unknown event", port_id);
+	reset_ports[port_id] = 1;
+	rte_wmb();
+}
+
 int
 start_port(portid_t pid)
 {
@@ -1350,6 +1363,10 @@ start_port(portid_t pid)
 				return -1;
 			}
 		}
+
+		/* register reset interrupt callback */
+		rte_eth_dev_callback_register(pi, RTE_ETH_EVENT_INTR_RESET,
+			reset_event_callback, NULL);
 		if (port->need_reconfig_queues > 0) {
 			port->need_reconfig_queues = 0;
 			/* setup tx queues */
@@ -1559,6 +1576,54 @@ close_port(portid_t pid)
 }
 
 void
+reset_port(portid_t pid)
+{
+	portid_t pi;
+	struct rte_port *port;
+
+	if (port_id_is_invalid(pid, ENABLED_WARN))
+		return;
+
+	printf("Reset ports...\n");
+
+	FOREACH_PORT(pi, ports) {
+		if (pid != pi && pid != (portid_t)RTE_PORT_ALL)
+			continue;
+
+		if (port_is_forwarding(pi) != 0 && test_done == 0) {
+			printf("Please remove port %d from forwarding "
+					"configuration.\n", pi);
+			continue;
+		}
+
+		if (port_is_bonding_slave(pi)) {
+			printf("Please remove port %d from "
+					"bonded device.\n", pi);
+			continue;
+		}
+
+		if (!reset_ports[pi]) {
+			printf("vf must get reset port %d info from "
+					"pf before reset.\n", pi);
+			continue;
+		}
+
+		port = &ports[pi];
+		if (rte_atomic16_cmpset(&(port->port_status),
+			RTE_PORT_STOPPED, RTE_PORT_HANDLING) == 1) {
+			printf("Port %d is not stopped\n", pi);
+			continue;
+		}
+
+		rte_eth_dev_reset(pi);
+		reset_ports[pi] = 0;
+		rte_wmb();
+	}
+
+	printf("Done\n");
+}
+
+void
 attach_port(char *identifier)
 {
 	portid_t pi = 0;
diff --git a/app/test-pmd/testpmd.h b/app/test-pmd/testpmd.h
index 8cf2860..0c7e44c 100644
--- a/app/test-pmd/testpmd.h
+++ b/app/test-pmd/testpmd.h
@@ -586,6 +586,7 @@ int init_port_dcb_config(portid_t pid, enum dcb_mode_enable dcb_mode,
 int start_port(portid_t pid);
 void stop_port(portid_t pid);
 void close_port(portid_t pid);
+void reset_port(portid_t pid);
 void attach_port(char *identifier);
 void detach_port(uint8_t port_id);
 int all_ports_stopped(void);
-- 
2.9.3

[dpdk-dev] [PATCH v7 0/3] net/i40e: vf port reset

[Wei Zhao]

The patches mainly finish following functions:
1) get pf reset vf comamand falg from interrupt server function.
2) reset vf port from testpmd app using a command.
3) sore and restore vf related parameters.

v2:
-change the order of patch in the set.
-add more details in patch comment to clarify that
 the rte and tespmd patch can also used in pf port reset.
-add  rte_free for memory free after restore.
-change varible name of reset_number to reset_falg.
-fix patch check warning.

v3:
-fix error of author mail address

v4:
-fix typo error
-add rte_wmb() after change the reset_ports.

v5:
-add more comments for rte reset function. 

v6:
-fix checkpatch warning.

v7:
-fix typo in patch commet log

zhao wei (3):
  app/testpmd: add port reset command into testpmd
  lib/librte_ether: add support for port reset
  net/i40e: implement device reset on port

 app/test-pmd/cmdline.c                 |  17 ++-
 app/test-pmd/testpmd.c                 |  65 ++++++++++++
 app/test-pmd/testpmd.h                 |   1 +
 drivers/net/i40e/i40e_ethdev.c         |   2 +-
 drivers/net/i40e/i40e_ethdev.h         |  16 +++
 drivers/net/i40e/i40e_ethdev_vf.c      | 185 +++++++++++++++++++++++++++++++++
 drivers/net/i40e/i40e_rxtx.c           |  10 ++
 drivers/net/i40e/i40e_rxtx.h           |   4 +
 lib/librte_ether/rte_ethdev.c          |  17 +++
 lib/librte_ether/rte_ethdev.h          |  28 +++++
 lib/librte_ether/rte_ether_version.map |   6 ++
 11 files changed, 346 insertions(+), 5 deletions(-)

-- 
2.9.3

Acked-by: Jingjing Wu <jingjing.wu@intel.com>

[dpdk-dev] [PATCH v7 1/3] lib/librte_ether: add support for port reset

[Wei Zhao]

Add support for port reset in rte layer.This reset
feature can be used not only in vf port reset in the following
code develop process later, but also pf port.But in this patch
set, we only limit the discussion scope to vf reset.
This patch add an API to restart the device.
It's for VF device in this scenario, kernel PF + DPDK VF.
When the PF port state is down->up, APP should call this API to
restart VF port. Most likely, APP should call it in its
management thread and guarantee the thread safe. It means
APP should stop the rx/tx and the device, then restart
the device, then recover the device and rx/tx.This API can
also do some restore work for the port.

Signed-off-by: Wenzhuo Lu <wenzhuo.lu@intel.com>
Signed-off-by: Wei Zhao <wei.zhao1@intel.com>
---
 lib/librte_ether/rte_ethdev.c          | 17 +++++++++++++++++
 lib/librte_ether/rte_ethdev.h          | 28 ++++++++++++++++++++++++++++
 lib/librte_ether/rte_ether_version.map |  6 ++++++
 3 files changed, 51 insertions(+)

diff --git a/lib/librte_ether/rte_ethdev.c b/lib/librte_ether/rte_ethdev.c
index eb0a94a..2e06dca 100644
--- a/lib/librte_ether/rte_ethdev.c
+++ b/lib/librte_ether/rte_ethdev.c
@@ -3273,3 +3273,20 @@ rte_eth_dev_l2_tunnel_offload_set(uint8_t port_id,
 				-ENOTSUP);
 	return (*dev->dev_ops->l2_tunnel_offload_set)(dev, l2_tunnel, mask, en);
 }
+
+int
+rte_eth_dev_reset(uint8_t port_id)
+{
+	struct rte_eth_dev *dev;
+	int diag;
+
+	RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -ENODEV);
+
+	dev = &rte_eth_devices[port_id];
+
+	RTE_FUNC_PTR_OR_ERR_RET(*dev->dev_ops->dev_reset, -ENOTSUP);
+
+	diag = (*dev->dev_ops->dev_reset)(dev);
+
+	return diag;
+}
diff --git a/lib/librte_ether/rte_ethdev.h b/lib/librte_ether/rte_ethdev.h
index 4be217c..8287c50 100644
--- a/lib/librte_ether/rte_ethdev.h
+++ b/lib/librte_ether/rte_ethdev.h
@@ -1367,6 +1367,9 @@ typedef int (*eth_l2_tunnel_offload_set_t)
 	 uint8_t en);
 /**< @internal enable/disable the l2 tunnel offload functions */
 
+typedef int  (*eth_dev_reset_t)(struct rte_eth_dev *dev);
+/**< @internal Function used to reset a configured Ethernet device. */
+
 #ifdef RTE_NIC_BYPASS
 
 enum {
@@ -1509,6 +1512,9 @@ struct eth_dev_ops {
 	eth_l2_tunnel_offload_set_t   l2_tunnel_offload_set;
 	/** Enable/disable l2 tunnel offload functions. */
 
+	/** Reset device. */
+	eth_dev_reset_t dev_reset;
+
 	eth_set_queue_rate_limit_t set_queue_rate_limit; /**< Set queue rate limit. */
 
 	rss_hash_update_t          rss_hash_update; /** Configure RSS hash protocols. */
@@ -4413,6 +4419,28 @@ int
 rte_eth_dev_get_name_by_port(uint8_t port_id, char *name);
 
 /**
+ * Reset an ethernet device when it's not working. One scenario is, after PF
+ * port is down and up, the related VF port should be reset.
+ * The API will stop the port, clear the rx/tx queues, re-setup the rx/tx
+ * queues, restart the port.
+ * Before calling this API, APP should stop the rx/tx. When tx is being stopped,
+ * APP can drop the packets and release the buffer instead of sending them.
+ * This function can also do some restore work for the port, for example, it can
+ * restore the added parameters of vlan,  mac_addrs, promisc_unicast_enabled
+ * flag and promisc_multicast_enabled flag.
+ *
+ * @param port_id
+ *   The port identifier of the Ethernet device.
+ *
+ * @return
+ *   - (0) if successful.
+ *   - (-ENODEV) if port identifier is invalid.
+ *   - (-ENOTSUP) if hardware doesn't support this function.
+ */
+int
+rte_eth_dev_reset(uint8_t port_id);
+
+/**
  * @internal
  * Wrapper for use by pci drivers as a .probe function to attach to a ethdev
  * interface.
diff --git a/lib/librte_ether/rte_ether_version.map b/lib/librte_ether/rte_ether_version.map
index c6c9d0d..529b27f 100644
--- a/lib/librte_ether/rte_ether_version.map
+++ b/lib/librte_ether/rte_ether_version.map
@@ -154,3 +154,9 @@ DPDK_17.02 {
 	rte_flow_validate;
 
 } DPDK_16.11;
+
+DPDK_17.05 {
+	global:
+
+	rte_eth_dev_reset;
+} DPDK_17.02;
\ No newline at end of file
-- 
2.9.3

Re: [dpdk-dev] [PATCH v7 1/3] lib/librte_ether: add support for port reset

[Thomas Monjalon]

10/04/2017 05:02, Wei Zhao:
> --- a/lib/librte_ether/rte_ethdev.h
> +++ b/lib/librte_ether/rte_ethdev.h
> @@ -1509,6 +1512,9 @@ struct eth_dev_ops {
>  	eth_l2_tunnel_offload_set_t   l2_tunnel_offload_set;
>  	/** Enable/disable l2 tunnel offload functions. */
> 
> +	/** Reset device. */
> +	eth_dev_reset_t dev_reset;
> +
>  	eth_set_queue_rate_limit_t set_queue_rate_limit; /**< Set queue rate
> limit. */
> 
>  	rss_hash_update_t          rss_hash_update; /** Configure RSS hash

This new op should be added at the end of the structure
to avoid ABI issue.

> protocols. */ @@ -4413,6 +4419,28 @@ int
>  rte_eth_dev_get_name_by_port(uint8_t port_id, char *name);
> 
>  /**
> + * Reset an ethernet device when it's not working. One scenario is, after
> PF + * port is down and up, the related VF port should be reset.
> + * The API will stop the port, clear the rx/tx queues, re-setup the rx/tx
> + * queues, restart the port.
> + * Before calling this API, APP should stop the rx/tx. When tx is being
> stopped, + * APP can drop the packets and release the buffer instead of
> sending them. + * This function can also do some restore work for the port,
> for example, it can + * restore the added parameters of vlan,  mac_addrs,
> promisc_unicast_enabled + * flag and promisc_multicast_enabled flag.
> + *
> + * @param port_id
> + *   The port identifier of the Ethernet device.
> + *
> + * @return
> + *   - (0) if successful.
> + *   - (-ENODEV) if port identifier is invalid.
> + *   - (-ENOTSUP) if hardware doesn't support this function.
> + */
> +int
> +rte_eth_dev_reset(uint8_t port_id);

The declarations and function definitions should be better placed
after start and stop functions.

Re: [dpdk-dev] [PATCH v7 1/3] lib/librte_ether: add support for port reset

[Zhao1, Wei]

Hi,  Thomas

> -----Original Message-----
> From: Thomas Monjalon [mailto:thomas@monjalon.net]
> Sent: Friday, April 21, 2017 4:50 AM
> To: Zhao1, Wei <wei.zhao1@intel.com>; Lu, Wenzhuo
> <wenzhuo.lu@intel.com>
> Cc: dev@dpdk.org
> Subject: Re: [dpdk-dev] [PATCH v7 1/3] lib/librte_ether: add support for port
> reset
> 
> 10/04/2017 05:02, Wei Zhao:
> > --- a/lib/librte_ether/rte_ethdev.h
> > +++ b/lib/librte_ether/rte_ethdev.h
> > @@ -1509,6 +1512,9 @@ struct eth_dev_ops {
> >  	eth_l2_tunnel_offload_set_t   l2_tunnel_offload_set;
> >  	/** Enable/disable l2 tunnel offload functions. */
> >
> > +	/** Reset device. */
> > +	eth_dev_reset_t dev_reset;
> > +
> >  	eth_set_queue_rate_limit_t set_queue_rate_limit; /**< Set queue
> rate
> > limit. */
> >
> >  	rss_hash_update_t          rss_hash_update; /** Configure RSS hash
> 
> This new op should be added at the end of the structure to avoid ABI issue.

Ok, I will change as your suggestion in next version.

> 
> > protocols. */ @@ -4413,6 +4419,28 @@ int
> > rte_eth_dev_get_name_by_port(uint8_t port_id, char *name);
> >
> >  /**
> > + * Reset an ethernet device when it's not working. One scenario is,
> > + after
> > PF + * port is down and up, the related VF port should be reset.
> > + * The API will stop the port, clear the rx/tx queues, re-setup the
> > + rx/tx
> > + * queues, restart the port.
> > + * Before calling this API, APP should stop the rx/tx. When tx is
> > + being
> > stopped, + * APP can drop the packets and release the buffer instead
> > of sending them. + * This function can also do some restore work for
> > the port, for example, it can + * restore the added parameters of
> > vlan,  mac_addrs, promisc_unicast_enabled + * flag and
> promisc_multicast_enabled flag.
> > + *
> > + * @param port_id
> > + *   The port identifier of the Ethernet device.
> > + *
> > + * @return
> > + *   - (0) if successful.
> > + *   - (-ENODEV) if port identifier is invalid.
> > + *   - (-ENOTSUP) if hardware doesn't support this function.
> > + */
> > +int
> > +rte_eth_dev_reset(uint8_t port_id);
> 
> The declarations and function definitions should be better placed after start
> and stop functions.

Ok, I will change as your suggestion in next version.

Re: [dpdk-dev] [PATCH v7 1/3] lib/librte_ether: add support for port reset

[Thomas Monjalon]

10/04/2017 05:02, Wei Zhao:
> Add support for port reset in rte layer.This reset
> feature can be used not only in vf port reset in the following
> code develop process later, but also pf port.But in this patch
> set, we only limit the discussion scope to vf reset.
> This patch add an API to restart the device.
> It's for VF device in this scenario, kernel PF + DPDK VF.
> When the PF port state is down->up, APP should call this API to
> restart VF port. Most likely, APP should call it in its
> management thread and guarantee the thread safe. It means
> APP should stop the rx/tx and the device, then restart
> the device, then recover the device and rx/tx.This API can
> also do some restore work for the port.
> 
> Signed-off-by: Wenzhuo Lu <wenzhuo.lu@intel.com>
> Signed-off-by: Wei Zhao <wei.zhao1@intel.com>
> ---
>  lib/librte_ether/rte_ethdev.c          | 17 +++++++++++++++++
>  lib/librte_ether/rte_ethdev.h          | 28 ++++++++++++++++++++++++++++
>  lib/librte_ether/rte_ether_version.map |  6 ++++++
>  3 files changed, 51 insertions(+)

This new feature should be referenced in the matrix
doc/guides/nics/features/default.ini

[dpdk-dev] [PATCH v7 2/3] net/i40e: implement device reset on port

[Wei Zhao]

This patch implement the device reset function on vf port.
This restart function will detach device then
attach device, reconfigure dev, re-setup
the Rx/Tx queues.

Signed-off-by: Wei Zhao <wei.zhao1@intel.com>
Signed-off-by: Wenzhuo Lu <wenzhuo.lu@intel.com>
---
 drivers/net/i40e/i40e_ethdev.c    |   2 +-
 drivers/net/i40e/i40e_ethdev.h    |  16 ++++
 drivers/net/i40e/i40e_ethdev_vf.c | 185 ++++++++++++++++++++++++++++++++++++++
 drivers/net/i40e/i40e_rxtx.c      |  10 +++
 drivers/net/i40e/i40e_rxtx.h      |   4 +
 5 files changed, 216 insertions(+), 1 deletion(-)

diff --git a/drivers/net/i40e/i40e_ethdev.c b/drivers/net/i40e/i40e_ethdev.c
index 3702214..f2fdb2f 100644
--- a/drivers/net/i40e/i40e_ethdev.c
+++ b/drivers/net/i40e/i40e_ethdev.c
@@ -6020,7 +6020,7 @@ i40e_find_vlan_filter(struct i40e_vsi *vsi,
 		return 0;
 }
 
-static void
+void
 i40e_store_vlan_filter(struct i40e_vsi *vsi,
 		       uint16_t vlan_id, bool on)
 {
diff --git a/drivers/net/i40e/i40e_ethdev.h b/drivers/net/i40e/i40e_ethdev.h
index aebb097..515a288 100644
--- a/drivers/net/i40e/i40e_ethdev.h
+++ b/drivers/net/i40e/i40e_ethdev.h
@@ -652,6 +652,16 @@ struct i40e_vf_tx_queues {
 	uint32_t tx_ring_len;
 };
 
+struct i40e_vf_reset_store {
+	struct ether_addr *mac_addrs;  /* Device Ethernet Link address. */
+	bool promisc_unicast_enabled;
+	bool promisc_multicast_enabled;
+	uint16_t vlan_num;    /* Total VLAN number */
+	uint32_t vfta[I40E_VFTA_SIZE];        /* VLAN bitmap */
+	uint16_t mac_num;        /* Total mac number */
+};
+
+
 /*
  * Structure to store private data specific for VF instance.
  */
@@ -709,6 +719,10 @@ struct i40e_adapter {
 	struct rte_timecounter systime_tc;
 	struct rte_timecounter rx_tstamp_tc;
 	struct rte_timecounter tx_tstamp_tc;
+
+	/* For port reset */
+	volatile uint8_t reset_flag;
+	void *reset_store_data;
 };
 
 extern const struct rte_flow_ops i40e_flow_ops;
@@ -749,6 +763,8 @@ int i40e_dev_link_update(struct rte_eth_dev *dev,
 			 __rte_unused int wait_to_complete);
 void i40e_vsi_queues_bind_intr(struct i40e_vsi *vsi);
 void i40e_vsi_queues_unbind_intr(struct i40e_vsi *vsi);
+void i40e_store_vlan_filter(struct i40e_vsi *vsi,
+		       uint16_t vlan_id, bool on);
 int i40e_vsi_vlan_pvid_set(struct i40e_vsi *vsi,
 			   struct i40e_vsi_vlan_pvid_info *info);
 int i40e_vsi_config_vlan_stripping(struct i40e_vsi *vsi, bool on);
diff --git a/drivers/net/i40e/i40e_ethdev_vf.c b/drivers/net/i40e/i40e_ethdev_vf.c
index 55fd344..9ab035c 100644
--- a/drivers/net/i40e/i40e_ethdev_vf.c
+++ b/drivers/net/i40e/i40e_ethdev_vf.c
@@ -161,6 +161,9 @@ i40evf_dev_rx_queue_intr_disable(struct rte_eth_dev *dev, uint16_t queue_id);
 static void i40evf_handle_pf_event(__rte_unused struct rte_eth_dev *dev,
 				   uint8_t *msg,
 				   uint16_t msglen);
+static int i40evf_dev_uninit(struct rte_eth_dev *eth_dev);
+static int i40evf_dev_init(struct rte_eth_dev *eth_dev);
+static int i40evf_reset_dev(struct rte_eth_dev *dev);
 
 /* Default hash key buffer for RSS */
 static uint32_t rss_key_default[I40E_VFQF_HKEY_MAX_INDEX + 1];
@@ -230,6 +233,7 @@ static const struct eth_dev_ops i40evf_eth_dev_ops = {
 	.rss_hash_conf_get    = i40evf_dev_rss_hash_conf_get,
 	.mtu_set              = i40evf_dev_mtu_set,
 	.mac_addr_set         = i40evf_set_default_mac_addr,
+	.dev_reset            = i40evf_reset_dev,
 };
 
 /*
@@ -889,6 +893,7 @@ i40evf_add_mac_addr(struct rte_eth_dev *dev,
 		PMD_DRV_LOG(ERR, "fail to execute command "
 			    "OP_ADD_ETHER_ADDRESS");
 
+	vf->vsi.mac_num++;
 	return;
 }
 
@@ -926,6 +931,7 @@ i40evf_del_mac_addr_by_addr(struct rte_eth_dev *dev,
 	if (err)
 		PMD_DRV_LOG(ERR, "fail to execute command "
 			    "OP_DEL_ETHER_ADDRESS");
+	vf->vsi.mac_num--;
 	return;
 }
 
@@ -1047,6 +1053,7 @@ static int
 i40evf_add_vlan(struct rte_eth_dev *dev, uint16_t vlanid)
 {
 	struct i40e_vf *vf = I40EVF_DEV_PRIVATE_TO_VF(dev->data->dev_private);
+	struct i40e_vsi *vsi = &vf->vsi;
 	struct i40e_virtchnl_vlan_filter_list *vlan_list;
 	uint8_t cmd_buffer[sizeof(struct i40e_virtchnl_vlan_filter_list) +
 							sizeof(uint16_t)];
@@ -1066,6 +1073,8 @@ i40evf_add_vlan(struct rte_eth_dev *dev, uint16_t vlanid)
 	err = i40evf_execute_vf_cmd(dev, &args);
 	if (err)
 		PMD_DRV_LOG(ERR, "fail to execute command OP_ADD_VLAN");
+	i40e_store_vlan_filter(vsi, vlanid, 1);
+	vsi->vlan_num++;
 
 	return err;
 }
@@ -1074,6 +1083,7 @@ static int
 i40evf_del_vlan(struct rte_eth_dev *dev, uint16_t vlanid)
 {
 	struct i40e_vf *vf = I40EVF_DEV_PRIVATE_TO_VF(dev->data->dev_private);
+	struct i40e_vsi *vsi = &vf->vsi;
 	struct i40e_virtchnl_vlan_filter_list *vlan_list;
 	uint8_t cmd_buffer[sizeof(struct i40e_virtchnl_vlan_filter_list) +
 							sizeof(uint16_t)];
@@ -1093,6 +1103,8 @@ i40evf_del_vlan(struct rte_eth_dev *dev, uint16_t vlanid)
 	err = i40evf_execute_vf_cmd(dev, &args);
 	if (err)
 		PMD_DRV_LOG(ERR, "fail to execute command OP_DEL_VLAN");
+	i40e_store_vlan_filter(vsi, vlanid, 0);
+	vsi->vlan_num--;
 
 	return err;
 }
@@ -2716,3 +2728,176 @@ i40evf_set_default_mac_addr(struct rte_eth_dev *dev,
 
 	i40evf_add_mac_addr(dev, mac_addr, 0, 0);
 }
+
+static void
+i40evf_restore_vlan_filter(struct rte_eth_dev *dev,
+			uint32_t *vfta)
+{
+	uint32_t vid_idx, vid_bit;
+	uint16_t vlan_id;
+
+	for  (vid_idx = 0; vid_idx < I40E_VFTA_SIZE; vid_idx++) {
+		for  (vid_bit = 0; vid_bit < I40E_UINT32_BIT_SIZE; vid_bit++) {
+			if (vfta[vid_idx] & (1 << vid_bit)) {
+				vlan_id = (vid_idx << 5) + vid_bit;
+				i40evf_add_vlan(dev, vlan_id);
+			}
+		}
+	}
+}
+
+static void
+i40evf_restore_macaddr(struct rte_eth_dev *dev,
+		struct ether_addr *mac_addrs)
+{
+	struct ether_addr *addr;
+	uint16_t i;
+
+	/* replay MAC address configuration including default MAC */
+	addr = &mac_addrs[0];
+
+	i40evf_set_default_mac_addr(dev, addr);
+	memcpy(dev->data->mac_addrs, mac_addrs,
+			ETHER_ADDR_LEN * I40E_NUM_MACADDR_MAX);
+
+	for (i = 1; i < I40E_NUM_MACADDR_MAX; i++) {
+		addr = &mac_addrs[i];
+
+		/* skip zero address */
+		if (is_zero_ether_addr(addr))
+			continue;
+
+		i40evf_add_mac_addr(dev, addr, 0, 0);
+	}
+}
+
+
+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_store_before_reset(struct rte_eth_dev *dev)
+{
+	struct i40e_adapter *adapter =
+		I40E_DEV_PRIVATE_TO_ADAPTER(dev->data->dev_private);
+	struct i40e_vf_reset_store *store_data;
+	struct i40e_vf *vf = I40EVF_DEV_PRIVATE_TO_VF(dev->data->dev_private);
+
+	adapter->reset_store_data = rte_zmalloc("i40evf_store_reset",
+					sizeof(struct i40e_vf_reset_store), 0);
+	if (adapter->reset_store_data == NULL) {
+		PMD_INIT_LOG(ERR, "Failed to allocate %ld bytes needed to"
+				" to store data when reset vf",
+				sizeof(struct i40e_vf_reset_store));
+		return -ENOMEM;
+	}
+	store_data =
+		(struct i40e_vf_reset_store *)adapter->reset_store_data;
+	store_data->mac_addrs = rte_zmalloc("i40evf_mac_store_reset",
+			ETHER_ADDR_LEN * I40E_NUM_MACADDR_MAX, 0);
+	if (store_data->mac_addrs == NULL) {
+		PMD_INIT_LOG(ERR, "Failed to allocate %d bytes needed to"
+				" to store MAC addresses when reset vf",
+				ETHER_ADDR_LEN * I40E_NUM_MACADDR_MAX);
+	}
+
+	memcpy(store_data->mac_addrs, dev->data->mac_addrs,
+			ETHER_ADDR_LEN * I40E_NUM_MACADDR_MAX);
+
+	store_data->promisc_unicast_enabled = vf->promisc_unicast_enabled;
+	store_data->promisc_multicast_enabled = vf->promisc_multicast_enabled;
+
+	store_data->vlan_num = vf->vsi.vlan_num;
+	memcpy(store_data->vfta, vf->vsi.vfta,
+			sizeof(uint32_t) * I40E_VFTA_SIZE);
+
+	store_data->mac_num = vf->vsi.mac_num;
+
+	return 0;
+}
+
+static void
+i40evf_restore_after_reset(struct rte_eth_dev *dev)
+{
+	struct i40e_adapter *adapter =
+		I40E_DEV_PRIVATE_TO_ADAPTER(dev->data->dev_private);
+	struct i40e_vf_reset_store *store_data =
+		(struct i40e_vf_reset_store *)adapter->reset_store_data;
+
+	if (store_data->promisc_unicast_enabled)
+		i40evf_dev_promiscuous_enable(dev);
+	if (store_data->promisc_multicast_enabled)
+		i40evf_dev_allmulticast_enable(dev);
+
+	if (store_data->vlan_num)
+		i40evf_restore_vlan_filter(dev, store_data->vfta);
+
+	if (store_data->mac_num)
+		i40evf_restore_macaddr(dev, store_data->mac_addrs);
+
+	rte_free(store_data->mac_addrs);
+	rte_free(adapter->reset_store_data);
+}
+
+static int
+i40evf_reset_dev(struct rte_eth_dev *dev)
+{
+	struct i40e_adapter *adapter =
+		I40E_DEV_PRIVATE_TO_ADAPTER(dev->data->dev_private);
+
+	adapter->reset_flag = 1;
+	i40evf_store_before_reset(dev);
+
+	i40evf_dev_close(dev);
+	PMD_DRV_LOG(DEBUG, "i40evf dev close complete");
+
+	i40evf_dev_uninit(dev);
+	PMD_DRV_LOG(DEBUG, "i40evf dev detached");
+
+	memset(dev->data->dev_private, 0,
+	       (uint64_t)&adapter->reset_flag - (uint64_t)adapter);
+
+	i40evf_dev_init(dev);
+	PMD_DRV_LOG(DEBUG, "i40evf dev attached");
+
+	i40evf_dev_configure(dev);
+
+	i40e_vf_queue_reset(dev);
+	PMD_DRV_LOG(DEBUG, "i40evf queue reset");
+
+	i40evf_restore_after_reset(dev);
+
+	i40evf_dev_start(dev);
+	PMD_DRV_LOG(DEBUG, "i40evf dev restart");
+
+	adapter->reset_flag = 0;
+
+	return 0;
+}
diff --git a/drivers/net/i40e/i40e_rxtx.c b/drivers/net/i40e/i40e_rxtx.c
index 02367b7..d891a54 100644
--- a/drivers/net/i40e/i40e_rxtx.c
+++ b/drivers/net/i40e/i40e_rxtx.c
@@ -1709,6 +1709,7 @@ i40e_dev_rx_queue_setup(struct rte_eth_dev *dev,
 	uint16_t len, i;
 	uint16_t base, bsf, tc_mapping;
 	int use_def_burst_func = 1;
+	struct rte_eth_rxconf conf = *rx_conf;
 
 	if (hw->mac.type == I40E_MAC_VF || hw->mac.type == I40E_MAC_X722_VF) {
 		struct i40e_vf *vf =
@@ -1748,6 +1749,8 @@ i40e_dev_rx_queue_setup(struct rte_eth_dev *dev,
 	}
 	rxq->mp = mp;
 	rxq->nb_rx_desc = nb_desc;
+	rxq->socket_id = socket_id;
+	rxq->rxconf = conf;
 	rxq->rx_free_thresh = rx_conf->rx_free_thresh;
 	rxq->queue_id = queue_idx;
 	if (hw->mac.type == I40E_MAC_VF || hw->mac.type == I40E_MAC_X722_VF)
@@ -1932,6 +1935,7 @@ i40e_dev_tx_queue_setup(struct rte_eth_dev *dev,
 	uint32_t ring_size;
 	uint16_t tx_rs_thresh, tx_free_thresh;
 	uint16_t i, base, bsf, tc_mapping;
+	struct rte_eth_txconf conf = *tx_conf;
 
 	if (hw->mac.type == I40E_MAC_VF || hw->mac.type == I40E_MAC_X722_VF) {
 		struct i40e_vf *vf =
@@ -2054,6 +2058,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;
@@ -2520,8 +2526,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_flag)
+		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 a87bdb0..0e3cc19 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.9.3

Re: [dpdk-dev] [PATCH v7 2/3] net/i40e: implement device reset on port

[Thomas Monjalon]

10/04/2017 05:02, Wei Zhao:
> +	if (adapter->reset_store_data == NULL) {
> +		PMD_INIT_LOG(ERR, "Failed to allocate %ld bytes needed to"
> +				" to store data when reset vf",
> +				sizeof(struct i40e_vf_reset_store));

Compilation fails for 32-bit.
%ld must be replaced by %zu

Re: [dpdk-dev] [PATCH v7 2/3] net/i40e: implement device reset on port

[Zhao1, Wei]

Hi,  Thomas

> -----Original Message-----
> From: Thomas Monjalon [mailto:thomas@monjalon.net]
> Sent: Friday, April 21, 2017 5:12 AM
> To: Zhao1, Wei <wei.zhao1@intel.com>; Lu, Wenzhuo
> <wenzhuo.lu@intel.com>
> Cc: dev@dpdk.org
> Subject: Re: [dpdk-dev] [PATCH v7 2/3] net/i40e: implement device reset on
> port
> 
> 10/04/2017 05:02, Wei Zhao:
> > +	if (adapter->reset_store_data == NULL) {
> > +		PMD_INIT_LOG(ERR, "Failed to allocate %ld bytes needed
> to"
> > +				" to store data when reset vf",
> > +				sizeof(struct i40e_vf_reset_store));
> 
> Compilation fails for 32-bit.
> %ld must be replaced by %zu

Ok, I will change as your suggestion in next version.

Re: [dpdk-dev] [PATCH v7 2/3] net/i40e: implement device reset on port

[Thomas Monjalon]

10/04/2017 05:02, Wei Zhao:
> +	memset(dev->data->dev_private, 0,
> +	       (uint64_t)&adapter->reset_flag - (uint64_t)adapter);

It does not compile for 32-bit.
Should it be replaced by offsetof()?

Does it mean that new fields should be added before reset_flag?
There is no comment about position importance of this field in the struct.
By the way, there is a field ptype_tbl appeared recently. Where should it
be positionned after rebase?

[dpdk-dev] [PATCH v7 3/3] app/testpmd: add port reset command into testpmd

[Wei Zhao]

Add port reset command into testpmd project,
it is the interface for user to reset port.
And also it is not limit to be used only in vf reset,
but also for pf port reset.But this patch set only
realted to vf reset feature.

Signed-off-by: Wei Zhao <wei.zhao1@intel.com>
Signed-off-by: Wenzhuo Lu <wenzhuo.lu@intel.com>
---
 app/test-pmd/cmdline.c | 17 +++++++++----
 app/test-pmd/testpmd.c | 65 ++++++++++++++++++++++++++++++++++++++++++++++++++
 app/test-pmd/testpmd.h |  1 +
 3 files changed, 79 insertions(+), 4 deletions(-)

diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c
index 47f935d..6cbdcc8 100644
--- a/app/test-pmd/cmdline.c
+++ b/app/test-pmd/cmdline.c
@@ -599,6 +599,9 @@ static void cmd_help_long_parsed(void *parsed_result,
 			"port close (port_id|all)\n"
 			"    Close all ports or port_id.\n\n"
 
+			"port reset (port_id|all)\n"
+			"    Reset all ports or port_id.\n\n"
+
 			"port attach (ident)\n"
 			"    Attach physical or virtual dev by pci address or virtual device name\n\n"
 
@@ -909,6 +912,8 @@ static void cmd_operate_port_parsed(void *parsed_result,
 		stop_port(RTE_PORT_ALL);
 	else if (!strcmp(res->name, "close"))
 		close_port(RTE_PORT_ALL);
+	else if (!strcmp(res->name, "reset"))
+		reset_port(RTE_PORT_ALL);
 	else
 		printf("Unknown parameter\n");
 }
@@ -918,14 +923,15 @@ cmdline_parse_token_string_t cmd_operate_port_all_cmd =
 								"port");
 cmdline_parse_token_string_t cmd_operate_port_all_port =
 	TOKEN_STRING_INITIALIZER(struct cmd_operate_port_result, name,
-						"start#stop#close");
+						"start#stop#close#reset");
 cmdline_parse_token_string_t cmd_operate_port_all_all =
 	TOKEN_STRING_INITIALIZER(struct cmd_operate_port_result, value, "all");
 
 cmdline_parse_inst_t cmd_operate_port = {
 	.f = cmd_operate_port_parsed,
 	.data = NULL,
-	.help_str = "port start|stop|close all: Start/Stop/Close all ports",
+	.help_str = "port start|stop|close|reset all: Start/Stop/Close/"
+			"Reset all ports",
 	.tokens = {
 		(void *)&cmd_operate_port_all_cmd,
 		(void *)&cmd_operate_port_all_port,
@@ -953,6 +959,8 @@ static void cmd_operate_specific_port_parsed(void *parsed_result,
 		stop_port(res->value);
 	else if (!strcmp(res->name, "close"))
 		close_port(res->value);
+	else if (!strcmp(res->name, "reset"))
+		reset_port(res->value);
 	else
 		printf("Unknown parameter\n");
 }
@@ -962,7 +970,7 @@ cmdline_parse_token_string_t cmd_operate_specific_port_cmd =
 							keyword, "port");
 cmdline_parse_token_string_t cmd_operate_specific_port_port =
 	TOKEN_STRING_INITIALIZER(struct cmd_operate_specific_port_result,
-						name, "start#stop#close");
+						name, "start#stop#close#reset");
 cmdline_parse_token_num_t cmd_operate_specific_port_id =
 	TOKEN_NUM_INITIALIZER(struct cmd_operate_specific_port_result,
 							value, UINT8);
@@ -970,7 +978,8 @@ cmdline_parse_token_num_t cmd_operate_specific_port_id =
 cmdline_parse_inst_t cmd_operate_specific_port = {
 	.f = cmd_operate_specific_port_parsed,
 	.data = NULL,
-	.help_str = "port start|stop|close <port_id>: Start/Stop/Close port_id",
+	.help_str = "port start|stop|close|reset <port_id>: Start/Stop/Close/"
+			"Reset port_id",
 	.tokens = {
 		(void *)&cmd_operate_specific_port_cmd,
 		(void *)&cmd_operate_specific_port_port,
diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c
index 484c19b..a4f5330 100644
--- a/app/test-pmd/testpmd.c
+++ b/app/test-pmd/testpmd.c
@@ -137,6 +137,7 @@ portid_t  nb_fwd_ports;  /**< Number of forwarding ports. */
 
 unsigned int fwd_lcores_cpuids[RTE_MAX_LCORE]; /**< CPU ids configuration. */
 portid_t fwd_ports_ids[RTE_MAX_ETHPORTS];      /**< Port ids configuration. */
+volatile char reset_ports[RTE_MAX_ETHPORTS] = {0};  /**< Port reset flag. */
 
 struct fwd_stream **fwd_streams; /**< For each RX queue of each port. */
 streamid_t nb_fwd_streams;       /**< Is equal to (nb_ports * nb_rxq). */
@@ -1305,6 +1306,18 @@ port_is_closed(portid_t port_id)
 	return 1;
 }
 
+static void
+reset_event_callback(uint8_t port_id, enum rte_eth_event_type type, void *param)
+{
+	RTE_SET_USED(param);
+
+	printf("Event type: %s on port %d\n",
+		type == RTE_ETH_EVENT_INTR_RESET ? "RESET interrupt" :
+		"unknown event", port_id);
+	reset_ports[port_id] = 1;
+	rte_wmb();
+}
+
 int
 start_port(portid_t pid)
 {
@@ -1350,6 +1363,10 @@ start_port(portid_t pid)
 				return -1;
 			}
 		}
+
+		/* register reset interrupt callback */
+		rte_eth_dev_callback_register(pi, RTE_ETH_EVENT_INTR_RESET,
+			reset_event_callback, NULL);
 		if (port->need_reconfig_queues > 0) {
 			port->need_reconfig_queues = 0;
 			/* setup tx queues */
@@ -1559,6 +1576,54 @@ close_port(portid_t pid)
 }
 
 void
+reset_port(portid_t pid)
+{
+	portid_t pi;
+	struct rte_port *port;
+
+	if (port_id_is_invalid(pid, ENABLED_WARN))
+		return;
+
+	printf("Reset ports...\n");
+
+	FOREACH_PORT(pi, ports) {
+		if (pid != pi && pid != (portid_t)RTE_PORT_ALL)
+			continue;
+
+		if (port_is_forwarding(pi) != 0 && test_done == 0) {
+			printf("Please remove port %d from forwarding "
+					"configuration.\n", pi);
+			continue;
+		}
+
+		if (port_is_bonding_slave(pi)) {
+			printf("Please remove port %d from "
+					"bonded device.\n", pi);
+			continue;
+		}
+
+		if (!reset_ports[pi]) {
+			printf("vf must get reset port %d info from "
+					"pf before reset.\n", pi);
+			continue;
+		}
+
+		port = &ports[pi];
+		if (rte_atomic16_cmpset(&(port->port_status),
+			RTE_PORT_STOPPED, RTE_PORT_HANDLING) == 1) {
+			printf("Port %d is not stopped\n", pi);
+			continue;
+		}
+
+		rte_eth_dev_reset(pi);
+		reset_ports[pi] = 0;
+		rte_wmb();
+	}
+
+	printf("Done\n");
+}
+
+void
 attach_port(char *identifier)
 {
 	portid_t pi = 0;
diff --git a/app/test-pmd/testpmd.h b/app/test-pmd/testpmd.h
index 8cf2860..0c7e44c 100644
--- a/app/test-pmd/testpmd.h
+++ b/app/test-pmd/testpmd.h
@@ -586,6 +586,7 @@ int init_port_dcb_config(portid_t pid, enum dcb_mode_enable dcb_mode,
 int start_port(portid_t pid);
 void stop_port(portid_t pid);
 void close_port(portid_t pid);
+void reset_port(portid_t pid);
 void attach_port(char *identifier);
 void detach_port(uint8_t port_id);
 int all_ports_stopped(void);
-- 
2.9.3

Re: [dpdk-dev] [PATCH v7 0/3] net/i40e: vf port reset

[Thomas Monjalon]

10/04/2017 05:02, Wei Zhao:
> The patches mainly finish following functions:
> 1) get pf reset vf comamand falg from interrupt server function.
> 2) reset vf port from testpmd app using a command.
> 3) sore and restore vf related parameters.
[...]
> zhao wei (3):
>   app/testpmd: add port reset command into testpmd
>   lib/librte_ether: add support for port reset
>   net/i40e: implement device reset on port

There are some misses and compilation issues in this series.

As there is neither a support from other maintainers no strong nack,
I was ready to apply this series.
However after a better look and compilation issues revealed in RC2 phase,
I think it should be postponed to 17.08.
In any case, PMD maintainers are welcome to give their opinions.