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://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 <[email protected]> > Signed-off-by: Darrell Ball <[email protected]> > --- > 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(). > 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); > } > _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
