[v7,02/14] ethdev: add support for hairpin queue
Checks
Commit Message
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@mellanox.com>
Reviewed-by: Andrew Rybchenko <arybchenko@solarflare.com>
---
V7:
- Move is_rx/tx_hairpin_queue to ethdev.c and ethdev.h also remove the inline.
- change checks for max number of hairpin queues.
- modify log messages.
V6:
- change comparision of rte_eth_dev_is_tx/tx_hairpin_queue to boolean.
- add hairpin to release note.
V5:
- add function to check if queue is hairpin queue.
- modify log messages to be more distinct.
- update log messages to be only on one line.
- change peer_n to peer_count.
V4:
- update according to ML comments.
V3:
- update according to ML comments.
V2:
- update according to ML comments
---
doc/guides/rel_notes/release_19_11.rst | 5 +
lib/librte_ethdev/rte_ethdev.c | 232 +++++++++++++++++++++++++++++++
lib/librte_ethdev/rte_ethdev.h | 171 ++++++++++++++++++++++-
lib/librte_ethdev/rte_ethdev_core.h | 91 +++++++++++-
lib/librte_ethdev/rte_ethdev_driver.h | 1 +
lib/librte_ethdev/rte_ethdev_version.map | 3 +
6 files changed, 495 insertions(+), 8 deletions(-)
Comments
On 10/31/19 2:53 AM, 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@mellanox.com>
> Reviewed-by: Andrew Rybchenko <arybchenko@solarflare.com>
LGTM, one note below which could be fixed on applying
> diff --git a/lib/librte_ethdev/rte_ethdev_version.map b/lib/librte_ethdev/rte_ethdev_version.map
> index e59d516..48b5389 100644
> --- a/lib/librte_ethdev/rte_ethdev_version.map
> +++ b/lib/librte_ethdev/rte_ethdev_version.map
> @@ -288,4 +288,7 @@ EXPERIMENTAL {
> rte_eth_rx_burst_mode_get;
> rte_eth_tx_burst_mode_get;
> rte_eth_burst_mode_option_name;
> + rte_eth_rx_hairpin_queue_setup;
> + rte_eth_tx_hairpin_queue_setup;
> + rte_eth_dev_hairpin_capability_get;
> };
As I understand rte_eth_dev_is_rx_hairpin_queue() and
rte_eth_dev_is_tx_hairpin_queue() should be listed above.
Yes, these functions are internal, but used in header
inline functions and should be visible outside.
We have discussed similar case recently in [1].
[1] http://inbox.dpdk.org/dev/20191030142938.bpi4txlrebqfq7uw@platinum/
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@mellanox.com>
> Reviewed-by: Andrew Rybchenko <arybchenko@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).
[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
> -----Original Message-----
> From: Ferruh Yigit <ferruh.yigit@intel.com>
> Sent: Tuesday, November 5, 2019 1:25 PM
> To: Ori Kam <orika@mellanox.com>; John McNamara
> <john.mcnamara@intel.com>; Marko Kovacevic
> <marko.kovacevic@intel.com>; Thomas Monjalon <thomas@monjalon.net>;
> Andrew Rybchenko <arybchenko@solarflare.com>
> Cc: dev@dpdk.org; jingjing.wu@intel.com; stephen@networkplumber.org; Jerin
> Jacob <jerin.jacob@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@mellanox.com>
> > Reviewed-by: Andrew Rybchenko <arybchenko@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
On 11/5/19 2:36 PM, Ori Kam wrote:
>
>
>> -----Original Message-----
>> From: Ferruh Yigit <ferruh.yigit@intel.com>
>> Sent: Tuesday, November 5, 2019 1:25 PM
>> To: Ori Kam <orika@mellanox.com>; John McNamara
>> <john.mcnamara@intel.com>; Marko Kovacevic
>> <marko.kovacevic@intel.com>; Thomas Monjalon <thomas@monjalon.net>;
>> Andrew Rybchenko <arybchenko@solarflare.com>
>> Cc: dev@dpdk.org; jingjing.wu@intel.com; stephen@networkplumber.org; Jerin
>> Jacob <jerin.jacob@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@mellanox.com>
>>> Reviewed-by: Andrew Rybchenko <arybchenko@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).
>> [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
> -----Original Message-----
> From: Andrew Rybchenko <arybchenko@solarflare.com>
> Sent: Tuesday, November 5, 2019 1:49 PM
> To: Ori Kam <orika@mellanox.com>; Ferruh Yigit <ferruh.yigit@intel.com>;
> John McNamara <john.mcnamara@intel.com>; Marko Kovacevic
> <marko.kovacevic@intel.com>; Thomas Monjalon <thomas@monjalon.net>
> Cc: dev@dpdk.org; jingjing.wu@intel.com; stephen@networkplumber.org; Jerin
> Jacob <jerin.jacob@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.yigit@intel.com>
> >> Sent: Tuesday, November 5, 2019 1:25 PM
> >> To: Ori Kam <orika@mellanox.com>; John McNamara
> >> <john.mcnamara@intel.com>; Marko Kovacevic
> >> <marko.kovacevic@intel.com>; Thomas Monjalon <thomas@monjalon.net>;
> >> Andrew Rybchenko <arybchenko@solarflare.com>
> >> Cc: dev@dpdk.org; jingjing.wu@intel.com; stephen@networkplumber.org;
> Jerin
> >> Jacob <jerin.jacob@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@mellanox.com>
> >>> Reviewed-by: Andrew Rybchenko <arybchenko@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
On 11/5/2019 11:49 AM, Andrew Rybchenko wrote:
> On 11/5/19 2:36 PM, Ori Kam wrote:
>>
>>
>>> -----Original Message-----
>>> From: Ferruh Yigit <ferruh.yigit@intel.com>
>>> Sent: Tuesday, November 5, 2019 1:25 PM
>>> To: Ori Kam <orika@mellanox.com>; John McNamara
>>> <john.mcnamara@intel.com>; Marko Kovacevic
>>> <marko.kovacevic@intel.com>; Thomas Monjalon <thomas@monjalon.net>;
>>> Andrew Rybchenko <arybchenko@solarflare.com>
>>> Cc: dev@dpdk.org; jingjing.wu@intel.com; stephen@networkplumber.org; Jerin
>>> Jacob <jerin.jacob@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@mellanox.com>
>>>> Reviewed-by: Andrew Rybchenko <arybchenko@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.
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?
> 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.
The question above was do we need this API, or instead should remove the check
from rx/tx_burst APIs?
>
>>> [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
>
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:
>>>
>>>
>>>> -----Original Message-----
>>>> From: Ferruh Yigit <ferruh.yigit@intel.com>
>>>> Sent: Tuesday, November 5, 2019 1:25 PM
>>>> To: Ori Kam <orika@mellanox.com>; John McNamara
>>>> <john.mcnamara@intel.com>; Marko Kovacevic
>>>> <marko.kovacevic@intel.com>; Thomas Monjalon <thomas@monjalon.net>;
>>>> Andrew Rybchenko <arybchenko@solarflare.com>
>>>> Cc: dev@dpdk.org; jingjing.wu@intel.com; stephen@networkplumber.org; Jerin
>>>> Jacob <jerin.jacob@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@mellanox.com>
>>>>> Reviewed-by: Andrew Rybchenko <arybchenko@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.
>
> 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.
>> 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.
We have discussed similar thing with Olivier recently [1].
[1] http://inbox.dpdk.org/dev/20191030142938.bpi4txlrebqfq7uw@platinum/
> The question above was do we need this API, or instead should remove the check
> from rx/tx_burst APIs?
I think these checks are useful to ensure that these functions
are not used for hairpin queues. At least to catch it with debug
enabled.
>>>> [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
>>
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:
>>>>
>>>>
>>>>> -----Original Message-----
>>>>> From: Ferruh Yigit <ferruh.yigit@intel.com>
>>>>> Sent: Tuesday, November 5, 2019 1:25 PM
>>>>> To: Ori Kam <orika@mellanox.com>; John McNamara
>>>>> <john.mcnamara@intel.com>; Marko Kovacevic
>>>>> <marko.kovacevic@intel.com>; Thomas Monjalon <thomas@monjalon.net>;
>>>>> Andrew Rybchenko <arybchenko@solarflare.com>
>>>>> Cc: dev@dpdk.org; jingjing.wu@intel.com; stephen@networkplumber.org; Jerin
>>>>> Jacob <jerin.jacob@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@mellanox.com>
>>>>>> Reviewed-by: Andrew Rybchenko <arybchenko@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.
>>
>> 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.
>
> We have discussed similar thing with Olivier recently [1].
>
> [1] http://inbox.dpdk.org/dev/20191030142938.bpi4txlrebqfq7uw@platinum/
Yes we can say they are internal but there won't be anything preventing
applications to use them.
>
>> The question above was do we need this API, or instead should remove the check
>> from rx/tx_burst APIs?
>
> I think these checks are useful to ensure that these functions
> are not used for hairpin queues. At least to catch it with debug
> enabled.
OK, if so what not make them proper API? Any concern on it?
>
>>>>> [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
>>>
>
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:
>>>>>
>>>>>> -----Original Message-----
>>>>>> From: Ferruh Yigit <ferruh.yigit@intel.com>
>>>>>> Sent: Tuesday, November 5, 2019 1:25 PM
>>>>>> To: Ori Kam <orika@mellanox.com>; John McNamara
>>>>>> <john.mcnamara@intel.com>; Marko Kovacevic
>>>>>> <marko.kovacevic@intel.com>; Thomas Monjalon <thomas@monjalon.net>;
>>>>>> Andrew Rybchenko <arybchenko@solarflare.com>
>>>>>> Cc: dev@dpdk.org; jingjing.wu@intel.com; stephen@networkplumber.org; Jerin
>>>>>> Jacob <jerin.jacob@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@mellanox.com>
>>>>>>> Reviewed-by: Andrew Rybchenko <arybchenko@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.
>>> 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.
>> We have discussed similar thing with Olivier recently [1].
>>
>> [1] http://inbox.dpdk.org/dev/20191030142938.bpi4txlrebqfq7uw@platinum/
> Yes we can say they are internal but there won't be anything preventing
> applications to use them.
That's true, but making it internal says - don't use it.
Anyway, I have no strong opinion on experimental vs internal.
>>> The question above was do we need this API, or instead should remove the check
>>> from rx/tx_burst APIs?
>> I think these checks are useful to ensure that these functions
>> are not used for hairpin queues. At least to catch it with debug
>> enabled.
> OK, if so what not make them proper API? Any concern on it?
>
>>>>>> [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
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.yigit@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.
> >> We have discussed similar thing with Olivier recently [1].
> >>
> >> [1] http://inbox.dpdk.org/dev/20191030142938.bpi4txlrebqfq7uw@platinum/
> > Yes we can say they are internal but there won't be anything preventing
> > applications to use them.
>
> That's true, but making it internal says - don't use it.
> Anyway, I have no strong opinion on experimental vs internal.
>
> >>> The question above was do we need this API, or instead should remove the check
> >>> from rx/tx_burst APIs?
> >> I think these checks are useful to ensure that these functions
> >> are not used for hairpin queues. At least to catch it with debug
> >> enabled.
> > OK, if so what not make them proper API? Any concern on it?
Why we should not use this API in applications?
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.yigit@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.
>>>> We have discussed similar thing with Olivier recently [1].
>>>>
>>>> [1] http://inbox.dpdk.org/dev/20191030142938.bpi4txlrebqfq7uw@platinum/
>>> Yes we can say they are internal but there won't be anything preventing
>>> applications to use them.
>>
>> That's true, but making it internal says - don't use it.
>> Anyway, I have no strong opinion on experimental vs internal.
>>
>>>>> The question above was do we need this API, or instead should remove the check
>>>>> from rx/tx_burst APIs?
>>>> I think these checks are useful to ensure that these functions
>>>> are not used for hairpin queues. At least to catch it with debug
>>>> enabled.
>>> OK, if so what not make them proper API? Any concern on it?
>
> Why we should not use this API in applications?
I think the valid question is why application needs the API.
Basically I don't mind, just want to be sure that only required
API is exposed.
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.yigit@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.
> >>>> We have discussed similar thing with Olivier recently [1].
> >>>>
> >>>> [1] http://inbox.dpdk.org/dev/20191030142938.bpi4txlrebqfq7uw@platinum/
> >>> Yes we can say they are internal but there won't be anything preventing
> >>> applications to use them.
> >>
> >> That's true, but making it internal says - don't use it.
> >> Anyway, I have no strong opinion on experimental vs internal.
> >>
> >>>>> The question above was do we need this API, or instead should remove the check
> >>>>> from rx/tx_burst APIs?
> >>>> I think these checks are useful to ensure that these functions
> >>>> are not used for hairpin queues. At least to catch it with debug
> >>>> enabled.
> >>> OK, if so what not make them proper API? Any concern on it?
> >
> > Why we should not use this API in applications?
>
> I think the valid question is why application needs the API.
> Basically I don't mind, just want to be sure that only required
> API is exposed.
Because hairpin queues are not standard queues,
we may need to distinguish them.
I see it as a good helper for applications.
Am I missing something obvious?
Hi All
> -----Original Message-----
> From: Thomas Monjalon <thomas@monjalon.net>
> Sent: Tuesday, November 5, 2019 3:02 PM
> To: Andrew Rybchenko <arybchenko@solarflare.com>
> Cc: Ferruh Yigit <ferruh.yigit@intel.com>; Ori Kam <orika@mellanox.com>;
> John McNamara <john.mcnamara@intel.com>; Marko Kovacevic
> <marko.kovacevic@intel.com>; dev@dpdk.org; jingjing.wu@intel.com;
> stephen@networkplumber.org; Jerin Jacob <jerin.jacob@caviumnetworks.com>
> Subject: Re: [dpdk-dev] [PATCH v7 02/14] ethdev: add support for hairpin queue
>
> 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.yigit@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.
> > >>>> We have discussed similar thing with Olivier recently [1].
> > >>>>
> > >>>> [1]
> https://eur03.safelinks.protection.outlook.com/?url=http%3A%2F%2Finbox.dpdk
> .org%2Fdev%2F20191030142938.bpi4txlrebqfq7uw%40platinum%2F&data
> =02%7C01%7Corika%40mellanox.com%7Cb065a7e085aa47723cc408d761f06c9
> f%7Ca652971c7d2e4d9ba6a4d149256f461b%7C0%7C1%7C6370855575589250
> 92&sdata=6kY30p%2BEr4DMQiMqbBPX%2BJZ7h0eMp0FxnzhnE%2F7U%2
> BeM%3D&reserved=0
> > >>> Yes we can say they are internal but there won't be anything preventing
> > >>> applications to use them.
> > >>
> > >> That's true, but making it internal says - don't use it.
> > >> Anyway, I have no strong opinion on experimental vs internal.
> > >>
> > >>>>> The question above was do we need this API, or instead should remove
> the check
> > >>>>> from rx/tx_burst APIs?
> > >>>> I think these checks are useful to ensure that these functions
> > >>>> are not used for hairpin queues. At least to catch it with debug
> > >>>> enabled.
> > >>> OK, if so what not make them proper API? Any concern on it?
> > >
> > > Why we should not use this API in applications?
> >
> > I think the valid question is why application needs the API.
> > Basically I don't mind, just want to be sure that only required
> > API is exposed.
>
> Because hairpin queues are not standard queues,
> we may need to distinguish them.
> I see it as a good helper for applications.
> Am I missing something obvious?
>
>
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.
Best,
Ori
05/11/2019 14:23, Ori Kam:
> From: Thomas Monjalon <thomas@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.yigit@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.
On 11/5/19 4:27 PM, Thomas Monjalon wrote:
> 05/11/2019 14:23, Ori Kam:
>> From: Thomas Monjalon <thomas@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.yigit@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
On 11/5/19 4:02 PM, Thomas Monjalon wrote:
> 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.yigit@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.
>>>>>> We have discussed similar thing with Olivier recently [1].
>>>>>>
>>>>>> [1] http://inbox.dpdk.org/dev/20191030142938.bpi4txlrebqfq7uw@platinum/
>>>>> Yes we can say they are internal but there won't be anything preventing
>>>>> applications to use them.
>>>> That's true, but making it internal says - don't use it.
>>>> Anyway, I have no strong opinion on experimental vs internal.
>>>>
>>>>>>> The question above was do we need this API, or instead should remove the check
>>>>>>> from rx/tx_burst APIs?
>>>>>> I think these checks are useful to ensure that these functions
>>>>>> are not used for hairpin queues. At least to catch it with debug
>>>>>> enabled.
>>>>> OK, if so what not make them proper API? Any concern on it?
>>> Why we should not use this API in applications?
>> I think the valid question is why application needs the API.
>> Basically I don't mind, just want to be sure that only required
>> API is exposed.
> Because hairpin queues are not standard queues,
> we may need to distinguish them.
> I see it as a good helper for applications.
> Am I missing something obvious?
I think no. I would prefer explicit reason, but since
these function are simple, I'm OK to go with
"may need to distinguish them"
@@ -231,6 +231,11 @@ New Features
* Added a console command to testpmd app, ``show port (port_id) ptypes`` which
gives ability to print port supported ptypes in different protocol layers.
+* **Added hairpin queue.**
+
+ On supported NICs, we can now setup haipin queue which will offload packets
+ from the wire, backto the wire.
+
Removed Items
-------------
@@ -923,6 +923,13 @@ struct rte_eth_dev *
RTE_FUNC_PTR_OR_ERR_RET(*dev->dev_ops->rx_queue_start, -ENOTSUP);
+ if (rte_eth_dev_is_rx_hairpin_queue(dev, rx_queue_id)) {
+ RTE_ETHDEV_LOG(INFO,
+ "Can't start Rx hairpin queue %"PRIu16" of device with port_id=%"PRIu16"\n",
+ rx_queue_id, port_id);
+ return -EINVAL;
+ }
+
if (dev->data->rx_queue_state[rx_queue_id] != RTE_ETH_QUEUE_STATE_STOPPED) {
RTE_ETHDEV_LOG(INFO,
"Queue %"PRIu16" of device with port_id=%"PRIu16" already started\n",
@@ -950,6 +957,13 @@ struct rte_eth_dev *
RTE_FUNC_PTR_OR_ERR_RET(*dev->dev_ops->rx_queue_stop, -ENOTSUP);
+ if (rte_eth_dev_is_rx_hairpin_queue(dev, rx_queue_id)) {
+ RTE_ETHDEV_LOG(INFO,
+ "Can't stop Rx hairpin queue %"PRIu16" of device with port_id=%"PRIu16"\n",
+ rx_queue_id, port_id);
+ return -EINVAL;
+ }
+
if (dev->data->rx_queue_state[rx_queue_id] == RTE_ETH_QUEUE_STATE_STOPPED) {
RTE_ETHDEV_LOG(INFO,
"Queue %"PRIu16" of device with port_id=%"PRIu16" already stopped\n",
@@ -983,6 +997,13 @@ struct rte_eth_dev *
RTE_FUNC_PTR_OR_ERR_RET(*dev->dev_ops->tx_queue_start, -ENOTSUP);
+ if (rte_eth_dev_is_tx_hairpin_queue(dev, tx_queue_id)) {
+ RTE_ETHDEV_LOG(INFO,
+ "Can't start Tx hairpin queue %"PRIu16" of device with port_id=%"PRIu16"\n",
+ tx_queue_id, port_id);
+ return -EINVAL;
+ }
+
if (dev->data->tx_queue_state[tx_queue_id] != RTE_ETH_QUEUE_STATE_STOPPED) {
RTE_ETHDEV_LOG(INFO,
"Queue %"PRIu16" of device with port_id=%"PRIu16" already started\n",
@@ -1008,6 +1029,13 @@ struct rte_eth_dev *
RTE_FUNC_PTR_OR_ERR_RET(*dev->dev_ops->tx_queue_stop, -ENOTSUP);
+ if (rte_eth_dev_is_tx_hairpin_queue(dev, tx_queue_id)) {
+ RTE_ETHDEV_LOG(INFO,
+ "Can't stop Tx hairpin queue %"PRIu16" of device with port_id=%"PRIu16"\n",
+ tx_queue_id, port_id);
+ return -EINVAL;
+ }
+
if (dev->data->tx_queue_state[tx_queue_id] == RTE_ETH_QUEUE_STATE_STOPPED) {
RTE_ETHDEV_LOG(INFO,
"Queue %"PRIu16" of device with port_id=%"PRIu16" already stopped\n",
@@ -1780,6 +1808,78 @@ struct rte_eth_dev *
}
int
+rte_eth_rx_hairpin_queue_setup(uint16_t port_id, uint16_t rx_queue_id,
+ uint16_t nb_rx_desc,
+ const struct rte_eth_hairpin_conf *conf)
+{
+ int ret;
+ struct rte_eth_dev *dev;
+ struct rte_eth_hairpin_cap cap;
+ void **rxq;
+ int i;
+ int count;
+
+ RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -EINVAL);
+
+ dev = &rte_eth_devices[port_id];
+ if (rx_queue_id >= dev->data->nb_rx_queues) {
+ RTE_ETHDEV_LOG(ERR, "Invalid RX queue_id=%u\n", rx_queue_id);
+ return -EINVAL;
+ }
+ ret = rte_eth_dev_hairpin_capability_get(port_id, &cap);
+ if (ret != 0)
+ return ret;
+ RTE_FUNC_PTR_OR_ERR_RET(*dev->dev_ops->rx_hairpin_queue_setup,
+ -ENOTSUP);
+ /* if nb_rx_desc is zero use max number of desc from the driver. */
+ if (nb_rx_desc == 0)
+ nb_rx_desc = cap.max_nb_desc;
+ if (nb_rx_desc > cap.max_nb_desc) {
+ RTE_ETHDEV_LOG(ERR,
+ "Invalid value for nb_rx_desc(=%hu), should be: <= %hu",
+ nb_rx_desc, cap.max_nb_desc);
+ return -EINVAL;
+ }
+ if (conf->peer_count > cap.max_rx_2_tx) {
+ RTE_ETHDEV_LOG(ERR,
+ "Invalid value for number of peers for Rx queue(=%hu), should be: <= %hu",
+ conf->peer_count, cap.max_rx_2_tx);
+ return -EINVAL;
+ }
+ if (conf->peer_count == 0) {
+ RTE_ETHDEV_LOG(ERR,
+ "Invalid value for number of peers for Rx queue(=%hu), should be: > 0",
+ conf->peer_count);
+ return -EINVAL;
+ }
+ for (i = 0, count = 0; i < dev->data->nb_rx_queues &&
+ cap.max_nb_queues != UINT16_MAX; i++) {
+ if (i == rx_queue_id || rte_eth_dev_is_rx_hairpin_queue(dev, i))
+ count++;
+ }
+ if (count > cap.max_nb_queues) {
+ RTE_ETHDEV_LOG(ERR, "To many Rx hairpin queues max is %d",
+ cap.max_nb_queues);
+ return -EINVAL;
+ }
+ if (dev->data->dev_started)
+ return -EBUSY;
+ rxq = dev->data->rx_queues;
+ if (rxq[rx_queue_id] != NULL) {
+ RTE_FUNC_PTR_OR_ERR_RET(*dev->dev_ops->rx_queue_release,
+ -ENOTSUP);
+ (*dev->dev_ops->rx_queue_release)(rxq[rx_queue_id]);
+ rxq[rx_queue_id] = NULL;
+ }
+ ret = (*dev->dev_ops->rx_hairpin_queue_setup)(dev, rx_queue_id,
+ nb_rx_desc, conf);
+ if (ret == 0)
+ dev->data->rx_queue_state[rx_queue_id] =
+ RTE_ETH_QUEUE_STATE_HAIRPIN;
+ return eth_err(port_id, ret);
+}
+
+int
rte_eth_tx_queue_setup(uint16_t port_id, uint16_t tx_queue_id,
uint16_t nb_tx_desc, unsigned int socket_id,
const struct rte_eth_txconf *tx_conf)
@@ -1878,6 +1978,77 @@ struct rte_eth_dev *
tx_queue_id, nb_tx_desc, socket_id, &local_conf));
}
+int
+rte_eth_tx_hairpin_queue_setup(uint16_t port_id, uint16_t tx_queue_id,
+ uint16_t nb_tx_desc,
+ const struct rte_eth_hairpin_conf *conf)
+{
+ struct rte_eth_dev *dev;
+ struct rte_eth_hairpin_cap cap;
+ void **txq;
+ int i;
+ int count;
+ int ret;
+
+ RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -EINVAL);
+ dev = &rte_eth_devices[port_id];
+ if (tx_queue_id >= dev->data->nb_tx_queues) {
+ RTE_ETHDEV_LOG(ERR, "Invalid TX queue_id=%u\n", tx_queue_id);
+ return -EINVAL;
+ }
+ ret = rte_eth_dev_hairpin_capability_get(port_id, &cap);
+ if (ret != 0)
+ return ret;
+ RTE_FUNC_PTR_OR_ERR_RET(*dev->dev_ops->tx_hairpin_queue_setup,
+ -ENOTSUP);
+ /* if nb_rx_desc is zero use max number of desc from the driver. */
+ if (nb_tx_desc == 0)
+ nb_tx_desc = cap.max_nb_desc;
+ if (nb_tx_desc > cap.max_nb_desc) {
+ RTE_ETHDEV_LOG(ERR,
+ "Invalid value for nb_tx_desc(=%hu), should be: <= %hu",
+ nb_tx_desc, cap.max_nb_desc);
+ return -EINVAL;
+ }
+ if (conf->peer_count > cap.max_tx_2_rx) {
+ RTE_ETHDEV_LOG(ERR,
+ "Invalid value for number of peers for Tx queue(=%hu), should be: <= %hu",
+ conf->peer_count, cap.max_tx_2_rx);
+ return -EINVAL;
+ }
+ if (conf->peer_count == 0) {
+ RTE_ETHDEV_LOG(ERR,
+ "Invalid value for number of peers for Tx queue(=%hu), should be: > 0",
+ conf->peer_count);
+ return -EINVAL;
+ }
+ for (i = 0, count = 0; i < dev->data->nb_tx_queues &&
+ cap.max_nb_queues != UINT16_MAX; i++) {
+ if (i == tx_queue_id || rte_eth_dev_is_tx_hairpin_queue(dev, i))
+ count++;
+ }
+ if (count > cap.max_nb_queues) {
+ RTE_ETHDEV_LOG(ERR, "To many Tx hairpin queues max is %d",
+ cap.max_nb_queues);
+ return -EINVAL;
+ }
+ if (dev->data->dev_started)
+ return -EBUSY;
+ txq = dev->data->tx_queues;
+ if (txq[tx_queue_id] != NULL) {
+ RTE_FUNC_PTR_OR_ERR_RET(*dev->dev_ops->tx_queue_release,
+ -ENOTSUP);
+ (*dev->dev_ops->tx_queue_release)(txq[tx_queue_id]);
+ txq[tx_queue_id] = NULL;
+ }
+ ret = (*dev->dev_ops->tx_hairpin_queue_setup)
+ (dev, tx_queue_id, nb_tx_desc, conf);
+ if (ret == 0)
+ dev->data->tx_queue_state[tx_queue_id] =
+ RTE_ETH_QUEUE_STATE_HAIRPIN;
+ return eth_err(port_id, ret);
+}
+
void
rte_eth_tx_buffer_drop_callback(struct rte_mbuf **pkts, uint16_t unsent,
void *userdata __rte_unused)
@@ -4007,12 +4178,19 @@ int rte_eth_set_queue_rate_limit(uint16_t port_id, uint16_t queue_idx,
rte_errno = ENOTSUP;
return NULL;
#endif
+ struct rte_eth_dev *dev;
+
/* check input parameters */
if (!rte_eth_dev_is_valid_port(port_id) || fn == NULL ||
queue_id >= rte_eth_devices[port_id].data->nb_rx_queues) {
rte_errno = EINVAL;
return NULL;
}
+ dev = &rte_eth_devices[port_id];
+ if (rte_eth_dev_is_rx_hairpin_queue(dev, queue_id)) {
+ rte_errno = EINVAL;
+ return NULL;
+ }
struct rte_eth_rxtx_callback *cb = rte_zmalloc(NULL, sizeof(*cb), 0);
if (cb == NULL) {
@@ -4084,6 +4262,8 @@ int rte_eth_set_queue_rate_limit(uint16_t port_id, uint16_t queue_idx,
rte_errno = ENOTSUP;
return NULL;
#endif
+ struct rte_eth_dev *dev;
+
/* check input parameters */
if (!rte_eth_dev_is_valid_port(port_id) || fn == NULL ||
queue_id >= rte_eth_devices[port_id].data->nb_tx_queues) {
@@ -4091,6 +4271,12 @@ int rte_eth_set_queue_rate_limit(uint16_t port_id, uint16_t queue_idx,
return NULL;
}
+ dev = &rte_eth_devices[port_id];
+ if (rte_eth_dev_is_tx_hairpin_queue(dev, queue_id)) {
+ rte_errno = EINVAL;
+ return NULL;
+ }
+
struct rte_eth_rxtx_callback *cb = rte_zmalloc(NULL, sizeof(*cb), 0);
if (cb == NULL) {
@@ -4204,6 +4390,13 @@ int rte_eth_set_queue_rate_limit(uint16_t port_id, uint16_t queue_idx,
return -EINVAL;
}
+ if (rte_eth_dev_is_rx_hairpin_queue(dev, queue_id)) {
+ RTE_ETHDEV_LOG(INFO,
+ "Can't get hairpin Rx queue %"PRIu16" info of device with port_id=%"PRIu16"\n",
+ queue_id, port_id);
+ return -EINVAL;
+ }
+
RTE_FUNC_PTR_OR_ERR_RET(*dev->dev_ops->rxq_info_get, -ENOTSUP);
memset(qinfo, 0, sizeof(*qinfo));
@@ -4228,6 +4421,13 @@ int rte_eth_set_queue_rate_limit(uint16_t port_id, uint16_t queue_idx,
return -EINVAL;
}
+ if (rte_eth_dev_is_tx_hairpin_queue(dev, queue_id)) {
+ RTE_ETHDEV_LOG(INFO,
+ "Can't get hairpin Tx queue %"PRIu16" info of device with port_id=%"PRIu16"\n",
+ queue_id, port_id);
+ return -EINVAL;
+ }
+
RTE_FUNC_PTR_OR_ERR_RET(*dev->dev_ops->txq_info_get, -ENOTSUP);
memset(qinfo, 0, sizeof(*qinfo));
@@ -4600,6 +4800,38 @@ int rte_eth_set_queue_rate_limit(uint16_t port_id, uint16_t queue_idx,
}
int
+rte_eth_dev_hairpin_capability_get(uint16_t port_id,
+ struct rte_eth_hairpin_cap *cap)
+{
+ struct rte_eth_dev *dev;
+
+ RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -EINVAL);
+
+ dev = &rte_eth_devices[port_id];
+ RTE_FUNC_PTR_OR_ERR_RET(*dev->dev_ops->hairpin_cap_get, -ENOTSUP);
+ memset(cap, 0, sizeof(*cap));
+ return eth_err(port_id, (*dev->dev_ops->hairpin_cap_get)(dev, cap));
+}
+
+int
+rte_eth_dev_is_rx_hairpin_queue(struct rte_eth_dev *dev, uint16_t queue_id)
+{
+ if (dev->data->rx_queue_state[queue_id] ==
+ RTE_ETH_QUEUE_STATE_HAIRPIN)
+ return 1;
+ return 0;
+}
+
+int
+rte_eth_dev_is_tx_hairpin_queue(struct rte_eth_dev *dev, uint16_t queue_id)
+{
+ if (dev->data->tx_queue_state[queue_id] ==
+ RTE_ETH_QUEUE_STATE_HAIRPIN)
+ return 1;
+ return 0;
+}
+
+int
rte_eth_dev_pool_ops_supported(uint16_t port_id, const char *pool)
{
struct rte_eth_dev *dev;
@@ -839,6 +839,46 @@ struct rte_eth_txconf {
};
/**
+ * @warning
+ * @b EXPERIMENTAL: this API may change, or be removed, without prior notice
+ *
+ * A structure used to return the hairpin capabilities that are supported.
+ */
+struct rte_eth_hairpin_cap {
+ /** The max number of hairpin queues (different bindings). */
+ uint16_t max_nb_queues;
+ /** Max number of Rx queues to be connected to one Tx queue. */
+ uint16_t max_rx_2_tx;
+ /** Max number of Tx queues to be connected to one Rx queue. */
+ uint16_t max_tx_2_rx;
+ uint16_t max_nb_desc; /**< The max num of descriptors. */
+};
+
+#define RTE_ETH_MAX_HAIRPIN_PEERS 32
+
+/**
+ * @warning
+ * @b EXPERIMENTAL: this API may change, or be removed, without prior notice
+ *
+ * A structure used to hold hairpin peer data.
+ */
+struct rte_eth_hairpin_peer {
+ uint16_t port; /**< Peer port. */
+ uint16_t queue; /**< Peer queue. */
+};
+
+/**
+ * @warning
+ * @b EXPERIMENTAL: this API may change, or be removed, without prior notice
+ *
+ * A structure used to configure hairpin binding.
+ */
+struct rte_eth_hairpin_conf {
+ uint16_t peer_count; /**< The number of peers. */
+ struct rte_eth_hairpin_peer peers[RTE_ETH_MAX_HAIRPIN_PEERS];
+};
+
+/**
* A structure contains information about HW descriptor ring limitations.
*/
struct rte_eth_desc_lim {
@@ -1829,6 +1869,37 @@ 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().
+ * @param nb_rx_desc
+ * The number of receive descriptors to allocate for the receive ring.
+ * 0 means the PMD will use default value.
+ * @param conf
+ * The pointer to the hairpin configuration.
+ *
+ * @return
+ * - (0) if successful.
+ * - (-ENOTSUP) if hardware doesn't support.
+ * - (-EINVAL) if bad parameter.
+ * - (-ENOMEM) if unable to allocate the resources.
+ */
+__rte_experimental
+int rte_eth_rx_hairpin_queue_setup
+ (uint16_t port_id, uint16_t rx_queue_id, uint16_t nb_rx_desc,
+ const struct rte_eth_hairpin_conf *conf);
+
+/**
* Allocate and set up a transmit queue for an Ethernet device.
*
* @param port_id
@@ -1881,6 +1952,35 @@ int rte_eth_tx_queue_setup(uint16_t port_id, uint16_t tx_queue_id,
const struct rte_eth_txconf *tx_conf);
/**
+ * @warning
+ * @b EXPERIMENTAL: this API may change, or be removed, without prior notice
+ *
+ * Allocate and set up a transmit hairpin queue for an Ethernet device.
+ *
+ * @param port_id
+ * The port identifier of the Ethernet device.
+ * @param tx_queue_id
+ * The index of the transmit queue to set up.
+ * The value must be in the range [0, nb_tx_queue - 1] previously supplied
+ * to rte_eth_dev_configure().
+ * @param nb_tx_desc
+ * The number of transmit descriptors to allocate for the transmit ring.
+ * 0 to set default PMD value.
+ * @param conf
+ * The hairpin configuration.
+ *
+ * @return
+ * - (0) if successful.
+ * - (-ENOTSUP) if hardware doesn't support.
+ * - (-EINVAL) if bad parameter.
+ * - (-ENOMEM) if unable to allocate the resources.
+ */
+__rte_experimental
+int rte_eth_tx_hairpin_queue_setup
+ (uint16_t port_id, uint16_t tx_queue_id, uint16_t nb_tx_desc,
+ const struct rte_eth_hairpin_conf *conf);
+
+/**
* Return the NUMA socket to which an Ethernet device is connected
*
* @param port_id
@@ -1915,7 +2015,7 @@ int rte_eth_tx_queue_setup(uint16_t port_id, uint16_t tx_queue_id,
* to rte_eth_dev_configure().
* @return
* - 0: Success, the receive queue is started.
- * - -EINVAL: The port_id or the queue_id out of range.
+ * - -EINVAL: The port_id or the queue_id out of range or belong to hairpin.
* - -EIO: if device is removed.
* - -ENOTSUP: The function not supported in PMD driver.
*/
@@ -1932,7 +2032,7 @@ int rte_eth_tx_queue_setup(uint16_t port_id, uint16_t tx_queue_id,
* to rte_eth_dev_configure().
* @return
* - 0: Success, the receive queue is stopped.
- * - -EINVAL: The port_id or the queue_id out of range.
+ * - -EINVAL: The port_id or the queue_id out of range or belong to hairpin.
* - -EIO: if device is removed.
* - -ENOTSUP: The function not supported in PMD driver.
*/
@@ -1950,7 +2050,7 @@ int rte_eth_tx_queue_setup(uint16_t port_id, uint16_t tx_queue_id,
* to rte_eth_dev_configure().
* @return
* - 0: Success, the transmit queue is started.
- * - -EINVAL: The port_id or the queue_id out of range.
+ * - -EINVAL: The port_id or the queue_id out of range or belong to hairpin.
* - -EIO: if device is removed.
* - -ENOTSUP: The function not supported in PMD driver.
*/
@@ -1967,7 +2067,7 @@ int rte_eth_tx_queue_setup(uint16_t port_id, uint16_t tx_queue_id,
* to rte_eth_dev_configure().
* @return
* - 0: Success, the transmit queue is stopped.
- * - -EINVAL: The port_id or the queue_id out of range.
+ * - -EINVAL: The port_id or the queue_id out of range or belong to hairpin.
* - -EIO: if device is removed.
* - -ENOTSUP: The function not supported in PMD driver.
*/
@@ -3633,7 +3733,8 @@ int rte_eth_remove_tx_callback(uint16_t port_id, uint16_t queue_id,
* @return
* - 0: Success
* - -ENOTSUP: routine is not supported by the device PMD.
- * - -EINVAL: The port_id or the queue_id is out of range.
+ * - -EINVAL: The port_id or the queue_id is out of range, or the queue
+ * is hairpin queue.
*/
int rte_eth_rx_queue_info_get(uint16_t port_id, uint16_t queue_id,
struct rte_eth_rxq_info *qinfo);
@@ -3653,7 +3754,8 @@ int rte_eth_rx_queue_info_get(uint16_t port_id, uint16_t queue_id,
* @return
* - 0: Success
* - -ENOTSUP: routine is not supported by the device PMD.
- * - -EINVAL: The port_id or the queue_id is out of range.
+ * - -EINVAL: The port_id or the queue_id is out of range, or the queue
+ * is hairpin queue.
*/
int rte_eth_tx_queue_info_get(uint16_t port_id, uint16_t queue_id,
struct rte_eth_txq_info *qinfo);
@@ -4151,10 +4253,57 @@ int rte_eth_dev_adjust_nb_rx_tx_desc(uint16_t port_id,
void *
rte_eth_dev_get_sec_ctx(uint16_t port_id);
+/**
+ * @warning
+ * @b EXPERIMENTAL: this API may change, or be removed, without prior notice
+ *
+ * Query the device hairpin capabilities.
+ *
+ * @param port_id
+ * The port identifier of the Ethernet device.
+ * @param cap
+ * Pointer to a structure that will hold the hairpin capabilities.
+ * @return
+ * - (0) if successful.
+ * - (-ENOTSUP) if hardware doesn't support.
+ */
+__rte_experimental
+int rte_eth_dev_hairpin_capability_get(uint16_t port_id,
+ struct rte_eth_hairpin_cap *cap);
#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);
+
+/**
*
* 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
#ifdef RTE_ETHDEV_RXTX_CALLBACKS
@@ -509,6 +509,86 @@ typedef int (*eth_pool_ops_supported_t)(struct rte_eth_dev *dev,
/**< @internal Test if a port supports specific mempool ops */
/**
+ * @internal
+ * Get the hairpin capabilities.
+ *
+ * @param dev
+ * ethdev handle of port.
+ * @param cap
+ * returns the hairpin capabilities from the device.
+ *
+ * @return
+ * Negative errno value on error, 0 on success.
+ *
+ * @retval 0
+ * Success, hairpin is supported.
+ * @retval -ENOTSUP
+ * Hairpin is not supported.
+ */
+typedef int (*eth_hairpin_cap_get_t)(struct rte_eth_dev *dev,
+ struct rte_eth_hairpin_cap *cap);
+
+/**
+ * @internal
+ * Setup RX hairpin queue.
+ *
+ * @param dev
+ * ethdev handle of port.
+ * @param rx_queue_id
+ * the selected RX queue index.
+ * @param nb_rx_desc
+ * the requested number of descriptors for this queue. 0 - use PMD default.
+ * @param conf
+ * the RX hairpin configuration structure.
+ *
+ * @return
+ * Negative errno value on error, 0 on success.
+ *
+ * @retval 0
+ * Success, hairpin is supported.
+ * @retval -ENOTSUP
+ * Hairpin is not supported.
+ * @retval -EINVAL
+ * One of the parameters is invalid.
+ * @retval -ENOMEM
+ * Unable to allocate resources.
+ */
+typedef int (*eth_rx_hairpin_queue_setup_t)
+ (struct rte_eth_dev *dev, uint16_t rx_queue_id,
+ uint16_t nb_rx_desc,
+ const struct rte_eth_hairpin_conf *conf);
+
+/**
+ * @internal
+ * Setup TX hairpin queue.
+ *
+ * @param dev
+ * ethdev handle of port.
+ * @param tx_queue_id
+ * the selected TX queue index.
+ * @param nb_tx_desc
+ * the requested number of descriptors for this queue. 0 - use PMD default.
+ * @param conf
+ * the TX hairpin configuration structure.
+ *
+ * @return
+ * Negative errno value on error, 0 on success.
+ *
+ * @retval 0
+ * Success, hairpin is supported.
+ * @retval -ENOTSUP
+ * Hairpin is not supported.
+ * @retval -EINVAL
+ * One of the parameters is invalid.
+ * @retval -ENOMEM
+ * Unable to allocate resources.
+ */
+typedef int (*eth_tx_hairpin_queue_setup_t)
+ (struct rte_eth_dev *dev, uint16_t tx_queue_id,
+ uint16_t nb_tx_desc,
+ const struct rte_eth_hairpin_conf *hairpin_conf);
+
+/**
* @internal A structure containing the functions exported by an Ethernet driver.
*/
struct eth_dev_ops {
@@ -644,6 +724,13 @@ struct eth_dev_ops {
eth_pool_ops_supported_t pool_ops_supported;
/**< Test if a port supports specific mempool ops */
+
+ eth_hairpin_cap_get_t hairpin_cap_get;
+ /**< Returns the hairpin capabilities. */
+ eth_rx_hairpin_queue_setup_t rx_hairpin_queue_setup;
+ /**< Set up device RX hairpin queue. */
+ eth_tx_hairpin_queue_setup_t tx_hairpin_queue_setup;
+ /**< Set up device TX hairpin queue. */
};
/**
@@ -751,9 +838,9 @@ struct rte_eth_dev_data {
dev_started : 1, /**< Device state: STARTED(1) / STOPPED(0). */
lro : 1; /**< RX LRO is ON(1) / OFF(0) */
uint8_t rx_queue_state[RTE_MAX_QUEUES_PER_PORT];
- /**< Queues state: STARTED(1) / STOPPED(0). */
+ /**< Queues state: HAIRPIN(2) / STARTED(1) / STOPPED(0). */
uint8_t tx_queue_state[RTE_MAX_QUEUES_PER_PORT];
- /**< Queues state: STARTED(1) / STOPPED(0). */
+ /**< Queues state: HAIRPIN(2) / STARTED(1) / STOPPED(0). */
uint32_t dev_flags; /**< Capabilities. */
enum rte_kernel_driver kdrv; /**< Kernel driver passthrough. */
int numa_node; /**< NUMA node connection. */
@@ -26,6 +26,7 @@
*/
#define RTE_ETH_QUEUE_STATE_STOPPED 0
#define RTE_ETH_QUEUE_STATE_STARTED 1
+#define RTE_ETH_QUEUE_STATE_HAIRPIN 2
/**
* @internal
@@ -288,4 +288,7 @@ EXPERIMENTAL {
rte_eth_rx_burst_mode_get;
rte_eth_tx_burst_mode_get;
rte_eth_burst_mode_option_name;
+ rte_eth_rx_hairpin_queue_setup;
+ rte_eth_tx_hairpin_queue_setup;
+ rte_eth_dev_hairpin_capability_get;
};