DPDK patches and discussions
 help / color / mirror / Atom feed
From: Adrien Mazarguil <adrien.mazarguil@6wind.com>
To: Shahaf Shuler <shahafs@mellanox.com>
Cc: "Xueming(Steven) Li" <xuemingl@mellanox.com>,
	"dev@dpdk.org" <dev@dpdk.org>,
	Guillaume Gaudonville <guillaume.gaudonville@6wind.com>,
	Wisam Monther <wisamm@mellanox.com>,
	Raslan Darawsheh <rasland@mellanox.com>,
	Olga Shern <olgas@mellanox.com>
Subject: Re: [dpdk-dev] [PATCH v2 6/7] net/mlx5: probe all port representors
Date: Thu, 28 Jun 2018 11:13:09 +0200	[thread overview]
Message-ID: <20180628091309.GB4025@6wind.com> (raw)
In-Reply-To: <DB7PR05MB4426CCCA0B5786C25AA96132C34F0@DB7PR05MB4426.eurprd05.prod.outlook.com>

On Thu, Jun 28, 2018 at 05:57:03AM +0000, Shahaf Shuler wrote:
> Wednesday, June 27, 2018 4:33 PM, Adrien Mazarguil:
> > Subject: Re: [dpdk-dev] [PATCH v2 6/7] net/mlx5: probe all port representors
> > 
> > On Sun, Jun 24, 2018 at 01:33:31PM +0000, Shahaf Shuler wrote:
> > > One more input,
> > >
> > > Sunday, June 17, 2018 1:15 PM, Shahaf Shuler:
> > > > Subject: RE: [dpdk-dev] [PATCH v2 6/7] net/mlx5: probe all port
> > > > representors
> > >
> > > [...]
> > >
> > > > > > +	eth_list = tmp;
> > > > > >  	for (i = 0; i < attr.orig_attr.phys_port_cnt; ++i) {
> > > > > > -		eth_list[i] = mlx5_dev_spawn_one(dpdk_dev,
> > ibv_dev, vf,
> > > > > > -						 &attr, i + 1);
> > > > > > -		if (eth_list[i])
> > > > > > -			continue;
> > > > > > -		/* Save rte_errno and roll back in case of failure. */
> > > > > > -		ret = rte_errno;
> > > > > > -		while (i--) {
> > > > > > -			mlx5_dev_close(eth_list[i]);
> > > > > > -			if (rte_eal_process_type() ==
> > RTE_PROC_PRIMARY)
> > > > > > -				rte_free(eth_list[i]->data-
> > >dev_private);
> > > > > > -
> > 	claim_zero(rte_eth_dev_release_port(eth_list[i]));
> > > > > > -		}
> > > > > > -		free(eth_list);
> > > > > > -		rte_errno = ret;
> > > > > > -		return NULL;
> > > > > > +		eth_list[n] = mlx5_dev_spawn_one(dpdk_dev,
> > ibv_dev[j],
> > > > > vf,
> > > > > > +						 &attr, i + 1,
> > > > > > +						 j ? eth_list[0] : NULL,
> > > > > > +						 j - 1);
> > > >
> > > > The representor id is according to the sort made by qsort (based on
> > > > device names).
> > > > A better way may be to set it according to the sysfs information,
> > > > like you do in the mlx5_get_ifname function.
> > > > What do you think?
> > >
> > > In fact relaying on linear increasing port numbers is dangerous. In may
> > break on special scenarios like BlueField.
> > > In BlueField there are representors between the x86 and the ARM cores.
> > Those are not VF representors. The phys_port_name of those is -1 and each
> > of them belongs to different phys_switch_id.
> > >
> > > We can argue whether it is correct/not to assign them w/ -1 value, but I
> > think the suggested approach above can detect the right "vf_id" for those
> > and not break the current behavior on x86.
> > > Let me know if you have other suggestions.
> > 
> > I didn't know that. Assuming that with these, there is exactly only one
> > representor per device, I think we can manage, the main issue being that "-
> > 1" will be difficult to parse as a valid "representor" argument which uses "-"
> > for ranges.
> 
> The -1 value is not for the representor id, It is for the id of the entity which exists on the other size of the representor. 
> The repesentor index is still 0, meaning the command line -w <pci_bdf>,representor=0 is correct on this case.
> 
> The problems comes from the assumption you do in your code about the representor id.
> What you do currently is to receive the representors and qsort them by device name. then you assign the priv->rep_id based on the qsort output.
> Later on when querying the if_name (mlx5_get_ifname) you assume that the phys_port_name of representor (which include the enumeration of what exists on its other side) is the same.
> 
> For x86 it probably works. On BlueField it breaks, as from some reason the phys_port_name is -1. 
> 
> My suggestion is to set the priv->rep_id based on the phys_port_name instead of qsort output. 

Yes, understood. The only drawback using this approach is that mlx5 devices
won't be usable at all if no netdevice can be associated with them (e.g. in
case it was moved to another netns). Currently all matching IB devs are
probed regardless, except they are handled as normal devices when the PMD
can't determine whether it's dealing with a representor.

> > Anyway, I suggest to deal with Bluefield specifics in a subsequent series.
> > This one focuses on and is validated with VF representors only.
> 
> It is related to VF representors only. BlueField is just an example. 

By "BlueField specifics", I mean the translation of -1 to 0 which so far is
specific to BlueField. Another patch is needed for that.

For devices where representors are properly numbered starting from 0, we must
rely on the uninterpreted phys_port_name value directly, which must be a
positive integer instead of a qsort() interpretation in order to properly
handle holes in the sequence due to missing devices (netns).

I intend to modify this patch as described.

-- 
Adrien Mazarguil
6WIND

  reply	other threads:[~2018-06-28  9:13 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 [this message]
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
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=20180628091309.GB4025@6wind.com \
    --to=adrien.mazarguil@6wind.com \
    --cc=dev@dpdk.org \
    --cc=guillaume.gaudonville@6wind.com \
    --cc=olgas@mellanox.com \
    --cc=rasland@mellanox.com \
    --cc=shahafs@mellanox.com \
    --cc=wisamm@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).