From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga09.intel.com (mga09.intel.com [134.134.136.24]) by dpdk.org (Postfix) with ESMTP id E86CE16E for ; Wed, 6 Sep 2017 16:45:40 +0200 (CEST) Received: from fmsmga005.fm.intel.com ([10.253.24.32]) by orsmga102.jf.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 06 Sep 2017 07:45:31 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.41,484,1498546800"; d="scan'208";a="148170603" Received: from irsmsx153.ger.corp.intel.com ([163.33.192.75]) by fmsmga005.fm.intel.com with ESMTP; 06 Sep 2017 07:45:31 -0700 Received: from irsmsx102.ger.corp.intel.com ([169.254.2.59]) by IRSMSX153.ger.corp.intel.com ([169.254.9.34]) with mapi id 14.03.0319.002; Wed, 6 Sep 2017 15:45:30 +0100 From: "Van Haaren, Harry" To: Jerin Jacob CC: "dev@dpdk.org" Thread-Topic: [PATCH] eventdev: add dev id checks to config functions Thread-Index: AQHTBHs8f1RgAs9h/Eezx6DJaEWlB6KkYV4AgAPQbZA= Date: Wed, 6 Sep 2017 14:45:29 +0000 Message-ID: References: <1500900500-144237-1-git-send-email-harry.van.haaren@intel.com> <20170904052038.GA11420@jerin> In-Reply-To: <20170904052038.GA11420@jerin> Accept-Language: en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-titus-metadata-40: eyJDYXRlZ29yeUxhYmVscyI6IiIsIk1ldGFkYXRhIjp7Im5zIjoiaHR0cDpcL1wvd3d3LnRpdHVzLmNvbVwvbnNcL0ludGVsMyIsImlkIjoiYjEzMzcyNDgtZTM1Yy00MmY1LWI3Y2UtYjZlYWJhOTc2ZDIzIiwicHJvcHMiOlt7Im4iOiJDVFBDbGFzc2lmaWNhdGlvbiIsInZhbHMiOlt7InZhbHVlIjoiQ1RQX0lDIn1dfV19LCJTdWJqZWN0TGFiZWxzIjpbXSwiVE1DVmVyc2lvbiI6IjE2LjUuOS4zIiwiVHJ1c3RlZExhYmVsSGFzaCI6IjcxdTFQclBGMnlRMjdUaWxCd3drMVJWV3JaWkNXOWsybmtNamp2QlRuMlE9In0= x-ctpclassification: CTP_IC dlp-product: dlpe-windows dlp-version: 11.0.0.116 dlp-reaction: no-action x-originating-ip: [163.33.239.181] Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 Subject: Re: [dpdk-dev] [PATCH] eventdev: add dev id checks to config functions 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, 06 Sep 2017 14:45:41 -0000 > From: Jerin Jacob [mailto:jerin.jacob@caviumnetworks.com] > Sent: Monday, September 4, 2017 6:21 AM > To: Van Haaren, Harry > Cc: dev@dpdk.org > Subject: Re: [PATCH] eventdev: add dev id checks to config functions >=20 > -----Original Message----- > > Date: Mon, 24 Jul 2017 13:48:20 +0100 > > From: Harry van Haaren > > To: dev@dpdk.org > > CC: jerin.jacob@caviumnetworks.com, Harry van Haaren > > > > Subject: [PATCH] eventdev: add dev id checks to config functions > > X-Mailer: git-send-email 2.7.4 > > > > This commit adds checks to verify the device ID is valid > > to the following functions. Given that they are non-datapath, > > these checks are always performed. >=20 > Makes sense. Great - lets discuss implementation ;) > > This commit also updates the event/octeontx test-cases to > > have the correct signed-ness, as the API has changes this > > change is required in order to compile. > > > > Suggested-by: Jesse Bruni > > Signed-off-by: Harry van Haaren > > > > --- > > @@ -1288,9 +1293,10 @@ worker_ordered_flow_producer(void *arg) > > static inline int > > test_producer_consumer_ingress_order_test(int (*fn)(void *)) > > { > > - uint8_t nr_ports; > > + int16_t nr_ports; > > > > - nr_ports =3D RTE_MIN(rte_event_port_count(evdev), rte_lcore_count() -= 1); > > + nr_ports =3D RTE_MIN(rte_event_port_count(evdev), > > + (int)rte_lcore_count() - 1); >=20 > While I agree on the problem statement, I am trying to see > 1/ an API symmetrical to ethdev APIs. Similar problem solved in a differe= ntly in > ethdev. see rte_eth_dev_adjust_nb_rx_tx_desc(). > Just want to make sure, all the APIs across ethdev, eventdev looks same >=20 > 2/ How to get rid of above typecasting >=20 > Considering above two points and following the > rte_eth_dev_adjust_nb_rx_tx_desc() pattern. How about, >=20 > Removing, > rte_event_port_dequeue_depth() > rte_event_port_enqueue_depth() > rte_event_port_count() >=20 > rte_event_queue_count() > rte_event_queue_priority() >=20 > and change to, >=20 > int rte_event_port_attr_get(uint8_t dev_id, uint8_t port_id, > uint8_t *enqueue_depth /*out */, uint8_t *dequeue_depth /* out*/, uin8_t = *count /*out*/); >=20 > int rte_event_queue_attr_get(uint8_t dev_id, uint8_t port_id, > uin8_t *prio /* out */, uint8_t *count /*out */); >=20 > or something similar. Hmm, I don't like that we'd have to break ABI every time we want to add an = item to attr_get().. so adding a parameter "attr_id" would allow adding eve= nts in future. This solution feels a bit like a re-implementation of the xs= tats API.. Thoughts? -H enum { PORT_COUNT, PORT_DEQUEUE_DEPTH, PORT_ENQUEUE_DEPTH, } /* retrieve value of port=20 int rte_event_port_attr_get(uint8_t dev_id, uint8_t port_id, uint32_t attr_= id, uint32_t *attr_value /* out */);