From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from dpdk.org (dpdk.org [92.243.14.124]) by inbox.dpdk.org (Postfix) with ESMTP id 9630BA00BE; Tue, 29 Oct 2019 08:38:58 +0100 (CET) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id 1376F1BED3; Tue, 29 Oct 2019 08:38:58 +0100 (CET) Received: from dispatch1-us1.ppe-hosted.com (dispatch1-us1.ppe-hosted.com [148.163.129.52]) by dpdk.org (Postfix) with ESMTP id 79A3F1F5 for ; Tue, 29 Oct 2019 08:38:56 +0100 (CET) X-Virus-Scanned: Proofpoint Essentials engine Received: from webmail.solarflare.com (uk.solarflare.com [193.34.186.16]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-SHA384 (256/256 bits)) (No client certificate requested) by mx1-us3.ppe-hosted.com (PPE Hosted ESMTP Server) with ESMTPS id D55A248006A; Tue, 29 Oct 2019 07:38:53 +0000 (UTC) Received: from [192.168.38.17] (91.220.146.112) by ukex01.SolarFlarecom.com (10.17.10.4) with Microsoft SMTP Server (TLS) id 15.0.1395.4; Tue, 29 Oct 2019 07:38:45 +0000 To: Ori Kam , John McNamara , Marko Kovacevic , Thomas Monjalon , Ferruh Yigit CC: "dev@dpdk.org" , "jingjing.wu@intel.com" , "stephen@networkplumber.org" References: <1569479349-36962-1-git-send-email-orika@mellanox.com> <1572179102-163236-1-git-send-email-orika@mellanox.com> <1572179102-163236-3-git-send-email-orika@mellanox.com> <0ceaf36f-a18d-4bb8-0356-7dc28057ae2f@solarflare.com> From: Andrew Rybchenko Message-ID: Date: Tue, 29 Oct 2019 10:38:41 +0300 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.9.0 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset="utf-8"; format=flowed Content-Transfer-Encoding: 8bit Content-Language: en-GB X-Originating-IP: [91.220.146.112] X-ClientProxiedBy: ocex03.SolarFlarecom.com (10.20.40.36) To ukex01.SolarFlarecom.com (10.17.10.4) X-TM-AS-Product-Ver: SMEX-12.5.0.1300-8.5.1010-25008.003 X-TM-AS-Result: No-12.270200-8.000000-10 X-TMASE-MatchedRID: 8HTFlOrbAtHmLzc6AOD8DfHkpkyUphL9Ud7Bjfo+5jTVEYfep7SzppWi xnDxF7NQtqy4fGtVdQqTH1CW/Tkdqh8owvkAm18tMiMrbc70PfdKgIbix5+XxHy/Hx1AgJrrNrO 5iotCPoZh3GzIVInWKBk2nfcAgjJAlWwztvtayXUtuNep3N20JS+w46aA6jWZbJ0dUVCXnPR1ql G6ceWU3Stede/KJ7VLuUh+Til4IvMdBxyJwXmivaNT4eJsLoMras1wAJHsfyJ7eGs179ltWa2Sw IjG5UkRMYnDVMyJZTv2giI8Cizol79z4PlYm1BmCLQsumV/5S8K696Fr7oYGMLmw90hW1s77n9V s5dSf+vzJ1FYvQ1M8TfRt2mcYU6gpeAIK6XfhPqCYB3gC22mf0tc8DbogbSE31GU/N5W5BDDw2s knbo+ZWvHBlNaXPoaS1Q9HwKD1wq4Gwz1apLtugXGi/7cli9j+IfriO3cV8Q4v/3Wd2CNuW8owp OmCmRIblc670GKxHV2ABUBrtEwpcLmHS108BqLHPYwOJi6PLlM8zV+hbhmA8OtrkhuZC9W16PIr 2HYULFWBsmZtTuN0LeR8WxecLNJ4hjUBITbCT52GcWKGZufBTz/1sPAs6BHmJBe2bRXwlMGMScp H7qOAAcuLuUim0bVqxUkxD1PpJYuQK2tsehFmovqrlGw2G/k+li7VJuaaADb4C+yr/MvQWcuY4i x4PoKzT9JS//OrWnAUn+gwJj2f09cAHNo1eqrqjZ865FPtppoCCPatSxjlZc8I4HeDcpLVCB++i itAT7yn5LqqvsUHqefyJf3QLyZqAWKvlXcAJ2eAiCmPx4NwFkMvWAuahr8+gD2vYtOFhgqtq5d3 cxkNQwWxr7XDKH8uttsXOJt/fJdlGCl9JbXprKGEN/E+xIwUZWrce+mOkFucdZ+nC3yvA== X-TM-AS-User-Approved-Sender: Yes X-TM-AS-User-Blocked-Sender: No X-TMASE-Result: 10--12.270200-8.000000 X-TMASE-Version: SMEX-12.5.0.1300-8.5.1010-25008.003 X-MDID: 1572334735-x7WrvqmzFC1j Subject: Re: [dpdk-dev] [PATCH v6 02/14] ethdev: add support for hairpin queue X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" On 10/28/19 9:44 PM, Ori Kam wrote: > Hi Andrew, > > >> -----Original Message----- >> From: Andrew Rybchenko >> Sent: Monday, October 28, 2019 5:16 PM >> To: Ori Kam ; John McNamara >> ; Marko Kovacevic >> ; Thomas Monjalon ; >> Ferruh Yigit >> 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 >>> Reviewed-by: Andrew Rybchenko >> 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