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 67646A00BE; Wed, 30 Oct 2019 07:39:18 +0100 (CET) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id 9774A1BEB6; Wed, 30 Oct 2019 07:39:17 +0100 (CET) Received: from dispatch1-us1.ppe-hosted.com (dispatch1-us1.ppe-hosted.com [67.231.154.164]) by dpdk.org (Postfix) with ESMTP id 45BD31BEB5 for ; Wed, 30 Oct 2019 07:39:16 +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 E6F01B80070; Wed, 30 Oct 2019 06:39:14 +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; Wed, 30 Oct 2019 06:39:08 +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: Wed, 30 Oct 2019 09:39:04 +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-25010.003 X-TM-AS-Result: No-9.340100-8.000000-10 X-TMASE-MatchedRID: eVEkOcJu0F7mLzc6AOD8DfHkpkyUphL9SoCG4sefl8RBbp4JobErAtgS kKeGM5sEhElS9On82DuOhzhlWjzNYnaIbEjxkzvhUyxW4vmvLt2ByxVkfd04JCBVkB/ybnTYCK1 n+RaAKL8aUiTYmfzyKc4LhbnE8G2ee3Q7TyoyQrX0UeM2i2l3EMG1o1Ri9o2EHApIKLQpe4SSsV tqQZCUs2mSdbU4Wxf1l+IrlvQnhmLUxcsh+ite3zdfT4zyWoZSWNbBpQ++I1m/btrChQPzdKKkU 0t47vZI5a2+qAkZtjBlUy4bNNqkKAtgJ854eHwOhL9NX2TqmkDJJ/setCfcGjqJ6+sGwm/bAYuf 5WdNtJVFleQSA5qYTJGTpe1iiCJq71zr0FZRMbBWdFebWIc3VsRB0bsfrpPIfiAqrjYtFiTEX9C d0jYOaj6AwSYo28bcEW0u3qdlvf+/PeRCK/KSUn7cGd19dSFd X-TM-AS-User-Approved-Sender: Yes X-TM-AS-User-Blocked-Sender: No X-TMASE-Result: 10--9.340100-8.000000 X-TMASE-Version: SMEX-12.5.0.1300-8.5.1010-25010.003 X-MDID: 1572417555-40INFixZiqqf 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" 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.