From patchwork Tue Jun 6 08:30:49 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Darrell Ball X-Patchwork-Id: 771695 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from mail.linuxfoundation.org (mail.linuxfoundation.org [140.211.169.12]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 3whlJV13d8z9s78 for ; Tue, 6 Jun 2017 18:31:06 +1000 (AEST) Authentication-Results: ozlabs.org; dkim=fail reason="signature verification failed" (2048-bit key; unprotected) header.d=gmail.com header.i=@gmail.com header.b="DIEd4IFG"; dkim-atps=neutral Received: from mail.linux-foundation.org (localhost [127.0.0.1]) by mail.linuxfoundation.org (Postfix) with ESMTP id 0FC11A5E; Tue, 6 Jun 2017 08:31:04 +0000 (UTC) X-Original-To: dev@openvswitch.org Delivered-To: ovs-dev@mail.linuxfoundation.org Received: from smtp1.linuxfoundation.org (smtp1.linux-foundation.org [172.17.192.35]) by mail.linuxfoundation.org (Postfix) with ESMTPS id 7138E904 for ; Tue, 6 Jun 2017 08:31:02 +0000 (UTC) X-Greylist: whitelisted by SQLgrey-1.7.6 Received: from mail-pf0-f196.google.com (mail-pf0-f196.google.com [209.85.192.196]) by smtp1.linuxfoundation.org (Postfix) with ESMTPS id C253CE2 for ; Tue, 6 Jun 2017 08:31:01 +0000 (UTC) Received: by mail-pf0-f196.google.com with SMTP id y7so5115648pfd.3 for ; Tue, 06 Jun 2017 01:31:01 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=from:to:cc:subject:date:message-id; bh=sGT735Y/cJJj1sc7jGFPIzZIIfvtYLoOVHIykdu5jps=; b=DIEd4IFGJozq30bQgCAQu2cA5UC9GS+a+FnnqcV5iWoV3kPL1uHX4eP2YrmoHRJeYG nvOpuSGb9Hq9SWAlOBdZtneDSGXkC7EGXatsrKjr408Zgjboj9CUIY+KMIwGFD26fLW4 aGTLy7yc/V1hGokT7O7W5/pVOPNiMHk4vWD46Fy0wAnNTMhfb5rZkKtLUB8DIZsYLTDg EIace+tQxAFqGP982f2/hbbwtfdPWXEt4K2rBWi0uHyF9hx05jzD2gS50DSLLvL8dZYR sf+4la3FKh9KkJia/VCqu6DB0rDkyTIHYxgA2zV+uD1nAHnDBB3efww4MBwh7BJLm/g9 zryQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:cc:subject:date:message-id; bh=sGT735Y/cJJj1sc7jGFPIzZIIfvtYLoOVHIykdu5jps=; b=dm3CZVSMEeIN/GO2Jdwd3hieNL+FEcPfVCzgxck91ynIj56EaNoxRDEaa59cCoo9mN 87HpWl5Dy9e694gGkQoJQsvK/6UP5CYZSfWT92TNGn0uQhFmALWKnbZEodzT0/6gZJh4 ae9s5vMy5tX5GQOyEMddfVIY/GHRUAiE3m1isdekP/+ecC86L+ULI5sofubIFJfJ4cND lCUcHm3SBLGTiIgIyg3Y+WdXajHdAQzfqYjcTHYT2k21oEmwnf2+JjtahpW+sjx6HjzW DP5S78xpitPG6yQ8hjtoAL4HMsUs2CfFgL+EYdIOqbKiAlQ3d4IVLAzr/pSKdaxvFfXv v7Jg== X-Gm-Message-State: AODbwcDaetz3L2NEjwUeB5ME9BMw6Bc9HXdGhLpgTVR1exRhl3k1EjWC ggAVbGtTmKuF7Xh2 X-Received: by 10.98.245.155 with SMTP id b27mr24222268pfm.181.1496737861227; Tue, 06 Jun 2017 01:31:01 -0700 (PDT) Received: from ubuntu.localdomain (c-73-162-236-45.hsd1.ca.comcast.net. [73.162.236.45]) by smtp.gmail.com with ESMTPSA id o66sm70208692pga.64.2017.06.06.01.31.00 (version=TLS1_2 cipher=ECDHE-RSA-AES128-SHA bits=128/128); Tue, 06 Jun 2017 01:31:00 -0700 (PDT) From: Darrell Ball To: dev@openvswitch.org Date: Tue, 6 Jun 2017 01:30:49 -0700 Message-Id: <1496737849-50926-1-git-send-email-dlu998@gmail.com> X-Mailer: git-send-email 1.9.1 X-Spam-Status: No, score=-1.7 required=5.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,FREEMAIL_ENVFROM_END_DIGIT,FREEMAIL_FROM, RCVD_IN_DNSWL_NONE autolearn=no version=3.3.1 X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on smtp1.linux-foundation.org Cc: Ilya Maximets Subject: [ovs-dev] [patch_v1] dpdk: Fix device cleanup. X-BeenThere: ovs-dev@openvswitch.org X-Mailman-Version: 2.1.12 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , MIME-Version: 1.0 Sender: ovs-dev-bounces@openvswitch.org Errors-To: ovs-dev-bounces@openvswitch.org 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. Fixes: 5dcde09c80a8 ("netdev-dpdk: Fix device leak on port deletion.") CC: Ilya Maximets Signed-off-by: Darrell Ball --- 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;