From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga04.intel.com (mga04.intel.com [192.55.52.120]) by dpdk.org (Postfix) with ESMTP id 1B2611B154 for ; Fri, 28 Sep 2018 00:05:25 +0200 (CEST) X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from fmsmga007.fm.intel.com ([10.253.24.52]) by fmsmga104.fm.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 27 Sep 2018 15:05:24 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.54,312,1534834800"; d="scan'208";a="73616156" Received: from fyigit-mobl.ger.corp.intel.com (HELO [10.252.3.51]) ([10.252.3.51]) by fmsmga007.fm.intel.com with ESMTP; 27 Sep 2018 15:04:15 -0700 To: Dan Gora Cc: dev@dpdk.org, Igor Ryzhov , Stephen Hemminger References: <20180911232906.18352-1-dg@adax.com> <20180919195549.5585-1-dg@adax.com> <20180919195549.5585-6-dg@adax.com> <1bc03874-f981-f5f2-e071-76c4fc1da96e@intel.com> From: Ferruh Yigit Openpgp: preference=signencrypt Message-ID: Date: Thu, 27 Sep 2018 23:04:15 +0100 User-Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:52.0) Gecko/20100101 Thunderbird/52.9.1 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit Subject: Re: [dpdk-dev] [PATCH v2 5/5] examples/kni: improve zeroing statistics X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 27 Sep 2018 22:05:26 -0000 On 9/27/2018 4:53 PM, Dan Gora wrote: > On Thu, Sep 27, 2018 at 8:40 AM Ferruh Yigit wrote: >>>> Not sure about introducing an extra check to datapath for possible error on >>>> stats zero. I am for dropping this patch, what do you think? >>> >>> This is literally adding one instruction to the datapath. Not even an >>> atomic instruction. There is no effect on the performance caused by >>> this change. >>> >>> Is that not better than the user (like me who experienced this) >>> wondering why they cannot zero the counters even when there is no >>> traffic? >> >> Can we have something like, stop the forwarding, clear the stats and start >> forwarding back? > > This is what is broken. The stats do not zero because you have the > two worker threads who are constantly performing: > > kni_stats[port_id].rx_packets += num; > > and > > kni_stats[port_id].tx_packets += nb_tx; > > You know how read/inc/write races work, right? > > These are not atomic increments, so the other thread zeroing these > counters is _always_ going to race with these worker threads > overwriting the counters with the old values. > > With no traffic it's worse because the worker threads perform these > increments even more often! Dear Dan, Your implementation doesn't prevent the race when there is traffic, it can be useful when there is no traffic, that is why my suggestion was in signal handler, stop worker threads, zero stats, start workers again. But for traffic case this will cause packet lost so overall may not be desirable. > >> Yes effect is small but it is for each packet, it doesn't make sense to me to >> add extra check for each packet for the rare case of stats reset. > > Its not rare at all! You cannot zero the statistics around 80% of the > time on my machine when there is no traffic. It's trivial to > reproduce this. Just run a little traffic, stop the traffic, zero the > stats and check the stats. I was trying to say, packet transfer is millions per second, so the waste is in that scale, but clear stats will be called in the scale of seconds/minutes? Or not at all perhaps for some cases? > > If you cannot zero the statistics reliably under any circumstance then > the statistics themselves are worthless and should be removed. It's > better to have no stats than completely unreliable ones. How much really you rely on sample application stats clear, and stats are still useful of course since you can use them without reset.