From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga03.intel.com (mga03.intel.com [134.134.136.65]) by dpdk.org (Postfix) with ESMTP id ED72E2B9A for ; Wed, 18 Oct 2017 23:48:06 +0200 (CEST) Received: from fmsmga006.fm.intel.com ([10.253.24.20]) by orsmga103.jf.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 18 Oct 2017 14:48:05 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.43,398,1503385200"; d="scan'208";a="164723470" Received: from fmsmsx107.amr.corp.intel.com ([10.18.124.205]) by fmsmga006.fm.intel.com with ESMTP; 18 Oct 2017 14:48:05 -0700 Received: from fmsmsx115.amr.corp.intel.com ([169.254.4.50]) by fmsmsx107.amr.corp.intel.com ([169.254.6.9]) with mapi id 14.03.0319.002; Wed, 18 Oct 2017 14:48:05 -0700 From: "Carrillo, Erik G" To: Pavan Nikhilesh Bhagavatula CC: "dev@dpdk.org" Thread-Topic: [RFC PATCH v2 0/1] eventtimer: introduce event timer adapter Thread-Index: AQHTPFU9hJTRCPtWNUuCjLI3ncbr+qLcAGiAgArqJoCAAAkXgIADQkbw Date: Wed, 18 Oct 2017 21:48:04 +0000 Message-ID: References: <20170817161104.24293-1-jerin.jacob@caviumnetworks.com> <1506093431-57588-1-git-send-email-erik.g.carrillo@intel.com> <20171003143723.GB10493@jerin> <20171016120436.GA20698@PBHAGAVATULA-LT> <20171016123708.GA21136@PBHAGAVATULA-LT> In-Reply-To: <20171016123708.GA21136@PBHAGAVATULA-LT> 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: eyJDYXRlZ29yeUxhYmVscyI6IiIsIk1ldGFkYXRhIjp7Im5zIjoiaHR0cDpcL1wvd3d3LnRpdHVzLmNvbVwvbnNcL0ludGVsMyIsImlkIjoiYmVjNWU5YzAtNzcwZS00MzU2LWI0ZDYtZTFmODcxNWRmZWIzIiwicHJvcHMiOlt7Im4iOiJDVFBDbGFzc2lmaWNhdGlvbiIsInZhbHMiOlt7InZhbHVlIjoiQ1RQX0lDIn1dfV19LCJTdWJqZWN0TGFiZWxzIjpbXSwiVE1DVmVyc2lvbiI6IjE3LjIuNS4xOCIsIlRydXN0ZWRMYWJlbEhhc2giOiJlZ2NBcHk0ODlwQlU0bndIcDNyT1lmTktnenJoUVVsTmFGMmpMQ3hVZG8rQnltUlB2RDJBTitFSVpnUmVReDBtIn0= x-ctpclassification: CTP_IC x-originating-ip: [10.1.200.108] Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 Subject: Re: [dpdk-dev] [RFC PATCH v2 0/1] eventtimer: introduce event timer adapter 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, 18 Oct 2017 21:48:07 -0000 > -----Original Message----- > From: Pavan Nikhilesh Bhagavatula > [mailto:pbhagavatula@caviumnetworks.com] > Sent: Monday, October 16, 2017 7:37 AM > To: Carrillo, Erik G > Cc: dev@dpdk.org > Subject: Re: [RFC PATCH v2 0/1] eventtimer: introduce event timer adapter >=20 > On Mon, Oct 16, 2017 at 05:34:36PM +0530, Pavan Nikhilesh Bhagavatula > wrote: > > On Mon, Oct 09, 2017 at 08:30:35PM +0000, Carrillo, Erik G wrote: > > > > > > > > > > The second big change is to replace API parameters specifying > > > > > pointer to rte_event_timer_adapter with ids, which seems more > > > > > common throughout DPDK. > > > > > > > > > > Other changes include: > > > > > - removed rte_event_timer_adapter_lookup() function, since APIs > > > > > no > > > > longer > > > > > accept pointer to rte_event_timer_adapter > > > > > > > > There is one difference between ethdev rx adapter, where we have > > > > rte_event_timer_arm_burst(), > rte_event_timer_arm_tmo_tick_burst(), > > > > rte_event_timer_cancel_burst(), > > > > APIs in fastpath. So any multi process scheme where resolving the > > > > fastpath API functions in _one_ or zero redirection is fine. > > > > > > I see, makes sense. > > > > > > > > > > > I guess in we may need 2 or 3 indirection to resolve the fastpath > > > > functions with id scheme. Please choose scheme with one 1 or no > redirection. > > > > I think, > > > > - By allocating adapter memory from the heap and > > > > > > Just to check, are you talking about the heap (e.g., rte_malloc) in > hugepage memory? > > > > Yes, If we use rte_memzone_reserve and create the adapter in the > > primary process, the name can be used to do a lookup in the secondary. > > > > > > > - adapter hold the function pointers for multi process and > > > > - mempool kind of pointer object scheme without id and lookup() > > > > Can resolve function pointers without any indirection. > > > > > > > > > > Can you elaborate here? In the mempool implementation, it looks like > there's a per-process array of ops objects, and each mempool instance has > an index into the array that it uses to select the ops object to call thr= ough. > Since the ops structures are initialized per-process, they get function > pointers valid in that process - which relates to the second point above.= If the > adapter holds function pointers for a primary process (for example), they= 'll > be invalid in the secondary. Am I missing something? > > > > > > > Once adapter is created using memzone in the Primary process, the > > secondary process can use the memzone name to get the pointer to the > > adapter and use data such as ops_id stored in the memzone to fixup the > fastpath function pointers. > > The slowpath could still use ops_id based function pointers(mempool). > > > > > Thanks, > > > Gabriel > > > > > > > So please analyze on those lines as well. > > > > > > > > > > > > > > > Pavan >=20 > Adding to this >=20 > Have a two level scheme such as: >=20 > --Allocate this structure from a global array based on the adapter ID: >=20 > struct rte_event_timer_wheel { > event_timer_arm_burst arm_burst; > /**< Pointer to driver arm_burst function. */ > event_timer_arm_tmo_tick_burst arm_tmo_tick_burst; > /**< Pointer to driver arm_tmo_tick_burst function. */ > event_timer_cancel_burst cancel_burst; > /**< Pointer to driver cancel function. */ >=20 > --Allocate the data section using memzone reserve. > struct rte_event_timer_wheel_data *data; >=20 >=20 > /**< Pointer to timer wheel data*/ > struct rte_event_timer_wheel_ops *ops; > /**< Driver specific ops */ > } __rte_cache_aligned; >=20 > /** > * Timer wheel data structure. > */ > struct rte_event_timer_wheel_data { > void *wheel_priv; > /**< Timer wheel private data*/ > uint8_t ops_index; > /**< Timer wheel ops index id. */ > uint8_t wheel_id; > /**< Timer wheel id. */ > uint8_t event_port_id; > /**< Optional Event port used when the inbuilt port is absent.*/ > const struct rte_memzone *mz; > /**< Timer wheel memzone pointer. */ > struct rte_event_timer_wheel_config conf; > /**< Configuration used to configure the wheel. */ >=20 > RTE_STD_C11 > uint8_t started : 1; > /**< Flag to indicate wheel started. */ } __rte_cache_aligned; >=20 > /* Primary process */ > *_create(wheel_id, ...) > { > struct rte_event_timer_wheel *wl; >=20 > /* do checks */ > ... >=20 > wl =3D &rte_event_timer_wheels[wheel_id]; >=20 > ... > /* Get ops ID based on eventdev */ > ... > ret =3D rte_event_dev_get_timer_wheel_capability(event_dev_id, > &flags, > &ops_id); >=20 > mz =3D rte_memzone_reserve(name, ...); > ... > /* fill in data */ > data =3D mz->addr; > ... >=20 > data =3D ops_id; > ... > /* Call the driver specific functions. */ } >=20 > /*Secondaty process*/ > struct rte_event_timer_wheel * > rte_event_timer_wheel_lookup(uint16_t wheel_id) { > ... > struct rte_event_timer_wheel *wl; > struct rte_event_timer_wheel_data *data; > ... >=20 > /* Do checks*/ > ... > /* Look up memzone created in primary. */ > mz =3D rte_memzone_lookup(name); >=20 > data =3D mz->addr; >=20 > /* Setup up global array in secondary */ > wl =3D &rte_event_timer_wheels[data->wheel_id]; > wl->data =3D data; > ... > /* Use the ops_id set in primary to get the ops in secondary*/ > wl->ops =3D rte_event_timer_wheel_get_ops(data->ops_index); >=20 > /* Reset the ops*/ > (*wl->ops->set_ops)(wl); >=20 > return wl; > } >=20 > Hope this clears few things up. Hi Pavan, OK, I think I see what you mean. We could have a global table of ops struc= ts (one per plugin), where each entry contained function pointers valid in = that process. We could save the ops_index in the shared data portion of th= e timer wheel, and in create() or lookup(), we could use that index to retr= ieve the ops struct, which could contain an operation to set the fastpath p= ointers for the timer wheel instance. I assume the *lookup() function wou= ld rely on the memzone having a predictable name (e.g. "timer_wheel_memzone= _") so that it could locate it knowing only the wheel_id.=20 Thanks for the info, Gabriel