On 06.06.2017 11:30, Darrell Ball wrote:
> On port deletion, device resources cannot be cleaned
> up if the device was attached when vswitchd was started.
> The issue is mainly due to introduction of the 'attached'
> field for the netdev_dpdk device context. The logic
> setting the 'attached' field did not include startup
> handling. The use of an extra 'attached' field was
> introduced to guard against detaching a device that could
> not be detached, but should not be necessary as the rte and
> eal keep the attached state, as they must, and also filter
> trying to detach a device that is not detachable. If
> there were any bugs in this regard, which I did not find
> with basic testing, then they should be fixed in rte/eal.
Why you want to free resources of these devices?
It's, actually, was a default behaviour of OVS for years.
If you want to detach hotplugable devices than you may just not
attach them using cmdline. Not hotplugable devices will not
be detached anyway.
>From the other side this patch will allow following scenario
for not detachable devices:
- add_port(05.00.0)
- configure()
- stop()
- close()
- detach(05.00.0) -----> Failure
- add_port(05.00.0)
- configure()
- start()
At this point OvS will get closed device by name and will
try to configure it and start again.
My concern is that we can't be sure that it will continue
work properly after close.
According to DPDK API, *device can not be started back after
close*. Unfortunately, there is no documentation about
trying to reinitialize closed device. So, we can't
rely on your assumption that device will work properly
after that.
> Fixes: 5dcde09c80a8 ("netdev-dpdk: Fix device leak on port deletion.")
> CC: Ilya Maximets <[email protected]>
> Signed-off-by: Darrell Ball <[email protected]>
> ---
> lib/netdev-dpdk.c | 14 +++-----------
> 1 file changed, 3 insertions(+), 11 deletions(-)
>
> diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
> index b770b70..753345d 100644
> --- a/lib/netdev-dpdk.c
> +++ b/lib/netdev-dpdk.c
> @@ -365,9 +365,6 @@ struct netdev_dpdk {
> /* Device arguments for dpdk ports */
> char *devargs;
>
> - /* If true, device was attached by rte_eth_dev_attach(). */
> - bool attached;
> -
> /* In dpdk_list. */
> struct ovs_list list_node OVS_GUARDED_BY(dpdk_mutex);
>
> @@ -862,7 +859,6 @@ common_construct(struct netdev *netdev, dpdk_port_t
> port_no,
> dev->max_packet_len = MTU_TO_FRAME_LEN(dev->mtu);
> ovsrcu_index_init(&dev->vid, -1);
> dev->vhost_reconfigured = false;
> - dev->attached = false;
>
> ovsrcu_init(&dev->qos_conf, NULL);
>
> @@ -1016,7 +1012,7 @@ netdev_dpdk_destruct(struct netdev *netdev)
>
> rte_eth_dev_stop(dev->port_id);
>
> - if (dev->attached) {
> + if (rte_eth_dev_is_valid_port(dev->port_id)) {
> rte_eth_dev_close(dev->port_id);
> if (rte_eth_dev_detach(dev->port_id, devname) < 0) {
> VLOG_ERR("Device '%s' can not be detached", dev->devargs);
> @@ -1136,8 +1132,7 @@ netdev_dpdk_lookup_by_port_id(dpdk_port_t port_id)
> }
>
> static dpdk_port_t
> -netdev_dpdk_process_devargs(struct netdev_dpdk *dev,
> - const char *devargs, char **errp)
> +netdev_dpdk_process_devargs(const char *devargs, char **errp)
> {
> /* Get the name up to the first comma. */
> char *name = xmemdup0(devargs, strcspn(devargs, ","));
> @@ -1148,8 +1143,6 @@ netdev_dpdk_process_devargs(struct netdev_dpdk *dev,
> || !rte_eth_dev_is_valid_port(new_port_id)) {
> /* Device not found in DPDK, attempt to attach it */
> if (!rte_eth_dev_attach(devargs, &new_port_id)) {
> - /* Attach successful */
> - dev->attached = true;
> VLOG_INFO("Device '%s' attached to DPDK", devargs);
> } else {
> /* Attach unsuccessful */
> @@ -1236,8 +1229,7 @@ netdev_dpdk_set_config(struct netdev *netdev, const
> struct smap *args,
> * is valid */
> if (!(dev->devargs && !strcmp(dev->devargs, new_devargs)
> && rte_eth_dev_is_valid_port(dev->port_id))) {
> - dpdk_port_t new_port_id = netdev_dpdk_process_devargs(dev,
> -
> new_devargs,
> + dpdk_port_t new_port_id =
> netdev_dpdk_process_devargs(new_devargs,
> errp);
> if (!rte_eth_dev_is_valid_port(new_port_id)) {
> err = EINVAL;
>
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
