From 6096a4603d6f6ed752e3b4147406f3720318b0e5 Mon Sep 17 00:00:00 2001 From: =?utf8?q?N=C3=A9lio=20Laranjeiro?= Date: Wed, 23 Aug 2017 10:15:10 +0200 Subject: [PATCH] net/mlx5: fix non working secondary process by removing it 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 --- doc/guides/nics/features/mlx5.ini | 1 - 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 -------- 7 files changed, 4 insertions(+), 289 deletions(-) diff --git a/doc/guides/nics/features/mlx5.ini b/doc/guides/nics/features/mlx5.ini index 2913591c9d..99a8d932a0 100644 --- a/doc/guides/nics/features/mlx5.ini +++ b/doc/guides/nics/features/mlx5.ini @@ -34,7 +34,6 @@ Tx descriptor status = Y Basic stats = Y Extended stats = Y Stats per queue = Y -Multiprocess aware = Y Other kdrv = Y ARMv8 = Y Power8 = Y diff --git a/drivers/net/mlx5/mlx5.c b/drivers/net/mlx5/mlx5.c index bfcff70130..bd66a7c77a 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 31d0c49e1f..2392be5894 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 8258d4a4c5..57f6237036 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; } /** @@ -1335,163 +1330,6 @@ mlx5_set_link_up(struct rte_eth_dev *dev) return err; } -/** - * 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. * diff --git a/drivers/net/mlx5/mlx5_rxq.c b/drivers/net/mlx5/mlx5_rxq.c index 94fdd32bdf..35c5cb42e9 100644 --- a/drivers/net/mlx5/mlx5_rxq.c +++ b/drivers/net/mlx5/mlx5_rxq.c @@ -1222,47 +1222,6 @@ mlx5_rx_queue_release(void *dpdk_rxq) priv_unlock(priv); } -/** - * 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. * diff --git a/drivers/net/mlx5/mlx5_rxtx.h b/drivers/net/mlx5/mlx5_rxtx.h index 0226ff91a4..b3b161da58 100644 --- a/drivers/net/mlx5/mlx5_rxtx.h +++ b/drivers/net/mlx5/mlx5_rxtx.h @@ -302,7 +302,6 @@ void rxq_cleanup(struct rxq_ctrl *); 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 @@ -318,7 +317,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 45e9037b71..4b0b532b1f 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); -} -- 2.20.1