From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-pa0-f52.google.com (mail-pa0-f52.google.com [209.85.220.52]) by dpdk.org (Postfix) with ESMTP id 352E25690 for ; Mon, 16 Mar 2015 21:11:13 +0100 (CET) Received: by pagr17 with SMTP id r17so74409269pag.0 for ; Mon, 16 Mar 2015 13:11:12 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:date:from:to:cc:subject:message-id:in-reply-to :references:mime-version:content-type:content-transfer-encoding; bh=7U5/PtOui0JAsUcfGCqUW0iHrumpAzfMZJJAVTfq2Kc=; b=GgP7IwG6uI+FojeOeI5/YUgAy9UI7/SYeTh4iM5AvO3wJUeaZQRaXF6Tz48t/nFu5T ylyNXyrF84uET04vtiP8OR7bn6lbAnwOqZ10Ssddq5EKWW9T4EWsldIaTdtYWnN0ubJa WWcoiCqpZMbsL+Ou9ruMhgBIH/0M7hVWo9ehuJtyfCCX9AGVzO+skzR9HQWwaGSC5WgE SnAagyB4RLI6rB8yWQmUCBIiyL9hjktACemWrr8iPXZRV8HfQnqX7v+EwssIXpx4BFW9 JC4gKUqz4r+gQvBOylex7mvUAhAYGPQQGJRIQMr+Eyknrp1nEq5VFWxzj9366D7NxvVM VlBg== X-Gm-Message-State: ALoCoQneut0oAwUVI1wzCsUq6qfO+QaMMIQ4bL1dyjAbXH2T1xZkbYBcyYLfiJTNenWYJzI5CueQ X-Received: by 10.66.227.169 with SMTP id sb9mr103633961pac.11.1426536672527; Mon, 16 Mar 2015 13:11:12 -0700 (PDT) Received: from urahara (static-50-53-82-155.bvtn.or.frontiernet.net. [50.53.82.155]) by mx.google.com with ESMTPSA id t5sm18672797pdn.58.2015.03.16.13.11.11 (version=TLSv1.2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Mon, 16 Mar 2015 13:11:12 -0700 (PDT) Date: Mon, 16 Mar 2015 13:11:12 -0700 From: Stephen Hemminger To: "Dumitrescu, Cristian" Message-ID: <20150316131112.6a945025@urahara> In-Reply-To: <3EB4FA525960D640B5BDFFD6A3D8912632323539@IRSMSX108.ger.corp.intel.com> References: <1426004018-25948-1-git-send-email-stephen@networkplumber.org> <1426004018-25948-6-git-send-email-stephen@networkplumber.org> <3EB4FA525960D640B5BDFFD6A3D89126323225F4@IRSMSX108.ger.corp.intel.com> <20150312160626.47315f49@urahara> <3EB4FA525960D640B5BDFFD6A3D8912632323539@IRSMSX108.ger.corp.intel.com> MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Cc: "dev@dpdk.org" , Stephen Hemminger Subject: Re: [dpdk-dev] [PATCH v2 5/6] rte_sched: allow reading statistics without clearing X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: patches and discussions about DPDK List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 16 Mar 2015 20:11:13 -0000 On Mon, 16 Mar 2015 19:58:51 +0000 "Dumitrescu, Cristian" wrote: > > > > -----Original Message----- > > From: Stephen Hemminger [mailto:stephen@networkplumber.org] > > Sent: Thursday, March 12, 2015 11:06 PM > > To: Dumitrescu, Cristian > > Cc: dev@dpdk.org; Stephen Hemminger > > Subject: Re: [PATCH v2 5/6] rte_sched: allow reading statistics without > > clearing > > > > On Thu, 12 Mar 2015 19:28:11 +0000 > > "Dumitrescu, Cristian" wrote: > > > > > Hi Stephen, > > > > > > Thank you for adding flexibility over clearing the stats or not. > > > > > > I have one concern though: why change the stats read API to add a clear > > parameter rather than keep prototype for the stats functions unchanged and > > add the flag as part of the port creation parameters in struct > > rte_sched_port_params? This parameter could be saved into the internal > > struct rte_sched_port, which is passed (as a pointer) to the stats read > > functions. In my opinion, this approach is slightly more elegant and it keeps > > the changes to a minimum. > > > > > > int > > > rte_sched_queue_read_stats(struct rte_sched_port *port, > > > uint32_t queue_id, > > > struct rte_sched_queue_stats *stats, > > > uint16_t *qlen) > > > { > > > ... > > > if (port->clear_stats_on_read) > > > memset(...); > > > } > > > > > > I made this suggestion during the previous round, but I did not get any > > opinion from you on it yet. > > > > > > Regards, > > > Cristian > > > > I rejected the config parameter idea because I don't like it is inconsistent > > with other statistics in DPDK and in related software. There is not a > > config parameter that changes what BSD or Linux kernel API does. > > Your approach has the advantage of being able to clear/not clear the stats per each read operation rather than configuring the behavior globally. I think this approach allows for the ultimate flexibility, so I am OK to go with it. > > > > > The only reason for keeping the read and clear in one operation is > > because you like it, and there are somebody might have built code > > that expects it. > > > > Clearing the stats with a delay after the stats have been read is prone to a race condition, as during this time more packets could be processed, and these packets will not show up in the counters that the user read. > I think it depends on the need of each particular application whether this race condition is important or not: if the counters are read rarely (e.g. once per day) and only course accuracy is required, the error is probably irrelevant; if the app is looking for fine great accuracy (e.g. rate measurement, debugging, etc), then the error is not allowed. You seem to favour the former and ignore the later case. > > > > Changing the function signature is a nice red flag so that people will > > notice at change. > > There is a small API change here. I am OK with it for the above reasons, provided that there are no objections on this from other contributors. > If you really wanted a fast/safe read and clear operation, how about using exchange instruction to exchange in a zero?