Re: [dpdk-dev] [PATCH v9 1/5] ethdev: add support of NIC reset

[Luca Boccassi <luca.boccassi@gmail.com>]

On Sun, 2017-07-23 at 17:15 +0800, Wei Dai wrote:
> This patch adds a new eth_dev layer API function rte_eth_dev_reset(),
> which a DPDK application can call to reset a NIC and keep its port id
> afterwards. It means that all software resources allocated in the
> ethdev
> layer are kept, and software & hardware resources of the NIC within
> the
> NIC's PMD are reset to a state simular to that obtained by calling
> the
> PCI dev_uninit() and then dev_init(). This effective sequence of
> dev_uninit() and dev_init() is packed into a single API function
> rte_eth_dev_reset().
> 
> Please see the comments before the declaration of rte_eht_dev_reset()
> in lib/librte_ether/rte_ethdev.h to get more details on why this
> function is needed, what it does, when it should be called
> and what an application should do after calling this function.
> 
> Signed-off-by: Wei Dai <wei.dai@intel.com>
> Reviewed-by: Remy Horton <remy.horton@intel.com>
> ---
>  lib/librte_ether/rte_ethdev.c          | 17 +++++++++++++++++
>  lib/librte_ether/rte_ethdev.h          | 34
> ++++++++++++++++++++++++++++++++++
>  lib/librte_ether/rte_ether_version.map |  1 +
>  3 files changed, 52 insertions(+)
> 
> diff --git a/lib/librte_ether/rte_ethdev.c
> b/lib/librte_ether/rte_ethdev.c
> index d4ebb1b..68ba64d 100644
> --- a/lib/librte_ether/rte_ethdev.c
> +++ b/lib/librte_ether/rte_ethdev.c
> @@ -1004,6 +1004,23 @@ rte_eth_dev_close(uint8_t port_id)
>  }
>  
>  int
> +rte_eth_dev_reset(uint8_t port_id)
> +{
> +	struct rte_eth_dev *dev;
> +	int ret;
> +
> +	RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -EINVAL);
> +	dev = &rte_eth_devices[port_id];
> +
> +	RTE_FUNC_PTR_OR_ERR_RET(*dev->dev_ops->dev_reset, -ENOTSUP);
> +
> +	rte_eth_dev_stop(port_id);
> +	ret = dev->dev_ops->dev_reset(dev);
> +
> +	return ret;
> +}
> +
> +int
>  rte_eth_rx_queue_setup(uint8_t port_id, uint16_t rx_queue_id,
>  		       uint16_t nb_rx_desc, unsigned int socket_id,
>  		       const struct rte_eth_rxconf *rx_conf,
> diff --git a/lib/librte_ether/rte_ethdev.h
> b/lib/librte_ether/rte_ethdev.h
> index 0e99090..fde92a1 100644
> --- a/lib/librte_ether/rte_ethdev.h
> +++ b/lib/librte_ether/rte_ethdev.h
> @@ -1115,6 +1115,9 @@ typedef int  (*eth_dev_set_link_down_t)(struct
> rte_eth_dev *dev);
>  typedef void (*eth_dev_close_t)(struct rte_eth_dev *dev);
>  /**< @internal Function used to close a configured Ethernet device.
> */
>  
> +typedef int (*eth_dev_reset_t)(struct rte_eth_dev *dev);
> +/** <@internal Function used to reset a configured Ethernet device.
> */
> +
>  typedef void (*eth_promiscuous_enable_t)(struct rte_eth_dev *dev);
>  /**< @internal Function used to enable the RX promiscuous mode of an
> Ethernet device. */
>  
> @@ -1435,6 +1438,7 @@ struct eth_dev_ops {
>  	eth_dev_set_link_up_t      dev_set_link_up;   /**< Device
> link up. */
>  	eth_dev_set_link_down_t    dev_set_link_down; /**< Device
> link down. */
>  	eth_dev_close_t            dev_close;     /**< Close device.
> */
> +	eth_dev_reset_t		   dev_reset;	  /**<
> Reset device. */
>  	eth_link_update_t          link_update;   /**< Get device
> link state. */
>  
>  	eth_promiscuous_enable_t   promiscuous_enable; /**<
> Promiscuous ON. */
> @@ -2140,6 +2144,36 @@ int rte_eth_dev_set_link_down(uint8_t
> port_id);
>  void rte_eth_dev_close(uint8_t port_id);
>  
>  /**
> + * Reset a Ethernet device and keep its port id.
> + *
> + * When a port has to be reset passively, the DPDK application can
> invoke
> + * this function. For example when a PF is reset, all its VFs should
> also
> + * be reset. Normally a DPDK application can invoke this function
> when
> + * RTE_ETH_EVENT_INTR_RESET event is detected, but can also use it
> to start
> + * a port reset in other circumstances.
> + *
> + * When this function is called, it first stops the port and then
> calls the
> + * PMD specific dev_uninit( ) and dev_init( ) to return the port to
> initial
> + * state, in which no Tx and Rx queues are setup, as if the port has
> been
> + * reset and not started. The port keeps the port id it had before
> the
> + * function call.
> + *
> + * After calling rte_eth_dev_reset( ), the application should use
> + * rte_eth_dev_configure( ), rte_eth_rx_queue_setup( ),
> + * rte_eth_tx_queue_setup( ), and rte_eth_dev_start( )
> + * to reconfigure the device as appropriate.
> + *
> + * Note: To avoid unexpected behavior, the application should stop
> calling
> + * Tx and Rx functions before calling rte_eth_dev_reset( ). For
> thread
> + * safety, all these controlling functions should be called from the
> same
> + * thread.
> + *
> + * @param port_id
> + *   The port identifier of the Ethernet device.
> + */
> +int rte_eth_dev_reset(uint8_t port_id);
> +
> +/**
>   * Enable receipt in promiscuous mode for an Ethernet device.
>   *
>   * @param port_id
> diff --git a/lib/librte_ether/rte_ether_version.map
> b/lib/librte_ether/rte_ether_version.map
> index 4283728..e86d51e 100644
> --- a/lib/librte_ether/rte_ether_version.map
> +++ b/lib/librte_ether/rte_ether_version.map
> @@ -155,6 +155,7 @@ DPDK_17.08 {
>  	rte_eth_dev_adjust_nb_rx_tx_desc;
>  	rte_flow_copy;
>  	rte_flow_isolate;
> +	rte_eth_dev_reset;
>  	rte_tm_capabilities_get;
>  	rte_tm_get_leaf_nodes;
>  	rte_tm_hierarchy_commit;

Hello Wei,

First of all thanks for your work, we have a use case where it's
extremely important to be able to reset the VFs when the PF (Linux)
changes state.

Question: can the rte_eth_dev_reset call block in the current
implementation that was merged?

Last year a patchset was sent by, I believe, a colleague of yours,
Wenzhuo Lu, and I've been using it in production.

One of the things that I immediately noticed is that the _reset call
was blocking - that was a show stopper for our application, so I sent a
couple of patches to take a "blocking" boolean, and return EAGAIN
instead of blocking.

You can find the original patches this link:

http://dpdk.org/ml/archives/dev/2016-June/042018.html

The description of the problem we had with the blocking call:

http://dpdk.org/ml/archives/dev/2016-July/043446.html

The "blocking" and EAGAIN changes:

http://dpdk.org/ml/archives/dev/2016-July/043535.html
http://dpdk.org/ml/archives/dev/2016-July/043818.html

Functionality-wise, from the point of view of an application, are you
aware of big differences between that patchset and what was merged
today?

Thanks!

-- 
Kind regards,
Luca Boccassi