-----Original Message----- > Date: Wed, 6 Sep 2017 14:45:29 +0000 > From: "Van Haaren, Harry" <harry.van.haa...@intel.com> > To: Jerin Jacob <jerin.ja...@caviumnetworks.com> > CC: "dev@dpdk.org" <dev@dpdk.org> > Subject: RE: [PATCH] eventdev: add dev id checks to config functions > > > From: Jerin Jacob [mailto:jerin.ja...@caviumnetworks.com] > > Sent: Monday, September 4, 2017 6:21 AM > > To: Van Haaren, Harry <harry.van.haa...@intel.com> > > Cc: dev@dpdk.org > > Subject: Re: [PATCH] eventdev: add dev id checks to config functions > > > > -----Original Message----- > > > Date: Mon, 24 Jul 2017 13:48:20 +0100 > > > From: Harry van Haaren <harry.van.haa...@intel.com> > > > To: dev@dpdk.org > > > CC: jerin.ja...@caviumnetworks.com, Harry van Haaren > > > <harry.van.haa...@intel.com> > > > 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. > > > > 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 <jesse.br...@intel.com> > > > Signed-off-by: Harry van Haaren <harry.van.haa...@intel.com> > > > > > > --- > > > @@ -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 = RTE_MIN(rte_event_port_count(evdev), rte_lcore_count() - 1); > > > + nr_ports = RTE_MIN(rte_event_port_count(evdev), > > > + (int)rte_lcore_count() - 1); > > > > While I agree on the problem statement, I am trying to see > > 1/ an API symmetrical to ethdev APIs. Similar problem solved in a > > differently in > > ethdev. see rte_eth_dev_adjust_nb_rx_tx_desc(). > > Just want to make sure, all the APIs across ethdev, eventdev looks same > > > > 2/ How to get rid of above typecasting > > > > Considering above two points and following the > > rte_eth_dev_adjust_nb_rx_tx_desc() pattern. How about, > > > > Removing, > > rte_event_port_dequeue_depth() > > rte_event_port_enqueue_depth() > > rte_event_port_count() > > > > rte_event_queue_count() > > rte_event_queue_priority() > > > > and change to, > > > > 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*/); > > > > int rte_event_queue_attr_get(uint8_t dev_id, uint8_t port_id, > > uin8_t *prio /* out */, uint8_t *count /*out */); > > > > 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 > events in future. This solution feels a bit like a re-implementation of the > xstats API.. > > Thoughts? -H > > > enum { > PORT_COUNT, > PORT_DEQUEUE_DEPTH, > PORT_ENQUEUE_DEPTH, > } > > /* retrieve value of port > int rte_event_port_attr_get(uint8_t dev_id, uint8_t port_id, uint32_t > attr_id, uint32_t *attr_value /* out */);
Looks good to me. > >
