DPDK patches and discussions
 help / color / mirror / Atom feed
From: Adrien Mazarguil <adrien.mazarguil@6wind.com>
To: "Xueming(Steven) Li" <xuemingl@mellanox.com>
Cc: Shahaf Shuler <shahafs@mellanox.com>, "dev@dpdk.org" <dev@dpdk.org>
Subject: Re: [dpdk-dev] [PATCH v2 6/7] net/mlx5: probe all port representors
Date: Wed, 27 Jun 2018 15:32:17 +0200	[thread overview]
Message-ID: <20180627133217.GU4025@6wind.com> (raw)
In-Reply-To: <AM5PR0501MB2420DF73EF1FD771CAB09F65AC730@AM5PR0501MB2420.eurprd05.prod.outlook.com>

On Sat, Jun 16, 2018 at 08:57:51AM +0000, Xueming(Steven) Li wrote:
> Reviewed-by: Xueming Li <xuemingl@mellanox.com>
> 
> Minor comments inside:
> 
> > -----Original Message-----
> > From: dev <dev-bounces@dpdk.org> On Behalf Of Adrien Mazarguil
> > Sent: Thursday, June 14, 2018 4:35 PM
> > To: Shahaf Shuler <shahafs@mellanox.com>
> > 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 <adrien.mazarguil@6wind.com>
> > --
> > 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
<snip>
> > --- 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

  parent reply	other threads:[~2018-06-27 13:32 UTC|newest]

Thread overview: 100+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-05-25 16:35 [dpdk-dev] [PATCH 0/7] net/mlx5: add port representor support Adrien Mazarguil
2018-05-25 16:35 ` [dpdk-dev] [PATCH 1/7] net/mlx5: rename confusing object in probe code Adrien Mazarguil
2018-06-10 11:00   ` Xueming(Steven) Li
2018-06-12 13:19     ` Adrien Mazarguil
2018-05-25 16:35 ` [dpdk-dev] [PATCH 2/7] net/mlx5: remove redundant objects " Adrien Mazarguil
2018-06-10 11:00   ` Xueming(Steven) Li
2018-06-12 13:19     ` Adrien Mazarguil
2018-05-25 16:35 ` [dpdk-dev] [PATCH 3/7] net/mlx5: split PCI from generic probing code Adrien Mazarguil
2018-06-10 12:59   ` Xueming(Steven) Li
2018-06-12 13:20     ` Adrien Mazarguil
2018-05-25 16:35 ` [dpdk-dev] [PATCH 4/7] net/mlx5: re-indent generic probing function Adrien Mazarguil
2018-06-11 11:42   ` Xueming(Steven) Li
2018-05-25 16:35 ` [dpdk-dev] [PATCH 5/7] net/mlx5: add port representor awareness Adrien Mazarguil
2018-05-25 16:35 ` [dpdk-dev] [PATCH 6/7] net/mlx5: probe all port representors Adrien Mazarguil
2018-06-12  6:42   ` Xueming(Steven) Li
2018-06-12 13:20     ` Adrien Mazarguil
2018-05-25 16:35 ` [dpdk-dev] [PATCH 7/7] net/mlx5: add parameter for " Adrien Mazarguil
2018-06-12  8:02   ` Xueming(Steven) Li
2018-06-12 13:20     ` Adrien Mazarguil
2018-06-12 13:43       ` Xueming(Steven) Li
2018-06-14  8:01         ` Adrien Mazarguil
2018-06-12 14:44   ` Xueming(Steven) Li
2018-06-13 13:11     ` Adrien Mazarguil
2018-06-14  8:34 ` [dpdk-dev] [PATCH v2 0/7] net/mlx5: add port representor support Adrien Mazarguil
2018-06-14  8:34   ` [dpdk-dev] [PATCH v2 1/7] net/mlx5: rename confusing object in probe code Adrien Mazarguil
2018-06-16  8:24     ` Xueming(Steven) Li
2018-06-14  8:34   ` [dpdk-dev] [PATCH v2 2/7] net/mlx5: remove redundant objects " Adrien Mazarguil
2018-06-16  8:27     ` Xueming(Steven) Li
2018-06-17 10:14     ` Shahaf Shuler
2018-06-27 13:30       ` Adrien Mazarguil
2018-06-28  5:35         ` Shahaf Shuler
2018-06-14  8:34   ` [dpdk-dev] [PATCH v2 3/7] net/mlx5: split PCI from generic probing code Adrien Mazarguil
2018-06-16  8:29     ` Xueming(Steven) Li
2018-06-17 10:14     ` Shahaf Shuler
2018-06-27 13:31       ` Adrien Mazarguil
2018-06-14  8:34   ` [dpdk-dev] [PATCH v2 4/7] net/mlx5: re-indent generic probing function Adrien Mazarguil
2018-06-14  8:34   ` [dpdk-dev] [PATCH v2 5/7] net/mlx5: add port representor awareness Adrien Mazarguil
2018-06-16  8:37     ` Xueming(Steven) Li
2018-06-27 13:32       ` Adrien Mazarguil
2018-06-14  8:35   ` [dpdk-dev] [PATCH v2 6/7] net/mlx5: probe all port representors Adrien Mazarguil
2018-06-16  8:57     ` Xueming(Steven) Li
2018-06-17 10:15       ` Shahaf Shuler
2018-06-24 13:33         ` Shahaf Shuler
2018-06-27 13:32           ` Adrien Mazarguil
2018-06-28  5:57             ` Shahaf Shuler
2018-06-28  9:13               ` Adrien Mazarguil
2018-06-27 13:32         ` Adrien Mazarguil
2018-06-27 17:30           ` Xueming(Steven) Li
2018-06-28  6:01           ` Shahaf Shuler
2018-06-28  8:45             ` Adrien Mazarguil
2018-06-28  9:06               ` Shahaf Shuler
2018-06-27 13:32       ` Adrien Mazarguil [this message]
2018-06-14  8:35   ` [dpdk-dev] [PATCH v2 7/7] net/mlx5: add parameter for " Adrien Mazarguil
2018-06-16  8:59     ` Xueming(Steven) Li
2018-07-04 17:27   ` [dpdk-dev] [PATCH v3 00/10] net/mlx5: add port representor support Adrien Mazarguil
2018-07-04 17:27     ` [dpdk-dev] [PATCH v3 01/10] net/mlx5: rename confusing object in probe code Adrien Mazarguil
2018-07-04 17:27     ` [dpdk-dev] [PATCH v3 02/10] net/mlx5: remove redundant objects " Adrien Mazarguil
2018-07-04 17:27     ` [dpdk-dev] [PATCH v3 03/10] net/mlx5: drop useless support for several Verbs ports Adrien Mazarguil
2018-07-04 17:27     ` [dpdk-dev] [PATCH v3 04/10] net/mlx5: split PCI from generic probing code Adrien Mazarguil
2018-07-04 17:27     ` [dpdk-dev] [PATCH v3 05/10] net/mlx5: re-indent generic probing function Adrien Mazarguil
2018-07-04 17:27     ` [dpdk-dev] [PATCH v3 06/10] net/mlx5: add port representor awareness Adrien Mazarguil
2018-07-04 17:27     ` [dpdk-dev] [PATCH v3 07/10] net/mlx5: probe all port representors Adrien Mazarguil
2018-07-04 17:27     ` [dpdk-dev] [PATCH v3 08/10] net/mlx5: probe port representors in natural order Adrien Mazarguil
2018-07-04 17:27     ` [dpdk-dev] [PATCH v3 09/10] net/mlx5: add parameter for port representors Adrien Mazarguil
2018-07-04 17:27     ` [dpdk-dev] [PATCH v3 10/10] net/mlx5: support negative identifiers " Adrien Mazarguil
2018-07-05  8:45     ` [dpdk-dev] [PATCH v4 00/10] net/mlx5: add port representor support Adrien Mazarguil
2018-07-05  8:45       ` [dpdk-dev] [PATCH v4 01/10] net/mlx5: rename confusing object in probe code Adrien Mazarguil
2018-07-05  8:45       ` [dpdk-dev] [PATCH v4 02/10] net/mlx5: remove redundant objects " Adrien Mazarguil
2018-07-05  8:45       ` [dpdk-dev] [PATCH v4 03/10] net/mlx5: drop useless support for several Verbs ports Adrien Mazarguil
2018-07-05  8:45       ` [dpdk-dev] [PATCH v4 04/10] net/mlx5: split PCI from generic probing code Adrien Mazarguil
2018-07-05  8:45       ` [dpdk-dev] [PATCH v4 05/10] net/mlx5: re-indent generic probing function Adrien Mazarguil
2018-07-05  8:45       ` [dpdk-dev] [PATCH v4 06/10] net/mlx5: add port representor awareness Adrien Mazarguil
2018-07-05  8:45       ` [dpdk-dev] [PATCH v4 07/10] net/mlx5: probe all port representors Adrien Mazarguil
2018-07-09 11:57         ` Shahaf Shuler
2018-07-10  9:37           ` Adrien Mazarguil
2018-07-10 10:13             ` Shahaf Shuler
2018-07-10 10:58               ` Adrien Mazarguil
2018-07-10 11:17                 ` Shahaf Shuler
2018-07-05  8:45       ` [dpdk-dev] [PATCH v4 08/10] net/mlx5: probe port representors in natural order Adrien Mazarguil
2018-07-05  8:45       ` [dpdk-dev] [PATCH v4 09/10] net/mlx5: add parameter for port representors Adrien Mazarguil
2018-07-09 11:57         ` Shahaf Shuler
2018-07-10  9:37           ` Adrien Mazarguil
2018-07-10 10:16             ` Shahaf Shuler
2018-07-10 10:58               ` Adrien Mazarguil
2018-07-10 11:15                 ` Shahaf Shuler
2018-07-05  8:45       ` [dpdk-dev] [PATCH v4 10/10] net/mlx5: support negative identifiers " Adrien Mazarguil
2018-07-09 11:58         ` Shahaf Shuler
2018-07-10  9:37           ` Adrien Mazarguil
2018-07-10 16:04       ` [dpdk-dev] [PATCH v5 00/10] net/mlx5: add port representor support Adrien Mazarguil
2018-07-10 16:04         ` [dpdk-dev] [PATCH v5 01/10] net/mlx5: rename confusing object in probe code Adrien Mazarguil
2018-07-10 16:04         ` [dpdk-dev] [PATCH v5 02/10] net/mlx5: remove redundant objects " Adrien Mazarguil
2018-07-10 16:04         ` [dpdk-dev] [PATCH v5 03/10] net/mlx5: drop useless support for several Verbs ports Adrien Mazarguil
2018-07-10 16:04         ` [dpdk-dev] [PATCH v5 04/10] net/mlx5: split PCI from generic probing code Adrien Mazarguil
2018-07-10 16:04         ` [dpdk-dev] [PATCH v5 05/10] net/mlx5: re-indent generic probing function Adrien Mazarguil
2018-07-10 16:04         ` [dpdk-dev] [PATCH v5 06/10] net/mlx5: add port representor awareness Adrien Mazarguil
2018-07-10 16:04         ` [dpdk-dev] [PATCH v5 07/10] net/mlx5: probe all port representors Adrien Mazarguil
2018-07-10 16:04         ` [dpdk-dev] [PATCH v5 08/10] net/mlx5: probe port representors in natural order Adrien Mazarguil
2018-07-10 16:04         ` [dpdk-dev] [PATCH v5 09/10] net/mlx5: add parameter for port representors Adrien Mazarguil
2018-07-10 16:05         ` [dpdk-dev] [PATCH v5 10/10] net/mlx5: support negative identifiers " Adrien Mazarguil
2018-07-12  7:51         ` [dpdk-dev] [PATCH v5 00/10] net/mlx5: add port representor support Shahaf Shuler

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20180627133217.GU4025@6wind.com \
    --to=adrien.mazarguil@6wind.com \
    --cc=dev@dpdk.org \
    --cc=shahafs@mellanox.com \
    --cc=xuemingl@mellanox.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).