On 11/5/19 4:27 PM, Thomas Monjalon wrote: > 05/11/2019 14:23, Ori Kam: >> From: Thomas Monjalon <tho...@monjalon.net> >>> 05/11/2019 13:53, Andrew Rybchenko: >>>> On 11/5/19 3:51 PM, Thomas Monjalon wrote: >>>>> 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. > Using an experimental function in an inline API may propagate the > experimental flag further. > >> Moving the API to experimental results in the following error: >> error: 'rte_eth_dev_is_rx_hairpin_queue' is deprecated (declared at >> /.autodirect/mtrswgwork/orika/pegasus04_share/dpdk.org/x86_64-native-linuxapp-gcc/include/rte_ethdev.h:4276): >> Symbol is not yet part of stable ABI [-Werror=deprecated-declarations] >> >> I suggest that we remove the checks, in any case this checks are only on >> debug mode, and when using data path the user must be very careful and what >> he >> is doing. > I agree it seems better to remove these checks for now. > We can decide to re-introduce them later as non-experimental.
OK
