On Thu, Aug 17, 2017 at 03:38:22PM +0100, Ferruh Yigit wrote:
> On 8/1/2017 1:09 PM, Nelio Laranjeiro wrote:
> > 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().
>
> Does this means multi process support for mlx5 broken with this patch?
> If so features file and release notes should be updated if this won't be
> fixed back in this release..
No currently the secondary process in mlx5 is not working at all. This is
only removing a non working code from the PMD.
> And what is special required for mlx5 secondary process support?
There is some work to make this secondary process work correctly, but as it
will be totally different, it does not make sense to keep a non working code
to replace it by a working one.
Concerning the release note and features, this new implementation is
expected in 17.11 same release as this patch.
> > 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 <nelio.laranjeiro@6wind.com>
> > Acked-by: Shahaf Shuler <shahafs@mellanox.com>
>
> <...>
I'll update the feature documentation in a v2 to remove the "Multiprocess
aware" from the feature list in this patch.
It will be added back with the new implementation.
Thanks,
@@ -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;
@@ -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 *);
@@ -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
@@ -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
@@ -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 */
@@ -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);
-}