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>
Subject: Re: [dpdk-dev] [PATCH v2 6/7] net/mlx5: probe all port representors
Date: Wed, 27 Jun 2018 15:32:28 +0200 [thread overview]
Message-ID: <20180627133228.GV4025@6wind.com> (raw)
In-Reply-To: <DB7PR05MB44269879B18DBD1B8A3481AEC3720@DB7PR05MB4426.eurprd05.prod.outlook.com>
On Sun, Jun 17, 2018 at 10:15:07AM +0000, Shahaf Shuler wrote:
> Hi Adrien,
>
> Saturday, June 16, 2018 11:58 AM, Xueming(Steven) Li:
> > Subject: RE: [dpdk-dev] [PATCH v2 6/7] net/mlx5: probe all port representors
> >
> > > -----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.
>
> This is breaking compatibility for application using the device names in order to attach them to the application (e.g. OVS-DPDK).
> Before this patch the naming scheme for non-representor port is "{DBDF}".
>
> Can we preserve the compatibility and add appropriate suffix for the representor case?
There's one issue if representors are hot-plugged. The name of the master
device, which happens to be that of the switch domain, cannot be
updated. The form "net_{DBDF}_0" seems expected for PMDs that support
representors (see ixgbe and i40e).
Now since representor hot-plugging is not supported yet, I guess we could
postpone this problem by keeping the old format in the meantime, however
ideally, these applications should not rely on it. The only safe assumption
they can make is the uniqueness of any given name among ethdevs.
PCI bus addresses, if needed, should be retrieved by looking at the
underlying bus object.
By the way, while thinking again about a past comment from Xueming [1],
maybe it's finally time to remove support for multiple Verbs ports on mlx5
after all. This should drop another unnecessary loop and the need for the
unused "port %u" suffix at all while naming the device.
So how about the following plan for v3:
- Adding a patch that drops support for multiple Verbs ports (note for
Xueming, yes I changed my mind *again* :)
- If you really think this will break OVS (please confirm), then when no
"representor" parameter is provided (regardless of the presence of any
representors), name format will use the usual "{DBDF}" notation as you
suggested.
- Otherwise as soon as a "representor" is found on the command line, the new
format will be used, again regardless of the presence of any representors.
- In both cases, representors if any, will be named according to the format
specified in this patch.
[1] https://mails.dpdk.org/archives/dev/2018-June/104015.html
<snip>
> > > 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?
I agree that the current approach sucks, hence the big fat warnings I left
around (see discussion with Xueming [2]). Problem is that the needed
information is not yet known at this stage; there is no private structure to
rely on to use mlx5_get_ifname() directly.
I'd also rather see these assumptions go in any case. I'll attempt to
improve things for v3 in preparation of allowing representors to be probed
on their own anytime, possibly even before the master device.
[2] https://mails.dpdk.org/archives/dev/2018-June/104059.html
<snip>
> > > diff --git a/drivers/net/mlx5/mlx5.h b/drivers/net/mlx5/mlx5.h index
> > > 997b04a33..0fe467140 100644
> > > --- a/drivers/net/mlx5/mlx5.h
> > > +++ b/drivers/net/mlx5/mlx5.h
> > > @@ -161,6 +161,10 @@ struct priv {
> > > uint16_t mtu; /* Configured MTU. */
> > > uint8_t port; /* Physical port number. */
> > > unsigned int isolated:1; /* Whether isolated mode is enabled. */
> > > + unsigned int representor:1; /* Device is a port representor. */
>
> Why we need above flag? Why can't we use RTE_ETH_DEV_REPRESENTOR from eth_dev->data->dev_flags.
Problem is that this flag can only be set once the ethdev is fully
instantiated and can't be relied on internally where needed (e.g. during
clean up in error handling code). It's reported to applications but not used
internally.
As a device property, it's actually pretty similar to the VF bit or
offloaded capabilities where checking exposed information would be
needlessly complex.
Now maybe it could be part of struct mlx5_dev_config as well. I initially
assumed this object was only for user-provided parameters but looks like
it's not the case. I intend to move it there for v3.
--
Adrien Mazarguil
6WIND
next prev 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 [this message]
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=20180627133228.GV4025@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).