[v6,3/5] telemetry: use unique socket paths for in-memory mode

Message ID 20211005135909.726091-4-bruce.richardson@intel.com (mailing list archive)
State Superseded, archived
Delegated to: David Marchand
Headers
Series improve telemetry support with in-memory mode |

Checks

Context Check Description
ci/checkpatch success coding style OK

Commit Message

Bruce Richardson Oct. 5, 2021, 1:59 p.m. UTC
  When DPDK is run using "in-memory" flag, multiple processes can be run
using the same file-prefix and hence the same runtime directory. To
avoid problems with conflicting telemetry unix socket paths, we can
put the pid of the process into the socket name. As with the existing
telemetry socket files, these sockets are removed on normal program
exit.

Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
---
 doc/guides/howto/telemetry.rst     | 17 ++++++++++++++++-
 lib/eal/freebsd/eal.c              |  1 +
 lib/eal/linux/eal.c                |  1 +
 lib/telemetry/telemetry.c          | 15 ++++++++++++---
 lib/telemetry/telemetry_internal.h |  3 ++-
 5 files changed, 32 insertions(+), 5 deletions(-)
  

Comments

Kevin Traynor Oct. 5, 2021, 2:41 p.m. UTC | #1
Hi Bruce, I started reviewing v5 before v6 was pushed, so these are just 
comments from v5,

On 05/10/2021 14:59, Bruce Richardson wrote:
> When DPDK is run using "in-memory" flag, multiple processes can be run
> using the same file-prefix and hence the same runtime directory. To
> avoid problems with conflicting telemetry unix socket paths, we can
> put the pid of the process into the socket name. As with the existing
> telemetry socket files, these sockets are removed on normal program
> exit.
> 
> Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
> ---
>   doc/guides/howto/telemetry.rst     | 17 ++++++++++++++++-
>   lib/eal/freebsd/eal.c              |  1 +
>   lib/eal/linux/eal.c                |  1 +
>   lib/telemetry/telemetry.c          | 15 ++++++++++++---
>   lib/telemetry/telemetry_internal.h |  3 ++-
>   5 files changed, 32 insertions(+), 5 deletions(-)
> 
> diff --git a/doc/guides/howto/telemetry.rst b/doc/guides/howto/telemetry.rst
> index 8f4fa1a510..8a61302459 100644
> --- a/doc/guides/howto/telemetry.rst
> +++ b/doc/guides/howto/telemetry.rst
> @@ -13,12 +13,27 @@ ethdev port list, and eal parameters.
>   Telemetry Interface
>   -------------------
>   
> -The :doc:`../prog_guide/telemetry_lib` opens a socket with path
> +For applications run normally, i.e. without the `--in-memory` EAL flag,
> +the :doc:`../prog_guide/telemetry_lib` opens a socket with path
>   *<runtime_directory>/dpdk_telemetry.<version>*. The version represents the
>   telemetry version, the latest is v2. For example, a client would connect to a
>   socket with path  */var/run/dpdk/\*/dpdk_telemetry.v2* (when the primary process
>   is run by a root user).
>   
> +For applications run with the `--in-memory` EAL flag,
> +the socket file is created with an additional suffix of the process PID.
> +This is because multiple independent DPDK processes can be run simultaneously
> +using the same runtime directory when *in-memory* mode is used.
> +For example, when a user with UID 1000 runs processes with in-memory mode,
> +we would find sockets available such as::
> +
> +  /run/user/1000/dpdk/rte/dpdk_telemetry.v2.1982
> +  /run/user/1000/dpdk/rte/dpdk_telemetry.v2.1935
> +

It seems an unnecessary step unless there is multiple process. As a 
suggestion, how about "simplifying" by always adding a check for an 
active socket with the default name. If it is not found, create it with 
the default (pre patches) name. If it is found and active, create a new 
one with process id appended. e.g.

First:
/run/user/1000/dpdk/rte/dpdk_telemetry.v2

Next:
/run/user/1000/dpdk/rte/dpdk_telemetry.v2.1982
/run/user/1000/dpdk/rte/dpdk_telemetry.v2.1935

It means that existing socket can still be used by anything using 
telemetry. I think it is a nice default to keep as it doesn't require 
any changes for anything that will connect (e.g. collectd?)

The downside is that one will have a different name but it seems an 
acceptable trade-off to keep compatibility for single process case.

WDYT?

> +Where `/run/user/<uid>` is the runtime directory for the user given by the
> +`$XDG_RUNTIME_DIR` environment variable,
> +and `rte` is the default DPDK file prefix used for a runtime directory.
> +
>   
>   Telemetry Initialization
>   ------------------------
> diff --git a/lib/eal/freebsd/eal.c b/lib/eal/freebsd/eal.c
> index b06a2c1662..ed39d10b4e 100644
> --- a/lib/eal/freebsd/eal.c
> +++ b/lib/eal/freebsd/eal.c
> @@ -952,6 +952,7 @@ rte_eal_init(int argc, char **argv)
>   		if (tlog < 0)
>   			tlog = RTE_LOGTYPE_EAL;
>   		if (rte_telemetry_init(rte_eal_get_runtime_dir(),
> +				internal_conf->in_memory | internal_conf->no_shconf,
>   				rte_version(),
>   				&internal_conf->ctrl_cpuset, rte_log, tlog) != 0)
>   			return -1;
> diff --git a/lib/eal/linux/eal.c b/lib/eal/linux/eal.c
> index 0d0fc66668..9db4eb7913 100644
> --- a/lib/eal/linux/eal.c
> +++ b/lib/eal/linux/eal.c
> @@ -1326,6 +1326,7 @@ rte_eal_init(int argc, char **argv)
>   		if (tlog < 0)
>   			tlog = RTE_LOGTYPE_EAL;
>   		if (rte_telemetry_init(rte_eal_get_runtime_dir(),
> +				internal_conf->in_memory | internal_conf->no_shconf,

should be logical OR

>   				rte_version(),
>   				&internal_conf->ctrl_cpuset, rte_log, tlog) != 0)
>   			return -1;
> diff --git a/lib/telemetry/telemetry.c b/lib/telemetry/telemetry.c
> index bd804a25a9..24441d100b 100644
> --- a/lib/telemetry/telemetry.c
> +++ b/lib/telemetry/telemetry.c
> @@ -51,6 +51,7 @@ static struct socket v1_socket; /* socket for v1 telemetry */
>   
>   static const char *telemetry_version; /* save rte_version */
>   static const char *socket_dir;        /* runtime directory */
> +static bool socket_uses_pid;          /* for in-memory mode, we need different socket paths */
>   static rte_cpuset_t *thread_cpuset;
>   static rte_log_fn rte_log_ptr;
>   static uint32_t logtype;
> @@ -432,8 +433,14 @@ static inline char *
>   get_socket_path(const char *runtime_dir, const int version)
>   {
>   	static char path[PATH_MAX];
> -	snprintf(path, sizeof(path), "%s/dpdk_telemetry.v%d",
> -			strlen(runtime_dir) ? runtime_dir : "/tmp", version);
> +	if (!socket_uses_pid)
> +		snprintf(path, sizeof(path), "%s/dpdk_telemetry.v%d",
> +				strlen(runtime_dir) ? runtime_dir : "/tmp", version);
> +	else
> +		snprintf(path, sizeof(path), "%s/dpdk_telemetry.v%d.%u",
> +				strlen(runtime_dir) ? runtime_dir : "/tmp",
> +				version,
> +				(unsigned int)getpid());
>   	return path;
>   }
>   
> @@ -591,11 +598,13 @@ telemetry_v2_init(void)
>   #endif /* !RTE_EXEC_ENV_WINDOWS */
>   
>   int32_t
> -rte_telemetry_init(const char *runtime_dir, const char *rte_version, rte_cpuset_t *cpuset,
> +rte_telemetry_init(const char *runtime_dir, bool in_memory,
> +		const char *rte_version, rte_cpuset_t *cpuset,
>   		rte_log_fn log_fn, uint32_t registered_logtype)
>   {
>   	telemetry_version = rte_version;
>   	socket_dir = runtime_dir;
> +	socket_uses_pid = in_memory; /* for in-memory mode use pid in sock path for uniqueness */
>   	thread_cpuset = cpuset;
>   	rte_log_ptr = log_fn;
>   	logtype = registered_logtype;
> diff --git a/lib/telemetry/telemetry_internal.h b/lib/telemetry/telemetry_internal.h
> index d085c492dc..d8fb37a633 100644
> --- a/lib/telemetry/telemetry_internal.h
> +++ b/lib/telemetry/telemetry_internal.h
> @@ -109,7 +109,8 @@ typedef int (*rte_log_fn)(uint32_t level, uint32_t logtype, const char *format,
>    */
>   __rte_internal
>   int
> -rte_telemetry_init(const char *runtime_dir, const char *rte_version, rte_cpuset_t *cpuset,
> +rte_telemetry_init(const char *runtime_dir, bool in_memory,
> +		const char *rte_version, rte_cpuset_t *cpuset,
>   		rte_log_fn log_fn, uint32_t registered_logtype);
>   
>   #endif
>
  
Bruce Richardson Oct. 5, 2021, 2:52 p.m. UTC | #2
On Tue, Oct 05, 2021 at 03:41:17PM +0100, Kevin Traynor wrote:
> Hi Bruce, I started reviewing v5 before v6 was pushed, so these are just
> comments from v5,
> 

No problem. Changes V6-v5 are fairly small anyway. Thanks for the review.

> On 05/10/2021 14:59, Bruce Richardson wrote:
> > When DPDK is run using "in-memory" flag, multiple processes can be run
> > using the same file-prefix and hence the same runtime directory. To
> > avoid problems with conflicting telemetry unix socket paths, we can
> > put the pid of the process into the socket name. As with the existing
> > telemetry socket files, these sockets are removed on normal program
> > exit.
> > 
> > Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
> > ---
> >   doc/guides/howto/telemetry.rst     | 17 ++++++++++++++++-
> >   lib/eal/freebsd/eal.c              |  1 +
> >   lib/eal/linux/eal.c                |  1 +
> >   lib/telemetry/telemetry.c          | 15 ++++++++++++---
> >   lib/telemetry/telemetry_internal.h |  3 ++-
> >   5 files changed, 32 insertions(+), 5 deletions(-)
> > 
> > diff --git a/doc/guides/howto/telemetry.rst b/doc/guides/howto/telemetry.rst
> > index 8f4fa1a510..8a61302459 100644
> > --- a/doc/guides/howto/telemetry.rst
> > +++ b/doc/guides/howto/telemetry.rst
> > @@ -13,12 +13,27 @@ ethdev port list, and eal parameters.
> >   Telemetry Interface
> >   -------------------
> > -The :doc:`../prog_guide/telemetry_lib` opens a socket with path
> > +For applications run normally, i.e. without the `--in-memory` EAL flag,
> > +the :doc:`../prog_guide/telemetry_lib` opens a socket with path
> >   *<runtime_directory>/dpdk_telemetry.<version>*. The version represents the
> >   telemetry version, the latest is v2. For example, a client would connect to a
> >   socket with path  */var/run/dpdk/\*/dpdk_telemetry.v2* (when the primary process
> >   is run by a root user).
> > +For applications run with the `--in-memory` EAL flag,
> > +the socket file is created with an additional suffix of the process PID.
> > +This is because multiple independent DPDK processes can be run simultaneously
> > +using the same runtime directory when *in-memory* mode is used.
> > +For example, when a user with UID 1000 runs processes with in-memory mode,
> > +we would find sockets available such as::
> > +
> > +  /run/user/1000/dpdk/rte/dpdk_telemetry.v2.1982
> > +  /run/user/1000/dpdk/rte/dpdk_telemetry.v2.1935
> > +
> 
> It seems an unnecessary step unless there is multiple process. As a
> suggestion, how about "simplifying" by always adding a check for an active
> socket with the default name. If it is not found, create it with the default
> (pre patches) name. If it is found and active, create a new one with process
> id appended. e.g.
> 
> First:
> /run/user/1000/dpdk/rte/dpdk_telemetry.v2
> 
> Next:
> /run/user/1000/dpdk/rte/dpdk_telemetry.v2.1982
> /run/user/1000/dpdk/rte/dpdk_telemetry.v2.1935
> 
> It means that existing socket can still be used by anything using telemetry.
> I think it is a nice default to keep as it doesn't require any changes for
> anything that will connect (e.g. collectd?)
> 
> The downside is that one will have a different name but it seems an
> acceptable trade-off to keep compatibility for single process case.
> 
> WDYT?
> 

Yes, that is an interesting idea, and probably not a bad one.

Taking things further, I wonder if using the pid is the best choice for a
suffix for the non-single-process case. If we used .1, .2 etc. it would
make things more predictable, perhaps easing integration for other tools.
Each process starting up would use the lowest free suffix, and any
external monitoring tools could just be set up to monitor the
first/second/third instance, with reproducable names across process
restarts.

> > +Where `/run/user/<uid>` is the runtime directory for the user given by the
> > +`$XDG_RUNTIME_DIR` environment variable,
> > +and `rte` is the default DPDK file prefix used for a runtime directory.
> > +
> >   Telemetry Initialization
> >   ------------------------
> > diff --git a/lib/eal/freebsd/eal.c b/lib/eal/freebsd/eal.c
> > index b06a2c1662..ed39d10b4e 100644
> > --- a/lib/eal/freebsd/eal.c
> > +++ b/lib/eal/freebsd/eal.c
> > @@ -952,6 +952,7 @@ rte_eal_init(int argc, char **argv)
> >   		if (tlog < 0)
> >   			tlog = RTE_LOGTYPE_EAL;
> >   		if (rte_telemetry_init(rte_eal_get_runtime_dir(),
> > +				internal_conf->in_memory | internal_conf->no_shconf,
> >   				rte_version(),
> >   				&internal_conf->ctrl_cpuset, rte_log, tlog) != 0)
> >   			return -1;
> > diff --git a/lib/eal/linux/eal.c b/lib/eal/linux/eal.c
> > index 0d0fc66668..9db4eb7913 100644
> > --- a/lib/eal/linux/eal.c
> > +++ b/lib/eal/linux/eal.c
> > @@ -1326,6 +1326,7 @@ rte_eal_init(int argc, char **argv)
> >   		if (tlog < 0)
> >   			tlog = RTE_LOGTYPE_EAL;
> >   		if (rte_telemetry_init(rte_eal_get_runtime_dir(),
> > +				internal_conf->in_memory | internal_conf->no_shconf,
> 
> should be logical OR
> 

I don't think it actually matters since any non-zero value will come
through as "true". However, will change it in v7.

/Bruce
  
Kevin Traynor Oct. 5, 2021, 3:14 p.m. UTC | #3
On 05/10/2021 15:52, Bruce Richardson wrote:
> On Tue, Oct 05, 2021 at 03:41:17PM +0100, Kevin Traynor wrote:
>> Hi Bruce, I started reviewing v5 before v6 was pushed, so these are just
>> comments from v5,
>>
> 
> No problem. Changes V6-v5 are fairly small anyway. Thanks for the review.
> 
>> On 05/10/2021 14:59, Bruce Richardson wrote:
>>> When DPDK is run using "in-memory" flag, multiple processes can be run
>>> using the same file-prefix and hence the same runtime directory. To
>>> avoid problems with conflicting telemetry unix socket paths, we can
>>> put the pid of the process into the socket name. As with the existing
>>> telemetry socket files, these sockets are removed on normal program
>>> exit.
>>>
>>> Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
>>> ---
>>>    doc/guides/howto/telemetry.rst     | 17 ++++++++++++++++-
>>>    lib/eal/freebsd/eal.c              |  1 +
>>>    lib/eal/linux/eal.c                |  1 +
>>>    lib/telemetry/telemetry.c          | 15 ++++++++++++---
>>>    lib/telemetry/telemetry_internal.h |  3 ++-
>>>    5 files changed, 32 insertions(+), 5 deletions(-)
>>>
>>> diff --git a/doc/guides/howto/telemetry.rst b/doc/guides/howto/telemetry.rst
>>> index 8f4fa1a510..8a61302459 100644
>>> --- a/doc/guides/howto/telemetry.rst
>>> +++ b/doc/guides/howto/telemetry.rst
>>> @@ -13,12 +13,27 @@ ethdev port list, and eal parameters.
>>>    Telemetry Interface
>>>    -------------------
>>> -The :doc:`../prog_guide/telemetry_lib` opens a socket with path
>>> +For applications run normally, i.e. without the `--in-memory` EAL flag,
>>> +the :doc:`../prog_guide/telemetry_lib` opens a socket with path
>>>    *<runtime_directory>/dpdk_telemetry.<version>*. The version represents the
>>>    telemetry version, the latest is v2. For example, a client would connect to a
>>>    socket with path  */var/run/dpdk/\*/dpdk_telemetry.v2* (when the primary process
>>>    is run by a root user).
>>> +For applications run with the `--in-memory` EAL flag,
>>> +the socket file is created with an additional suffix of the process PID.
>>> +This is because multiple independent DPDK processes can be run simultaneously
>>> +using the same runtime directory when *in-memory* mode is used.
>>> +For example, when a user with UID 1000 runs processes with in-memory mode,
>>> +we would find sockets available such as::
>>> +
>>> +  /run/user/1000/dpdk/rte/dpdk_telemetry.v2.1982
>>> +  /run/user/1000/dpdk/rte/dpdk_telemetry.v2.1935
>>> +
>>
>> It seems an unnecessary step unless there is multiple process. As a
>> suggestion, how about "simplifying" by always adding a check for an active
>> socket with the default name. If it is not found, create it with the default
>> (pre patches) name. If it is found and active, create a new one with process
>> id appended. e.g.
>>
>> First:
>> /run/user/1000/dpdk/rte/dpdk_telemetry.v2
>>
>> Next:
>> /run/user/1000/dpdk/rte/dpdk_telemetry.v2.1982
>> /run/user/1000/dpdk/rte/dpdk_telemetry.v2.1935
>>
>> It means that existing socket can still be used by anything using telemetry.
>> I think it is a nice default to keep as it doesn't require any changes for
>> anything that will connect (e.g. collectd?)
>>
>> The downside is that one will have a different name but it seems an
>> acceptable trade-off to keep compatibility for single process case.
>>
>> WDYT?
>>
> 
> Yes, that is an interesting idea, and probably not a bad one.
> 
> Taking things further, I wonder if using the pid is the best choice for a
> suffix for the non-single-process case. If we used .1, .2 etc. it would
> make things more predictable, perhaps easing integration for other tools.
> Each process starting up would use the lowest free suffix, and any
> external monitoring tools could just be set up to monitor the
> first/second/third instance, with reproducable names across process
> restarts.
> 

ok, cool - that sounds better again. Probably you can post and see if 
anyone else finds a hole in it or has a better idea for the next few days.

>>> +Where `/run/user/<uid>` is the runtime directory for the user given by the
>>> +`$XDG_RUNTIME_DIR` environment variable,
>>> +and `rte` is the default DPDK file prefix used for a runtime directory.
>>> +
>>>    Telemetry Initialization
>>>    ------------------------
>>> diff --git a/lib/eal/freebsd/eal.c b/lib/eal/freebsd/eal.c
>>> index b06a2c1662..ed39d10b4e 100644
>>> --- a/lib/eal/freebsd/eal.c
>>> +++ b/lib/eal/freebsd/eal.c
>>> @@ -952,6 +952,7 @@ rte_eal_init(int argc, char **argv)
>>>    		if (tlog < 0)
>>>    			tlog = RTE_LOGTYPE_EAL;
>>>    		if (rte_telemetry_init(rte_eal_get_runtime_dir(),
>>> +				internal_conf->in_memory | internal_conf->no_shconf,
>>>    				rte_version(),
>>>    				&internal_conf->ctrl_cpuset, rte_log, tlog) != 0)
>>>    			return -1;
>>> diff --git a/lib/eal/linux/eal.c b/lib/eal/linux/eal.c
>>> index 0d0fc66668..9db4eb7913 100644
>>> --- a/lib/eal/linux/eal.c
>>> +++ b/lib/eal/linux/eal.c
>>> @@ -1326,6 +1326,7 @@ rte_eal_init(int argc, char **argv)
>>>    		if (tlog < 0)
>>>    			tlog = RTE_LOGTYPE_EAL;
>>>    		if (rte_telemetry_init(rte_eal_get_runtime_dir(),
>>> +				internal_conf->in_memory | internal_conf->no_shconf,
>>
>> should be logical OR
>>
> 
> I don't think it actually matters since any non-zero value will come
> through as "true". However, will change it in v7.
> 

It was just a nit-pick, I admit it :-)

> /Bruce
>
  
Conor Walsh Oct. 5, 2021, 3:19 p.m. UTC | #4
On 05/10/2021 15:52, Bruce Richardson wrote:
> On Tue, Oct 05, 2021 at 03:41:17PM +0100, Kevin Traynor wrote:
>> Hi Bruce, I started reviewing v5 before v6 was pushed, so these are just
>> comments from v5,
>>
> No problem. Changes V6-v5 are fairly small anyway. Thanks for the review.
>
>> On 05/10/2021 14:59, Bruce Richardson wrote:
>>> When DPDK is run using "in-memory" flag, multiple processes can be run
>>> using the same file-prefix and hence the same runtime directory. To
>>> avoid problems with conflicting telemetry unix socket paths, we can
>>> put the pid of the process into the socket name. As with the existing
>>> telemetry socket files, these sockets are removed on normal program
>>> exit.
>>>
>>> Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
>>> ---

Legacy telemetry does not prevent multiple v2 sockets anymore.

Thanks,

Conor.

Tested-by: Conor Walsh <conor.walsh@intel.com>

<snip>
  
Power, Ciara Oct. 7, 2021, 1:39 p.m. UTC | #5
>-----Original Message-----
>From: Kevin Traynor <ktraynor@redhat.com>
>Sent: Tuesday 5 October 2021 16:15
>To: Richardson, Bruce <bruce.richardson@intel.com>
>Cc: dev@dpdk.org; Power, Ciara <ciara.power@intel.com>; David Marchand
><david.marchand@redhat.com>; Burakov, Anatoly <anatoly.burakov@intel.com>
>Subject: Re: [PATCH v6 3/5] telemetry: use unique socket paths for in-memory
>mode
>
>On 05/10/2021 15:52, Bruce Richardson wrote:
>> On Tue, Oct 05, 2021 at 03:41:17PM +0100, Kevin Traynor wrote:
>>> Hi Bruce, I started reviewing v5 before v6 was pushed, so these are
>>> just comments from v5,
>>>
>>
>> No problem. Changes V6-v5 are fairly small anyway. Thanks for the review.
>>
>>> On 05/10/2021 14:59, Bruce Richardson wrote:
>>>> When DPDK is run using "in-memory" flag, multiple processes can be
>>>> run using the same file-prefix and hence the same runtime directory.
>>>> To avoid problems with conflicting telemetry unix socket paths, we
>>>> can put the pid of the process into the socket name. As with the
>>>> existing telemetry socket files, these sockets are removed on normal
>>>> program exit.
>>>>
>>>> Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
>>>> ---
>>>>    doc/guides/howto/telemetry.rst     | 17 ++++++++++++++++-
>>>>    lib/eal/freebsd/eal.c              |  1 +
>>>>    lib/eal/linux/eal.c                |  1 +
>>>>    lib/telemetry/telemetry.c          | 15 ++++++++++++---
>>>>    lib/telemetry/telemetry_internal.h |  3 ++-
>>>>    5 files changed, 32 insertions(+), 5 deletions(-)
>>>>
>>>> diff --git a/doc/guides/howto/telemetry.rst
>>>> b/doc/guides/howto/telemetry.rst index 8f4fa1a510..8a61302459 100644
>>>> --- a/doc/guides/howto/telemetry.rst
>>>> +++ b/doc/guides/howto/telemetry.rst
>>>> @@ -13,12 +13,27 @@ ethdev port list, and eal parameters.
>>>>    Telemetry Interface
>>>>    -------------------
>>>> -The :doc:`../prog_guide/telemetry_lib` opens a socket with path
>>>> +For applications run normally, i.e. without the `--in-memory` EAL
>>>> +flag, the :doc:`../prog_guide/telemetry_lib` opens a socket with
>>>> +path
>>>>    *<runtime_directory>/dpdk_telemetry.<version>*. The version represents
>the
>>>>    telemetry version, the latest is v2. For example, a client would connect to a
>>>>    socket with path  */var/run/dpdk/\*/dpdk_telemetry.v2* (when the
>primary process
>>>>    is run by a root user).
>>>> +For applications run with the `--in-memory` EAL flag, the socket
>>>> +file is created with an additional suffix of the process PID.
>>>> +This is because multiple independent DPDK processes can be run
>>>> +simultaneously using the same runtime directory when *in-memory* mode
>is used.
>>>> +For example, when a user with UID 1000 runs processes with
>>>> +in-memory mode, we would find sockets available such as::
>>>> +
>>>> +  /run/user/1000/dpdk/rte/dpdk_telemetry.v2.1982
>>>> +  /run/user/1000/dpdk/rte/dpdk_telemetry.v2.1935
>>>> +
>>>
>>> It seems an unnecessary step unless there is multiple process. As a
>>> suggestion, how about "simplifying" by always adding a check for an
>>> active socket with the default name. If it is not found, create it
>>> with the default (pre patches) name. If it is found and active,
>>> create a new one with process id appended. e.g.
>>>
>>> First:
>>> /run/user/1000/dpdk/rte/dpdk_telemetry.v2
>>>
>>> Next:
>>> /run/user/1000/dpdk/rte/dpdk_telemetry.v2.1982
>>> /run/user/1000/dpdk/rte/dpdk_telemetry.v2.1935
>>>
>>> It means that existing socket can still be used by anything using telemetry.
>>> I think it is a nice default to keep as it doesn't require any
>>> changes for anything that will connect (e.g. collectd?)
>>>
>>> The downside is that one will have a different name but it seems an
>>> acceptable trade-off to keep compatibility for single process case.
>>>
>>> WDYT?
>>>
>>
>> Yes, that is an interesting idea, and probably not a bad one.
>>
>> Taking things further, I wonder if using the pid is the best choice
>> for a suffix for the non-single-process case. If we used .1, .2 etc.
>> it would make things more predictable, perhaps easing integration for other
>tools.
>> Each process starting up would use the lowest free suffix, and any
>> external monitoring tools could just be set up to monitor the
>> first/second/third instance, with reproducable names across process
>> restarts.
>>
>
>ok, cool - that sounds better again. Probably you can post and see if anyone else
>finds a hole in it or has a better idea for the next few days.

+1 for this approach of using a counter suffix, would be much simpler to use.

Thanks,
Ciara
  

Patch

diff --git a/doc/guides/howto/telemetry.rst b/doc/guides/howto/telemetry.rst
index 8f4fa1a510..8a61302459 100644
--- a/doc/guides/howto/telemetry.rst
+++ b/doc/guides/howto/telemetry.rst
@@ -13,12 +13,27 @@  ethdev port list, and eal parameters.
 Telemetry Interface
 -------------------
 
-The :doc:`../prog_guide/telemetry_lib` opens a socket with path
+For applications run normally, i.e. without the `--in-memory` EAL flag,
+the :doc:`../prog_guide/telemetry_lib` opens a socket with path
 *<runtime_directory>/dpdk_telemetry.<version>*. The version represents the
 telemetry version, the latest is v2. For example, a client would connect to a
 socket with path  */var/run/dpdk/\*/dpdk_telemetry.v2* (when the primary process
 is run by a root user).
 
+For applications run with the `--in-memory` EAL flag,
+the socket file is created with an additional suffix of the process PID.
+This is because multiple independent DPDK processes can be run simultaneously
+using the same runtime directory when *in-memory* mode is used.
+For example, when a user with UID 1000 runs processes with in-memory mode,
+we would find sockets available such as::
+
+  /run/user/1000/dpdk/rte/dpdk_telemetry.v2.1982
+  /run/user/1000/dpdk/rte/dpdk_telemetry.v2.1935
+
+Where `/run/user/<uid>` is the runtime directory for the user given by the
+`$XDG_RUNTIME_DIR` environment variable,
+and `rte` is the default DPDK file prefix used for a runtime directory.
+
 
 Telemetry Initialization
 ------------------------
diff --git a/lib/eal/freebsd/eal.c b/lib/eal/freebsd/eal.c
index b06a2c1662..ed39d10b4e 100644
--- a/lib/eal/freebsd/eal.c
+++ b/lib/eal/freebsd/eal.c
@@ -952,6 +952,7 @@  rte_eal_init(int argc, char **argv)
 		if (tlog < 0)
 			tlog = RTE_LOGTYPE_EAL;
 		if (rte_telemetry_init(rte_eal_get_runtime_dir(),
+				internal_conf->in_memory | internal_conf->no_shconf,
 				rte_version(),
 				&internal_conf->ctrl_cpuset, rte_log, tlog) != 0)
 			return -1;
diff --git a/lib/eal/linux/eal.c b/lib/eal/linux/eal.c
index 0d0fc66668..9db4eb7913 100644
--- a/lib/eal/linux/eal.c
+++ b/lib/eal/linux/eal.c
@@ -1326,6 +1326,7 @@  rte_eal_init(int argc, char **argv)
 		if (tlog < 0)
 			tlog = RTE_LOGTYPE_EAL;
 		if (rte_telemetry_init(rte_eal_get_runtime_dir(),
+				internal_conf->in_memory | internal_conf->no_shconf,
 				rte_version(),
 				&internal_conf->ctrl_cpuset, rte_log, tlog) != 0)
 			return -1;
diff --git a/lib/telemetry/telemetry.c b/lib/telemetry/telemetry.c
index bd804a25a9..24441d100b 100644
--- a/lib/telemetry/telemetry.c
+++ b/lib/telemetry/telemetry.c
@@ -51,6 +51,7 @@  static struct socket v1_socket; /* socket for v1 telemetry */
 
 static const char *telemetry_version; /* save rte_version */
 static const char *socket_dir;        /* runtime directory */
+static bool socket_uses_pid;          /* for in-memory mode, we need different socket paths */
 static rte_cpuset_t *thread_cpuset;
 static rte_log_fn rte_log_ptr;
 static uint32_t logtype;
@@ -432,8 +433,14 @@  static inline char *
 get_socket_path(const char *runtime_dir, const int version)
 {
 	static char path[PATH_MAX];
-	snprintf(path, sizeof(path), "%s/dpdk_telemetry.v%d",
-			strlen(runtime_dir) ? runtime_dir : "/tmp", version);
+	if (!socket_uses_pid)
+		snprintf(path, sizeof(path), "%s/dpdk_telemetry.v%d",
+				strlen(runtime_dir) ? runtime_dir : "/tmp", version);
+	else
+		snprintf(path, sizeof(path), "%s/dpdk_telemetry.v%d.%u",
+				strlen(runtime_dir) ? runtime_dir : "/tmp",
+				version,
+				(unsigned int)getpid());
 	return path;
 }
 
@@ -591,11 +598,13 @@  telemetry_v2_init(void)
 #endif /* !RTE_EXEC_ENV_WINDOWS */
 
 int32_t
-rte_telemetry_init(const char *runtime_dir, const char *rte_version, rte_cpuset_t *cpuset,
+rte_telemetry_init(const char *runtime_dir, bool in_memory,
+		const char *rte_version, rte_cpuset_t *cpuset,
 		rte_log_fn log_fn, uint32_t registered_logtype)
 {
 	telemetry_version = rte_version;
 	socket_dir = runtime_dir;
+	socket_uses_pid = in_memory; /* for in-memory mode use pid in sock path for uniqueness */
 	thread_cpuset = cpuset;
 	rte_log_ptr = log_fn;
 	logtype = registered_logtype;
diff --git a/lib/telemetry/telemetry_internal.h b/lib/telemetry/telemetry_internal.h
index d085c492dc..d8fb37a633 100644
--- a/lib/telemetry/telemetry_internal.h
+++ b/lib/telemetry/telemetry_internal.h
@@ -109,7 +109,8 @@  typedef int (*rte_log_fn)(uint32_t level, uint32_t logtype, const char *format,
  */
 __rte_internal
 int
-rte_telemetry_init(const char *runtime_dir, const char *rte_version, rte_cpuset_t *cpuset,
+rte_telemetry_init(const char *runtime_dir, bool in_memory,
+		const char *rte_version, rte_cpuset_t *cpuset,
 		rte_log_fn log_fn, uint32_t registered_logtype);
 
 #endif