DPDK patches and discussions
 help / color / mirror / Atom feed
From: "Tahhan, Maryam" <maryam.tahhan@intel.com>
To: "Ananyev, Konstantin" <konstantin.ananyev@intel.com>,
	John Ousterhout <ouster@cs.stanford.edu>,
	"thomas.monjalon@6wind.com" <thomas.monjalon@6wind.com>
Cc: "dev@dpdk.org" <dev@dpdk.org>
Subject: Re: [dpdk-dev] [PATCH] eal: avoid unnecessary conflicts over rte_config	file
Date: Thu, 13 Oct 2016 21:39:25 +0000	[thread overview]
Message-ID: <1A27633A6DA49C4A92FCD5D4312DBF536B37DF3E@IRSMSX106.ger.corp.intel.com> (raw)
In-Reply-To: <2601191342CEEE43887BDE71AB9772583F0C1256@irsmsx105.ger.corp.intel.com>

> Hi John,
> 
> > Before this patch, DPDK used the file ~/.rte_config as a lock to
> > detect potential interference between multiple DPDK applications
> > running on the same machine. However, if a single user ran DPDK
> > applications concurrently on several different machines, and if the
> > user's home directory was shared between the machines via NFS, DPDK
> > would incorrectly detect conflicts for all but the first application
> > and abort them. This patch fixes the problem by incorporating the
> > machine name into the config file name (e.g., ~/.rte_hostname_config).
> >
> > Signed-off-by: John Ousterhout <ouster@cs.stanford.edu>
> > ---
> >  doc/guides/prog_guide/multi_proc_support.rst | 11 +++++++----
> >  lib/librte_eal/common/eal_common_proc.c      |  8 ++------
> >  lib/librte_eal/common/eal_filesystem.h       | 15 +++++++++++++--
> >  3 files changed, 22 insertions(+), 12 deletions(-)
> >
> > diff --git a/doc/guides/prog_guide/multi_proc_support.rst
> > b/doc/guides/prog_guide/multi_proc_support.rst
> > index badd102..a54fa1c 100644
> > --- a/doc/guides/prog_guide/multi_proc_support.rst
> > +++ b/doc/guides/prog_guide/multi_proc_support.rst
> > @@ -129,10 +129,13 @@ Support for this usage scenario is provided
> > using the ``--file-prefix`` paramete
> >
> >  By default, the EAL creates hugepage files on each hugetlbfs
> > filesystem using the rtemap_X filename,  where X is in the range 0 to the
> maximum number of hugepages -1.
> > -Similarly, it creates shared configuration files, memory mapped in
> > each process, using the /var/run/.rte_config filename, -when run as
> > root (or $HOME/.rte_config when run as a non-root user; -if filesystem and
> device permissions are set up to allow this).
> > -The rte part of the filenames of each of the above is configurable using the
> file-prefix parameter.
> > +Similarly, it creates shared configuration files, memory mapped in each
> process.
> > +When run as root, the name of the configuration file will be
> > +/var/run/.rte_*host*_config, where *host* is the name of the machine.
> > +When run as a non-root user, the the name of the configuration file
> > +will be $HOME/.rte_*host*_config (if filesystem and device permissions
> are set up to allow this).
> > +If the ``--file-prefix`` parameter has been specified, its value will
> > +be used in place of "rte" in the file names.
> 
> I am not sure that we need to handle all such cases inside EAL.
> User can easily overcome that problem by just adding something like:
> --file-prefix=`uname -n`
> to his command-line.
> Konstantin
> 

I agree with Konstantin, there's no need to include the hostname in the rte config file + I'm not sure this will be backward compatible with existing DPDK applications that use a secondary processes that use the config file (as in, multiprocess DPDK applications in use would all need to be updated). What Konstantin suggests fixes the issue you were encountering without breaking backward compatibility.
Is addition, the hostname is not unique... you could in theory have 2 hosts with the same hostname and encounter the issue you were seeing again.

> >
> >  In addition to specifying the file-prefix parameter,  any DPDK
> > applications that are to be run side-by-side must explicitly limit their
> memory use.
> > diff --git a/lib/librte_eal/common/eal_common_proc.c
> > b/lib/librte_eal/common/eal_common_proc.c
> > index 12e0fca..517aa0c 100644
> > --- a/lib/librte_eal/common/eal_common_proc.c
> > +++ b/lib/librte_eal/common/eal_common_proc.c
> > @@ -45,12 +45,8 @@ rte_eal_primary_proc_alive(const char
> > *config_file_path)
> >
> >  	if (config_file_path)
> >  		config_fd = open(config_file_path, O_RDONLY);
> > -	else {
> > -		char default_path[PATH_MAX+1];
> > -		snprintf(default_path, PATH_MAX, RUNTIME_CONFIG_FMT,
> > -			 default_config_dir, "rte");
> > -		config_fd = open(default_path, O_RDONLY);
> > -	}
> > +	else
> > +		config_fd = open(eal_runtime_config_path(), O_RDONLY);
> >  	if (config_fd < 0)
> >  		return 0;
> >
> > diff --git a/lib/librte_eal/common/eal_filesystem.h
> > b/lib/librte_eal/common/eal_filesystem.h
> > index fdb4a70..4929aa3 100644
> > --- a/lib/librte_eal/common/eal_filesystem.h
> > +++ b/lib/librte_eal/common/eal_filesystem.h
> > @@ -41,7 +41,7 @@
> >  #define EAL_FILESYSTEM_H
> >
> >  /** Path of rte config file. */
> > -#define RUNTIME_CONFIG_FMT "%s/.%s_config"
> > +#define RUNTIME_CONFIG_FMT "%s/.%s_%s_config"
> >
> >  #include <stdint.h>
> >  #include <limits.h>
> > @@ -59,11 +59,22 @@ eal_runtime_config_path(void)
> >  	static char buffer[PATH_MAX]; /* static so auto-zeroed */
> >  	const char *directory = default_config_dir;
> >  	const char *home_dir = getenv("HOME");
> > +	static char nameBuffer[1000];
> > +	int result;
> >
> >  	if (getuid() != 0 && home_dir != NULL)
> >  		directory = home_dir;
> > +
> > +	/*
> > +	 * Include the name of the host in the config file path. Otherwise,
> > +	 * if DPDK applications run on different hosts but share a home
> > +	 * directory (e.g. via NFS), they will choose the same config
> > +	 * file and conflict unnecessarily.
> > +	 */
> > +	result = gethostname(nameBuffer, sizeof(nameBuffer)-1);
> >  	snprintf(buffer, sizeof(buffer) - 1, RUNTIME_CONFIG_FMT,
> directory,
> > -			internal_config.hugefile_prefix);
> > +			internal_config.hugefile_prefix,
> > +		(result == 0) ? nameBuffer : "unknown-host");
> >  	return buffer;
> >  }
> >
> > --
> > 2.8.3

  parent reply	other threads:[~2016-10-13 21:39 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-10-12 21:29 John Ousterhout
2016-10-13 10:53 ` Ananyev, Konstantin
2016-10-13 15:42   ` John Ousterhout
2016-10-13 16:07     ` Van Haaren, Harry
2016-10-13 16:20       ` John Ousterhout
2016-10-13 16:30         ` Van Haaren, Harry
2016-10-13 18:28           ` Stephen Hemminger
2016-10-13 16:31         ` Thomas Monjalon
2016-10-13 17:01     ` Ananyev, Konstantin
2016-10-13 21:39   ` Tahhan, Maryam [this message]
2016-10-14 16:27     ` John Ousterhout
2019-01-11 21:52       ` 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=1A27633A6DA49C4A92FCD5D4312DBF536B37DF3E@IRSMSX106.ger.corp.intel.com \
    --to=maryam.tahhan@intel.com \
    --cc=dev@dpdk.org \
    --cc=konstantin.ananyev@intel.com \
    --cc=ouster@cs.stanford.edu \
    --cc=thomas.monjalon@6wind.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).