From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga04.intel.com (mga04.intel.com [192.55.52.120]) by dpdk.org (Postfix) with ESMTP id 0F3EEAAB9 for ; Mon, 12 Mar 2018 17:22:18 +0100 (CET) X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from orsmga003.jf.intel.com ([10.7.209.27]) by fmsmga104.fm.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 12 Mar 2018 09:22:17 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.47,462,1515484800"; d="scan'208";a="34334914" Received: from fmsmsx106.amr.corp.intel.com ([10.18.124.204]) by orsmga003.jf.intel.com with ESMTP; 12 Mar 2018 09:22:16 -0700 Received: from fmsmsx120.amr.corp.intel.com (10.18.124.208) by FMSMSX106.amr.corp.intel.com (10.18.124.204) with Microsoft SMTP Server (TLS) id 14.3.319.2; Mon, 12 Mar 2018 09:22:06 -0700 Received: from fmsmsx115.amr.corp.intel.com ([169.254.4.147]) by fmsmsx120.amr.corp.intel.com ([169.254.15.251]) with mapi id 14.03.0319.002; Mon, 12 Mar 2018 09:22:06 -0700 From: "Carrillo, Erik G" To: Jerin Jacob CC: "dev@dpdk.org" Thread-Topic: [PATCH v7 1/7] eventtimer: add event timer adapter API Thread-Index: AQHTuddTAAvPxs/lt0uaFas+sZk5ZKPMxzaA Date: Mon, 12 Mar 2018 16:22:05 +0000 Message-ID: References: <1515630074-29020-1-git-send-email-erik.g.carrillo@intel.com> <1520546046-6973-1-git-send-email-erik.g.carrillo@intel.com> <1520546046-6973-2-git-send-email-erik.g.carrillo@intel.com> <20180312075332.GA7119@jerin> In-Reply-To: <20180312075332.GA7119@jerin> Accept-Language: en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: dlp-product: dlpe-windows dlp-version: 11.0.0.116 dlp-reaction: no-action x-titus-metadata-40: eyJDYXRlZ29yeUxhYmVscyI6IiIsIk1ldGFkYXRhIjp7Im5zIjoiaHR0cDpcL1wvd3d3LnRpdHVzLmNvbVwvbnNcL0ludGVsMyIsImlkIjoiYjkzMmZjMzctZWVkZi00MWU3LTk4ZjYtMGEzZDM5ZGFiYmViIiwicHJvcHMiOlt7Im4iOiJDVFBDbGFzc2lmaWNhdGlvbiIsInZhbHMiOlt7InZhbHVlIjoiQ1RQX05UIn1dfV19LCJTdWJqZWN0TGFiZWxzIjpbXSwiVE1DVmVyc2lvbiI6IjE3LjIuNS4xOCIsIlRydXN0ZWRMYWJlbEhhc2giOiIxblEwbkF6M0lkVDI0TndvZ0ZObGgwbEhSSXV0dzhFaWRwVFVjMENDU1VYUHVLc3k3MnZJTlFaMGI0WVcyVG4zIn0= x-ctpclassification: CTP_NT x-originating-ip: [10.1.200.107] Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 Subject: Re: [dpdk-dev] [PATCH v7 1/7] eventtimer: add event timer adapter API 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, 12 Mar 2018 16:22:19 -0000 Hi Jerin, Thanks for reviewing. I've responded in-line: > -----Original Message----- > From: Jerin Jacob [mailto:jerin.jacob@caviumnetworks.com] > Sent: Monday, March 12, 2018 2:54 AM > To: Carrillo, Erik G > Cc: pbhagavatula@caviumnetworks.com; dev@dpdk.org; > nipun.gupta@nxp.com; hemant.agrawal@nxp.com > Subject: Re: [PATCH v7 1/7] eventtimer: add event timer adapter API >=20 > -----Original Message----- > > Date: Thu, 8 Mar 2018 15:54:00 -0600 > > From: Erik Gabriel Carrillo > > To: pbhagavatula@caviumnetworks.com > > CC: dev@dpdk.org, jerin.jacob@caviumnetworks.com, > nipun.gupta@nxp.com, > > hemant.agrawal@nxp.com > > Subject: [PATCH v7 1/7] eventtimer: add event timer adapter API > > X-Mailer: git-send-email 1.7.10 >=20 > IMO, you can add some git commit description here. Will do. >=20 > > > > Signed-off-by: Jerin Jacob > > Signed-off-by: Pavan Nikhilesh > > Signed-off-by: Erik Gabriel Carrillo > > --- > > lib/librte_eventdev/Makefile | 1 + > > lib/librte_eventdev/rte_event_timer_adapter.h | 645 > ++++++++++++++++++++++++++ > > lib/librte_eventdev/rte_eventdev.h | 41 +- > > 3 files changed, 653 insertions(+), 34 deletions(-) create mode > > 100644 lib/librte_eventdev/rte_event_timer_adapter.h > > > > + > > +/** > > + * @warning > > + * @b EXPERIMENTAL: this structure may change without prior notice > > + * > > + * Timer adapter configuration structure */ struct > > +rte_event_timer_adapter_conf { > > + uint8_t event_dev_id; > > + /**< Event device identifier */ > > + uint16_t timer_adapter_id; > > + /**< Event timer adapter identifier */ > > + uint32_t socket_id; > > + /**< Identifer of socket from which to allocate memory for adapter > > +*/ >=20 > s/Identifer/Identifier > Will fix. =20 > > + enum rte_event_timer_adapter_clk_src clk_src; > > + /**< Clock source for timer adapter */ > > + uint64_t timer_tick_ns; > > + /**< Timer adapter resolution in ns */ > > + uint64_t max_tmo_ns; > > + /**< Maximum timer timeout(expiry) in ns */ > > + uint64_t nb_timers; > > + /**< Total number of timers per adapter */ > > + uint64_t flags; > > + /**< Timer adapter config flags (RTE_EVENT_TIMER_ADAPTER_F_*) > */ }; > > +/** > > + > > +/** > > + * @warning > > + * @b EXPERIMENTAL: this API may change without prior notice > > + * > > + * Set an event timer's initial state and initialize the event it carr= ies. > > + * > > + * @param evtim > > + * A pointer to an event timer structure. > > + */ > > +void __rte_experimental > > +rte_event_timer_init(struct rte_event_timer *evtim); >=20 > Since it can be used in fastpath, How about making it as "static inline" > function? > Any it is just setting some variables. >=20 Will do. > > + > > +/** > > + * @warning > > + * @b EXPERIMENTAL: this API may change without prior notice > > + * > > + * Arm a burst of event timers with separate expiration timeout tick > > +for each > > + * event timer. > > + * > > + * Before calling this function, the application allocates > > + * ``struct rte_event_timer`` objects from mempool or huge page > > +backed > > + * application buffers of desired size. On successful allocation, > > + * application updates the `struct rte_event_timer`` attributes such > > +as > > + * expiry event attributes, timeout ticks from now. > > + * This function submits the event timer arm requests to the event > > +timer adapter > > + * and on expiry, the events will be injected to designated event queu= e. > > + * > > + * @param adapter > > + * A pointer to an event timer adapter structure. > > + * @param evtims > > + * Pointer to an array of objects of type *rte_event_timer* structur= e. > > + * @param nb_evtims > > + * Number of event timers in the supplied array. > > + * > > + * @return > > + * The number of successfully armed event timers. The return value c= an > be less > > + * than the value of the *nb_evtims* parameter. If the return value = is > less > > + * than *nb_evtims*, the remaining event timers at the end of *evtim= s* > > + * are not consumed, and the caller has to take care of them, and > rte_errno > > + * is set accordingly. Possible errno values include: > > + * - EINVAL Invalid timer adapter, expiry event queue ID is invalid,= or an > > + * expiry event's sched type doesn't match the capabilities of the > > + * destination event queue. > > + * - EAGAIN Specified timer adapter is not running > > + * - EALREADY A timer was encountered that was already armed > > + */ > > +int __rte_experimental >=20 > To maintain the consistency across eventdev and other subsystems in DPDK. > We could return uint16_t for all the fast path functions. ie. exiting "in= t" error > return can be changed to >=20 > rte_errno =3D -EINVAL; > return 0; >=20 > This would avoid some series typecasting issues as well for the _retry_ c= ase. Sounds good. I'll change the return type to uint16_t, but the errno behavi= or seems to already be what you describe so I think we should be good there. <... snipped ...> > > - * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND > CONTRIBUTORS > > - * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT > NOT > > - * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND > FITNESS FOR > > - * A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE > COPYRIGHT > > - * OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, > INCIDENTAL, > > - * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, > BUT NOT > > - * LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; > LOSS OF USE, > > - * DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED > AND ON ANY > > - * THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR > TORT > > - * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT > OF THE USE > > - * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH > DAMAGE. > > +/* SPDX-License-Identifier: BSD-3-Clause > > + * Copyright(c) 2016 Cavium, Inc. > > + * Copyright(c) 2016-2018 Intel Corporation. > > + * Copyright(c) 2016 NXP. >=20 > Please send existing license changes as separate patch as we need to get > ACK from all vendors. >=20 Will do. > > + * All rights reserved. > > */ > > > > #ifndef _RTE_EVENTDEV_H_ > > @@ -923,8 +896,8 @@ rte_event_dev_close(uint8_t dev_id); /**< The > > event generated from ethdev subsystem */ > > #define RTE_EVENT_TYPE_CRYPTODEV 0x1 > > /**< The event generated from crypodev subsystem */ > > -#define RTE_EVENT_TYPE_TIMERDEV 0x2 > > -/**< The event generated from timerdev subsystem */ > > +#define RTE_EVENT_TYPE_TIMER 0x2 > > +/**< The event generated from event timer adapter */ > > #define RTE_EVENT_TYPE_CPU 0x3 > > /**< The event generated from cpu for pipelining. > > * Application may use *sub_event_type* to further classify the event > > -- > > 2.6.4 >=20 > Other than above minor changes, It looks really good to me. Thanks, Gabriel