[dpdk-dev] [PATCH] eal devargs: don't call rte_log when not initialized

Olivier MATZ olivier.matz at 6wind.com
Mon May 25 13:53:47 CEST 2015


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 at networkplumber.org>

Acked-by: Olivier Matz <olivier.matz at 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;
>  	}
>  
> 


More information about the dev mailing list