>-----Original Message----- >From: Kevin Traynor <ktray...@redhat.com> >Sent: Tuesday 5 October 2021 16:15 >To: Richardson, Bruce <bruce.richard...@intel.com> >Cc: dev@dpdk.org; Power, Ciara <ciara.po...@intel.com>; David Marchand ><david.march...@redhat.com>; Burakov, Anatoly <anatoly.bura...@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.richard...@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
