[v6,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>
---
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 | 217 +++++++++++++++++++++++++++++++
lib/librte_ethdev/rte_ethdev.h | 141 +++++++++++++++++++-
lib/librte_ethdev/rte_ethdev_core.h | 91 ++++++++++++-
lib/librte_ethdev/rte_ethdev_driver.h | 44 +++++++
lib/librte_ethdev/rte_ethdev_version.map | 3 +
6 files changed, 493 insertions(+), 8 deletions(-)
Comments
Hi Ori,
On 10/27/19 3:24 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>
LGTM, nothing critical may be except maximum number check
which I lost from my view before.
Plus few style suggestions which may be dropped, but I'd be
happier if applied.
Thanks.
[snip]
> diff --git a/lib/librte_ethdev/rte_ethdev.c b/lib/librte_ethdev/rte_ethdev.c
> index 7743205..68aca1f 100644
> --- a/lib/librte_ethdev/rte_ethdev.c
> +++ b/lib/librte_ethdev/rte_ethdev.c
> @@ -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 queue %"PRIu16" of device with port_id=%"PRIu16" is hairpin queue\n",
Log message looks a bit strange:
Can't start Rx queue 5 of device with port_id=0 is hairpin queue
may be to put key information first:
Can't start hairpin Rx queue 5 of device with port_id=0
> + 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 queue %"PRIu16" of device with port_id=%"PRIu16" is hairpin queue\n",
Same
> + 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 queue %"PRIu16" of device with port_id=%"PRIu16" is hairpin queue\n",
Same
> + 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 queue %"PRIu16" of device with port_id=%"PRIu16" is hairpin queue\n",
Same
> + 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,79 @@ 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 = 0;
> +
> + 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);
Most likely unsupported hairpin is caught by capability get above.
So, may be it is better to move the check just before usage far below.
Also, if line length is sufficient I think it would better to put -ENOTSUP
to the previous line just to follow port_id check style.
> + /* Use default specified by driver, if nb_rx_desc is zero */
> + if (nb_rx_desc == 0)
> + nb_rx_desc = cap.max_nb_desc;
Function description and comment above mentions PMD default, but
there is no default. It uses just maximum. I have no strong opinion
if default is really required or it is OK to say that maximum is used.
The only concern is: why maximum?
> + 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;
> + }
> + if (cap.max_nb_queues != UINT16_MAX) {
I'm not sure that we need to handle it separately. Code below
should handle it and it and there is no point to optimize it.
> + for (i = 0; i < dev->data->nb_rx_queues; i++) {
May I suggest to assign count = 0 to make it a bit easier to read and
more robust against future changes.
> + if (rte_eth_dev_is_rx_hairpin_queue(dev, i))
The condition should be more tricky if we resetup hairpin queue.
I.e. we should check if i is rx_queue_id and count it anyway.
> + count++;
> + }
> + if (count > cap.max_nb_queues) {
> + RTE_ETHDEV_LOG(ERR, "To many Rx hairpin queues %d",
I think it would be useful to log max here as well to catch
unset max cases easier.
> + count);
> + 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 +1979,78 @@ 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)
Same notes as for Rx queue above.
> +{
> + struct rte_eth_dev *dev;
> + struct rte_eth_hairpin_cap cap;
> + void **txq;
> + int i;
> + int count = 0;
> + 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);
> + /* Use default specified by driver, if nb_tx_desc is zero */
> + 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;
> + }
> + if (cap.max_nb_queues != UINT16_MAX) {
> + for (i = 0; i < dev->data->nb_tx_queues; i++) {
> + if (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 %d", count);
> + 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 +4180,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 +4264,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 +4273,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 +4392,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 queue info for Rx queue %"PRIu16" of device with port_id=%"PRIu16" is hairpin queue\n",
"queue" is repeated 3 times above ;) I'm afraid it is too much, may be:
"Can't get hairpin Rx queue %" PRIu16 " port %" PRIu16 " info\n"
or
"Can't get hairpin Rx queue %" PRIu16 " info of device with port_id=%"
PRIu16 "\n"
Anyway up to you.
> + 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 +4423,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 queue info for Tx queue %"PRIu16" of device with port_id=%"PRIu16" is hairpin queue\n",
Same
> + 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 +4802,21 @@ 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);
Please, move -ENOTSUP to the previous line since line length is sufficient
and make it similar to port_id check above.
> + memset(cap, 0, sizeof(*cap));
> + return eth_err(port_id, (*dev->dev_ops->hairpin_cap_get)(dev, cap));
> +}
> +
> +int
> rte_eth_dev_pool_ops_supported(uint16_t port_id, const char *pool)
> {
> struct rte_eth_dev *dev;
> diff --git a/lib/librte_ethdev/rte_ethdev.h b/lib/librte_ethdev/rte_ethdev.h
> index 9e1f9ae..9b69255 100644
> --- a/lib/librte_ethdev/rte_ethdev.h
> +++ b/lib/librte_ethdev/rte_ethdev.h
> @@ -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. */
Should be /**
> + uint16_t max_rx_2_tx;
> + /**< Max number of Tx queues to be connected to one Rx queue. */
Should be /**
[snip]
Hi Andrew,
> -----Original Message-----
> From: Andrew Rybchenko <arybchenko@solarflare.com>
> Sent: Monday, October 28, 2019 5:16 PM
> To: Ori Kam <orika@mellanox.com>; John McNamara
> <john.mcnamara@intel.com>; Marko Kovacevic
> <marko.kovacevic@intel.com>; Thomas Monjalon <thomas@monjalon.net>;
> Ferruh Yigit <ferruh.yigit@intel.com>
> Cc: dev@dpdk.org; jingjing.wu@intel.com; stephen@networkplumber.org
> Subject: Re: [dpdk-dev] [PATCH v6 02/14] ethdev: add support for hairpin queue
>
> Hi Ori,
>
> On 10/27/19 3:24 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>
>
> LGTM, nothing critical may be except maximum number check
> which I lost from my view before.
> Plus few style suggestions which may be dropped, but I'd be
> happier if applied.
>
> Thanks.
>
I really apricate your time and comments,
This patch is the base of a number of other series (Meta/Metering)
So if it is nothing critical I prefer to get this set merged and then change what is needed,
if it is O.K by you.
Detail comments please see below.
> [snip]
>
> > diff --git a/lib/librte_ethdev/rte_ethdev.c b/lib/librte_ethdev/rte_ethdev.c
> > index 7743205..68aca1f 100644
> > --- a/lib/librte_ethdev/rte_ethdev.c
> > +++ b/lib/librte_ethdev/rte_ethdev.c
> > @@ -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 queue %"PRIu16" of device with
> port_id=%"PRIu16" is hairpin queue\n",
>
> Log message looks a bit strange:
> Can't start Rx queue 5 of device with port_id=0 is hairpin queue
> may be to put key information first:
> Can't start hairpin Rx queue 5 of device with port_id=0
>
I'm not a native English speaker but I think the meaning is different.
In my original log it means that you try to start a queue but fail due to
the fact that the queue is hairpin queue.
In your version it means that you can't start an hairpin queue but there is no
reason why not.
What do you think?
> > + 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 queue %"PRIu16" of device with
> port_id=%"PRIu16" is hairpin queue\n",
>
> Same
>
Please see comment above.
> > + 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 queue %"PRIu16" of device with
> port_id=%"PRIu16" is hairpin queue\n",
>
> Same
>
Please see comment above.
> > + 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 queue %"PRIu16" of device with
> port_id=%"PRIu16" is hairpin queue\n",
>
> Same
>
Please see comment above.
> > + 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,79 @@ 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 = 0;
> > +
> > + 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);
>
> Most likely unsupported hairpin is caught by capability get above.
> So, may be it is better to move the check just before usage far below.
> Also, if line length is sufficient I think it would better to put -ENOTSUP
> to the previous line just to follow port_id check style.
>
I think that in most function we are starting with the check.
personally I like to have basic checks in the beginning of the code.
But I will do what you think is best. If I remember correctly the line
length is to short, but I will test again.
> > + /* Use default specified by driver, if nb_rx_desc is zero */
> > + if (nb_rx_desc == 0)
> > + nb_rx_desc = cap.max_nb_desc;
>
> Function description and comment above mentions PMD default, but
> there is no default. It uses just maximum. I have no strong opinion
> if default is really required or it is OK to say that maximum is used.
> The only concern is: why maximum?
>
Most likely the best value is the max, but I can add a new field to the cap
that say default value. What do you think?
> > + 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;
> > + }
> > + if (cap.max_nb_queues != UINT16_MAX) {
>
> I'm not sure that we need to handle it separately. Code below
> should handle it and it and there is no point to optimize it.
>
This is done to save time if the user set uint16_max there is no point to the
loop, I can add the check as condition to the loop but then it looks incorrect
since we are checking some think that can’t be changed.
What do you think?
> > + for (i = 0; i < dev->data->nb_rx_queues; i++) {
>
> May I suggest to assign count = 0 to make it a bit easier to read and
> more robust against future changes.
>
You mean add count = 0 to the first part of the loop?
> > + if (rte_eth_dev_is_rx_hairpin_queue(dev, i))
>
> The condition should be more tricky if we resetup hairpin queue.
> I.e. we should check if i is rx_queue_id and count it anyway.
>
> > + count++;
> > + }
> > + if (count > cap.max_nb_queues) {
> > + RTE_ETHDEV_LOG(ERR, "To many Rx hairpin queues
> %d",
>
> I think it would be useful to log max here as well to catch
> unset max cases easier.
>
I'm not sure I understand.
> > + count);
> > + 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 +1979,78 @@ 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)
>
> Same notes as for Rx queue above.
>
O.K. same comments.
> > +{
> > + struct rte_eth_dev *dev;
> > + struct rte_eth_hairpin_cap cap;
> > + void **txq;
> > + int i;
> > + int count = 0;
> > + 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);
> > + /* Use default specified by driver, if nb_tx_desc is zero */
> > + 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;
> > + }
> > + if (cap.max_nb_queues != UINT16_MAX) {
> > + for (i = 0; i < dev->data->nb_tx_queues; i++) {
> > + if (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 %d", count);
> > + 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 +4180,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 +4264,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 +4273,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 +4392,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 queue info for Rx queue %"PRIu16" of device
> with port_id=%"PRIu16" is hairpin queue\n",
>
> "queue" is repeated 3 times above ;) I'm afraid it is too much, may be:
> "Can't get hairpin Rx queue %" PRIu16 " port %" PRIu16 " info\n"
> or
> "Can't get hairpin Rx queue %" PRIu16 " info of device with port_id=%"
> PRIu16 "\n"
> Anyway up to you.
>
O.K. I will update.
> > + 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 +4423,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 queue info for Tx queue %"PRIu16" of device
> with port_id=%"PRIu16" is hairpin queue\n",
>
> Same
>
Same.
> > + 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 +4802,21 @@ 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);
>
> Please, move -ENOTSUP to the previous line since line length is sufficient
> and make it similar to port_id check above.
>
Last time I check it didn't have room, I will check again.
> > + memset(cap, 0, sizeof(*cap));
> > + return eth_err(port_id, (*dev->dev_ops->hairpin_cap_get)(dev, cap));
> > +}
> > +
> > +int
> > rte_eth_dev_pool_ops_supported(uint16_t port_id, const char *pool)
> > {
> > struct rte_eth_dev *dev;
> > diff --git a/lib/librte_ethdev/rte_ethdev.h b/lib/librte_ethdev/rte_ethdev.h
> > index 9e1f9ae..9b69255 100644
> > --- a/lib/librte_ethdev/rte_ethdev.h
> > +++ b/lib/librte_ethdev/rte_ethdev.h
> > @@ -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. */
>
> Should be /**
>
Will fix.
> > + uint16_t max_rx_2_tx;
> > + /**< Max number of Tx queues to be connected to one Rx queue. */
>
> Should be /**
>
Will fix.
> [snip]
Thanks,
Ori
On 10/28/19 9:44 PM, Ori Kam wrote:
> Hi Andrew,
>
>
>> -----Original Message-----
>> From: Andrew Rybchenko <arybchenko@solarflare.com>
>> Sent: Monday, October 28, 2019 5:16 PM
>> To: Ori Kam <orika@mellanox.com>; John McNamara
>> <john.mcnamara@intel.com>; Marko Kovacevic
>> <marko.kovacevic@intel.com>; Thomas Monjalon <thomas@monjalon.net>;
>> Ferruh Yigit <ferruh.yigit@intel.com>
>> Cc: dev@dpdk.org; jingjing.wu@intel.com; stephen@networkplumber.org
>> Subject: Re: [dpdk-dev] [PATCH v6 02/14] ethdev: add support for hairpin queue
>>
>> Hi Ori,
>>
>> On 10/27/19 3:24 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>
>> LGTM, nothing critical may be except maximum number check
>> which I lost from my view before.
>> Plus few style suggestions which may be dropped, but I'd be
>> happier if applied.
>>
>> Thanks.
>>
> I really apricate your time and comments,
> This patch is the base of a number of other series (Meta/Metering)
> So if it is nothing critical I prefer to get this set merged and then change what is needed,
> if it is O.K by you.
OK for me
> Detail comments please see below.
>
>> [snip]
>>
>>> diff --git a/lib/librte_ethdev/rte_ethdev.c b/lib/librte_ethdev/rte_ethdev.c
>>> index 7743205..68aca1f 100644
>>> --- a/lib/librte_ethdev/rte_ethdev.c
>>> +++ b/lib/librte_ethdev/rte_ethdev.c
>>> @@ -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 queue %"PRIu16" of device with
>> port_id=%"PRIu16" is hairpin queue\n",
>>
>> Log message looks a bit strange:
>> Can't start Rx queue 5 of device with port_id=0 is hairpin queue
>> may be to put key information first:
>> Can't start hairpin Rx queue 5 of device with port_id=0
>>
> I'm not a native English speaker but I think the meaning is different.
Obviously me too
> In my original log it means that you try to start a queue but fail due to
> the fact that the queue is hairpin queue.
>
> In your version it means that you can't start an hairpin queue but there is no
> reason why not.
>
> What do you think?
Let's keep your version if there is no better suggestions from native
speakers.
>>> + 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 queue %"PRIu16" of device with
>> port_id=%"PRIu16" is hairpin queue\n",
>>
>> Same
>>
> Please see comment above.
>
>>> + 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 queue %"PRIu16" of device with
>> port_id=%"PRIu16" is hairpin queue\n",
>>
>> Same
>>
> Please see comment above.
>
>>> + 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 queue %"PRIu16" of device with
>> port_id=%"PRIu16" is hairpin queue\n",
>>
>> Same
>>
> Please see comment above.
>
>>> + 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,79 @@ 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 = 0;
>>> +
>>> + 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);
>> Most likely unsupported hairpin is caught by capability get above.
>> So, may be it is better to move the check just before usage far below.
>> Also, if line length is sufficient I think it would better to put -ENOTSUP
>> to the previous line just to follow port_id check style.
>>
> I think that in most function we are starting with the check.
> personally I like to have basic checks in the beginning of the code.
> But I will do what you think is best. If I remember correctly the line
> length is to short, but I will test again.
Up to you. Thanks.
>>> + /* Use default specified by driver, if nb_rx_desc is zero */
>>> + if (nb_rx_desc == 0)
>>> + nb_rx_desc = cap.max_nb_desc;
>> Function description and comment above mentions PMD default, but
>> there is no default. It uses just maximum. I have no strong opinion
>> if default is really required or it is OK to say that maximum is used.
>> The only concern is: why maximum?
>>
> Most likely the best value is the max, but I can add a new field to the cap
> that say default value. What do you think?
I'm not 100% sure since default requires 0 value handling and
I think fallback to maximum could be the right handling here.
May be it is better to document that maximum is used and
introduce default if it is really required in the future.
It should be reconsidered when API is promoted to stable
from experimental.
Basically both options are OK for me.
>>> + 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;
>>> + }
>>> + if (cap.max_nb_queues != UINT16_MAX) {
>> I'm not sure that we need to handle it separately. Code below
>> should handle it and it and there is no point to optimize it.
>>
> This is done to save time if the user set uint16_max there is no point to the
> loop, I can add the check as condition to the loop but then it looks incorrect
> since we are checking some think that can’t be changed.
> What do you think?
Frankly speaking I see no value in the optimization. It is control
path and I'd prefer simpler code here.
>>> + for (i = 0; i < dev->data->nb_rx_queues; i++) {
>> May I suggest to assign count = 0 to make it a bit easier to read and
>> more robust against future changes.
>>
> You mean add count = 0 to the first part of the loop?
Yes, right now count initialization is done too far from the line.
>>> + if (rte_eth_dev_is_rx_hairpin_queue(dev, i))
>> The condition should be more tricky if we resetup hairpin queue.
>> I.e. we should check if i is rx_queue_id and count it anyway.
>>
>>> + count++;
>>> + }
>>> + if (count > cap.max_nb_queues) {
>>> + RTE_ETHDEV_LOG(ERR, "To many Rx hairpin queues
>> %d",
>>
>> I think it would be useful to log max here as well to catch
>> unset max cases easier.
>>
> I'm not sure I understand.
If the question is about logging, the answer is simple:
if the user forget to initialize maximum number of hairpin queues
properly, it will be zero and setup will fail here. So, it would be
good to log maximum value here just to make it clear which
limit is exceeded.
If the question is about above check, let's consider the case when
maximum is one and one hairpin queue is already setup, but
user tries to setup one more. Above loop will count only one since
hairpin state for current queue is set below. So, the condition will
allow to setup the second hairpin queue.
In theory, we could initialize cound=1 to count this one, but
it would break the case when we call setup once again for the
queue which is already hairpin. API allows and handles it.
>>> + count);
>>> + 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 +1979,78 @@ 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)
>> Same notes as for Rx queue above.
>>
> O.K. same comments.
>
>>> +{
>>> + struct rte_eth_dev *dev;
>>> + struct rte_eth_hairpin_cap cap;
>>> + void **txq;
>>> + int i;
>>> + int count = 0;
>>> + 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);
>>> + /* Use default specified by driver, if nb_tx_desc is zero */
>>> + 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;
>>> + }
>>> + if (cap.max_nb_queues != UINT16_MAX) {
>>> + for (i = 0; i < dev->data->nb_tx_queues; i++) {
>>> + if (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 %d", count);
>>> + 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 +4180,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 +4264,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 +4273,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 +4392,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 queue info for Rx queue %"PRIu16" of device
>> with port_id=%"PRIu16" is hairpin queue\n",
>>
>> "queue" is repeated 3 times above ;) I'm afraid it is too much, may be:
>> "Can't get hairpin Rx queue %" PRIu16 " port %" PRIu16 " info\n"
>> or
>> "Can't get hairpin Rx queue %" PRIu16 " info of device with port_id=%"
>> PRIu16 "\n"
>> Anyway up to you.
>>
> O.K. I will update.
>
>>> + 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 +4423,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 queue info for Tx queue %"PRIu16" of device
>> with port_id=%"PRIu16" is hairpin queue\n",
>>
>> Same
>>
> Same.
>
>>> + 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 +4802,21 @@ 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);
>> Please, move -ENOTSUP to the previous line since line length is sufficient
>> and make it similar to port_id check above.
>>
> Last time I check it didn't have room, I will check again.
>
>>> + memset(cap, 0, sizeof(*cap));
>>> + return eth_err(port_id, (*dev->dev_ops->hairpin_cap_get)(dev, cap));
>>> +}
>>> +
>>> +int
>>> rte_eth_dev_pool_ops_supported(uint16_t port_id, const char *pool)
>>> {
>>> struct rte_eth_dev *dev;
>>> diff --git a/lib/librte_ethdev/rte_ethdev.h b/lib/librte_ethdev/rte_ethdev.h
>>> index 9e1f9ae..9b69255 100644
>>> --- a/lib/librte_ethdev/rte_ethdev.h
>>> +++ b/lib/librte_ethdev/rte_ethdev.h
>>> @@ -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. */
>> Should be /**
>>
> Will fix.
>
>>> + uint16_t max_rx_2_tx;
>>> + /**< Max number of Tx queues to be connected to one Rx queue. */
>> Should be /**
>>
> Will fix.
>
>> [snip]
>
> Thanks,
> Ori
Hi Andrew,
> -----Original Message-----
> From: Andrew Rybchenko <arybchenko@solarflare.com>
> Sent: Tuesday, October 29, 2019 9:39 AM
> To: Ori Kam <orika@mellanox.com>; John McNamara
> <john.mcnamara@intel.com>; Marko Kovacevic
> <marko.kovacevic@intel.com>; Thomas Monjalon <thomas@monjalon.net>;
> Ferruh Yigit <ferruh.yigit@intel.com>
> Cc: dev@dpdk.org; jingjing.wu@intel.com; stephen@networkplumber.org
> Subject: Re: [dpdk-dev] [PATCH v6 02/14] ethdev: add support for hairpin queue
>
> On 10/28/19 9:44 PM, Ori Kam wrote:
> > Hi Andrew,
> >
> >
> >> -----Original Message-----
> >> From: Andrew Rybchenko <arybchenko@solarflare.com>
> >> Sent: Monday, October 28, 2019 5:16 PM
> >> To: Ori Kam <orika@mellanox.com>; John McNamara
> >> <john.mcnamara@intel.com>; Marko Kovacevic
> >> <marko.kovacevic@intel.com>; Thomas Monjalon <thomas@monjalon.net>;
> >> Ferruh Yigit <ferruh.yigit@intel.com>
> >> Cc: dev@dpdk.org; jingjing.wu@intel.com; stephen@networkplumber.org
> >> Subject: Re: [dpdk-dev] [PATCH v6 02/14] ethdev: add support for hairpin
> queue
> >>
> >> Hi Ori,
> >>
> >> On 10/27/19 3:24 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>
> >> LGTM, nothing critical may be except maximum number check
> >> which I lost from my view before.
> >> Plus few style suggestions which may be dropped, but I'd be
> >> happier if applied.
> >>
> >> Thanks.
> >>
> > I really apricate your time and comments,
> > This patch is the base of a number of other series (Meta/Metering)
> > So if it is nothing critical I prefer to get this set merged and then change what
> is needed,
> > if it is O.K by you.
>
> OK for me
>
Thanks, I will send a new patch as soon as this get merged.
> > Detail comments please see below.
> >
> >> [snip]
> >>
> >>> diff --git a/lib/librte_ethdev/rte_ethdev.c b/lib/librte_ethdev/rte_ethdev.c
> >>> index 7743205..68aca1f 100644
> >>> --- a/lib/librte_ethdev/rte_ethdev.c
> >>> +++ b/lib/librte_ethdev/rte_ethdev.c
> >>> @@ -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 queue %"PRIu16" of device with
> >> port_id=%"PRIu16" is hairpin queue\n",
> >>
> >> Log message looks a bit strange:
> >> Can't start Rx queue 5 of device with port_id=0 is hairpin queue
> >> may be to put key information first:
> >> Can't start hairpin Rx queue 5 of device with port_id=0
> >>
> > I'm not a native English speaker but I think the meaning is different.
>
> Obviously me too
>
> > In my original log it means that you try to start a queue but fail due to
> > the fact that the queue is hairpin queue.
> >
> > In your version it means that you can't start an hairpin queue but there is no
> > reason why not.
> >
> > What do you think?
>
> Let's keep your version if there is no better suggestions from native
> speakers.
>
Thanks,
> >>> + 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 queue %"PRIu16" of device with
> >> port_id=%"PRIu16" is hairpin queue\n",
> >>
> >> Same
> >>
> > Please see comment above.
> >
> >>> + 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 queue %"PRIu16" of device with
> >> port_id=%"PRIu16" is hairpin queue\n",
> >>
> >> Same
> >>
> > Please see comment above.
> >
> >>> + 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 queue %"PRIu16" of device with
> >> port_id=%"PRIu16" is hairpin queue\n",
> >>
> >> Same
> >>
> > Please see comment above.
> >
> >>> + 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,79 @@ 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 = 0;
> >>> +
> >>> + 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);
> >> Most likely unsupported hairpin is caught by capability get above.
> >> So, may be it is better to move the check just before usage far below.
> >> Also, if line length is sufficient I think it would better to put -ENOTSUP
> >> to the previous line just to follow port_id check style.
> >>
> > I think that in most function we are starting with the check.
> > personally I like to have basic checks in the beginning of the code.
> > But I will do what you think is best. If I remember correctly the line
> > length is to short, but I will test again.
>
> Up to you. Thanks.
>
I will keep my version, but will test again if I can merge it to one line.
> >>> + /* Use default specified by driver, if nb_rx_desc is zero */
> >>> + if (nb_rx_desc == 0)
> >>> + nb_rx_desc = cap.max_nb_desc;
> >> Function description and comment above mentions PMD default, but
> >> there is no default. It uses just maximum. I have no strong opinion
> >> if default is really required or it is OK to say that maximum is used.
> >> The only concern is: why maximum?
> >>
> > Most likely the best value is the max, but I can add a new field to the cap
> > that say default value. What do you think?
>
> I'm not 100% sure since default requires 0 value handling and
> I think fallback to maximum could be the right handling here.
> May be it is better to document that maximum is used and
> introduce default if it is really required in the future.
> It should be reconsidered when API is promoted to stable
> from experimental.
>
> Basically both options are OK for me.
>
I like your idea, I will change the documentation to say that the max will be used.
> >>> + 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;
> >>> + }
> >>> + if (cap.max_nb_queues != UINT16_MAX) {
> >> I'm not sure that we need to handle it separately. Code below
> >> should handle it and it and there is no point to optimize it.
> >>
> > This is done to save time if the user set uint16_max there is no point to the
> > loop, I can add the check as condition to the loop but then it looks incorrect
> > since we are checking some think that can’t be changed.
> > What do you think?
>
> Frankly speaking I see no value in the optimization. It is control
> path and I'd prefer simpler code here.
>
O.K. will add the check in the for command.
> >>> + for (i = 0; i < dev->data->nb_rx_queues; i++) {
> >> May I suggest to assign count = 0 to make it a bit easier to read and
> >> more robust against future changes.
> >>
> > You mean add count = 0 to the first part of the loop?
>
> Yes, right now count initialization is done too far from the line.
>
Will fix.
> >>> + if (rte_eth_dev_is_rx_hairpin_queue(dev, i))
> >> The condition should be more tricky if we resetup hairpin queue.
> >> I.e. we should check if i is rx_queue_id and count it anyway.
> >>
> >>> + count++;
> >>> + }
> >>> + if (count > cap.max_nb_queues) {
> >>> + RTE_ETHDEV_LOG(ERR, "To many Rx hairpin queues
> >> %d",
> >>
> >> I think it would be useful to log max here as well to catch
> >> unset max cases easier.
> >>
> > I'm not sure I understand.
>
> If the question is about logging, the answer is simple:
> if the user forget to initialize maximum number of hairpin queues
> properly, it will be zero and setup will fail here. So, it would be
> good to log maximum value here just to make it clear which
> limit is exceeded.
>
Maybe I'm missing something but the PMD sets the max number of hairpin queues.
But in any case I agree we should log what the user requested and what is the max
that the PMD reports.
> If the question is about above check, let's consider the case when
> maximum is one and one hairpin queue is already setup, but
> user tries to setup one more. Above loop will count only one since
> hairpin state for current queue is set below. So, the condition will
> allow to setup the second hairpin queue.
> In theory, we could initialize cound=1 to count this one, but
> it would break the case when we call setup once again for the
> queue which is already hairpin. API allows and handles it.
>
Nice catch. I think the best solution is to compare the count to cap.max_nb_queues - 1.
and even before this comparison check if the current queue is already hairpin queue if so
we can skip this check.
What do you think?
> >>> + count);
> >>> + 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 +1979,78 @@ 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)
> >> Same notes as for Rx queue above.
> >>
> > O.K. same comments.
> >
> >>> +{
> >>> + struct rte_eth_dev *dev;
> >>> + struct rte_eth_hairpin_cap cap;
> >>> + void **txq;
> >>> + int i;
> >>> + int count = 0;
> >>> + 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);
> >>> + /* Use default specified by driver, if nb_tx_desc is zero */
> >>> + 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;
> >>> + }
> >>> + if (cap.max_nb_queues != UINT16_MAX) {
> >>> + for (i = 0; i < dev->data->nb_tx_queues; i++) {
> >>> + if (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 %d", count);
> >>> + 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 +4180,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 +4264,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 +4273,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 +4392,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 queue info for Rx queue %"PRIu16" of device
> >> with port_id=%"PRIu16" is hairpin queue\n",
> >>
> >> "queue" is repeated 3 times above ;) I'm afraid it is too much, may be:
> >> "Can't get hairpin Rx queue %" PRIu16 " port %" PRIu16 " info\n"
> >> or
> >> "Can't get hairpin Rx queue %" PRIu16 " info of device with port_id=%"
> >> PRIu16 "\n"
> >> Anyway up to you.
> >>
> > O.K. I will update.
> >
> >>> + 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 +4423,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 queue info for Tx queue %"PRIu16" of device
> >> with port_id=%"PRIu16" is hairpin queue\n",
> >>
> >> Same
> >>
> > Same.
> >
> >>> + 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 +4802,21 @@ 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);
> >> Please, move -ENOTSUP to the previous line since line length is sufficient
> >> and make it similar to port_id check above.
> >>
> > Last time I check it didn't have room, I will check again.
> >
> >>> + memset(cap, 0, sizeof(*cap));
> >>> + return eth_err(port_id, (*dev->dev_ops->hairpin_cap_get)(dev, cap));
> >>> +}
> >>> +
> >>> +int
> >>> rte_eth_dev_pool_ops_supported(uint16_t port_id, const char *pool)
> >>> {
> >>> struct rte_eth_dev *dev;
> >>> diff --git a/lib/librte_ethdev/rte_ethdev.h b/lib/librte_ethdev/rte_ethdev.h
> >>> index 9e1f9ae..9b69255 100644
> >>> --- a/lib/librte_ethdev/rte_ethdev.h
> >>> +++ b/lib/librte_ethdev/rte_ethdev.h
> >>> @@ -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. */
> >> Should be /**
> >>
> > Will fix.
> >
> >>> + uint16_t max_rx_2_tx;
> >>> + /**< Max number of Tx queues to be connected to one Rx queue. */
> >> Should be /**
> >>
> > Will fix.
> >
> >> [snip]
> >
> > Thanks,
> > Ori
Thanks,
Ori
Hi Ori,
On 10/29/19 10:39 PM, Ori Kam wrote:
>> On 10/28/19 9:44 PM, Ori Kam wrote:
>>>> On 10/27/19 3:24 PM, Ori Kam wrote:
>>>>> + if (rte_eth_dev_is_rx_hairpin_queue(dev, i))
>>>> The condition should be more tricky if we resetup hairpin queue.
>>>> I.e. we should check if i is rx_queue_id and count it anyway.
>>>>
>>>>> + count++;
>>>>> + }
>>>>> + if (count > cap.max_nb_queues) {
>>>>> + RTE_ETHDEV_LOG(ERR, "To many Rx hairpin queues
>>>> %d",
>>>>
>>>> I think it would be useful to log max here as well to catch
>>>> unset max cases easier.
>>>>
>>> I'm not sure I understand.
>> If the question is about logging, the answer is simple:
>> if the user forget to initialize maximum number of hairpin queues
>> properly, it will be zero and setup will fail here. So, it would be
>> good to log maximum value here just to make it clear which
>> limit is exceeded.
>>
> Maybe I'm missing something but the PMD sets the max number of hairpin queues.
Yes, it is just my paranoia to simplify debugging the case if PMD
forgets to do it.
> But in any case I agree we should log what the user requested and what is the max
> that the PMD reports.
>
>> If the question is about above check, let's consider the case when
>> maximum is one and one hairpin queue is already setup, but
>> user tries to setup one more. Above loop will count only one since
>> hairpin state for current queue is set below. So, the condition will
>> allow to setup the second hairpin queue.
>> In theory, we could initialize cound=1 to count this one, but
>> it would break the case when we call setup once again for the
>> queue which is already hairpin. API allows and handles it.
>>
> Nice catch. I think the best solution is to compare the count to cap.max_nb_queues - 1.
> and even before this comparison check if the current queue is already hairpin queue if so
> we can skip this check.
> What do you think?
I think the right solution is always count current queue since it is either
becoming hairpin or already hairpin, i.e.
if (i == rx_queue_id || rte_eth_dev_is_rx_hairpin_queue(dev, i))
So, the result will be always total number of hairpin queues if
this one is hairpin.
Andrew.
Hi Andrew,
> -----Original Message-----
> From: Andrew Rybchenko <arybchenko@solarflare.com>
> Sent: Wednesday, October 30, 2019 8:39 AM
> To: Ori Kam <orika@mellanox.com>; John McNamara
> <john.mcnamara@intel.com>; Marko Kovacevic
> <marko.kovacevic@intel.com>; Thomas Monjalon <thomas@monjalon.net>;
> Ferruh Yigit <ferruh.yigit@intel.com>
> Cc: dev@dpdk.org; jingjing.wu@intel.com; stephen@networkplumber.org
> Subject: Re: [dpdk-dev] [PATCH v6 02/14] ethdev: add support for hairpin queue
>
> Hi Ori,
>
> On 10/29/19 10:39 PM, Ori Kam wrote:
> >> On 10/28/19 9:44 PM, Ori Kam wrote:
> >>>> On 10/27/19 3:24 PM, Ori Kam wrote:
> >>>>> + if (rte_eth_dev_is_rx_hairpin_queue(dev, i))
> >>>> The condition should be more tricky if we resetup hairpin queue.
> >>>> I.e. we should check if i is rx_queue_id and count it anyway.
> >>>>
> >>>>> + count++;
> >>>>> + }
> >>>>> + if (count > cap.max_nb_queues) {
> >>>>> + RTE_ETHDEV_LOG(ERR, "To many Rx hairpin
> queues
> >>>> %d",
> >>>>
> >>>> I think it would be useful to log max here as well to catch
> >>>> unset max cases easier.
> >>>>
> >>> I'm not sure I understand.
> >> If the question is about logging, the answer is simple:
> >> if the user forget to initialize maximum number of hairpin queues
> >> properly, it will be zero and setup will fail here. So, it would be
> >> good to log maximum value here just to make it clear which
> >> limit is exceeded.
> >>
> > Maybe I'm missing something but the PMD sets the max number of hairpin
> queues.
>
> Yes, it is just my paranoia to simplify debugging the case if PMD
> forgets to do it.
>
Paranoia is a good thing.
I will add the logging.
> > But in any case I agree we should log what the user requested and what is the
> max
> > that the PMD reports.
> >
> >> If the question is about above check, let's consider the case when
> >> maximum is one and one hairpin queue is already setup, but
> >> user tries to setup one more. Above loop will count only one since
> >> hairpin state for current queue is set below. So, the condition will
> >> allow to setup the second hairpin queue.
> >> In theory, we could initialize cound=1 to count this one, but
> >> it would break the case when we call setup once again for the
> >> queue which is already hairpin. API allows and handles it.
> >>
> > Nice catch. I think the best solution is to compare the count to
> cap.max_nb_queues - 1.
> > and even before this comparison check if the current queue is already hairpin
> queue if so
> > we can skip this check.
> > What do you think?
>
> I think the right solution is always count current queue since it is either
> becoming hairpin or already hairpin, i.e.
>
> if (i == rx_queue_id || rte_eth_dev_is_rx_hairpin_queue(dev, i))
>
> So, the result will be always total number of hairpin queues if
> this one is hairpin.
>
> Andrew.
Good Idea will implement it.
Thanks,
Ori
@@ -224,6 +224,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 queue %"PRIu16" of device with port_id=%"PRIu16" is hairpin queue\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 queue %"PRIu16" of device with port_id=%"PRIu16" is hairpin queue\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 queue %"PRIu16" of device with port_id=%"PRIu16" is hairpin queue\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 queue %"PRIu16" of device with port_id=%"PRIu16" is hairpin queue\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,79 @@ 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 = 0;
+
+ 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);
+ /* Use default specified by driver, if nb_rx_desc is zero */
+ 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;
+ }
+ if (cap.max_nb_queues != UINT16_MAX) {
+ for (i = 0; i < dev->data->nb_rx_queues; i++) {
+ if (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 %d",
+ count);
+ 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 +1979,78 @@ 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 = 0;
+ 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);
+ /* Use default specified by driver, if nb_tx_desc is zero */
+ 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;
+ }
+ if (cap.max_nb_queues != UINT16_MAX) {
+ for (i = 0; i < dev->data->nb_tx_queues; i++) {
+ if (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 %d", count);
+ 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 +4180,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 +4264,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 +4273,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 +4392,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 queue info for Rx queue %"PRIu16" of device with port_id=%"PRIu16" is hairpin queue\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 +4423,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 queue info for Tx queue %"PRIu16" of device with port_id=%"PRIu16" is hairpin queue\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 +4802,21 @@ 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_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,6 +4253,23 @@ 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>
@@ -4251,6 +4370,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 +4641,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,50 @@
*/
#define RTE_ETH_QUEUE_STATE_STOPPED 0
#define RTE_ETH_QUEUE_STATE_STARTED 1
+#define RTE_ETH_QUEUE_STATE_HAIRPIN 2
+
+/**
+ * @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.
+ */
+static inline 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;
+}
+
+
+/**
+ * @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.
+ */
+static inline 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;
+}
/**
* @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;
};