From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga02.intel.com (mga02.intel.com [134.134.136.20]) by dpdk.org (Postfix) with ESMTP id D11D61023 for ; Wed, 25 Jan 2017 18:29:24 +0100 (CET) Received: from fmsmga005.fm.intel.com ([10.253.24.32]) by orsmga101.jf.intel.com with ESMTP; 25 Jan 2017 09:29:19 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.33,285,1477983600"; d="scan'208";a="57469033" Received: from irsmsx110.ger.corp.intel.com ([163.33.3.25]) by fmsmga005.fm.intel.com with ESMTP; 25 Jan 2017 09:29:19 -0800 Received: from irsmsx105.ger.corp.intel.com ([169.254.7.38]) by irsmsx110.ger.corp.intel.com ([169.254.15.101]) with mapi id 14.03.0248.002; Wed, 25 Jan 2017 17:29:18 +0000 From: "Ananyev, Konstantin" To: "Richardson, Bruce" , "Wiles, Keith" CC: Olivier MATZ , "dev@dpdk.org" Thread-Topic: [dpdk-dev] rte_ring features in use (or not) Thread-Index: AQHSdwTDXzS4gFOvp0uIXjDlqRz1PKFJLaoAgAAJRgCAAA8dgIAAFA2AgAAQIgCAAAhpEA== Date: Wed, 25 Jan 2017 17:29:18 +0000 Message-ID: <2601191342CEEE43887BDE71AB9772583F10CD7E@irsmsx105.ger.corp.intel.com> References: <20170125121456.GA24344@bricha3-MOBL3.ger.corp.intel.com> <20170125142052.7989e0ec@glumotte.dev.6wind.com> <20170125135404.GA24352@bricha3-MOBL3.ger.corp.intel.com> <20170125144809.GA26936@bricha3-MOBL3.ger.corp.intel.com> <647B0F09-667B-41D2-A8E1-964F71D4C365@intel.com> <20170125165740.GA33248@bricha3-MOBL3.ger.corp.intel.com> In-Reply-To: <20170125165740.GA33248@bricha3-MOBL3.ger.corp.intel.com> Accept-Language: en-IE, en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: 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] rte_ring features in use (or not) 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: Wed, 25 Jan 2017 17:29:25 -0000 > -----Original Message----- > From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Bruce Richardson > Sent: Wednesday, January 25, 2017 4:58 PM > To: Wiles, Keith > Cc: Olivier MATZ ; dev@dpdk.org > Subject: Re: [dpdk-dev] rte_ring features in use (or not) >=20 > On Wed, Jan 25, 2017 at 03:59:55PM +0000, Wiles, Keith wrote: > > > > > > Sent from my iPhone > > > > > On Jan 25, 2017, at 7:48 AM, Bruce Richardson wrote: > > > > > >> On Wed, Jan 25, 2017 at 01:54:04PM +0000, Bruce Richardson wrote: > > >>> On Wed, Jan 25, 2017 at 02:20:52PM +0100, Olivier MATZ wrote: > > >>> On Wed, 25 Jan 2017 12:14:56 +0000, Bruce Richardson > > >>> wrote: > > >>>> Hi all, > > >>>> > > >>>> while looking at the rte_ring code, I'm wondering if we can simpli= fy > > >>>> that a bit by removing some of the code it in that may not be used= . > > >>>> Specifically: > > >>>> > > >>>> * Does anyone use the NIC stats functionality for debugging? I've > > >>>> certainly never seen it used, and it's presence makes the rest le= ss > > >>>> readable. Can it be dropped? > > >>> > > >>> What do you call NIC stats? The stats that are enabled with > > >>> RTE_LIBRTE_RING_DEBUG? > > >> > > >> Yes. By NIC I meant ring. :-( > > >>> > > > > > >>> For the ring, in my opinion, the stats could be fully removed. > > >> > > >> That is my thinking too. For mempool, I'd wait to see the potential > > >> performance hits before deciding whether or not to enable by default= . > > >> Having them run-time enabled may also be an option too - if the bran= ches > > >> get predicted properly, there should be little to no impact as we av= oid > > >> all the writes to the stats, which is likely to be where the biggest= hit > > >> is. > > >> > > >>> > > >>> > > >>>> * RTE_RING_PAUSE_REP_COUNT is set to be disabled at build time, an= d > > >>>> so does anyone actually use this? Can it be dropped? > > >>> > > >>> This option looks like a hack to use the ring in conditions where i= t > > >>> should no be used (preemptable threads). And having a compile-time > > >>> option for this kind of stuff is not in vogue ;) > > >> > > > > > >>> > > >>> > > >>>> * Who uses the watermarks feature as is? I know we have a sample a= pp > > >>>> that uses it, but there are better ways I think to achieve the sa= me > > >>>> goal while simplifying the ring implementation. Rather than have = a > > >>>> set watermark on enqueue, have both enqueue and dequeue functions > > >>>> return the number of free or used slots available in the ring (in > > >>>> case of enqueue, how many free there are, in case of dequeue, how > > >>>> many items are available). Easier to implement and far more useful= to > > >>>> the app. > > >>> > > >>> +1 > > >>> > > > Bonus question: > > > * Do we know how widely used the enq_bulk/deq_bulk functions are? The= y > > > are useful for unit tests, so they do have uses, but I think it woul= d > > > be good if we harmonized the return values between bulk and burst > > > functions. Right now: > > > enq_bulk - only enqueues all elements or none. Returns 0 for all,= or > > > negative error for none. > > > enq_burst - enqueues as many elements as possible. Returns the num= ber > > > enqueued. > > > > I do use the apis in pktgen and the difference in return values has got= me once. Making them common would be great, but the problem is > backward compat to old versions I would need to have an ifdef in pktgen n= ow. So it seems like we moved the problem to the application. > > >=20 > Yes, an ifdef would be needed, but how many versions of DPDK back do you > support? Could the ifdef be removed again after say, 6 months? >=20 > > I would like to see the old API kept and a new API with the new behavio= r. I know it adds another API but one of the API would be nothing > more than wrapper function if not a macro. > > > > Would that be more reasonable then changing the ABI? >=20 > Technically, this would be an API rather than ABI change, since the > functions are inlined in the code. However, it's not the only API change > I'm looking to make here - I'd like to have all the functions start > returning details of the state of the ring, rather than have the > watermarks facility. If we add all new functions for this and keep the > old ones around, we are just increasing our maintenance burden. >=20 > I'd like other opinions here. Do we see increasing the API surface as > the best solution, or are we ok to change the APIs of a key library like > the rings one? I am ok with changing API to make both _bulk and _burst return the same thi= ng. Konstantin=20