From: "Gaëtan Rivet" <gaetan.rivet@6wind.com>
To: Matan Azrad <matan@mellanox.com>
Cc: Ferruh Yigit <ferruh.yigit@intel.com>,
Thomas Monjalon <thomas@monjalon.net>,
"dev@dpdk.org" <dev@dpdk.org>,
"stephen@networkplumber.org" <stephen@networkplumber.org>
Subject: Re: [dpdk-dev] [PATCH v3 3/8] net/failsafe: support probed sub-devices getting
Date: Tue, 16 Jan 2018 23:31:04 +0100 [thread overview]
Message-ID: <20180116223104.y2iesdbxqkf775xp@bidouze.vm.6wind.com> (raw)
In-Reply-To: <AM6PR0502MB3797F6283E4B21B581F281F8D2EA0@AM6PR0502MB3797.eurprd05.prod.outlook.com>
Hi Matan,
On Tue, Jan 16, 2018 at 05:20:27PM +0000, Matan Azrad wrote:
> Hi Gaetan
>
<snip>
> > > > In 18.05, or 18.08 there should be an EAL function that would be
> > > > able to identify a device given a specific ID string (very close to an
> > rte_devargs).
> > > > Currently, this API does not exist.
> > > >
> > > > You can hack your way around this for the moment, IF you really,
> > > > really
> > > > want: parse your devargs, get the bus, use the bus->parse() function
> > > > to get a binary device representation, and compare bytes per bytes
> > > > the binary representation given by your devargs and by the device-
> > >name.
> > > >
> > > > But this is a hack, and a pretty ugly one at that: you have no way
> > > > of knowing the size taken by this binary representation, so you can
> > > > restrict yourself to the vdev and PCI bus for the moment and take
> > > > the larger of an rte_vdev_driver pointer and an rte_pci_addr....
> > > >
> > > > {
> > > > union {
> > > > rte_vdev_driver *drv;
> > > > struct rte_pci_addr pci_addr;
> > > > } bindev1, bindev2;
> > > > memset(&bindev1, 0, sizeof(bindev1));
> > > > memset(&bindev2, 0, sizeof(bindev2));
> > > > rte_eal_devargs_parse(device->name, da1);
> > > > rte_eal_devargs_parse(your_devstr, da2);
> > > > RTE_ASSERT(da1->bus == rte_bus_find_by_name("pci") ||
> > > > da1->bus == rte_bus_find_by_name("vdev"));
> > > > RTE_ASSERT(da2->bus == rte_bus_find_by_name("pci") ||
> > > > da2->bus == rte_bus_find_by_name("vdev"));
> > > > da1->bus->parse(da1->name, &bindev1);
> > > > da1->bus->parse(da2->name, &bindev2);
> > > > if (memcmp(&bindev1, &bindev2, sizeof(bindev1)) == 0) {
> > > > /* found the device */
> > > > } else {
> > > > /* not found */
> > > > }
> > > > }
> > > >
> > > > So, really, really ugly. Anyway.
> > > >
> > > Yes, ugly :) Thanks for this update!
> > > Will keep the comparison by device->name.
> > >
> >
> > Well as explained, above, the comparison by device->name only works with
> > whitelisted devices.
> >
>
> > So either implement something broken right now that you will need to
> > update in 18.05, or implement it properly in 18.05 from the get go.
> >
> For the current needs it is enough.
> We can also say that it is the user responsibility to pass to failsafe the same names and same args as he passes for EAL(or default EAL names).
> I think I emphasized it in documentation.
>
Okay, as you wish. Just be aware of this limitation.
I think this functionality is good and useful, but it needs to be made clean.
The proper function should be available soon, then this implementaion should
be cleaned up.
> > > > <snip>
> > > >
> > > > > > > + /* Take control of device probed by EAL
> > options. */
> > > > > > > + DEBUG("Taking control of a probed sub
> > device"
> > > > > > > + " %d named %s", i, da->name);
> > > > > >
> > > > > > In this case, the devargs of the probed device must be copied
> > > > > > within the sub- device definition and removed from the EAL using
> > > > > > the proper rte_devargs API.
> > > > > >
> > > > > > Note that there is no rte_devargs copy function. You can use
> > > > > > rte_devargs_parse instead, "parsing" again the original devargs
> > > > > > into the sub- device one. It is necessary for complying with
> > > > > > internal rte_devargs requirements (da->args being malloc-ed, at
> > > > > > the moment,
> > > > but may evolve).
> > > > > >
> > > > > > The rte_eal_devargs_parse function is not easy enough to use
> > > > > > right now, you will have to build a devargs string (using snprintf) and
> > submit it.
> > > > > > I proposed a change this release for it but it will not make it
> > > > > > for 18.02, that would have simplified your implementation.
> > > > > >
> > > > >
> > > > > Got you. You right we need to remove the created devargs in
> > > > > fail-safe
> > > > parse level.
> > > > > What do you think about checking it in the parse level and avoid
> > > > > the new
> > > > devargs creation?
> > > > > Also to do the copy in parse level(same method as we are doing in
> > > > > probe
> > > > level)?
> > > > >
> > > >
> > > > Not sure I follow here, but the new rte_devargs is part of the
> > > > sub-device (it is not a pointer, but allocated alongside the sub_device).
> > > >
> > > > So keep everything here, it is the right place to deal with these things.
> > > >
> > > But it will prevent the double parsing and also saves the method:
> > > If the device already parsed - copy its devargs and continue.
> > > If the device already probed - copy the device pointer and continue.
> > >
> > > I think this is the right dealing, no?
> > > Why to deal with parse level in probe level? Just keep all the parse work to
> > parse level and the probe work to probe level.
> >
> > After re-reading, I think we misunderstood each other.
> > You cannot remove the rte_devargs created during parsing: it is allocated
> > alongside the sub_device structure.
> >
> > You must only remove the rte_devargs allocated by the EAL (using
> > rte_eal_devargs_remove()).
> >
>
> Sure.
>
> > Before removing it, you must copy its content in the local sub_device
> > rte_devargs structure. I only proposed a way to do this copy that would not
> > deal with rte_devargs internals, as it is bound to evolve rather soon.
> >
> Yes.
>
> > Otherwise, no, I do not want to complicate the parsing operations, they are
> > already too complicated and too criticals. Better to keep it all here.
>
> I think fs_parse_device function is not complicated and it is the natural place for devargs games.
> For me this is the right place for the copy & remove devargs.
> Are you insisting to put all in fs_bus_init?
You would have to put fs_ethdev_portid_find in failsafe_args, which is
mixing layers. Sorry but yes, please keep all these changes in this
file.
Thanks,
--
Gaëtan Rivet
6WIND
next prev parent reply other threads:[~2018-01-16 22:31 UTC|newest]
Thread overview: 112+ messages / expand[flat|nested] mbox.gz Atom feed top
[not found] <20171124160801.GU4062@6wind.com>
[not found] ` <20171124164812.GV4062@6wind.com>
2017-11-24 17:21 ` [dpdk-dev] [RFC] Introduce virtual PMD for Hyper-V/Azure platforms Adrien Mazarguil
2017-12-18 16:46 ` [dpdk-dev] [PATCH v1 0/3] " Adrien Mazarguil
2017-12-18 16:46 ` [dpdk-dev] [PATCH v1 1/3] net/hyperv: introduce MS Hyper-V platform driver Adrien Mazarguil
2017-12-18 18:28 ` Stephen Hemminger
2017-12-18 19:54 ` Thomas Monjalon
2017-12-18 21:17 ` Stephen Hemminger
2017-12-19 10:01 ` Adrien Mazarguil
2017-12-19 11:15 ` Thomas Monjalon
2017-12-19 13:13 ` Adrien Mazarguil
2017-12-18 16:46 ` [dpdk-dev] [PATCH v1 2/3] net/hyperv: implement core functionality Adrien Mazarguil
2017-12-18 17:04 ` Wiles, Keith
2017-12-18 17:59 ` Adrien Mazarguil
2017-12-18 18:43 ` Wiles, Keith
2017-12-19 8:25 ` Nelio Laranjeiro
2017-12-18 18:26 ` Stephen Hemminger
2017-12-18 20:21 ` Adrien Mazarguil
2017-12-18 21:03 ` Thomas Monjalon
2017-12-18 21:19 ` Stephen Hemminger
2017-12-18 18:34 ` Stephen Hemminger
2017-12-18 20:23 ` Adrien Mazarguil
2017-12-19 9:53 ` Bruce Richardson
2017-12-19 10:15 ` Adrien Mazarguil
2017-12-19 15:31 ` Stephen Hemminger
2017-12-18 23:59 ` Stephen Hemminger
2017-12-19 10:01 ` Adrien Mazarguil
2017-12-19 15:37 ` Stephen Hemminger
2017-12-19 1:54 ` Ferruh Yigit
2017-12-19 15:06 ` Adrien Mazarguil
2017-12-19 20:44 ` Ferruh Yigit
2017-12-20 14:13 ` Thomas Monjalon
2017-12-21 16:19 ` Adrien Mazarguil
2017-12-18 16:46 ` [dpdk-dev] [PATCH v1 3/3] net/hyperv: add "force" parameter Adrien Mazarguil
2017-12-18 18:23 ` [dpdk-dev] [PATCH v1 0/3] Introduce virtual PMD for Hyper-V/Azure platforms Stephen Hemminger
2017-12-18 20:13 ` Thomas Monjalon
2017-12-19 0:40 ` Stephen Hemminger
2017-12-18 20:21 ` Adrien Mazarguil
2017-12-22 18:01 ` [dpdk-dev] [PATCH v2 0/5] " Adrien Mazarguil
2017-12-22 18:01 ` [dpdk-dev] [PATCH v2 1/5] net/failsafe: fix invalid free Adrien Mazarguil
2017-12-22 18:01 ` [dpdk-dev] [PATCH v2 2/5] net/failsafe: add "fd" parameter Adrien Mazarguil
2017-12-22 18:01 ` [dpdk-dev] [PATCH v2 3/5] net/vdev_netvsc: introduce Hyper-V platform driver Adrien Mazarguil
2017-12-22 18:01 ` [dpdk-dev] [PATCH v2 4/5] net/vdev_netvsc: implement core functionality Adrien Mazarguil
2017-12-22 18:01 ` [dpdk-dev] [PATCH v2 5/5] net/vdev_netvsc: add "force" parameter Adrien Mazarguil
2017-12-23 2:06 ` [dpdk-dev] [PATCH v2 0/5] Introduce virtual PMD for Hyper-V/Azure platforms Stephen Hemminger
2017-12-23 14:28 ` Thomas Monjalon
2018-01-09 14:47 ` [dpdk-dev] [PATCH v3 0/8] Introduce virtual driver " Matan Azrad
2018-01-09 14:47 ` [dpdk-dev] [PATCH v3 1/8] net/failsafe: fix invalid free Matan Azrad
2018-01-16 10:24 ` Gaëtan Rivet
2018-01-09 14:47 ` [dpdk-dev] [PATCH v3 2/8] net/failsafe: add "fd" parameter Matan Azrad
2018-01-16 10:54 ` Gaëtan Rivet
2018-01-16 11:19 ` Gaëtan Rivet
2018-01-16 16:17 ` Matan Azrad
2018-01-09 14:47 ` [dpdk-dev] [PATCH v3 3/8] net/failsafe: support probed sub-devices getting Matan Azrad
2018-01-16 11:09 ` Gaëtan Rivet
2018-01-16 12:27 ` Matan Azrad
2018-01-16 14:40 ` Gaëtan Rivet
2018-01-16 16:15 ` Matan Azrad
2018-01-16 16:54 ` Gaëtan Rivet
2018-01-16 17:20 ` Matan Azrad
2018-01-16 22:31 ` Gaëtan Rivet [this message]
2018-01-17 8:40 ` Matan Azrad
2018-01-09 14:47 ` [dpdk-dev] [PATCH v3 4/8] net/vdev_netvsc: introduce Hyper-V platform driver Matan Azrad
2018-01-09 14:47 ` [dpdk-dev] [PATCH v3 5/8] net/vdev_netvsc: implement core functionality Matan Azrad
2018-01-09 18:49 ` Stephen Hemminger
2018-01-10 15:02 ` Matan Azrad
2018-01-17 16:51 ` Thomas Monjalon
2018-01-09 14:47 ` [dpdk-dev] [PATCH v3 6/8] net/vdev_netvsc: skip routed netvsc probing Matan Azrad
2018-01-09 18:51 ` Stephen Hemminger
2018-01-10 15:07 ` Matan Azrad
2018-01-10 16:43 ` Stephen Hemminger
2018-01-11 9:00 ` Matan Azrad
2018-01-17 16:59 ` Thomas Monjalon
2018-01-09 14:47 ` [dpdk-dev] [PATCH v3 7/8] net/vdev_netvsc: add "force" parameter Matan Azrad
2018-01-09 14:47 ` [dpdk-dev] [PATCH v3 8/8] net/vdev_netvsc: add automatic probing Matan Azrad
2018-01-18 8:43 ` [dpdk-dev] [PATCH v4 0/8] Introduce virtual driver for Hyper-V/Azure platforms Matan Azrad
2018-01-18 8:43 ` [dpdk-dev] [PATCH v4 1/8] net/failsafe: fix invalid free Matan Azrad
2018-01-18 8:43 ` [dpdk-dev] [PATCH v4 2/8] net/failsafe: add "fd" parameter Matan Azrad
2018-01-18 8:51 ` Gaëtan Rivet
2018-01-18 8:43 ` [dpdk-dev] [PATCH v4 3/8] net/failsafe: add probed etherdev capture Matan Azrad
2018-01-18 9:10 ` Gaëtan Rivet
2018-01-18 9:33 ` Matan Azrad
2018-01-18 8:43 ` [dpdk-dev] [PATCH v4 4/8] net/vdev_netvsc: introduce Hyper-V platform driver Matan Azrad
2018-01-18 8:43 ` [dpdk-dev] [PATCH v4 5/8] net/vdev_netvsc: implement core functionality Matan Azrad
2018-01-18 18:25 ` Stephen Hemminger
2018-01-18 18:28 ` Matan Azrad
2018-01-18 8:43 ` [dpdk-dev] [PATCH v4 6/8] net/vdev_netvsc: skip routed netvsc probing Matan Azrad
2018-01-18 18:26 ` Stephen Hemminger
2018-01-18 18:47 ` Thomas Monjalon
2018-01-18 8:43 ` [dpdk-dev] [PATCH v4 7/8] net/vdev_netvsc: add "force" parameter Matan Azrad
2018-01-18 18:27 ` Stephen Hemminger
2018-01-18 18:30 ` Matan Azrad
2018-01-18 8:43 ` [dpdk-dev] [PATCH v4 8/8] net/vdev_netvsc: add automatic probing Matan Azrad
2018-01-18 10:01 ` [dpdk-dev] [PATCH v5 0/8] Introduce virtual driver for Hyper-V/Azure platforms Matan Azrad
2018-01-18 10:01 ` [dpdk-dev] [PATCH v5 1/8] net/failsafe: fix invalid free Matan Azrad
2018-01-18 10:01 ` [dpdk-dev] [PATCH v5 2/8] net/failsafe: add "fd" parameter Matan Azrad
2018-01-18 10:01 ` [dpdk-dev] [PATCH v5 3/8] net/failsafe: add probed etherdev capture Matan Azrad
2018-01-18 10:08 ` Gaëtan Rivet
2018-01-18 10:01 ` [dpdk-dev] [PATCH v5 4/8] net/vdev_netvsc: introduce Hyper-V platform driver Matan Azrad
2018-01-18 10:01 ` [dpdk-dev] [PATCH v5 5/8] net/vdev_netvsc: implement core functionality Matan Azrad
2018-01-18 10:01 ` [dpdk-dev] [PATCH v5 6/8] net/vdev_netvsc: skip routed netvsc probing Matan Azrad
2018-01-18 10:01 ` [dpdk-dev] [PATCH v5 7/8] net/vdev_netvsc: add "force" parameter Matan Azrad
2018-01-18 10:01 ` [dpdk-dev] [PATCH v5 8/8] net/vdev_netvsc: add automatic probing Matan Azrad
2018-01-18 13:51 ` [dpdk-dev] [PATCH v6 0/8] Introduce virtual driver for Hyper-V/Azure platforms Matan Azrad
2018-01-18 13:51 ` [dpdk-dev] [PATCH v6 1/8] net/failsafe: fix invalid free Matan Azrad
2018-01-18 13:51 ` [dpdk-dev] [PATCH v6 2/8] net/failsafe: add "fd" parameter Matan Azrad
2018-01-18 13:51 ` [dpdk-dev] [PATCH v6 3/8] net/failsafe: add probed etherdev capture Matan Azrad
2018-01-18 22:34 ` Thomas Monjalon
2018-01-18 13:51 ` [dpdk-dev] [PATCH v6 4/8] net/vdev_netvsc: introduce Hyper-V platform driver Matan Azrad
2018-01-18 13:51 ` [dpdk-dev] [PATCH v6 5/8] net/vdev_netvsc: implement core functionality Matan Azrad
2018-01-18 13:51 ` [dpdk-dev] [PATCH v6 6/8] net/vdev_netvsc: skip routed netvsc probing Matan Azrad
2018-01-18 13:51 ` [dpdk-dev] [PATCH v6 7/8] net/vdev_netvsc: add "force" parameter Matan Azrad
2018-01-18 13:51 ` [dpdk-dev] [PATCH v6 8/8] net/vdev_netvsc: add automatic probing Matan Azrad
2018-01-20 1:15 ` [dpdk-dev] [PATCH v6 0/8] Introduce virtual driver for Hyper-V/Azure platforms Ferruh Yigit
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=20180116223104.y2iesdbxqkf775xp@bidouze.vm.6wind.com \
--to=gaetan.rivet@6wind.com \
--cc=dev@dpdk.org \
--cc=ferruh.yigit@intel.com \
--cc=matan@mellanox.com \
--cc=stephen@networkplumber.org \
--cc=thomas@monjalon.net \
/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).