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 2BB5742DBC; Mon, 3 Jul 2023 05:58:49 +0200 (CEST) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id B07A940EF0; Mon, 3 Jul 2023 05:58:48 +0200 (CEST) Received: from szxga01-in.huawei.com (szxga01-in.huawei.com [45.249.212.187]) by mails.dpdk.org (Postfix) with ESMTP id 25FE340156 for ; Mon, 3 Jul 2023 05:58:46 +0200 (CEST) Received: from dggpeml100024.china.huawei.com (unknown [172.30.72.54]) by szxga01-in.huawei.com (SkyGuard) with ESMTP id 4QvXG44VCXzqVN1; Mon, 3 Jul 2023 11:58:20 +0800 (CST) Received: from [10.67.100.224] (10.67.100.224) by dggpeml100024.china.huawei.com (7.185.36.115) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.2507.27; Mon, 3 Jul 2023 11:58:43 +0800 Subject: Re: [PATCH v5 2/2] ethdev: support xstats reset telemetry command To: Thomas Monjalon , Bruce Richardson , Dongdong Liu CC: , Ferruh Yigit , , , , References: <20221219090723.29356-1-fengchengwen@huawei.com> <4ba214e6-6e9c-a527-0027-9f98b1fec537@huawei.com> <12802674.ZYm5mLc6kN@thomas> From: fengchengwen Message-ID: Date: Mon, 3 Jul 2023 11:58:43 +0800 User-Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:68.0) Gecko/20100101 Thunderbird/68.11.0 MIME-Version: 1.0 In-Reply-To: <12802674.ZYm5mLc6kN@thomas> Content-Type: text/plain; charset="utf-8" Content-Language: en-US Content-Transfer-Encoding: 7bit X-Originating-IP: [10.67.100.224] X-ClientProxiedBy: dggems705-chm.china.huawei.com (10.3.19.182) To dggpeml100024.china.huawei.com (7.185.36.115) X-CFilter-Loop: Reflected 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 On 2023/2/20 21:05, Thomas Monjalon wrote: > 17/02/2023 10:44, fengchengwen: >> On 2023/2/16 20:54, Bruce Richardson wrote: >>> On Thu, Feb 16, 2023 at 08:42:34PM +0800, fengchengwen wrote: >>>> On 2023/2/16 20:06, Ferruh Yigit wrote: >>>>> On 2/16/2023 11:53 AM, fengchengwen wrote: >>>>>> On 2023/2/15 11:19, Dongdong Liu wrote: >>>>>>> Hi Chengwen >>>>>>> >>>>>>> On 2023/2/9 10:32, Chengwen Feng wrote: >>>>>>>> The xstats reset is useful for debugging, so add it to the ethdev >>>>>>>> telemetry command lists. >>>>>>>> >>>>>>>> Signed-off-by: Chengwen Feng >>>>>>> This patch looks good, so >>>>>>> Reviewed-by: Dongdong Liu >>>>>>> >>>>>>> A minior question >>>>>>> Do we need to support stats reset ? >>>>>> >>>>>> Stats is contained by xstats, and future direction I think is xstats. >>>>>> So I think we don't need support stats reset. >>>>>> >>>>> >>>>> I have similar question with Dongdong, readonly values are safe for >>>>> telemetry, but modifying data can be more tricky since we don't have >>>>> locking in ethdev APIs, this can cause concurrency issues. >>>> >>>> Yes, it indeed has concurrency issues. >>>> >>>>> >>>>> Overall do we want telemetry go that way and become something that >>>>> alters ethdev data/config? >>>> >>>> There are at least two part of data: config and status. >>>> For stats (which belong status data) could help for debugging, I think it's acceptable. >>>> >>>> As for concurrency issues. People should know what to do and when to do, just like >>>> the don't invoke config API (e.g. dev_configure/dev_start/...) concurrency. >>>> >>> While this is probably ok for now, I think in next release we should look >>> to add some sort of support for locking for destructive ops in a future >>> release. For example, we could: >>> >>> 1. Add support for marking a callback as "destructive" and only allow it to >>> be called if only one connection is present or >>> >>> 2. Make it possible for callbacks to query the number of connections so >>> that the callback itself is non-destructive in more than one connection is >>> open. >>> >>> [Both of these will require locking support so that new connections aren't >>> openned when the callback is in-flight!] >> >> Except telemetry, the application may have other console could execute DPDK API. >> So I think trying to keep it simple, it's up to the user to invoke. > > No, the user should not be responsible for concurrency issues. > We can ask the app developper to take care, > but not to the user who has no control on what happens in the app. > > On a more general note, I feel the expansion of telemetry is not controlled enough. > I would like to stop on adding more telemetry until we have a clear guideline > about what is telemetry for and how to use it. Hi Thomas, Should this be discussed on TB? Thanks. > > > . >