From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga05.intel.com (mga05.intel.com [192.55.52.43]) by dpdk.org (Postfix) with ESMTP id 15DD9DE5 for ; Wed, 25 Jan 2017 23:27:33 +0100 (CET) Received: from orsmga005.jf.intel.com ([10.7.209.41]) by fmsmga105.fm.intel.com with ESMTP; 25 Jan 2017 14:27:32 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.33,286,1477983600"; d="scan'208";a="57258225" Received: from fmsmsx103.amr.corp.intel.com ([10.18.124.201]) by orsmga005.jf.intel.com with ESMTP; 25 Jan 2017 14:27:32 -0800 Received: from fmsmsx119.amr.corp.intel.com (10.18.124.207) by FMSMSX103.amr.corp.intel.com (10.18.124.201) with Microsoft SMTP Server (TLS) id 14.3.248.2; Wed, 25 Jan 2017 14:27:31 -0800 Received: from fmsmsx113.amr.corp.intel.com ([169.254.13.230]) by FMSMSX119.amr.corp.intel.com ([169.254.14.85]) with mapi id 14.03.0248.002; Wed, 25 Jan 2017 14:27:31 -0800 From: "Wiles, Keith" To: "Richardson, Bruce" CC: Olivier MATZ , "dev@dpdk.org" Thread-Topic: [dpdk-dev] rte_ring features in use (or not) Thread-Index: AQHSdwSwvh3h4M7sA0qJyI1ITrdeEaFJs8YAgAAJRwCAAA8cgP//jfLugACWPgCAAFwnAA== Date: Wed, 25 Jan 2017 22:27:31 +0000 Message-ID: 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-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-originating-ip: [10.255.81.198] Content-Type: text/plain; charset="us-ascii" Content-ID: <93E0D15E3B49B545B2EDAC63F80BDDAA@intel.com> 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 22:27:34 -0000 > On Jan 25, 2017, at 9:57 AM, Richardson, Bruce wrote: >=20 > On Wed, Jan 25, 2017 at 03:59:55PM +0000, Wiles, Keith wrote: >>=20 >>=20 >> Sent from my iPhone >>=20 >>> On Jan 25, 2017, at 7:48 AM, Bruce Richardson wrote: >>>=20 >>>> 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, >>>>>>=20 >>>>>> while looking at the rte_ring code, I'm wondering if we can simplify >>>>>> that a bit by removing some of the code it in that may not be used. >>>>>> Specifically: >>>>>>=20 >>>>>> * Does anyone use the NIC stats functionality for debugging? I've >>>>>> certainly never seen it used, and it's presence makes the rest less >>>>>> readable. Can it be dropped? >>>>>=20 >>>>> What do you call NIC stats? The stats that are enabled with >>>>> RTE_LIBRTE_RING_DEBUG? >>>>=20 >>>> Yes. By NIC I meant ring. :-( >>>>>=20 >>> >>>>> For the ring, in my opinion, the stats could be fully removed. >>>>=20 >>>> 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 branch= es >>>> get predicted properly, there should be little to no impact as we avoi= d >>>> all the writes to the stats, which is likely to be where the biggest h= it >>>> is. >>>>=20 >>>>>=20 >>>>>=20 >>>>>> * RTE_RING_PAUSE_REP_COUNT is set to be disabled at build time, and >>>>>> so does anyone actually use this? Can it be dropped? >>>>>=20 >>>>> This option looks like a hack to use the ring in conditions where it >>>>> should no be used (preemptable threads). And having a compile-time >>>>> option for this kind of stuff is not in vogue ;) >>>>=20 >>> >>>>>=20 >>>>>=20 >>>>>> * Who uses the watermarks feature as is? I know we have a sample app >>>>>> that uses it, but there are better ways I think to achieve the same >>>>>> 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 t= o >>>>>> the app. >>>>>=20 >>>>> +1 >>>>>=20 >>> Bonus question: >>> * Do we know how widely used the enq_bulk/deq_bulk functions are? They >>> are useful for unit tests, so they do have uses, but I think it would >>> 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 number >>> enqueued. >>=20 >> 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 co= mpat to old versions I would need to have an ifdef in pktgen now. So it see= ms like we moved the problem to the application. >>=20 >=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? I have people trying to run 2.1 and 2.2 versions of Pktgen. I can cut them = off, but I would prefer not to. >=20 >> I would like to see the old API kept and a new API with the new behavior= . I know it adds another API but one of the API would be nothing more than = wrapper function if not a macro.=20 >>=20 >> 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? >=20 > /Bruce Regards, Keith