> 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 */);