[dpdk-dev] [PATCH v7 02/14] ethdev: add support for hairpin queue

[Ori Kam]

> -----Original Message-----
> From: Ferruh Yigit <ferruh.yigit at intel.com>
> Sent: Tuesday, November 5, 2019 1:25 PM
> To: Ori Kam <orika at mellanox.com>; John McNamara
> <john.mcnamara at intel.com>; Marko Kovacevic
> <marko.kovacevic at intel.com>; Thomas Monjalon <thomas at monjalon.net>;
> Andrew Rybchenko <arybchenko at solarflare.com>
> Cc: dev at dpdk.org; jingjing.wu at intel.com; stephen at networkplumber.org; Jerin
> Jacob <jerin.jacob at caviumnetworks.com>
> Subject: Re: [PATCH v7 02/14] ethdev: add support for hairpin queue
> 
> On 10/30/2019 11:53 PM, Ori Kam wrote:
> > This commit introduce hairpin queue type.
> >
> > The hairpin queue in build from Rx queue binded to Tx queue.
> > It is used to offload traffic coming from the wire and redirect it back
> > to the wire.
> >
> > There are 3 new functions:
> > - rte_eth_dev_hairpin_capability_get
> > - rte_eth_rx_hairpin_queue_setup
> > - rte_eth_tx_hairpin_queue_setup
> >
> > In order to use the queue, there is a need to create rte_flow
> > with queue / RSS action that targets one or more of the Rx queues.
> >
> > Signed-off-by: Ori Kam <orika at mellanox.com>
> > Reviewed-by: Andrew Rybchenko <arybchenko at solarflare.com>
> 
> <...>
> 
> >  #include <rte_ethdev_core.h>
> >
> >  /**
> > + * @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);
> > +
> > +/**
> 
> If these functions are internal why there are in 'rte_ethdev.h' ?
> 
> >   *
> >   * Retrieve a burst of input packets from a receive queue of an Ethernet
> >   * device. The retrieved packets are stored in *rte_mbuf* structures whose
> > @@ -4251,6 +4400,11 @@ int rte_eth_dev_adjust_nb_rx_tx_desc(uint16_t
> port_id,
> >  		RTE_ETHDEV_LOG(ERR, "Invalid RX queue_id=%u\n",
> queue_id);
> >  		return 0;
> >  	}
> > +	if (rte_eth_dev_is_rx_hairpin_queue(dev, queue_id)) {
> > +		RTE_ETHDEV_LOG(ERR, "Rx burst failed, queue_id=%u is
> hairpin queue\n",
> > +			       queue_id);
> > +		return 0;
> > +	}
> >  #endif
> >  	nb_rx = (*dev->rx_pkt_burst)(dev->data->rx_queues[queue_id],
> >  				     rx_pkts, nb_pkts);
> > @@ -4517,6 +4671,11 @@ static inline int
> rte_eth_tx_descriptor_status(uint16_t port_id,
> >  		RTE_ETHDEV_LOG(ERR, "Invalid TX queue_id=%u\n",
> queue_id);
> >  		return 0;
> >  	}
> > +	if (rte_eth_dev_is_tx_hairpin_queue(dev, queue_id)) {
> > +		RTE_ETHDEV_LOG(ERR, "Tx burst failed, queue_id=%u is
> hairpin queue\n",
> > +			       queue_id);
> > +		return 0;
> > +	}
> >  #endif
> 
> Hi Ori,
> 
> 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.


> 
> 
> [1]
> /usr/bin/ld: rte_event_eth_rx_adapter.o: in function `rxa_eth_rx':
> rte_event_eth_rx_adapter.c:(.text+0x1728): undefined reference to
> `rte_eth_dev_is_rx_hairpin_queue'
> /usr/bin/ld: rte_event_eth_rx_adapter.o: in function `rxa_service_func':
> rte_event_eth_rx_adapter.c:(.text+0x22ab): undefined reference to
> `rte_eth_dev_is_rx_hairpin_queue'
> /usr/bin/ld: rte_event_eth_tx_adapter.o: in function `txa_service_buffer_retry':
> rte_event_eth_tx_adapter.c:(.text+0xa43): undefined reference to
> `rte_eth_dev_is_tx_hairpin_queue'
> /usr/bin/ld: rte_event_eth_tx_adapter.o: in function `txa_service_func':
> rte_event_eth_tx_adapter.c:(.text+0xe7d): undefined reference to
> `rte_eth_dev_is_tx_hairpin_queue'
> /usr/bin/ld: rte_event_eth_tx_adapter.c:(.text+0x1155): undefined reference to
> `rte_eth_dev_is_tx_hairpin_queue'
> collect2: error: ld returned 1 exit status