From: Olivier MATZ <olivier.matz@6wind.com>
To: Stephen Hemminger <stephen@networkplumber.org>, dev@dpdk.org
Subject: Re: [dpdk-dev] [PATCH] eal devargs: don't call rte_log when not initialized
Date: Mon, 25 May 2015 13:53:47 +0200 [thread overview]
Message-ID: <55630D4B.40708@6wind.com> (raw)
In-Reply-To: <1431707860-19562-1-git-send-email-stephen@networkplumber.org>
Hi Stephen,
On 05/15/2015 06:37 PM, Stephen Hemminger wrote:
> This problem was discovered when passing invalid PCI id to the
> blacklist API in devargs.
>
> Any failures in rte_devargs_add would cause a core dump because
> it would call rte_log() before the the EAL log environment was
> initailized. Rather than try and log just remove the messages
> and leave it up to the caller to check the return value.
>
> Most of the other failure possibilities are when malloc() fails, and if
> that happens any logging that used malloc() would also fail.
>
> This failure was not caught by the standalone tests to devargs
> because the tests are run after calling rte_eal_init (which is not
> how devargs is intended to be used).
>
> Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
Acked-by: Olivier Matz <olivier.matz@6wind.com>
Thanks,
Olivier
> ---
> lib/librte_eal/common/eal_common_devargs.c | 30 +++++++++++++-----------------
> 1 file changed, 13 insertions(+), 17 deletions(-)
>
> diff --git a/lib/librte_eal/common/eal_common_devargs.c b/lib/librte_eal/common/eal_common_devargs.c
> index 615945e..ec56165 100644
> --- a/lib/librte_eal/common/eal_common_devargs.c
> +++ b/lib/librte_eal/common/eal_common_devargs.c
> @@ -31,11 +31,15 @@
> */
>
> /* This file manages the list of devices and their arguments, as given
> - * by the user at startup */
> + * by the user at startup
> + *
> + * Code here should not call rte_log since the EAL environment
> + * may not be initialized.
> + */
>
> +#include <stdio.h>
> #include <string.h>
>
> -#include <rte_log.h>
> #include <rte_pci.h>
> #include <rte_devargs.h>
> #include "eal_private.h"
> @@ -54,11 +58,8 @@ rte_eal_parse_devargs_str(const char *devargs_str,
> return -1;
>
> *drvname = strdup(devargs_str);
> - if (drvname == NULL) {
> - RTE_LOG(ERR, EAL,
> - "cannot allocate temp memory for driver name\n");
> + if (drvname == NULL)
> return -1;
> - }
>
> /* set the first ',' to '\0' to split name and arguments */
> sep = strchr(*drvname, ',');
> @@ -70,8 +71,6 @@ rte_eal_parse_devargs_str(const char *devargs_str,
> }
>
> if (*drvargs == NULL) {
> - RTE_LOG(ERR, EAL,
> - "cannot allocate temp memory for driver arguments\n");
> free(*drvname);
> return -1;
> }
> @@ -88,10 +87,9 @@ rte_eal_devargs_add(enum rte_devtype devtype, const char *devargs_str)
>
> /* use malloc instead of rte_malloc as it's called early at init */
> devargs = malloc(sizeof(*devargs));
> - if (devargs == NULL) {
> - RTE_LOG(ERR, EAL, "cannot allocate devargs\n");
> + if (devargs == NULL)
> goto fail;
> - }
> +
> memset(devargs, 0, sizeof(*devargs));
> devargs->type = devtype;
>
> @@ -103,19 +101,17 @@ rte_eal_devargs_add(enum rte_devtype devtype, const char *devargs_str)
> case RTE_DEVTYPE_BLACKLISTED_PCI:
> /* try to parse pci identifier */
> if (eal_parse_pci_BDF(buf, &devargs->pci.addr) != 0 &&
> - eal_parse_pci_DomBDF(buf, &devargs->pci.addr) != 0) {
> - RTE_LOG(ERR, EAL, "invalid PCI identifier <%s>\n", buf);
> + eal_parse_pci_DomBDF(buf, &devargs->pci.addr) != 0)
> goto fail;
> - }
> +
> break;
> case RTE_DEVTYPE_VIRTUAL:
> /* save driver name */
> ret = snprintf(devargs->virtual.drv_name,
> sizeof(devargs->virtual.drv_name), "%s", buf);
> - if (ret < 0 || ret >= (int)sizeof(devargs->virtual.drv_name)) {
> - RTE_LOG(ERR, EAL, "driver name too large: <%s>\n", buf);
> + if (ret < 0 || ret >= (int)sizeof(devargs->virtual.drv_name))
> goto fail;
> - }
> +
> break;
> }
>
>
next prev parent reply other threads:[~2015-05-25 11:53 UTC|newest]
Thread overview: 3+ messages / expand[flat|nested] mbox.gz Atom feed top
2015-05-15 16:37 Stephen Hemminger
2015-05-25 11:53 ` Olivier MATZ [this message]
2015-07-20 0:59 ` Thomas Monjalon
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=55630D4B.40708@6wind.com \
--to=olivier.matz@6wind.com \
--cc=dev@dpdk.org \
--cc=stephen@networkplumber.org \
/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).