From: Matan Azrad <matan@mellanox.com>
To: "Gaëtan Rivet" <gaetan.rivet@6wind.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 12:27:57 +0000 [thread overview]
Message-ID: <AM6PR0502MB37979AB3BCBCF1276CF8315AD2EA0@AM6PR0502MB3797.eurprd05.prod.outlook.com> (raw)
In-Reply-To: <20180116110920.vqp3bqjroudsdjm4@bidouze.vm.6wind.com>
Hi Gaetan
From: Gaëtan Rivet, Tuesday, January 16, 2018 1:09 PM
> Hi Matan,
>
> I'n not fond of the commit title, how about:
>
> [PATCH v3 3/8] net/failsafe: add probed etherdev capture
>
> ?
>
OK, no problem.
> On Tue, Jan 09, 2018 at 02:47:28PM +0000, Matan Azrad wrote:
> > Previous fail-safe code didn't support getting probed sub-devices and
> > failed when it tried to probe them.
> >
> > Skip fail-safe sub-device probing when it already was probed.
> >
> > Signed-off-by: Matan Azrad <matan@mellanox.com>
> > Cc: Gaetan Rivet <gaetan.rivet@6wind.com>
> > ---
> > doc/guides/nics/fail_safe.rst | 5 ++++
> > drivers/net/failsafe/failsafe_eal.c | 60
> > ++++++++++++++++++++++++-------------
> > 2 files changed, 45 insertions(+), 20 deletions(-)
> >
> > diff --git a/doc/guides/nics/fail_safe.rst
> > b/doc/guides/nics/fail_safe.rst index 5b1b47e..b89e53b 100644
> > --- a/doc/guides/nics/fail_safe.rst
> > +++ b/doc/guides/nics/fail_safe.rst
> > @@ -115,6 +115,11 @@ Fail-safe command line parameters
> > order to take only the last line into account (unlike ``exec()``) at every
> > probe attempt.
> >
> > +.. note::
> > +
> > + In case of whitelist sub-device probed by EAL, fail-safe PMD will take the
> device
> > + as is, which means that EAL device options are taken in this case.
> > +
> > - **mac** parameter [MAC address]
> >
> > This parameter allows the user to set a default MAC address to the
> > fail-safe diff --git a/drivers/net/failsafe/failsafe_eal.c
> > b/drivers/net/failsafe/failsafe_eal.c
> > index 19d26f5..7bc7453 100644
> > --- a/drivers/net/failsafe/failsafe_eal.c
> > +++ b/drivers/net/failsafe/failsafe_eal.c
> > @@ -36,39 +36,59 @@
> > #include "failsafe_private.h"
> >
> > static int
> > +fs_get_port_by_device_name(const char *name, uint16_t *port_id)
>
> The naming convention for the failsafe driver is
>
> namespace_object_sub-object_action()
>
OK.
> With an ordering of objects by their scope (std, rte, failsafe, file).
> Also, "get" as an action is not descriptive enough.
>
Isn't "get by device name" descriptive?
> static int
> fs_ethdev_capture(const char *name, uint16_t *port_id);
>
You miss here the main reason why we need this function instead of using rte_eth_dev_get_port_by_name.
The reason we need this function is because we want to find the device by the device name and not ethdev name.
What's about fs_port_capture_by_device_name?
Maybe comparing it to device->devargs->name is better, What do you think?
> > +{
> > + uint16_t pid;
> > + size_t len;
> > +
> > + if (name == NULL) {
> > + DEBUG("Null pointer is specified\n");
> > + return -EINVAL;
> > + }
> > + len = strlen(name);
> > + RTE_ETH_FOREACH_DEV(pid) {
> > + if (!strncmp(name, rte_eth_devices[pid].device->name,
> len)) {
> > + *port_id = pid;
> > + return 0;
> > + }
> > + }
> > + return -ENODEV;
> > +}
> > +
> > +static int
> > fs_bus_init(struct rte_eth_dev *dev)
> > {
> > struct sub_device *sdev;
> > struct rte_devargs *da;
> > uint8_t i;
> > - uint16_t j;
> > + uint16_t pid;
> > int ret;
> >
> > FOREACH_SUBDEV(sdev, i, dev) {
> > if (sdev->state != DEV_PARSED)
> > continue;
> > da = &sdev->devargs;
> > - ret = rte_eal_hotplug_add(da->bus->name,
> > - da->name,
> > - da->args);
> > - if (ret) {
> > - ERROR("sub_device %d probe failed %s%s%s", i,
> > - rte_errno ? "(" : "",
> > - rte_errno ? strerror(rte_errno) : "",
> > - rte_errno ? ")" : "");
> > - continue;
> > - }
> > - RTE_ETH_FOREACH_DEV(j) {
> > - if (strcmp(rte_eth_devices[j].device->name,
> > - da->name) == 0) {
> > - ETH(sdev) = &rte_eth_devices[j];
> > - break;
> > + if (fs_get_port_by_device_name(da->name, &pid) != 0) {
> > + ret = rte_eal_hotplug_add(da->bus->name,
> > + da->name,
> > + da->args);
> > + if (ret) {
> > + ERROR("sub_device %d probe failed
> %s%s%s", i,
> > + rte_errno ? "(" : "",
> > + rte_errno ? strerror(rte_errno) : "",
> > + rte_errno ? ")" : "");
> > + continue;
> > }
> > + if (fs_get_port_by_device_name(da->name, &pid)
> != 0) {
> > + ERROR("sub_device %d init went wrong", i);
> > + return -ENODEV;
> > + }
> > + } else {
> > + /* 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)?
> --
> Gaëtan Rivet
> 6WIND
next prev parent reply other threads:[~2018-01-16 12:27 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 [this message]
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
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=AM6PR0502MB37979AB3BCBCF1276CF8315AD2EA0@AM6PR0502MB3797.eurprd05.prod.outlook.com \
--to=matan@mellanox.com \
--cc=dev@dpdk.org \
--cc=ferruh.yigit@intel.com \
--cc=gaetan.rivet@6wind.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).