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 3D5B343F48 for ; Mon, 29 Apr 2024 11:30:59 +0200 (CEST) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id E362E402A3; Mon, 29 Apr 2024 11:30:58 +0200 (CEST) Received: from mail.lysator.liu.se (mail.lysator.liu.se [130.236.254.3]) by mails.dpdk.org (Postfix) with ESMTP id BFB734025C; Mon, 29 Apr 2024 11:30:57 +0200 (CEST) Received: from mail.lysator.liu.se (localhost [127.0.0.1]) by mail.lysator.liu.se (Postfix) with ESMTP id 99DA115AC; Mon, 29 Apr 2024 11:30:56 +0200 (CEST) Received: by mail.lysator.liu.se (Postfix, from userid 1004) id 8E1B6154D; Mon, 29 Apr 2024 11:30:56 +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 DCDA514EE; Mon, 29 Apr 2024 11:30:54 +0200 (CEST) Message-ID: Date: Mon, 29 Apr 2024 11:30:54 +0200 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH] ethdev: document that stats reset APIs are not thread-safe To: =?UTF-8?Q?Morten_Br=C3=B8rup?= , Ferruh Yigit , Thomas Monjalon , Andrew Rybchenko Cc: dev@dpdk.org, stable@dpdk.org, =?UTF-8?Q?Mattias_R=C3=B6nnblom?= , Stephen Hemminger References: <20240425165308.1078454-1-ferruh.yigit@amd.com> <98CBD80474FA8B44BF855DF32C47DC35E9F3F7@smartserver.smartshare.dk> <7b783d37-281d-49eb-a58d-445841fb2c32@lysator.liu.se> <98CBD80474FA8B44BF855DF32C47DC35E9F401@smartserver.smartshare.dk> Content-Language: en-US From: =?UTF-8?Q?Mattias_R=C3=B6nnblom?= In-Reply-To: <98CBD80474FA8B44BF855DF32C47DC35E9F401@smartserver.smartshare.dk> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit X-Virus-Scanned: ClamAV using ClamSMTP X-BeenThere: stable@dpdk.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: patches for DPDK stable branches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: stable-bounces@dpdk.org On 2024-04-29 09:57, Morten Brørup wrote: >> From: Mattias Rönnblom [mailto:hofors@lysator.liu.se] >> Sent: Sunday, 28 April 2024 17.49 >> >> On 2024-04-26 14:20, Morten Brørup wrote: >>>> From: Ferruh Yigit [mailto:ferruh.yigit@amd.com] >>>> Sent: Thursday, 25 April 2024 18.53 >>>> >>>> Making 'rte_eth_stats_reset()' and 'rte_eth_xstats_reset()' APIs thread >>>> safe has performance impact on datapath. >>>> >>>> Instead document APIs as not thread safe and add condition for reliable >>>> stats reset functionality, forwarding should be stopped. >>> >> >> What does "forwarding" mean here? Sounds like something from testpmd. >> >> Isn't what needs to stop here other threads calling into the ethdev API >> for this particular device? > > Doesn't suffice. > The NIC hardware counters increment if the NIC's DMA engine receives or transmits packets. > E.g. in the RX direction, the NIC hardware counters increment according to what the NIC detects happening on the wire. If some packets are DMA'ed into memory and the associated RX descriptors are filled, the NIC's hardware counters progress. It doesn't matter if the software has looked at those RX descriptors or not. > >> >>> I'm not sure stopping forwarding suffices. >>> NIC hardware counters will keep progressing unless RX and TX is stopped at >> NIC level. >>> >> >> Why would that be an issue? > > My point is: > Unless you stop everything, including the NIC hardware RX & TX DMA, the counters might not be zero when returning from the stats_reset() functions. > The reset can still be MT safe, even though there is no way to retrieve the exact counter state prior to the reset and also is no guarantee that the counters are zero immediately after the function returns (whatever that means). If you do want to be able to do that, the means to that end is to teach the reset function to return the prior counter state as a part of the reset call, not to force the user to have the system grind to a halt. >> >>> I don't have any suggestions for a better wording, though. :-( > > I think it is fine providing a notice about MT-unsafety in the API documentation. > But the only way to prevent it is useless in real applications, as it would cause packet loss. So it's not helpful describing how to do that. > >>> >>> Anyway, better with the patch than without... >>> Acked-by: Morten Brørup >>>