From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from dpdk.org (dpdk.org [92.243.14.124]) by inbox.dpdk.org (Postfix) with ESMTP id 3D4D5A04E7; Wed, 11 Nov 2020 00:48:26 +0100 (CET) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id 37B07F90; Wed, 11 Nov 2020 00:48:24 +0100 (CET) Received: from linux.microsoft.com (linux.microsoft.com [13.77.154.182]) by dpdk.org (Postfix) with ESMTP id AFE27F64 for ; Wed, 11 Nov 2020 00:48:21 +0100 (CET) Received: by linux.microsoft.com (Postfix, from userid 1059) id EE55220C27F5; Tue, 10 Nov 2020 15:48:19 -0800 (PST) DKIM-Filter: OpenDKIM Filter v2.11.0 linux.microsoft.com EE55220C27F5 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linux.microsoft.com; s=default; t=1605052099; bh=FG3ex4EaP9CYWTtAZlcsfSYlOUuHW95b4WF79bK8OSk=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=jVvNe73f68M6k9Clc1iZ+XplDoYhS/1NhglMELHb1loLGa45Fg5KOzB+NmRql9ZlL Yy0gnam0dXqx0RHz6OZjOmmguaYsot/5XbraY58sJaYaB9mOZTqoMl2H7WqcVkg4wm L22+UcQoBZunolEmlVROAko9C6UVpOON6JDeenBA= Date: Tue, 10 Nov 2020 15:48:19 -0800 From: Narcisa Ana Maria Vasile To: Ophir Munk Cc: dev@dpdk.org, Raslan Darawsheh , Matan Azrad , Tal Shnaiderman , Thomas Monjalon , Omar Cardona Message-ID: <20201110234819.GA9449@linuxonhyperv3.guj3yctzbm1etfxqx2vob5hsef.xx.internal.cloudapp.net> References: <20201027232335.31427-1-ophirmu@nvidia.com> <20201027232335.31427-59-ophirmu@nvidia.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20201027232335.31427-59-ophirmu@nvidia.com> User-Agent: Mutt/1.5.21 (2010-09-15) Subject: Re: [dpdk-dev] [PATCH v1 58/72] net/mlx5/windws: spawn eth devices 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" 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 > --- > 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