[ovs-dev] [patch_v1] dpdk: Fix device cleanup.
Darrell Ball
dball at vmware.com
Fri Jul 28 16:55:17 UTC 2017
-----Original Message-----
From: <ovs-dev-bounces at openvswitch.org> on behalf of Ilya Maximets <i.maximets at samsung.com>
Date: Friday, July 28, 2017 at 7:00 AM
To: Darrell Ball <dlu998 at gmail.com>, "dev at openvswitch.org" <dev at openvswitch.org>
Subject: Re: [ovs-dev] [patch_v1] dpdk: Fix device cleanup.
In general, I have no objections to return 'detach' appctl command.
Comments inline.
On 26.07.2017 08:09, Darrell Ball wrote:
> Commit 5dcde09c80a8 was introduced to make detaching more
> automatic without using an additional command.
>
> Sometimes, since commit 5dcde09c80a8, dpdk devices are
> not detached when del-port is issued; command example:
>
> sudo ovs-vsctl del-port br0 dpdk1
>
> This can happen when vswitchd is (re)started with an existing
> database and devices are already bound to dpdk.
>
> A discussion is here:
> https://urldefense.proofpoint.com/v2/url?u=https-3A__mail.openvswitch.org_pipermail_ovs-2Ddev_2017-2DJune_333462.html&d=DwICAg&c=uilaK90D4TOVoH58JNXRgQ&r=BVhFA09CGX7JQ5Ih-uZnsw&m=LDRU1lBPWYWf9qU7EP9aqwRSUkMhKNITwLCbKWc1muE&s=jAJAy7_3Tjk9afNa5P1nUGPferw28LxsUeAmGdtOCfk&e=
> along with a possible solution.
>
> Since we are nearing the end of a release, a safe approach is needed,
> at this time.
> One approach is to revert 5dcde09c80a8. This patch does not do that
> but reinstates the command ovs-appctl netdev-dpdk/detach to handle
> cases when del-port will not work.
>
> Fixes: 5dcde09c80a8 ("netdev-dpdk: Fix device leak on port deletion.")
> CC: Ilya Maximets <i.maximets at samsung.com>
> Signed-off-by: Darrell Ball <dlu998 at gmail.com>
> ---
> Documentation/howto/dpdk.rst | 12 ++++++++++
> lib/netdev-dpdk.c | 52 +++++++++++++++++++++++++++++++++++++++++++-
> 2 files changed, 63 insertions(+), 1 deletion(-)
>
> diff --git a/Documentation/howto/dpdk.rst b/Documentation/howto/dpdk.rst
> index af01d3e..3c198a2 100644
> --- a/Documentation/howto/dpdk.rst
> +++ b/Documentation/howto/dpdk.rst
> @@ -331,6 +331,18 @@ Detaching will be performed while processing del-port command::
>
> $ ovs-vsctl del-port dpdkx
>
> +Sometimes, the del-port command may not detach the device.
> +Detaching can be confirmed by the appearance of an INFO log.
> +For example::
> +
> + Device '0000:04:00.0' has been detached
> +
> +If the log is not seen, then the port can be detached using::
> +
> +$ ovs-appctl netdev-dpdk/detach 0000:01:00.0
> +
> +Again, detaching can be confirmed by the above INFO log.
> +
> This feature is not supported with VFIO and does not work with some NICs.
I think, we need to clarify here that attempt to detach of not detachable
device will lead to closing that device and possible inability to re-add it
back to OVS. Even if this device will be added successfully after execution
of 'ovs-vsctl add-port' it may not work properly (possible crashes or HW faults).
For example, 'cxgbe' DPDK driver can not be restored after the rte_eth_dev_close().
I traced this driver code before.
I can add a general warning about “not detachable devices” with reference to the cxgbe driver as a possible example.
> For more information please refer to the `DPDK Port Hotplug Framework
> <https://urldefense.proofpoint.com/v2/url?u=http-3A__dpdk.org_doc_guides_prog-5Fguide_port-5Fhotplug-5Fframework.html-23hotplug&d=DwICAg&c=uilaK90D4TOVoH58JNXRgQ&r=BVhFA09CGX7JQ5Ih-uZnsw&m=LDRU1lBPWYWf9qU7EP9aqwRSUkMhKNITwLCbKWc1muE&s=kJOAyBhal3hUOVxV6MghxgIk0dkn0NPwNi4ael5T-cE&e= >`__.
> diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
> index ea17b97..812d262 100644
> --- a/lib/netdev-dpdk.c
> +++ b/lib/netdev-dpdk.c
> @@ -1013,7 +1013,7 @@ netdev_dpdk_destruct(struct netdev *netdev)
> if (rte_eth_dev_detach(dev->port_id, devname) < 0) {
> VLOG_ERR("Device '%s' can not be detached", dev->devargs);
> } else {
> - VLOG_INFO("Device '%s' detached", devname);
> + VLOG_INFO("Device '%s' has been detached", devname);
> }
> }
>
> @@ -2449,6 +2449,53 @@ netdev_dpdk_set_admin_state(struct unixctl_conn *conn, int argc,
> unixctl_command_reply(conn, "OK");
> }
>
> +static void
> +netdev_dpdk_detach(struct unixctl_conn *conn, int argc OVS_UNUSED,
> + const char *argv[], void *aux OVS_UNUSED)
> +{
> + int ret;
> + char *response;
> + uint8_t port_id;
> + char devname[RTE_ETH_NAME_MAX_LEN];
> + struct netdev_dpdk *dev;
> +
> + ovs_mutex_lock(&dpdk_mutex);
> +
> + if (!rte_eth_dev_count() || rte_eth_dev_get_port_by_name(argv[1],
> + &port_id)) {
> + response = xasprintf("Device '%s' not found in DPDK", argv[1]);
> + goto error;
> + }
> +
> + dev = netdev_dpdk_lookup_by_port_id(port_id);
> + if (dev) {
> + response = xasprintf("Device '%s' is being used by interface '%s'. "
> + "Remove it before detaching",
> + argv[1], netdev_get_name(&dev->up));
> + goto error;
> + }
> +
> + rte_eth_dev_close(port_id);
> +
> + ret = rte_eth_dev_detach(port_id, devname);
> + if (ret < 0) {
> + response = xasprintf("Device '%s' can not be detached", argv[1]);
> + goto error;
> + }
> +
> + response = xasprintf("Device '%s' has been detached", argv[1]);
> +
> + ovs_mutex_unlock(&dpdk_mutex);
> + unixctl_command_reply(conn, response);
> + free(response);
> + return;
> +
> +error:
> + ovs_mutex_unlock(&dpdk_mutex);
> + unixctl_command_reply_error(conn, response);
> + free(response);
> +}
> +
> /*
> * Set virtqueue flags so that we do not receive interrupts.
> */
> @@ -2724,6 +2771,9 @@ netdev_dpdk_class_init(void)
> unixctl_command_register("netdev-dpdk/set-admin-state",
> "[netdev] up|down", 1, 2,
> netdev_dpdk_set_admin_state, NULL);
> + unixctl_command_register("netdev-dpdk/detach",
> + "pci address of device", 1, 1,
> + netdev_dpdk_detach, NULL);
>
> ovsthread_once_done(&once);
> }
>
_______________________________________________
dev mailing list
dev at openvswitch.org
https://urldefense.proofpoint.com/v2/url?u=https-3A__mail.openvswitch.org_mailman_listinfo_ovs-2Ddev&d=DwICAg&c=uilaK90D4TOVoH58JNXRgQ&r=BVhFA09CGX7JQ5Ih-uZnsw&m=LDRU1lBPWYWf9qU7EP9aqwRSUkMhKNITwLCbKWc1muE&s=9CgF9YNZW-KddZgYrxThkCGyrEOgAaovDTihJqHFhkk&e=
More information about the dev
mailing list