From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga07.intel.com (mga07.intel.com [134.134.136.100]) by dpdk.org (Postfix) with ESMTP id 10AE52C66 for ; Mon, 20 Feb 2017 13:12:38 +0100 (CET) Received: from orsmga001.jf.intel.com ([10.7.209.18]) by orsmga105.jf.intel.com with ESMTP; 20 Feb 2017 04:12:37 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.35,186,1484035200"; d="scan'208";a="1100174759" Received: from irsmsx103.ger.corp.intel.com ([163.33.3.157]) by orsmga001.jf.intel.com with ESMTP; 20 Feb 2017 04:12:37 -0800 Received: from irsmsx102.ger.corp.intel.com ([169.254.2.230]) by IRSMSX103.ger.corp.intel.com ([163.33.3.157]) with mapi id 14.03.0248.002; Mon, 20 Feb 2017 12:12:36 +0000 From: "Van Haaren, Harry" To: Jerin Jacob CC: "dev@dpdk.org" , "Richardson, Bruce" Thread-Topic: [PATCH v3 04/17] eventdev: add APIs for extended stats Thread-Index: AQHSiqxL+IX3A0HZSEGuTudqSAP+vaFxvQiA Date: Mon, 20 Feb 2017 12:12:35 +0000 Message-ID: References: <1485879273-86228-1-git-send-email-harry.van.haaren@intel.com> <1487343252-16092-1-git-send-email-harry.van.haaren@intel.com> <1487343252-16092-5-git-send-email-harry.van.haaren@intel.com> <20170219123224.GC7400@localhost.localdomain> In-Reply-To: <20170219123224.GC7400@localhost.localdomain> Accept-Language: en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-titus-metadata-40: eyJDYXRlZ29yeUxhYmVscyI6IiIsIk1ldGFkYXRhIjp7Im5zIjoiaHR0cDpcL1wvd3d3LnRpdHVzLmNvbVwvbnNcL0ludGVsMyIsImlkIjoiNWEwYTA5YzgtZDJkOS00Yzc5LWEzNTktNjAxNmI4ZmU5MjIxIiwicHJvcHMiOlt7Im4iOiJDVFBDbGFzc2lmaWNhdGlvbiIsInZhbHMiOlt7InZhbHVlIjoiQ1RQX1BVQkxJQyJ9XX1dfSwiU3ViamVjdExhYmVscyI6W10sIlRNQ1ZlcnNpb24iOiIxNS45LjYuNiIsIlRydXN0ZWRMYWJlbEhhc2giOiI5M3J3aEN6enRSUGx0Vk5tTjJ6cGtONzh1UUtvMDZIZzFmUmtLVDBYNUJFPSJ9 x-ctpclassification: CTP_PUBLIC x-originating-ip: [163.33.239.180] Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 Subject: Re: [dpdk-dev] [PATCH v3 04/17] eventdev: add APIs for extended stats 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: Mon, 20 Feb 2017 12:12:39 -0000 > -----Original Message----- > From: Jerin Jacob [mailto:jerin.jacob@caviumnetworks.com] > Sent: Sunday, February 19, 2017 12:32 PM > To: Van Haaren, Harry > Cc: dev@dpdk.org; Richardson, Bruce > Subject: Re: [PATCH v3 04/17] eventdev: add APIs for extended stats > > +/** > > + * Reset the values of the xstats on the whole device. > > + * > > + * @param dev_id > > + * The identifier of the device > > + * @return > > + * - zero: successfully reset the statistics to zero > > + * - negative value: -EINVAL invalid dev_id, -ENOTSUP if not support= ed. > > + */ > > +int > > +rte_event_dev_xstats_reset(uint8_t dev_id); >=20 > I think it would be useful to selectively reset the specific counter if > needed. Ok - I like the simplicity of a single reset() covering the whole eventdev,= so that the stats should be consistent. No objection to changing the API, = as applications can do a full reset using the "partial" reset API if they r= equire. > something like below(Just to share my thought in C code) >=20 > int > rte_event_dev_xstats_reset(uint8_t dev_id, > enum rte_event_dev_xstats_mode mode/* INT_MAX to specify all xstats on t= he whole device */ > const unsigned int ids /* UINT_MAX to specify all the ids on the specifi= c mode */ =09 The other functions that take a mode require a "queue_port_id" to select th= e component (port number or queue id number). I'll add the parameter. Also I don't like the INT_MAX solution, as the enum type doesn't have to be= INT, it can be char or uintX_t - compiler and CFLAGS can be used to change= the enum type IIRC. The ids parameter is an array in the xstats_get() functions, allowing multi= ple ids to be retrieved in one call. This also allows a NULL parameter to i= ndicate all. I think this suits better than a single int id, and UINT_MAX f= or all. TL;DR: I'm suggesting int rte_event_dev_xstats_reset(uint8_t dev_id, enum rte_event_dev_xstats_mode mode, /* device, port or queue */ int16_t queue_port_id, /* Queue or Port to reset. -= 1 resets all of *mode* ports or queues. */ const uint32_t ids[]); /* NULL array indicates to r= eset all stats */ Thoughts? > Other than above comments, Its looks very good. Thanks for review!