From patchwork Tue Aug 1 12:09:29 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: =?utf-8?q?N=C3=A9lio_Laranjeiro?= X-Patchwork-Id: 27308 X-Patchwork-Delegate: ferruh.yigit@amd.com Return-Path: X-Original-To: patchwork@dpdk.org Delivered-To: patchwork@dpdk.org Received: from [92.243.14.124] (localhost [IPv6:::1]) by dpdk.org (Postfix) with ESMTP id B977D9B97; Tue, 1 Aug 2017 14:09:56 +0200 (CEST) Received: from mail-wm0-f42.google.com (mail-wm0-f42.google.com [74.125.82.42]) by dpdk.org (Postfix) with ESMTP id D3C9799F5 for ; Tue, 1 Aug 2017 14:09:51 +0200 (CEST) Received: by mail-wm0-f42.google.com with SMTP id k20so22430818wmg.0 for ; Tue, 01 Aug 2017 05:09:51 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=6wind-com.20150623.gappssmtp.com; s=20150623; h=from:to:cc:subject:date:message-id:in-reply-to:references :in-reply-to:references; bh=CI7vDdSMOhj//D61TCCbvGl5iA4AaxbpyOgsVANslJY=; b=mrdWbMxLMcqu4lhLRkTyzOpZBW8MalYW2V3MJ9oiM/vgD17iJ29Va3+LuhFh9JqH1/ v0V19megI2fPc0wUgnulx2DAkceKg2C5UPVzweLP4AfwSFFTJVhxxH0gMg2BD8QJz7D3 fmzq1g2OVIZJyX3tFwSQIfBMtZQA3Tf+wwOEZ5PTCySzNY8x1UNP4F21lTAjedRSw/d+ U21ZpGNqbw8fpOBheuE/ebIsIqtk0SM7gApt1m8UNv1/7nqfApzzyo4+7W0e3gtRu0m1 wXMe2P+BIMsLieiXfK5hX7n4S4zGxi+0tcmuyVuGMHhVFqI2R6v/miBmwFMnha+b+yhL XIgA== 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:in-reply-to:references; bh=CI7vDdSMOhj//D61TCCbvGl5iA4AaxbpyOgsVANslJY=; b=Jqh73KmHM938Al/tCb91fm5y6b6XaTrectHqAhZYRxQSKDDyc5o+gXM2IYa0FNuu9w d5cA9/1OjLx/MJLBWMGy3Ip7hJ/U3r8LpFlvVejCwX0lT+mTTlTPnKaaoAElXspmGWi0 rHBeXVEPH/lEm9yZUDSX0CvdXD0dw32LWXI0vwIxyX5HLTOjZ/GQJmIoRZ6aAurahEX9 u/Lcs2jCG0V+7Fe93rqlVgHUS5YQSlnfZjsKbtMrZPIbKx1AwgsB0ij5WjZSdbrx0hJW 1yoGA7w+D2/Ggb4Fz+WzjB+ucdRKTmMb5BeEU04MmOe/PwpS3ieu6vvkSQEbJEzEN0XT Otiw== X-Gm-Message-State: AIVw111p0lOHW9aQKSoH/mUMifnyzbZ4QCf4okiyUVnOpdDKi1hzjtWf +7qLWGK/X35d6iwj X-Received: by 10.28.11.212 with SMTP id 203mr1417496wml.105.1501589391373; Tue, 01 Aug 2017 05:09:51 -0700 (PDT) Received: from ping.dev.6wind.com (host.78.145.23.62.rev.coltfrance.com. [62.23.145.78]) by smtp.gmail.com with ESMTPSA id a3sm34318755wra.17.2017.08.01.05.09.50 (version=TLS1_2 cipher=ECDHE-RSA-AES128-SHA bits=128/128); Tue, 01 Aug 2017 05:09:50 -0700 (PDT) From: Nelio Laranjeiro To: Adrien Mazarguil , dev@dpdk.org Cc: Shahaf Shuler Date: Tue, 1 Aug 2017 14:09:29 +0200 Message-Id: X-Mailer: git-send-email 2.1.4 In-Reply-To: References: In-Reply-To: References: Subject: [dpdk-dev] [PATCH 3/5] net/mlx5: fix non working secondary process by removing it 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: , Errors-To: dev-bounces@dpdk.org Sender: "dev" Secondary process is a copy/paste of the mlx4 drivers, it was never tested and it even segfault at the secondary process start in the mlx5_pci_probe(). This makes more sense to wipe this non working feature to re-write a working and functional version. Fixes: a48deada651b ("mlx5: allow operation in secondary processes") Signed-off-by: Nelio Laranjeiro Acked-by: Shahaf Shuler --- drivers/net/mlx5/mlx5.c | 33 +------- drivers/net/mlx5/mlx5.h | 9 --- drivers/net/mlx5/mlx5_ethdev.c | 166 +---------------------------------------- drivers/net/mlx5/mlx5_rxq.c | 41 ---------- drivers/net/mlx5/mlx5_rxtx.h | 2 - drivers/net/mlx5/mlx5_txq.c | 41 ---------- 6 files changed, 4 insertions(+), 288 deletions(-) diff --git a/drivers/net/mlx5/mlx5.c b/drivers/net/mlx5/mlx5.c index bfcff70..bd66a7c 100644 --- a/drivers/net/mlx5/mlx5.c +++ b/drivers/net/mlx5/mlx5.c @@ -771,37 +771,8 @@ mlx5_pci_probe(struct rte_pci_driver *pci_drv, struct rte_pci_device *pci_dev) err = ENOMEM; goto port_error; } - - /* Secondary processes have to use local storage for their - * private data as well as a copy of eth_dev->data, but this - * pointer must not be modified before burst functions are - * actually called. */ - if (mlx5_is_secondary()) { - struct mlx5_secondary_data *sd = - &mlx5_secondary_data[eth_dev->data->port_id]; - sd->primary_priv = eth_dev->data->dev_private; - if (sd->primary_priv == NULL) { - ERROR("no private data for port %u", - eth_dev->data->port_id); - err = EINVAL; - goto port_error; - } - sd->shared_dev_data = eth_dev->data; - rte_spinlock_init(&sd->lock); - memcpy(sd->data.name, sd->shared_dev_data->name, - sizeof(sd->data.name)); - sd->data.dev_private = priv; - sd->data.rx_mbuf_alloc_failed = 0; - sd->data.mtu = ETHER_MTU; - sd->data.port_id = sd->shared_dev_data->port_id; - sd->data.mac_addrs = priv->mac; - eth_dev->tx_pkt_burst = mlx5_tx_burst_secondary_setup; - eth_dev->rx_pkt_burst = mlx5_rx_burst_secondary_setup; - } else { - eth_dev->data->dev_private = priv; - eth_dev->data->mac_addrs = priv->mac; - } - + eth_dev->data->dev_private = priv; + eth_dev->data->mac_addrs = priv->mac; eth_dev->device = &pci_dev->device; rte_eth_copy_pci_info(eth_dev, pci_dev); eth_dev->data->dev_flags |= RTE_ETH_DEV_DETACHABLE; diff --git a/drivers/net/mlx5/mlx5.h b/drivers/net/mlx5/mlx5.h index 31d0c49..2392be5 100644 --- a/drivers/net/mlx5/mlx5.h +++ b/drivers/net/mlx5/mlx5.h @@ -158,14 +158,6 @@ struct priv { rte_spinlock_t lock; /* Lock for control functions. */ }; -/* Local storage for secondary process data. */ -struct mlx5_secondary_data { - struct rte_eth_dev_data data; /* Local device data. */ - struct priv *primary_priv; /* Private structure from primary. */ - struct rte_eth_dev_data *shared_dev_data; /* Shared device data. */ - rte_spinlock_t lock; /* Port configuration lock. */ -} mlx5_secondary_data[RTE_MAX_ETHPORTS]; - /** * Lock private structure to protect it from concurrent access in the * control path. @@ -221,7 +213,6 @@ void priv_dev_interrupt_handler_uninstall(struct priv *, struct rte_eth_dev *); void priv_dev_interrupt_handler_install(struct priv *, struct rte_eth_dev *); int mlx5_set_link_down(struct rte_eth_dev *dev); int mlx5_set_link_up(struct rte_eth_dev *dev); -struct priv *mlx5_secondary_data_setup(struct priv *priv); void priv_select_tx_function(struct priv *); void priv_select_rx_function(struct priv *); diff --git a/drivers/net/mlx5/mlx5_ethdev.c b/drivers/net/mlx5/mlx5_ethdev.c index 36872b5..0e0a99e 100644 --- a/drivers/net/mlx5/mlx5_ethdev.c +++ b/drivers/net/mlx5/mlx5_ethdev.c @@ -126,12 +126,7 @@ struct ethtool_link_settings { struct priv * mlx5_get_priv(struct rte_eth_dev *dev) { - struct mlx5_secondary_data *sd; - - if (!mlx5_is_secondary()) - return dev->data->dev_private; - sd = &mlx5_secondary_data[dev->data->port_id]; - return sd->data.dev_private; + return dev->data->dev_private; } /** @@ -143,7 +138,7 @@ mlx5_get_priv(struct rte_eth_dev *dev) inline int mlx5_is_secondary(void) { - return rte_eal_process_type() != RTE_PROC_PRIMARY; + return rte_eal_process_type() == RTE_PROC_SECONDARY; } /** @@ -1336,163 +1331,6 @@ mlx5_set_link_up(struct rte_eth_dev *dev) } /** - * Configure secondary process queues from a private data pointer (primary - * or secondary) and update burst callbacks. Can take place only once. - * - * All queues must have been previously created by the primary process to - * avoid undefined behavior. - * - * @param priv - * Private data pointer from either primary or secondary process. - * - * @return - * Private data pointer from secondary process, NULL in case of error. - */ -struct priv * -mlx5_secondary_data_setup(struct priv *priv) -{ - unsigned int port_id = 0; - struct mlx5_secondary_data *sd; - void **tx_queues; - void **rx_queues; - unsigned int nb_tx_queues; - unsigned int nb_rx_queues; - unsigned int i; - - /* priv must be valid at this point. */ - assert(priv != NULL); - /* priv->dev must also be valid but may point to local memory from - * another process, possibly with the same address and must not - * be dereferenced yet. */ - assert(priv->dev != NULL); - /* Determine port ID by finding out where priv comes from. */ - while (1) { - sd = &mlx5_secondary_data[port_id]; - rte_spinlock_lock(&sd->lock); - /* Primary process? */ - if (sd->primary_priv == priv) - break; - /* Secondary process? */ - if (sd->data.dev_private == priv) - break; - rte_spinlock_unlock(&sd->lock); - if (++port_id == RTE_DIM(mlx5_secondary_data)) - port_id = 0; - } - /* Switch to secondary private structure. If private data has already - * been updated by another thread, there is nothing else to do. */ - priv = sd->data.dev_private; - if (priv->dev->data == &sd->data) - goto end; - /* Sanity checks. Secondary private structure is supposed to point - * to local eth_dev, itself still pointing to the shared device data - * structure allocated by the primary process. */ - assert(sd->shared_dev_data != &sd->data); - assert(sd->data.nb_tx_queues == 0); - assert(sd->data.tx_queues == NULL); - assert(sd->data.nb_rx_queues == 0); - assert(sd->data.rx_queues == NULL); - assert(priv != sd->primary_priv); - assert(priv->dev->data == sd->shared_dev_data); - assert(priv->txqs_n == 0); - assert(priv->txqs == NULL); - assert(priv->rxqs_n == 0); - assert(priv->rxqs == NULL); - nb_tx_queues = sd->shared_dev_data->nb_tx_queues; - nb_rx_queues = sd->shared_dev_data->nb_rx_queues; - /* Allocate local storage for queues. */ - tx_queues = rte_zmalloc("secondary ethdev->tx_queues", - sizeof(sd->data.tx_queues[0]) * nb_tx_queues, - RTE_CACHE_LINE_SIZE); - rx_queues = rte_zmalloc("secondary ethdev->rx_queues", - sizeof(sd->data.rx_queues[0]) * nb_rx_queues, - RTE_CACHE_LINE_SIZE); - if (tx_queues == NULL || rx_queues == NULL) - goto error; - /* Lock to prevent control operations during setup. */ - priv_lock(priv); - /* TX queues. */ - for (i = 0; i != nb_tx_queues; ++i) { - struct txq *primary_txq = (*sd->primary_priv->txqs)[i]; - struct txq_ctrl *primary_txq_ctrl; - struct txq_ctrl *txq_ctrl; - - if (primary_txq == NULL) - continue; - primary_txq_ctrl = container_of(primary_txq, - struct txq_ctrl, txq); - txq_ctrl = rte_calloc_socket("TXQ", 1, sizeof(*txq_ctrl) + - (1 << primary_txq->elts_n) * - sizeof(struct rte_mbuf *), 0, - primary_txq_ctrl->socket); - if (txq_ctrl != NULL) { - if (txq_ctrl_setup(priv->dev, - txq_ctrl, - 1 << primary_txq->elts_n, - primary_txq_ctrl->socket, - NULL) == 0) { - txq_ctrl->txq.stats.idx = - primary_txq->stats.idx; - tx_queues[i] = &txq_ctrl->txq; - continue; - } - rte_free(txq_ctrl); - } - while (i) { - txq_ctrl = tx_queues[--i]; - txq_cleanup(txq_ctrl); - rte_free(txq_ctrl); - } - goto error; - } - /* RX queues. */ - for (i = 0; i != nb_rx_queues; ++i) { - struct rxq_ctrl *primary_rxq = - container_of((*sd->primary_priv->rxqs)[i], - struct rxq_ctrl, rxq); - - if (primary_rxq == NULL) - continue; - /* Not supported yet. */ - rx_queues[i] = NULL; - } - /* Update everything. */ - priv->txqs = (void *)tx_queues; - priv->txqs_n = nb_tx_queues; - priv->rxqs = (void *)rx_queues; - priv->rxqs_n = nb_rx_queues; - sd->data.rx_queues = rx_queues; - sd->data.tx_queues = tx_queues; - sd->data.nb_rx_queues = nb_rx_queues; - sd->data.nb_tx_queues = nb_tx_queues; - sd->data.dev_link = sd->shared_dev_data->dev_link; - sd->data.mtu = sd->shared_dev_data->mtu; - memcpy(sd->data.rx_queue_state, sd->shared_dev_data->rx_queue_state, - sizeof(sd->data.rx_queue_state)); - memcpy(sd->data.tx_queue_state, sd->shared_dev_data->tx_queue_state, - sizeof(sd->data.tx_queue_state)); - sd->data.dev_flags = sd->shared_dev_data->dev_flags; - /* Use local data from now on. */ - rte_mb(); - priv->dev->data = &sd->data; - rte_mb(); - priv_select_tx_function(priv); - priv_select_rx_function(priv); - priv_unlock(priv); -end: - /* More sanity checks. */ - assert(priv->dev->data == &sd->data); - rte_spinlock_unlock(&sd->lock); - return priv; -error: - priv_unlock(priv); - rte_free(tx_queues); - rte_free(rx_queues); - rte_spinlock_unlock(&sd->lock); - return NULL; -} - -/** * Configure the TX function to use. * * @param priv diff --git a/drivers/net/mlx5/mlx5_rxq.c b/drivers/net/mlx5/mlx5_rxq.c index 2ea7a35..b52de98 100644 --- a/drivers/net/mlx5/mlx5_rxq.c +++ b/drivers/net/mlx5/mlx5_rxq.c @@ -1223,47 +1223,6 @@ mlx5_rx_queue_release(void *dpdk_rxq) } /** - * DPDK callback for RX in secondary processes. - * - * This function configures all queues from primary process information - * if necessary before reverting to the normal RX burst callback. - * - * @param dpdk_rxq - * Generic pointer to RX queue structure. - * @param[out] pkts - * Array to store received packets. - * @param pkts_n - * Maximum number of packets in array. - * - * @return - * Number of packets successfully received (<= pkts_n). - */ -uint16_t -mlx5_rx_burst_secondary_setup(void *dpdk_rxq, struct rte_mbuf **pkts, - uint16_t pkts_n) -{ - struct rxq *rxq = dpdk_rxq; - struct rxq_ctrl *rxq_ctrl = container_of(rxq, struct rxq_ctrl, rxq); - struct priv *priv = mlx5_secondary_data_setup(rxq_ctrl->priv); - struct priv *primary_priv; - unsigned int index; - - if (priv == NULL) - return 0; - primary_priv = - mlx5_secondary_data[priv->dev->data->port_id].primary_priv; - /* Look for queue index in both private structures. */ - for (index = 0; index != priv->rxqs_n; ++index) - if (((*primary_priv->rxqs)[index] == rxq) || - ((*priv->rxqs)[index] == rxq)) - break; - if (index == priv->rxqs_n) - return 0; - rxq = (*priv->rxqs)[index]; - return priv->dev->rx_pkt_burst(rxq, pkts, pkts_n); -} - -/** * Allocate queue vector and fill epoll fd list for Rx interrupts. * * @param priv diff --git a/drivers/net/mlx5/mlx5_rxtx.h b/drivers/net/mlx5/mlx5_rxtx.h index baf2b25..333e5af 100644 --- a/drivers/net/mlx5/mlx5_rxtx.h +++ b/drivers/net/mlx5/mlx5_rxtx.h @@ -305,7 +305,6 @@ int rxq_ctrl_setup(struct rte_eth_dev *, struct rxq_ctrl *, uint16_t, int mlx5_rx_queue_setup(struct rte_eth_dev *, uint16_t, uint16_t, unsigned int, const struct rte_eth_rxconf *, struct rte_mempool *); void mlx5_rx_queue_release(void *); -uint16_t mlx5_rx_burst_secondary_setup(void *, struct rte_mbuf **, uint16_t); int priv_rx_intr_vec_enable(struct priv *priv); void priv_rx_intr_vec_disable(struct priv *priv); #ifdef HAVE_UPDATE_CQ_CI @@ -321,7 +320,6 @@ int txq_ctrl_setup(struct rte_eth_dev *, struct txq_ctrl *, uint16_t, int mlx5_tx_queue_setup(struct rte_eth_dev *, uint16_t, uint16_t, unsigned int, const struct rte_eth_txconf *); void mlx5_tx_queue_release(void *); -uint16_t mlx5_tx_burst_secondary_setup(void *, struct rte_mbuf **, uint16_t); /* mlx5_rxtx.c */ diff --git a/drivers/net/mlx5/mlx5_txq.c b/drivers/net/mlx5/mlx5_txq.c index 45e9037..4b0b532 100644 --- a/drivers/net/mlx5/mlx5_txq.c +++ b/drivers/net/mlx5/mlx5_txq.c @@ -526,44 +526,3 @@ mlx5_tx_queue_release(void *dpdk_txq) rte_free(txq_ctrl); priv_unlock(priv); } - -/** - * DPDK callback for TX in secondary processes. - * - * This function configures all queues from primary process information - * if necessary before reverting to the normal TX burst callback. - * - * @param dpdk_txq - * Generic pointer to TX queue structure. - * @param[in] pkts - * Packets to transmit. - * @param pkts_n - * Number of packets in array. - * - * @return - * Number of packets successfully transmitted (<= pkts_n). - */ -uint16_t -mlx5_tx_burst_secondary_setup(void *dpdk_txq, struct rte_mbuf **pkts, - uint16_t pkts_n) -{ - struct txq *txq = dpdk_txq; - struct txq_ctrl *txq_ctrl = container_of(txq, struct txq_ctrl, txq); - struct priv *priv = mlx5_secondary_data_setup(txq_ctrl->priv); - struct priv *primary_priv; - unsigned int index; - - if (priv == NULL) - return 0; - primary_priv = - mlx5_secondary_data[priv->dev->data->port_id].primary_priv; - /* Look for queue index in both private structures. */ - for (index = 0; index != priv->txqs_n; ++index) - if (((*primary_priv->txqs)[index] == txq) || - ((*priv->txqs)[index] == txq)) - break; - if (index == priv->txqs_n) - return 0; - txq = (*priv->txqs)[index]; - return priv->dev->tx_pkt_burst(txq, pkts, pkts_n); -}