Hi Andrew, @Thomas Monjalon, @Ferruh Yigit
Please comment if you have any issues with my answer. Thanks, Ori > -----Original Message----- > From: Andrew Rybchenko <arybche...@solarflare.com> > Sent: Thursday, October 3, 2019 4:26 PM > To: Ori Kam <or...@mellanox.com>; Thomas Monjalon > <tho...@monjalon.net>; Ferruh Yigit <ferruh.yi...@intel.com> > Cc: dev@dpdk.org; jingjing...@intel.com; step...@networkplumber.org > Subject: Re: [dpdk-dev] [PATCH 01/13] ethdev: support setup function for > hairpin queue > > Hi Ori, > > @Thomas, @Ferruh, please, see question below. > > On 10/2/19 3:19 PM, Ori Kam wrote: > > Hi Andrew, > > > > Sorry it took me some time to responded, (I'm on vacation 😊) > > I think we are in most cases in agreement. The only open issue is the > > checks so please see my comments below. > > As soon as we get to understanding about this issue, I will start working > > on V2. > > > > Thanks, > > Ori > > [snip] > > >>>>>>> @@ -1769,6 +1793,60 @@ int rte_eth_rx_queue_setup(uint16_t > port_id, > >>>>>> uint16_t rx_queue_id, > >>>>>>> struct rte_mempool *mb_pool); > >>>>>>> > >>>>>>> /** > >>>>>>> + * @warning > >>>>>>> + * @b EXPERIMENTAL: this API may change, or be removed, without > prior > >>>>>>> + notice > >>>>>>> + * > >>>>>>> + * Allocate and set up a hairpin receive queue for an Ethernet > >>>>>>> device. > >>>>>>> + * > >>>>>>> + * The function set up the selected queue to be used in hairpin. > >>>>>>> + * > >>>>>>> + * @param port_id > >>>>>>> + * The port identifier of the Ethernet device. > >>>>>>> + * @param rx_queue_id > >>>>>>> + * The index of the receive queue to set up. > >>>>>>> + * The value must be in the range [0, nb_rx_queue - 1] previously > supplied > >>>>>>> + * to rte_eth_dev_configure(). > >>>>>> Is any Rx queue may be setup as hairpin queue? > >>>>>> Can it be still used for regular traffic? > >>>>>> > >>>>> No if a queue is used as hairpin it can't be used for normal traffic. > >>>>> This is also why I like the idea of two different functions, in order to > create > >>>>> This distinction. > >>>> If so, do we need at least debug-level checks in Tx/Rx burst functions? > >>>> Is it required to patch rte flow RSS action to ensure that Rx queues of > >>>> only one kind are specified? > >>>> What about attempt to add Rx/Tx callbacks for hairpin queues? > >>>> > >>> I think the checks should be done in PMD level. Since from high level they > are the > >>> same. > >> Sorry, I don't understand why. If something could be checked on generic > level, > >> it should be done to avoid duplication in all drivers. > > The issue with this approach is that at the ethdev level we don't know > anything about the queue. > > This will mean that we will need to add extra functions to query the queue > type for each PMD. > > We could also assume that if to get type function exist in the pmd then the > queue is always standard queue. > > So my suggestion if you would like to move the checks is to add queue type > enum in the ethdev level, and add > > function call to query the queue type. What do you think? > > I would consider to use dev_data rx_queue_state and tx_queue_state to > keep the information to have it directly available without extra function > calls. Or add extra information. dev_data is internal and it looks like not > a problem. What do you think? > I like the new state idea, it will save some memory in the dev_data, compared to having it in the dev_data. Will also avoid extra ABI change. > >>> Call backs for Rx/Tx doesn't make sense, since the idea is to bypass the > CPU. > >> If so, I think rte_eth_add_tx_callback() should be patched to return an > >> error > >> if specified queue is hairpin. Same for Rx. > >> Any other cases? > >> > > Same answer as above. > > [snip] > > Andrew.
