[ovs-dev] [patch_v1] dpdk: Fix device cleanup.

Darrell Ball dball at vmware.com
Fri Jul 28 16:22:23 UTC 2017




-----Original Message-----
From: <ovs-dev-bounces at openvswitch.org> on behalf of "Fischetti, Antonio" <antonio.fischetti at intel.com>
Date: Friday, July 28, 2017 at 8:43 AM
To: Darrell Ball <dlu998 at gmail.com>, "dev at openvswitch.org" <dev at openvswitch.org>
Cc: Ilya Maximets <i.maximets at samsung.com>
Subject: Re: [ovs-dev] [patch_v1] dpdk: Fix device cleanup.

    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?

[Darrell]
I will add the minimal recipe of 4 steps in the commit message
It is in the link provided but that is embedded in several replies and I see that was not followed.

1/ Starting with 

darrell at prmh-nsx-perf-server125:~$ sudo ovs-vsctl show
1c50d8ee-b17f-4fac-a595-03b0da8c8275
    Bridge "br0"
        Port "br0"
            Interface "br0"
                type: internal
        Port "dpdk1"
            Interface "dpdk1"
                type: dpdk
                options: {dpdk-devargs="0000:04:00.1"}
        Port "dpdk0"
            Interface "dpdk0"
                type: dpdk
                options: {dpdk-devargs="0000:04:00.0"}

darrell at prmh-nsx-perf-server125:~$ /usr/src/dpdk-16.11/tools/dpdk-devbind.py --status

Network devices using DPDK-compatible driver
============================================
0000:04:00.0 'Ethernet Controller 10-Gigabit X540-AT2' drv=uio_pci_generic unused=ixgbe,vfio-pci
0000:04:00.1 'Ethernet Controller 10-Gigabit X540-AT2' drv=uio_pci_generic unused=ixgbe,vfio-pci

2/ restart vswitchd

3/ run
 sudo ovs-vsctl del-port br0 dpdk1 

and find the interface is NOT detached
NO info log ‘Device '0000:04:00.1' has been detached’
Instrumented function tracing shows same

4/ To detach, run the reinstated command 
ovs-appctl netdev-dpdk/detach 0000:04:00.1
Observe printf to console
‘Device '0000:04:00.1' has been detached’
Instrumented function tracing shows same

////////////////////////////////////////////////
    
    
    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://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=JKDaMV4Tg98XfYuRRB0EnNSg1VGxX0Cn94m67796NUU&s=GybcZ30-FUEi-0BaiOtBknu21TwWNfEsXgzjmY_IM6M&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.
    >  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=JKDaMV4Tg98XfYuRRB0EnNSg1VGxX0Cn94m67796NUU&s=tVhJN1WR2mXq4eWiernRTn4HWnmjnp3x6TJiIoik_EQ&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);
    >      }
    > --
    > 1.9.1
    > 
    > _______________________________________________
    > 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=JKDaMV4Tg98XfYuRRB0EnNSg1VGxX0Cn94m67796NUU&s=msyyZIsmnrKcTbyB3eaUelH8BK0ufDR_21KtGSDN4zA&e= 
    _______________________________________________
    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=JKDaMV4Tg98XfYuRRB0EnNSg1VGxX0Cn94m67796NUU&s=msyyZIsmnrKcTbyB3eaUelH8BK0ufDR_21KtGSDN4zA&e= 
    



More information about the dev mailing list