From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mails.dpdk.org (mails.dpdk.org [217.70.189.124]) by inbox.dpdk.org (Postfix) with ESMTP id 2EB39A0C4C; Tue, 5 Oct 2021 17:14:52 +0200 (CEST) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id EDCB8413BF; Tue, 5 Oct 2021 17:14:51 +0200 (CEST) Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.133.124]) by mails.dpdk.org (Postfix) with ESMTP id 1986E413BD for ; Tue, 5 Oct 2021 17:14:51 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1633446890; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=WiJBrdedinZApqmwvg7aCvCyQ3x3YimjRLs49TjPrrI=; b=A/sOqG+wJkSFq/b8/6on/yXTda3oVCal6FozTRFl5+ropX0gKYrEsLluDSLdXT/0hpksW9 ZPfz+mIatusRgb1PCgYaF2RCgxV2SL6+Bd4pNR88c2F1MCkjkgdo7LgEH3wiLy6t9auyTh j95GV9jPldB2LmflXiz+Hw/+BA+m5Rc= Received: from mail-wr1-f72.google.com (mail-wr1-f72.google.com [209.85.221.72]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-398-aO5Poq_QPVS3-HHxHIKfJQ-1; Tue, 05 Oct 2021 11:14:49 -0400 X-MC-Unique: aO5Poq_QPVS3-HHxHIKfJQ-1 Received: by mail-wr1-f72.google.com with SMTP id r16-20020adfbb10000000b00160958ed8acso4076784wrg.16 for ; Tue, 05 Oct 2021 08:14:49 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:message-id:date:mime-version:user-agent :content-language:to:cc:references:from:subject:in-reply-to :content-transfer-encoding; bh=WiJBrdedinZApqmwvg7aCvCyQ3x3YimjRLs49TjPrrI=; b=tVoYHLaIfYs9myVAvviW4xI5Lct8tL4C8ncmoGtIG0dboEpIr+LwaKlS86/HuY3kca h7hbQefRjiYEtrT5l9TVUNeo814P9nez8eGTm/FdZA/Yaf4re6YNSchOYbgKbtk0eKy9 ZpkOXDtczA88NfAoWWkKCEOw2pj50CbR9UEUmIdFz/NRNdko3P3Fwhotf1zVMpBXzLOc 1PSEgQXBBr7M0fVavLn3Z65FCxuAx6iOuQ+EGvWlIJzrugCsVSEs5ZKZPSYYiNhQrW/u dtV9LtmvTq+vJxE1gJ8I/yuNvtBjVAZSiD0xcy7YKpHlKHjIWZz7AuSvaicXtbufbjfC gPjA== X-Gm-Message-State: AOAM530tD2+d+1nWSBjoPWlru0enCFwAKDxVzlJQHfcmsGEzNI1Pv4MS 4uSlQ9RvLlcJviiEhR88nhQzm/l33HeDInHr1C0pDXbyWMzLaM4d+P5nz1GYccND0PL2UrViQoa 7ymg= X-Received: by 2002:a5d:670e:: with SMTP id o14mr496891wru.417.1633446887884; Tue, 05 Oct 2021 08:14:47 -0700 (PDT) X-Google-Smtp-Source: ABdhPJzkGKstYKW06CvSlAXqrvio4OOOD15+gguT3qeqSmyAEkEt9j6rdZ3XjUlZ9DiLAUYjOawNgA== X-Received: by 2002:a5d:670e:: with SMTP id o14mr496845wru.417.1633446887606; Tue, 05 Oct 2021 08:14:47 -0700 (PDT) Received: from [192.168.0.36] ([78.19.105.235]) by smtp.gmail.com with ESMTPSA id u1sm2084033wmc.29.2021.10.05.08.14.46 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Tue, 05 Oct 2021 08:14:46 -0700 (PDT) Message-ID: <03495416-205a-f181-adf2-52e9455e0b73@redhat.com> Date: Tue, 5 Oct 2021 16:14:45 +0100 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101 Thunderbird/91.1.0 To: Bruce Richardson Cc: dev@dpdk.org, Ciara Power , David Marchand , Anatoly Burakov References: <20210915141030.23514-1-bruce.richardson@intel.com> <20211005135909.726091-1-bruce.richardson@intel.com> <20211005135909.726091-4-bruce.richardson@intel.com> <93a5ae93-7acf-658a-4413-0e879c65e14d@redhat.com> From: Kevin Traynor In-Reply-To: Authentication-Results: relay.mimecast.com; auth=pass smtp.auth=CUSA124A263 smtp.mailfrom=ktraynor@redhat.com X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Language: en-US Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [dpdk-dev] [PATCH v6 3/5] telemetry: use unique socket paths for in-memory mode X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" 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 >>> --- >>> 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 >>> */dpdk_telemetry.*. 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/` 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 >