05/11/2019 13:27, Andrew Rybchenko: > On 11/5/19 3:23 PM, Ferruh Yigit wrote: > > On 11/5/2019 12:12 PM, Andrew Rybchenko wrote: > >> On 11/5/19 3:05 PM, Ferruh Yigit wrote: > >>> On 11/5/2019 11:49 AM, Andrew Rybchenko wrote: > >>>> On 11/5/19 2:36 PM, Ori Kam wrote: >>>>>> From: Ferruh Yigit <ferruh.yi...@intel.com> > >>>>>>> /** > >>>>>>> + * @internal > >>>>>>> + * Check if the selected Rx queue is hairpin queue. > >>>>>>> + * > >>>>>>> + * @param dev > >>>>>>> + * Pointer to the selected device. > >>>>>>> + * @param queue_id > >>>>>>> + * The selected queue. > >>>>>>> + * > >>>>>>> + * @return > >>>>>>> + * - (1) if the queue is hairpin queue, 0 otherwise. > >>>>>>> + */ > >>>>>>> +int > >>>>>>> +rte_eth_dev_is_rx_hairpin_queue(struct rte_eth_dev *dev, uint16_t > >>>>>> queue_id); > >>>>>>> + > >>>>>>> +/** > >>>>>>> + * @internal > >>>>>>> + * Check if the selected Tx queue is hairpin queue. > >>>>>>> + * > >>>>>>> + * @param dev > >>>>>>> + * Pointer to the selected device. > >>>>>>> + * @param queue_id > >>>>>>> + * The selected queue. > >>>>>>> + * > >>>>>>> + * @return > >>>>>>> + * - (1) if the queue is hairpin queue, 0 otherwise. > >>>>>>> + */ > >>>>>>> +int > >>>>>>> +rte_eth_dev_is_tx_hairpin_queue(struct rte_eth_dev *dev, uint16_t > >>>>>> queue_id); [...] > >>>>>> These are causing build error, thanks Jerin for catching, because they > >>>>>> are > >>>>>> internal and called by a public static inline API, so whoever calls > >>>>>> 'rte_eth_rx/tx_burst()' APIs in the shared build, can't find > >>>>>> 'rte_eth_dev_is_rx/tx_hairpin_queue()' functions [1], > >>>>>> > >>>>>> as far as I can see there are two options: > >>>>>> 1) Remove these checks > >>>>>> 2) Make 'rte_eth_dev_is_rx/tx_hairpin_queue()' public API instead of > >>>>>> internal > >>>>>> > >>>>>> If there is a value to make 'rte_eth_dev_is_rx/tx_hairpin_queue()' > >>>>>> public API > >>>>>> we > >>>>>> should go with (2) else (1). > >>>>>> > >>>>> I think we can skip the tests, > >>>>> But it was Andrew request so we must get is response. > >>>>> It was also his empathies that they should be internal. > >>>> It is important for me to keep rte_eth_dev_state internal and > >>>> few patches ago rte_eth_dev_is_rx_hairpin_queue() was inline. > >>> Are you saying you don't want to option to make > >>> 'rte_eth_dev_is_rx_hairpin_queue()' static inline because it will force > >>> the > >>> 'RTE_ETH_QUEUE_STATE_xxx' being public? > >> Yes. > > +1 > > > >>>> I'm OK to make the function experimental or keep it internal > >>>> (no API/ABI stability requirements) but externally visible (in .map). > >>> I think we can't do this, add a function deceleration to the public > >>> header file > >>> and add it to the .map file but keep it internal. Instead we can make it a > >>> proper API and it should be experimental at least first release. > >> We have discussed similar thing with Olivier recently [1]. > >> > >> [1] http://inbox.dpdk.org/dev/20191030142938.bpi4txlrebqfq7uw@platinum/ > > Yes we can say they are internal but there won't be anything preventing > > applications to use them. > > That's true, but making it internal says - don't use it. > Anyway, I have no strong opinion on experimental vs internal. > > >>> The question above was do we need this API, or instead should remove the > >>> check > >>> from rx/tx_burst APIs? > >> I think these checks are useful to ensure that these functions > >> are not used for hairpin queues. At least to catch it with debug > >> enabled. > > OK, if so what not make them proper API? Any concern on it?
Why we should not use this API in applications?
