From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-wr0-f196.google.com (mail-wr0-f196.google.com [209.85.128.196]) by dpdk.org (Postfix) with ESMTP id D32F21BDF0 for ; Wed, 27 Jun 2018 15:32:34 +0200 (CEST) Received: by mail-wr0-f196.google.com with SMTP id c5-v6so2060511wrs.10 for ; Wed, 27 Jun 2018 06:32:34 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=6wind-com.20150623.gappssmtp.com; s=20150623; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to; bh=fyx+CTrMz8A8lfZj9tjEG2PpapnjH1KvUH8/fyP6oP0=; b=kfanju0oC9HyDajx3ejipQmZuqQaFgTC65MI1CcCGhAgyHF4m2jsAhpRspb9kN8zZk xi9HnBjyVK0VWIs/n4SbLKwVn1AzrnFSZJwd4keQpjAMnbptGMrayKr+abOatxZwb76r 3JmdSRWYFzHRkjkVyPi4GwSiVJPCUvkIfxc3qWzdA5n9mFWIObMy+GjHtXt76aPW9Mla 2Hd1b365cpTs7Vd58Yp3Ulp9B6dCESb72bnSqIxTYPiPsqsw9I5Qp6HvizbSTwAXIML+ HNdhdTQxr7davrTeS9idOC598lUYk51pnEXnQpINjeyrBvQFSt/y0uyIP2n8lgciaRDm UTBw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to; bh=fyx+CTrMz8A8lfZj9tjEG2PpapnjH1KvUH8/fyP6oP0=; b=S5gI0eMQMKOgjouMSQiyzD8of72os5guywTLKPacis00M/1E6XGL5Tz3KF9ZYOgxPf DKTylakMmMNStd3wPpOGVMCN2J0h03Ql8cC/F6cfI+r0sTAJ//3mQzFlbSF8V3OdutbS BmF7RD4pN76LHN2PokSkrsQQ62fwxTYzo32HV/PsObgUDLZQn2JnylLDOcgrfV1vSlJ0 t+NjoRdq4fitRo0jlLYGXk/c79eHmVfPaVQuCrcMBKyd8upjtGaYEVOnceQmNFiCjBki 9byaYGQiurj1vxp92P/tz65LYWpuznNdiGS9eX1+KzCim//WESqRoUTs5B2vs6xDedXr 911A== X-Gm-Message-State: APt69E3o8f9RSZ3gMeFBr0Xii5WV52vCwk/SXxUDV9yLrcaZKKN/X0y3 mAE+VF//7pUccr4Q1xOl90Rb0TLw X-Google-Smtp-Source: AAOMgpcTe6YC4jkjJNDN0e0i7LZr0nS9xlFGMuMNB5C8S4UJTE5WarcJzMhu0SO43jEJ+KmXHDrDuQ== X-Received: by 2002:adf:b726:: with SMTP id l38-v6mr228395wre.115.1530106354625; Wed, 27 Jun 2018 06:32:34 -0700 (PDT) Received: from 6wind.com (host.78.145.23.62.rev.coltfrance.com. [62.23.145.78]) by smtp.gmail.com with ESMTPSA id a8-v6sm4980787wrn.50.2018.06.27.06.32.33 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Wed, 27 Jun 2018 06:32:33 -0700 (PDT) Date: Wed, 27 Jun 2018 15:32:17 +0200 From: Adrien Mazarguil To: "Xueming(Steven) Li" Cc: Shahaf Shuler , "dev@dpdk.org" Message-ID: <20180627133217.GU4025@6wind.com> References: <20180525161814.13873-1-adrien.mazarguil@6wind.com> <20180614083047.10812-1-adrien.mazarguil@6wind.com> <20180614083047.10812-7-adrien.mazarguil@6wind.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: Subject: Re: [dpdk-dev] [PATCH v2 6/7] net/mlx5: probe all port representors 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: , X-List-Received-Date: Wed, 27 Jun 2018 13:32:35 -0000 On Sat, Jun 16, 2018 at 08:57:51AM +0000, Xueming(Steven) Li wrote: > Reviewed-by: Xueming Li > > Minor comments inside: > > > -----Original Message----- > > From: dev On Behalf Of Adrien Mazarguil > > Sent: Thursday, June 14, 2018 4:35 PM > > To: Shahaf Shuler > > Cc: dev@dpdk.org > > Subject: [dpdk-dev] [PATCH v2 6/7] net/mlx5: probe all port representors > > > > Probe existing port representors in addition to their master device and associate them automatically. > > > > To avoid name collision between Ethernet devices, their names use the same convention as ixgbe and > > i40e PMDs, that is, instead of only a PCI address in DBDF notation: > > > > - "net_{DBDF}_0" for master/switch devices. > > - "net_{DBDF}_representor_{rep}" with "rep" starting from 0 for port > > representors. > > > > Both optionally suffixed with "_port_{num}" instead of " port {num}" for devices that expose several > > Verbs ports (note this is never the case on mlx5, but kept for historical reasons for the time being). > > > > (Patch based on prior work from Yuanhan Liu) > > > > Signed-off-by: Adrien Mazarguil > > -- > > v2 changes: > > > > - Added representor information to dev_infos_get(). DPDK port ID of master > > device is now stored in the private structure to retrieve it > > conveniently. > > - Master device is assigned dummy representor ID value -1 to better > > distinguish from the the first actual representor reported by > > dev_infos_get() as those are indexed from 0. > > - Added RTE_ETH_DEV_REPRESENTOR device flag. > > --- > > drivers/net/mlx5/mlx5.c | 138 ++++++++++++++++++++++++-------- > > drivers/net/mlx5/mlx5.h | 9 ++- > > drivers/net/mlx5/mlx5_ethdev.c | 151 ++++++++++++++++++++++++++++++++---- > > drivers/net/mlx5/mlx5_mac.c | 2 +- > > drivers/net/mlx5/mlx5_stats.c | 6 +- > > 5 files changed, 252 insertions(+), 54 deletions(-) > > > > diff --git a/drivers/net/mlx5/mlx5.c b/drivers/net/mlx5/mlx5.c index 498f80c89..716c9d9a5 100644 > > --- a/drivers/net/mlx5/mlx5.c > > +++ b/drivers/net/mlx5/mlx5.c > > @@ -304,6 +304,9 @@ mlx5_dev_close(struct rte_eth_dev *dev) > > if (ret) > > DRV_LOG(WARNING, "port %u some flows still remain", > > dev->data->port_id); > > + if (!priv->representor && > > + priv->domain_id != RTE_ETH_DEV_SWITCH_DOMAIN_ID_INVALID) > > + claim_zero(rte_eth_switch_domain_free(priv->domain_id)); > > memset(priv, 0, sizeof(*priv)); > > } > > > > @@ -648,6 +651,10 @@ mlx5_uar_init_secondary(struct rte_eth_dev *dev) > > * Verbs device attributes. > > * @param port > > * Verbs port to use (indexed from 1). > > + * @param master > > + * Master device in case @p ibv_dev is a port representor. > > + * @param rep_id > > + * Representor identifier when @p master is non-NULL. > > * > > * @return > > * A valid Ethernet device object on success, NULL otherwise and rte_errno > > @@ -658,7 +665,9 @@ mlx5_dev_spawn_one(struct rte_device *dpdk_dev, > > struct ibv_device *ibv_dev, > > int vf, > > const struct ibv_device_attr_ex *attr, > > - unsigned int port) > > + unsigned int port, > > + struct rte_eth_dev *master, > > + unsigned int rep_id) > > { > > struct ibv_context *ctx; > > struct ibv_port_attr port_attr; > > @@ -802,11 +811,14 @@ mlx5_dev_spawn_one(struct rte_device *dpdk_dev, > > " old OFED/rdma-core version or firmware configuration"); #endif > > config.mpls_en = mpls_en; > > - if (attr->orig_attr.phys_port_cnt > 1) > > - snprintf(name, sizeof(name), "%s port %u", > > - dpdk_dev->name, port); > > + if (!master) > > + snprintf(name, sizeof(name), "net_%s_0", dpdk_dev->name); > > else > > - snprintf(name, sizeof(name), "%s", dpdk_dev->name); > > + snprintf(name, sizeof(name), "net_%s_representor_%u", > > + dpdk_dev->name, rep_id); > > + if (attr->orig_attr.phys_port_cnt > 1) > > + snprintf(name, sizeof(name), "%s_port_%u", name, port); > > + DRV_LOG(DEBUG, "naming Ethernet device \"%s\"", name); > > if (rte_eal_process_type() == RTE_PROC_SECONDARY) { > > eth_dev = rte_eth_dev_attach_secondary(name); > > if (eth_dev == NULL) { > > @@ -883,6 +895,30 @@ mlx5_dev_spawn_one(struct rte_device *dpdk_dev, > > priv->port = port; > > priv->pd = pd; > > priv->mtu = ETHER_MTU; > > + /* > > + * Allocate a switch domain for master devices and share it with > > + * port representors. > > + */ > > + if (!master) { > > + priv->representor = 0; > > + priv->master_id = -1; /* Updated once known. */ > > + priv->domain_id = RTE_ETH_DEV_SWITCH_DOMAIN_ID_INVALID; > > Domain_id will override below. It's done as a safety measure. If rte_eth_switch_domain_alloc() happened to fail, rte_eth_switch_domain_free() would otherwise be attempted on an uninitialized value when cleaning up priv, possibly destroying an unrelated domain. This is prevented thanks to RTE_ETH_DEV_SWITCH_DOMAIN_ID_INVALID. > > + priv->rep_id = -1; /* Dummy unique value. */ > > + err = rte_eth_switch_domain_alloc(&priv->domain_id); > > + if (err) { > > + err = rte_errno; > > + DRV_LOG(ERR, "unable to allocate switch domain: %s", > > + strerror(rte_errno)); > > + goto error; > > + } > > + } else { > > + priv->representor = 1; > > + priv->master_id = > > + ((struct priv *)master->data->dev_private)->master_id; > > + priv->domain_id = > > + ((struct priv *)master->data->dev_private)->domain_id; > > + priv->rep_id = rep_id; > > + } > > Do you think such information should be set as well in secondary process? Unless I'm mistaken, it's implicitly the case as secondaries do not allocate their own private structure, they inherit it from the primary. Thanks for the review. -- Adrien Mazarguil 6WIND