From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-pl0-f65.google.com (mail-pl0-f65.google.com [209.85.160.65]) by dpdk.org (Postfix) with ESMTP id C2F3A1B296 for ; Wed, 1 Aug 2018 23:59:55 +0200 (CEST) Received: by mail-pl0-f65.google.com with SMTP id d5-v6so56414pll.4 for ; Wed, 01 Aug 2018 14:59:55 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=networkplumber-org.20150623.gappssmtp.com; s=20150623; h=from:to:cc:subject:date:message-id; bh=sC8clQkZR4wfb6bMeYDiHpDu6eAUVoJLguVPni/1TSI=; b=QMWYFhw1jXJ7AaRnAHoOiLIQPJwG9j4K4bSnf+nGEavdf+IYurkFTZe9AfS7z7y1EW Q5ZC9pOqlflNYeta6AtLS4H9oQZ7k83lgVh0ndiX7aFd/pfx0M9o53OICtPpOL9o8Ojk /KaDrvKm0hAee88AMyAq5IXN3lvaTTQaAtjcCv1UlNeiQ/aANdEf+Bsx0ctw8dtpPjVt QCEG9my0HEVSX51wpk+rwZ3C/l8zANzQliaunCu5CLG4FyeTkF1IkubcB9tLIpCfViiV /QiEEw6U+rLslqVSVMErWEg4YwtofaDotK9mUGcZhN8TOrJLbjRTzbefM9jvpN8m7nSF 7J4w== 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=sC8clQkZR4wfb6bMeYDiHpDu6eAUVoJLguVPni/1TSI=; b=V+u2jSbNUAz2RRNb+IsJCEFGAXdYwxFN5BMO2haJPWAOssEsjkx7cmeGyTC5vJ3tsd /aJ1pBSp60xHXEfqsse/rLVl22f/xBQbjMyyOFJS4pTuKzQhFY+9veGwXOOotpfYb25O j5GYfTjKudrXuJgbikoS/hf5eFM7Dyl/gAPS3isY5MnmVVEemuVxMfdsr6RfZEsmyMLD zM7OZLLXYSq4a9/sUDK5bXx3v5efGZhC++mHjM00rxQ399B3YvRYlr94h2XgKqAEDVoP JYZfx0iyI+Iwv9+U4+t6ADPU2T4Ekl7LZCq9oMdosqjZfxJUENahXU1gOCQJxVPQj+dC 54jg== X-Gm-Message-State: AOUpUlEI12VahSy/Nb7PKd6vnSfmWfJXtQIuEaW7rECdiZWBZDT4YRmN G3wdewSr1nTk1rof5MxHvGaUDw== X-Google-Smtp-Source: AAOMgpduecjG0T64FLl2AqG+2ckgZZyQdvJqlC6gK0xZNiHbMkcbUooud01H2QrEfpuFpQCxKmgqxQ== X-Received: by 2002:a17:902:6802:: with SMTP id h2-v6mr86566plk.113.1533160794858; Wed, 01 Aug 2018 14:59:54 -0700 (PDT) Received: from xeon-e3.wavecable.com (204-195-22-127.wavecable.com. [204.195.22.127]) by smtp.gmail.com with ESMTPSA id h132-v6sm74459pfc.100.2018.08.01.14.59.53 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Wed, 01 Aug 2018 14:59:54 -0700 (PDT) From: Stephen Hemminger To: shahafs@mellanox.com, yskoh@mellanox.com Cc: dev@dpdk.org, Stephen Hemminger , Stephen Hemminger Date: Wed, 1 Aug 2018 14:59:52 -0700 Message-Id: <20180801215952.25326-1-stephen@networkplumber.org> X-Mailer: git-send-email 2.18.0 Subject: [dpdk-dev] [RFC] mlx5: fix error unwind in device start X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 01 Aug 2018 21:59:56 -0000 The error handling in start of the mlx5 driver is buggy. For example, if setting up the flows fails the device driver will then get stuck in mlx5_flow_rxq_flags_clear waiting for something that will never happen. The problem is that the code jumps to a common error label and does unwind for portions of the driver which have not been setup. This suggested patch breaks it into different labels with each failure path only unwinding what was done. Also, the ethdev driver should not be manipulating the dev_started flag directly. That is handled by the common ethdev layer. The patch works for the success case, but furthur testing is needed to actually exercise all the error paths. This is left as exercise for the maintainers. Signed-off-by: Stephen Hemminger --- drivers/net/mlx5/mlx5_trigger.c | 26 +++++++++++++------------- 1 file changed, 13 insertions(+), 13 deletions(-) diff --git a/drivers/net/mlx5/mlx5_trigger.c b/drivers/net/mlx5/mlx5_trigger.c index e2a9bb703261..79a7b233986a 100644 --- a/drivers/net/mlx5/mlx5_trigger.c +++ b/drivers/net/mlx5/mlx5_trigger.c @@ -171,42 +171,42 @@ mlx5_dev_start(struct rte_eth_dev *dev) if (ret) { DRV_LOG(ERR, "port %u Rx queue allocation failed: %s", dev->data->port_id, strerror(rte_errno)); - mlx5_txq_stop(dev); - return -rte_errno; + goto error_txq_stop; } - dev->data->dev_started = 1; + ret = mlx5_rx_intr_vec_enable(dev); if (ret) { DRV_LOG(ERR, "port %u Rx interrupt vector creation failed", dev->data->port_id); - goto error; + goto error_rxq_stop; } mlx5_xstats_init(dev); ret = mlx5_traffic_enable(dev); if (ret) { DRV_LOG(DEBUG, "port %u failed to set defaults flows", dev->data->port_id); - goto error; + goto error_intr_vec_disable; } ret = mlx5_flow_start(dev, &priv->flows); if (ret) { DRV_LOG(DEBUG, "port %u failed to set flows", dev->data->port_id); - goto error; + goto error_traffic_disable; } + dev->tx_pkt_burst = mlx5_select_tx_function(dev); dev->rx_pkt_burst = mlx5_select_rx_function(dev); mlx5_dev_interrupt_handler_install(dev); return 0; -error: - ret = rte_errno; /* Save rte_errno before cleanup. */ - /* Rollback. */ - dev->data->dev_started = 0; - mlx5_flow_stop(dev, &priv->flows); + +error_traffic_disable: mlx5_traffic_disable(dev); - mlx5_txq_stop(dev); +error_intr_vec_disable: + mlx5_rx_intr_vec_disable(dev); +error_rxq_stop: mlx5_rxq_stop(dev); - rte_errno = ret; /* Restore rte_errno. */ +error_txq_stop: + mlx5_txq_stop(dev); return -rte_errno; } -- 2.18.0