From: Ori Kam <orika@mellanox.com>
To: Andrew Rybchenko <arybchenko@solarflare.com>,
Thomas Monjalon <thomas@monjalon.net>,
Ferruh Yigit <ferruh.yigit@intel.com>
Cc: "dev@dpdk.org" <dev@dpdk.org>,
"jingjing.wu@intel.com" <jingjing.wu@intel.com>,
"stephen@networkplumber.org" <stephen@networkplumber.org>
Subject: Re: [dpdk-dev] [PATCH v2 01/14] ethdev: add support for hairpin queue
Date: Mon, 14 Oct 2019 10:19:30 +0000 [thread overview]
Message-ID: <AM4PR05MB3425D021EF6542F3CBDEEF0EDB900@AM4PR05MB3425.eurprd05.prod.outlook.com> (raw)
In-Reply-To: <e759f562-354b-292e-4e1a-f910ec2fa464@solarflare.com>
Thanks for your comments,
I will start working on V3
Ori
> -----Original Message-----
> From: Andrew Rybchenko <arybchenko@solarflare.com>
> Sent: Monday, October 14, 2019 12:37 PM
> To: Ori Kam <orika@mellanox.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: [PATCH v2 01/14] ethdev: add support for hairpin queue
>
> Hi Ori,
>
> see my answers below.
>
> On 10/11/19 12:07 AM, Ori Kam wrote:
> > Hi Andrew,
> >
> > Thanks for your comments,
> >
> > PSB,
> > Ori
> >
> >> -----Original Message-----
> >> From: Andrew Rybchenko <arybchenko@solarflare.com>
> >> Sent: Tuesday, October 8, 2019 7:11 PM
> >> To: Ori Kam <orika@mellanox.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: [PATCH v2 01/14] ethdev: add support for hairpin queue
> >>
> >> Hi Ori,
> >>
> >> thanks for updated version. See my notes below.
> >>
> >> There are few style notes about line breaks which are not defined in
> >> coding style. Of course, it may be ignored.
> >>
> > I will fix what I can.
> >
> >> On 10/4/19 10:54 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>
> >> rte_eth_dev_[rt]x_queue_stop() should return error if used for hairpin
> >> queue.
> >> Right now rte_eth_dev_[rt]x_queue_start() will return 0. Not sure about it.
> >> What about rte_eth_rx_queue_info_get() and rte_eth_tx_queue_info_get()?
> >> Any other Rx/Tx queue functions?
> >>
> > I will add error to both functions (Tx/Rx)
> > I don't see any other function (only the info_get and queue_stop)
> >
> >>> ---
> >>> V2:
> >>> - update according to ML comments.
> >>>
> >>> ---
> >>> lib/librte_ethdev/rte_ethdev.c | 214
> >> ++++++++++++++++++++++++++++++-
> >>> lib/librte_ethdev/rte_ethdev.h | 126 ++++++++++++++++++
> >>> lib/librte_ethdev/rte_ethdev_core.h | 27 +++-
> >>> lib/librte_ethdev/rte_ethdev_version.map | 5 +
> >>> 4 files changed, 368 insertions(+), 4 deletions(-)
> >>>
> >>> diff --git a/lib/librte_ethdev/rte_ethdev.c b/lib/librte_ethdev/rte_ethdev.c
> >>> index af82360..ee8af42 100644
> >>> --- a/lib/librte_ethdev/rte_ethdev.c
> >>> +++ b/lib/librte_ethdev/rte_ethdev.c
> >>> @@ -1752,12 +1752,102 @@ struct rte_eth_dev *
> >>> if (!dev->data->min_rx_buf_size ||
> >>> dev->data->min_rx_buf_size > mbp_buf_size)
> >>> dev->data->min_rx_buf_size = mbp_buf_size;
> >>> + if (dev->data->rx_queue_state[rx_queue_id] ==
> >>> + RTE_ETH_QUEUE_STATE_HAIRPIN)
> >>> + dev->data->rx_queue_state[rx_queue_id] =
> >>> + RTE_ETH_QUEUE_STATE_STOPPED;
> >> I don't understand it. Why is rte_eth_rx_queue_setup() changed?
> >>
> > This was done so user can reset the queue back to normal queue.
>
> I think it should be done in queue release.
>
> >>> }
> >>>
> >>> return eth_err(port_id, ret);
> >>> }
> >>>
> >>> 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;
> >>> + struct rte_eth_dev_info dev_info;
> >>> + 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;
> >>> + }
> >>> + RTE_FUNC_PTR_OR_ERR_RET(*dev->dev_ops->dev_infos_get, -
> >> ENOTSUP);
> >>> + RTE_FUNC_PTR_OR_ERR_RET(*dev->dev_ops->hairpin_cap_get,
> >>> + -ENOTSUP);
> >>> + RTE_FUNC_PTR_OR_ERR_RET(*dev->dev_ops-
> >>> rx_hairpin_queue_setup,
> >>> + -ENOTSUP);
> >>> + rte_eth_dev_hairpin_capability_get(port_id, &cap);
> >> Return value should be checked. It makes hairpin_cap_get check above
> >> unnecessary.
> >>
> > I will add a check.
> >
> >>> + /* 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;
> >>> + }
> >>> + ret = rte_eth_dev_info_get(port_id, &dev_info);
> >>> + if (ret != 0)
> >>> + return ret;
> >>> + if (dev->data->dev_started &&
> >>> + !(dev_info.dev_capa &
> >>> + RTE_ETH_DEV_CAPA_RUNTIME_RX_QUEUE_SETUP))
> >>> + return -EBUSY;
> >>> + if (dev->data->dev_started &&
> >>> + (dev->data->rx_queue_state[rx_queue_id] !=
> >>> + RTE_ETH_QUEUE_STATE_STOPPED))
> >>> + return -EBUSY;
> >>> + if (conf->peer_n > cap.max_rx_2_tx) {
> >>> + RTE_ETHDEV_LOG(ERR,
> >>> + "Invalid value for number of peers(=%hu), "
> >>> + "should be: <= %hu", conf->peer_n,
> >>> + cap.max_rx_2_tx);
> >>> + return -EINVAL;
> >>> + }
> >>> + if (conf->peer_n == 0) {
> >>> + RTE_ETHDEV_LOG(ERR,
> >>> + "Invalid value for number of peers(=%hu), "
> >>> + "should be: > 0", conf->peer_n);
> >>> + return -EINVAL;
> >>> + }
> >>> + if (cap.max_n_queues != -1) {
> >>> + for (i = 0; i < dev->data->nb_rx_queues; i++) {
> >>> + if (dev->data->rx_queue_state[i] ==
> >>> + RTE_ETH_QUEUE_STATE_HAIRPIN)
> >>> + count++;
> >>> + }
> >>> + if (count > cap.max_n_queues) {
> >>> + RTE_ETHDEV_LOG(ERR,
> >>> + "To many Rx hairpin queues %d", count);
> >>> + return -EINVAL;
> >>> + }
> >>> + }
> >>> + rxq = dev->data->rx_queues;
> >>> + if (rxq[rx_queue_id]) {
> >> Please, compare with NULL (I know that rte_eth_rx_queue_setup() does
> >> like above).
> >>
> > O.K. I will change, but may I ask why? If in the rte_eth_rx_queue_setup
> function it is written the same way,
> > why do you want this change?
>
> https://eur03.safelinks.protection.outlook.com/?url=https%3A%2F%2Fdoc.dpdk.
> org%2Fguides%2Fcontributing%2Fcoding_style.html%23null-
> pointers&data=02%7C01%7Corika%40mellanox.com%7C022cd953964f4a2
> 0d50508d7508a259f%7Ca652971c7d2e4d9ba6a4d149256f461b%7C0%7C0%7C
> 637066426584029629&sdata=IBOo6AyTGn4DiNLHBCC5MbwJkRuqdsGqdrl
> vm0xS9Vw%3D&reserved=0
>
Thanks,
> >>> + 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)
> >> Please, compare with 0
> >>
> > Will do, but again just for my knowledge why?
>
> https://eur03.safelinks.protection.outlook.com/?url=https%3A%2F%2Fdoc.dpdk.
> org%2Fguides%2Fcontributing%2Fcoding_style.html%23function-
> calls&data=02%7C01%7Corika%40mellanox.com%7C022cd953964f4a20d5
> 0508d7508a259f%7Ca652971c7d2e4d9ba6a4d149256f461b%7C0%7C0%7C637
> 066426584029629&sdata=G8ZxEWFFsv1kLWc6L7sKT8O6CSiBcj5ZuwQqmK
> 0Q6nY%3D&reserved=0
>
Thanks,
> >>> + 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)
> >>> @@ -1851,9 +1941,97 @@ struct rte_eth_dev *
> >>> __func__);
> >>> return -EINVAL;
> >>> }
> >>> + ret = (*dev->dev_ops->tx_queue_setup)(dev, tx_queue_id, nb_tx_desc,
> >>> + socket_id, &local_conf);
> >>> + if (!ret)
> >>> + if (dev->data->tx_queue_state[tx_queue_id] ==
> >>> + RTE_ETH_QUEUE_STATE_HAIRPIN)
> >>> + dev->data->tx_queue_state[tx_queue_id] =
> >>> + RTE_ETH_QUEUE_STATE_STOPPED;
> >> Why is it changed?
> >>
> > Like in the Rx to enable switching back to normal queue.
> >
> >>> + return eth_err(port_id, ret);
> >>> +}
> >>> +
> >>> +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;
> >>> + struct rte_eth_dev_info dev_info;
> >>> + void **txq;
> >>> + int i;
> >>> + int count = 0;
> >>> + int ret;
> >>>
> >>> - return eth_err(port_id, (*dev->dev_ops->tx_queue_setup)(dev,
> >>> - tx_queue_id, nb_tx_desc, socket_id, &local_conf));
> >>> + 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;
> >>> + }
> >>> + RTE_FUNC_PTR_OR_ERR_RET(*dev->dev_ops->dev_infos_get, -
> >> ENOTSUP);
> >>> + RTE_FUNC_PTR_OR_ERR_RET(*dev->dev_ops->hairpin_cap_get,
> >>> + -ENOTSUP);
> >>> + RTE_FUNC_PTR_OR_ERR_RET(*dev->dev_ops-
> >>> tx_hairpin_queue_setup,
> >>> + -ENOTSUP);
> >>> + rte_eth_dev_info_get(port_id, &dev_info);
> >> return value should be checked.
> >>
> >>> + rte_eth_dev_hairpin_capability_get(port_id, &cap);
> >> Check return value and you can rely on hairpin_cap_get check inside.
> >>
> > Same as above I will add a check but I'm not sure I understand the second
> part
> > Just to make sure I understand, you are talking about that if I don't check
> > the return value I'm not allowed to check the cap right?
>
> It is about dev_ops->hairpin_cap_get vs NULL check.
> It is checked inside rte_eth_dev_hairpin_capability_get() and we should
> not duplicate the check above.
>
O.K.
> >>> + /* 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_n > cap.max_tx_2_rx) {
> >>> + RTE_ETHDEV_LOG(ERR,
> >>> + "Invalid value for number of peers(=%hu), "
> >>> + "should be: <= %hu", conf->peer_n,
> >>> + cap.max_tx_2_rx);
> >>> + return -EINVAL;
> >>> + }
> >>> + if (conf->peer_n == 0) {
> >>> + RTE_ETHDEV_LOG(ERR,
> >>> + "Invalid value for number of peers(=%hu), "
> >>> + "should be: > 0", conf->peer_n);
> >>> + return -EINVAL;
> >>> + }
> >>> + if (cap.max_n_queues != -1) {
> >>> + for (i = 0; i < dev->data->nb_tx_queues; i++) {
> >>> + if (dev->data->tx_queue_state[i] ==
> >>> + RTE_ETH_QUEUE_STATE_HAIRPIN)
> >>> + count++;
> >>> + }
> >>> + if (count > cap.max_n_queues) {
> >>> + RTE_ETHDEV_LOG(ERR,
> >>> + "To many Rx hairpin queues %d", count);
> >>> + return -EINVAL;
> >>> + }
> >>> + }
> >>> + if (dev->data->dev_started &&
> >>> + !(dev_info.dev_capa &
> >>> + RTE_ETH_DEV_CAPA_RUNTIME_TX_QUEUE_SETUP))
> >>> + return -EBUSY;
> >>> + if (dev->data->dev_started &&
> >>> + (dev->data->tx_queue_state[tx_queue_id] !=
> >>> + RTE_ETH_QUEUE_STATE_STOPPED))
> >>> + return -EBUSY;
> >>> + txq = dev->data->tx_queues;
> >>> + if (txq[tx_queue_id]) {
> >> Please, compare with NULL
> >>
> > O.K.
> >
> >>> + 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)
> >> Please, compare with 0
> >>
> > O.K.
> >
> >>> + dev->data->tx_queue_state[tx_queue_id] =
> >>> + RTE_ETH_QUEUE_STATE_HAIRPIN;
> >>> + return eth_err(port_id, ret);
> >>> }
> >>>
> >>> void
> >>> @@ -3981,12 +4159,20 @@ 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 (dev->data->rx_queue_state[queue_id] ==
> >>> + RTE_ETH_QUEUE_STATE_HAIRPIN) {
> >> It looks like line break is not required above. Just to make code a bit
> >> shorter.
> >>
> > Will fix if possible.
> >
> >>> + rte_errno = EINVAL;
> >>> + return NULL;
> >>> + }
> >>> struct rte_eth_rxtx_callback *cb = rte_zmalloc(NULL, sizeof(*cb), 0);
> >>>
> >>> if (cb == NULL) {
> >>> @@ -4058,6 +4244,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) {
> >>> @@ -4065,6 +4253,13 @@ int rte_eth_set_queue_rate_limit(uint16_t
> >> port_id, uint16_t queue_idx,
> >>> return NULL;
> >>> }
> >>>
> >>> + dev = &rte_eth_devices[port_id];
> >>> + if (dev->data->tx_queue_state[queue_id] ==
> >>> + RTE_ETH_QUEUE_STATE_HAIRPIN) {
> >> It looks like line break is not required above. Just to make code a bit
> >> shorter.
> >>
> > Will fix.
> >
> >>> + rte_errno = EINVAL;
> >>> + return NULL;
> >>> + }
> >>> +
> >>> struct rte_eth_rxtx_callback *cb = rte_zmalloc(NULL, sizeof(*cb), 0);
> >>>
> >>> if (cb == NULL) {
> >>> @@ -4510,6 +4705,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);
> >> I think it would be useful to memset() cap with 0 to be sure that it is
> >> initialized.
> >>
> > O.K but if we follow your comments from above, the result is not valid if we
> return
> > error, so it is not a must, but like I said I don't care to add it.
> >
> >>> + return eth_err(port_id, (*dev->dev_ops->hairpin_cap_get)
> >>> + (dev, cap));
> >>
> >> Please, consider to avoid line breaks above to make code a bit shorter.
> >> Of course, it should not exceed line length limit.
> >>
> > Will check.
> >
> >>> +}
> >>> +
> >>> +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 d937fb4..29dcfea 100644
> >>> --- a/lib/librte_ethdev/rte_ethdev.h
> >>> +++ b/lib/librte_ethdev/rte_ethdev.h
> >>> @@ -804,6 +804,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 supportd.
> >> supportd -> supported
> >>
> > Will fix.
> >
> >>> + */
> >>> +struct rte_eth_hairpin_cap {
> >>> + int16_t max_n_queues;
> >>> + /**< The max number of hairpin queuesi. -1 no limit. */
> >> I'm not sure that I like type difference from max_rx_queues here.
> >> I think it would be better to use uint16_t. I would say there is point
> >> to highlight no limit (first of all I'm not sure that it makes sense,
> >> second, UINT16_MAX will obviously do the job in uint16_t.
> >>
> >> Is it both Rx and Tx?
> >>
> >>> + int16_t max_rx_2_tx;
> >>> + /**< Max number of Rx queues to be connected to one Tx queue. */
> >>> + int16_t max_tx_2_rx;
> >>> + /**< Max number of Tx queues to be connected to one Rx queue. */
> >> I would prefer to have uint16_t here as well. Mainly for consistency.
> >>
> > Agree, will fix,
> >
> >>> + uint16_t max_nb_desc; /**< The max num of descriptors. */
> >> Is it common for Rx and Tx? What about min?
> >>
> > I think min is always 1, by definition.
>
> OK
>
> >>> +};
> >>> +
> >>> +#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.
> >> It should be explained what happens if Rx queue has many Tx queues.
> >> Is packet duplicated? Or distributed?
> >>
> > Like we said before, I don't know it depends on the Nic.
> > In case of Mellanox we don't support 1 to many.
>
> I see, but what application should expect?
I think that when some Nic supports such connection, it should add in the capabilities
or in documentation the expected results.
Personally I think that the common use case will be distributed (RSS on the Tx) but it is only
my personal thinking, your question will be answered when someone will implement it.
>
> >>> + */
> >>> +struct rte_eth_hairpin_conf {
> >>> + uint16_t peer_n; /**< 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 {
> >>> @@ -1080,6 +1120,8 @@ struct rte_eth_conf {
> >>> /**< Device supports Rx queue setup after device started*/
> >>> #define RTE_ETH_DEV_CAPA_RUNTIME_TX_QUEUE_SETUP 0x00000002
> >>> /**< Device supports Tx queue setup after device started*/
> >>> +#define RTE_ETH_DEV_CAPA_HAIRPIN_SUPPORT 0x00000004
> >>> +/**< Device supports hairpin queues. */
> >> Do we really need it? Isn't rte_eth_dev_hairpin_capability_get() returning
> >> -ENOTSUP sufficient?
> >>
> > I agree with you, but my thinking was that using the cap the application
> > can avoid calling to function that will fail.
> > If you approve I will remove this cap.
>
> Runtime setup flags are required since these functions are always
> supported, but it is not alway possible to use these function when
> device is started. In this case I think there is no necessity to add
> the capability since we have no similar caps for anything else.
>
O.K. will remove it.
> >>> /*
> >>> * If new Tx offload capabilities are defined, they also must be
> >>> @@ -1277,6 +1319,7 @@ struct rte_eth_dcb_info {
> >>> */
> >>> #define RTE_ETH_QUEUE_STATE_STOPPED 0
> >>> #define RTE_ETH_QUEUE_STATE_STARTED 1
> >>> +#define RTE_ETH_QUEUE_STATE_HAIRPIN 2
> >>>
> >>> #define RTE_ETH_ALL RTE_MAX_ETHPORTS
> >>>
> >>> @@ -1771,6 +1814,34 @@ 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.
> >> Is it allows to specify 0 to pick driver recommended value?
> >>
> > Yes, (assuming you are talking about the nb_rx_desc)
>
> Please, highlight it in the description.
>
O.K.
[nip]
Thanks,
Ori
next prev parent reply other threads:[~2019-10-14 10:19 UTC|newest]
Thread overview: 186+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-09-26 6:28 [dpdk-dev] [PATCH 00/13] add hairpin feature Ori Kam
2019-09-26 6:28 ` [dpdk-dev] [PATCH 01/13] ethdev: support setup function for hairpin queue Ori Kam
2019-09-26 12:18 ` Andrew Rybchenko
[not found] ` <AM0PR0502MB4019A2FEADE5F9DCD0D9DDFED2860@AM0PR0502MB4019.eurprd05.prod.outlook.com>
2019-09-26 15:58 ` Ori Kam
2019-09-26 17:24 ` Andrew Rybchenko
2019-09-28 15:19 ` Ori Kam
2019-09-29 12:10 ` Andrew Rybchenko
2019-10-02 12:19 ` Ori Kam
2019-10-03 13:26 ` Andrew Rybchenko
2019-10-03 17:46 ` Ori Kam
2019-10-03 18:39 ` Ray Kinsella
2019-09-26 6:28 ` [dpdk-dev] [PATCH 02/13] net/mlx5: query hca hairpin capabilities Ori Kam
2019-09-26 9:31 ` Slava Ovsiienko
2019-09-26 6:28 ` [dpdk-dev] [PATCH 03/13] net/mlx5: support Rx hairpin queues Ori Kam
2019-09-26 9:32 ` Slava Ovsiienko
2019-09-26 6:29 ` [dpdk-dev] [PATCH 04/13] net/mlx5: prepare txq to work with different types Ori Kam
2019-09-26 9:32 ` Slava Ovsiienko
2019-09-26 6:29 ` [dpdk-dev] [PATCH 05/13] net/mlx5: support Tx hairpin queues Ori Kam
2019-09-26 9:32 ` Slava Ovsiienko
2019-09-26 6:29 ` [dpdk-dev] [PATCH 06/13] app/testpmd: add hairpin support Ori Kam
2019-09-26 9:32 ` Slava Ovsiienko
2019-09-26 6:29 ` [dpdk-dev] [PATCH 07/13] net/mlx5: add hairpin binding function Ori Kam
2019-09-26 9:33 ` Slava Ovsiienko
2019-09-26 6:29 ` [dpdk-dev] [PATCH 08/13] net/mlx5: add support for hairpin hrxq Ori Kam
2019-09-26 9:33 ` Slava Ovsiienko
2019-09-26 6:29 ` [dpdk-dev] [PATCH 09/13] net/mlx5: add internal tag item and action Ori Kam
2019-09-26 9:33 ` Slava Ovsiienko
2019-09-26 6:29 ` [dpdk-dev] [PATCH 10/13] net/mlx5: add id generation function Ori Kam
2019-09-26 9:34 ` Slava Ovsiienko
2019-09-26 6:29 ` [dpdk-dev] [PATCH 11/13] net/mlx5: add default flows for hairpin Ori Kam
2019-09-26 9:34 ` Slava Ovsiienko
2019-09-26 6:29 ` [dpdk-dev] [PATCH 12/13] net/mlx5: split hairpin flows Ori Kam
2019-09-26 9:34 ` Slava Ovsiienko
2019-09-26 6:29 ` [dpdk-dev] [PATCH 13/13] doc: add hairpin feature Ori Kam
2019-09-26 9:34 ` Slava Ovsiienko
2019-09-26 12:32 ` [dpdk-dev] [PATCH 00/13] " Andrew Rybchenko
2019-09-26 15:22 ` Ori Kam
2019-09-26 15:48 ` Andrew Rybchenko
2019-09-26 16:11 ` Ori Kam
2019-10-04 19:54 ` [dpdk-dev] [PATCH v2 00/14] " Ori Kam
2019-10-04 19:54 ` [dpdk-dev] [PATCH v2 01/14] ethdev: add support for hairpin queue Ori Kam
2019-10-08 16:11 ` Andrew Rybchenko
2019-10-10 21:07 ` Ori Kam
2019-10-14 9:37 ` Andrew Rybchenko
2019-10-14 10:19 ` Ori Kam [this message]
2019-10-04 19:54 ` [dpdk-dev] [PATCH v2 02/14] net/mlx5: query hca hairpin capabilities Ori Kam
2019-10-04 19:54 ` [dpdk-dev] [PATCH v2 03/14] net/mlx5: support Rx hairpin queues Ori Kam
2019-10-04 19:54 ` [dpdk-dev] [PATCH v2 04/14] net/mlx5: prepare txq to work with different types Ori Kam
2019-10-04 19:54 ` [dpdk-dev] [PATCH v2 05/14] net/mlx5: support Tx hairpin queues Ori Kam
2019-10-04 19:54 ` [dpdk-dev] [PATCH v2 06/14] net/mlx5: add get hairpin capabilities Ori Kam
2019-10-04 19:54 ` [dpdk-dev] [PATCH v2 07/14] app/testpmd: add hairpin support Ori Kam
2019-10-04 19:54 ` [dpdk-dev] [PATCH v2 08/14] net/mlx5: add hairpin binding function Ori Kam
2019-10-04 19:54 ` [dpdk-dev] [PATCH v2 09/14] net/mlx5: add support for hairpin hrxq Ori Kam
2019-10-04 19:54 ` [dpdk-dev] [PATCH v2 10/14] net/mlx5: add internal tag item and action Ori Kam
2019-10-04 19:54 ` [dpdk-dev] [PATCH v2 11/14] net/mlx5: add id generation function Ori Kam
2019-10-04 19:54 ` [dpdk-dev] [PATCH v2 12/14] net/mlx5: add default flows for hairpin Ori Kam
2019-10-04 19:54 ` [dpdk-dev] [PATCH v2 13/14] net/mlx5: split hairpin flows Ori Kam
2019-10-04 19:54 ` [dpdk-dev] [PATCH v2 14/14] doc: add hairpin feature Ori Kam
2019-10-08 14:55 ` Andrew Rybchenko
2019-10-10 8:24 ` Ori Kam
2019-10-15 9:04 ` [dpdk-dev] [PATCH v3 00/14] " Ori Kam
2019-10-15 9:04 ` [dpdk-dev] [PATCH v3 01/14] ethdev: add support for hairpin queue Ori Kam
2019-10-15 10:12 ` Andrew Rybchenko
2019-10-16 19:36 ` Ori Kam
2019-10-17 10:41 ` Andrew Rybchenko
2019-10-15 9:04 ` [dpdk-dev] [PATCH v3 02/14] net/mlx5: query hca hairpin capabilities Ori Kam
2019-10-15 9:04 ` [dpdk-dev] [PATCH v3 03/14] net/mlx5: support Rx hairpin queues Ori Kam
2019-10-15 9:04 ` [dpdk-dev] [PATCH v3 04/14] net/mlx5: prepare txq to work with different types Ori Kam
2019-10-15 9:04 ` [dpdk-dev] [PATCH v3 05/14] net/mlx5: support Tx hairpin queues Ori Kam
2019-10-15 9:04 ` [dpdk-dev] [PATCH v3 06/14] net/mlx5: add get hairpin capabilities Ori Kam
2019-10-15 9:04 ` [dpdk-dev] [PATCH v3 07/14] app/testpmd: add hairpin support Ori Kam
2019-10-15 9:04 ` [dpdk-dev] [PATCH v3 08/14] net/mlx5: add hairpin binding function Ori Kam
2019-10-15 9:04 ` [dpdk-dev] [PATCH v3 09/14] net/mlx5: add support for hairpin hrxq Ori Kam
2019-10-15 9:04 ` [dpdk-dev] [PATCH v3 10/14] net/mlx5: add internal tag item and action Ori Kam
2019-10-15 9:04 ` [dpdk-dev] [PATCH v3 11/14] net/mlx5: add id generation function Ori Kam
2019-10-15 9:04 ` [dpdk-dev] [PATCH v3 12/14] net/mlx5: add default flows for hairpin Ori Kam
2019-10-15 9:04 ` [dpdk-dev] [PATCH v3 13/14] net/mlx5: split hairpin flows Ori Kam
2019-10-15 9:04 ` [dpdk-dev] [PATCH v3 14/14] doc: add hairpin feature Ori Kam
2019-10-17 15:32 ` [dpdk-dev] [PATCH v4 00/15] " Ori Kam
2019-10-17 15:32 ` [dpdk-dev] [PATCH v4 01/15] ethdev: move queue state defines to private file Ori Kam
2019-10-17 15:37 ` Stephen Hemminger
2019-10-22 10:59 ` Andrew Rybchenko
2019-10-17 15:32 ` [dpdk-dev] [PATCH v4 02/15] ethdev: add support for hairpin queue Ori Kam
2019-10-17 21:01 ` Thomas Monjalon
2019-10-22 11:37 ` Andrew Rybchenko
2019-10-23 6:23 ` Ori Kam
2019-10-23 7:04 ` Thomas Monjalon
2019-10-23 10:09 ` Ori Kam
2019-10-23 10:18 ` Bruce Richardson
2019-10-17 15:32 ` [dpdk-dev] [PATCH v4 03/15] net/mlx5: query hca hairpin capabilities Ori Kam
2019-10-17 15:32 ` [dpdk-dev] [PATCH v4 04/15] net/mlx5: support Rx hairpin queues Ori Kam
2019-10-17 15:32 ` [dpdk-dev] [PATCH v4 05/15] net/mlx5: prepare txq to work with different types Ori Kam
2019-10-17 15:32 ` [dpdk-dev] [PATCH v4 06/15] net/mlx5: support Tx hairpin queues Ori Kam
2019-10-17 15:32 ` [dpdk-dev] [PATCH v4 07/15] net/mlx5: add get hairpin capabilities Ori Kam
2019-10-17 15:32 ` [dpdk-dev] [PATCH v4 08/15] app/testpmd: add hairpin support Ori Kam
2019-10-17 15:32 ` [dpdk-dev] [PATCH v4 09/15] net/mlx5: add hairpin binding function Ori Kam
2019-10-17 15:32 ` [dpdk-dev] [PATCH v4 10/15] net/mlx5: add support for hairpin hrxq Ori Kam
2019-10-17 15:32 ` [dpdk-dev] [PATCH v4 11/15] net/mlx5: add internal tag item and action Ori Kam
2019-10-17 15:32 ` [dpdk-dev] [PATCH v4 12/15] net/mlx5: add id generation function Ori Kam
2019-10-17 15:32 ` [dpdk-dev] [PATCH v4 13/15] net/mlx5: add default flows for hairpin Ori Kam
2019-10-17 15:32 ` [dpdk-dev] [PATCH v4 14/15] net/mlx5: split hairpin flows Ori Kam
2019-10-17 15:32 ` [dpdk-dev] [PATCH v4 15/15] doc: add hairpin feature Ori Kam
2019-10-18 19:07 ` [dpdk-dev] [PATCH v4 00/15] " Ferruh Yigit
2019-10-23 13:37 ` [dpdk-dev] [PATCH v5 " Ori Kam
2019-10-23 13:37 ` [dpdk-dev] [PATCH v5 01/15] ethdev: move queue state defines to private file Ori Kam
2019-10-23 13:37 ` [dpdk-dev] [PATCH v5 02/15] ethdev: add support for hairpin queue Ori Kam
2019-10-24 7:54 ` Andrew Rybchenko
2019-10-24 8:29 ` Ori Kam
2019-10-24 14:47 ` Andrew Rybchenko
2019-10-24 15:17 ` Thomas Monjalon
2019-10-24 15:30 ` Andrew Rybchenko
2019-10-24 15:34 ` Thomas Monjalon
2019-10-25 19:01 ` Ori Kam
2019-10-25 22:16 ` Thomas Monjalon
2019-10-23 13:37 ` [dpdk-dev] [PATCH v5 03/15] net/mlx5: query hca hairpin capabilities Ori Kam
2019-10-23 13:37 ` [dpdk-dev] [PATCH v5 04/15] net/mlx5: support Rx hairpin queues Ori Kam
2019-10-23 13:37 ` [dpdk-dev] [PATCH v5 05/15] net/mlx5: prepare txq to work with different types Ori Kam
2019-10-23 13:37 ` [dpdk-dev] [PATCH v5 06/15] net/mlx5: support Tx hairpin queues Ori Kam
2019-10-23 13:37 ` [dpdk-dev] [PATCH v5 07/15] net/mlx5: add get hairpin capabilities Ori Kam
2019-10-23 13:37 ` [dpdk-dev] [PATCH v5 08/15] app/testpmd: add hairpin support Ori Kam
2019-10-23 13:37 ` [dpdk-dev] [PATCH v5 09/15] net/mlx5: add hairpin binding function Ori Kam
2019-10-23 13:37 ` [dpdk-dev] [PATCH v5 10/15] net/mlx5: add support for hairpin hrxq Ori Kam
2019-10-23 13:37 ` [dpdk-dev] [PATCH v5 11/15] net/mlx5: add internal tag item and action Ori Kam
2019-10-23 13:37 ` [dpdk-dev] [PATCH v5 12/15] net/mlx5: add id generation function Ori Kam
2019-10-23 13:37 ` [dpdk-dev] [PATCH v5 13/15] net/mlx5: add default flows for hairpin Ori Kam
2019-10-23 13:37 ` [dpdk-dev] [PATCH v5 14/15] net/mlx5: split hairpin flows Ori Kam
2019-10-23 13:37 ` [dpdk-dev] [PATCH v5 15/15] doc: add hairpin feature Ori Kam
2019-10-24 8:11 ` Thomas Monjalon
2019-10-25 18:49 ` [dpdk-dev] [PATCH v5 00/15] " Ferruh Yigit
2019-10-27 12:24 ` [dpdk-dev] [PATCH v6 00/14] " Ori Kam
2019-10-27 12:24 ` [dpdk-dev] [PATCH v6 01/14] ethdev: move queue state defines to private file Ori Kam
2019-10-27 12:24 ` [dpdk-dev] [PATCH v6 02/14] ethdev: add support for hairpin queue Ori Kam
2019-10-28 15:16 ` Andrew Rybchenko
2019-10-28 18:44 ` Ori Kam
2019-10-29 7:38 ` Andrew Rybchenko
2019-10-29 19:39 ` Ori Kam
2019-10-30 6:39 ` Andrew Rybchenko
2019-10-30 6:56 ` Ori Kam
2019-10-27 12:24 ` [dpdk-dev] [PATCH v6 03/14] net/mlx5: query hca hairpin capabilities Ori Kam
2019-10-27 12:24 ` [dpdk-dev] [PATCH v6 04/14] net/mlx5: support Rx hairpin queues Ori Kam
2019-10-27 12:24 ` [dpdk-dev] [PATCH v6 05/14] net/mlx5: prepare txq to work with different types Ori Kam
2019-10-27 12:24 ` [dpdk-dev] [PATCH v6 06/14] net/mlx5: support Tx hairpin queues Ori Kam
2019-10-27 12:24 ` [dpdk-dev] [PATCH v6 07/14] net/mlx5: add get hairpin capabilities Ori Kam
2019-10-27 12:24 ` [dpdk-dev] [PATCH v6 08/14] app/testpmd: add hairpin support Ori Kam
2019-10-27 12:24 ` [dpdk-dev] [PATCH v6 09/14] net/mlx5: add hairpin binding function Ori Kam
2019-10-27 12:24 ` [dpdk-dev] [PATCH v6 10/14] net/mlx5: add support for hairpin hrxq Ori Kam
2019-10-27 12:24 ` [dpdk-dev] [PATCH v6 11/14] net/mlx5: add internal tag item and action Ori Kam
2019-10-27 12:24 ` [dpdk-dev] [PATCH v6 12/14] net/mlx5: add id generation function Ori Kam
2019-10-27 12:25 ` [dpdk-dev] [PATCH v6 13/14] net/mlx5: add default flows for hairpin Ori Kam
2019-10-27 12:25 ` [dpdk-dev] [PATCH v6 14/14] net/mlx5: split hairpin flows Ori Kam
2019-10-30 23:53 ` [dpdk-dev] [PATCH v7 00/14] add hairpin feature Ori Kam
2019-10-30 23:53 ` [dpdk-dev] [PATCH v7 01/14] ethdev: move queue state defines to private file Ori Kam
2019-10-30 23:53 ` [dpdk-dev] [PATCH v7 02/14] ethdev: add support for hairpin queue Ori Kam
2019-10-31 8:25 ` Andrew Rybchenko
2019-11-05 11:24 ` Ferruh Yigit
2019-11-05 11:36 ` Ori Kam
2019-11-05 11:49 ` Andrew Rybchenko
2019-11-05 12:00 ` Ori Kam
2019-11-05 12:05 ` Ferruh Yigit
2019-11-05 12:12 ` Andrew Rybchenko
2019-11-05 12:23 ` Ferruh Yigit
2019-11-05 12:27 ` Andrew Rybchenko
2019-11-05 12:51 ` Thomas Monjalon
2019-11-05 12:53 ` Andrew Rybchenko
2019-11-05 13:02 ` Thomas Monjalon
2019-11-05 13:23 ` Ori Kam
2019-11-05 13:27 ` Thomas Monjalon
2019-11-05 13:34 ` Andrew Rybchenko
2019-11-05 13:41 ` Andrew Rybchenko
2019-10-30 23:53 ` [dpdk-dev] [PATCH v7 03/14] net/mlx5: query hca hairpin capabilities Ori Kam
2019-10-30 23:53 ` [dpdk-dev] [PATCH v7 04/14] net/mlx5: support Rx hairpin queues Ori Kam
2019-10-30 23:53 ` [dpdk-dev] [PATCH v7 05/14] net/mlx5: prepare txq to work with different types Ori Kam
2019-10-30 23:53 ` [dpdk-dev] [PATCH v7 06/14] net/mlx5: support Tx hairpin queues Ori Kam
2019-10-30 23:53 ` [dpdk-dev] [PATCH v7 07/14] net/mlx5: add get hairpin capabilities Ori Kam
2019-10-30 23:53 ` [dpdk-dev] [PATCH v7 08/14] app/testpmd: add hairpin support Ori Kam
2019-10-31 17:11 ` Ferruh Yigit
2019-10-31 17:36 ` Ori Kam
2019-10-31 17:54 ` Ferruh Yigit
2019-10-31 18:59 ` Ori Kam
2019-10-30 23:53 ` [dpdk-dev] [PATCH v7 09/14] net/mlx5: add hairpin binding function Ori Kam
2019-10-30 23:53 ` [dpdk-dev] [PATCH v7 10/14] net/mlx5: add support for hairpin hrxq Ori Kam
2019-10-30 23:53 ` [dpdk-dev] [PATCH v7 11/14] net/mlx5: add internal tag item and action Ori Kam
2019-10-30 23:53 ` [dpdk-dev] [PATCH v7 12/14] net/mlx5: add id generation function Ori Kam
2019-10-30 23:53 ` [dpdk-dev] [PATCH v7 13/14] net/mlx5: add default flows for hairpin Ori Kam
2019-10-30 23:53 ` [dpdk-dev] [PATCH v7 14/14] net/mlx5: split hairpin flows Ori Kam
2019-10-31 17:13 ` [dpdk-dev] [PATCH v7 00/14] add hairpin feature Ferruh Yigit
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=AM4PR05MB3425D021EF6542F3CBDEEF0EDB900@AM4PR05MB3425.eurprd05.prod.outlook.com \
--to=orika@mellanox.com \
--cc=arybchenko@solarflare.com \
--cc=dev@dpdk.org \
--cc=ferruh.yigit@intel.com \
--cc=jingjing.wu@intel.com \
--cc=stephen@networkplumber.org \
--cc=thomas@monjalon.net \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).