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 09AD0A32A2 for ; Thu, 24 Oct 2019 16:48:02 +0200 (CEST) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id 2AA231EB0E; Thu, 24 Oct 2019 16:48:02 +0200 (CEST) Received: from dispatch1-us1.ppe-hosted.com (dispatch1-us1.ppe-hosted.com [148.163.129.52]) by dpdk.org (Postfix) with ESMTP id 1547B1EB0A for ; Thu, 24 Oct 2019 16:48:00 +0200 (CEST) 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 17AF9B40085; Thu, 24 Oct 2019 14:47:58 +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; Thu, 24 Oct 2019 15:47:52 +0100 To: Ori Kam , 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> <1571837852-45975-1-git-send-email-orika@mellanox.com> <1571837852-45975-3-git-send-email-orika@mellanox.com> <473249c0-4f96-7f64-83a4-be612ede1dd3@solarflare.com> From: Andrew Rybchenko Message-ID: Date: Thu, 24 Oct 2019 17:47:48 +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: 7bit 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-24998.003 X-TM-AS-Result: No-11.773300-8.000000-10 X-TMASE-MatchedRID: fE0JoqABJp3mLzc6AOD8DfHkpkyUphL9BnIRIVcCWN9+SLLtNOiBhp1b YL8THayDnnt2ffmyyM4mWLi5gN9xFpYWMVQkytLjzNY33yIEF4Z4PXLLrqnAoB3RY4pGTCyH4w1 D/FF1D52w6C/crbkfaSoijibIe+MHKh5xfxw1U6FOHhVJB+QhXG4gOG7cxxfbN8Ttkd26AVgibo uXLfNTTRe7jLTk+2Ivhgd8iRwssrrTgcOJmvNIXS2416nc3bQleRqZANnpuF8HZBaLwEXlKIpbw G9fIuITk9r2/LjEfWhhP9rG6e+NGKGnquim0WHzSHCU59h5KrGbYJTL8pwAntqUml4VvcDY7X6x +u30BYiWkqk2IvguWvBWlqoQgSfA97rw/tQZJe04YnIkEf/g0Gpiq4KsutXCmyiLZetSf8nJ4y0 wP1A6AKEwgORH8p/AjaPj0W1qn0Q7AFczfjr/7EAD/ZrGT2uGCZWxuMxv4M9mNwajEni7G1i5Sq QBRYL/N2kP8OrmxVI= X-TM-AS-User-Approved-Sender: Yes X-TM-AS-User-Blocked-Sender: No X-TMASE-Result: 10--11.773300-8.000000 X-TMASE-Version: SMEX-12.5.0.1300-8.5.1010-24998.003 X-MDID: 1571928479-sDtXQm1Ky3N3 Subject: Re: [dpdk-dev] [PATCH v5 02/15] 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/24/19 11:29 AM, Ori Kam wrote: > Hi Andrew, > > When writing the new function I thought about using bool, but > I decided against it for the following reasons: > 1. There is no use of bool any where in the code, and there is not special reason to add it now. rte_ethdev.c includes stdbool.h and uses bool > 2. Other functions of this kind already returns int. for example (rte_eth_dev_is_valid_port / rte_eth_is_valid_owner_id) > > Thanks, > Ori > >> -----Original Message----- >> From: Andrew Rybchenko >> Sent: Thursday, October 24, 2019 10:55 AM >> To: Ori Kam ; Thomas Monjalon >> ; Ferruh Yigit >> Cc: dev@dpdk.org; jingjing.wu@intel.com; stephen@networkplumber.org >> Subject: Re: [dpdk-dev] [PATCH v5 02/15] ethdev: add support for hairpin queue >> >> Hi Ori, >> >> thanks for review notes applied. Please, see below. >> >> On 10/23/19 4:37 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 >> Just a bit below >> Reviewed-by: Andrew Rybchenko >> >> [snip] >> >>> diff --git a/lib/librte_ethdev/rte_ethdev.c b/lib/librte_ethdev/rte_ethdev.c >>> index 78da293..199e96e 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) == 1) { >> I think the function should return bool and it results should be >> used as is without == 1 or something like this. >> Everyrwhere in the patch. >> >> [snip] >> >>> diff --git a/lib/librte_ethdev/rte_ethdev_driver.h >> b/lib/librte_ethdev/rte_ethdev_driver.h >>> index c404f17..98023d7 100644 >>> --- a/lib/librte_ethdev/rte_ethdev_driver.h >>> +++ b/lib/librte_ethdev/rte_ethdev_driver.h >>> @@ -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 >> I think the function should return bool (and stdbool.h should be included). >> Return value description should be updated. >> >>> +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 >> Same here. >> >>> +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 >> [snip]