From mboxrd@z Thu Jan 1 00:00:00 1970 From: "Jozef Martiniak -X (jozmarti - PANTHEON TECHNOLOGIES at Cisco)" Subject: Re: [PATCH] rte_delay_us can be replaced with user function Date: Tue, 19 Jul 2016 13:52:38 +0000 Message-ID: References: <1468932143-9321-1-git-send-email-jozmarti@cisco.com> <81613915.CvR5looQMP@xps13> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable Cc: "dev@dpdk.org" To: Thomas Monjalon Return-path: Received: from rcdn-iport-3.cisco.com (rcdn-iport-3.cisco.com [173.37.86.74]) by dpdk.org (Postfix) with ESMTP id 19A35379B for ; Tue, 19 Jul 2016 15:52:39 +0200 (CEST) In-Reply-To: <81613915.CvR5looQMP@xps13> Content-Language: en-US List-Id: patches and discussions about DPDK List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" Hi, On Tue, 2016-07-19 at 15:21 +0200, Thomas Monjalon wrote: Hi, >=20 > 2016-07-19 14:42, jozmarti@cisco.com: > > when running single-core, some drivers tend to call rte_delay_us for a > > long time, and that is causing packet drops. > > Attached patch introduces 2 new functions: > >=20 > > void rte_delay_us_callback_register(void(*userfunc)(unsigned)); > > void rte_delay_us_callback_unregister(void); > >=20 > > First one replaces rte_delay_us with userfunc and second one restores > > original rte_delay_us. >=20 > I think we could avoid the function unregister by exporting the > default implementation (let's say rte_delay_us_block). >=20 I think register/unregister is the standard way how to handle callbacks. Un= register func is now "empty" but can be extended in the future. > > +REGISTER_TEST_COMMAND(user_delay_us, test_user_delay_us); >=20 > Thanks for providing an unit test. >=20 >=20 > > --- a/lib/librte_eal/common/eal_common_timer.c > > +++ b/lib/librte_eal/common/eal_common_timer.c > > void > > rte_delay_us(unsigned us) > > { > > + if (unlikely(rte_delay_us_override !=3D NULL)) > > + { > > + rte_delay_us_override(us); > > + return; > > + } >=20 > Why not always call the registered callback and initialize it > to the default implementation (maybe using a constructor)? >=20 I wanted to touch as few things as possible with this patch.