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.
