On Tue, Oct 27, 2020 at 11:23:21PM +0000, Ophir Munk wrote:
> This commit implements mlx5_dev_spawn() API which allocates an eth
> device (struct rte_eth_dev) for each PCI device. When working with
> representors virtual functions (as in Linux), one PCI device may spawn
> several eth devices: the master device for the main physical function
> (PF) and several representors for the virtual functions (VFs). However,
> currently Windows does not work in switch dev mode, therefore, no VFs
> are created and no representors are spawned. In this case one eth device
> is created per one PCI main port. In addition to device creation - the
> device configuration must be correctly set. The device arguments
> (devargs - set by the user) are parsed but they may be overridden by
> Windows limitations or hardware configurations. Some associated network
> parameters are stored in eth device (e.g. ifindex, MAC address, MTU) and
> some callback (e.g. burst functions) are set.
>
> Signed-off-by: Ophir Munk <ophi...@nvidia.com>
> ---
> drivers/common/mlx5/windows/mlx5_win_defs.h | 6 +
> drivers/net/mlx5/windows/mlx5_os.c | 481
> +++++++++++++++++++++++++++-
> 2 files changed, 482 insertions(+), 5 deletions(-)
>
>
> +static int
> +mlx5_alloc_shared_dr(struct mlx5_priv *priv)
> +{
> + struct mlx5_dev_ctx_shared *sh = priv->sh;
> + char s[MLX5_HLIST_NAMESIZE];
This array is not used for now, you can remove it.
> + int err = 0;
> +
> + if (!sh->flow_tbls)
> + err = mlx5_alloc_table_hash_list(priv);
> + else
> + DRV_LOG(DEBUG, "sh->flow_tbls[%p] already created, reuse\n",
> + (void *)sh->flow_tbls);
> + return err;
> +}
> +
> @@ -231,17 +310,409 @@ mlx5_os_open_device(const struct mlx5_dev_spawn_data
> *spawn,
> * Device configuration parameters.
> *
> * @return
> - * NULL pointer. Operation is not supported and rte_errno is set to
> ENOTSUP.
> + * A valid Ethernet device object on success, NULL otherwise and rte_errno
> + * is set. The following errors are defined:
> + *
> + * EEXIST: device is already spawned
> */
> static struct rte_eth_dev *
> mlx5_dev_spawn(struct rte_device *dpdk_dev,
> struct mlx5_dev_spawn_data *spawn,
> struct mlx5_dev_config *config)
> {
> + mlx5_malloc_mem_select(config->sys_mem_en);
> + sh = mlx5_alloc_shared_dev_ctx(spawn, config);
> + if (!sh)
> + return NULL;
Should we set err to rte_errno here? Also maybe "goto error" instead of "return
NULL" for consistency.
> + config->devx = sh->devx;
> + /* Initialize the shutdown event in mlx5_dev_spawn to
> + * support mlx5_is_removed for Windows.
> + */
> + * Allocate the buffer for flow creating, just once.
> + * The allocation must be done before any flow creating.
> + */
> + mlx5_flow_alloc_intermediate(eth_dev);
Where is this function defined? Should it be added with the flow support patch?
> + /* Query availability of metadata reg_c's. */
> + err = mlx5_flow_discover_mreg_c(eth_dev);
> + if (err < 0) {
> + err = -err;
> + goto error;
> + }
> + return eth_dev;
> +error:
> + if (priv) {
Should we also call mlx5_os_free_shared_dr()? If the call to
mlx5_alloc_shared_dr() above succeeds.
> + if (priv->mreg_cp_tbl)
> + mlx5_hlist_destroy(priv->mreg_cp_tbl, NULL, NULL);
> + if (priv->qrss_id_pool)
> + mlx5_flow_id_pool_release(priv->qrss_id_pool);
> + if (own_domain_id)
> + claim_zero(rte_eth_switch_domain_free(priv->domain_id));
> + mlx5_free(priv);
> + if (eth_dev != NULL)
> + eth_dev->data->dev_private = NULL;
> + }
> + if (eth_dev != NULL) {
> + /* mac_addrs must not be freed alone because part of
> + * dev_private
> + **/
> + eth_dev->data->mac_addrs = NULL;
Looks like rte_eth_dev_release_port() frees mac_addrs as well. Why is it set to
NULL here?
Should this line be removed or moved after the _release_port() call?
> + rte_eth_dev_release_port(eth_dev);
> + }
> + if (sh)
> + mlx5_free_shared_dev_ctx(sh);
> + MLX5_ASSERT(err > 0);
> + rte_errno = err;
> return NULL;
> }
>
> --
> 2.8.4
