> -----Original Message-----
> From: Andrew Rybchenko <arybche...@solarflare.com>
> Sent: Tuesday, November 5, 2019 1:49 PM
> To: Ori Kam <or...@mellanox.com>; Ferruh Yigit <ferruh.yi...@intel.com>;
> John McNamara <john.mcnam...@intel.com>; Marko Kovacevic
> <marko.kovace...@intel.com>; Thomas Monjalon <tho...@monjalon.net>
> Cc: dev@dpdk.org; jingjing...@intel.com; step...@networkplumber.org; Jerin
> Jacob <jerin.ja...@caviumnetworks.com>
> Subject: Re: [dpdk-dev] [PATCH v7 02/14] ethdev: add support for hairpin queue
>
> On 11/5/19 2:36 PM, Ori Kam wrote:
> >
> >
> >> -----Original Message-----
> >> From: Ferruh Yigit <ferruh.yi...@intel.com>
> >> Sent: Tuesday, November 5, 2019 1:25 PM
> >> To: Ori Kam <or...@mellanox.com>; John McNamara
> >> <john.mcnam...@intel.com>; Marko Kovacevic
> >> <marko.kovace...@intel.com>; Thomas Monjalon <tho...@monjalon.net>;
> >> Andrew Rybchenko <arybche...@solarflare.com>
> >> Cc: dev@dpdk.org; jingjing...@intel.com; step...@networkplumber.org;
> Jerin
> >> Jacob <jerin.ja...@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 <or...@mellanox.com>
> >>> Reviewed-by: Andrew Rybchenko <arybche...@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.
>
> 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.
> I'm OK to make the function experimental or keep it internal
> (no API/ABI stability requirements) but externally visible (in .map).
>
Just to make sure I understand you mean just to add the is_rx_hairpin_queue to
the map file right?
> >> [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
