From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-wr1-f68.google.com (mail-wr1-f68.google.com [209.85.221.68]) by dpdk.org (Postfix) with ESMTP id 7A1642B92 for ; Mon, 19 Nov 2018 13:26:04 +0100 (CET) Received: by mail-wr1-f68.google.com with SMTP id u9-v6so31835164wrr.0 for ; Mon, 19 Nov 2018 04:26:04 -0800 (PST) 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:in-reply-to :references:mime-version:content-transfer-encoding; bh=gfaD/hvKY9qSJYoNoU3jClEeMTvLWSchAEHtULkNmTk=; b=bMgFUnoHmU6a5f9x8wjW0/gpsb1f4TPVCHle9Vcl3ZrrwP502vz/FyxJo1JeRXtn0e oCzTrbVAuBhJYpFMR71QMgqiL3QF0eS9yU6bqIE30TY9xW4RXWzwkjOiQMLziKJts2SJ iGVhQcBF+oqKySaKyTYYgX8zJ+lHPDOnhhxafdW1Zv2b92pwSZaB+TJ/M8EJ48Hw+Vsi QphF0MkHg5RVYM8yX7gg0dQCaYcpVR3l4jc6fhybNumv/RCZYZb3PAMoEqEbP5U57yrX I/ZBBgSeyCzfMBOZ8QY0WsoLermUjfJKcwaVyhBVrZLP66hEFypp4D4SLevdqoz+xfc/ i0/Q== X-Gm-Message-State: AA+aEWbPwheCfiixvv4uHtR17791McGBPvfMJgymjN1NtN+Rh89sG6fu j23gdDCoeYKih1ttEYAT/u8dK8qx X-Google-Smtp-Source: AFSGD/Vwd5BS0aTRK46FoNXG0WvQsSFSxC2yaeutnVoFuGvKrg0VpF0yFjXSgQ4PsRMXyBPy7qjiMA== X-Received: by 2002:adf:e9d1:: with SMTP id l17mr6338898wrn.73.1542630364015; Mon, 19 Nov 2018 04:26:04 -0800 (PST) Received: from localhost ([2a01:4b00:f419:6f00:8361:8946:ba2b:d556]) by smtp.gmail.com with ESMTPSA id b129sm13720994wmd.24.2018.11.19.04.26.03 (version=TLS1_2 cipher=ECDHE-RSA-CHACHA20-POLY1305 bits=256/256); Mon, 19 Nov 2018 04:26:03 -0800 (PST) From: Luca Boccassi To: Wenzhuo Lu Cc: Ferruh Yigit , Andrew Rybchenko , dpdk stable Date: Mon, 19 Nov 2018 12:25:30 +0000 Message-Id: <20181119122538.14207-13-bluca@debian.org> X-Mailer: git-send-email 2.19.1 In-Reply-To: <20181119122538.14207-1-bluca@debian.org> References: <20181108180111.25873-1-bluca@debian.org> <20181119122538.14207-1-bluca@debian.org> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit Subject: [dpdk-stable] patch 'ethdev: fix invalid configuration after failure' has been queued to LTS release 16.11.9 X-BeenThere: stable@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: patches for DPDK stable branches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 19 Nov 2018 12:26:04 -0000 Hi, FYI, your patch has been queued to LTS release 16.11.9 Note it hasn't been pushed to http://dpdk.org/browse/dpdk-stable yet. It will be pushed if I get no objections before 11/21/18. So please shout if anyone has objections. Also note that after the patch there's a diff of the upstream commit vs the patch applied to the branch. If the code is different (ie: not only metadata diffs), due for example to a change in context or macro names, please double check it. Thanks. Luca Boccassi --- >>From 729e04dd9956bcccd08acf7f97e76b3ec452100f Mon Sep 17 00:00:00 2001 From: Wenzhuo Lu Date: Tue, 13 Nov 2018 11:12:36 +0000 Subject: [PATCH] ethdev: fix invalid configuration after failure [ upstream commit aa28ec5d27b0ead28877081b30ccf0b74a16bbcd ] The new configuration is stored during the rte_eth_dev_configure() API but the API may fail. After failure stored configuration will be invalid since it is not fully applied to the device. We better roll the configuration back after failure. Fixes: af75078fece3 ("first public release") Signed-off-by: Wenzhuo Lu Signed-off-by: Ferruh Yigit Reviewed-by: Andrew Rybchenko --- lib/librte_ether/rte_ethdev.c | 37 ++++++++++++++++++++++++++--------- 1 file changed, 28 insertions(+), 9 deletions(-) diff --git a/lib/librte_ether/rte_ethdev.c b/lib/librte_ether/rte_ethdev.c index 9b28e95ae..59722e04e 100644 --- a/lib/librte_ether/rte_ethdev.c +++ b/lib/librte_ether/rte_ethdev.c @@ -795,7 +795,9 @@ rte_eth_dev_configure(uint8_t port_id, uint16_t nb_rx_q, uint16_t nb_tx_q, { struct rte_eth_dev *dev; struct rte_eth_dev_info dev_info; + struct rte_eth_conf orig_conf; int diag; + int ret; RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -EINVAL); @@ -824,6 +826,9 @@ rte_eth_dev_configure(uint8_t port_id, uint16_t nb_rx_q, uint16_t nb_tx_q, return -EBUSY; } + /* Store original config, as rollback required on failure */ + memcpy(&orig_conf, &dev->data->dev_conf, sizeof(dev->data->dev_conf)); + /* Copy the dev_conf parameter into the dev structure */ memcpy(&dev->data->dev_conf, dev_conf, sizeof(dev->data->dev_conf)); @@ -836,19 +841,22 @@ rte_eth_dev_configure(uint8_t port_id, uint16_t nb_rx_q, uint16_t nb_tx_q, if (nb_rx_q == 0 && nb_tx_q == 0) { RTE_PMD_DEBUG_TRACE("ethdev port_id=%d both rx and tx queue cannot be 0\n", port_id); - return -EINVAL; + ret = -EINVAL; + goto rollback; } if (nb_rx_q > dev_info.max_rx_queues) { RTE_PMD_DEBUG_TRACE("ethdev port_id=%d nb_rx_queues=%d > %d\n", port_id, nb_rx_q, dev_info.max_rx_queues); - return -EINVAL; + ret = -EINVAL; + goto rollback; } if (nb_tx_q > dev_info.max_tx_queues) { RTE_PMD_DEBUG_TRACE("ethdev port_id=%d nb_tx_queues=%d > %d\n", port_id, nb_tx_q, dev_info.max_tx_queues); - return -EINVAL; + ret = -EINVAL; + goto rollback; } /* @@ -859,7 +867,8 @@ rte_eth_dev_configure(uint8_t port_id, uint16_t nb_rx_q, uint16_t nb_tx_q, (!(dev->data->dev_flags & RTE_ETH_DEV_INTR_LSC))) { RTE_PMD_DEBUG_TRACE("driver %s does not support lsc\n", dev->data->drv_name); - return -EINVAL; + ret = -EINVAL; + goto rollback; } /* @@ -874,14 +883,16 @@ rte_eth_dev_configure(uint8_t port_id, uint16_t nb_rx_q, uint16_t nb_tx_q, port_id, (unsigned)dev_conf->rxmode.max_rx_pkt_len, (unsigned)dev_info.max_rx_pktlen); - return -EINVAL; + ret = -EINVAL; + goto rollback; } else if (dev_conf->rxmode.max_rx_pkt_len < ETHER_MIN_LEN) { RTE_PMD_DEBUG_TRACE("ethdev port_id=%d max_rx_pkt_len %u" " < min valid value %u\n", port_id, (unsigned)dev_conf->rxmode.max_rx_pkt_len, (unsigned)ETHER_MIN_LEN); - return -EINVAL; + ret = -EINVAL; + goto rollback; } } else { if (dev_conf->rxmode.max_rx_pkt_len < ETHER_MIN_LEN || @@ -898,7 +909,8 @@ rte_eth_dev_configure(uint8_t port_id, uint16_t nb_rx_q, uint16_t nb_tx_q, if (diag != 0) { RTE_PMD_DEBUG_TRACE("port%d rte_eth_dev_rx_queue_config = %d\n", port_id, diag); - return diag; + ret = diag; + goto rollback; } diag = rte_eth_dev_tx_queue_config(dev, nb_tx_q); @@ -906,7 +918,8 @@ rte_eth_dev_configure(uint8_t port_id, uint16_t nb_rx_q, uint16_t nb_tx_q, RTE_PMD_DEBUG_TRACE("port%d rte_eth_dev_tx_queue_config = %d\n", port_id, diag); rte_eth_dev_rx_queue_config(dev, 0); - return diag; + ret = diag; + goto rollback; } diag = (*dev->dev_ops->dev_configure)(dev); @@ -915,10 +928,16 @@ rte_eth_dev_configure(uint8_t port_id, uint16_t nb_rx_q, uint16_t nb_tx_q, port_id, diag); rte_eth_dev_rx_queue_config(dev, 0); rte_eth_dev_tx_queue_config(dev, 0); - return diag; + ret = diag; + goto rollback; } return 0; + +rollback: + memcpy(&dev->data->dev_conf, &orig_conf, sizeof(dev->data->dev_conf)); + + return ret; } static void -- 2.19.1 --- Diff of the applied patch vs upstream commit (please double-check if non-empty: --- --- - 2018-11-19 12:15:18.376202148 +0000 +++ 0013-ethdev-fix-invalid-configuration-after-failure.patch 2018-11-19 12:15:18.099611432 +0000 @@ -1,8 +1,10 @@ -From aa28ec5d27b0ead28877081b30ccf0b74a16bbcd Mon Sep 17 00:00:00 2001 +From 729e04dd9956bcccd08acf7f97e76b3ec452100f Mon Sep 17 00:00:00 2001 From: Wenzhuo Lu Date: Tue, 13 Nov 2018 11:12:36 +0000 Subject: [PATCH] ethdev: fix invalid configuration after failure +[ upstream commit aa28ec5d27b0ead28877081b30ccf0b74a16bbcd ] + The new configuration is stored during the rte_eth_dev_configure() API but the API may fail. After failure stored configuration will be invalid since it is not fully applied to the device. @@ -10,87 +12,86 @@ We better roll the configuration back after failure. Fixes: af75078fece3 ("first public release") -Cc: stable@dpdk.org Signed-off-by: Wenzhuo Lu Signed-off-by: Ferruh Yigit Reviewed-by: Andrew Rybchenko --- - lib/librte_ethdev/rte_ethdev.c | 49 +++++++++++++++++++++++++--------- - 1 file changed, 36 insertions(+), 13 deletions(-) + lib/librte_ether/rte_ethdev.c | 37 ++++++++++++++++++++++++++--------- + 1 file changed, 28 insertions(+), 9 deletions(-) -diff --git a/lib/librte_ethdev/rte_ethdev.c b/lib/librte_ethdev/rte_ethdev.c -index 8eaa5fcc7..04dff1f5e 100644 ---- a/lib/librte_ethdev/rte_ethdev.c -+++ b/lib/librte_ethdev/rte_ethdev.c -@@ -1092,8 +1092,10 @@ rte_eth_dev_configure(uint16_t port_id, uint16_t nb_rx_q, uint16_t nb_tx_q, +diff --git a/lib/librte_ether/rte_ethdev.c b/lib/librte_ether/rte_ethdev.c +index 9b28e95ae..59722e04e 100644 +--- a/lib/librte_ether/rte_ethdev.c ++++ b/lib/librte_ether/rte_ethdev.c +@@ -795,7 +795,9 @@ rte_eth_dev_configure(uint8_t port_id, uint16_t nb_rx_q, uint16_t nb_tx_q, { struct rte_eth_dev *dev; struct rte_eth_dev_info dev_info; + struct rte_eth_conf orig_conf; - struct rte_eth_conf local_conf = *dev_conf; int diag; + int ret; RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -EINVAL); -@@ -1140,6 +1142,9 @@ rte_eth_dev_configure(uint16_t port_id, uint16_t nb_rx_q, uint16_t nb_tx_q, +@@ -824,6 +826,9 @@ rte_eth_dev_configure(uint8_t port_id, uint16_t nb_rx_q, uint16_t nb_tx_q, return -EBUSY; } -+ /* Store original config, as rollback required on failure */ ++ /* Store original config, as rollback required on failure */ + memcpy(&orig_conf, &dev->data->dev_conf, sizeof(dev->data->dev_conf)); + /* Copy the dev_conf parameter into the dev structure */ - memcpy(&dev->data->dev_conf, &local_conf, sizeof(dev->data->dev_conf)); + memcpy(&dev->data->dev_conf, dev_conf, sizeof(dev->data->dev_conf)); -@@ -1151,13 +1156,15 @@ rte_eth_dev_configure(uint16_t port_id, uint16_t nb_rx_q, uint16_t nb_tx_q, - if (nb_rx_q > dev_info.max_rx_queues) { - RTE_ETHDEV_LOG(ERR, "Ethdev port_id=%u nb_rx_queues=%u > %u\n", - port_id, nb_rx_q, dev_info.max_rx_queues); +@@ -836,19 +841,22 @@ rte_eth_dev_configure(uint8_t port_id, uint16_t nb_rx_q, uint16_t nb_tx_q, + + if (nb_rx_q == 0 && nb_tx_q == 0) { + RTE_PMD_DEBUG_TRACE("ethdev port_id=%d both rx and tx queue cannot be 0\n", port_id); - return -EINVAL; + ret = -EINVAL; + goto rollback; } - if (nb_tx_q > dev_info.max_tx_queues) { - RTE_ETHDEV_LOG(ERR, "Ethdev port_id=%u nb_tx_queues=%u > %u\n", - port_id, nb_tx_q, dev_info.max_tx_queues); + if (nb_rx_q > dev_info.max_rx_queues) { + RTE_PMD_DEBUG_TRACE("ethdev port_id=%d nb_rx_queues=%d > %d\n", + port_id, nb_rx_q, dev_info.max_rx_queues); - return -EINVAL; + ret = -EINVAL; + goto rollback; } - /* Check that the device supports requested interrupts */ -@@ -1165,13 +1172,15 @@ rte_eth_dev_configure(uint16_t port_id, uint16_t nb_rx_q, uint16_t nb_tx_q, - (!(dev->data->dev_flags & RTE_ETH_DEV_INTR_LSC))) { - RTE_ETHDEV_LOG(ERR, "Driver %s does not support lsc\n", - dev->device->driver->name); + if (nb_tx_q > dev_info.max_tx_queues) { + RTE_PMD_DEBUG_TRACE("ethdev port_id=%d nb_tx_queues=%d > %d\n", + port_id, nb_tx_q, dev_info.max_tx_queues); - return -EINVAL; + ret = -EINVAL; + goto rollback; } - if ((dev_conf->intr_conf.rmv == 1) && - (!(dev->data->dev_flags & RTE_ETH_DEV_INTR_RMV))) { - RTE_ETHDEV_LOG(ERR, "Driver %s does not support rmv\n", - dev->device->driver->name); -- return -EINVAL; -+ ret = -EINVAL; -+ goto rollback; + + /* +@@ -859,7 +867,8 @@ rte_eth_dev_configure(uint8_t port_id, uint16_t nb_rx_q, uint16_t nb_tx_q, + (!(dev->data->dev_flags & RTE_ETH_DEV_INTR_LSC))) { + RTE_PMD_DEBUG_TRACE("driver %s does not support lsc\n", + dev->data->drv_name); +- return -EINVAL; ++ ret = -EINVAL; ++ goto rollback; } /* -@@ -1184,13 +1193,15 @@ rte_eth_dev_configure(uint16_t port_id, uint16_t nb_rx_q, uint16_t nb_tx_q, - "Ethdev port_id=%u max_rx_pkt_len %u > max valid value %u\n", - port_id, dev_conf->rxmode.max_rx_pkt_len, - dev_info.max_rx_pktlen); +@@ -874,14 +883,16 @@ rte_eth_dev_configure(uint8_t port_id, uint16_t nb_rx_q, uint16_t nb_tx_q, + port_id, + (unsigned)dev_conf->rxmode.max_rx_pkt_len, + (unsigned)dev_info.max_rx_pktlen); - return -EINVAL; + ret = -EINVAL; + goto rollback; } else if (dev_conf->rxmode.max_rx_pkt_len < ETHER_MIN_LEN) { - RTE_ETHDEV_LOG(ERR, - "Ethdev port_id=%u max_rx_pkt_len %u < min valid value %u\n", - port_id, dev_conf->rxmode.max_rx_pkt_len, + RTE_PMD_DEBUG_TRACE("ethdev port_id=%d max_rx_pkt_len %u" + " < min valid value %u\n", + port_id, + (unsigned)dev_conf->rxmode.max_rx_pkt_len, (unsigned)ETHER_MIN_LEN); - return -EINVAL; + ret = -EINVAL; @@ -98,49 +99,19 @@ } } else { if (dev_conf->rxmode.max_rx_pkt_len < ETHER_MIN_LEN || -@@ -1209,7 +1220,8 @@ rte_eth_dev_configure(uint16_t port_id, uint16_t nb_rx_q, uint16_t nb_tx_q, - port_id, local_conf.rxmode.offloads, - dev_info.rx_offload_capa, - __func__); -- return -EINVAL; -+ ret = -EINVAL; -+ goto rollback; - } - if ((local_conf.txmode.offloads & dev_info.tx_offload_capa) != - local_conf.txmode.offloads) { -@@ -1219,7 +1231,8 @@ rte_eth_dev_configure(uint16_t port_id, uint16_t nb_rx_q, uint16_t nb_tx_q, - port_id, local_conf.txmode.offloads, - dev_info.tx_offload_capa, - __func__); -- return -EINVAL; -+ ret = -EINVAL; -+ goto rollback; - } - - /* Check that device supports requested rss hash functions. */ -@@ -1230,7 +1243,8 @@ rte_eth_dev_configure(uint16_t port_id, uint16_t nb_rx_q, uint16_t nb_tx_q, - "Ethdev port_id=%u invalid rss_hf: 0x%"PRIx64", valid value: 0x%"PRIx64"\n", - port_id, dev_conf->rx_adv_conf.rss_conf.rss_hf, - dev_info.flow_type_rss_offloads); -- return -EINVAL; -+ ret = -EINVAL; -+ goto rollback; - } - - /* -@@ -1241,7 +1255,8 @@ rte_eth_dev_configure(uint16_t port_id, uint16_t nb_rx_q, uint16_t nb_tx_q, - RTE_ETHDEV_LOG(ERR, - "Port%u rte_eth_dev_rx_queue_config = %d\n", - port_id, diag); +@@ -898,7 +909,8 @@ rte_eth_dev_configure(uint8_t port_id, uint16_t nb_rx_q, uint16_t nb_tx_q, + if (diag != 0) { + RTE_PMD_DEBUG_TRACE("port%d rte_eth_dev_rx_queue_config = %d\n", + port_id, diag); - return diag; + ret = diag; + goto rollback; } diag = rte_eth_dev_tx_queue_config(dev, nb_tx_q); -@@ -1250,7 +1265,8 @@ rte_eth_dev_configure(uint16_t port_id, uint16_t nb_rx_q, uint16_t nb_tx_q, - "Port%u rte_eth_dev_tx_queue_config = %d\n", - port_id, diag); +@@ -906,7 +918,8 @@ rte_eth_dev_configure(uint8_t port_id, uint16_t nb_rx_q, uint16_t nb_tx_q, + RTE_PMD_DEBUG_TRACE("port%d rte_eth_dev_tx_queue_config = %d\n", + port_id, diag); rte_eth_dev_rx_queue_config(dev, 0); - return diag; + ret = diag; @@ -148,22 +119,12 @@ } diag = (*dev->dev_ops->dev_configure)(dev); -@@ -1259,7 +1275,8 @@ rte_eth_dev_configure(uint16_t port_id, uint16_t nb_rx_q, uint16_t nb_tx_q, - port_id, diag); +@@ -915,10 +928,16 @@ rte_eth_dev_configure(uint8_t port_id, uint16_t nb_rx_q, uint16_t nb_tx_q, + port_id, diag); rte_eth_dev_rx_queue_config(dev, 0); rte_eth_dev_tx_queue_config(dev, 0); -- return eth_err(port_id, diag); -+ ret = eth_err(port_id, diag); -+ goto rollback; - } - - /* Initialize Rx profiling if enabled at compilation time. */ -@@ -1269,10 +1286,16 @@ rte_eth_dev_configure(uint16_t port_id, uint16_t nb_rx_q, uint16_t nb_tx_q, - port_id, diag); - rte_eth_dev_rx_queue_config(dev, 0); - rte_eth_dev_tx_queue_config(dev, 0); -- return eth_err(port_id, diag); -+ ret = eth_err(port_id, diag); +- return diag; ++ ret = diag; + goto rollback; } @@ -175,7 +136,7 @@ + return ret; } - void + static void -- 2.19.1