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 313C943FD1; Wed, 8 May 2024 08:28:32 +0200 (CEST) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 18E1B40A6E; Wed, 8 May 2024 08:28:32 +0200 (CEST) Received: from mail.lysator.liu.se (mail.lysator.liu.se [130.236.254.3]) by mails.dpdk.org (Postfix) with ESMTP id 7B8CC402AF for ; Wed, 8 May 2024 08:28:31 +0200 (CEST) Received: from mail.lysator.liu.se (localhost [127.0.0.1]) by mail.lysator.liu.se (Postfix) with ESMTP id 419CA8E3F for ; Wed, 8 May 2024 08:28:31 +0200 (CEST) Received: by mail.lysator.liu.se (Postfix, from userid 1004) id 3460A8E3E; Wed, 8 May 2024 08:28:31 +0200 (CEST) X-Spam-Checker-Version: SpamAssassin 4.0.0 (2022-12-13) on hermod.lysator.liu.se X-Spam-Level: X-Spam-Status: No, score=-1.3 required=5.0 tests=ALL_TRUSTED,AWL, T_SCC_BODY_TEXT_LINE autolearn=disabled version=4.0.0 X-Spam-Score: -1.3 Received: from [192.168.1.59] (h-62-63-215-114.A163.priv.bahnhof.se [62.63.215.114]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by mail.lysator.liu.se (Postfix) with ESMTPSA id 4F4E78DBC; Wed, 8 May 2024 08:28:29 +0200 (CEST) Message-ID: Date: Wed, 8 May 2024 08:28:29 +0200 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [RFC v2] net/af_packet: make stats reset reliable To: Stephen Hemminger , Ferruh Yigit Cc: "John W. Linville" , Thomas Monjalon , dev@dpdk.org, =?UTF-8?Q?Mattias_R=C3=B6nnblom?= , =?UTF-8?Q?Morten_Br=C3=B8rup?= References: <20240425174617.2126159-1-ferruh.yigit@amd.com> <20240426143848.2280689-1-ferruh.yigit@amd.com> <108e0c40-33e7-4eed-83de-eaedee454480@lysator.liu.se> <238675d1-b0bb-41df-8338-a1052c1a88c1@lysator.liu.se> <20240507075128.36484b4b@hermes.local> Content-Language: en-US From: =?UTF-8?Q?Mattias_R=C3=B6nnblom?= In-Reply-To: <20240507075128.36484b4b@hermes.local> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit X-Virus-Scanned: ClamAV using ClamSMTP 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 2024-05-07 16:51, Stephen Hemminger wrote: > On Tue, 7 May 2024 14:49:19 +0100 > Ferruh Yigit wrote: > >> On 5/7/2024 8:23 AM, Mattias Rönnblom wrote: >>> On 2024-04-28 17:11, Mattias Rönnblom wrote: >>>> On 2024-04-26 16:38, Ferruh Yigit wrote: >>>>> For stats reset, use an offset instead of zeroing out actual stats >>>>> values, >>>>> get_stats() displays diff between stats and offset. >>>>> This way stats only updated in datapath and offset only updated in stats >>>>> reset function. This makes stats reset function more reliable. >>>>> >>>>> As stats only written by single thread, we can remove 'volatile' >>>>> qualifier >>>>> which should improve the performance in datapath. >>>>> >>>> >>>> volatile wouldn't help you if you had multiple writers, so that can't >>>> be the reason for its removal. It would be more accurate to say it >>>> should be replaced with atomic updates. If you don't use volatile and >>>> don't use atomics, you have to consider if the compiler can reach the >>>> conclusion that it does not need to store the counter value for future >>>> use *for that thread*. Since otherwise, I don't think the store >>>> actually needs to occur. Since DPDK statistics tend to work, it's >>>> pretty obvious that current compilers tend not to reach this conclusion. >>>> >>>> If this should be done 100% properly, the update operation should be a >>>> non-atomic load, non-atomic add, and an atomic store. Similarly, for >>>> the reset, the offset store should be atomic. >>>> >>>> Considered the state of the rest of the DPDK code base, I think a >>>> non-atomic, non-volatile solution is also fine. >>>> >>>> (That said, I think we're better off just deprecating stats reset >>>> altogether, and returning -ENOTSUP here meanwhile.) >>>> >>>>> Signed-off-by: Ferruh Yigit >>>>> --- >>>>> Cc: Mattias Rönnblom >>>>> Cc: Stephen Hemminger >>>>> Cc: Morten Brørup >>>>> >>>>> This update triggered by mail list discussion [1]. >>>>> >>>>> [1] >>>>> https://inbox.dpdk.org/dev/3b2cf48e-2293-4226-b6cd-5f4dd3969f99@lysator.liu.se/ > > I would prefer that the SW statistics be handled generically by ethdev > layers and used by all such drivers. > > The most complete version of SW stats now is in the virtio driver. > If reset needs to be reliable (debatable), then it needs to be done without > atomics. Why it needs to be done without atomics? Whatever that means. In what sense should they be unreliable, needs to be documented.