[ovs-dev] [patch_v1] dpdk: Fix device cleanup.
Fischetti, Antonio
antonio.fischetti at intel.com
Fri Jul 28 15:43:43 UTC 2017
LGTM I agree with this safe approach.
I did some basic testing with a Niantic NIC but I couldn't
replicate the issue. Below some details on my testing.
To cause the issue I killed and restarted vswitchd only, so
that it could find and run with an existing database with
the devices already bound to dpdk. Then I deleted one dpdk port
and re-added it later on.
In both cases - with and without this patch - I couldn't see
any difference, in particular when debugging the following
functions:
common_construct()
netdev_dpdk_destruct()
netdev_dpdk_process_devargs()
With the patch, when I delete a port I don't see messages like
Device '0000:xx:00.0' has been detached
should I expect to see that?
I saw that if I call
ovs-appctl netdev-dpdk/detach 0000:xx:00.0
before deleting the port it works fine by displaying
Device YYY is being used by interface... Remove it...
Regards,
-Antonio
> -----Original Message-----
> From: ovs-dev-bounces at openvswitch.org [mailto:ovs-dev-
> bounces at openvswitch.org] On Behalf Of Darrell Ball
> Sent: Wednesday, July 26, 2017 6:10 AM
> To: dev at openvswitch.org
> Cc: Ilya Maximets <i.maximets at samsung.com>
> Subject: [ovs-dev] [patch_v1] dpdk: Fix device cleanup.
>
> 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://mail.openvswitch.org/pipermail/ovs-dev/2017-June/333462.html
> 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.
> For more information please refer to the `DPDK Port Hotplug Framework
>
> <http://dpdk.org/doc/guides/prog_guide/port_hotplug_framework.html#hotplug
> >`__.
> 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);
> }
> --
> 1.9.1
>
> _______________________________________________
> dev mailing list
> dev at openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
More information about the dev
mailing list