[dpdk-dev] [PATCH] eal: avoid unnecessary conflicts over rte_config file

[John Ousterhout]

It sounds like my patch would break some existing software, so it probably
doesn't make sense right now.

I'd still argue that the current mechanism has a number of problems, and it
should probably undergo a comprehensive overhaul at some point in the
future.

-John-

On Thu, Oct 13, 2016 at 2:39 PM, Tahhan, Maryam <maryam.tahhan at intel.com>
wrote:

> > 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 at 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
>
>