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

[John Ousterhout <ouster@cs.stanford.edu>]

Hi Harry,

But, given the existence of the --file-prefix option, isn't it already
unsafe for Collectd to check only for .rte_config? If it's important for
other programs to be able to find the config files, it seems to me that a
more robust mechanism is needed than the current one.

-John-

On Thu, Oct 13, 2016 at 9:07 AM, Van Haaren, Harry <
harry.van.haaren@intel.com> wrote:

> Hi John,
>
> > From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of John Ousterhout
> > Subject: Re: [dpdk-dev] [PATCH] eal: avoid unnecessary conflicts over
> rte_config file
>
> <snip>
>
> > For example, it took me several hours
> > to figure out why the problem was occurring and then to hunt down the
> > --file-prefix solution. Is there some reason why it would be a bad idea
> to
> > fix this in DPDK?
>
> Right now, DPDK will by default always use a consistent .rte_config
> location, which allows other applications to monitor that. For example,
> Collectd is able to monitor a DPDK primary process by checking if the
> .rte_config file exists at its default location[1].
>
> If we change the default behavior of DPDK, other projects can no longer
> rely on that file being created, and these monitoring projects must be
> updated in sync with DPDK to avoid breakage if versions mismatch.
>
> Although I see your use-case, I think using the --file-prefix as
> Konstantin suggests is a better solution in this case.
>
> Regards, -Harry
>
> [1] https://github.com/collectd/collectd/blob/master/src/dpdkstat.c#L60
>
>
> > On Thu, Oct 13, 2016 at 3:53 AM, Ananyev, Konstantin <
> > konstantin.ananyev@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@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
> > >
> > > >
> > > >  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
> > >
> > >
>